diff --git a/pyproject.toml b/pyproject.toml index c8eddfa2..5c7a6a85 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,12 +94,14 @@ exclude_lines = [ "def __str__", "def __unicode__", "def __repr__", + "if TYPE_CHECKING", ] omit = [ "*/migrations/*", "*/apps.py", "*/admin.py", "manage.py", + "timed/redmine/management/commands/import_project_data.py", "timed/settings_*.py", "timed/wsgi.py", "timed/forms.py", diff --git a/ruff.toml b/ruff.toml index 33f0169a..87feb6b5 100644 --- a/ruff.toml +++ b/ruff.toml @@ -11,27 +11,106 @@ docstring-code-format = true docstring-code-line-length = 88 [lint] -select = ["E", "F", "W", "I", "BLE", "T10"] +select = [ + "F", # pyflakes + "E", # pycodestyle errors + "I", # isort + "C90", # mccabe + "N", # pep8-naming + "D", # pydocstyle + "UP", # pyupgrade + "ANN", # flake8-annotations + "ASYNC", # flake8-async + "S", # flake8-bandit + "BLE", # flake8-blind-exception + "FBT", # flake8-boolean-trap + "B", # flake8-bugbear + "A", # flake8-builtins + "COM", # flake8-commas + "C4", # flake8-comprehensions + "T10", # flake8-debugger + "DJ", # flake8-django + "EM", # flake8-errmsg + "EXE", # flake8-executable + "FA", # flake8-future-annotations + "ISC", # flake8-implicit-str-concat + "ICN", # flake8-import-conventions + "G", # flake8-logging-format + "INP", # flake8-no-pep420 + "PIE", # flake8-pie + "T20", # flake8-print + "PYI", # flake8-pyi + "PT", # flake8-pytest-style + "Q", # flake8-quotes + "RSE", # flake8-raise + "RET", # flake8-return + "SLF", # flake8-self + "SLOT", # flake8-slots + "SIM", # flake8-simplify + "TID", # flake8-tidy-imports + "TCH", # flake8-type-checking + "INT", # flake8-gettext + "ARG", # flake8-unused-arguments + "PTH", # flake8-use-pathlib + "TD", # flake8-todos + "ERA", # eradicate + "PGH", # pygrep-hooks + "PL", # pylint + "TRY", # tryceratops + "PERF", # perflint + "RUF", # ruff specific rules + "W605", # invalid escape sequence +] ignore = [ - # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules - "W191", - "E111", - "E114", - "E117", - "E501", - "D206", - "D300", - "Q000", - "Q001", - "Q002", - "Q003", - "COM812", - "COM819", - "ISC001", - "ISC002" + "ANN101", # this is deprecated and annotating self is unnecessary + "D203", # we prefer blank-line-before-class (D211) for black compat + "D213", # we prefer multi-line-summary-first-line (D212) + "COM812", # ignore due to conflict with formatter + "ISC001", # ignore due to conflict with formatter + "E501", # managed by formatter + "TD002", # don't require author of TODO + "TD003", # don't require link to TODO + "D100", # don't enforce existance of docstrings + "D101", # don't enforce existance of docstrings + "D102", # don't enforce existance of docstrings + "D103", # don't enforce existance of docstrings + "D104", # don't enforce existance of docstrings + "D105", # don't enforce existance of docstrings + "D106", # don't enforce existance of docstrings + "D107", # don't enforce existance of docstrings + "DJ001", # null=True on string-based fields such as CharField (#1052) +] + +[lint.per-file-ignores] +"**/{conftest.py,tests/*.py}" = [ + "D", # pydocstyle is optional for tests + "ANN", # flake8-annotations are optional for tests + "S101", # assert is allow in tests + "S105", # tests may have hardcoded secrets + "S106", # tests may have hardcoded passwords + "S311", # tests may use pseudo-random generators + "S108", # /tmp is allowed in tests since it's expected to be mocked + "DTZ00", # tests often run in UTC + "INP001", # tests do not need a dunder init + "PLR0913", # tests can have a lot of arguments (fixtures) + "PLR2004", # tests can use magic values +] +"**/*/factories.py" = [ + "S311", # factories may use pseudo-random generators ] [lint.isort] known-first-party = ["timed"] known-third-party = ["pytest_factoryboy"] combine-as-imports = true + +[lint.flake8-annotations] +# TODO: drop this, its temporary +# https://github.com/adfinis/timed-backend/issues/1054 +ignore-fully-untyped = true + +[lint.flake8-unused-arguments] +ignore-variadic-names = true + +[lint.flake8-pytest-style] +fixture-parentheses = false \ No newline at end of file diff --git a/timed/authentication.py b/timed/authentication.py index e5f7dac7..491299ef 100644 --- a/timed/authentication.py +++ b/timed/authentication.py @@ -12,13 +12,10 @@ class TimedOIDCAuthenticationBackend(OIDCAuthenticationBackend): - def get_introspection(self, access_token, id_token, payload): + def get_introspection(self, access_token, _id_token, _payload): """Return user details dictionary.""" - basic = base64.b64encode( - f"{settings.OIDC_RP_INTROSPECT_CLIENT_ID}:{settings.OIDC_RP_INTROSPECT_CLIENT_SECRET}".encode( - "utf-8" - ) + f"{settings.OIDC_RP_INTROSPECT_CLIENT_ID}:{settings.OIDC_RP_INTROSPECT_CLIENT_SECRET}".encode() ).decode() headers = { "Authorization": f"Basic {basic}", @@ -29,19 +26,17 @@ def get_introspection(self, access_token, id_token, payload): verify=settings.OIDC_VERIFY_SSL, headers=headers, data={"token": access_token}, + timeout=10, ) response.raise_for_status() return response.json() def get_userinfo_or_introspection(self, access_token): try: - claims = self.cached_request( - self.get_userinfo, access_token, "auth.userinfo" - ) - return claims - except requests.HTTPError as e: - if e.response.status_code not in [401, 403]: - raise e + return self.cached_request(self.get_userinfo, access_token, "auth.userinfo") + except requests.HTTPError as exc: + if exc.response.status_code not in [401, 403]: + raise if settings.OIDC_CHECK_INTROSPECT: try: # check introspection if userinfo fails (confidential client) @@ -49,22 +44,21 @@ def get_userinfo_or_introspection(self, access_token): self.get_introspection, access_token, "auth.introspection" ) if "client_id" not in claims: - raise SuspiciousOperation( - "client_id not present in introspection" - ) - return claims + msg = "client_id not present in introspection" + raise SuspiciousOperation(msg) except requests.HTTPError as e: # if the authorization fails it's not a valid client or # the token is expired and permission is denied. # Handing on the 401 Client Error would be transformed into # a 500 by Django's exception handling. But that's not what we want. if e.response.status_code not in [401, 403]: # pragma: no cover - raise e - raise AuthenticationFailed() + raise + else: + return claims + raise AuthenticationFailed from exc - def get_or_create_user(self, access_token, id_token, payload): + def get_or_create_user(self, access_token, _id_token, _payload): """Verify claims and return user, otherwise raise an Exception.""" - claims = self.get_userinfo_or_introspection(access_token) users = self.filter_users_by_claims(claims) @@ -73,15 +67,14 @@ def get_or_create_user(self, access_token, id_token, payload): user = users.get() self.update_user_from_claims(user, claims) return user - elif settings.OIDC_CREATE_USER: + if settings.OIDC_CREATE_USER: return self.create_user(claims) - else: - LOGGER.debug( - "Login failed: No user with username %s found, and " - "OIDC_CREATE_USER is False", - self.get_username(claims), - ) - return None + LOGGER.debug( + "Login failed: No user with username %s found, and " + "OIDC_CREATE_USER is False", + self.get_username(claims), + ) + return None def update_user_from_claims(self, user, claims): user.email = claims.get(settings.OIDC_EMAIL_CLAIM, "") @@ -106,7 +99,6 @@ def cached_request(self, method, token, cache_prefix): def create_user(self, claims): """Return object for a newly created user account.""" - username = self.get_username(claims) email = claims.get(settings.OIDC_EMAIL_CLAIM, "") first_name = claims.get(settings.OIDC_FIRSTNAME_CLAIM, "") @@ -119,5 +111,6 @@ def create_user(self, claims): def get_username(self, claims): try: return claims[settings.OIDC_USERNAME_CLAIM] - except KeyError: - raise SuspiciousOperation("Couldn't find username claim") + except KeyError as exc: + msg = "Couldn't find username claim" + raise SuspiciousOperation(msg) from exc diff --git a/timed/conftest.py b/timed/conftest.py index 725c8223..526996a0 100644 --- a/timed/conftest.py +++ b/timed/conftest.py @@ -14,8 +14,8 @@ def register_module(module): - for name, obj in inspect.getmembers(module): - if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract: + for _name, obj in inspect.getmembers(module): + if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract: # noqa: SLF001 register(obj) @@ -26,7 +26,7 @@ def register_module(module): @pytest.fixture -def auth_user(db): +def auth_user(db): # noqa: ARG001 return get_user_model().objects.create_user( username="user", password="123qweasd", @@ -38,7 +38,7 @@ def auth_user(db): @pytest.fixture -def admin_user(db): +def admin_user(db): # noqa: ARG001 return get_user_model().objects.create_user( username="admin", password="123qweasd", @@ -50,7 +50,7 @@ def admin_user(db): @pytest.fixture -def superadmin_user(db): +def superadmin_user(db): # noqa: ARG001 return get_user_model().objects.create_user( username="superadmin", password="123qweasd", @@ -62,7 +62,7 @@ def superadmin_user(db): @pytest.fixture -def external_employee(db): +def external_employee(db): # noqa: ARG001 user = get_user_model().objects.create_user( username="user", password="123qweasd", @@ -76,7 +76,7 @@ def external_employee(db): @pytest.fixture -def internal_employee(db): +def internal_employee(db): # noqa: ARG001 user = get_user_model().objects.create_user( username="user", password="123qweasd", @@ -140,7 +140,7 @@ def internal_employee_client(internal_employee): return client -@pytest.fixture(scope="function", autouse=True) +@pytest.fixture(autouse=True) def _autoclear_cache(): cache.clear() @@ -148,8 +148,7 @@ def _autoclear_cache(): def setup_customer_and_employment_status( user, is_assignee, is_customer, is_employed, is_external ): - """ - Set up customer and employment status. + """Set up customer and employment status. Return a 2-tuple of assignee and employment, if they were created diff --git a/timed/employment/admin.py b/timed/employment/admin.py index 436beae5..aa683f08 100644 --- a/timed/employment/admin.py +++ b/timed/employment/admin.py @@ -32,7 +32,7 @@ class SupervisorForm(forms.ModelForm): class Meta: """Meta information for the supervisor form.""" - fields = "__all__" + fields = "__all__" # noqa: DJ007 model = models.User.supervisors.through @@ -51,12 +51,12 @@ class SuperviseeForm(forms.ModelForm): class Meta: """Meta information for the supervisee form.""" - fields = "__all__" + fields = "__all__" # noqa: DJ007 model = models.User.supervisors.through class SupervisorInline(admin.TabularInline): - autocomplete_fields = ["to_user"] + autocomplete_fields = ("to_user",) form = SupervisorForm model = models.User.supervisors.through extra = 0 @@ -66,7 +66,7 @@ class SupervisorInline(admin.TabularInline): class SuperviseeInline(admin.TabularInline): - autocomplete_fields = ["from_user"] + autocomplete_fields = ("from_user",) form = SuperviseeForm model = models.User.supervisors.through extra = 0 @@ -80,15 +80,11 @@ class EmploymentForm(forms.ModelForm): worktime_per_day = DurationInHoursField(label=_("Worktime per day in hours")) - def clean(self): + def clean(self) -> dict: """Validate the employment as a whole. Ensure the end date is after the start date and there is only one active employment per user and there are no overlapping employments. - - :throws: django.core.exceptions.ValidationError - :return: The cleaned data - :rtype: dict """ data = super().clean() @@ -101,11 +97,9 @@ def clean(self): raise ValidationError(_("The end date must be after the start date")) if any( - [ - e.start_date <= (data.get("end_date") or datetime.date.today()) - and data.get("start_date") <= (e.end_date or datetime.date.today()) - for e in employments - ] + e.start_date <= (data.get("end_date") or datetime.date.today()) + and data.get("start_date") <= (e.end_date or datetime.date.today()) + for e in employments ): raise ValidationError( _("A user can't have multiple employments at the same time") @@ -116,7 +110,7 @@ def clean(self): class Meta: """Meta information for the employment form.""" - fields = "__all__" + fields = "__all__" # noqa: DJ007 model = models.Employment @@ -146,22 +140,28 @@ class AbsenceCreditInline(admin.TabularInline): class UserAdmin(UserAdmin): """Timed specific user admin.""" - inlines = [ + inlines = ( SupervisorInline, SuperviseeInline, EmploymentInline, OvertimeCreditInline, AbsenceCreditInline, - ] - list_display = ("username", "first_name", "last_name", "is_staff", "is_active") - search_fields = ["username"] + ) + list_display = ( + "username", + "first_name", + "last_name", + "is_staff", + "is_active", + ) + search_fields = ("username",) - actions = [ + actions = ( "disable_users", "enable_users", "disable_staff_status", "enable_staff_status", - ] + ) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -169,27 +169,27 @@ def __init__(self, *args, **kwargs): (_("Extra fields"), {"fields": ["tour_done", "is_accountant"]}), ) - def disable_users(self, request, queryset): + def disable_users(self, _request, queryset): queryset.update(is_active=False) disable_users.short_description = _("Disable selected users") - def enable_users(self, request, queryset): + def enable_users(self, _request, queryset): queryset.update(is_active=True) enable_users.short_description = _("Enable selected users") - def disable_staff_status(self, request, queryset): + def disable_staff_status(self, _request, queryset): queryset.update(is_staff=False) disable_staff_status.short_description = _("Disable staff status of selected users") - def enable_staff_status(self, request, queryset): + def enable_staff_status(self, _request, queryset): queryset.update(is_staff=True) enable_staff_status.short_description = _("Enable staff status of selected users") - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.reports.exists() @@ -197,10 +197,10 @@ def has_delete_permission(self, request, obj=None): class LocationAdmin(admin.ModelAdmin): """Location admin view.""" - list_display = ["name"] - search_fields = ["name"] + list_display = ("name",) + search_fields = ("name",) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.employments.exists() @@ -208,15 +208,19 @@ def has_delete_permission(self, request, obj=None): class PublicHolidayAdmin(admin.ModelAdmin): """Public holiday admin view.""" - list_display = ["__str__", "date", "location"] - list_filter = ["location"] + list_display = ( + "__str__", + "date", + "location", + ) + list_filter = ("location",) @admin.register(models.AbsenceType) class AbsenceTypeAdmin(admin.ModelAdmin): """Absence type admin view.""" - list_display = ["name"] + list_display = ("name",) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.absences.exists() and not obj.absencecredit_set.exists() diff --git a/timed/employment/factories.py b/timed/employment/factories.py index 2fd347ac..667c8fa7 100644 --- a/timed/employment/factories.py +++ b/timed/employment/factories.py @@ -60,12 +60,8 @@ class EmploymentFactory(DjangoModelFactory): is_external = False @lazy_attribute - def worktime_per_day(self): - """Generate the worktime per day based on the percentage. - - :return: The generated worktime - :rtype: datetime.timedelta - """ + def worktime_per_day(self) -> datetime.timedelta: + """Generate the worktime per day based on the percentage.""" return datetime.timedelta(minutes=60 * 8.5 * self.percentage / 100) class Meta: @@ -107,12 +103,8 @@ class OvertimeCreditFactory(DjangoModelFactory): date = Faker("date_object") @lazy_attribute - def duration(self): - """Generate a random duration. - - :return: The generated duration - :rtype: datetime.timedelta - """ + def duration(self) -> datetime.timedelta: + """Generate a random duration.""" return datetime.timedelta(hours=random.randint(5, 40)) class Meta: diff --git a/timed/employment/filters.py b/timed/employment/filters.py index c74a8b13..30869cfe 100644 --- a/timed/employment/filters.py +++ b/timed/employment/filters.py @@ -27,7 +27,13 @@ class Meta: """Meta information for the public holiday filter set.""" model = models.PublicHoliday - fields = ["year", "location", "date", "from_date", "to_date"] + fields = ( + "year", + "location", + "date", + "from_date", + "to_date", + ) class AbsenceTypeFilterSet(FilterSet): @@ -37,7 +43,7 @@ class Meta: """Meta information for the public holiday filter set.""" model = models.AbsenceType - fields = ["fill_worktime"] + fields = ("fill_worktime",) class UserFilterSet(FilterSet): @@ -48,44 +54,45 @@ class UserFilterSet(FilterSet): is_accountant = NumberFilter(field_name="is_accountant") is_external = NumberFilter(method="filter_is_external") - def filter_is_external(self, queryset, name, value): + def filter_is_external(self, queryset, _name, value): return queryset.filter(employments__is_external=value) - def filter_is_reviewer(self, queryset, name, value): + def filter_is_reviewer(self, queryset, _name, value): if value: return queryset.filter(pk__in=User.objects.all_reviewers()) return queryset.exclude(pk__in=User.objects.all_reviewers()) - def filter_is_supervisor(self, queryset, name, value): + def filter_is_supervisor(self, queryset, _name, value): if value: return queryset.filter(pk__in=User.objects.all_supervisors()) return queryset.exclude(pk__in=User.objects.all_supervisors()) class Meta: model = models.User - fields = [ + fields = ( "active", "supervisor", "is_reviewer", "is_supervisor", "is_accountant", - ] + ) class EmploymentFilterSet(FilterSet): date = DateFilter(method="filter_date") - def filter_date(self, queryset, name, value): - queryset = queryset.filter( + def filter_date(self, queryset, _name, value): + return queryset.filter( Q(start_date__lte=value) & Q(Q(end_date__gte=value) | Q(end_date__isnull=True)) ) - return queryset - class Meta: model = models.Employment - fields = ["user", "location"] + fields = ( + "user", + "location", + ) class OvertimeCreditFilterSet(FilterSet): @@ -95,7 +102,13 @@ class OvertimeCreditFilterSet(FilterSet): class Meta: model = models.OvertimeCredit - fields = ["year", "user", "date", "from_date", "to_date"] + fields = ( + "year", + "user", + "date", + "from_date", + "to_date", + ) class AbsenceCreditFilterSet(FilterSet): @@ -105,19 +118,23 @@ class AbsenceCreditFilterSet(FilterSet): class Meta: model = models.AbsenceCredit - fields = ["year", "user", "date", "from_date", "to_date", "absence_type"] + fields = ( + "year", + "user", + "date", + "from_date", + "to_date", + "absence_type", + ) class WorktimeBalanceFilterSet(FilterSet): user = NumberFilter(field_name="id") supervisor = NumberFilter(field_name="supervisors") - # additional filters analyzed in WorktimeBalanceView - # date = DateFilter() - # last_reported_date = NumberFilter() class Meta: model = models.User - fields = ["user"] + fields = ("user",) class AbsenceBalanceFilterSet(FilterSet): @@ -125,4 +142,4 @@ class AbsenceBalanceFilterSet(FilterSet): class Meta: model = models.AbsenceType - fields = ["absence_type"] + fields = ("absence_type",) diff --git a/timed/employment/models.py b/timed/employment/models.py index 8e54db54..704be41a 100644 --- a/timed/employment/models.py +++ b/timed/employment/models.py @@ -27,17 +27,13 @@ class Location(models.Model): Workdays defined per location, default is Monday - Friday """ - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return self.name - class Meta: ordering = ("name",) + def __str__(self) -> str: + """Represent the model as a string.""" + return self.name + class PublicHoliday(models.Model): """Public holiday model. @@ -52,20 +48,16 @@ class PublicHoliday(models.Model): Location, on_delete=models.CASCADE, related_name="public_holidays" ) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return "{0} {1}".format(self.name, self.date.strftime("%Y")) - class Meta: """Meta information for the public holiday model.""" - indexes = [models.Index(fields=["date"])] + indexes = (models.Index(fields=["date"]),) ordering = ("date",) + def __str__(self) -> str: + """Represent the model as a string.""" + return "{} {}".format(self.name, self.date.strftime("%Y")) + class AbsenceType(models.Model): """Absence type model. @@ -77,34 +69,29 @@ class AbsenceType(models.Model): name = models.CharField(max_length=50) fill_worktime = models.BooleanField(default=False) - def __str__(self): - """Represent the model as a string. + class Meta: + ordering = ("name",) - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return self.name def calculate_credit(self, user, start, end): - """ - Calculate approved days of type for user in given time frame. + """Calculate approved days of type for user in given time frame. For absence types which fill worktime this will be None. """ if self.fill_worktime: return None - credits = AbsenceCredit.objects.filter( + credits = AbsenceCredit.objects.filter( # noqa: A001 user=user, absence_type=self, date__range=[start, end] ) data = credits.aggregate(credit=Sum("days")) - credit = data["credit"] or 0 - - return credit + return data["credit"] or 0 def calculate_used_days(self, user, start, end): - """ - Calculate used days of type for user in given time frame. + """Calculate used days of type for user in given time frame. For absence types which fill worktime this will be None. """ @@ -114,11 +101,7 @@ def calculate_used_days(self, user, start, end): absences = Absence.objects.filter( user=user, absence_type=self, date__range=[start, end] ) - used_days = absences.count() - return used_days - - class Meta: - ordering = ("name",) + return absences.count() class AbsenceCredit(models.Model): @@ -143,6 +126,9 @@ class AbsenceCredit(models.Model): Mark whether this absence credit is a transfer from last year. """ + def __str__(self) -> str: + return f"{self.user.username} ({self.duration})" + class OvertimeCredit(models.Model): """Overtime credit model. @@ -164,6 +150,9 @@ class OvertimeCredit(models.Model): Mark whether this absence credit is a transfer from last year. """ + def __str__(self) -> str: + return f"{self.user.username} ({self.duration})" + class EmploymentManager(models.Manager): """Custom manager for employments.""" @@ -219,19 +208,21 @@ class Employment(models.Model): worktime_per_day = models.DurationField() start_date = models.DateField() end_date = models.DateField(blank=True, null=True) - objects = EmploymentManager() added = models.DateTimeField(auto_now_add=True) updated = models.DateTimeField(auto_now=True) is_external = models.BooleanField(default=False) - def __str__(self): - """Represent the model as a string. + objects = EmploymentManager() - :return: The string representation - :rtype: str - """ - return "{0} ({1} - {2})".format( + class Meta: + """Meta information for the employment model.""" + + indexes = (models.Index(fields=["start_date", "end_date"]),) + + def __str__(self) -> str: + """Represent the model as a string.""" + return "{} ({} - {})".format( self.user.username, self.start_date.strftime("%d.%m.%Y"), self.end_date.strftime("%d.%m.%Y") if self.end_date else "today", @@ -274,7 +265,7 @@ def calculate_worktime(self, start, end): # converting workdays as db expects 1 (Sunday) to 7 (Saturday) workdays_db = [ # special case for Sunday - int(day) == 7 and 1 or int(day) + 1 + int(day) == 7 and 1 or int(day) + 1 # noqa: PLR2004 for day in self.location.workdays ] holidays = PublicHoliday.objects.filter( @@ -310,11 +301,6 @@ def calculate_worktime(self, start, end): return (reported, expected_worktime, reported - expected_worktime) - class Meta: - """Meta information for the employment model.""" - - indexes = [models.Index(fields=["start_date", "end_date"])] - class UserManager(UserManager): def all_supervisors(self): @@ -410,7 +396,6 @@ def get_active_employment(self): If the user doesn't have a return None. """ try: - current_employment = Employment.objects.get_at(user=self, date=date.today()) - return current_employment + return Employment.objects.get_at(user=self, date=date.today()) except Employment.DoesNotExist: return None diff --git a/timed/employment/permissions.py b/timed/employment/permissions.py index 218b5f4d..f363e5c1 100644 --- a/timed/employment/permissions.py +++ b/timed/employment/permissions.py @@ -2,5 +2,5 @@ class NoReports(BasePermission): - def has_object_permission(self, request, view, obj): + def has_object_permission(self, _view, _request, obj): return not obj.reports.exists() diff --git a/timed/employment/serializers.py b/timed/employment/serializers.py index 0a4490a0..fd097c9f 100644 --- a/timed/employment/serializers.py +++ b/timed/employment/serializers.py @@ -1,6 +1,9 @@ """Serializers for the employment app.""" +from __future__ import annotations + from datetime import date, timedelta +from typing import TYPE_CHECKING from django.contrib.auth import get_user_model from django.db.models import Max, Value @@ -18,9 +21,12 @@ from timed.employment import models from timed.tracking.models import Absence, Report +if TYPE_CHECKING: + from typing import ClassVar + class UserSerializer(ModelSerializer): - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "supervisors": "timed.employment.serializers.UserSerializer", "supervisees": "timed.employment.serializers.UserSerializer", } @@ -29,7 +35,7 @@ class Meta: """Meta information for the user serializer.""" model = get_user_model() - fields = [ + fields = ( "email", "first_name", "is_active", @@ -42,8 +48,8 @@ class Meta: "username", "is_reviewer", "is_accountant", - ] - read_only_fields = [ + ) + read_only_fields = ( "first_name", "is_active", "is_staff", @@ -54,7 +60,7 @@ class Meta: "username", "is_reviewer", "is_accountant", - ] + ) class WorktimeBalanceSerializer(Serializer): @@ -94,7 +100,9 @@ def get_balance(self, instance): _, _, balance = instance.id.calculate_worktime(start, balance_date) return duration_string(balance) - included_serializers = {"user": "timed.employment.serializers.UserSerializer"} + included_serializers: ClassVar[dict[str, str]] = { + "user": "timed.employment.serializers.UserSerializer" + } class Meta: resource_name = "worktime-balances" @@ -123,8 +131,7 @@ def _get_start(self, instance): return date(instance.date.year, 1, 1) def get_credit(self, instance): - """ - Calculate how many days are approved for given absence type. + """Calculate how many days are approved for given absence type. For absence types which fill worktime this will be None. """ @@ -143,8 +150,7 @@ def get_credit(self, instance): return instance["credit"] def get_used_days(self, instance): - """ - Calculate how many days are used of given absence type. + """Calculate how many days are used of given absence type. For absence types which fill worktime this will be None. """ @@ -163,8 +169,7 @@ def get_used_days(self, instance): return instance["used_days"] def get_used_duration(self, instance): - """ - Calculate duration of absence type. + """Calculate duration of absence type. For absence types which fill worktime this will be None. """ @@ -217,7 +222,7 @@ def get_balance(self, instance): return self.get_credit(instance) - self.get_used_days(instance) - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "absence_type": "timed.employment.serializers.AbsenceTypeSerializer", "absence_credits": "timed.employment.serializers.AbsenceCreditSerializer", } @@ -227,20 +232,16 @@ class Meta: class EmploymentSerializer(ModelSerializer): - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "location": "timed.employment.serializers.LocationSerializer", } - def validate(self, data): + def validate(self, data: dict) -> dict: """Validate the employment as a whole. Ensure the end date is after the start date and there is only one active employment per user and there are no overlapping employments. - - :throws: django.core.exceptions.ValidationError - :return: validated data - :rtype: dict """ instance = self.instance start_date = data.get("start_date", instance and instance.start_date) @@ -258,7 +259,7 @@ def validate(self, data): if instance: employments = employments.exclude(id=instance.id) - if any([e.start_date <= end_date and start_date <= e.end for e in employments]): + if any(e.start_date <= end_date and start_date <= e.end for e in employments): raise ValidationError( _("A user can't have multiple employments at the same time") ) @@ -267,7 +268,7 @@ def validate(self, data): class Meta: model = models.Employment - fields = [ + fields = ( "user", "location", "percentage", @@ -275,7 +276,7 @@ class Meta: "start_date", "end_date", "is_external", - ] + ) class LocationSerializer(ModelSerializer): @@ -285,7 +286,10 @@ class Meta: """Meta information for the location serializer.""" model = models.Location - fields = ["name", "workdays"] + fields = ( + "name", + "workdays", + ) class PublicHolidaySerializer(ModelSerializer): @@ -293,7 +297,7 @@ class PublicHolidaySerializer(ModelSerializer): location = relations.ResourceRelatedField(read_only=True) - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "location": "timed.employment.serializers.LocationSerializer" } @@ -301,7 +305,11 @@ class Meta: """Meta information for the public holiday serializer.""" model = models.PublicHoliday - fields = ["name", "date", "location"] + fields = ( + "name", + "date", + "location", + ) class AbsenceTypeSerializer(ModelSerializer): @@ -311,13 +319,16 @@ class Meta: """Meta information for the absence type serializer.""" model = models.AbsenceType - fields = ["name", "fill_worktime"] + fields = ( + "name", + "fill_worktime", + ) class AbsenceCreditSerializer(ModelSerializer): """Absence credit serializer.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "absence_type": "timed.employment.serializers.AbsenceTypeSerializer" } @@ -325,10 +336,23 @@ class Meta: """Meta information for the absence credit serializer.""" model = models.AbsenceCredit - fields = ["user", "absence_type", "date", "days", "comment", "transfer"] + fields = ( + "user", + "absence_type", + "date", + "days", + "comment", + "transfer", + ) class OvertimeCreditSerializer(ModelSerializer): class Meta: model = models.OvertimeCredit - fields = ["user", "date", "duration", "comment", "transfer"] + fields = ( + "user", + "date", + "duration", + "comment", + "transfer", + ) diff --git a/timed/employment/tests/__init__.py b/timed/employment/tests/__init__.py index 6e031999..e69de29b 100644 --- a/timed/employment/tests/__init__.py +++ b/timed/employment/tests/__init__.py @@ -1 +0,0 @@ -# noqa: D104 diff --git a/timed/employment/tests/test_absence_balance.py b/timed/employment/tests/test_absence_balance.py index df776043..2d72b956 100644 --- a/timed/employment/tests/test_absence_balance.py +++ b/timed/employment/tests/test_absence_balance.py @@ -47,7 +47,7 @@ def test_absence_balance_full_day(auth_client, django_assert_num_queries): assert len(json["data"]) == 1 entry = json["data"][0] - assert entry["id"] == "{0}_{1}_2017-03-01".format(user.id, absence_type.id) + assert entry["id"] == f"{user.id}_{absence_type.id}_2017-03-01" assert entry["attributes"]["credit"] == 5 assert entry["attributes"]["used-days"] == 2 assert entry["attributes"]["used-duration"] is None @@ -92,7 +92,7 @@ def test_absence_balance_fill_worktime(auth_client, django_assert_num_queries): assert len(json["data"]) == 1 entry = json["data"][0] - assert entry["id"] == "{0}_{1}_2017-03-01".format(user.id, absence_type.id) + assert entry["id"] == f"{user.id}_{absence_type.id}_2017-03-01" assert entry["attributes"]["credit"] is None assert entry["attributes"]["balance"] is None @@ -105,7 +105,7 @@ def test_absence_balance_detail(auth_client): absence_type = AbsenceTypeFactory.create() url = reverse( "absence-balance-detail", - args=["{0}_{1}_2017-03-01".format(user.id, absence_type.id)], + args=[f"{user.id}_{absence_type.id}_2017-03-01"], ) result = auth_client.get(url) @@ -139,7 +139,7 @@ def test_absence_balance_detail_none_supervisee(auth_client): url = reverse( "absence-balance-detail", - args=["{0}_{1}_2017-03-01".format(unrelated_user.id, absence_type.id)], + args=[f"{unrelated_user.id}_{absence_type.id}_2017-03-01"], ) result = auth_client.get(url) diff --git a/timed/employment/tests/test_absence_type.py b/timed/employment/tests/test_absence_type.py index ad681122..440fa891 100644 --- a/timed/employment/tests/test_absence_type.py +++ b/timed/employment/tests/test_absence_type.py @@ -7,7 +7,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected", + ("is_employed", "is_customer_assignee", "is_customer", "expected"), [ (False, True, True, 0), (False, True, False, 0), @@ -51,7 +51,7 @@ def test_absence_type_list_filter_fill_worktime(internal_employee_client): @pytest.mark.parametrize( - "is_employed, expected", + ("is_employed", "expected"), [ (True, status.HTTP_200_OK), (False, status.HTTP_404_NOT_FOUND), diff --git a/timed/employment/tests/test_employment.py b/timed/employment/tests/test_employment.py index 97ea8f73..06d24e92 100644 --- a/timed/employment/tests/test_employment.py +++ b/timed/employment/tests/test_employment.py @@ -136,29 +136,32 @@ def test_employment_list_supervisor(auth_client): assert len(json["data"]) == 2 -def test_employment_unique_active(db): +@pytest.mark.django_db() +def test_employment_unique_active(): """Should only be able to have one active employment per user.""" user = UserFactory.create() EmploymentFactory.create(user=user, end_date=None) employment = EmploymentFactory.create(user=user) form = EmploymentForm({"end_date": None}, instance=employment) - with pytest.raises(ValueError): + with pytest.raises(ValueError): # noqa: PT011 form.save() -def test_employment_start_before_end(db): +@pytest.mark.django_db() +def test_employment_start_before_end(): employment = EmploymentFactory.create() form = EmploymentForm( {"start_date": date(2009, 1, 1), "end_date": date(2016, 1, 1)}, instance=employment, ) - with pytest.raises(ValueError): + with pytest.raises(ValueError): # noqa: PT011 form.save() -def test_employment_get_at(db): +@pytest.mark.django_db() +def test_employment_get_at(): """Should return the right employment on a date.""" user = UserFactory.create() employment = EmploymentFactory.create(user=user) @@ -173,7 +176,8 @@ def test_employment_get_at(db): Employment.objects.get_at(user, employment.start_date + timedelta(days=21)) -def test_worktime_balance_partial(db): +@pytest.mark.django_db() +def test_worktime_balance_partial(): """ Test partial calculation of worktime balance. @@ -219,7 +223,8 @@ def test_worktime_balance_partial(db): assert balance == expected_balance -def test_worktime_balance_longer(db): +@pytest.mark.django_db() +def test_worktime_balance_longer(): """Test calculation of worktime when frame is longer than employment.""" employment = factories.EmploymentFactory.create( start_date=date(2017, 3, 21), @@ -272,7 +277,8 @@ def test_worktime_balance_longer(db): assert balance == expected_balance -def test_employment_for_user(db): +@pytest.mark.django_db() +def test_employment_for_user(): user = factories.UserFactory.create() # employment overlapping time frame (early start) factories.EmploymentFactory.create( diff --git a/timed/employment/tests/test_location.py b/timed/employment/tests/test_location.py index 5685afda..ec0e8cca 100644 --- a/timed/employment/tests/test_location.py +++ b/timed/employment/tests/test_location.py @@ -6,8 +6,10 @@ from timed.employment.factories import EmploymentFactory, LocationFactory +@pytest.mark.django_db() +@pytest.mark.usefixtures("location") @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected", + ("is_employed", "is_customer_assignee", "is_customer", "expected"), [ (False, True, True, 0), (False, True, False, 0), @@ -17,7 +19,7 @@ ], ) def test_location_list( - auth_client, is_employed, is_customer_assignee, is_customer, expected, location + auth_client, is_employed, is_customer_assignee, is_customer, expected ): setup_customer_and_employment_status( user=auth_client.user, @@ -38,7 +40,7 @@ def test_location_list( @pytest.mark.parametrize( - "is_employed, expected", + ("is_employed", "expected"), [ (True, status.HTTP_200_OK), (False, status.HTTP_404_NOT_FOUND), diff --git a/timed/employment/tests/test_public_holiday.py b/timed/employment/tests/test_public_holiday.py index 03325287..4a19dd85 100644 --- a/timed/employment/tests/test_public_holiday.py +++ b/timed/employment/tests/test_public_holiday.py @@ -9,7 +9,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected", + ("is_employed", "is_customer_assignee", "is_customer", "expected"), [ (False, True, True, 0), (False, True, False, 0), @@ -39,7 +39,7 @@ def test_public_holiday_list( @pytest.mark.parametrize( - "is_employed, expected", + ("is_employed", "expected"), [ (True, status.HTTP_200_OK), (False, status.HTTP_404_NOT_FOUND), diff --git a/timed/employment/tests/test_user.py b/timed/employment/tests/test_user.py index 136e62a2..4ecb970e 100644 --- a/timed/employment/tests/test_user.py +++ b/timed/employment/tests/test_user.py @@ -23,14 +23,16 @@ def test_user_list_unauthenticated(client): assert response.status_code == status.HTTP_401_UNAUTHORIZED -def test_user_update_unauthenticated(client, db): +@pytest.mark.django_db() +def test_user_update_unauthenticated(client): user = UserFactory.create() url = reverse("user-detail", args=[user.id]) response = client.patch(url) assert response.status_code == status.HTTP_401_UNAUTHORIZED -def test_user_list(db, internal_employee_client, django_assert_num_queries): +@pytest.mark.django_db() +def test_user_list(internal_employee_client, django_assert_num_queries): UserFactory.create_batch(2) url = reverse("user-list") @@ -144,7 +146,8 @@ def test_user_delete_superuser(superadmin_client): assert response.status_code == status.HTTP_204_NO_CONTENT -def test_user_delete_with_reports_superuser(superadmin_client, db): +@pytest.mark.django_db() +def test_user_delete_with_reports_superuser(superadmin_client): """Test that user with reports may not be deleted.""" user = UserFactory.create() ReportFactory.create(user=user) @@ -205,7 +208,7 @@ def test_user_transfer(superadmin_client): assert absence_credit.comment == "Transfer 2017" -@pytest.mark.parametrize("value,expected", [(1, 2), (0, 2)]) +@pytest.mark.parametrize(("value", "expected"), [(1, 2), (0, 2)]) def test_user_is_external_filter(internal_employee_client, value, expected): """Should filter users if they have an internal employment.""" user = UserFactory.create() @@ -220,7 +223,7 @@ def test_user_is_external_filter(internal_employee_client, value, expected): assert len(response.json()["data"]) == expected -@pytest.mark.parametrize("value,expected", [(1, 1), (0, 4)]) +@pytest.mark.parametrize(("value", "expected"), [(1, 1), (0, 4)]) def test_user_is_reviewer_filter(internal_employee_client, value, expected): """Should filter users if they are a reviewer.""" user = UserFactory.create() @@ -232,7 +235,7 @@ def test_user_is_reviewer_filter(internal_employee_client, value, expected): assert len(res.json()["data"]) == expected -@pytest.mark.parametrize("value,expected", [(1, 1), (0, 5)]) +@pytest.mark.parametrize(("value", "expected"), [(1, 1), (0, 5)]) def test_user_is_supervisor_filter(internal_employee_client, value, expected): """Should filter useres if they are a supervisor.""" users = UserFactory.create_batch(2) @@ -286,7 +289,7 @@ def test_user_me_anonymous(client): @pytest.mark.parametrize( - "is_customer, expected, status_code", + ("is_customer", "expected", "status_code"), [(True, 1, status.HTTP_200_OK), (False, 0, status.HTTP_403_FORBIDDEN)], ) def test_user_list_no_employment(auth_client, is_customer, expected, status_code): diff --git a/timed/employment/tests/test_worktime_balance.py b/timed/employment/tests/test_worktime_balance.py index 4db21dde..da4c4223 100644 --- a/timed/employment/tests/test_worktime_balance.py +++ b/timed/employment/tests/test_worktime_balance.py @@ -34,7 +34,7 @@ def test_worktime_balance_no_employment(auth_client, django_assert_num_queries): json = result.json() assert len(json["data"]) == 1 data = json["data"][0] - assert data["id"] == "{0}_2017-01-01".format(auth_client.user.id) + assert data["id"] == f"{auth_client.user.id}_2017-01-01" assert data["attributes"]["balance"] == "00:00:00" @@ -86,7 +86,7 @@ def test_worktime_balance_with_employments(auth_client, django_assert_num_querie url = reverse( "worktime-balance-detail", - args=["{0}_{1}".format(auth_client.user.id, end_date.strftime("%Y-%m-%d"))], + args=["{}_{}".format(auth_client.user.id, end_date.strftime("%Y-%m-%d"))], ) with django_assert_num_queries(11): diff --git a/timed/employment/views.py b/timed/employment/views.py index 712c52be..9548aae6 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -30,27 +30,32 @@ class UserViewSet(ModelViewSet): - """ - Expose user actions. + """Expose user actions. Users are managed in admin therefore this end point only allows retrieving and updating. """ - permission_classes = [ - # only owner, superuser and supervisor may update user - (IsOwner | IsSuperUser | IsSupervisor) & IsUpdateOnly - # only superuser may delete users without reports - | IsSuperUser & IsDeleteOnly & NoReports - # only superuser may create users - | IsSuperUser & IsCreateOnly - # all authenticated users may read - | IsAuthenticated & IsReadOnly - ] + permission_classes = ( + ( + # only owner, superuser and supervisor may update user + (IsOwner | IsSuperUser | IsSupervisor) & IsUpdateOnly + # only superuser may delete users without reports + | IsSuperUser & IsDeleteOnly & NoReports + # only superuser may create users + | IsSuperUser & IsCreateOnly + # all authenticated users may read + | IsAuthenticated & IsReadOnly + ), + ) serializer_class = serializers.UserSerializer filterset_class = filters.UserFilterSet - search_fields = ("username", "first_name", "last_name") + search_fields = ( + "username", + "first_name", + "last_name", + ) def get_queryset(self): user = self.request.user @@ -62,24 +67,6 @@ def get_queryset(self): current_employment = models.Employment.objects.get_at( user=user, date=datetime.date.today() ) - if current_employment.is_external: - assigned_tasks = Task.objects.filter( - Q(task_assignees__user=user, task_assignees__is_reviewer=True) - | Q( - project__project_assignees__user=user, - project__project_assignees__is_reviewer=True, - ) - | Q( - project__customer__customer_assignees__user=user, - project__customer__customer_assignees__is_reviewer=True, - ) - ) - visible_reports = Report.objects.all().filter( - Q(task__in=assigned_tasks) | Q(user=user) - ) - - return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) - return queryset except models.Employment.DoesNotExist: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): assigned_tasks = Task.objects.filter( @@ -92,20 +79,37 @@ def get_queryset(self): Q(task__in=assigned_tasks) | Q(user=user) ) return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) - raise exceptions.PermissionDenied("User has no employment") + msg = "User has no employment" + raise exceptions.PermissionDenied(msg) from None + if current_employment.is_external: + assigned_tasks = Task.objects.filter( + Q(task_assignees__user=user, task_assignees__is_reviewer=True) + | Q( + project__project_assignees__user=user, + project__project_assignees__is_reviewer=True, + ) + | Q( + project__customer__customer_assignees__user=user, + project__customer__customer_assignees__is_reviewer=True, + ) + ) + visible_reports = Report.objects.all().filter( + Q(task__in=assigned_tasks) | Q(user=user) + ) + + return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) + return queryset @action(methods=["get"], detail=False) - def me(self, request, pk=None): - User = get_user_model() - self.object = get_object_or_404(User, pk=request.user.id) + def me(self, request, _pk=None): + self.object = get_object_or_404(get_user_model(), pk=request.user.id) serializer = self.get_serializer(self.object) return Response(serializer.data) @action(methods=["post"], detail=True) - def transfer(self, request, pk=None): - """ - Transfer worktime and absence balance to new year. + def transfer(self, _request, pk=None): # noqa: ARG002 + """Transfer worktime and absence balance to new year. It will skip any credits if a credit already exists on the first of the new year. @@ -160,8 +164,7 @@ class WorktimeBalanceViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): filterset_class = filters.WorktimeBalanceFilterSet def _extract_date(self): - """ - Extract date from request. + """Extract date from request. In detail route extract it from pk and it list from query params. @@ -173,8 +176,8 @@ def _extract_date(self): try: return datetime.datetime.strptime(pk.split("_")[1], "%Y-%m-%d") - except (ValueError, TypeError, IndexError): - raise exceptions.NotFound() + except (ValueError, TypeError, IndexError) as exc: + raise exceptions.NotFound from exc # list case query_params = self.request.query_params @@ -182,11 +185,11 @@ def _extract_date(self): return datetime.datetime.strptime( query_params.get("date"), "%Y-%m-%d" ).date() - except ValueError: - raise exceptions.ParseError(_("Date is invalid")) - except TypeError: + except ValueError as exc: + raise exceptions.ParseError(_("Date is invalid")) from exc + except TypeError as exc: if query_params.get("last_reported_date", "0") == "0": - raise exceptions.ParseError(_("Date filter needs to be set")) + raise exceptions.ParseError(_("Date filter needs to be set")) from exc return None @@ -220,8 +223,7 @@ class AbsenceBalanceViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): filterset_class = filters.AbsenceBalanceFilterSet def _extract_date(self): - """ - Extract date from request. + """Extract date from request. In detail route extract it from pk and it list from query params. @@ -233,22 +235,21 @@ def _extract_date(self): try: return datetime.datetime.strptime(pk.split("_")[2], "%Y-%m-%d") - except (ValueError, TypeError, IndexError): - raise exceptions.NotFound() + except (ValueError, TypeError, IndexError) as exc: + raise exceptions.NotFound from exc # list case try: return datetime.datetime.strptime( self.request.query_params.get("date"), "%Y-%m-%d" ).date() - except ValueError: - raise exceptions.ParseError(_("Date is invalid")) - except TypeError: - raise exceptions.ParseError(_("Date filter needs to be set")) + except ValueError as exc: + raise exceptions.ParseError(_("Date is invalid")) from exc + except TypeError as exc: + raise exceptions.ParseError(_("Date filter needs to be set")) from exc def _extract_user(self): - """ - Extract user from request. + """Extract user from request. In detail route extract it from pk and it list from query params. @@ -263,8 +264,8 @@ def _extract_user(self): if self.request.user.id == user_id: return self.request.user return get_user_model().objects.get(pk=pk.split("_")[0]) - except (ValueError, get_user_model().DoesNotExist): - raise exceptions.NotFound() + except (ValueError, get_user_model().DoesNotExist) as exc: + raise exceptions.NotFound from exc # list case try: @@ -277,8 +278,8 @@ def _extract_user(self): return self.request.user return get_user_model().objects.get(pk=user_id) - except (ValueError, get_user_model().DoesNotExist): - raise exceptions.ParseError(_("User is invalid")) + except (ValueError, get_user_model().DoesNotExist) as exc: + raise exceptions.ParseError(_("User is invalid")) from exc def get_queryset(self): date = self._extract_date() @@ -301,10 +302,12 @@ def get_queryset(self): # only myself, superuser and supervisors may see by absence balances current_user = self.request.user - if not current_user.is_superuser: - if current_user.id != user.id: - if not current_user.supervisees.filter(id=user.id).exists(): - return models.AbsenceType.objects.none() + if ( + not current_user.is_superuser + and current_user.id != user.id + and not current_user.supervisees.filter(id=user.id).exists() + ): + return models.AbsenceType.objects.none() return queryset @@ -313,16 +316,17 @@ class EmploymentViewSet(ModelViewSet): serializer_class = serializers.EmploymentSerializer ordering = ("-end_date",) filterset_class = filters.EmploymentFilterSet - permission_classes = [ - # super user can add/read overtime credits - IsSuperUser - # user may only read filtered results - | IsAuthenticated & IsReadOnly - ] + permission_classes = ( + ( + # super user can add/read overtime credits + IsSuperUser + # user may only read filtered results + | IsAuthenticated & IsReadOnly + ), + ) def get_queryset(self): - """ - Get queryset of employments. + """Get queryset of employments. Following rules apply: 1. super user may see all @@ -405,16 +409,17 @@ class AbsenceCreditViewSet(ModelViewSet): filterset_class = filters.AbsenceCreditFilterSet serializer_class = serializers.AbsenceCreditSerializer - permission_classes = [ - # super user can add/read absence credits - IsSuperUser - # user may only read filtered results - | IsAuthenticated & IsReadOnly - ] + permission_classes = ( + ( + # super user can add/read absence credits + IsSuperUser + # user may only read filtered results + | IsAuthenticated & IsReadOnly + ), + ) def get_queryset(self): - """ - Get queryset of absence credits. + """Get queryset of absence credits. Following rules apply: 1. super user may see all @@ -436,16 +441,17 @@ class OvertimeCreditViewSet(ModelViewSet): filterset_class = filters.OvertimeCreditFilterSet serializer_class = serializers.OvertimeCreditSerializer - permission_classes = [ - # super user can add/read overtime credits - IsSuperUser - # user may only read filtered results - | IsAuthenticated & IsReadOnly - ] + permission_classes = ( + ( + # super user can add/read overtime credits + IsSuperUser + # user may only read filtered results + | IsAuthenticated & IsReadOnly + ), + ) def get_queryset(self): - """ - Get queryset of overtime credits. + """Get queryset of overtime credits. Following rules apply: 1. super user may see all diff --git a/timed/mixins.py b/timed/mixins.py index 94d9825d..3ee36a4a 100644 --- a/timed/mixins.py +++ b/timed/mixins.py @@ -3,9 +3,8 @@ from timed.serializers import AggregateObject -class AggregateQuerysetMixin(object): - """ - Add support for aggregate queryset in view. +class AggregateQuerysetMixin: + """Add support for aggregate queryset in view. Wrap queryst dicts into aggregate object to support renderer which expect attributes. @@ -31,8 +30,7 @@ class AggregateQuerysetMixin(object): """ def _is_related_field(self, val): - """ - Check whether value is a related field. + """Check whether value is a related field. Ignores serializer method fields which define logic separately. """ @@ -52,7 +50,7 @@ def get_serializer(self, data=None, *args, **kwargs): # prefetch data for all related fields prefetch_per_field = {} serializer_class = self.get_serializer_class() - for key, value in serializer_class._declared_fields.items(): + for key, value in serializer_class._declared_fields.items(): # noqa: SLF001 if self._is_related_field(value): source = value.source or key if many: diff --git a/timed/models.py b/timed/models.py index a615408d..3b89580e 100644 --- a/timed/models.py +++ b/timed/models.py @@ -6,8 +6,7 @@ class WeekdaysField(MultiSelectField): - """ - Multi select field using weekdays as choices. + """Multi select field using weekdays as choices. Stores weekdays as comma-separated values in database as iso week day (MON = 1, SUN = 7). diff --git a/timed/notifications/management/commands/__init__.py b/timed/notifications/management/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/timed/notifications/management/commands/budget_check.py b/timed/notifications/management/commands/budget_check.py index 0a4442cb..6395077c 100644 --- a/timed/notifications/management/commands/budget_check.py +++ b/timed/notifications/management/commands/budget_check.py @@ -49,7 +49,7 @@ def handle(self, *args, **options): estimated_hours = project.estimated_time.total_seconds() / 3600 budget_percentage = billable_hours / estimated_hours - if budget_percentage <= 0.3: + if budget_percentage <= 0.3: # noqa: PLR2004 continue try: issue = redmine.issue.get(project.redmine_project.issue_id) @@ -63,7 +63,7 @@ def handle(self, *args, **options): notification, _ = Notification.objects.get_or_create( notification_type=Notification.BUDGET_CHECK_30 - if budget_percentage <= 0.7 + if budget_percentage <= 0.7 # noqa: PLR2004 else Notification.BUDGET_CHECK_70, project=project, ) diff --git a/timed/notifications/management/commands/notify_changed_employments.py b/timed/notifications/management/commands/notify_changed_employments.py index 3a57878d..3ead238e 100644 --- a/timed/notifications/management/commands/notify_changed_employments.py +++ b/timed/notifications/management/commands/notify_changed_employments.py @@ -13,8 +13,7 @@ class Command(BaseCommand): - """ - Notify given email address on changed employments. + """Notify given email address on changed employments. Notifications will be sent when there are employments which changed in given last days. @@ -48,7 +47,7 @@ def handle(self, *args, **options): employments = Employment.objects.filter(updated__range=[start, end]) if employments.exists(): from_email = settings.DEFAULT_FROM_EMAIL - subject = "[Timed] Employments changed in last {0} days".format(last_days) + subject = f"[Timed] Employments changed in last {last_days} days" body = template.render({"employments": employments}) message = EmailMessage( subject=subject, diff --git a/timed/notifications/management/commands/notify_reviewers_unverified.py b/timed/notifications/management/commands/notify_reviewers_unverified.py index a1ea535f..59ba0e2d 100644 --- a/timed/notifications/management/commands/notify_reviewers_unverified.py +++ b/timed/notifications/management/commands/notify_reviewers_unverified.py @@ -17,8 +17,7 @@ class Command(BaseCommand): - """ - Notify reviewers of projects with unverified reports. + """Notify reviewers of projects with unverified reports. Notifications will be sent when reviewer has projects with reports which are unverified in given time frame. @@ -83,23 +82,22 @@ def handle(self, *args, **options): self._notify_reviewers(start, end, reports, message, cc) def _get_unverified_reports(self, start, end): - """ - Get unverified reports. + """Get unverified reports. Unverified reports are reports on project which have a reviewer assigned but are not verified in given time frame. """ return Report.objects.filter(date__range=[start, end], verified_by__isnull=True) - def _notify_reviewers(self, start, end, reports, optional_message, cc): + def _notify_reviewers(self, start, end, reports, optional_message, cc): # noqa: PLR0913 """Notify reviewers on their unverified reports. Only the reviewers lowest in the hierarchy should be notified. If a project has a project assignee and a task assignee with reviewer role, then only the task assignee should be notified about unverified reports. """ - User = get_user_model() - reviewers = User.objects.all_reviewers().filter(email__isnull=False) + user_model = get_user_model() + reviewers = user_model.objects.all_reviewers().filter(email__isnull=False) subject = "[Timed] Verification of reports" from_email = settings.DEFAULT_FROM_EMAIL connection = get_connection() diff --git a/timed/notifications/management/commands/notify_supervisors_shorttime.py b/timed/notifications/management/commands/notify_supervisors_shorttime.py index 1cd0d20f..dedcd810 100644 --- a/timed/notifications/management/commands/notify_supervisors_shorttime.py +++ b/timed/notifications/management/commands/notify_supervisors_shorttime.py @@ -13,8 +13,7 @@ class Command(BaseCommand): - """ - Send notification when supervisees have shorttime in given time frame. + """Send notification when supervisees have shorttime in given time frame. Example how it works: @@ -75,8 +74,7 @@ def _decimal_hours(self, duration): return duration.total_seconds() / 3600 def _get_supervisees_with_shorttime(self, start, end, ratio): - """ - Get supervisees which reported less hours than they should have. + """Get supervisees which reported less hours than they should have. :return: dict mapping all supervisees with shorttime with dict of reported, expected, delta, actual ratio and balance. @@ -107,8 +105,7 @@ def _get_supervisees_with_shorttime(self, start, end, ratio): return supervisees_shorttime def _notify_supervisors(self, start, end, ratio, supervisees): - """ - Notify supervisors about their supervisees. + """Notify supervisors about their supervisees. :param supervisees: dict whereas key is id of supervisee and value as a worktime dict of diff --git a/timed/notifications/models.py b/timed/notifications/models.py index 00339ceb..7027bd0a 100644 --- a/timed/notifications/models.py +++ b/timed/notifications/models.py @@ -11,16 +11,16 @@ class Notification(models.Model): SUPERVISORS_SHORTTIME = "supervisors_shorttime" NOTIFY_ACCOUNTANTS = "notify_accountants" - NOTIFICATION_TYPE_CHOICES = [ + NOTIFICATION_TYPE_CHOICES = ( (BUDGET_CHECK_30, "project budget exceeded 30%"), (BUDGET_CHECK_70, "project budget exceeded 70%"), (CHANGED_EMPLOYMENT, "recently changed employment"), (REVIEWER_UNVERIFIED, "reviewer has reports to verify"), (SUPERVISORS_SHORTTIME, "supervisor has supervisees with short time"), (NOTIFY_ACCOUNTANTS, "notify accountats"), - ] + ) - NOTIFICATION_TYPES = [n for n, _ in NOTIFICATION_TYPE_CHOICES] + NOTIFICATION_TYPES = tuple(n for n, _ in NOTIFICATION_TYPE_CHOICES) sent_at = models.DateTimeField(null=True) project = models.ForeignKey( diff --git a/timed/notifications/tests/test_budget_check.py b/timed/notifications/tests/test_budget_check.py index 0b4b4569..d1dc1082 100644 --- a/timed/notifications/tests/test_budget_check.py +++ b/timed/notifications/tests/test_budget_check.py @@ -10,12 +10,13 @@ from timed.redmine.models import RedmineProject +@pytest.mark.django_db() @pytest.mark.parametrize( - "duration, percentage_exceeded, notification_count", + ("duration", "percentage_exceeded", "notification_count"), [(1, 0, 0), (3, 0, 0), (4, 30, 1), (8, 70, 2), (0, 0, 0)], ) def test_budget_check_1( - db, mocker, report_factory, duration, percentage_exceeded, notification_count + mocker, report_factory, duration, percentage_exceeded, notification_count ): """Test budget check.""" redmine_instance = mocker.MagicMock() @@ -55,7 +56,8 @@ def test_budget_check_1( assert Notification.objects.all().count() == notification_count -def test_budget_check_skip_notification(db, capsys, mocker, report_factory): +@pytest.mark.django_db() +def test_budget_check_skip_notification(capsys, mocker, report_factory): redmine_instance = mocker.MagicMock() issue = mocker.MagicMock() redmine_instance.issue.get.return_value = issue @@ -84,7 +86,8 @@ def test_budget_check_skip_notification(db, capsys, mocker, report_factory): ) -def test_budget_check_no_estimated_timed(db, mocker, capsys, report_factory): +@pytest.mark.django_db() +def test_budget_check_no_estimated_timed(mocker, report_factory): redmine_instance = mocker.MagicMock() issue = mocker.MagicMock() redmine_instance.issue.get.return_value = issue @@ -104,7 +107,8 @@ def test_budget_check_no_estimated_timed(db, mocker, capsys, report_factory): assert Notification.objects.count() == 0 -def test_budget_check_invalid_issue(db, mocker, capsys, report_factory): +@pytest.mark.django_db() +def test_budget_check_invalid_issue(mocker, capsys, report_factory): redmine_instance = mocker.MagicMock() redmine_class = mocker.patch("redminelib.Redmine") redmine_class.return_value = redmine_instance diff --git a/timed/notifications/tests/test_notify_changed_employments.py b/timed/notifications/tests/test_notify_changed_employments.py index d901bcd1..9837bd62 100644 --- a/timed/notifications/tests/test_notify_changed_employments.py +++ b/timed/notifications/tests/test_notify_changed_employments.py @@ -1,12 +1,14 @@ from datetime import date +import pytest from django.core.management import call_command from timed.employment.factories import EmploymentFactory from timed.notifications.models import Notification -def test_notify_changed_employments(db, mailoutbox, freezer): +@pytest.mark.django_db() +def test_notify_changed_employments(mailoutbox, freezer): email = "test@example.net" # employments changed too far in the past @@ -25,7 +27,6 @@ def test_notify_changed_employments(db, mailoutbox, freezer): assert len(mailoutbox) == 1 mail = mailoutbox[0] assert mail.to == [email] - print(mail.body) - assert "80% {0}".format(finished.user.get_full_name()) in mail.body - assert "None 100% {0}".format(new.user.get_full_name()) in mail.body + assert f"80% {finished.user.get_full_name()}" in mail.body + assert f"None 100% {new.user.get_full_name()}" in mail.body assert Notification.objects.all().count() == 1 diff --git a/timed/notifications/tests/test_notify_reviewers_unverified.py b/timed/notifications/tests/test_notify_reviewers_unverified.py index b008997d..9a42c0ab 100644 --- a/timed/notifications/tests/test_notify_reviewers_unverified.py +++ b/timed/notifications/tests/test_notify_reviewers_unverified.py @@ -14,9 +14,10 @@ from timed.tracking.factories import ReportFactory +@pytest.mark.django_db() @pytest.mark.freeze_time("2017-8-4") @pytest.mark.parametrize( - "cc,message", + ("cc", "message"), [ ("", ""), ("example@example.com", ""), @@ -24,7 +25,7 @@ ("", "This is a test"), ], ) -def test_notify_reviewers_with_cc_and_message(db, mailoutbox, cc, message): +def test_notify_reviewers_with_cc_and_message(mailoutbox, cc, message): """Test time range 2017-7-1 till 2017-7-31.""" # a reviewer which will be notified reviewer_work = UserFactory.create() @@ -48,8 +49,8 @@ def test_notify_reviewers_with_cc_and_message(db, mailoutbox, cc, message): call_command( "notify_reviewers_unverified", - "--cc={0}".format(cc), - "--message={0}".format(message), + f"--cc={cc}", + f"--message={message}", ) # checks @@ -65,8 +66,9 @@ def test_notify_reviewers_with_cc_and_message(db, mailoutbox, cc, message): assert mail.cc[0] == cc +@pytest.mark.django_db() @pytest.mark.freeze_time("2017-8-4") -def test_notify_reviewers(db, mailoutbox): +def test_notify_reviewers(mailoutbox): """Test time range 2017-7-1 till 2017-7-31.""" # a reviewer which will be notified reviewer_work = UserFactory.create() @@ -91,8 +93,9 @@ def test_notify_reviewers(db, mailoutbox): assert Notification.objects.count() == 1 +@pytest.mark.django_db() @pytest.mark.freeze_time("2017-8-4") -def test_notify_reviewers_reviewer_hierarchy(db, mailoutbox): +def test_notify_reviewers_reviewer_hierarchy(mailoutbox): """Test notification with reviewer hierarchy. Test if only the lowest in reviewer hierarchy gets notified. diff --git a/timed/notifications/tests/test_notify_supervisors_shorttime.py b/timed/notifications/tests/test_notify_supervisors_shorttime.py index e6c00174..ecab6494 100644 --- a/timed/notifications/tests/test_notify_supervisors_shorttime.py +++ b/timed/notifications/tests/test_notify_supervisors_shorttime.py @@ -10,8 +10,9 @@ from timed.tracking.factories import ReportFactory +@pytest.mark.django_db() @pytest.mark.freeze_time("2017-7-27") -def test_notify_supervisors(db, mailoutbox): +def test_notify_supervisors(mailoutbox): """Test time range 2017-7-17 till 2017-7-23.""" start = date(2017, 7, 14) # supervisee with short time @@ -41,14 +42,15 @@ def test_notify_supervisors(db, mailoutbox): assert mail.to == [supervisor.email] body = mail.body assert "Time range: July 17, 2017 - July 23, 2017\nRatio: 0.9" in body - expected = ("{0} 35.0/42.5 (Ratio 0.82 Delta -7.5 Balance -9.0)").format( - supervisee.get_full_name() + expected = ( + f"{supervisee.get_full_name()} 35.0/42.5 (Ratio 0.82 Delta -7.5 Balance -9.0)" ) assert expected in body assert Notification.objects.count() == 1 -def test_notify_supervisors_no_employment(db, mailoutbox): +@pytest.mark.django_db() +def test_notify_supervisors_no_employment(mailoutbox): """Check that supervisees without employment do not notify supervisor.""" supervisee = UserFactory.create() supervisor = UserFactory.create() diff --git a/timed/permissions.py b/timed/permissions.py index 3db7e916..a5d5b525 100644 --- a/timed/permissions.py +++ b/timed/permissions.py @@ -1,4 +1,3 @@ -# from django.utils import timezone from datetime import date from django.db.models import Q @@ -12,69 +11,68 @@ class IsUnverified(BasePermission): """Allows access only to verified objects.""" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, _request, _view, obj): return obj.verified_by_id is None class IsReadOnly(BasePermission): """Allows read only methods.""" - def has_permission(self, request, view): + def has_permission(self, request, _view): return request.method in SAFE_METHODS - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) class IsDeleteOnly(BasePermission): """Allows only delete method.""" - def has_permission(self, request, view): + def has_permission(self, request, _view): return request.method == "DELETE" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) class IsNotDelete(BasePermission): """Disallow delete method.""" - def has_permission(self, request, view): + def has_permission(self, request, _view): return request.method != "DELETE" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) class IsCreateOnly(BasePermission): """Allows only create method.""" - def has_permission(self, request, view): + def has_permission(self, request, _view): return request.method == "POST" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) class IsUpdateOnly(BasePermission): """Allows only update method.""" - def has_permission(self, request, view): + def has_permission(self, request, _view): return request.method in ["PATCH", "PUT"] - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) class IsAuthenticated(IsAuthenticated): - """ - Support mixing permission IsAuthenticated with object permission. + """Support mixing permission IsAuthenticated with object permission. This is needed to use IsAuthenticated with rest condition and or operator. """ - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) @@ -128,7 +126,8 @@ def has_object_permission(self, request, view, obj): if isinstance(obj, tracking_models.Report): task = obj.task else: # pragma: no cover - raise RuntimeError("IsReviewer permission called on unsupported model") + msg = "IsReviewer permission called on unsupported model" + raise TypeError(msg) return ( projects_models.Task.objects.filter(pk=task.pk) .filter( @@ -155,14 +154,14 @@ def has_permission(self, request, view): return request.user.is_superuser - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) class IsNotTransferred(IsAuthenticated): """Allows access only to not transferred objects.""" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, _request, _view, obj): return not obj.transferred @@ -251,7 +250,7 @@ def has_object_permission(self, request, view, obj): ) .exists() ) - elif isinstance(obj, projects_models.Project): + if isinstance(obj, projects_models.Project): return ( projects_models.Project.objects.filter(pk=obj.pk) .filter( @@ -270,8 +269,8 @@ def has_object_permission(self, request, view, obj): ) .exists() ) - else: # pragma: no cover - raise RuntimeError("IsManager permission called on unsupported model") + msg = "IsManager permission called on unsupported model" # pragma: no cover + raise RuntimeError(msg) # pragma: no cover class IsResource(IsAuthenticated): @@ -295,9 +294,7 @@ def has_object_permission(self, request, view, obj): user = request.user - if isinstance(obj, tracking_models.Activity) or isinstance( - obj, tracking_models.Report - ): + if isinstance(obj, (tracking_models.Activity, tracking_models.Report)): if obj.task: return ( projects_models.Task.objects.filter(pk=obj.task.pk) @@ -314,10 +311,9 @@ def has_object_permission(self, request, view, obj): ) .exists() ) - else: # pragma: no cover - return True - else: # pragma: no cover - raise RuntimeError("IsResource permission called on unsupported model") + return True # pragma: no cover + msg = "IsResource permission called on unsupported model" # pragma: no cover + raise RuntimeError(msg) # pragma: no cover class IsAccountant(IsAuthenticated): diff --git a/timed/projects/admin.py b/timed/projects/admin.py index b593da8a..41806155 100644 --- a/timed/projects/admin.py +++ b/timed/projects/admin.py @@ -12,19 +12,19 @@ class CustomerAssigneeInline(admin.TabularInline): - autocomplete_fields = ["user"] + autocomplete_fields = ("user",) model = models.CustomerAssignee extra = 0 class ProjectAssigneeInline(NestedStackedInline): - autocomplete_fields = ["user"] + autocomplete_fields = ("user",) model = models.ProjectAssignee extra = 0 class TaskAssigneeInline(NestedStackedInline): - autocomplete_fields = ["user"] + autocomplete_fields = ("user",) model = models.TaskAssignee extra = 1 @@ -33,29 +33,31 @@ class TaskAssigneeInline(NestedStackedInline): class CustomerAdmin(admin.ModelAdmin): """Customer admin view.""" - list_display = ["name"] - search_fields = ["name"] - inlines = [CustomerAssigneeInline] + list_display = ("name",) + search_fields = ("name",) + inlines = (CustomerAssigneeInline,) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.projects.exists() @admin.register(models.BillingType) class BillingType(admin.ModelAdmin): - list_display = ["name"] - search_fields = ["name"] + list_display = ("name",) + search_fields = ("name",) @admin.register(models.CostCenter) class CostCenter(admin.ModelAdmin): - list_display = ["name", "reference"] - search_fields = ["name"] + list_display = ( + "name", + "reference", + ) + search_fields = ("name",) class TaskForm(forms.ModelForm): - """ - Task form making sure that initial forms are marked as changed. + """Task form making sure that initial forms are marked as changed. Otherwise when saving project default tasks would not be saved. """ @@ -91,12 +93,11 @@ class TaskInline(NestedStackedInline): form = TaskForm model = models.Task extra = 0 - inlines = [TaskAssigneeInline] + inlines = (TaskAssigneeInline,) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _request, _obj): # for some reason obj is parent object and not task # so this doesn't work - # return obj and not obj.reports.exists() return False @@ -112,13 +113,19 @@ class ProjectAdmin(NestedModelAdmin): """Project admin view.""" form = ProjectForm - list_display = ["name", "customer"] - list_filter = ["customer"] - search_fields = ["name", "customer__name"] + list_display = ( + "name", + "customer", + ) + list_filter = ("customer",) + search_fields = ( + "name", + "customer__name", + ) - inlines = [TaskInline, RedmineProjectInline, ProjectAssigneeInline] + inlines = (TaskInline, RedmineProjectInline, ProjectAssigneeInline) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.tasks.exists() @@ -126,4 +133,4 @@ def has_delete_permission(self, request, obj=None): class TaskTemplateAdmin(admin.ModelAdmin): """Task template admin view.""" - list_display = ["name"] + list_display = ("name",) diff --git a/timed/projects/filters.py b/timed/projects/filters.py index ec0768bc..7c51c605 100644 --- a/timed/projects/filters.py +++ b/timed/projects/filters.py @@ -22,7 +22,10 @@ class Meta: """Meta information for the customer filter set.""" model = models.Customer - fields = ["archived", "reference"] + fields = ( + "archived", + "reference", + ) class ProjectFilterSet(FilterSet): @@ -33,7 +36,7 @@ class ProjectFilterSet(FilterSet): has_reviewer = NumberFilter(method="filter_has_reviewer") customer = NumberInFilter(field_name="customer") - def filter_has_manager(self, queryset, name, value): + def filter_has_manager(self, queryset, _name, value): if not value: # pragma: no cover return queryset return queryset.filter( @@ -49,7 +52,7 @@ def filter_has_manager(self, queryset, name, value): ) ) - def filter_has_reviewer(self, queryset, name, value): + def filter_has_reviewer(self, queryset, _name, value): if not value: # pragma: no cover return queryset return queryset.filter( @@ -69,17 +72,21 @@ class Meta: """Meta information for the project filter set.""" model = models.Project - fields = ["archived", "customer", "billing_type", "cost_center", "reference"] + fields = ( + "archived", + "customer", + "billing_type", + "cost_center", + "reference", + ) class MyMostFrequentTaskFilter(Filter): - """Filter most frequently used tasks. + """Filter most frequently used tasks.""" - TODO: - From an api and framework standpoint instead of an additional filter it - would be more desirable to assign an ordering field frecency and to - limit by use paging. This is way harder to implement therefore on hold. - """ + # TODO: From an api and framework standpoint instead of an additional filter it + # would be more desirable to assign an ordering field frecency and to + # limit by use paging. This is way harder to implement therefore on hold. def filter(self, qs, value): """Filter for given most frequently used tasks. @@ -107,9 +114,7 @@ def filter(self, qs, value): ) qs = qs.annotate(frequency=Count("reports")).order_by("-frequency") # limit number of results to given value - qs = qs[: int(value)] - - return qs + return qs[: int(value)] class TaskFilterSet(FilterSet): @@ -123,7 +128,13 @@ class Meta: """Meta information for the task filter set.""" model = models.Task - fields = ["archived", "project", "my_most_frequent", "reference", "cost_center"] + fields = ( + "archived", + "project", + "my_most_frequent", + "reference", + "cost_center", + ) class TaskAssigneeFilterSet(FilterSet): @@ -137,7 +148,13 @@ class Meta: """Meta information for the task assignee filter set.""" model = models.TaskAssignee - fields = ["task", "user", "is_reviewer", "is_manager", "is_resource"] + fields = ( + "task", + "user", + "is_reviewer", + "is_manager", + "is_resource", + ) class ProjectAssigneeFilterSet(FilterSet): @@ -151,7 +168,13 @@ class Meta: """Meta information for the project assignee filter set.""" model = models.ProjectAssignee - fields = ["project", "user", "is_reviewer", "is_manager", "is_resource"] + fields = ( + "project", + "user", + "is_reviewer", + "is_manager", + "is_resource", + ) class CustomerAssigneeFilterSet(FilterSet): @@ -165,4 +188,10 @@ class Meta: """Meta information for the customer assignee filter set.""" model = models.CustomerAssignee - fields = ["customer", "user", "is_reviewer", "is_manager", "is_resource"] + fields = ( + "customer", + "user", + "is_reviewer", + "is_manager", + "is_resource", + ) diff --git a/timed/projects/migrations/0003_auto_20170831_1624.py b/timed/projects/migrations/0003_auto_20170831_1624.py index 2f4f2d06..68d0fe7d 100644 --- a/timed/projects/migrations/0003_auto_20170831_1624.py +++ b/timed/projects/migrations/0003_auto_20170831_1624.py @@ -9,10 +9,10 @@ class Migration(migrations.Migration): dependencies = [("projects", "0002_auto_20170823_1045")] operations = [ - migrations.AlterModelOptions(name="customer", options={"ordering": ["name"]}), - migrations.AlterModelOptions(name="project", options={"ordering": ["name"]}), - migrations.AlterModelOptions(name="task", options={"ordering": ["name"]}), + migrations.AlterModelOptions(name="customer", options={"ordering": ("name",)}), + migrations.AlterModelOptions(name="project", options={"ordering": ("name",)}), + migrations.AlterModelOptions(name="task", options={"ordering": ("name",)}), migrations.AlterModelOptions( - name="tasktemplate", options={"ordering": ["name"]} + name="tasktemplate", options={"ordering": ("name",)} ), ] diff --git a/timed/projects/migrations/0005_auto_20170907_0938.py b/timed/projects/migrations/0005_auto_20170907_0938.py index 6e0543f2..80592483 100644 --- a/timed/projects/migrations/0005_auto_20170907_0938.py +++ b/timed/projects/migrations/0005_auto_20170907_0938.py @@ -10,7 +10,7 @@ class Migration(migrations.Migration): operations = [ migrations.AlterModelOptions( - name="billingtype", options={"ordering": ["name"]} + name="billingtype", options={"ordering": ("name",)} ), migrations.RemoveIndex( model_name="customer", name="projects_cu_name_e0e97a_idx" diff --git a/timed/projects/migrations/0006_auto_20171010_1423.py b/timed/projects/migrations/0006_auto_20171010_1423.py index be0f3e84..afeaed47 100644 --- a/timed/projects/migrations/0006_auto_20171010_1423.py +++ b/timed/projects/migrations/0006_auto_20171010_1423.py @@ -25,7 +25,7 @@ class Migration(migrations.Migration): ("name", models.CharField(max_length=255, unique=True)), ("reference", models.CharField(blank=True, max_length=255, null=True)), ], - options={"ordering": ["name"]}, + options={"ordering": ("name",)}, ), migrations.AddField( model_name="project", diff --git a/timed/projects/models.py b/timed/projects/models.py index a8b6acb5..92e3b920 100644 --- a/timed/projects/models.py +++ b/timed/projects/models.py @@ -31,18 +31,14 @@ class Customer(models.Model): related_name="assigned_to_customers", ) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return self.name - class Meta: """Meta informations for the customer model.""" - ordering = ["name"] + ordering = ("name",) + + def __str__(self): + """Represent the model as a string.""" + return self.name class CostCenter(models.Model): @@ -51,12 +47,12 @@ class CostCenter(models.Model): name = models.CharField(max_length=255, unique=True) reference = models.CharField(max_length=255, blank=True, null=True) + class Meta: + ordering = ("name",) + def __str__(self): return self.name - class Meta: - ordering = ["name"] - class BillingType(models.Model): """Billing type defining how a project, resp. reports are being billed.""" @@ -64,12 +60,13 @@ class BillingType(models.Model): name = models.CharField(max_length=255, unique=True) reference = models.CharField(max_length=255, blank=True, null=True) + class Meta: + ordering = ("name",) + def __str__(self): + """Represent the model as a string.""" return self.name - class Meta: - ordering = ["name"] - class Project(models.Model): """Project model. @@ -116,16 +113,12 @@ class Project(models.Model): remaining_effort_tracking = models.BooleanField(default=False) total_remaining_effort = models.DurationField(default=timedelta(0)) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return "{0} > {1}".format(self.customer, self.name) - class Meta: - ordering = ["name"] + ordering = ("name",) + + def __str__(self) -> str: + """Represent the model as a string.""" + return f"{self.customer} > {self.name}" class Task(models.Model): @@ -162,18 +155,14 @@ class Task(models.Model): ) most_recent_remaining_effort = models.DurationField(blank=True, null=True) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return "{0} > {1}".format(self.project, self.name) - class Meta: """Meta informations for the task model.""" - ordering = ["name"] + ordering = ("name",) + + def __str__(self) -> str: + """Represent the model as a string.""" + return f"{self.project} > {self.name}" class TaskTemplate(models.Model): @@ -185,17 +174,13 @@ class TaskTemplate(models.Model): name = models.CharField(max_length=255) - def __str__(self): - """Represent the model as a string. + class Meta: + ordering = ("name",) - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return self.name - class Meta: - ordering = ["name"] - class CustomerAssignee(models.Model): """Customer assignee model. @@ -216,6 +201,10 @@ class CustomerAssignee(models.Model): is_manager = models.BooleanField(default=False) is_customer = models.BooleanField(default=False) + def __str__(self) -> str: + """Represent the model as a string.""" + return f"{self.user.username} {self.customer}" + class ProjectAssignee(models.Model): """Project assignee model. @@ -236,6 +225,9 @@ class ProjectAssignee(models.Model): is_manager = models.BooleanField(default=False) is_customer = models.BooleanField(default=False) + def __str__(self) -> str: + return f"{self.user.username} {self.project}" + class TaskAssignee(models.Model): """Task assignee model. @@ -256,9 +248,12 @@ class TaskAssignee(models.Model): is_manager = models.BooleanField(default=False) is_customer = models.BooleanField(default=False) + def __str__(self) -> str: + return f"{self.user.username} {self.task}" + @receiver(pre_save, sender=Project) -def update_billed_flag_on_reports(sender, instance, **kwargs): +def update_billed_flag_on_reports(sender, instance, **kwargs): # noqa: ARG001 """Update billed flag on all reports from the updated project. Only update reports if the billed flag on the project was changed. @@ -272,8 +267,5 @@ def update_billed_flag_on_reports(sender, instance, **kwargs): return # check whether the project was created or is being updated - if instance.pk: - if instance.billed != Project.objects.get(id=instance.id).billed: - Report.objects.filter(Q(task__project=instance)).update( - billed=instance.billed - ) + if instance.pk and instance.billed != Project.objects.get(id=instance.id).billed: + Report.objects.filter(Q(task__project=instance)).update(billed=instance.billed) diff --git a/timed/projects/serializers.py b/timed/projects/serializers.py index 1ab02de7..b76ec501 100644 --- a/timed/projects/serializers.py +++ b/timed/projects/serializers.py @@ -1,6 +1,9 @@ """Serializers for the projects app.""" +from __future__ import annotations + from datetime import timedelta +from typing import TYPE_CHECKING from django.db.models import Q, Sum from django.utils.duration import duration_string @@ -10,6 +13,9 @@ from timed.projects import models from timed.tracking.models import Report +if TYPE_CHECKING: + from typing import ClassVar + class CustomerSerializer(ModelSerializer): """Customer serializer.""" @@ -18,26 +24,32 @@ class Meta: """Meta information for the customer serializer.""" model = models.Customer - fields = [ + fields = ( "name", "reference", "email", "website", "comment", "archived", - ] + ) class BillingTypeSerializer(ModelSerializer): class Meta: model = models.BillingType - fields = ["name", "reference"] + fields = ( + "name", + "reference", + ) class CostCenterSerializer(ModelSerializer): class Meta: model = models.CostCenter - fields = ["name", "reference"] + fields = ( + "name", + "reference", + ) class ProjectSerializer(ModelSerializer): @@ -46,13 +58,13 @@ class ProjectSerializer(ModelSerializer): customer = ResourceRelatedField(queryset=models.Customer.objects.all()) billing_type = ResourceRelatedField(queryset=models.BillingType.objects.all()) - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "customer": "timed.projects.serializers.CustomerSerializer", "billing_type": "timed.projects.serializers.BillingTypeSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", } - def get_root_meta(self, resource, many): + def get_root_meta(self, _resource, many): if not many: queryset = Report.objects.filter(task__project=self.instance) data = queryset.aggregate(spent_time=Sum("duration")) @@ -87,16 +99,15 @@ def validate_remaining_effort_tracking(self, value): ) ).exists() ): - raise ValidationError( - "Only managers, accountants and superuser may activate remaining effort tracking!" - ) + msg = "Only managers, accountants and superuser may activate remaining effort tracking!" + raise ValidationError(msg) return value class Meta: """Meta information for the project serializer.""" model = models.Project - fields = [ + fields = ( "name", "reference", "comment", @@ -109,7 +120,7 @@ class Meta: "customer_visible", "remaining_effort_tracking", "total_remaining_effort", - ] + ) class TaskSerializer(ModelSerializer): @@ -117,13 +128,13 @@ class TaskSerializer(ModelSerializer): project = ResourceRelatedField(queryset=models.Project.objects.all()) - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "activities": "timed.tracking.serializers.ActivitySerializer", "project": "timed.projects.serializers.ProjectSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", } - def get_root_meta(self, resource, many): + def get_root_meta(self, _resource, many): if not many: queryset = Report.objects.filter(task=self.instance) data = queryset.aggregate(spent_time=Sum("duration")) @@ -165,8 +176,9 @@ def validate(self, data): .exists() ): return data + return None # pragma: no cover # check if user is manager when creating a task - elif ( + if ( user.is_superuser or models.Project.objects.filter(pk=data["project"].id) .filter( @@ -182,12 +194,13 @@ def validate(self, data): .exists() ): return data + return None # pragma: no cover class Meta: """Meta information for the task serializer.""" model = models.Task - fields = [ + fields = ( "name", "reference", "estimated_time", @@ -195,13 +208,13 @@ class Meta: "project", "cost_center", "most_recent_remaining_effort", - ] + ) class CustomerAssigneeSerializer(ModelSerializer): """Customer assignee serializer.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "customer": "timed.projects.serializers.CustomerSerializer", } @@ -210,20 +223,20 @@ class Meta: """Meta information for the customer assignee serializer.""" model = models.CustomerAssignee - fields = [ + fields = ( "user", "customer", "is_reviewer", "is_manager", "is_resource", "is_customer", - ] + ) class ProjectAssigneeSerializer(ModelSerializer): """Project assignee serializer.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "project": "timed.projects.serializers.ProjectSerializer", } @@ -232,20 +245,20 @@ class Meta: """Meta information for the project assignee serializer.""" model = models.ProjectAssignee - fields = [ + fields = ( "user", "project", "is_reviewer", "is_manager", "is_resource", "is_customer", - ] + ) class TaskAssigneeSerializer(ModelSerializer): """Task assignees serializer.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "task": "timed.projects.serializers.TaskSerializer", } @@ -254,11 +267,11 @@ class Meta: """Meta information for the task assignee serializer.""" model = models.TaskAssignee - fields = [ + fields = ( "user", "task", "is_reviewer", "is_manager", "is_resource", "is_customer", - ] + ) diff --git a/timed/projects/tests/__init__.py b/timed/projects/tests/__init__.py index 6e031999..e69de29b 100644 --- a/timed/projects/tests/__init__.py +++ b/timed/projects/tests/__init__.py @@ -1 +0,0 @@ -# noqa: D104 diff --git a/timed/projects/tests/test_billing_type.py b/timed/projects/tests/test_billing_type.py index f4e9fcb2..2f4dd6cb 100644 --- a/timed/projects/tests/test_billing_type.py +++ b/timed/projects/tests/test_billing_type.py @@ -7,7 +7,15 @@ @pytest.mark.parametrize( - "is_employed, is_external, is_customer_assignee, is_customer, customer_visible, expected, status_code", + ( + "is_employed", + "is_external", + "is_customer_assignee", + "is_customer", + "customer_visible", + "expected", + "status_code", + ), [ (False, False, True, False, False, 0, HTTP_403_FORBIDDEN), (False, False, True, True, False, 0, HTTP_200_OK), diff --git a/timed/projects/tests/test_cost_center.py b/timed/projects/tests/test_cost_center.py index 08d6422e..46c26ae9 100644 --- a/timed/projects/tests/test_cost_center.py +++ b/timed/projects/tests/test_cost_center.py @@ -7,7 +7,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, status_code", + ("is_employed", "is_customer_assignee", "is_customer", "status_code"), [ (False, True, False, HTTP_403_FORBIDDEN), (False, True, True, HTTP_403_FORBIDDEN), diff --git a/timed/projects/tests/test_customer.py b/timed/projects/tests/test_customer.py index 6a901a80..07de2da1 100644 --- a/timed/projects/tests/test_customer.py +++ b/timed/projects/tests/test_customer.py @@ -55,7 +55,7 @@ def test_customer_delete(auth_client): assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED -@pytest.mark.parametrize("is_assigned, expected", [(True, 1), (False, 0)]) +@pytest.mark.parametrize(("is_assigned", "expected"), [(True, 1), (False, 0)]) def test_customer_list_external_employee( external_employee_client, is_assigned, expected ): @@ -74,7 +74,7 @@ def test_customer_list_external_employee( @pytest.mark.parametrize( - "is_customer, expected", + ("is_customer", "expected"), [(True, 1), (False, 0)], ) def test_customer_list_no_employment(auth_client, is_customer, expected): diff --git a/timed/projects/tests/test_customer_assignee.py b/timed/projects/tests/test_customer_assignee.py index 73c7232d..0a761999 100644 --- a/timed/projects/tests/test_customer_assignee.py +++ b/timed/projects/tests/test_customer_assignee.py @@ -7,7 +7,7 @@ @pytest.mark.parametrize( - "is_employed, is_external, is_customer_assignee, is_customer, expected", + ("is_employed", "is_external", "is_customer_assignee", "is_customer", "expected"), [ (False, False, True, False, 0), (False, False, True, True, 0), diff --git a/timed/projects/tests/test_project.py b/timed/projects/tests/test_project.py index 7ef69b31..955ff59f 100644 --- a/timed/projects/tests/test_project.py +++ b/timed/projects/tests/test_project.py @@ -51,7 +51,8 @@ def test_project_list_include( assert json["data"][0]["id"] == str(project.id) -def test_project_detail_no_auth(db, client, project): +@pytest.mark.django_db() +def test_project_detail_no_auth(client, project): url = reverse("project-detail", args=[project.id]) res = client.get(url) @@ -116,7 +117,7 @@ def test_project_delete(auth_client, project): assert response.status_code == status.HTTP_403_FORBIDDEN -@pytest.mark.parametrize("is_assigned, expected", [(True, 1), (False, 0)]) +@pytest.mark.parametrize(("is_assigned", "expected"), [(True, 1), (False, 0)]) def test_project_list_external_employee( external_employee_client, is_assigned, expected ): @@ -169,7 +170,8 @@ def test_project_multi_number_value_filter(internal_employee_client): assert len(json["data"]) == 2 -def test_project_update_billed_flag(internal_employee_client, report_factory): +@pytest.mark.usefixtures("internal_employee_client") +def test_project_update_billed_flag(report_factory): report = report_factory.create() project = report.task.project assert not report.billed @@ -188,7 +190,7 @@ def test_project_update_billed_flag(internal_employee_client, report_factory): @pytest.mark.parametrize( - "is_customer, project__customer_visible, expected", + ("is_customer", "project__customer_visible", "expected"), [ (True, True, 1), (True, False, 0), @@ -213,7 +215,7 @@ def test_project_list_no_employment(auth_client, project, is_customer, expected) @pytest.mark.parametrize( - "assignee_level, status_code", + ("assignee_level", "status_code"), [ ("customer", status.HTTP_200_OK), ("project", status.HTTP_200_OK), diff --git a/timed/projects/tests/test_project_assignee.py b/timed/projects/tests/test_project_assignee.py index 6fc5eec2..30766088 100644 --- a/timed/projects/tests/test_project_assignee.py +++ b/timed/projects/tests/test_project_assignee.py @@ -7,7 +7,7 @@ @pytest.mark.parametrize( - "is_employed, is_external, is_customer_assignee, is_customer, expected", + ("is_employed", "is_external", "is_customer_assignee", "is_customer", "expected"), [ (False, False, True, False, 0), (False, False, True, True, 0), diff --git a/timed/projects/tests/test_task.py b/timed/projects/tests/test_task.py index 217d4bf9..df79ca74 100644 --- a/timed/projects/tests/test_task.py +++ b/timed/projects/tests/test_task.py @@ -68,7 +68,13 @@ def test_task_detail(internal_employee_client, task): @pytest.mark.parametrize( - "project_assignee__is_resource, project_assignee__is_manager, project_assignee__is_reviewer, customer_assignee__is_manager, expected", + ( + "project_assignee__is_resource", + "project_assignee__is_manager", + "project_assignee__is_reviewer", + "customer_assignee__is_manager", + "expected", + ), [ (True, False, False, False, status.HTTP_403_FORBIDDEN), (False, True, False, False, status.HTTP_201_CREATED), @@ -103,7 +109,15 @@ def test_task_create( @pytest.mark.parametrize( - "task_assignee__is_resource, task_assignee__is_manager, task_assignee__is_reviewer, project_assignee__is_reviewer, project_assignee__is_manager, different_project, expected", + ( + "task_assignee__is_resource", + "task_assignee__is_manager", + "task_assignee__is_reviewer", + "project_assignee__is_reviewer", + "project_assignee__is_manager", + "different_project", + "expected", + ), [ (True, False, False, False, False, False, status.HTTP_403_FORBIDDEN), (False, True, False, False, False, False, status.HTTP_200_OK), @@ -143,7 +157,12 @@ def test_task_update( @pytest.mark.parametrize( - "project_assignee__is_resource, project_assignee__is_manager, project_assignee__is_reviewer, expected", + ( + "project_assignee__is_resource", + "project_assignee__is_manager", + "project_assignee__is_reviewer", + "expected", + ), [ (True, False, False, status.HTTP_403_FORBIDDEN), (False, True, False, status.HTTP_204_NO_CONTENT), @@ -187,7 +206,7 @@ def test_task_detail_with_reports(internal_employee_client, task, report_factory assert json["meta"]["spent-time"] == "02:30:00" -@pytest.mark.parametrize("is_assigned, expected", [(True, 1), (False, 0)]) +@pytest.mark.parametrize(("is_assigned", "expected"), [(True, 1), (False, 0)]) def test_task_list_external_employee(external_employee_client, is_assigned, expected): TaskFactory.create_batch(4) task = TaskFactory.create() @@ -204,7 +223,7 @@ def test_task_list_external_employee(external_employee_client, is_assigned, expe @pytest.mark.parametrize( - "is_customer, customer_visible, expected", + ("is_customer", "customer_visible", "expected"), [ (True, True, 1), (True, False, 0), diff --git a/timed/projects/tests/test_task_assignee.py b/timed/projects/tests/test_task_assignee.py index 522350e2..ce1253e2 100644 --- a/timed/projects/tests/test_task_assignee.py +++ b/timed/projects/tests/test_task_assignee.py @@ -7,7 +7,7 @@ @pytest.mark.parametrize( - "is_employed, is_external, is_customer_assignee, is_customer, expected", + ("is_employed", "is_external", "is_customer_assignee", "is_customer", "expected"), [ (False, False, True, False, 0), (False, False, True, True, 0), diff --git a/timed/projects/views.py b/timed/projects/views.py index fb849776..99c4b950 100644 --- a/timed/projects/views.py +++ b/timed/projects/views.py @@ -21,7 +21,7 @@ class CustomerViewSet(ReadOnlyModelViewSet): serializer_class = serializers.CustomerSerializer filterset_class = filters.CustomerFilterSet - ordering = "name" + ordering = ("name",) def get_queryset(self): """Prefetch related data. @@ -53,13 +53,15 @@ def get_queryset(self): class BillingTypeViewSet(ReadOnlyModelViewSet): serializer_class = serializers.BillingTypeSerializer - ordering = "name" - permission_classes = [ - # superuser may edit all billing types - IsSuperUser - # internal employees may read all billing types - | IsAuthenticated & (IsInternal | IsCustomer) & IsReadOnly - ] + ordering = ("name",) + permission_classes = ( + ( + # superuser may edit all billing types + IsSuperUser + # internal employees may read all billing types + | IsAuthenticated & (IsInternal | IsCustomer) & IsReadOnly + ), + ) def get_queryset(self): """Get billing types depending on the user's role. @@ -85,26 +87,26 @@ def get_queryset(self): projects__customer__customer_assignees__is_customer=True, ) return queryset - else: - if models.CustomerAssignee.objects.filter( - user=user, is_customer=True - ).exists(): - return queryset.filter( - projects__customer_visible=True, - projects__customer__customer_assignees__user=user, - projects__customer__customer_assignees__is_customer=True, - ) + if models.CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): + return queryset.filter( + projects__customer_visible=True, + projects__customer__customer_assignees__user=user, + projects__customer__customer_assignees__is_customer=True, + ) + return None # pragma: no cover class CostCenterViewSet(ReadOnlyModelViewSet): serializer_class = serializers.CostCenterSerializer - ordering = "name" - permission_classes = [ - # superuser may edit all cost centers - IsSuperUser - # internal employees may read all cost centers - | IsAuthenticated & IsInternal & IsReadOnly - ] + ordering = ("name",) + permission_classes = ( + ( + # superuser may edit all cost centers + IsSuperUser + # internal employees may read all cost centers + | IsAuthenticated & IsInternal & IsReadOnly + ), + ) def get_queryset(self): return models.CostCenter.objects.all() @@ -115,19 +117,24 @@ class ProjectViewSet(ModelViewSet): serializer_class = serializers.ProjectSerializer filterset_class = filters.ProjectFilterSet - ordering_fields = ("customer__name", "name") - ordering = "name" + ordering_fields = ( + "customer__name", + "name", + ) + ordering = ("name",) queryset = models.Project.objects.all() - permission_classes = [ - # superuser may edit all projects - IsSuperUser - # accountants may edit all projects - | IsAccountant - # managers may edit only assigned projects - | IsManager & IsUpdateOnly - # all authenticated users may read all tasks - | IsAuthenticated & IsReadOnly - ] + permission_classes = ( + ( + # superuser may edit all projects + IsSuperUser + # accountants may edit all projects + | IsAccountant + # managers may edit only assigned projects + | IsManager & IsUpdateOnly + # all authenticated users may read all tasks + | IsAuthenticated & IsReadOnly + ), + ) def get_queryset(self): """Get only assigned projects, if an employee is external.""" @@ -161,15 +168,17 @@ class TaskViewSet(ModelViewSet): serializer_class = serializers.TaskSerializer filterset_class = filters.TaskFilterSet queryset = models.Task.objects.select_related("project", "cost_center") - ordering = "name" - permission_classes = [ - # superuser may edit all tasks - IsSuperUser - # managers may edit all tasks - | IsManager - # all authenticated users may read all tasks - | IsAuthenticated & IsReadOnly - ] + ordering = ("name",) + permission_classes = ( + ( + # superuser may edit all tasks + IsSuperUser + # managers may edit all tasks + | IsManager + # all authenticated users may read all tasks + | IsAuthenticated & IsReadOnly + ), + ) def filter_queryset(self, queryset): """Specific filter queryset options.""" diff --git a/timed/redmine/management/__init__.py b/timed/redmine/management/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/timed/redmine/management/commands/__init__.py b/timed/redmine/management/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/timed/redmine/management/commands/redmine_report.py b/timed/redmine/management/commands/redmine_report.py index 37451e4d..a81058cd 100644 --- a/timed/redmine/management/commands/redmine_report.py +++ b/timed/redmine/management/commands/redmine_report.py @@ -85,8 +85,6 @@ def handle(self, *args, **options): issue.save() except redminelib.exceptions.BaseRedmineError: sys.stderr.write( - "Project {0} has an invalid Redmine " - "issue {1} assigned. Skipping".format( - project.name, project.redmine_project.issue_id - ) + f"Project {project.name} has an invalid Redmine " + f"issue {project.redmine_project.issue_id} assigned. Skipping" ) diff --git a/timed/redmine/models.py b/timed/redmine/models.py index 1c84de5b..c4a114b6 100644 --- a/timed/redmine/models.py +++ b/timed/redmine/models.py @@ -4,8 +4,7 @@ class RedmineProject(models.Model): - """ - Definition of a Redmine Project. + """Definition of a Redmine Project. Defines what Timed project belongs to what Redmine issue. """ @@ -14,3 +13,6 @@ class RedmineProject(models.Model): Project, on_delete=models.CASCADE, related_name="redmine_project" ) issue_id = models.PositiveIntegerField() + + def __str__(self) -> str: + return f"{self.issue_id} {self.project}" diff --git a/timed/redmine/tests/test_redmine_report.py b/timed/redmine/tests/test_redmine_report.py index 81b7e78f..c196bb92 100644 --- a/timed/redmine/tests/test_redmine_report.py +++ b/timed/redmine/tests/test_redmine_report.py @@ -5,9 +5,10 @@ from timed.redmine.models import RedmineProject +@pytest.mark.django_db() @pytest.mark.parametrize("not_billable", [False, True]) @pytest.mark.parametrize("review", [False, True]) -def test_redmine_report(db, freezer, mocker, report_factory, not_billable, review): +def test_redmine_report(freezer, mocker, report_factory, not_billable, review): """ Test redmine report. @@ -36,9 +37,9 @@ def test_redmine_report(db, freezer, mocker, report_factory, not_billable, revie redmine_instance.issue.get.assert_called_once_with(1000) assert issue.custom_fields == [{"id": 0, "value": report_hours}] - assert "Total hours: {0}".format(report_hours) in issue.notes - assert "Estimated hours: {0}".format(estimated_hours) in issue.notes - assert "Hours in last 7 days: {0}\n".format(report_hours) in issue.notes + assert f"Total hours: {report_hours}" in issue.notes + assert f"Estimated hours: {estimated_hours}" in issue.notes + assert f"Hours in last 7 days: {report_hours}\n" in issue.notes assert report.comment in issue.notes assert "[NB]" in issue.notes or not not_billable assert "[Rev]" in issue.notes or not review @@ -47,7 +48,8 @@ def test_redmine_report(db, freezer, mocker, report_factory, not_billable, revie issue.save.assert_called_once_with() -def test_redmine_report_no_estimated_time(db, freezer, mocker, task, report_factory): +@pytest.mark.django_db() +def test_redmine_report_no_estimated_time(freezer, mocker, task, report_factory): redmine_instance = mocker.MagicMock() issue = mocker.MagicMock() redmine_instance.issue.get.return_value = issue @@ -67,7 +69,8 @@ def test_redmine_report_no_estimated_time(db, freezer, mocker, task, report_fact issue.save.assert_called_once_with() -def test_redmine_report_invalid_issue(db, freezer, mocker, capsys, report_factory): +@pytest.mark.django_db() +def test_redmine_report_invalid_issue(freezer, mocker, capsys, report_factory): """Test case when issue is not available.""" redmine_instance = mocker.MagicMock() redmine_class = mocker.patch("redminelib.Redmine") @@ -85,9 +88,8 @@ def test_redmine_report_invalid_issue(db, freezer, mocker, capsys, report_factor assert "issue 1000 assigned" in err -def test_redmine_report_calculate_total_hours( - db, freezer, mocker, task, report_factory -): +@pytest.mark.django_db() +def test_redmine_report_calculate_total_hours(freezer, mocker, task, report_factory): redmine_instance = mocker.MagicMock() issue = mocker.MagicMock() redmine_instance.issue.get.return_value = issue @@ -114,7 +116,5 @@ def test_redmine_report_calculate_total_hours( call_command("redmine_report", last_days=7) redmine_instance.issue.get.assert_called_once_with(1000) - assert "Total hours: {0}".format(total_hours) in issue.notes - assert ( - "Hours in last 7 days: {0}\n".format(total_hours_last_seven_days) in issue.notes - ) + assert f"Total hours: {total_hours}" in issue.notes + assert f"Hours in last 7 days: {total_hours_last_seven_days}\n" in issue.notes diff --git a/timed/redmine/tests/test_update_project_expenditure.py b/timed/redmine/tests/test_update_project_expenditure.py index 23a166d7..0f0cd84a 100644 --- a/timed/redmine/tests/test_update_project_expenditure.py +++ b/timed/redmine/tests/test_update_project_expenditure.py @@ -7,10 +7,11 @@ from timed.redmine.models import RedmineProject +@pytest.mark.django_db() @pytest.mark.parametrize("pretend", [True, False]) @pytest.mark.parametrize("amount_offered", [None, 100.00, 0]) def test_update_project_expenditure( - db, mocker, capsys, report_factory, pretend, amount_offered + mocker, capsys, report_factory, pretend, amount_offered ): redmine_instance = mocker.MagicMock() issue = mocker.MagicMock() @@ -43,9 +44,9 @@ def test_update_project_expenditure( assert f"amount invoiced {project.amount_invoiced.amount}" in out -def test_update_project_expenditure_invalid_issue( - db, freezer, mocker, capsys, report_factory -): +@pytest.mark.django_db() +@pytest.mark.usefixtures("freezer") +def test_update_project_expenditure_invalid_issue(mocker, capsys, report_factory): redmine_instance = mocker.MagicMock() redmine_class = mocker.patch("redminelib.Redmine") redmine_class.return_value = redmine_instance diff --git a/timed/reports/filters.py b/timed/reports/filters.py index 113e26b2..c5547c62 100644 --- a/timed/reports/filters.py +++ b/timed/reports/filters.py @@ -11,7 +11,7 @@ class StatisticFiltersetBase: - def filter_has_reviewer(self, queryset, name, value): + def filter_has_reviewer(self, queryset, _name, value): if not value: # pragma: no cover return queryset @@ -37,9 +37,8 @@ def filter_has_reviewer(self, queryset, name, value): ) return queryset.filter_aggregate(the_filter).filter_base(the_filter) - def filter_cost_center(self, queryset, name, value): - """ - Filter report by cost center. + def filter_cost_center(self, queryset, _name, value): + """Filter report by cost center. The filter behaves slightly different depending on what the statistic summarizes: @@ -50,8 +49,7 @@ def filter_cost_center(self, queryset, name, value): * When viewing the statistic over tasks, only the tasks are filtered """ - - # TODO Discuss: Is this the desired behaviour by our users? + # TODO: Discuss Is this the desired behaviour by our users? if not value: # pragma: no cover return queryset @@ -72,29 +70,25 @@ def filter_cost_center(self, queryset, name, value): # Customer mode: We only need to filter aggregates, # as the customer has no cost center return queryset.filter_aggregate(filter_q) - else: - # Project or task: Filter both to get the correct result - return queryset.filter_base(filter_q).filter_aggregate(filter_q) + # Project or task: Filter both to get the correct result + return queryset.filter_base(filter_q).filter_aggregate(filter_q) def filter_queryset(self, queryset): qs = super().filter_queryset(queryset) duration_ref = self._refs["reports_ref"] + "__duration" - full_qs = qs._base.annotate( + full_qs = qs._base.annotate( # noqa: SLF001 duration=Coalesce( - Sum(duration_ref, filter=qs._agg_filters), + Sum(duration_ref, filter=qs._agg_filters), # noqa: SLF001 Value("00:00:00", DurationField(null=False)), ), pk=F("id"), ) - result = full_qs.values() - # Useful for QS debugging - # print(result.query) - return result + return full_qs.values() -def StatisticFiltersetBuilder(name, reports_ref, project_ref, customer_ref, task_ref): +def statistic_filterset_builder(name, reports_ref, project_ref, customer_ref, task_ref): reports_prefix = f"{reports_ref}__" if reports_ref else "" project_prefix = f"{project_ref}__" if project_ref else "" customer_prefix = f"{customer_ref}__" if customer_ref else "" @@ -141,7 +135,7 @@ def StatisticFiltersetBuilder(name, reports_ref, project_ref, customer_ref, task ) -CustomerStatisticFilterSet = StatisticFiltersetBuilder( +CustomerStatisticFilterSet = statistic_filterset_builder( "CustomerStatisticFilterSet", reports_ref="projects__tasks__reports", project_ref="projects", @@ -149,7 +143,7 @@ def StatisticFiltersetBuilder(name, reports_ref, project_ref, customer_ref, task customer_ref="", ) -ProjectStatisticFilterSet = StatisticFiltersetBuilder( +ProjectStatisticFilterSet = statistic_filterset_builder( "ProjectStatisticFilterSet", reports_ref="tasks__reports", project_ref="", @@ -157,7 +151,7 @@ def StatisticFiltersetBuilder(name, reports_ref, project_ref, customer_ref, task customer_ref="customer", ) -TaskStatisticFilterSet = StatisticFiltersetBuilder( +TaskStatisticFilterSet = statistic_filterset_builder( "TaskStatisticFilterSet", reports_ref="reports", project_ref="project", diff --git a/timed/reports/serializers.py b/timed/reports/serializers.py index abbf4901..a10786ed 100644 --- a/timed/reports/serializers.py +++ b/timed/reports/serializers.py @@ -1,3 +1,9 @@ +"""Serializers for the reports app.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + from django.contrib.auth import get_user_model from rest_framework_json_api import relations from rest_framework_json_api.serializers import ( @@ -11,6 +17,9 @@ from timed.projects.models import Customer, Project from timed.serializers import TotalTimeRootMetaMixin +if TYPE_CHECKING: + from typing import ClassVar + class YearStatisticSerializer(TotalTimeRootMetaMixin, Serializer): duration = DurationField() @@ -48,7 +57,9 @@ class ProjectStatisticSerializer(TotalTimeRootMetaMixin, Serializer): estimated_time = DurationField(read_only=True) total_remaining_effort = DurationField(read_only=True) - included_serializers = {"customer": "timed.projects.serializers.CustomerSerializer"} + included_serializers: ClassVar[dict[str, str]] = { + "customer": "timed.projects.serializers.CustomerSerializer" + } class Meta: resource_name = "project-statistics" @@ -61,7 +72,9 @@ class TaskStatisticSerializer(TotalTimeRootMetaMixin, Serializer): project = relations.ResourceRelatedField(model=Project, read_only=True) estimated_time = DurationField(read_only=True) - included_serializers = {"project": "timed.projects.serializers.ProjectSerializer"} + included_serializers: ClassVar[dict[str, str]] = { + "project": "timed.projects.serializers.ProjectSerializer" + } class Meta: resource_name = "task-statistics" @@ -71,7 +84,9 @@ class UserStatisticSerializer(TotalTimeRootMetaMixin, Serializer): duration = DurationField(read_only=True) user = relations.ResourceRelatedField(model=get_user_model(), read_only=True) - included_serializers = {"user": "timed.employment.serializers.UserSerializer"} + included_serializers: ClassVar[dict[str, str]] = { + "user": "timed.employment.serializers.UserSerializer" + } class Meta: resource_name = "user-statistics" diff --git a/timed/reports/tests/test_customer_statistic.py b/timed/reports/tests/test_customer_statistic.py index e3972cf7..c914732f 100644 --- a/timed/reports/tests/test_customer_statistic.py +++ b/timed/reports/tests/test_customer_statistic.py @@ -11,7 +11,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected, status_code", + ("is_employed", "is_customer_assignee", "is_customer", "expected", "status_code"), [ (False, True, False, 1, status.HTTP_403_FORBIDDEN), (False, True, True, 1, status.HTTP_403_FORBIDDEN), @@ -78,21 +78,19 @@ def test_customer_statistic_list( { "type": "customer-statistics", "id": str(third_customer.pk), - "attributes": { - "duration": "00:00:00", - "name": third_customer.name, - }, - } - ] + expected_data + "attributes": {"duration": "00:00:00", "name": third_customer.name}, + }, + *expected_data, + ] assert json["data"] == expected_data assert json["meta"]["total-time"] == "07:00:00" @pytest.mark.parametrize( - "filter, expected_result", + ("filter", "expected_result"), [("from_date", 5), ("customer", 3), ("cost_center", 3), ("reviewer", 3)], ) -def test_customer_statistic_filtered(auth_client, filter, expected_result): +def test_customer_statistic_filtered(auth_client, filter, expected_result): # noqa: A002 user = auth_client.user setup_customer_and_employment_status( user=user, @@ -132,7 +130,7 @@ def test_customer_statistic_filtered(auth_client, filter, expected_result): @pytest.mark.parametrize( - "is_employed, expected, status_code", + ("is_employed", "expected", "status_code"), [ (True, 5, status.HTTP_200_OK), (False, 1, status.HTTP_403_FORBIDDEN), diff --git a/timed/reports/tests/test_month_statistic.py b/timed/reports/tests/test_month_statistic.py index 2ae27471..27f463ec 100644 --- a/timed/reports/tests/test_month_statistic.py +++ b/timed/reports/tests/test_month_statistic.py @@ -9,7 +9,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected", + ("is_employed", "is_customer_assignee", "is_customer", "expected"), [ (False, True, False, status.HTTP_403_FORBIDDEN), (False, True, True, status.HTTP_403_FORBIDDEN), diff --git a/timed/reports/tests/test_project_statistic.py b/timed/reports/tests/test_project_statistic.py index 4345cece..dc2baf2b 100644 --- a/timed/reports/tests/test_project_statistic.py +++ b/timed/reports/tests/test_project_statistic.py @@ -11,7 +11,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected, status_code", + ("is_employed", "is_customer_assignee", "is_customer", "expected", "status_code"), [ (False, True, False, 1, status.HTTP_403_FORBIDDEN), (False, True, True, 1, status.HTTP_403_FORBIDDEN), @@ -105,10 +105,10 @@ def test_project_statistic_list( @pytest.mark.parametrize( - "filter, expected_result", + ("filter", "expected_result"), [("from_date", 5), ("customer", 3), ("cost_center", 3), ("reviewer", 3)], ) -def test_project_statistic_filtered(auth_client, filter, expected_result): +def test_project_statistic_filtered(auth_client, filter, expected_result): # noqa: A002 user = auth_client.user setup_customer_and_employment_status( user=user, diff --git a/timed/reports/tests/test_task_statistic.py b/timed/reports/tests/test_task_statistic.py index 94918e20..f80f36fa 100644 --- a/timed/reports/tests/test_task_statistic.py +++ b/timed/reports/tests/test_task_statistic.py @@ -11,7 +11,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected, status_code", + ("is_employed", "is_customer_assignee", "is_customer", "expected", "status_code"), [ (False, True, False, 1, status.HTTP_403_FORBIDDEN), (False, True, True, 1, status.HTTP_403_FORBIDDEN), @@ -93,12 +93,12 @@ def test_task_statistic_list( @pytest.mark.parametrize( - "filter, expected_result", + ("filter", "expected_result"), [("from_date", 5), ("customer", 3), ("cost_center", 3), ("reviewer", 3)], ) def test_task_statistic_filtered( auth_client, - filter, + filter, # noqa: A002 expected_result, ): user = auth_client.user diff --git a/timed/reports/tests/test_user_statistic.py b/timed/reports/tests/test_user_statistic.py index 50ebcc85..f10d968d 100644 --- a/timed/reports/tests/test_user_statistic.py +++ b/timed/reports/tests/test_user_statistic.py @@ -9,7 +9,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, status_code", + ("is_employed", "is_customer_assignee", "is_customer", "status_code"), [ (False, True, False, status.HTTP_403_FORBIDDEN), (False, True, True, status.HTTP_403_FORBIDDEN), diff --git a/timed/reports/tests/test_work_report.py b/timed/reports/tests/test_work_report.py index 9830a65c..f1d683eb 100644 --- a/timed/reports/tests/test_work_report.py +++ b/timed/reports/tests/test_work_report.py @@ -16,7 +16,7 @@ @pytest.mark.freeze_time("2017-09-01") @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected, status_code", + ("is_employed", "is_customer_assignee", "is_customer", "expected", "status_code"), [ (False, True, False, 1, status.HTTP_400_BAD_REQUEST), (False, True, True, 1, status.HTTP_400_BAD_REQUEST), @@ -91,7 +91,7 @@ def test_work_report_single_project( @pytest.mark.freeze_time("2017-09-01") @pytest.mark.parametrize( - "is_employed, status_code, expected", + ("is_employed", "status_code", "expected"), [ (True, status.HTTP_200_OK, 3), (False, status.HTTP_400_BAD_REQUEST, 1), @@ -100,15 +100,15 @@ def test_work_report_single_project( def test_work_report_multiple_projects( auth_client, is_employed, status_code, expected, django_assert_num_queries ): - NUM_PROJECTS = 2 + num_projects = 2 user = auth_client.user if is_employed: EmploymentFactory.create(user=user) customer = CustomerFactory.create(name="Customer") report_date = date(2017, 8, 17) - for i in range(NUM_PROJECTS): - project = ProjectFactory.create(customer=customer, name="Project{0}".format(i)) + for i in range(num_projects): + project = ProjectFactory.create(customer=customer, name=f"Project{i}") task = TaskFactory.create(project=project) ReportFactory.create_batch(10, user=user, task=task, date=report_date) @@ -121,10 +121,8 @@ def test_work_report_multiple_projects( content = io.BytesIO(res.content) with ZipFile(content, "r") as zipfile: - for i in range(NUM_PROJECTS): - ods_content = zipfile.read( - "1708-20170901-Customer-Project{0}.ods".format(i) - ) + for i in range(num_projects): + ods_content = zipfile.read(f"1708-20170901-Customer-Project{i}.ods") doc = ezodf.opendoc(io.BytesIO(ods_content)) table = doc.sheets[0] assert table["C5"].value == "2017-08-17" @@ -137,15 +135,16 @@ def test_work_report_empty(auth_client): assert res.status_code == status.HTTP_400_BAD_REQUEST +@pytest.mark.django_db() @pytest.mark.parametrize( - "customer_name,project_name,expected", + ("customer_name", "project_name", "expected"), [ ("Customer Name", "Project/", "1708-20170818-Customer_Name-Project.ods"), ("Customer-Name", "Project", "1708-20170818-Customer-Name-Project.ods"), ("Customer$Name", "Project", "1708-20170818-CustomerName-Project.ods"), ], ) -def test_generate_work_report_name(db, customer_name, project_name, expected): +def test_generate_work_report_name(customer_name, project_name, expected): test_date = date(2017, 8, 18) view = WorkReportViewSet() @@ -154,13 +153,13 @@ def test_generate_work_report_name(db, customer_name, project_name, expected): # slashes should be dropped from file name project = ProjectFactory.create(customer=customer, name=project_name) - name = view._generate_workreport_name(test_date, test_date, project) + name = view._generate_workreport_name(test_date, test_date, project) # noqa: SLF001 assert name == expected @pytest.mark.freeze_time("2017-09-01") @pytest.mark.parametrize( - "settings_count,given_count,expected_status", + ("settings_count", "given_count", "expected_status"), [ (-1, 9, status.HTTP_200_OK), (0, 9, status.HTTP_200_OK), diff --git a/timed/reports/tests/test_year_statistic.py b/timed/reports/tests/test_year_statistic.py index 132b3386..37534863 100644 --- a/timed/reports/tests/test_year_statistic.py +++ b/timed/reports/tests/test_year_statistic.py @@ -10,7 +10,7 @@ @pytest.mark.parametrize( - "is_employed, is_customer_assignee, is_customer, expected", + ("is_employed", "is_customer_assignee", "is_customer", "expected"), [ (False, True, False, status.HTTP_403_FORBIDDEN), (False, True, True, status.HTTP_403_FORBIDDEN), @@ -58,7 +58,7 @@ def test_year_statistic_list( @pytest.mark.parametrize( - "is_employed, expected", + ("is_employed", "expected"), [ (True, status.HTTP_200_OK), (False, status.HTTP_403_FORBIDDEN), diff --git a/timed/reports/views.py b/timed/reports/views.py index a3f3f8c4..48dd76b1 100644 --- a/timed/reports/views.py +++ b/timed/reports/views.py @@ -29,20 +29,23 @@ class YearStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.YearStatisticSerializer filterset_class = ReportFilterSet - ordering_fields = ("year", "duration") + ordering_fields = ( + "year", + "duration", + ) ordering = ("year",) - permission_classes = [ - # internal employees or super users may read all customer statistics - (IsInternal | IsSuperUser) & IsAuthenticated - ] + permission_classes = ( + ( + # internal employees or super users may read all customer statistics + (IsInternal | IsSuperUser) & IsAuthenticated + ), + ) def get_queryset(self): queryset = Report.objects.all() queryset = queryset.annotate(year=ExtractYear("date")).values("year") queryset = queryset.annotate(duration=Sum("duration")) - queryset = queryset.annotate(pk=F("year")) - - return queryset + return queryset.annotate(pk=F("year")) class MonthStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): @@ -50,12 +53,21 @@ class MonthStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.MonthStatisticSerializer filterset_class = ReportFilterSet - ordering_fields = ("year", "month", "duration") - ordering = ("year", "month") - permission_classes = [ - # internal employees or super users may read all customer statistics - (IsInternal | IsSuperUser) & IsAuthenticated - ] + ordering_fields = ( + "year", + "month", + "duration", + ) + ordering = ( + "year", + "month", + ) + permission_classes = ( + ( + # internal employees or super users may read all customer statistics + (IsInternal | IsSuperUser) & IsAuthenticated + ), + ) def get_queryset(self): queryset = Report.objects.all() @@ -64,9 +76,7 @@ def get_queryset(self): ) queryset = queryset.values("year", "month") queryset = queryset.annotate(duration=Sum("duration")) - queryset = queryset.annotate(pk=F("year") * 100 + F("month")) - - return queryset + return queryset.annotate(pk=F("year") * 100 + F("month")) class StatisticQueryset(QuerySet): @@ -78,13 +88,7 @@ def __init__(self, catch_prefixes, *args, base_qs=None, agg_filters=None, **kwar self._agg_filters = agg_filters self._catch_prefixes = catch_prefixes - def filter(self, *args, **kwargs): - if args: # pragma: no cover - # This is a check against programming errors, no need to test - raise RuntimeError( - "Unable to detect statistics filter type form Q objects. use " - "filter_aggregate() or filter_base() instead" - ) + def filter(self, /, **kwargs): my_filters = { k: v for k, v in kwargs.items() if not k.startswith(self._catch_prefixes) } @@ -112,16 +116,16 @@ def filter_base(self, *args, **kwargs): def _clone(self): return StatisticQueryset( model=self.model, - base_qs=self._base._clone(), + base_qs=self._base._clone(), # noqa: SLF001 catch_prefixes=self._catch_prefixes, agg_filters=self._agg_filters, ) - def __str__(self): - return f"StatisticQueryset({str(self._base)} | {str(self._agg_filters)})" + def __str__(self) -> str: + return f"StatisticQueryset({self._base!s} | {self._agg_filters!s})" - def __repr__(self): - return f"StatisticQueryset({repr(self._base)} | {repr(self._agg_filters)})" + def __repr__(self) -> str: + return f"StatisticQueryset({self._base!r} | {self._agg_filters!r})" def filter_aggregate(self, *args, **kwargs): filter_q = Q(*args, **kwargs) @@ -141,17 +145,20 @@ class CustomerStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.CustomerStatisticSerializer filterset_class = filters.CustomerStatisticFilterSet - ordering_fields = [ + ordering_fields = ( "name", "duration", "estimated_time", "remaining_effort", - ] + ) + ordering = ("name",) - permission_classes = [ - # internal employees or super users may read all customer statistics - (IsInternal | IsSuperUser) & IsAuthenticated - ] + permission_classes = ( + ( + # internal employees or super users may read all customer statistics + (IsInternal | IsSuperUser) & IsAuthenticated + ), + ) def get_queryset(self): return StatisticQueryset(model=Customer, catch_prefixes="projects__") @@ -162,17 +169,19 @@ class ProjectStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.ProjectStatisticSerializer filterset_class = filters.ProjectStatisticFilterSet - ordering_fields = [ + ordering_fields = ( "name", "duration", "estimated_time", "remaining_effort", - ] + ) ordering = ("name",) - permission_classes = [ - # internal employees or super users may read all customer statistics - (IsInternal | IsSuperUser) & IsAuthenticated - ] + permission_classes = ( + ( + # internal employees or super users may read all customer statistics + (IsInternal | IsSuperUser) & IsAuthenticated + ), + ) def get_queryset(self): return StatisticQueryset(model=Project, catch_prefixes="tasks__") @@ -183,17 +192,19 @@ class TaskStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.TaskStatisticSerializer filterset_class = filters.TaskStatisticFilterSet - ordering_fields = [ + ordering_fields = ( "name", "duration", "estimated_time", "remaining_effort", - ] + ) ordering = ("name",) - permission_classes = [ - # internal employees or super users may read all customer statistics - (IsInternal | IsSuperUser) & IsAuthenticated - ] + permission_classes = ( + ( + # internal employees or super users may read all customer statistics + (IsInternal | IsSuperUser) & IsAuthenticated + ), + ) def get_queryset(self): return StatisticQueryset(model=Task, catch_prefixes="tasks__") @@ -204,25 +215,27 @@ class UserStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.UserStatisticSerializer filterset_class = ReportFilterSet - ordering_fields = ("user__username", "duration") + ordering_fields = ( + "user__username", + "duration", + ) ordering = ("user__username",) - permission_classes = [ - # internal employees or super users may read all customer statistics - (IsInternal | IsSuperUser) & IsAuthenticated - ] + permission_classes = ( + ( + # internal employees or super users may read all customer statistics + (IsInternal | IsSuperUser) & IsAuthenticated + ), + ) def get_queryset(self): queryset = Report.objects.all() queryset = queryset.values("user") queryset = queryset.annotate(duration=Sum("duration")) - queryset = queryset.annotate(pk=F("user")) - - return queryset + return queryset.annotate(pk=F("user")) class WorkReportViewSet(GenericViewSet): - """ - Build a ods work report of reports with given filters. + """Build a ods work report of reports with given filters. It creates one work report per project. If given filters results in several projects work reports will be returned as zip. @@ -261,8 +274,7 @@ def _parse_query_params(self, queryset, request): return form.cleaned_data def _clean_filename(self, name): - """ - Clean name so it can be used in file paths. + """Clean name so it can be used in file paths. To accomplish this it will remove all special chars and replace spaces with underscores @@ -271,23 +283,21 @@ def _clean_filename(self, name): return re.sub(r"\s+", "_", escaped) def _generate_workreport_name(self, from_date, today, project): - """ - Generate workreport name. + """Generate workreport name. Name is in format: YYMM-YYYYMMDD-$Customer-$Project.ods whereas YYMM is year and month of from_date and YYYYMMDD is date when work reports gets created. """ - return "{0}-{1}-{2}-{3}.ods".format( + return "{}-{}-{}-{}.ods".format( from_date.strftime("%y%m"), today.strftime("%Y%m%d"), self._clean_filename(project.customer.name), self._clean_filename(project.name), ) - def _create_workreport(self, from_date, to_date, today, project, reports, user): - """ - Create ods workreport. + def _create_workreport(self, from_date, to_date, today, project, reports, user): # noqa: PLR0913 + """Create ods workreport. :rtype: tuple :return: tuple where as first value is name and second ezodf document @@ -359,9 +369,9 @@ def _create_workreport(self, from_date, to_date, today, project, reports, user): ) # calculate location of total hours as insert rows moved it - table[13 + len(reports) + len(tasks), 2].formula = "of:=SUM(C13:C{0})".format( - str(13 + len(reports) - 1) - ) + table[ + 13 + len(reports) + len(tasks), 2 + ].formula = f"of:=SUM(C13:C{13 + len(reports) - 1!s})" # calculate location of total not billable hours as insert rows moved it table[ @@ -390,9 +400,7 @@ def list(self, request, *args, **kwargs): and queryset.count() > settings.WORK_REPORTS_EXPORT_MAX_COUNT ): return Response( - "Your request exceeds the maximum allowed entries ({0})".format( - settings.WORK_REPORTS_EXPORT_MAX_COUNT - ), + f"Your request exceeds the maximum allowed entries ({settings.WORK_REPORTS_EXPORT_MAX_COUNT})", status=status.HTTP_400_BAD_REQUEST, ) diff --git a/timed/serializers.py b/timed/serializers.py index 0370b15b..28a25563 100644 --- a/timed/serializers.py +++ b/timed/serializers.py @@ -4,10 +4,10 @@ from django.utils.duration import duration_string -class TotalTimeRootMetaMixin(object): +class TotalTimeRootMetaMixin: duration_field = "duration" - def get_root_meta(self, resource, many): + def get_root_meta(self, _resource, many): """Add total hours over whole result (not just page) to meta.""" if many: view = self.context["view"] @@ -19,8 +19,7 @@ def get_root_meta(self, resource, many): class AggregateObject(dict): - """ - Wrap dict into an object. + """Wrap dict into an object. All values will be accessible through attributes. Note that keys must be valid python names for this to work. diff --git a/timed/settings.py b/timed/settings.py index e9447122..5444ad20 100644 --- a/timed/settings.py +++ b/timed/settings.py @@ -1,5 +1,6 @@ import os import re +from pathlib import Path import environ import sentry_sdk @@ -11,7 +12,7 @@ django_root = environ.Path(__file__) - 2 ENV_FILE = env.str("DJANGO_ENV_FILE", default=django_root(".env")) -if os.path.exists(ENV_FILE): # pragma: no cover +if Path(ENV_FILE).exists(): # pragma: no cover environ.Env.read_env(ENV_FILE) # per default production is enabled for security reasons @@ -42,7 +43,7 @@ def default(default_dev=env.NOTSET, default_prod=env.NOTSET): # Application definition -DEBUG = env.bool("DJANGO_DEBUG", default=default(True, False)) +DEBUG = env.bool("DJANGO_DEBUG", default=default(default_dev=True, default_prod=False)) SECRET_KEY = env.str("DJANGO_SECRET_KEY", default=default("uuuuuuuuuu")) ALLOWED_HOSTS = env.list("DJANGO_ALLOWED_HOSTS", default=default(["*"])) HOST_PROTOCOL = env.str("DJANGO_HOST_PROTOCOL", default=default("http")) @@ -210,13 +211,11 @@ def default(default_dev=env.NOTSET, default_prod=env.NOTSET): AUTH_PASSWORD_VALIDATORS = [ { - "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator" # noqa - }, - {"NAME": "django.contrib.auth.password_validation.MinimumLengthValidator"}, # noqa - {"NAME": "django.contrib.auth.password_validation.CommonPasswordValidator"}, # noqa - { - "NAME": "django.contrib.auth.password_validation.NumericPasswordValidator" # noqa + "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator" }, + {"NAME": "django.contrib.auth.password_validation.MinimumLengthValidator"}, + {"NAME": "django.contrib.auth.password_validation.CommonPasswordValidator"}, + {"NAME": "django.contrib.auth.password_validation.NumericPasswordValidator"}, ] # OIDC @@ -242,7 +241,9 @@ def default(default_dev=env.NOTSET, default_prod=env.NOTSET): OIDC_RP_CLIENT_ID = env.str("DJANGO_OIDC_RP_CLIENT_ID", default="timed-public") OIDC_RP_CLIENT_SECRET = env.str("DJANGO_OIDC_RP_CLIENT_SECRET", default=None) -OIDC_VERIFY_SSL = env.bool("DJANGO_OIDC_VERIFY_SSL", default=default(False, True)) +OIDC_VERIFY_SSL = env.bool( + "DJANGO_OIDC_VERIFY_SSL", default=default(default_dev=False, default_prod=True) +) OIDC_RP_SIGN_ALGO = env.str("DJANGO_OIDC_RP_SIGN_ALGO", default="RS256") OIDC_CREATE_USER = env.bool("DJANGO_OIDC_CREATE_USER", default=True) @@ -301,8 +302,7 @@ def default(default_dev=env.NOTSET, default_prod=env.NOTSET): def parse_admins(admins): - """ - Parse env admins to django admins. + """Parse env admins to django admins. Example of DJANGO_ADMINS environment variable: Test Example ,Test2 @@ -311,10 +311,11 @@ def parse_admins(admins): for admin in admins: match = re.search(r"(.+) \<(.+@.+)\>", admin) if not match: - raise environ.ImproperlyConfigured( - 'In DJANGO_ADMINS admin "{0}" is not in correct ' - '"Firstname Lastname "'.format(admin) + msg = ( + f'In DJANGO_ADMINS admin "{admin}" is not in correct ' + '"Firstname Lastname "' ) + raise environ.ImproperlyConfigured(msg) result.append((match.group(1), match.group(2))) return result @@ -388,7 +389,7 @@ def parse_admins(admins): # environment variable, or infer a git commit # SHA as release, however you may want to set # something more human-readable. - # release="myapp@1.0.0", + # release="myapp@1.0.0", # noqa: ERA001 ) SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") diff --git a/timed/subscription/admin.py b/timed/subscription/admin.py index 53f96022..a580d0e8 100644 --- a/timed/subscription/admin.py +++ b/timed/subscription/admin.py @@ -14,5 +14,9 @@ class PackageForm(forms.ModelForm): @admin.register(models.Package) class PackageAdmin(admin.ModelAdmin): - list_display = ["billing_type", "duration", "price"] + list_display = ( + "billing_type", + "duration", + "price", + ) form = PackageForm diff --git a/timed/subscription/filters.py b/timed/subscription/filters.py index 986dd395..b2ccbe54 100644 --- a/timed/subscription/filters.py +++ b/timed/subscription/filters.py @@ -8,13 +8,16 @@ class PackageFilter(FilterSet): customer = NumberFilter(method="filter_customer") - def filter_customer(self, queryset, name, value): + def filter_customer(self, queryset, _name, value): billing_types = Project.objects.filter(customer=value).values("billing_type") return queryset.filter(billing_type__in=billing_types) class Meta: model = models.Package - fields = ("billing_type", "customer") + fields = ( + "billing_type", + "customer", + ) class OrderFilter(FilterSet): @@ -23,4 +26,8 @@ class OrderFilter(FilterSet): class Meta: model = models.Order - fields = ("customer", "project", "acknowledged") + fields = ( + "customer", + "project", + "acknowledged", + ) diff --git a/timed/subscription/models.py b/timed/subscription/models.py index 18d173e7..9d522eaf 100644 --- a/timed/subscription/models.py +++ b/timed/subscription/models.py @@ -22,6 +22,9 @@ class Package(models.Model): duration = models.DurationField() price = MoneyField(max_digits=7, decimal_places=2, default_currency="CHF") + def __str__(self) -> str: + return f"{self.billing_type} {self.duration} {self.price}" + class Order(models.Model): """Order of customer for specific amount of hours.""" @@ -39,3 +42,6 @@ class Order(models.Model): blank=True, related_name="orders_confirmed", ) + + def __str__(self) -> str: + return f"{self.project} {self.duration}" diff --git a/timed/subscription/serializers.py b/timed/subscription/serializers.py index 1138e8cb..6160668e 100644 --- a/timed/subscription/serializers.py +++ b/timed/subscription/serializers.py @@ -1,4 +1,9 @@ +"""Serializers for the subscription app.""" + +from __future__ import annotations + from datetime import timedelta +from typing import TYPE_CHECKING from django.db.models import Sum from django.utils.duration import duration_string @@ -13,14 +18,16 @@ from .models import Order, Package +if TYPE_CHECKING: + from typing import ClassVar + class SubscriptionProjectSerializer(ModelSerializer): purchased_time = SerializerMethodField(source="get_purchased_time") spent_time = SerializerMethodField(source="get_spent_time") def get_purchased_time(self, obj): - """ - Calculate purchased time for given project. + """Calculate purchased time for given project. Only acknowledged hours are included. """ @@ -29,8 +36,7 @@ def get_purchased_time(self, obj): return duration_string(data["purchased_time"] or timedelta(0)) def get_spent_time(self, obj): - """ - Calculate spent time for given project. + """Calculate spent time for given project. Reports which are not billable or are in review are excluded. """ @@ -40,7 +46,7 @@ def get_spent_time(self, obj): data = reports.aggregate(spent_time=Sum("duration")) return duration_string(data["spent_time"] or timedelta()) - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "billing_type": "timed.projects.serializers.BillingTypeSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", "customer": "timed.projects.serializers.CustomerSerializer", @@ -65,22 +71,31 @@ class PackageSerializer(ModelSerializer): price = CharField() """CharField needed as it includes currency.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "billing_type": "timed.projects.serializers.BillingTypeSerializer" } class Meta: model = Package resource_name = "subscription-packages" - fields = ("duration", "price", "billing_type") + fields = ( + "duration", + "price", + "billing_type", + ) class OrderSerializer(ModelSerializer): - included_serializers = { - "project": ("timed.subscription.serializers" ".SubscriptionProjectSerializer") + included_serializers: ClassVar[dict[str, str]] = { + "project": "timed.subscription.serializers.SubscriptionProjectSerializer" } class Meta: model = Order resource_name = "subscription-orders" - fields = ("duration", "acknowledged", "ordered", "project") + fields = ( + "duration", + "acknowledged", + "ordered", + "project", + ) diff --git a/timed/subscription/tests/test_order.py b/timed/subscription/tests/test_order.py index 95b5c5f7..26ca7307 100644 --- a/timed/subscription/tests/test_order.py +++ b/timed/subscription/tests/test_order.py @@ -10,13 +10,12 @@ @pytest.mark.parametrize( - "is_customer, is_accountant, is_superuser", + ("is_customer", "is_accountant", "is_superuser"), [ (True, False, False), (False, True, False), (False, False, True), (False, False, False), - (False, False, False), ], ) def test_order_list(auth_client, is_customer, is_accountant, is_superuser): @@ -48,7 +47,7 @@ def test_order_list(auth_client, is_customer, is_accountant, is_superuser): @pytest.mark.parametrize( - "is_customer, is_accountant, is_superuser, confirmed, expected", + ("is_customer", "is_accountant", "is_superuser", "confirmed", "expected"), [ (True, False, False, True, status.HTTP_403_FORBIDDEN), (True, False, False, False, status.HTTP_403_FORBIDDEN), @@ -89,7 +88,7 @@ def test_order_delete( @pytest.mark.parametrize( - "is_superuser, is_accountant, is_customer, status_code", + ("is_superuser", "is_accountant", "is_customer", "status_code"), [ (True, False, False, status.HTTP_204_NO_CONTENT), (False, True, False, status.HTTP_204_NO_CONTENT), @@ -127,7 +126,15 @@ def test_order_confirm( @pytest.mark.parametrize( - "is_customer, is_accountant, is_superuser, acknowledged, mail_sent, project_estimate, expected", + ( + "is_customer", + "is_accountant", + "is_superuser", + "acknowledged", + "mail_sent", + "project_estimate", + "expected", + ), [ ( True, @@ -209,7 +216,7 @@ def test_order_create( @pytest.mark.parametrize( - "duration, expected, status_code", + ("duration", "expected", "status_code"), [ ("00:30:00", "0:30:00", status.HTTP_201_CREATED), ("30:00:00", "1 day, 6:00:00", status.HTTP_201_CREATED), @@ -257,7 +264,7 @@ def test_order_create_duration( @pytest.mark.parametrize( - "is_customer, is_accountant, is_superuser, acknowledged, expected", + ("is_customer", "is_accountant", "is_superuser", "acknowledged", "expected"), [ (True, False, False, True, status.HTTP_403_FORBIDDEN), (True, False, False, False, status.HTTP_403_FORBIDDEN), diff --git a/timed/subscription/tests/test_subscription_project.py b/timed/subscription/tests/test_subscription_project.py index 39336eb6..be215656 100644 --- a/timed/subscription/tests/test_subscription_project.py +++ b/timed/subscription/tests/test_subscription_project.py @@ -16,7 +16,7 @@ from timed.tracking.factories import ReportFactory -@pytest.mark.parametrize("is_external, expected", [(True, 0), (False, 1)]) +@pytest.mark.parametrize(("is_external", "expected"), [(True, 0), (False, 1)]) def test_subscription_project_list(auth_client, is_external, expected): employment = EmploymentFactory.create(user=auth_client.user, is_external=False) if is_external: @@ -64,7 +64,7 @@ def test_subscription_project_list(auth_client, is_external, expected): @pytest.mark.parametrize( - "is_customer, project_of_customer, has_employment, is_external, expected", + ("is_customer", "project_of_customer", "has_employment", "is_external", "expected"), [ (True, True, False, False, HTTP_200_OK), (True, False, False, False, HTTP_404_NOT_FOUND), diff --git a/timed/subscription/views.py b/timed/subscription/views.py index 33e51f4c..1cb08ec4 100644 --- a/timed/subscription/views.py +++ b/timed/subscription/views.py @@ -18,8 +18,7 @@ class SubscriptionProjectViewSet(viewsets.ReadOnlyModelViewSet): - """ - Subscription specific project view. + """Subscription specific project view. Subscription projects are not archived projects which have a billing type with packages. @@ -27,7 +26,10 @@ class SubscriptionProjectViewSet(viewsets.ReadOnlyModelViewSet): serializer_class = serializers.SubscriptionProjectSerializer filterset_class = ProjectFilterSet - ordering_fields = ("name", "id") + ordering_fields = ( + "name", + "id", + ) def get_queryset(self): user = self.request.user @@ -58,14 +60,16 @@ def get_queryset(self): class OrderViewSet(viewsets.ModelViewSet): serializer_class = serializers.OrderSerializer filterset_class = filters.OrderFilter - permission_classes = [ - # superuser and accountants may edit all orders - (IsSuperUser | IsAccountant) - # customers may only create orders - | IsCustomer & IsCreateOnly - # all authenticated users may read all orders - | IsAuthenticated & IsReadOnly - ] + permission_classes = ( + ( + # superuser and accountants may edit all orders + (IsSuperUser | IsAccountant) + # customers may only create orders + | IsCustomer & IsCreateOnly + # all authenticated users may read all orders + | IsAuthenticated & IsReadOnly + ), + ) def create(self, request, *args, **kwargs): """Override so we can issue emails on creation.""" @@ -75,7 +79,8 @@ def create(self, request, *args, **kwargs): and request.data.get("acknowledged") and not (request.user.is_accountant or request.user.is_superuser) ): - raise ValidationError("User can not create confirmed orders!") + msg = "User can not create confirmed orders!" + raise ValidationError(msg) project = Project.objects.get(id=request.data.get("project")["id"]) order_duration = request.data.get("duration") @@ -84,9 +89,8 @@ def create(self, request, *args, **kwargs): # don't allow customers to create orders with negative duration if not (request.user.is_accountant or request.user.is_superuser): if "-" in request.data.get("duration"): - raise ValidationError( - "Customer can not create orders with negative duration!" - ) + msg = "Customer can not create orders with negative duration!" + raise ValidationError(msg) notify_admin.prepare_and_send_email(project, order_duration) return super().create(request, *args, **kwargs) @@ -95,9 +99,8 @@ def create(self, request, *args, **kwargs): methods=["post"], permission_classes=[IsSuperUser | IsAccountant], ) - def confirm(self, request, pk=None): - """ - Confirm order. + def confirm(self, request, pk=None): # noqa: ARG002 + """Confirm order. Only allowed by staff members """ @@ -111,11 +114,11 @@ def confirm(self, request, pk=None): def get_queryset(self): return models.Order.objects.select_related("project") - def destroy(self, request, pk): + def destroy(self, _request, pk=None): # noqa: ARG002 instance = self.get_object() if instance.acknowledged: # acknowledge orders may not be deleted - raise exceptions.PermissionDenied() + raise exceptions.PermissionDenied instance.delete() return response.Response(status=status.HTTP_204_NO_CONTENT) diff --git a/timed/tests/test_authentication.py b/timed/tests/test_authentication.py index d18c8ca9..82dc75bb 100644 --- a/timed/tests/test_authentication.py +++ b/timed/tests/test_authentication.py @@ -13,24 +13,23 @@ from timed.employment.factories import UserFactory +@pytest.mark.django_db() @pytest.mark.parametrize("is_id_token", [True, False]) @pytest.mark.parametrize( - "authentication_header,authenticated,error", + ("authentication_header", "error"), [ - ("", False, False), - ("Bearer", False, True), - ("Bearer Too many params", False, True), - ("Basic Auth", False, True), - ("Bearer Token", True, False), + ("", False), + ("Bearer", True), + ("Bearer Too many params", True), + ("Basic Auth", True), + ("Bearer Token", False), ], ) @pytest.mark.parametrize("user__username", ["1"]) def test_authentication( - db, user, rf, authentication_header, - authenticated, error, is_id_token, requests_mock, @@ -64,12 +63,13 @@ def test_authentication( ) +@pytest.mark.django_db() @pytest.mark.parametrize( - "create_user,username,expected_count", + ("create_user", "username", "expected_count"), [(False, "", 0), (True, "", 1), (True, "foo@example.com", 1)], ) def test_authentication_new_user( - db, rf, requests_mock, settings, create_user, username, expected_count + rf, requests_mock, settings, create_user, username, expected_count ): settings.OIDC_CREATE_USER = create_user user_model = get_user_model() @@ -90,7 +90,8 @@ def test_authentication_new_user( assert user_model.objects.count() == expected_count -def test_authentication_update_user_data(db, rf, requests_mock, settings): +@pytest.mark.django_db() +def test_authentication_update_user_data(rf, requests_mock, settings): user_model = get_user_model() user = UserFactory.create() @@ -113,7 +114,8 @@ def test_authentication_update_user_data(db, rf, requests_mock, settings): assert user.email == "test@localhost" -def test_authentication_idp_502(db, rf, requests_mock, settings): +@pytest.mark.django_db() +def test_authentication_idp_502(rf, requests_mock, settings): requests_mock.get( settings.OIDC_OP_USER_ENDPOINT, status_code=status.HTTP_502_BAD_GATEWAY ) @@ -123,7 +125,8 @@ def test_authentication_idp_502(db, rf, requests_mock, settings): OIDCAuthentication().authenticate(request) -def test_authentication_idp_missing_claim(db, rf, requests_mock, settings): +@pytest.mark.django_db() +def test_authentication_idp_missing_claim(rf, requests_mock, settings): settings.OIDC_USERNAME_CLAIM = "missing" userinfo = {"preferred_username": "1"} requests_mock.get(settings.OIDC_OP_USER_ENDPOINT, text=json.dumps(userinfo)) @@ -133,7 +136,8 @@ def test_authentication_idp_missing_claim(db, rf, requests_mock, settings): OIDCAuthentication().authenticate(request) -def test_authentication_no_client(db, rf, requests_mock, settings): +@pytest.mark.django_db() +def test_authentication_no_client(rf, requests_mock, settings): requests_mock.get( settings.OIDC_OP_USER_ENDPOINT, status_code=status.HTTP_401_UNAUTHORIZED ) @@ -147,9 +151,10 @@ def test_authentication_no_client(db, rf, requests_mock, settings): OIDCAuthentication().authenticate(request) +@pytest.mark.django_db() @pytest.mark.parametrize("check_introspect", [True, False]) def test_userinfo_introspection_failure( - db, client, rf, requests_mock, settings, check_introspect + client, rf, requests_mock, settings, check_introspect ): settings.OIDC_CHECK_INTROSPECT = check_introspect requests_mock.get( diff --git a/timed/tests/test_settings.py b/timed/tests/test_settings.py index 33149b28..9c3dbbec 100644 --- a/timed/tests/test_settings.py +++ b/timed/tests/test_settings.py @@ -10,6 +10,6 @@ def test_admins(): ] -def test_invalid_admins(monkeypatch): +def test_invalid_admins(): with pytest.raises(environ.ImproperlyConfigured): settings.parse_admins(["Test Example datetime.timedelta: + """Generate a random duration between 0 and 5 hours.""" return datetime.timedelta(hours=randint(0, 4), minutes=randint(0, 59)) class Meta: diff --git a/timed/tracking/filters.py b/timed/tracking/filters.py index 482a7b53..a0af6d17 100644 --- a/timed/tracking/filters.py +++ b/timed/tracking/filters.py @@ -45,7 +45,7 @@ class ActivityActiveFilter(Filter): """ @boolean_filter - def filter(self, qs, value): + def filter(self, qs, _value): """Filter the queryset. :param QuerySet qs: The queryset to filter @@ -66,7 +66,10 @@ class Meta: """Meta information for the activity filter set.""" model = models.Activity - fields = ["active", "day"] + fields = ( + "active", + "day", + ) class AttendanceFilterSet(FilterSet): @@ -76,7 +79,7 @@ class Meta: """Meta information for the attendance filter set.""" model = models.Attendance - fields = ["date"] + fields = ("date",) class ReportFilterSet(FilterSet): @@ -101,7 +104,7 @@ class ReportFilterSet(FilterSet): cost_center = NumberFilter(method="filter_cost_center") rejected = NumberFilter(field_name="rejected") - def filter_has_reviewer(self, queryset, name, value): + def filter_has_reviewer(self, queryset, _name, value): if not value: # pragma: no cover return queryset @@ -155,7 +158,7 @@ def filter_has_reviewer(self, queryset, name, value): | reports_task_assignee_is_reviewer ) - def filter_editable(self, queryset, name, value): + def filter_editable(self, queryset, _name, value): """Filter reports whether they are editable by current user. When set to `1` filter all results to what is editable by current @@ -186,25 +189,22 @@ def filter_editable(self, queryset, name, value): if user.is_superuser: # superuser may edit all reports return queryset - elif user.is_accountant: + if user.is_accountant: return queryset.filter(unfinished_filter) # only owner, reviewer or supervisor may change unverified reports - queryset = queryset.filter(editable_filter).distinct() + return queryset.filter(editable_filter).distinct() - return queryset - else: # not editable - if user.is_superuser: - # no reports which are not editable - return queryset.none() - elif user.is_accountant: - return queryset.exclude(unfinished_filter) + # not editable + if user.is_superuser: + # no reports which are not editable + return queryset.none() + if user.is_accountant: + return queryset.exclude(unfinished_filter) - queryset = queryset.exclude(editable_filter) - return queryset + return queryset.exclude(editable_filter) - def filter_cost_center(self, queryset, name, value): - """ - Filter report by cost center. + def filter_cost_center(self, queryset, _name, value): + """Filter report by cost center. Cost center on task has higher priority over project cost center. @@ -243,4 +243,9 @@ class Meta: """Meta information for the absence filter set.""" model = models.Absence - fields = ["date", "from_date", "to_date", "user"] + fields = ( + "date", + "from_date", + "to_date", + "user", + ) diff --git a/timed/tracking/models.py b/timed/tracking/models.py index b216c2cc..7bb1cf13 100644 --- a/timed/tracking/models.py +++ b/timed/tracking/models.py @@ -31,19 +31,15 @@ class Activity(models.Model): settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="activities" ) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return "{0}: {1}".format(self.user, self.task) - class Meta: """Meta informations for the activity model.""" verbose_name_plural = "activities" - indexes = [models.Index(fields=["date"])] + indexes = (models.Index(fields=["date"]),) + + def __str__(self) -> str: + """Represent the model as a string.""" + return f"{self.user}: {self.task}" class Attendance(models.Model): @@ -61,13 +57,9 @@ class Attendance(models.Model): settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="attendances" ) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return "{0}: {1} {2} - {3}".format( + def __str__(self) -> str: + """Represent the model as a string.""" + return "{}: {} {} - {}".format( self.user, self.date.strftime("%Y-%m-%d"), self.from_time.strftime("%H:%M"), @@ -103,6 +95,15 @@ class Report(models.Model): rejected = models.BooleanField(default=False) remaining_effort = models.DurationField(default=timedelta(0), null=True) + class Meta: + """Meta information for the report model.""" + + indexes = (models.Index(fields=["date"]),) + + def __str__(self) -> str: + """Represent the model as a string.""" + return f"{self.user}: {self.task}" + def save(self, *args, **kwargs): """Save the report with some custom functionality. @@ -115,19 +116,6 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ - return "{0}: {1}".format(self.user, self.task) - - class Meta: - """Meta information for the report model.""" - - indexes = [models.Index(fields=["date"])] - class Absence(models.Model): """Absence model. @@ -145,9 +133,24 @@ class Absence(models.Model): settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="absences" ) + class Meta: + """Meta informations for the absence model.""" + + unique_together = ( + "date", + "user", + ) + + def __str__(self) -> str: + """Represent the model as a string.""" + return "{}: {} {}".format( + self.user, + self.date.strftime("%Y-%m-%d"), + self.comment, + ) + def calculate_duration(self, employment): - """ - Calculate duration of absence with given employment. + """Calculate duration of absence with given employment. For fullday absences duration is equal worktime per day of employment for absences which need to fill day calcuation needs to check @@ -165,8 +168,3 @@ def calculate_duration(self, employment): return timedelta() return employment.worktime_per_day - reported_time - - class Meta: - """Meta informations for the absence model.""" - - unique_together = ("date", "user") diff --git a/timed/tracking/serializers.py b/timed/tracking/serializers.py index 586b6a40..41d7a129 100644 --- a/timed/tracking/serializers.py +++ b/timed/tracking/serializers.py @@ -1,6 +1,9 @@ """Serializers for the tracking app.""" +from __future__ import annotations + from datetime import date, timedelta +from typing import TYPE_CHECKING from django.contrib.auth import get_user_model from django.db.models import BooleanField, Case, Q, When @@ -21,13 +24,16 @@ from timed.serializers import TotalTimeRootMetaMixin from timed.tracking import models +if TYPE_CHECKING: + from typing import ClassVar + class ActivitySerializer(ModelSerializer): """Activity serializer.""" user = CurrentUserResourceRelatedField() - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "task": "timed.projects.serializers.TaskSerializer", "user": "timed.employment.serializers.UserSerializer", } @@ -74,13 +80,20 @@ class AttendanceSerializer(ModelSerializer): user = CurrentUserResourceRelatedField() - included_serializers = {"user": "timed.employment.serializers.UserSerializer"} + included_serializers: ClassVar[dict[str, str]] = { + "user": "timed.employment.serializers.UserSerializer" + } class Meta: """Meta information for the attendance serializer.""" model = models.Attendance - fields = ["date", "from_time", "to_time", "user"] + fields = ( + "date", + "from_time", + "to_time", + "user", + ) class ReportSerializer(TotalTimeRootMetaMixin, ModelSerializer): @@ -95,7 +108,7 @@ class ReportSerializer(TotalTimeRootMetaMixin, ModelSerializer): queryset=get_user_model().objects, required=False, allow_null=True ) - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "task": "timed.projects.serializers.TaskSerializer", "user": "timed.employment.serializers.UserSerializer", "verified_by": "timed.employment.serializers.UserSerializer", @@ -106,7 +119,7 @@ def _validate_owner_only(self, value, field): user = self.context["request"].user owner = self.instance.user if getattr(self.instance, field) != value and user != owner: - raise ValidationError(_(f"Only owner may change {field}")) + raise ValidationError(_("Only owner may change %s") % field) return value @@ -120,11 +133,12 @@ def validate_duration(self, value): def validate_billed(self, value): """Only accountants may bill reports.""" - if self.instance is not None: - if not self.context["request"].user.is_accountant and ( - self.instance.billed != value - ): - raise ValidationError(_("Only accountants may bill reports.")) + if ( + self.instance is not None + and not self.context["request"].user.is_accountant + and (self.instance.billed != value) + ): + raise ValidationError(_("Only accountants may bill reports.")) return value @@ -140,8 +154,7 @@ def validate_rejected(self, value): return value def validate(self, data): - """ - Validate that verified by is only set by reviewer or superuser. + """Validate that verified by is only set by reviewer or superuser. Additionally make sure a report is cannot be verified_by if is still needs review. @@ -150,7 +163,6 @@ def validate(self, data): Check if remaing effort tracking is active on the corresponding project. """ - user = self.context["request"].user current_verified_by = self.instance and self.instance.verified_by new_verified_by = data.get("verified_by") @@ -180,9 +192,8 @@ def validate(self, data): # check if remaining effort tracking is active on the corresponding project if not task.project.remaining_effort_tracking and data.get("remaining_effort"): - raise ValidationError( - "Remaining effort tracking is not active on this project!" - ) + msg = "Remaining effort tracking is not active on this project!" + raise ValidationError(msg) if new_verified_by != current_verified_by: if not is_reviewer: @@ -202,9 +213,12 @@ def validate(self, data): # update billed flag on reports that are being moved to a different project # according to the billed flag of the project the report was moved to - if self.instance and data.get("task"): - if self.instance.task.id != data.get("task").id: - data["billed"] = data.get("task").project.billed + if ( + self.instance + and data.get("task") + and self.instance.task.id != data.get("task").id + ): + data["billed"] = data.get("task").project.billed current_employment = Employment.objects.get_at(user=user, date=date.today()) @@ -238,14 +252,15 @@ def validate(self, data): ) ).exists() ): - raise ValidationError( + msg = ( "User is not a resource on the corresponding task, project or customer" ) + raise ValidationError(msg) return data class Meta: model = models.Report - fields = [ + fields = ( "comment", "date", "duration", @@ -258,7 +273,7 @@ class Meta: "verified_by", "rejected", "remaining_effort", - ] + ) class ReportBulkSerializer(Serializer): @@ -279,8 +294,7 @@ class Meta: class ReportIntersectionSerializer(Serializer): - """ - Serializer of report intersections. + """Serializer of report intersections. Serializes a representation of all fields which are the same in given Report objects. If values of one field are not the same @@ -363,12 +377,12 @@ def get_verified(self, instance): def get_rejected(self, instance): return self._intersection(instance, "rejected") - def get_root_meta(self, resource, many): + def get_root_meta(self, *args): """Add number of results to meta.""" queryset = self.instance["queryset"] return {"count": queryset.count()} - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "customer": "timed.projects.serializers.CustomerSerializer", "project": "timed.projects.serializers.ProjectSerializer", "task": "timed.projects.serializers.TaskSerializer", @@ -386,7 +400,7 @@ class AbsenceSerializer(ModelSerializer): absence_type = ResourceRelatedField(queryset=AbsenceType.objects.all()) user = CurrentUserResourceRelatedField() - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "absence_type": "timed.employment.serializers.AbsenceTypeSerializer", } @@ -420,22 +434,19 @@ def validate_absence_type(self, value): return value - def validate(self, data): + def validate(self, data: dict) -> dict: """Validate the absence data. An absence should not be created on a public holiday or a weekend. - - :returns: The validated data - :rtype: dict """ instance = self.instance user = data.get("user", instance and instance.user) try: location = Employment.objects.get_at(user, data.get("date")).location - except Employment.DoesNotExist: # pragma: no cover + except Employment.DoesNotExist as exc: # pragma: no cover raise ValidationError( _("You can't create an absence on an unemployed day.") - ) + ) from exc if PublicHoliday.objects.filter( location_id=location.id, date=data.get("date") @@ -452,4 +463,10 @@ class Meta: """Meta information for the absence serializer.""" model = models.Absence - fields = ["comment", "date", "duration", "absence_type", "user"] + fields = ( + "comment", + "date", + "duration", + "absence_type", + "user", + ) diff --git a/timed/tracking/signals.py b/timed/tracking/signals.py index c08cede5..cf1894e6 100644 --- a/timed/tracking/signals.py +++ b/timed/tracking/signals.py @@ -6,7 +6,7 @@ @receiver(pre_save, sender=Report) -def update_rejected_on_reports(sender, instance, **kwargs): +def update_rejected_on_reports(sender, instance, **kwargs): # noqa: ARG001 """Unreject report when the task changes.""" # Check if the report is being created or updated if instance.pk and instance.rejected: @@ -16,7 +16,7 @@ def update_rejected_on_reports(sender, instance, **kwargs): @receiver(pre_save, sender=Report) -def update_most_recent_remaining_effort(sender, instance, **kwargs): +def update_most_recent_remaining_effort(sender, instance, **kwargs): # noqa: ARG001 """Update remaining effort on task, if remaining effort tracking is active. Update most_recent_remaining_effort on task and total_remaining_effort on project diff --git a/timed/tracking/tasks.py b/timed/tracking/tasks.py index 7422defb..3c55b93d 100644 --- a/timed/tracking/tasks.py +++ b/timed/tracking/tasks.py @@ -3,9 +3,8 @@ from django.template.loader import get_template -def _send_notification_emails(changes, reviewer, rejected=False): +def _send_notification_emails(changes, reviewer, rejected=False): # noqa: FBT002 """Send email for each user.""" - if rejected: subject = "[Timed] Your reports have been rejected" template = get_template("mail/notify_user_rejected_reports.tmpl", using="text") @@ -47,7 +46,7 @@ def _get_report_changeset(report, fields): "report": report, "changes": { key: {"old": getattr(report, key), "new": fields[key]} - for key in fields.keys() + for key in fields # skip if field is not changed or just a reviewer field if getattr(report, key) != fields[key] and key in settings.TRACKING_REPORT_VERIFIED_CHANGES @@ -93,10 +92,10 @@ def notify_user_changed_reports(queryset, fields, reviewer): def notify_user_rejected_report(report, reviewer): user_changes = {"user": report.user, "changes": [{"report": report}]} - _send_notification_emails([user_changes], reviewer, True) + _send_notification_emails([user_changes], reviewer, rejected=True) -def notify_user_rejected_reports(queryset, fields, reviewer): +def notify_user_rejected_reports(queryset, _fields, reviewer): users = [report.user for report in queryset.order_by("user").distinct("user")] user_changes = [] @@ -107,4 +106,4 @@ def notify_user_rejected_reports(queryset, fields, reviewer): changes.append(changeset) user_changes.append({"user": user, "changes": changes}) - _send_notification_emails(user_changes, reviewer, True) + _send_notification_emails(user_changes, reviewer, rejected=True) diff --git a/timed/tracking/templatetags/__init__.py b/timed/tracking/templatetags/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/timed/tracking/tests/__init__.py b/timed/tracking/tests/__init__.py index 6e031999..e69de29b 100644 --- a/timed/tracking/tests/__init__.py +++ b/timed/tracking/tests/__init__.py @@ -1 +0,0 @@ -# noqa: D104 diff --git a/timed/tracking/tests/test_absence.py b/timed/tracking/tests/test_absence.py index e5dc07c1..ccde010e 100644 --- a/timed/tracking/tests/test_absence.py +++ b/timed/tracking/tests/test_absence.py @@ -107,7 +107,7 @@ def test_absence_detail(internal_employee_client): @pytest.mark.parametrize( - "is_external, expected", + ("is_external", "expected"), [(False, status.HTTP_201_CREATED), (True, status.HTTP_403_FORBIDDEN)], ) def test_absence_create(auth_client, is_external, expected): diff --git a/timed/tracking/tests/test_activity.py b/timed/tracking/tests/test_activity.py index b94471df..8bd11fe6 100644 --- a/timed/tracking/tests/test_activity.py +++ b/timed/tracking/tests/test_activity.py @@ -30,7 +30,12 @@ def test_activity_detail(internal_employee_client): @pytest.mark.parametrize( - "task_assignee__is_resource, task_assignee__is_reviewer, is_external, expected", + ( + "task_assignee__is_resource", + "task_assignee__is_reviewer", + "is_external", + "expected", + ), [ (True, False, True, status.HTTP_201_CREATED), (False, True, True, status.HTTP_403_FORBIDDEN), @@ -102,7 +107,12 @@ def test_activity_create_no_task_external_employee(auth_client, task_assignee): @pytest.mark.parametrize( - "task_assignee__is_resource, task_assignee__is_reviewer, is_external, expected", + ( + "task_assignee__is_resource", + "task_assignee__is_reviewer", + "is_external", + "expected", + ), [ (True, False, True, status.HTTP_200_OK), (False, True, True, status.HTTP_403_FORBIDDEN), @@ -143,7 +153,12 @@ def test_activity_update(auth_client, is_external, task_assignee, expected): @pytest.mark.parametrize( - "task_assignee__is_resource, task_assignee__is_reviewer, is_external, expected", + ( + "task_assignee__is_resource", + "task_assignee__is_reviewer", + "is_external", + "expected", + ), [ (True, False, True, status.HTTP_204_NO_CONTENT), (False, True, True, status.HTTP_403_FORBIDDEN), diff --git a/timed/tracking/tests/test_attendance.py b/timed/tracking/tests/test_attendance.py index fc67e2b7..bd0de250 100644 --- a/timed/tracking/tests/test_attendance.py +++ b/timed/tracking/tests/test_attendance.py @@ -27,7 +27,7 @@ def test_attendance_detail(internal_employee_client): @pytest.mark.parametrize( - "is_external, task_assignee__is_resource, expected", + ("is_external", "task_assignee__is_resource", "expected"), [ (False, False, status.HTTP_201_CREATED), (True, True, status.HTTP_201_CREATED), diff --git a/timed/tracking/tests/test_report.py b/timed/tracking/tests/test_report.py index 7c5731fa..d30f69c7 100644 --- a/timed/tracking/tests/test_report.py +++ b/timed/tracking/tests/test_report.py @@ -67,9 +67,9 @@ def test_report_intersection_full( json = response.json() pk = json["data"].pop("id") - assert "task={0}".format(report.task.id) in pk - assert "project={0}".format(report.task.project.id) in pk - assert "customer={0}".format(report.task.project.customer.id) in pk + assert f"task={report.task.id}" in pk + assert f"project={report.task.project.id}" in pk + assert f"customer={report.task.project.customer.id}" in pk included = json.pop("included") assert len(included) == 3 @@ -245,7 +245,7 @@ def test_report_list_filter_id( url = reverse("report-list") response = internal_employee_client.get( - url, data={"id": "{0},{1}".format(report_1.id, report_2.id), "ordering": "id"} + url, data={"id": f"{report_1.id},{report_2.id}", "ordering": "id"} ) assert response.status_code == status.HTTP_200_OK json = response.json() @@ -455,7 +455,6 @@ def test_report_list_filter_billed( def test_report_export_missing_type( internal_employee_client, - report_factory, ): user = internal_employee_client.user url = reverse("report-export") @@ -479,7 +478,13 @@ def test_report_detail( @pytest.mark.parametrize( - "task_assignee__is_reviewer, task_assignee__is_manager, task_assignee__is_resource, is_external, expected", + ( + "task_assignee__is_reviewer", + "task_assignee__is_manager", + "task_assignee__is_resource", + "is_external", + "expected", + ), [ (True, False, False, True, status.HTTP_400_BAD_REQUEST), (False, True, False, True, status.HTTP_403_FORBIDDEN), @@ -489,9 +494,7 @@ def test_report_detail( (False, False, True, False, status.HTTP_201_CREATED), ], ) -def test_report_create( - auth_client, report_factory, task_factory, task_assignee, is_external, expected -): +def test_report_create(auth_client, task_factory, task_assignee, is_external, expected): """Should create a new report and automatically set the user.""" user = auth_client.user task = task_factory.create() @@ -532,9 +535,7 @@ def test_report_create( assert json["data"]["relationships"]["task"]["data"]["id"] == str(task.id) -def test_report_create_billed( - internal_employee_client, report_factory, project_factory, task_factory -): +def test_report_create_billed(internal_employee_client, project_factory, task_factory): """Should create a new report and automatically set the user.""" user = internal_employee_client.user project = project_factory.create(billed=True) @@ -649,7 +650,7 @@ def test_report_update_bulk_verify_reviewer( } response = internal_employee_client.post( - url + "?editable=1&reviewer={0}".format(user.id), data + url + f"?editable=1&reviewer={user.id}", data ) assert response.status_code == status.HTTP_204_NO_CONTENT @@ -678,7 +679,6 @@ def test_report_update_bulk_reset_verify(superadmin_client, report_factory): def test_report_update_bulk_not_editable( internal_employee_client, - report_factory, ): url = reverse("report-bulk") @@ -1005,7 +1005,14 @@ def test_report_reset_verified_and_billed_by_reviewer( @pytest.mark.parametrize( - "task_assignee__is_reviewer, task_assignee__is_manager, task_assignee__is_resource, is_external, verified, expected", + ( + "task_assignee__is_reviewer", + "task_assignee__is_manager", + "task_assignee__is_resource", + "is_external", + "verified", + "expected", + ), [ (True, False, False, False, True, status.HTTP_403_FORBIDDEN), (True, False, False, False, False, status.HTTP_204_NO_CONTENT), @@ -1044,7 +1051,13 @@ def test_report_delete_own_report( @pytest.mark.parametrize( - "task_assignee__is_reviewer, task_assignee__is_manager, task_assignee__is_resource, is_external, verified", + ( + "task_assignee__is_reviewer", + "task_assignee__is_manager", + "task_assignee__is_resource", + "is_external", + "verified", + ), [ (True, False, False, False, True), (True, False, False, False, False), @@ -1089,7 +1102,8 @@ def test_report_delete_not_report_owner( ] -def test_report_round_duration(db, report_factory): +@pytest.mark.django_db() +def test_report_round_duration(report_factory): """Should round the duration of a report to 15 minutes.""" report = report_factory.create() @@ -1162,11 +1176,11 @@ def test_report_list_filter_cost_center( @pytest.mark.parametrize("file_type", ["csv", "xlsx", "ods"]) @pytest.mark.parametrize( - "project_cs_name,task_cs_name,project_bt_name", + ("project_cs_name", "task_cs_name", "project_bt_name"), [("Project cost center", "Task cost center", "Some billing type")], ) @pytest.mark.parametrize( - "project_cs,task_cs,expected_cs_name", + ("project_cs", "task_cs", "expected_cs_name"), [ (True, True, "Task cost center"), (True, False, "Project cost center"), @@ -1175,7 +1189,7 @@ def test_report_list_filter_cost_center( ], ) @pytest.mark.parametrize( - "project_bt,expected_bt_name", [(True, "Some billing type"), (False, "")] + ("project_bt", "expected_bt_name"), [(True, "Some billing type"), (False, "")] ) def test_report_export( internal_employee_client, @@ -1224,7 +1238,7 @@ def test_report_export( @pytest.mark.parametrize( - "settings_count,given_count,expected_status", + ("settings_count", "given_count", "expected_status"), [ (-1, 9, status.HTTP_200_OK), (0, 9, status.HTTP_200_OK), @@ -1234,7 +1248,6 @@ def test_report_export( ) def test_report_export_max_count( internal_employee_client, - django_assert_num_queries, report_factory, task, settings, @@ -1298,14 +1311,14 @@ def test_report_update_bulk_verify_reviewer_multiple_notify( # every user received one mail assert len(mailoutbox) == 3 assert all(True for mail in mailoutbox if len(mail.to) == 1) - assert set(mail.to[0] for mail in mailoutbox) == set( + assert {mail.to[0] for mail in mailoutbox} == { user.email for user in [user1, user2, user3] - ) + } @pytest.mark.parametrize("own_report", [True, False]) @pytest.mark.parametrize( - "has_attributes,different_attributes,verified,expected", + ("has_attributes", "different_attributes", "verified", "expected"), [ (True, True, True, True), (True, True, False, True), @@ -1426,11 +1439,13 @@ def test_report_notify_rendering( snapshot.assert_match(mailoutbox[0].body) +@pytest.mark.django_db() +@pytest.mark.usefixtures("project", "task") @pytest.mark.parametrize( - "report__review,needs_review", [(True, False), (False, True), (True, True)] + ("report__review", "needs_review"), [(True, False), (False, True), (True, True)] ) def test_report_update_bulk_review_and_verified( - superadmin_client, project, task, report, user_factory, needs_review + superadmin_client, report, needs_review ): EmploymentFactory.create(user=superadmin_client.user) data = { @@ -1482,7 +1497,7 @@ def test_report_update_bulk_bill_reviewer( } response = internal_employee_client.post( - url + "?editable=1&reviewer={0}".format(user.id), data + url + f"?editable=1&reviewer={user.id}", data ) assert response.status_code == status.HTTP_400_BAD_REQUEST @@ -1536,7 +1551,7 @@ def test_report_update_billed_user( @pytest.mark.parametrize( - "is_accountant, expected", + ("is_accountant", "expected"), [ (True, status.HTTP_200_OK), (False, status.HTTP_400_BAD_REQUEST), @@ -1629,7 +1644,7 @@ def test_report_update_bulk_billed(internal_employee_client, report_factory, tas } response = internal_employee_client.post( - url + "?editable=1&reviewer={0}".format(user.id), data + url + f"?editable=1&reviewer={user.id}", data ) assert response.status_code == status.HTTP_204_NO_CONTENT @@ -1665,7 +1680,7 @@ def test_report_list_external_employee(external_employee_client, report_factory) @pytest.mark.parametrize( - "is_assigned, expected, status_code", + ("is_assigned", "expected", "status_code"), [(True, 1, status.HTTP_200_OK), (False, 0, status.HTTP_403_FORBIDDEN)], ) def test_report_list_no_employment( @@ -1692,7 +1707,7 @@ def test_report_list_no_employment( @pytest.mark.parametrize( - "report_owner, reviewer, expected, mail_count, status_code", + ("report_owner", "reviewer", "expected", "mail_count", "status_code"), [ (True, True, False, 0, status.HTTP_400_BAD_REQUEST), (False, True, True, 1, status.HTTP_200_OK), @@ -1766,7 +1781,6 @@ def test_report_update_rejected_owner( def test_report_reject_multiple_notify( internal_employee_client, task, - task_factory, project, report_factory, user_factory, @@ -1804,9 +1818,9 @@ def test_report_reject_multiple_notify( # every user received one mail assert len(mailoutbox) == 3 assert all(True for mail in mailoutbox if len(mail.to) == 1) - assert set(mail.to[0] for mail in mailoutbox) == set( + assert {mail.to[0] for mail in mailoutbox} == { user.email for user in [user1, user2, user3] - ) + } def test_report_automatic_unreject(internal_employee_client, report_factory, task): @@ -1866,7 +1880,7 @@ def test_report_bulk_automatic_unreject( @pytest.mark.parametrize( - "is_external, remaining_effort_active, is_superuser, expected", + ("is_external", "remaining_effort_active", "is_superuser", "expected"), [ (True, True, False, status.HTTP_403_FORBIDDEN), (True, False, False, status.HTTP_403_FORBIDDEN), @@ -1914,7 +1928,7 @@ def test_report_set_remaining_effort( @pytest.mark.parametrize( - "remaining_effort_active, expected", + ("remaining_effort_active", "expected"), [ (True, status.HTTP_201_CREATED), (False, status.HTTP_400_BAD_REQUEST), @@ -1922,7 +1936,6 @@ def test_report_set_remaining_effort( ) def test_report_create_remaining_effort( internal_employee_client, - report_factory, project_factory, task_factory, remaining_effort_active, diff --git a/timed/tracking/views.py b/timed/tracking/views.py index d4e7ef74..366c0ea7 100644 --- a/timed/tracking/views.py +++ b/timed/tracking/views.py @@ -40,13 +40,15 @@ class ActivityViewSet(ModelViewSet): serializer_class = serializers.ActivitySerializer filterset_class = filters.ActivityFilterSet - permission_classes = [ - # users may not change transferred activities - IsAuthenticated & IsInternal & IsNotTransferred - | IsAuthenticated & IsReadOnly - # only external employees with resource role may create not transferred activities - | IsAuthenticated & IsExternal & IsResource & IsNotTransferred - ] + permission_classes = ( + ( + # users may not change transferred activities + IsAuthenticated & IsInternal & IsNotTransferred + | IsAuthenticated & IsReadOnly + # only external employees with resource role may create not transferred activities + | IsAuthenticated & IsExternal & IsResource & IsNotTransferred + ), + ) def get_queryset(self): """Filter the queryset by the user of the request. @@ -64,14 +66,16 @@ class AttendanceViewSet(ModelViewSet): serializer_class = serializers.AttendanceSerializer filterset_class = filters.AttendanceFilterSet - permission_classes = [ - # superuser may edit all reports but not delete - IsSuperUser & IsNotDelete - # internal employees may change own attendances - | IsAuthenticated & IsInternal - # only external employees with resource role may change own attendances - | IsAuthenticated & IsExternal & IsResource - ] + permission_classes = ( + ( + # superuser may edit all reports but not delete + IsSuperUser & IsNotDelete + # internal employees may change own attendances + | IsAuthenticated & IsInternal + # only external employees with resource role may change own attendances + | IsAuthenticated & IsExternal & IsResource + ), + ) def get_queryset(self): """Filter the queryset by the user of the request. @@ -92,18 +96,23 @@ class ReportViewSet(ModelViewSet): queryset = models.Report.objects.select_related( "task", "user", "task__project", "task__project__customer" ) - permission_classes = [ - # superuser and accountants may edit all reports but not delete - (IsSuperUser | IsAccountant) & IsNotDelete - # reviewer and supervisor may change reports which aren't verfied but not delete them - | (IsReviewer | IsSupervisor) & IsUnverified & IsNotDelete - # internal employees may only change its own unverified reports - # only external employees with resource role may only change its own unverified reports - | IsOwner & IsUnverified & (IsInternal | (IsExternal & IsResource)) - # all authenticated users may read all reports - | IsAuthenticated & IsReadOnly - ] - ordering = ("date", "id") + permission_classes = ( + ( + # superuser and accountants may edit all reports but not delete + (IsSuperUser | IsAccountant) & IsNotDelete + # reviewer and supervisor may change reports which aren't verfied but not delete them + | (IsReviewer | IsSupervisor) & IsUnverified & IsNotDelete + # internal employees may only change its own unverified reports + # only external employees with resource role may only change its own unverified reports + | IsOwner & IsUnverified & (IsInternal | (IsExternal & IsResource)) + # all authenticated users may read all reports + | IsAuthenticated & IsReadOnly + ), + ) + ordering = ( + "date", + "id", + ) ordering_fields = ( "id", "date", @@ -129,22 +138,6 @@ def get_queryset(self): try: current_employment = Employment.objects.get_at(user=user, date=date.today()) - if not current_employment.is_external: - return queryset - - assigned_tasks = Task.objects.filter( - Q(task_assignees__user=user, task_assignees__is_reviewer=True) - | Q( - project__project_assignees__user=user, - project__project_assignees__is_reviewer=True, - ) - | Q( - project__customer__customer_assignees__user=user, - project__customer__customer_assignees__is_reviewer=True, - ) - ) - queryset = queryset.filter(Q(task__in=assigned_tasks) | Q(user=user)) - return queryset except Employment.DoesNotExist: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): return queryset.filter( @@ -153,13 +146,26 @@ def get_queryset(self): task__project__customer__customer_assignees__is_customer=True, ) ) - raise exceptions.PermissionDenied( - "User has no employment and isn't a customer!" + msg = "User has no employment and isn't a customer!" + raise exceptions.PermissionDenied(msg) from None + if not current_employment.is_external: + return queryset + + assigned_tasks = Task.objects.filter( + Q(task_assignees__user=user, task_assignees__is_reviewer=True) + | Q( + project__project_assignees__user=user, + project__project_assignees__is_reviewer=True, + ) + | Q( + project__customer__customer_assignees__user=user, + project__customer__customer_assignees__is_reviewer=True, ) + ) + return queryset.filter(Q(task__in=assigned_tasks) | Q(user=user)) def update(self, request, *args, **kwargs): """Override so we can issue emails on update.""" - partial = kwargs.get("partial", False) instance = self.get_object() serializer = self.get_serializer(instance, data=request.data, partial=partial) @@ -185,9 +191,8 @@ def update(self, request, *args, **kwargs): methods=["get"], serializer_class=serializers.ReportIntersectionSerializer, ) - def intersection(self, request): - """ - Get intersection in reports of common report fields. + def intersection(self, _request): + """Get intersection in reports of common report fields. Use case is for api caller to know what fields are the same in a list of reports. This will be mainly used for bulk update. @@ -248,11 +253,7 @@ def bulk(self, request): fields["verified_by"] = verified and user or None - if ( - "review" in fields - and fields["review"] - or any(queryset.values_list("review", flat=True)) - ): + if fields.get("review") or any(queryset.values_list("review", flat=True)): raise exceptions.ParseError( _("Reports can't both be set as `review` and `verified`.") ) @@ -318,10 +319,8 @@ def export(self, request): and queryset.count() > settings.REPORTS_EXPORT_MAX_COUNT ): return Response( - _( - "Your request exceeds the maximum allowed entries ({0} > {1})".format( - queryset.count(), settings.REPORTS_EXPORT_MAX_COUNT - ) + _("Your request exceeds the maximum allowed entries ({} > {})").format( + queryset.count(), settings.REPORTS_EXPORT_MAX_COUNT ), status=status.HTTP_400_BAD_REQUEST, ) @@ -365,14 +364,16 @@ class AbsenceViewSet(ModelViewSet): serializer_class = serializers.AbsenceSerializer filterset_class = filters.AbsenceFilterSet - permission_classes = [ - # superuser can change all but not delete - IsAuthenticated & IsSuperUser & IsNotDelete - # owner may change all its absences - | IsAuthenticated & IsOwner & IsInternal - # all authenticated users may read filtered result - | IsAuthenticated & IsReadOnly - ] + permission_classes = ( + ( + # superuser can change all but not delete + IsAuthenticated & IsSuperUser & IsNotDelete + # owner may change all its absences + | IsAuthenticated & IsOwner & IsInternal + # all authenticated users may read filtered result + | IsAuthenticated & IsReadOnly + ), + ) def get_queryset(self): """Get absences only for internal employees. @@ -382,10 +383,9 @@ def get_queryset(self): """ user = self.request.user if user.is_superuser: - queryset = models.Absence.objects.select_related("absence_type", "user") - return queryset + return models.Absence.objects.select_related("absence_type", "user") - queryset = ( + return ( models.Absence.objects.select_related("absence_type", "user") .filter(Q(user=user) | Q(user__in=user.supervisees.all())) .exclude( @@ -394,4 +394,3 @@ def get_queryset(self): ).values("date") ) ) - return queryset diff --git a/timed/wsgi.py b/timed/wsgi.py index 7e15165e..1738fc31 100644 --- a/timed/wsgi.py +++ b/timed/wsgi.py @@ -1,5 +1,4 @@ -""" -WSGI config for timed project. +"""WSGI config for timed project. It exposes the WSGI callable as a module-level variable named ``application``.