Shifts ‐ Structure

The shift app is a good representation of the state of Tapir's code: some of it is new, some of it dates back from the very beginning. Since we learned a lot since the creation of Tapir, the style is quite different between the old and the new parts. This page intends to define the style that we try to achieve and to show examples where it is well done and others where it's not.

Overall, we are going for a classic Model/View/Controller pattern, with the Controller being the Services.

Aim for thin model classes

Our shifts/models.py is over 1000 lines long. It would be nicer to have it smaller so that it's easier to get an overview of which what our models look like.

Model classes are already quite long with the minimum: field definitions, Meta classes, constants... Adding logic to them makes them too big, so we should put logic in dedicated service classes. Short helper functions are fine, for example ShiftSlot.get_display_name() is fine but ShiftSlot.user_can_attend() is too long and should be moved to a dedicated service class:

def get_display_name(self):
    display_name = self.shift.get_display_name()
    if self.name:
        display_name = "{} {}".format(self.name, display_name)    
    return display_name

def user_can_attend(self, user):
    return (
       # Slot must not be attended yet
       (
            not self.get_valid_attendance()
            or self.get_valid_attendance().state
            == ShiftAttendance.State.LOOKING_FOR_STAND_IN
        )
        and
        # User isn't already registered for this shift
        not self.shift.get_attendances()
        .filter(user=user)
        .with_valid_state()
        .exists()
        and
        # User must have all required capabilities
        set(self.required_capabilities).issubset(user.shift_user_data.capabilities)
        and self.shift.is_in_the_future()
        and not self.shift.cancelled
    )

Aim for thin view classes

Similarly to model classes, views already have a lot to them: defining permissions, forms, context data for the template... We compensated for that by making shifts/views a package instead of a file, but even with few classes per file, each class being long makes it difficult to have an overview. We should aim for thin view classes. For example, CreateShiftExemptionView.cancel_attendances_covered_by_exemption is a good example of a method that should be moved to a service class.

Logic goes in service classes

Putting logic in service classes allows us to thin our models and views, but they have other advantages:

• They allow us to put code around a single theme in a single place, instead of putting the different logic bits where they are called from.
• They let us name our logic blocks. For example, even if FrozenStatusService.should_freeze_member is only used in one place, having it in a function lets us name it and makes more explicit what that piece of code is about.
• They are a bit easier to test around because they are easier to build mocks for.