From d8861c510a89072b53c584892298551676235d16 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Tue, 16 Apr 2024 11:41:01 +0200 Subject: [PATCH 01/27] chore(linting): stricter ruff config --- ruff.toml | 102 +++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 85 insertions(+), 17 deletions(-) diff --git a/ruff.toml b/ruff.toml index 33f0169a..d0deb711 100644 --- a/ruff.toml +++ b/ruff.toml @@ -11,27 +11,95 @@ 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 + "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 +] + +[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] +ignore-fully-untyped = true \ No newline at end of file From 2061aa8dec1572b0c6a9015681dc602765cd1ce1 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Tue, 16 Apr 2024 15:03:36 +0200 Subject: [PATCH 02/27] chore: linted --- pyproject.toml | 1 + timed/authentication.py | 45 +++---- timed/conftest.py | 29 ++--- timed/employment/admin.py | 38 +++--- timed/employment/filters.py | 41 +++--- timed/employment/models.py | 61 ++++----- timed/employment/serializers.py | 48 +++---- timed/employment/tests/__init__.py | 1 - .../employment/tests/test_absence_balance.py | 8 +- timed/employment/tests/test_absence_type.py | 4 +- timed/employment/tests/test_employment.py | 18 ++- timed/employment/tests/test_location.py | 4 +- timed/employment/tests/test_public_holiday.py | 4 +- timed/employment/tests/test_user.py | 17 ++- .../employment/tests/test_worktime_balance.py | 4 +- timed/employment/views.py | 123 +++++++++--------- timed/mixins.py | 8 +- timed/models.py | 3 +- .../management/commands/__init__.py | 0 .../commands/notify_changed_employments.py | 5 +- .../commands/notify_reviewers_unverified.py | 6 +- .../commands/notify_supervisors_shorttime.py | 9 +- timed/notifications/models.py | 6 +- .../notifications/tests/test_budget_check.py | 14 +- .../tests/test_notify_changed_employments.py | 9 +- .../tests/test_notify_reviewers_unverified.py | 15 ++- .../test_notify_supervisors_shorttime.py | 10 +- timed/permissions.py | 24 ++-- timed/projects/admin.py | 41 +++--- timed/projects/filters.py | 23 ++-- timed/projects/models.py | 55 ++++---- timed/projects/serializers.py | 48 +++---- timed/projects/tests/__init__.py | 1 - timed/projects/tests/test_billing_type.py | 10 +- timed/projects/tests/test_cost_center.py | 2 +- timed/projects/tests/test_customer.py | 4 +- .../projects/tests/test_customer_assignee.py | 2 +- timed/projects/tests/test_project.py | 9 +- timed/projects/tests/test_project_assignee.py | 2 +- timed/projects/tests/test_task.py | 29 ++++- timed/projects/tests/test_task_assignee.py | 2 +- timed/projects/views.py | 84 ++++++------ timed/redmine/management/__init__.py | 0 timed/redmine/management/commands/__init__.py | 0 .../management/commands/redmine_report.py | 6 +- timed/redmine/models.py | 3 +- timed/redmine/tests/test_redmine_report.py | 26 ++-- .../tests/test_update_project_expenditure.py | 6 +- timed/reports/filters.py | 32 ++--- timed/reports/serializers.py | 14 +- .../reports/tests/test_customer_statistic.py | 16 +-- timed/reports/tests/test_month_statistic.py | 2 +- timed/reports/tests/test_project_statistic.py | 4 +- timed/reports/tests/test_task_statistic.py | 4 +- timed/reports/tests/test_user_statistic.py | 2 +- timed/reports/tests/test_work_report.py | 25 ++-- timed/reports/tests/test_year_statistic.py | 4 +- timed/reports/views.py | 122 ++++++++--------- timed/serializers.py | 5 +- timed/settings.py | 29 +++-- timed/subscription/admin.py | 2 +- timed/subscription/serializers.py | 13 +- timed/subscription/tests/test_order.py | 21 ++- .../tests/test_subscription_project.py | 4 +- timed/subscription/views.py | 34 ++--- timed/tests/test_authentication.py | 35 ++--- timed/tracking/filters.py | 32 ++--- timed/tracking/models.py | 55 ++++---- timed/tracking/serializers.py | 60 +++++---- timed/tracking/tasks.py | 3 +- timed/tracking/templatetags/__init__.py | 0 timed/tracking/tests/__init__.py | 1 - timed/tracking/tests/test_absence.py | 2 +- timed/tracking/tests/test_activity.py | 21 ++- timed/tracking/tests/test_attendance.py | 2 +- timed/tracking/tests/test_report.py | 87 +++++++------ timed/tracking/views.py | 106 ++++++++------- timed/wsgi.py | 3 +- 78 files changed, 863 insertions(+), 785 deletions(-) create mode 100644 timed/notifications/management/commands/__init__.py create mode 100644 timed/redmine/management/__init__.py create mode 100644 timed/redmine/management/commands/__init__.py create mode 100644 timed/tracking/templatetags/__init__.py diff --git a/pyproject.toml b/pyproject.toml index c8eddfa2..c7723d62 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -100,6 +100,7 @@ omit = [ "*/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/timed/authentication.py b/timed/authentication.py index e5f7dac7..9a4f8dca 100644 --- a/timed/authentication.py +++ b/timed/authentication.py @@ -14,11 +14,8 @@ class TimedOIDCAuthenticationBackend(OIDCAuthenticationBackend): 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 + return self.cached_request(self.get_userinfo, access_token, "auth.userinfo") except requests.HTTPError as e: if e.response.status_code not in [401, 403]: - raise e + 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 None 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, "") @@ -120,4 +112,5 @@ def get_username(self, claims): try: return claims[settings.OIDC_USERNAME_CLAIM] except KeyError: - raise SuspiciousOperation("Couldn't find username claim") + msg = "Couldn't find username claim" + raise SuspiciousOperation(msg) from None diff --git a/timed/conftest.py b/timed/conftest.py index 725c8223..0953b70d 100644 --- a/timed/conftest.py +++ b/timed/conftest.py @@ -14,7 +14,7 @@ def register_module(module): - for name, obj in inspect.getmembers(module): + for _name, obj in inspect.getmembers(module): if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract: register(obj) @@ -25,7 +25,7 @@ def register_module(module): register_module(tracking_factories) -@pytest.fixture +@pytest.fixture() def auth_user(db): return get_user_model().objects.create_user( username="user", @@ -37,7 +37,7 @@ def auth_user(db): ) -@pytest.fixture +@pytest.fixture() def admin_user(db): return get_user_model().objects.create_user( username="admin", @@ -49,7 +49,7 @@ def admin_user(db): ) -@pytest.fixture +@pytest.fixture() def superadmin_user(db): return get_user_model().objects.create_user( username="superadmin", @@ -61,7 +61,7 @@ def superadmin_user(db): ) -@pytest.fixture +@pytest.fixture() def external_employee(db): user = get_user_model().objects.create_user( username="user", @@ -75,7 +75,7 @@ def external_employee(db): return user -@pytest.fixture +@pytest.fixture() def internal_employee(db): user = get_user_model().objects.create_user( username="user", @@ -90,12 +90,12 @@ def internal_employee(db): return user -@pytest.fixture +@pytest.fixture() def client(): return APIClient() -@pytest.fixture +@pytest.fixture() def auth_client(auth_user): """Return instance of a APIClient that is logged in as test user.""" client = APIClient() @@ -104,7 +104,7 @@ def auth_client(auth_user): return client -@pytest.fixture +@pytest.fixture() def admin_client(admin_user): """Return instance of a APIClient that is logged in as a staff user.""" client = APIClient() @@ -113,7 +113,7 @@ def admin_client(admin_user): return client -@pytest.fixture +@pytest.fixture() def superadmin_client(superadmin_user): """Return instance of a APIClient that is logged in as superuser.""" client = APIClient() @@ -122,7 +122,7 @@ def superadmin_client(superadmin_user): return client -@pytest.fixture +@pytest.fixture() def external_employee_client(external_employee): """Return instance of a APIClient that is logged in as external test user.""" client = APIClient() @@ -131,7 +131,7 @@ def external_employee_client(external_employee): return client -@pytest.fixture +@pytest.fixture() def internal_employee_client(internal_employee): """Return instance of a APIClient that is logged in as external test user.""" client = APIClient() @@ -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..ec29c4c1 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 @@ -101,11 +101,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 +114,7 @@ def clean(self): class Meta: """Meta information for the employment form.""" - fields = "__all__" + fields = "__all__" # noqa: DJ007 model = models.Employment @@ -146,22 +144,22 @@ 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"] + search_fields = ("username",) - actions = [ + actions = ( "disable_users", "enable_users", "disable_staff_status", "enable_staff_status", - ] + ) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -197,8 +195,8 @@ 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): return obj and not obj.employments.exists() @@ -208,15 +206,15 @@ 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): return obj and not obj.absences.exists() and not obj.absencecredit_set.exists() diff --git a/timed/employment/filters.py b/timed/employment/filters.py index c74a8b13..d50b2b6f 100644 --- a/timed/employment/filters.py +++ b/timed/employment/filters.py @@ -27,7 +27,7 @@ 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 +37,7 @@ class Meta: """Meta information for the public holiday filter set.""" model = models.AbsenceType - fields = ["fill_worktime"] + fields = ("fill_worktime",) class UserFilterSet(FilterSet): @@ -63,29 +63,30 @@ def filter_is_supervisor(self, queryset, name, value): 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( + 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 +96,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 +112,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 +136,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..2e1b3228 100644 --- a/timed/employment/models.py +++ b/timed/employment/models.py @@ -27,6 +27,9 @@ class Location(models.Model): Workdays defined per location, default is Monday - Friday """ + class Meta: + ordering = ("name",) + def __str__(self): """Represent the model as a string. @@ -35,9 +38,6 @@ def __str__(self): """ return self.name - class Meta: - ordering = ("name",) - class PublicHoliday(models.Model): """Public holiday model. @@ -52,19 +52,19 @@ class PublicHoliday(models.Model): Location, on_delete=models.CASCADE, related_name="public_holidays" ) + class Meta: + """Meta information for the public holiday model.""" + + indexes = (models.Index(fields=["date"]),) + ordering = ("date",) + 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"])] - ordering = ("date",) + return "{} {}".format(self.name, self.date.strftime("%Y")) class AbsenceType(models.Model): @@ -77,6 +77,9 @@ class AbsenceType(models.Model): name = models.CharField(max_length=50) fill_worktime = models.BooleanField(default=False) + class Meta: + ordering = ("name",) + def __str__(self): """Represent the model as a string. @@ -86,25 +89,21 @@ def __str__(self): 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 +113,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): @@ -219,19 +214,25 @@ 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) + objects = EmploymentManager() + + class Meta: + """Meta information for the employment model.""" + + indexes = (models.Index(fields=["start_date", "end_date"]),) + def __str__(self): """Represent the model as a string. :return: The string representation :rtype: str """ - return "{0} ({1} - {2})".format( + 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 +275,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 +311,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 +406,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/serializers.py b/timed/employment/serializers.py index 0a4490a0..20f682e5 100644 --- a/timed/employment/serializers.py +++ b/timed/employment/serializers.py @@ -1,6 +1,7 @@ """Serializers for the employment app.""" from datetime import date, timedelta +from typing import ClassVar from django.contrib.auth import get_user_model from django.db.models import Max, Value @@ -20,7 +21,7 @@ class UserSerializer(ModelSerializer): - included_serializers = { + included_serializers: ClassVar = { "supervisors": "timed.employment.serializers.UserSerializer", "supervisees": "timed.employment.serializers.UserSerializer", } @@ -29,7 +30,7 @@ class Meta: """Meta information for the user serializer.""" model = get_user_model() - fields = [ + fields = ( "email", "first_name", "is_active", @@ -42,8 +43,8 @@ class Meta: "username", "is_reviewer", "is_accountant", - ] - read_only_fields = [ + ) + read_only_fields = ( "first_name", "is_active", "is_staff", @@ -54,7 +55,7 @@ class Meta: "username", "is_reviewer", "is_accountant", - ] + ) class WorktimeBalanceSerializer(Serializer): @@ -94,7 +95,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 = { + "user": "timed.employment.serializers.UserSerializer" + } class Meta: resource_name = "worktime-balances" @@ -123,8 +126,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 +145,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 +164,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 +217,7 @@ def get_balance(self, instance): return self.get_credit(instance) - self.get_used_days(instance) - included_serializers = { + included_serializers: ClassVar = { "absence_type": "timed.employment.serializers.AbsenceTypeSerializer", "absence_credits": "timed.employment.serializers.AbsenceCreditSerializer", } @@ -227,7 +227,7 @@ class Meta: class EmploymentSerializer(ModelSerializer): - included_serializers = { + included_serializers: ClassVar = { "user": "timed.employment.serializers.UserSerializer", "location": "timed.employment.serializers.LocationSerializer", } @@ -258,7 +258,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 +267,7 @@ def validate(self, data): class Meta: model = models.Employment - fields = [ + fields = ( "user", "location", "percentage", @@ -275,7 +275,7 @@ class Meta: "start_date", "end_date", "is_external", - ] + ) class LocationSerializer(ModelSerializer): @@ -285,7 +285,7 @@ class Meta: """Meta information for the location serializer.""" model = models.Location - fields = ["name", "workdays"] + fields = ("name", "workdays") class PublicHolidaySerializer(ModelSerializer): @@ -293,7 +293,7 @@ class PublicHolidaySerializer(ModelSerializer): location = relations.ResourceRelatedField(read_only=True) - included_serializers = { + included_serializers: ClassVar = { "location": "timed.employment.serializers.LocationSerializer" } @@ -301,7 +301,7 @@ 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 +311,13 @@ 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 = { "absence_type": "timed.employment.serializers.AbsenceTypeSerializer" } @@ -325,10 +325,10 @@ 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..de8e575f 100644 --- a/timed/employment/tests/test_employment.py +++ b/timed/employment/tests/test_employment.py @@ -136,7 +136,8 @@ 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) @@ -147,7 +148,8 @@ def test_employment_unique_active(db): 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)}, @@ -158,7 +160,8 @@ def test_employment_start_before_end(db): 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..3e3347b9 100644 --- a/timed/employment/tests/test_location.py +++ b/timed/employment/tests/test_location.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), @@ -38,7 +38,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..a229a363 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -30,23 +30,24 @@ 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 @@ -79,7 +80,6 @@ def get_queryset(self): ) 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 +92,21 @@ 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 + else: + 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) + 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. + """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 +161,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. @@ -174,7 +174,7 @@ def _extract_date(self): return datetime.datetime.strptime(pk.split("_")[1], "%Y-%m-%d") except (ValueError, TypeError, IndexError): - raise exceptions.NotFound() + raise exceptions.NotFound from None # list case query_params = self.request.query_params @@ -183,10 +183,10 @@ def _extract_date(self): query_params.get("date"), "%Y-%m-%d" ).date() except ValueError: - raise exceptions.ParseError(_("Date is invalid")) + raise exceptions.ParseError(_("Date is invalid")) from None except TypeError: 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 None return None @@ -220,8 +220,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. @@ -234,7 +233,7 @@ def _extract_date(self): return datetime.datetime.strptime(pk.split("_")[2], "%Y-%m-%d") except (ValueError, TypeError, IndexError): - raise exceptions.NotFound() + raise exceptions.NotFound from None # list case try: @@ -242,13 +241,12 @@ def _extract_date(self): self.request.query_params.get("date"), "%Y-%m-%d" ).date() except ValueError: - raise exceptions.ParseError(_("Date is invalid")) + raise exceptions.ParseError(_("Date is invalid")) from None except TypeError: - raise exceptions.ParseError(_("Date filter needs to be set")) + raise exceptions.ParseError(_("Date filter needs to be set")) from None 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. @@ -264,7 +262,7 @@ def _extract_user(self): return self.request.user return get_user_model().objects.get(pk=pk.split("_")[0]) except (ValueError, get_user_model().DoesNotExist): - raise exceptions.NotFound() + raise exceptions.NotFound from None # list case try: @@ -278,7 +276,7 @@ def _extract_user(self): return get_user_model().objects.get(pk=user_id) except (ValueError, get_user_model().DoesNotExist): - raise exceptions.ParseError(_("User is invalid")) + raise exceptions.ParseError(_("User is invalid")) from None def get_queryset(self): date = self._extract_date() @@ -301,10 +299,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 +313,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 +406,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 +438,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..a36d30c0 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. """ 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/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..dfbb1a06 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,8 +82,7 @@ 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. 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..0870ec0f 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, capsys, 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..c8b3919a 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 @@ -67,8 +66,7 @@ def has_object_permission(self, request, view, obj): 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. @@ -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( @@ -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..00080040 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,9 +33,9 @@ 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): return obj and not obj.projects.exists() @@ -43,19 +43,21 @@ def has_delete_permission(self, request, obj=None): @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,7 +93,7 @@ class TaskInline(NestedStackedInline): form = TaskForm model = models.Task extra = 0 - inlines = [TaskAssigneeInline] + inlines = (TaskAssigneeInline,) def has_delete_permission(self, request, obj=None): # for some reason obj is parent object and not task @@ -112,11 +114,14 @@ 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): return obj and not obj.tasks.exists() @@ -126,4 +131,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..8143f62e 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): @@ -69,16 +72,18 @@ 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. - TODO: + 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): @@ -107,9 +112,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 +126,7 @@ 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 +140,7 @@ 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 +154,7 @@ 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 +168,4 @@ 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/models.py b/timed/projects/models.py index a8b6acb5..73e2e07e 100644 --- a/timed/projects/models.py +++ b/timed/projects/models.py @@ -31,6 +31,11 @@ class Customer(models.Model): related_name="assigned_to_customers", ) + class Meta: + """Meta informations for the customer model.""" + + ordering = ("name",) + def __str__(self): """Represent the model as a string. @@ -39,11 +44,6 @@ def __str__(self): """ return self.name - class Meta: - """Meta informations for the customer model.""" - - ordering = ["name"] - class CostCenter(models.Model): """Cost center defining how cost of projects and tasks are allocated.""" @@ -51,12 +51,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 +64,12 @@ 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): return self.name - class Meta: - ordering = ["name"] - class Project(models.Model): """Project model. @@ -116,16 +116,16 @@ class Project(models.Model): remaining_effort_tracking = models.BooleanField(default=False) total_remaining_effort = models.DurationField(default=timedelta(0)) + class Meta: + ordering = ("name",) + 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"] + return f"{self.customer} > {self.name}" class Task(models.Model): @@ -162,18 +162,18 @@ class Task(models.Model): ) most_recent_remaining_effort = models.DurationField(blank=True, null=True) + class Meta: + """Meta informations for the task model.""" + + ordering = ("name",) + 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"] + return f"{self.project} > {self.name}" class TaskTemplate(models.Model): @@ -185,6 +185,9 @@ class TaskTemplate(models.Model): name = models.CharField(max_length=255) + class Meta: + ordering = ("name",) + def __str__(self): """Represent the model as a string. @@ -193,9 +196,6 @@ def __str__(self): """ return self.name - class Meta: - ordering = ["name"] - class CustomerAssignee(models.Model): """Customer assignee model. @@ -272,8 +272,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..8743bf54 100644 --- a/timed/projects/serializers.py +++ b/timed/projects/serializers.py @@ -1,6 +1,7 @@ """Serializers for the projects app.""" from datetime import timedelta +from typing import ClassVar from django.db.models import Q, Sum from django.utils.duration import duration_string @@ -18,26 +19,26 @@ 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,7 +47,7 @@ class ProjectSerializer(ModelSerializer): customer = ResourceRelatedField(queryset=models.Customer.objects.all()) billing_type = ResourceRelatedField(queryset=models.BillingType.objects.all()) - included_serializers = { + included_serializers: ClassVar = { "customer": "timed.projects.serializers.CustomerSerializer", "billing_type": "timed.projects.serializers.BillingTypeSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", @@ -87,16 +88,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 +109,7 @@ class Meta: "customer_visible", "remaining_effort_tracking", "total_remaining_effort", - ] + ) class TaskSerializer(ModelSerializer): @@ -117,7 +117,7 @@ class TaskSerializer(ModelSerializer): project = ResourceRelatedField(queryset=models.Project.objects.all()) - included_serializers = { + included_serializers: ClassVar = { "activities": "timed.tracking.serializers.ActivitySerializer", "project": "timed.projects.serializers.ProjectSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", @@ -165,8 +165,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 +183,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 +197,13 @@ class Meta: "project", "cost_center", "most_recent_remaining_effort", - ] + ) class CustomerAssigneeSerializer(ModelSerializer): """Customer assignee serializer.""" - included_serializers = { + included_serializers: ClassVar = { "user": "timed.employment.serializers.UserSerializer", "customer": "timed.projects.serializers.CustomerSerializer", } @@ -210,20 +212,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 = { "user": "timed.employment.serializers.UserSerializer", "project": "timed.projects.serializers.ProjectSerializer", } @@ -232,20 +234,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 = { "user": "timed.employment.serializers.UserSerializer", "task": "timed.projects.serializers.TaskSerializer", } @@ -254,11 +256,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..76f98622 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 ): @@ -188,7 +189,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 +214,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..10cb21bc 100644 --- a/timed/projects/views.py +++ b/timed/projects/views.py @@ -54,12 +54,14 @@ 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 - ] + 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 - ] + 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() @@ -118,16 +120,18 @@ class ProjectViewSet(ModelViewSet): 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.""" @@ -162,14 +166,16 @@ class TaskViewSet(ModelViewSet): 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 - ] + 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..dc4251fa 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. """ 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..d90cf491 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,8 +44,9 @@ def test_update_project_expenditure( assert f"amount invoiced {project.amount_invoiced.amount}" in out +@pytest.mark.django_db() def test_update_project_expenditure_invalid_issue( - db, freezer, mocker, capsys, report_factory + freezer, mocker, capsys, report_factory ): redmine_instance = mocker.MagicMock() redmine_class = mocker.patch("redminelib.Redmine") diff --git a/timed/reports/filters.py b/timed/reports/filters.py index 113e26b2..74c8599a 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): # noqa: ARG002 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): # noqa: ARG002 + """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? # noqa: TD002,TD003 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..d85e34f3 100644 --- a/timed/reports/serializers.py +++ b/timed/reports/serializers.py @@ -1,3 +1,5 @@ +from typing import ClassVar + from django.contrib.auth import get_user_model from rest_framework_json_api import relations from rest_framework_json_api.serializers import ( @@ -48,7 +50,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 = { + "customer": "timed.projects.serializers.CustomerSerializer" + } class Meta: resource_name = "project-statistics" @@ -61,7 +65,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 = { + "project": "timed.projects.serializers.ProjectSerializer" + } class Meta: resource_name = "task-statistics" @@ -71,7 +77,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 = { + "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..044bd1d5 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,18 +78,16 @@ 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): @@ -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..45bf9f24 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,7 +105,7 @@ 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): diff --git a/timed/reports/tests/test_task_statistic.py b/timed/reports/tests/test_task_statistic.py index 94918e20..ce92a375 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,7 +93,7 @@ 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( 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..877876fc 100644 --- a/timed/reports/views.py +++ b/timed/reports/views.py @@ -31,18 +31,18 @@ class YearStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): filterset_class = ReportFilterSet 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): @@ -52,10 +52,12 @@ class MonthStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): 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 - ] + 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 +66,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): @@ -81,10 +81,11 @@ def __init__(self, catch_prefixes, *args, base_qs=None, agg_filters=None, **kwar def filter(self, *args, **kwargs): if args: # pragma: no cover # This is a check against programming errors, no need to test - raise RuntimeError( + msg = ( "Unable to detect statistics filter type form Q objects. use " "filter_aggregate() or filter_base() instead" ) + raise RuntimeError(msg) my_filters = { k: v for k, v in kwargs.items() if not k.startswith(self._catch_prefixes) } @@ -117,11 +118,11 @@ def _clone(self): 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 +142,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 +166,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 +189,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__") @@ -206,23 +214,22 @@ class UserStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): filterset_class = ReportFilterSet 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 +268,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 +277,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 +363,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 +394,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..e8160adc 100644 --- a/timed/serializers.py +++ b/timed/serializers.py @@ -4,7 +4,7 @@ from django.utils.duration import duration_string -class TotalTimeRootMetaMixin(object): +class TotalTimeRootMetaMixin: duration_field = "duration" def get_root_meta(self, resource, many): @@ -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..674c22ee 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): # 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 diff --git a/timed/subscription/admin.py b/timed/subscription/admin.py index 53f96022..55bd09a5 100644 --- a/timed/subscription/admin.py +++ b/timed/subscription/admin.py @@ -14,5 +14,5 @@ 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/serializers.py b/timed/subscription/serializers.py index 1138e8cb..82894184 100644 --- a/timed/subscription/serializers.py +++ b/timed/subscription/serializers.py @@ -1,4 +1,5 @@ from datetime import timedelta +from typing import ClassVar from django.db.models import Sum from django.utils.duration import duration_string @@ -19,8 +20,7 @@ class SubscriptionProjectSerializer(ModelSerializer): 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 +29,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 +39,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 = { "billing_type": "timed.projects.serializers.BillingTypeSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", "customer": "timed.projects.serializers.CustomerSerializer", @@ -65,7 +64,7 @@ class PackageSerializer(ModelSerializer): price = CharField() """CharField needed as it includes currency.""" - included_serializers = { + included_serializers: ClassVar = { "billing_type": "timed.projects.serializers.BillingTypeSerializer" } @@ -76,7 +75,7 @@ class Meta: class OrderSerializer(ModelSerializer): - included_serializers = { + included_serializers: ClassVar = { "project": ("timed.subscription.serializers" ".SubscriptionProjectSerializer") } 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..63f1684a 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. @@ -58,14 +57,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 +76,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 +86,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) @@ -96,8 +97,7 @@ def create(self, request, *args, **kwargs): permission_classes=[IsSuperUser | IsAccountant], ) def confirm(self, request, pk=None): - """ - Confirm order. + """Confirm order. Only allowed by staff members """ @@ -115,7 +115,7 @@ def destroy(self, request, pk): 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/tracking/filters.py b/timed/tracking/filters.py index 482a7b53..64a9d348 100644 --- a/timed/tracking/filters.py +++ b/timed/tracking/filters.py @@ -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): @@ -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. + """Filter report by cost center. Cost center on task has higher priority over project cost center. @@ -243,4 +243,4 @@ 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..abf98ea4 100644 --- a/timed/tracking/models.py +++ b/timed/tracking/models.py @@ -31,19 +31,19 @@ class Activity(models.Model): settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="activities" ) + class Meta: + """Meta informations for the activity model.""" + + verbose_name_plural = "activities" + indexes = (models.Index(fields=["date"]),) + 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"])] + return f"{self.user}: {self.task}" class Attendance(models.Model): @@ -67,7 +67,7 @@ def __str__(self): :return: The string representation :rtype: str """ - return "{0}: {1} {2} - {3}".format( + return "{}: {} {} - {}".format( self.user, self.date.strftime("%Y-%m-%d"), self.from_time.strftime("%H:%M"), @@ -103,6 +103,19 @@ 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): + """Represent the model as a string. + + :return: The string representation + :rtype: str + """ + return f"{self.user}: {self.task}" + def save(self, *args, **kwargs): """Save the report with some custom functionality. @@ -115,19 +128,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 +145,13 @@ 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 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 +169,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..d45d2962 100644 --- a/timed/tracking/serializers.py +++ b/timed/tracking/serializers.py @@ -1,6 +1,7 @@ """Serializers for the tracking app.""" from datetime import date, timedelta +from typing import ClassVar from django.contrib.auth import get_user_model from django.db.models import BooleanField, Case, Q, When @@ -27,7 +28,7 @@ class ActivitySerializer(ModelSerializer): user = CurrentUserResourceRelatedField() - included_serializers = { + included_serializers: ClassVar = { "task": "timed.projects.serializers.TaskSerializer", "user": "timed.employment.serializers.UserSerializer", } @@ -74,13 +75,15 @@ class AttendanceSerializer(ModelSerializer): user = CurrentUserResourceRelatedField() - included_serializers = {"user": "timed.employment.serializers.UserSerializer"} + included_serializers: ClassVar = { + "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 +98,7 @@ class ReportSerializer(TotalTimeRootMetaMixin, ModelSerializer): queryset=get_user_model().objects, required=False, allow_null=True ) - included_serializers = { + included_serializers: ClassVar = { "task": "timed.projects.serializers.TaskSerializer", "user": "timed.employment.serializers.UserSerializer", "verified_by": "timed.employment.serializers.UserSerializer", @@ -106,7 +109,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 +123,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 +144,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 +153,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 +182,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 +203,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 +242,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 +263,7 @@ class Meta: "verified_by", "rejected", "remaining_effort", - ] + ) class ReportBulkSerializer(Serializer): @@ -279,8 +284,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 @@ -368,7 +372,7 @@ def get_root_meta(self, resource, many): queryset = self.instance["queryset"] return {"count": queryset.count()} - included_serializers = { + included_serializers: ClassVar = { "customer": "timed.projects.serializers.CustomerSerializer", "project": "timed.projects.serializers.ProjectSerializer", "task": "timed.projects.serializers.TaskSerializer", @@ -386,7 +390,7 @@ class AbsenceSerializer(ModelSerializer): absence_type = ResourceRelatedField(queryset=AbsenceType.objects.all()) user = CurrentUserResourceRelatedField() - included_serializers = { + included_serializers: ClassVar = { "user": "timed.employment.serializers.UserSerializer", "absence_type": "timed.employment.serializers.AbsenceTypeSerializer", } @@ -435,7 +439,7 @@ def validate(self, data): except Employment.DoesNotExist: # pragma: no cover raise ValidationError( _("You can't create an absence on an unemployed day.") - ) + ) from None if PublicHoliday.objects.filter( location_id=location.id, date=data.get("date") @@ -452,4 +456,4 @@ 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/tasks.py b/timed/tracking/tasks.py index 7422defb..c9bbbf9b 100644 --- a/timed/tracking/tasks.py +++ b/timed/tracking/tasks.py @@ -5,7 +5,6 @@ def _send_notification_emails(changes, reviewer, rejected=False): """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 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..bf6f653c 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), @@ -1427,10 +1440,10 @@ def test_report_notify_rendering( @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, project, task, report, needs_review ): EmploymentFactory.create(user=superadmin_client.user) data = { @@ -1482,7 +1495,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 +1549,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 +1642,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 +1678,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 +1705,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 +1779,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 +1816,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 +1878,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 +1926,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 +1934,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..aad7f28d 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,17 +96,19 @@ 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 - ] + 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", @@ -143,8 +149,7 @@ def get_queryset(self): project__customer__customer_assignees__is_reviewer=True, ) ) - queryset = queryset.filter(Q(task__in=assigned_tasks) | Q(user=user)) - return queryset + return queryset.filter(Q(task__in=assigned_tasks) | Q(user=user)) except Employment.DoesNotExist: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): return queryset.filter( @@ -153,13 +158,11 @@ 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 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) @@ -186,8 +189,7 @@ def update(self, request, *args, **kwargs): serializer_class=serializers.ReportIntersectionSerializer, ) def intersection(self, request): - """ - Get intersection in reports of common report fields. + """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 +250,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 +316,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 +361,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 +380,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 +391,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``. From 064a325c947c0313fb5f2ca6883ee869b1fa61cb Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Tue, 16 Apr 2024 15:07:52 +0200 Subject: [PATCH 03/27] chore: run makemigrations --- ...tions_alter_costcenter_options_and_more.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 timed/projects/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py diff --git a/timed/projects/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py b/timed/projects/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py new file mode 100644 index 00000000..5f5e0a33 --- /dev/null +++ b/timed/projects/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.11 on 2024-04-16 13:06 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0016_alter_project_amount_invoiced_currency_and_more'), + ] + + operations = [ + migrations.AlterModelOptions( + name='billingtype', + options={'ordering': ('name',)}, + ), + migrations.AlterModelOptions( + name='costcenter', + 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',)}, + ), + ] From 0ca6159d3657175c53496699c6581c1bf8d29145 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Tue, 16 Apr 2024 16:36:07 +0200 Subject: [PATCH 04/27] chore: raise from exc instead of None --- timed/authentication.py | 10 +++++----- timed/employment/views.py | 36 +++++++++++++++++------------------ timed/tracking/serializers.py | 4 ++-- timed/tracking/views.py | 4 ++-- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/timed/authentication.py b/timed/authentication.py index 9a4f8dca..485f9942 100644 --- a/timed/authentication.py +++ b/timed/authentication.py @@ -34,8 +34,8 @@ def get_introspection(self, access_token, id_token, payload): def get_userinfo_or_introspection(self, access_token): try: return self.cached_request(self.get_userinfo, access_token, "auth.userinfo") - except requests.HTTPError as e: - if e.response.status_code not in [401, 403]: + except requests.HTTPError as exc: + if exc.response.status_code not in [401, 403]: raise if settings.OIDC_CHECK_INTROSPECT: try: @@ -55,7 +55,7 @@ def get_userinfo_or_introspection(self, access_token): raise else: return claims - raise AuthenticationFailed from None + raise AuthenticationFailed from exc def get_or_create_user(self, access_token, id_token, payload): """Verify claims and return user, otherwise raise an Exception.""" @@ -111,6 +111,6 @@ def create_user(self, claims): def get_username(self, claims): try: return claims[settings.OIDC_USERNAME_CLAIM] - except KeyError: + except KeyError as exc: msg = "Couldn't find username claim" - raise SuspiciousOperation(msg) from None + raise SuspiciousOperation(msg) from exc diff --git a/timed/employment/views.py b/timed/employment/views.py index a229a363..34594d95 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -80,7 +80,7 @@ def get_queryset(self): ) return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) - except models.Employment.DoesNotExist: + except models.Employment.DoesNotExist as exc: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): assigned_tasks = Task.objects.filter( Q( @@ -93,7 +93,7 @@ def get_queryset(self): ) return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) msg = "User has no employment" - raise exceptions.PermissionDenied(msg) from None + raise exceptions.PermissionDenied(msg) from exc else: return queryset @@ -173,8 +173,8 @@ def _extract_date(self): try: return datetime.datetime.strptime(pk.split("_")[1], "%Y-%m-%d") - except (ValueError, TypeError, IndexError): - raise exceptions.NotFound from None + except (ValueError, TypeError, IndexError) as exc: + raise exceptions.NotFound from exc # list case query_params = self.request.query_params @@ -182,11 +182,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")) from None - 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")) from None + raise exceptions.ParseError(_("Date filter needs to be set")) from exc return None @@ -232,18 +232,18 @@ def _extract_date(self): try: return datetime.datetime.strptime(pk.split("_")[2], "%Y-%m-%d") - except (ValueError, TypeError, IndexError): - raise exceptions.NotFound from None + 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")) from None - except TypeError: - raise exceptions.ParseError(_("Date filter needs to be set")) from None + 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. @@ -261,8 +261,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 from None + except (ValueError, get_user_model().DoesNotExist) as exc: + raise exceptions.NotFound from exc # list case try: @@ -275,8 +275,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")) from None + except (ValueError, get_user_model().DoesNotExist) as exc: + raise exceptions.ParseError(_("User is invalid")) from exc def get_queryset(self): date = self._extract_date() diff --git a/timed/tracking/serializers.py b/timed/tracking/serializers.py index d45d2962..6620b022 100644 --- a/timed/tracking/serializers.py +++ b/timed/tracking/serializers.py @@ -436,10 +436,10 @@ def validate(self, data): 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 None + ) from exc if PublicHoliday.objects.filter( location_id=location.id, date=data.get("date") diff --git a/timed/tracking/views.py b/timed/tracking/views.py index aad7f28d..bcd1ac93 100644 --- a/timed/tracking/views.py +++ b/timed/tracking/views.py @@ -150,7 +150,7 @@ def get_queryset(self): ) ) return queryset.filter(Q(task__in=assigned_tasks) | Q(user=user)) - except Employment.DoesNotExist: + except Employment.DoesNotExist as exc: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): return queryset.filter( Q( @@ -159,7 +159,7 @@ def get_queryset(self): ) ) msg = "User has no employment and isn't a customer!" - raise exceptions.PermissionDenied(msg) from None + raise exceptions.PermissionDenied(msg) from exc def update(self, request, *args, **kwargs): """Override so we can issue emails on update.""" From f7f7f6f945097890a92f17097730714269063604 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 09:28:59 +0200 Subject: [PATCH 05/27] chore(linting): ignore-variadic-names for ARG --- ruff.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ruff.toml b/ruff.toml index d0deb711..836b9dee 100644 --- a/ruff.toml +++ b/ruff.toml @@ -102,4 +102,7 @@ known-third-party = ["pytest_factoryboy"] combine-as-imports = true [lint.flake8-annotations] -ignore-fully-untyped = true \ No newline at end of file +ignore-fully-untyped = true + +[lint.flake8-unused-arguments] +ignore-variadic-names = true \ No newline at end of file From 86f253ae539e431c2163037220a8ce17efd1cebe Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:02:20 +0200 Subject: [PATCH 06/27] chore: make ruff happier --- timed/authentication.py | 4 +-- timed/employment/admin.py | 14 +++++----- timed/employment/filters.py | 8 +++--- timed/employment/permissions.py | 2 +- timed/employment/tests/test_employment.py | 4 +-- timed/mixins.py | 2 +- .../management/commands/budget_check.py | 4 +-- .../commands/notify_reviewers_unverified.py | 4 +-- .../notifications/tests/test_budget_check.py | 2 +- timed/permissions.py | 28 +++++++++---------- timed/projects/admin.py | 7 ++--- timed/projects/filters.py | 4 +-- timed/projects/serializers.py | 4 +-- timed/projects/tests/test_project.py | 3 +- .../tests/test_update_project_expenditure.py | 5 ++-- timed/reports/filters.py | 4 +-- .../reports/tests/test_customer_statistic.py | 2 +- timed/reports/tests/test_project_statistic.py | 2 +- timed/reports/tests/test_task_statistic.py | 2 +- timed/reports/views.py | 2 +- timed/serializers.py | 2 +- timed/subscription/filters.py | 2 +- timed/subscription/views.py | 4 +-- timed/tests/test_settings.py | 2 +- timed/tracking/filters.py | 8 +++--- timed/tracking/serializers.py | 2 +- timed/tracking/signals.py | 4 +-- timed/tracking/tasks.py | 2 +- 28 files changed, 66 insertions(+), 67 deletions(-) diff --git a/timed/authentication.py b/timed/authentication.py index 485f9942..0242d241 100644 --- a/timed/authentication.py +++ b/timed/authentication.py @@ -12,7 +12,7 @@ class TimedOIDCAuthenticationBackend(OIDCAuthenticationBackend): - def get_introspection(self, access_token, id_token, payload): + def get_introspection(self, access_token, *args, **kwargs): """Return user details dictionary.""" basic = base64.b64encode( f"{settings.OIDC_RP_INTROSPECT_CLIENT_ID}:{settings.OIDC_RP_INTROSPECT_CLIENT_SECRET}".encode() @@ -57,7 +57,7 @@ def get_userinfo_or_introspection(self, access_token): 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, *args, **kwargs): """Verify claims and return user, otherwise raise an Exception.""" claims = self.get_userinfo_or_introspection(access_token) diff --git a/timed/employment/admin.py b/timed/employment/admin.py index ec29c4c1..5c14ebd7 100644 --- a/timed/employment/admin.py +++ b/timed/employment/admin.py @@ -167,27 +167,27 @@ def __init__(self, *args, **kwargs): (_("Extra fields"), {"fields": ["tour_done", "is_accountant"]}), ) - def disable_users(self, request, queryset): + def disable_users(self, _, queryset): queryset.update(is_active=False) disable_users.short_description = _("Disable selected users") - def enable_users(self, request, queryset): + def enable_users(self, _, 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, _, 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, _, 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, _, obj=None): return obj and not obj.reports.exists() @@ -198,7 +198,7 @@ class LocationAdmin(admin.ModelAdmin): list_display = ("name",) search_fields = ("name",) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _, obj=None): return obj and not obj.employments.exists() @@ -216,5 +216,5 @@ class AbsenceTypeAdmin(admin.ModelAdmin): list_display = ("name",) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _, obj=None): return obj and not obj.absences.exists() and not obj.absencecredit_set.exists() diff --git a/timed/employment/filters.py b/timed/employment/filters.py index d50b2b6f..91e57f39 100644 --- a/timed/employment/filters.py +++ b/timed/employment/filters.py @@ -48,15 +48,15 @@ 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, _, value): return queryset.filter(employments__is_external=value) - def filter_is_reviewer(self, queryset, name, value): + def filter_is_reviewer(self, queryset, _, 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, _, value): if value: return queryset.filter(pk__in=User.objects.all_supervisors()) return queryset.exclude(pk__in=User.objects.all_supervisors()) @@ -75,7 +75,7 @@ class Meta: class EmploymentFilterSet(FilterSet): date = DateFilter(method="filter_date") - def filter_date(self, queryset, name, value): + def filter_date(self, queryset, _, value): return queryset.filter( Q(start_date__lte=value) & Q(Q(end_date__gte=value) | Q(end_date__isnull=True)) diff --git a/timed/employment/permissions.py b/timed/employment/permissions.py index 218b5f4d..7ab3d69b 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, _, __, obj): return not obj.reports.exists() diff --git a/timed/employment/tests/test_employment.py b/timed/employment/tests/test_employment.py index de8e575f..06d24e92 100644 --- a/timed/employment/tests/test_employment.py +++ b/timed/employment/tests/test_employment.py @@ -144,7 +144,7 @@ def test_employment_unique_active(): employment = EmploymentFactory.create(user=user) form = EmploymentForm({"end_date": None}, instance=employment) - with pytest.raises(ValueError): + with pytest.raises(ValueError): # noqa: PT011 form.save() @@ -156,7 +156,7 @@ def test_employment_start_before_end(): instance=employment, ) - with pytest.raises(ValueError): + with pytest.raises(ValueError): # noqa: PT011 form.save() diff --git a/timed/mixins.py b/timed/mixins.py index a36d30c0..3ee36a4a 100644 --- a/timed/mixins.py +++ b/timed/mixins.py @@ -50,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/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_reviewers_unverified.py b/timed/notifications/management/commands/notify_reviewers_unverified.py index dfbb1a06..7197d6a9 100644 --- a/timed/notifications/management/commands/notify_reviewers_unverified.py +++ b/timed/notifications/management/commands/notify_reviewers_unverified.py @@ -96,8 +96,8 @@ def _notify_reviewers(self, start, end, reports, optional_message, cc): 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/tests/test_budget_check.py b/timed/notifications/tests/test_budget_check.py index 0870ec0f..d1dc1082 100644 --- a/timed/notifications/tests/test_budget_check.py +++ b/timed/notifications/tests/test_budget_check.py @@ -87,7 +87,7 @@ def test_budget_check_skip_notification(capsys, mocker, report_factory): @pytest.mark.django_db() -def test_budget_check_no_estimated_timed(mocker, capsys, report_factory): +def test_budget_check_no_estimated_timed(mocker, report_factory): redmine_instance = mocker.MagicMock() issue = mocker.MagicMock() redmine_instance.issue.get.return_value = issue diff --git a/timed/permissions.py b/timed/permissions.py index c8b3919a..2e9d41b3 100644 --- a/timed/permissions.py +++ b/timed/permissions.py @@ -11,57 +11,57 @@ class IsUnverified(BasePermission): """Allows access only to verified objects.""" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, _, __, 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, *args, **kwargs): return request.method in SAFE_METHODS - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, *args, **kwargs): return self.has_permission(request, view) class IsDeleteOnly(BasePermission): """Allows only delete method.""" - def has_permission(self, request, view): + def has_permission(self, request, *args, **kwargs): return request.method == "DELETE" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, *args, **kwargs): return self.has_permission(request, view) class IsNotDelete(BasePermission): """Disallow delete method.""" - def has_permission(self, request, view): + def has_permission(self, request, *args, **kwargs): return request.method != "DELETE" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, *args, **kwargs): return self.has_permission(request, view) class IsCreateOnly(BasePermission): """Allows only create method.""" - def has_permission(self, request, view): + def has_permission(self, request, *args, **kwargs): return request.method == "POST" - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, *args, **kwargs): return self.has_permission(request, view) class IsUpdateOnly(BasePermission): """Allows only update method.""" - def has_permission(self, request, view): + def has_permission(self, request, *args, **kwargs): return request.method in ["PATCH", "PUT"] - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, *args, **kwargs): return self.has_permission(request, view) @@ -72,7 +72,7 @@ class IsAuthenticated(IsAuthenticated): operator. """ - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, *args, **kwargs): return self.has_permission(request, view) @@ -154,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, *args, **kwargs): 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, _, __, obj): return not obj.transferred diff --git a/timed/projects/admin.py b/timed/projects/admin.py index 00080040..84b51752 100644 --- a/timed/projects/admin.py +++ b/timed/projects/admin.py @@ -37,7 +37,7 @@ class CustomerAdmin(admin.ModelAdmin): search_fields = ("name",) inlines = (CustomerAssigneeInline,) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _, obj=None): return obj and not obj.projects.exists() @@ -95,10 +95,9 @@ class TaskInline(NestedStackedInline): extra = 0 inlines = (TaskAssigneeInline,) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, *args, **kwargs): # for some reason obj is parent object and not task # so this doesn't work - # return obj and not obj.reports.exists() return False @@ -123,7 +122,7 @@ class ProjectAdmin(NestedModelAdmin): inlines = (TaskInline, RedmineProjectInline, ProjectAssigneeInline) - def has_delete_permission(self, request, obj=None): + def has_delete_permission(self, _, obj=None): return obj and not obj.tasks.exists() diff --git a/timed/projects/filters.py b/timed/projects/filters.py index 8143f62e..d11e34d5 100644 --- a/timed/projects/filters.py +++ b/timed/projects/filters.py @@ -36,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, _, value): if not value: # pragma: no cover return queryset return queryset.filter( @@ -52,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, _, value): if not value: # pragma: no cover return queryset return queryset.filter( diff --git a/timed/projects/serializers.py b/timed/projects/serializers.py index 8743bf54..786c8789 100644 --- a/timed/projects/serializers.py +++ b/timed/projects/serializers.py @@ -53,7 +53,7 @@ class ProjectSerializer(ModelSerializer): "cost_center": "timed.projects.serializers.CostCenterSerializer", } - def get_root_meta(self, resource, many): + def get_root_meta(self, _, many): if not many: queryset = Report.objects.filter(task__project=self.instance) data = queryset.aggregate(spent_time=Sum("duration")) @@ -123,7 +123,7 @@ class TaskSerializer(ModelSerializer): "cost_center": "timed.projects.serializers.CostCenterSerializer", } - def get_root_meta(self, resource, many): + def get_root_meta(self, _, many): if not many: queryset = Report.objects.filter(task=self.instance) data = queryset.aggregate(spent_time=Sum("duration")) diff --git a/timed/projects/tests/test_project.py b/timed/projects/tests/test_project.py index 76f98622..955ff59f 100644 --- a/timed/projects/tests/test_project.py +++ b/timed/projects/tests/test_project.py @@ -170,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 diff --git a/timed/redmine/tests/test_update_project_expenditure.py b/timed/redmine/tests/test_update_project_expenditure.py index d90cf491..0f0cd84a 100644 --- a/timed/redmine/tests/test_update_project_expenditure.py +++ b/timed/redmine/tests/test_update_project_expenditure.py @@ -45,9 +45,8 @@ def test_update_project_expenditure( @pytest.mark.django_db() -def test_update_project_expenditure_invalid_issue( - freezer, mocker, capsys, report_factory -): +@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 74c8599a..ac1e78f3 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): # noqa: ARG002 + def filter_has_reviewer(self, queryset, _, value): if not value: # pragma: no cover return queryset @@ -37,7 +37,7 @@ def filter_has_reviewer(self, queryset, name, value): # noqa: ARG002 ) return queryset.filter_aggregate(the_filter).filter_base(the_filter) - def filter_cost_center(self, queryset, name, value): # noqa: ARG002 + def filter_cost_center(self, queryset, _, value): """Filter report by cost center. The filter behaves slightly different depending on what the diff --git a/timed/reports/tests/test_customer_statistic.py b/timed/reports/tests/test_customer_statistic.py index 044bd1d5..c914732f 100644 --- a/timed/reports/tests/test_customer_statistic.py +++ b/timed/reports/tests/test_customer_statistic.py @@ -90,7 +90,7 @@ def test_customer_statistic_list( ("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, diff --git a/timed/reports/tests/test_project_statistic.py b/timed/reports/tests/test_project_statistic.py index 45bf9f24..dc2baf2b 100644 --- a/timed/reports/tests/test_project_statistic.py +++ b/timed/reports/tests/test_project_statistic.py @@ -108,7 +108,7 @@ def test_project_statistic_list( ("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 ce92a375..f80f36fa 100644 --- a/timed/reports/tests/test_task_statistic.py +++ b/timed/reports/tests/test_task_statistic.py @@ -98,7 +98,7 @@ def test_task_statistic_list( ) def test_task_statistic_filtered( auth_client, - filter, + filter, # noqa: A002 expected_result, ): user = auth_client.user diff --git a/timed/reports/views.py b/timed/reports/views.py index 877876fc..582d25df 100644 --- a/timed/reports/views.py +++ b/timed/reports/views.py @@ -113,7 +113,7 @@ 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, ) diff --git a/timed/serializers.py b/timed/serializers.py index e8160adc..152c508b 100644 --- a/timed/serializers.py +++ b/timed/serializers.py @@ -7,7 +7,7 @@ class TotalTimeRootMetaMixin: duration_field = "duration" - def get_root_meta(self, resource, many): + def get_root_meta(self, _, many): """Add total hours over whole result (not just page) to meta.""" if many: view = self.context["view"] diff --git a/timed/subscription/filters.py b/timed/subscription/filters.py index 986dd395..c5734ace 100644 --- a/timed/subscription/filters.py +++ b/timed/subscription/filters.py @@ -8,7 +8,7 @@ class PackageFilter(FilterSet): customer = NumberFilter(method="filter_customer") - def filter_customer(self, queryset, name, value): + def filter_customer(self, queryset, _, value): billing_types = Project.objects.filter(customer=value).values("billing_type") return queryset.filter(billing_type__in=billing_types) diff --git a/timed/subscription/views.py b/timed/subscription/views.py index 63f1684a..4e9e305e 100644 --- a/timed/subscription/views.py +++ b/timed/subscription/views.py @@ -96,7 +96,7 @@ def create(self, request, *args, **kwargs): methods=["post"], permission_classes=[IsSuperUser | IsAccountant], ) - def confirm(self, request, pk=None): + def confirm(self, request, *args, **kwargs): """Confirm order. Only allowed by staff members @@ -111,7 +111,7 @@ 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, *args, **kwargs): instance = self.get_object() if instance.acknowledged: # acknowledge orders may not be deleted 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 Date: Wed, 17 Apr 2024 10:18:03 +0200 Subject: [PATCH 07/27] chore: alter old migrations to use tuple and drop new migration --- .../migrations/0003_auto_20170831_1624.py | 8 ++-- .../migrations/0005_auto_20170907_0938.py | 2 +- .../migrations/0006_auto_20171010_1423.py | 2 +- ...tions_alter_costcenter_options_and_more.py | 37 ------------------- 4 files changed, 6 insertions(+), 43 deletions(-) delete mode 100644 timed/projects/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py 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/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py b/timed/projects/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py deleted file mode 100644 index 5f5e0a33..00000000 --- a/timed/projects/migrations/0017_alter_billingtype_options_alter_costcenter_options_and_more.py +++ /dev/null @@ -1,37 +0,0 @@ -# Generated by Django 4.2.11 on 2024-04-16 13:06 - -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('projects', '0016_alter_project_amount_invoiced_currency_and_more'), - ] - - operations = [ - migrations.AlterModelOptions( - name='billingtype', - options={'ordering': ('name',)}, - ), - migrations.AlterModelOptions( - name='costcenter', - 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',)}, - ), - ] From bb42765c83ca8b34e780d6ef167f476d7b571019 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:18:24 +0200 Subject: [PATCH 08/27] chore(linting): ignore more rules --- ruff.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ruff.toml b/ruff.toml index 836b9dee..06588069 100644 --- a/ruff.toml +++ b/ruff.toml @@ -68,6 +68,9 @@ ignore = [ "COM812", # ignore due to conflict with formatter "ISC001", # ignore due to conflict with formatter "E501", # managed by formatter + "SLF001", # allow accessing private members + "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 @@ -76,6 +79,7 @@ ignore = [ "D105", # don't enforce existance of docstrings "D106", # don't enforce existance of docstrings "D107", # don't enforce existance of docstrings + "RUF012", # don't require ClassVar annotation ] [lint.per-file-ignores] From a1cd7fc97a292b06bd312051c787e6c309ebeaa1 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:18:46 +0200 Subject: [PATCH 09/27] fix(settings): add missing .exists() --- timed/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/timed/settings.py b/timed/settings.py index 674c22ee..d3671d3a 100644 --- a/timed/settings.py +++ b/timed/settings.py @@ -12,7 +12,7 @@ django_root = environ.Path(__file__) - 2 ENV_FILE = env.str("DJANGO_ENV_FILE", default=django_root(".env")) -if Path(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 From 9f58039ef7479ac47bea8983a0644e139c8df75c Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:18:59 +0200 Subject: [PATCH 10/27] chore: linted --- timed/employment/views.py | 4 ++-- timed/mixins.py | 2 +- .../management/commands/notify_reviewers_unverified.py | 2 +- timed/reports/filters.py | 6 +++--- timed/reports/tests/test_work_report.py | 2 +- timed/reports/views.py | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/timed/employment/views.py b/timed/employment/views.py index 34594d95..53f977cb 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -98,14 +98,14 @@ def get_queryset(self): return queryset @action(methods=["get"], detail=False) - def me(self, request, pk=None): + def me(self, request, *args, **kwargs): 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): + def transfer(self, *args, **kwargs): """Transfer worktime and absence balance to new year. It will skip any credits if a credit already exists on the first diff --git a/timed/mixins.py b/timed/mixins.py index 3ee36a4a..a36d30c0 100644 --- a/timed/mixins.py +++ b/timed/mixins.py @@ -50,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(): # noqa: SLF001 + for key, value in serializer_class._declared_fields.items(): if self._is_related_field(value): source = value.source or key if many: diff --git a/timed/notifications/management/commands/notify_reviewers_unverified.py b/timed/notifications/management/commands/notify_reviewers_unverified.py index 7197d6a9..59ba0e2d 100644 --- a/timed/notifications/management/commands/notify_reviewers_unverified.py +++ b/timed/notifications/management/commands/notify_reviewers_unverified.py @@ -89,7 +89,7 @@ def _get_unverified_reports(self, start, end): """ 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. diff --git a/timed/reports/filters.py b/timed/reports/filters.py index ac1e78f3..5afa6f7d 100644 --- a/timed/reports/filters.py +++ b/timed/reports/filters.py @@ -49,7 +49,7 @@ def filter_cost_center(self, queryset, _, value): * When viewing the statistic over tasks, only the tasks are filtered """ - # TODO: Discuss Is this the desired behaviour by our users? # noqa: TD002,TD003 + # TODO: Discuss Is this the desired behaviour by our users? if not value: # pragma: no cover return queryset @@ -78,9 +78,9 @@ def filter_queryset(self, queryset): duration_ref = self._refs["reports_ref"] + "__duration" - full_qs = qs._base.annotate( # noqa: SLF001 + full_qs = qs._base.annotate( duration=Coalesce( - Sum(duration_ref, filter=qs._agg_filters), # noqa: SLF001 + Sum(duration_ref, filter=qs._agg_filters), Value("00:00:00", DurationField(null=False)), ), pk=F("id"), diff --git a/timed/reports/tests/test_work_report.py b/timed/reports/tests/test_work_report.py index f1d683eb..b5773086 100644 --- a/timed/reports/tests/test_work_report.py +++ b/timed/reports/tests/test_work_report.py @@ -153,7 +153,7 @@ def test_generate_work_report_name(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) # noqa: SLF001 + name = view._generate_workreport_name(test_date, test_date, project) assert name == expected diff --git a/timed/reports/views.py b/timed/reports/views.py index 582d25df..877876fc 100644 --- a/timed/reports/views.py +++ b/timed/reports/views.py @@ -113,7 +113,7 @@ def filter_base(self, *args, **kwargs): def _clone(self): return StatisticQueryset( model=self.model, - base_qs=self._base._clone(), # noqa: SLF001 + base_qs=self._base._clone(), catch_prefixes=self._catch_prefixes, agg_filters=self._agg_filters, ) From 5c962b30462f61e056b85b1a4743169d89c16bd0 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:23:41 +0200 Subject: [PATCH 11/27] chore: remove unnecessary ClassVar annotations --- timed/employment/serializers.py | 15 ++++++--------- timed/projects/serializers.py | 11 +++++------ timed/reports/serializers.py | 14 +++----------- timed/subscription/serializers.py | 7 +++---- timed/tracking/serializers.py | 13 +++++-------- 5 files changed, 22 insertions(+), 38 deletions(-) diff --git a/timed/employment/serializers.py b/timed/employment/serializers.py index 20f682e5..b44c8428 100644 --- a/timed/employment/serializers.py +++ b/timed/employment/serializers.py @@ -1,7 +1,6 @@ """Serializers for the employment app.""" from datetime import date, timedelta -from typing import ClassVar from django.contrib.auth import get_user_model from django.db.models import Max, Value @@ -21,7 +20,7 @@ class UserSerializer(ModelSerializer): - included_serializers: ClassVar = { + included_serializers = { "supervisors": "timed.employment.serializers.UserSerializer", "supervisees": "timed.employment.serializers.UserSerializer", } @@ -95,9 +94,7 @@ def get_balance(self, instance): _, _, balance = instance.id.calculate_worktime(start, balance_date) return duration_string(balance) - included_serializers: ClassVar = { - "user": "timed.employment.serializers.UserSerializer" - } + included_serializers = {"user": "timed.employment.serializers.UserSerializer"} class Meta: resource_name = "worktime-balances" @@ -217,7 +214,7 @@ def get_balance(self, instance): return self.get_credit(instance) - self.get_used_days(instance) - included_serializers: ClassVar = { + included_serializers = { "absence_type": "timed.employment.serializers.AbsenceTypeSerializer", "absence_credits": "timed.employment.serializers.AbsenceCreditSerializer", } @@ -227,7 +224,7 @@ class Meta: class EmploymentSerializer(ModelSerializer): - included_serializers: ClassVar = { + included_serializers = { "user": "timed.employment.serializers.UserSerializer", "location": "timed.employment.serializers.LocationSerializer", } @@ -293,7 +290,7 @@ class PublicHolidaySerializer(ModelSerializer): location = relations.ResourceRelatedField(read_only=True) - included_serializers: ClassVar = { + included_serializers = { "location": "timed.employment.serializers.LocationSerializer" } @@ -317,7 +314,7 @@ class Meta: class AbsenceCreditSerializer(ModelSerializer): """Absence credit serializer.""" - included_serializers: ClassVar = { + included_serializers = { "absence_type": "timed.employment.serializers.AbsenceTypeSerializer" } diff --git a/timed/projects/serializers.py b/timed/projects/serializers.py index 786c8789..1abb970f 100644 --- a/timed/projects/serializers.py +++ b/timed/projects/serializers.py @@ -1,7 +1,6 @@ """Serializers for the projects app.""" from datetime import timedelta -from typing import ClassVar from django.db.models import Q, Sum from django.utils.duration import duration_string @@ -47,7 +46,7 @@ class ProjectSerializer(ModelSerializer): customer = ResourceRelatedField(queryset=models.Customer.objects.all()) billing_type = ResourceRelatedField(queryset=models.BillingType.objects.all()) - included_serializers: ClassVar = { + included_serializers = { "customer": "timed.projects.serializers.CustomerSerializer", "billing_type": "timed.projects.serializers.BillingTypeSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", @@ -117,7 +116,7 @@ class TaskSerializer(ModelSerializer): project = ResourceRelatedField(queryset=models.Project.objects.all()) - included_serializers: ClassVar = { + included_serializers = { "activities": "timed.tracking.serializers.ActivitySerializer", "project": "timed.projects.serializers.ProjectSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", @@ -203,7 +202,7 @@ class Meta: class CustomerAssigneeSerializer(ModelSerializer): """Customer assignee serializer.""" - included_serializers: ClassVar = { + included_serializers = { "user": "timed.employment.serializers.UserSerializer", "customer": "timed.projects.serializers.CustomerSerializer", } @@ -225,7 +224,7 @@ class Meta: class ProjectAssigneeSerializer(ModelSerializer): """Project assignee serializer.""" - included_serializers: ClassVar = { + included_serializers = { "user": "timed.employment.serializers.UserSerializer", "project": "timed.projects.serializers.ProjectSerializer", } @@ -247,7 +246,7 @@ class Meta: class TaskAssigneeSerializer(ModelSerializer): """Task assignees serializer.""" - included_serializers: ClassVar = { + included_serializers = { "user": "timed.employment.serializers.UserSerializer", "task": "timed.projects.serializers.TaskSerializer", } diff --git a/timed/reports/serializers.py b/timed/reports/serializers.py index d85e34f3..abbf4901 100644 --- a/timed/reports/serializers.py +++ b/timed/reports/serializers.py @@ -1,5 +1,3 @@ -from typing import ClassVar - from django.contrib.auth import get_user_model from rest_framework_json_api import relations from rest_framework_json_api.serializers import ( @@ -50,9 +48,7 @@ class ProjectStatisticSerializer(TotalTimeRootMetaMixin, Serializer): estimated_time = DurationField(read_only=True) total_remaining_effort = DurationField(read_only=True) - included_serializers: ClassVar = { - "customer": "timed.projects.serializers.CustomerSerializer" - } + included_serializers = {"customer": "timed.projects.serializers.CustomerSerializer"} class Meta: resource_name = "project-statistics" @@ -65,9 +61,7 @@ class TaskStatisticSerializer(TotalTimeRootMetaMixin, Serializer): project = relations.ResourceRelatedField(model=Project, read_only=True) estimated_time = DurationField(read_only=True) - included_serializers: ClassVar = { - "project": "timed.projects.serializers.ProjectSerializer" - } + included_serializers = {"project": "timed.projects.serializers.ProjectSerializer"} class Meta: resource_name = "task-statistics" @@ -77,9 +71,7 @@ class UserStatisticSerializer(TotalTimeRootMetaMixin, Serializer): duration = DurationField(read_only=True) user = relations.ResourceRelatedField(model=get_user_model(), read_only=True) - included_serializers: ClassVar = { - "user": "timed.employment.serializers.UserSerializer" - } + included_serializers = {"user": "timed.employment.serializers.UserSerializer"} class Meta: resource_name = "user-statistics" diff --git a/timed/subscription/serializers.py b/timed/subscription/serializers.py index 82894184..05a032e9 100644 --- a/timed/subscription/serializers.py +++ b/timed/subscription/serializers.py @@ -1,5 +1,4 @@ from datetime import timedelta -from typing import ClassVar from django.db.models import Sum from django.utils.duration import duration_string @@ -39,7 +38,7 @@ def get_spent_time(self, obj): data = reports.aggregate(spent_time=Sum("duration")) return duration_string(data["spent_time"] or timedelta()) - included_serializers: ClassVar = { + included_serializers = { "billing_type": "timed.projects.serializers.BillingTypeSerializer", "cost_center": "timed.projects.serializers.CostCenterSerializer", "customer": "timed.projects.serializers.CustomerSerializer", @@ -64,7 +63,7 @@ class PackageSerializer(ModelSerializer): price = CharField() """CharField needed as it includes currency.""" - included_serializers: ClassVar = { + included_serializers = { "billing_type": "timed.projects.serializers.BillingTypeSerializer" } @@ -75,7 +74,7 @@ class Meta: class OrderSerializer(ModelSerializer): - included_serializers: ClassVar = { + included_serializers = { "project": ("timed.subscription.serializers" ".SubscriptionProjectSerializer") } diff --git a/timed/tracking/serializers.py b/timed/tracking/serializers.py index 5b6aae90..17a408a3 100644 --- a/timed/tracking/serializers.py +++ b/timed/tracking/serializers.py @@ -1,7 +1,6 @@ """Serializers for the tracking app.""" from datetime import date, timedelta -from typing import ClassVar from django.contrib.auth import get_user_model from django.db.models import BooleanField, Case, Q, When @@ -28,7 +27,7 @@ class ActivitySerializer(ModelSerializer): user = CurrentUserResourceRelatedField() - included_serializers: ClassVar = { + included_serializers = { "task": "timed.projects.serializers.TaskSerializer", "user": "timed.employment.serializers.UserSerializer", } @@ -75,9 +74,7 @@ class AttendanceSerializer(ModelSerializer): user = CurrentUserResourceRelatedField() - included_serializers: ClassVar = { - "user": "timed.employment.serializers.UserSerializer" - } + included_serializers = {"user": "timed.employment.serializers.UserSerializer"} class Meta: """Meta information for the attendance serializer.""" @@ -98,7 +95,7 @@ class ReportSerializer(TotalTimeRootMetaMixin, ModelSerializer): queryset=get_user_model().objects, required=False, allow_null=True ) - included_serializers: ClassVar = { + included_serializers = { "task": "timed.projects.serializers.TaskSerializer", "user": "timed.employment.serializers.UserSerializer", "verified_by": "timed.employment.serializers.UserSerializer", @@ -372,7 +369,7 @@ def get_root_meta(self, *args): queryset = self.instance["queryset"] return {"count": queryset.count()} - included_serializers: ClassVar = { + included_serializers = { "customer": "timed.projects.serializers.CustomerSerializer", "project": "timed.projects.serializers.ProjectSerializer", "task": "timed.projects.serializers.TaskSerializer", @@ -390,7 +387,7 @@ class AbsenceSerializer(ModelSerializer): absence_type = ResourceRelatedField(queryset=AbsenceType.objects.all()) user = CurrentUserResourceRelatedField() - included_serializers: ClassVar = { + included_serializers = { "user": "timed.employment.serializers.UserSerializer", "absence_type": "timed.employment.serializers.AbsenceTypeSerializer", } From 63d5010830846aa845a4269227ee2b243099a9c5 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:25:21 +0200 Subject: [PATCH 12/27] chore(linting): fix indentation in ruff.toml --- ruff.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruff.toml b/ruff.toml index 06588069..7536a10f 100644 --- a/ruff.toml +++ b/ruff.toml @@ -79,7 +79,7 @@ ignore = [ "D105", # don't enforce existance of docstrings "D106", # don't enforce existance of docstrings "D107", # don't enforce existance of docstrings - "RUF012", # don't require ClassVar annotation + "RUF012", # don't require ClassVar annotation ] [lint.per-file-ignores] From b662cf9612d381e0e4f28d13c9311972d60e298e Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:27:13 +0200 Subject: [PATCH 13/27] chore(linting): don't require parentheses after pytest.fixture --- ruff.toml | 5 ++++- timed/conftest.py | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/ruff.toml b/ruff.toml index 7536a10f..6e331fdc 100644 --- a/ruff.toml +++ b/ruff.toml @@ -109,4 +109,7 @@ combine-as-imports = true ignore-fully-untyped = true [lint.flake8-unused-arguments] -ignore-variadic-names = true \ No newline at end of file +ignore-variadic-names = true + +[lint.flake8-pytest-style] +fixture-parentheses = false \ No newline at end of file diff --git a/timed/conftest.py b/timed/conftest.py index 0953b70d..ccbf2eb5 100644 --- a/timed/conftest.py +++ b/timed/conftest.py @@ -25,7 +25,7 @@ def register_module(module): register_module(tracking_factories) -@pytest.fixture() +@pytest.fixture def auth_user(db): return get_user_model().objects.create_user( username="user", @@ -37,7 +37,7 @@ def auth_user(db): ) -@pytest.fixture() +@pytest.fixture def admin_user(db): return get_user_model().objects.create_user( username="admin", @@ -49,7 +49,7 @@ def admin_user(db): ) -@pytest.fixture() +@pytest.fixture def superadmin_user(db): return get_user_model().objects.create_user( username="superadmin", @@ -61,7 +61,7 @@ def superadmin_user(db): ) -@pytest.fixture() +@pytest.fixture def external_employee(db): user = get_user_model().objects.create_user( username="user", @@ -75,7 +75,7 @@ def external_employee(db): return user -@pytest.fixture() +@pytest.fixture def internal_employee(db): user = get_user_model().objects.create_user( username="user", @@ -90,12 +90,12 @@ def internal_employee(db): return user -@pytest.fixture() +@pytest.fixture def client(): return APIClient() -@pytest.fixture() +@pytest.fixture def auth_client(auth_user): """Return instance of a APIClient that is logged in as test user.""" client = APIClient() @@ -104,7 +104,7 @@ def auth_client(auth_user): return client -@pytest.fixture() +@pytest.fixture def admin_client(admin_user): """Return instance of a APIClient that is logged in as a staff user.""" client = APIClient() @@ -113,7 +113,7 @@ def admin_client(admin_user): return client -@pytest.fixture() +@pytest.fixture def superadmin_client(superadmin_user): """Return instance of a APIClient that is logged in as superuser.""" client = APIClient() @@ -122,7 +122,7 @@ def superadmin_client(superadmin_user): return client -@pytest.fixture() +@pytest.fixture def external_employee_client(external_employee): """Return instance of a APIClient that is logged in as external test user.""" client = APIClient() @@ -131,7 +131,7 @@ def external_employee_client(external_employee): return client -@pytest.fixture() +@pytest.fixture def internal_employee_client(internal_employee): """Return instance of a APIClient that is logged in as external test user.""" client = APIClient() From 98c99d43614a1b7af982a932e79e69a9c84234b3 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:32:34 +0200 Subject: [PATCH 14/27] docs(linting): mark ignore-fully-untyped as temporary --- ruff.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/ruff.toml b/ruff.toml index 6e331fdc..9ec6bc3f 100644 --- a/ruff.toml +++ b/ruff.toml @@ -106,6 +106,7 @@ known-third-party = ["pytest_factoryboy"] combine-as-imports = true [lint.flake8-annotations] +# TODO: drop this, its temporary ignore-fully-untyped = true [lint.flake8-unused-arguments] From 29fac56b1308897154e206e76d28276c900c5654 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:52:11 +0200 Subject: [PATCH 15/27] refactor: update code to make ruff happier tests: move unused fixtures to `usefixtures` conftest: ignore ARG001 for db fixture tracking: use named arguments for boolean (rejected) --- timed/conftest.py | 10 +++++----- timed/employment/tests/test_location.py | 4 +++- timed/projects/models.py | 2 +- timed/settings.py | 2 +- timed/tracking/tasks.py | 6 +++--- timed/tracking/tests/test_report.py | 4 +++- timed/tracking/views.py | 2 +- 7 files changed, 17 insertions(+), 13 deletions(-) diff --git a/timed/conftest.py b/timed/conftest.py index ccbf2eb5..10d24962 100644 --- a/timed/conftest.py +++ b/timed/conftest.py @@ -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", diff --git a/timed/employment/tests/test_location.py b/timed/employment/tests/test_location.py index 3e3347b9..ec0e8cca 100644 --- a/timed/employment/tests/test_location.py +++ b/timed/employment/tests/test_location.py @@ -6,6 +6,8 @@ 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"), [ @@ -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, diff --git a/timed/projects/models.py b/timed/projects/models.py index 73e2e07e..6321e49b 100644 --- a/timed/projects/models.py +++ b/timed/projects/models.py @@ -258,7 +258,7 @@ class TaskAssignee(models.Model): @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. diff --git a/timed/settings.py b/timed/settings.py index d3671d3a..5444ad20 100644 --- a/timed/settings.py +++ b/timed/settings.py @@ -389,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/tracking/tasks.py b/timed/tracking/tasks.py index 9f02681b..4cfbc6fa 100644 --- a/timed/tracking/tasks.py +++ b/timed/tracking/tasks.py @@ -92,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, _, reviewer): users = [report.user for report in queryset.order_by("user").distinct("user")] user_changes = [] @@ -106,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/tests/test_report.py b/timed/tracking/tests/test_report.py index bf6f653c..d30f69c7 100644 --- a/timed/tracking/tests/test_report.py +++ b/timed/tracking/tests/test_report.py @@ -1439,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)] ) def test_report_update_bulk_review_and_verified( - superadmin_client, project, task, report, needs_review + superadmin_client, report, needs_review ): EmploymentFactory.create(user=superadmin_client.user) data = { diff --git a/timed/tracking/views.py b/timed/tracking/views.py index bcd1ac93..6984f8b5 100644 --- a/timed/tracking/views.py +++ b/timed/tracking/views.py @@ -188,7 +188,7 @@ def update(self, request, *args, **kwargs): methods=["get"], serializer_class=serializers.ReportIntersectionSerializer, ) - def intersection(self, request): + def intersection(self, *args, **kwargs): """Get intersection in reports of common report fields. Use case is for api caller to know what fields are the same From 11366b97899e6aa2a07def23828f75f832a4cf67 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 10:58:03 +0200 Subject: [PATCH 16/27] chore: raise PermissionDenied errors from None --- timed/employment/views.py | 4 ++-- timed/tracking/views.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/timed/employment/views.py b/timed/employment/views.py index 53f977cb..ab7fd709 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -80,7 +80,7 @@ def get_queryset(self): ) return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) - except models.Employment.DoesNotExist as exc: + except models.Employment.DoesNotExist: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): assigned_tasks = Task.objects.filter( Q( @@ -93,7 +93,7 @@ def get_queryset(self): ) return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) msg = "User has no employment" - raise exceptions.PermissionDenied(msg) from exc + raise exceptions.PermissionDenied(msg) from None else: return queryset diff --git a/timed/tracking/views.py b/timed/tracking/views.py index 6984f8b5..fe823221 100644 --- a/timed/tracking/views.py +++ b/timed/tracking/views.py @@ -150,7 +150,7 @@ def get_queryset(self): ) ) return queryset.filter(Q(task__in=assigned_tasks) | Q(user=user)) - except Employment.DoesNotExist as exc: + except Employment.DoesNotExist: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): return queryset.filter( Q( @@ -159,7 +159,7 @@ def get_queryset(self): ) ) msg = "User has no employment and isn't a customer!" - raise exceptions.PermissionDenied(msg) from exc + raise exceptions.PermissionDenied(msg) from None def update(self, request, *args, **kwargs): """Override so we can issue emails on update.""" From 28b8f4555353716ede69af7edf2de187241afa97 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Wed, 17 Apr 2024 11:11:23 +0200 Subject: [PATCH 17/27] docs(models): add __str__ --- timed/employment/models.py | 6 ++++++ timed/projects/models.py | 9 +++++++++ timed/redmine/models.py | 3 +++ timed/subscription/models.py | 6 ++++++ timed/tracking/models.py | 12 ++++++++++++ 5 files changed, 36 insertions(+) diff --git a/timed/employment/models.py b/timed/employment/models.py index 2e1b3228..a8fb5dda 100644 --- a/timed/employment/models.py +++ b/timed/employment/models.py @@ -138,6 +138,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. @@ -159,6 +162,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.""" diff --git a/timed/projects/models.py b/timed/projects/models.py index 6321e49b..443909a4 100644 --- a/timed/projects/models.py +++ b/timed/projects/models.py @@ -216,6 +216,9 @@ class CustomerAssignee(models.Model): is_manager = models.BooleanField(default=False) is_customer = models.BooleanField(default=False) + def __str__(self) -> str: + return f"{self.user.username} {self.customer}" + class ProjectAssignee(models.Model): """Project assignee model. @@ -236,6 +239,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,6 +262,9 @@ 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): # noqa: ARG001 diff --git a/timed/redmine/models.py b/timed/redmine/models.py index dc4251fa..c4a114b6 100644 --- a/timed/redmine/models.py +++ b/timed/redmine/models.py @@ -13,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/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/tracking/models.py b/timed/tracking/models.py index abf98ea4..effe684b 100644 --- a/timed/tracking/models.py +++ b/timed/tracking/models.py @@ -150,6 +150,18 @@ class Meta: unique_together = ("date", "user") + def __str__(self): + """Represent the model as a string. + + :return: The string representation + :rtype: str + """ + return "{}: {} {}".format( + self.user, + self.date.strftime("%Y-%m-%d"), + self.comment, + ) + def calculate_duration(self, employment): """Calculate duration of absence with given employment. From da11076839c4be1731023c37d91568771b355f58 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Thu, 18 Apr 2024 08:51:52 +0200 Subject: [PATCH 18/27] chore(linting): add DJ001 to ignores --- ruff.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/ruff.toml b/ruff.toml index 9ec6bc3f..0dce444f 100644 --- a/ruff.toml +++ b/ruff.toml @@ -80,6 +80,7 @@ ignore = [ "D106", # don't enforce existance of docstrings "D107", # don't enforce existance of docstrings "RUF012", # don't require ClassVar annotation + "DJ001", # null=True on string-based fields such as CharField (#1052) ] [lint.per-file-ignores] From 3f1c88c1c90bcfca2a2e355e830e38580f955c90 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Thu, 18 Apr 2024 09:42:03 +0200 Subject: [PATCH 19/27] chore(linting): reenable SLF001 and noqa current occurrences reference: https://github.com/adfinis/timed-backend/pull/1049#discussion_r1570101528 --- ruff.toml | 1 - timed/conftest.py | 2 +- timed/mixins.py | 2 +- timed/reports/filters.py | 4 ++-- timed/reports/tests/test_work_report.py | 2 +- timed/reports/views.py | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/ruff.toml b/ruff.toml index 0dce444f..25125cd7 100644 --- a/ruff.toml +++ b/ruff.toml @@ -68,7 +68,6 @@ ignore = [ "COM812", # ignore due to conflict with formatter "ISC001", # ignore due to conflict with formatter "E501", # managed by formatter - "SLF001", # allow accessing private members "TD002", # don't require author of TODO "TD003", # don't require link to TODO "D100", # don't enforce existance of docstrings diff --git a/timed/conftest.py b/timed/conftest.py index 10d24962..526996a0 100644 --- a/timed/conftest.py +++ b/timed/conftest.py @@ -15,7 +15,7 @@ def register_module(module): for _name, obj in inspect.getmembers(module): - if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract: + if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract: # noqa: SLF001 register(obj) diff --git a/timed/mixins.py b/timed/mixins.py index a36d30c0..3ee36a4a 100644 --- a/timed/mixins.py +++ b/timed/mixins.py @@ -50,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/reports/filters.py b/timed/reports/filters.py index 5afa6f7d..52726275 100644 --- a/timed/reports/filters.py +++ b/timed/reports/filters.py @@ -78,9 +78,9 @@ def filter_queryset(self, 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"), diff --git a/timed/reports/tests/test_work_report.py b/timed/reports/tests/test_work_report.py index b5773086..f1d683eb 100644 --- a/timed/reports/tests/test_work_report.py +++ b/timed/reports/tests/test_work_report.py @@ -153,7 +153,7 @@ def test_generate_work_report_name(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 diff --git a/timed/reports/views.py b/timed/reports/views.py index 877876fc..582d25df 100644 --- a/timed/reports/views.py +++ b/timed/reports/views.py @@ -113,7 +113,7 @@ 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, ) From cf632115bb44b68aa4233ab40fa3398e7e0f7d5e Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Thu, 18 Apr 2024 09:58:15 +0200 Subject: [PATCH 20/27] refactor(statistics-qs): adjust function signature to not allow *args reference: https://github.com/adfinis/timed-backend/pull/1049#discussion_r1569634995 --- timed/reports/views.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/timed/reports/views.py b/timed/reports/views.py index 582d25df..cecf6451 100644 --- a/timed/reports/views.py +++ b/timed/reports/views.py @@ -78,14 +78,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 - msg = ( - "Unable to detect statistics filter type form Q objects. use " - "filter_aggregate() or filter_base() instead" - ) - raise RuntimeError(msg) + def filter(self, /, **kwargs): my_filters = { k: v for k, v in kwargs.items() if not k.startswith(self._catch_prefixes) } From 55deb9e7aafd754a34e640561a6178653d85de6b Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Thu, 18 Apr 2024 10:56:30 +0200 Subject: [PATCH 21/27] chore(linting): re-enable RUF012, format tuples to be one item per line serializers: added docstrings to files serializers: ClassVar[dict[str, str]] annotation for included_serializers views: make ordering tuple if its not already a tuple --- pyproject.toml | 1 + ruff.toml | 1 - timed/employment/admin.py | 14 ++++++-- timed/employment/filters.py | 8 ++++- timed/employment/serializers.py | 53 ++++++++++++++++++++++++------- timed/employment/views.py | 6 +++- timed/projects/admin.py | 5 ++- timed/projects/filters.py | 40 ++++++++++++++++++++--- timed/projects/serializers.py | 26 +++++++++++---- timed/projects/views.py | 15 +++++---- timed/reports/serializers.py | 21 ++++++++++-- timed/reports/views.py | 21 +++++++++--- timed/subscription/admin.py | 6 +++- timed/subscription/filters.py | 11 +++++-- timed/subscription/serializers.py | 29 +++++++++++++---- timed/subscription/views.py | 5 ++- timed/tracking/filters.py | 7 +++- timed/tracking/models.py | 5 ++- timed/tracking/serializers.py | 33 +++++++++++++++---- timed/tracking/views.py | 5 ++- 20 files changed, 250 insertions(+), 62 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c7723d62..5c7a6a85 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,6 +94,7 @@ exclude_lines = [ "def __str__", "def __unicode__", "def __repr__", + "if TYPE_CHECKING", ] omit = [ "*/migrations/*", diff --git a/ruff.toml b/ruff.toml index 25125cd7..82ec1dc4 100644 --- a/ruff.toml +++ b/ruff.toml @@ -78,7 +78,6 @@ ignore = [ "D105", # don't enforce existance of docstrings "D106", # don't enforce existance of docstrings "D107", # don't enforce existance of docstrings - "RUF012", # don't require ClassVar annotation "DJ001", # null=True on string-based fields such as CharField (#1052) ] diff --git a/timed/employment/admin.py b/timed/employment/admin.py index 5c14ebd7..802363b3 100644 --- a/timed/employment/admin.py +++ b/timed/employment/admin.py @@ -151,7 +151,13 @@ class UserAdmin(UserAdmin): OvertimeCreditInline, AbsenceCreditInline, ) - list_display = ("username", "first_name", "last_name", "is_staff", "is_active") + list_display = ( + "username", + "first_name", + "last_name", + "is_staff", + "is_active", + ) search_fields = ("username",) actions = ( @@ -206,7 +212,11 @@ def has_delete_permission(self, _, obj=None): class PublicHolidayAdmin(admin.ModelAdmin): """Public holiday admin view.""" - list_display = ("__str__", "date", "location") + list_display = ( + "__str__", + "date", + "location", + ) list_filter = ("location",) diff --git a/timed/employment/filters.py b/timed/employment/filters.py index 91e57f39..25986d1f 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): diff --git a/timed/employment/serializers.py b/timed/employment/serializers.py index b44c8428..e5c3d78f 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", } @@ -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" @@ -214,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", } @@ -224,7 +232,7 @@ class Meta: class EmploymentSerializer(ModelSerializer): - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "location": "timed.employment.serializers.LocationSerializer", } @@ -282,7 +290,10 @@ class Meta: """Meta information for the location serializer.""" model = models.Location - fields = ("name", "workdays") + fields = ( + "name", + "workdays", + ) class PublicHolidaySerializer(ModelSerializer): @@ -290,7 +301,7 @@ class PublicHolidaySerializer(ModelSerializer): location = relations.ResourceRelatedField(read_only=True) - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "location": "timed.employment.serializers.LocationSerializer" } @@ -298,7 +309,11 @@ class Meta: """Meta information for the public holiday serializer.""" model = models.PublicHoliday - fields = ("name", "date", "location") + fields = ( + "name", + "date", + "location", + ) class AbsenceTypeSerializer(ModelSerializer): @@ -308,13 +323,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" } @@ -322,10 +340,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/views.py b/timed/employment/views.py index ab7fd709..9f831585 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -51,7 +51,11 @@ class UserViewSet(ModelViewSet): 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 diff --git a/timed/projects/admin.py b/timed/projects/admin.py index 84b51752..d4677069 100644 --- a/timed/projects/admin.py +++ b/timed/projects/admin.py @@ -118,7 +118,10 @@ class ProjectAdmin(NestedModelAdmin): "customer", ) list_filter = ("customer",) - search_fields = ("name", "customer__name") + search_fields = ( + "name", + "customer__name", + ) inlines = (TaskInline, RedmineProjectInline, ProjectAssigneeInline) diff --git a/timed/projects/filters.py b/timed/projects/filters.py index d11e34d5..99990b47 100644 --- a/timed/projects/filters.py +++ b/timed/projects/filters.py @@ -72,7 +72,13 @@ 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): @@ -126,7 +132,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): @@ -140,7 +152,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): @@ -154,7 +172,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): @@ -168,4 +192,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/serializers.py b/timed/projects/serializers.py index 1abb970f..96d347ea 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.""" @@ -31,13 +37,19 @@ class Meta: 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,7 +58,7 @@ 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", @@ -116,7 +128,7 @@ 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", @@ -202,7 +214,7 @@ class Meta: class CustomerAssigneeSerializer(ModelSerializer): """Customer assignee serializer.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "customer": "timed.projects.serializers.CustomerSerializer", } @@ -224,7 +236,7 @@ class Meta: class ProjectAssigneeSerializer(ModelSerializer): """Project assignee serializer.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "project": "timed.projects.serializers.ProjectSerializer", } @@ -246,7 +258,7 @@ class Meta: class TaskAssigneeSerializer(ModelSerializer): """Task assignees serializer.""" - included_serializers = { + included_serializers: ClassVar[dict[str, str]] = { "user": "timed.employment.serializers.UserSerializer", "task": "timed.projects.serializers.TaskSerializer", } diff --git a/timed/projects/views.py b/timed/projects/views.py index 10cb21bc..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,7 +53,7 @@ def get_queryset(self): class BillingTypeViewSet(ReadOnlyModelViewSet): serializer_class = serializers.BillingTypeSerializer - ordering = "name" + ordering = ("name",) permission_classes = ( ( # superuser may edit all billing types @@ -98,7 +98,7 @@ def get_queryset(self): class CostCenterViewSet(ReadOnlyModelViewSet): serializer_class = serializers.CostCenterSerializer - ordering = "name" + ordering = ("name",) permission_classes = ( ( # superuser may edit all cost centers @@ -117,8 +117,11 @@ 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 = ( ( @@ -165,7 +168,7 @@ class TaskViewSet(ModelViewSet): serializer_class = serializers.TaskSerializer filterset_class = filters.TaskFilterSet queryset = models.Task.objects.select_related("project", "cost_center") - ordering = "name" + ordering = ("name",) permission_classes = ( ( # superuser may edit all tasks 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/views.py b/timed/reports/views.py index cecf6451..48dd76b1 100644 --- a/timed/reports/views.py +++ b/timed/reports/views.py @@ -29,7 +29,10 @@ class YearStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.YearStatisticSerializer filterset_class = ReportFilterSet - ordering_fields = ("year", "duration") + ordering_fields = ( + "year", + "duration", + ) ordering = ("year",) permission_classes = ( ( @@ -50,8 +53,15 @@ class MonthStatisticViewSet(AggregateQuerysetMixin, ReadOnlyModelViewSet): serializer_class = serializers.MonthStatisticSerializer filterset_class = ReportFilterSet - ordering_fields = ("year", "month", "duration") - ordering = ("year", "month") + ordering_fields = ( + "year", + "month", + "duration", + ) + ordering = ( + "year", + "month", + ) permission_classes = ( ( # internal employees or super users may read all customer statistics @@ -205,7 +215,10 @@ 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 = ( ( diff --git a/timed/subscription/admin.py b/timed/subscription/admin.py index 55bd09a5..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 c5734ace..50930757 100644 --- a/timed/subscription/filters.py +++ b/timed/subscription/filters.py @@ -14,7 +14,10 @@ def filter_customer(self, queryset, _, value): 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/serializers.py b/timed/subscription/serializers.py index 05a032e9..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,6 +18,9 @@ from .models import Order, Package +if TYPE_CHECKING: + from typing import ClassVar + class SubscriptionProjectSerializer(ModelSerializer): purchased_time = SerializerMethodField(source="get_purchased_time") @@ -38,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", @@ -63,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/views.py b/timed/subscription/views.py index 4e9e305e..e826df53 100644 --- a/timed/subscription/views.py +++ b/timed/subscription/views.py @@ -26,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 diff --git a/timed/tracking/filters.py b/timed/tracking/filters.py index ad179423..523439f0 100644 --- a/timed/tracking/filters.py +++ b/timed/tracking/filters.py @@ -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 effe684b..e8c9b226 100644 --- a/timed/tracking/models.py +++ b/timed/tracking/models.py @@ -148,7 +148,10 @@ class Absence(models.Model): class Meta: """Meta informations for the absence model.""" - unique_together = ("date", "user") + unique_together = ( + "date", + "user", + ) def __str__(self): """Represent the model as a string. diff --git a/timed/tracking/serializers.py b/timed/tracking/serializers.py index 17a408a3..e46b1417 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", @@ -369,7 +382,7 @@ def get_root_meta(self, *args): 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", @@ -387,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", } @@ -453,4 +466,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/views.py b/timed/tracking/views.py index fe823221..77949e08 100644 --- a/timed/tracking/views.py +++ b/timed/tracking/views.py @@ -109,7 +109,10 @@ class ReportViewSet(ModelViewSet): | IsAuthenticated & IsReadOnly ), ) - ordering = ("date", "id") + ordering = ( + "date", + "id", + ) ordering_fields = ( "id", "date", From b07d07ba771a0f4b3021f7463e6be25cb3180e9a Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Fri, 19 Apr 2024 10:07:00 +0200 Subject: [PATCH 22/27] docs(linting): add issue link to ignore-fully-untyped=true --- ruff.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/ruff.toml b/ruff.toml index 82ec1dc4..87feb6b5 100644 --- a/ruff.toml +++ b/ruff.toml @@ -106,6 +106,7 @@ 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] From cb72c91afa9807a292709b9d699c348f85221fb6 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Mon, 22 Apr 2024 10:32:00 +0200 Subject: [PATCH 23/27] chore(linting): replace _ in args with _{argname} --- timed/employment/admin.py | 14 +++++++------- timed/employment/filters.py | 8 ++++---- timed/employment/permissions.py | 2 +- timed/permissions.py | 4 ++-- timed/projects/admin.py | 4 ++-- timed/projects/filters.py | 4 ++-- timed/projects/serializers.py | 4 ++-- timed/reports/filters.py | 4 ++-- timed/serializers.py | 2 +- timed/subscription/filters.py | 2 +- timed/tracking/filters.py | 6 +++--- timed/tracking/tasks.py | 2 +- 12 files changed, 28 insertions(+), 28 deletions(-) diff --git a/timed/employment/admin.py b/timed/employment/admin.py index 802363b3..db6c2866 100644 --- a/timed/employment/admin.py +++ b/timed/employment/admin.py @@ -173,27 +173,27 @@ def __init__(self, *args, **kwargs): (_("Extra fields"), {"fields": ["tour_done", "is_accountant"]}), ) - def disable_users(self, _, queryset): + def disable_users(self, _request, queryset): queryset.update(is_active=False) disable_users.short_description = _("Disable selected users") - def enable_users(self, _, queryset): + def enable_users(self, _request, queryset): queryset.update(is_active=True) enable_users.short_description = _("Enable selected users") - def disable_staff_status(self, _, 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, _, 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, _, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.reports.exists() @@ -204,7 +204,7 @@ class LocationAdmin(admin.ModelAdmin): list_display = ("name",) search_fields = ("name",) - def has_delete_permission(self, _, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.employments.exists() @@ -226,5 +226,5 @@ class AbsenceTypeAdmin(admin.ModelAdmin): list_display = ("name",) - def has_delete_permission(self, _, 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/filters.py b/timed/employment/filters.py index 25986d1f..30869cfe 100644 --- a/timed/employment/filters.py +++ b/timed/employment/filters.py @@ -54,15 +54,15 @@ class UserFilterSet(FilterSet): is_accountant = NumberFilter(field_name="is_accountant") is_external = NumberFilter(method="filter_is_external") - def filter_is_external(self, queryset, _, value): + def filter_is_external(self, queryset, _name, value): return queryset.filter(employments__is_external=value) - def filter_is_reviewer(self, queryset, _, 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, _, 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()) @@ -81,7 +81,7 @@ class Meta: class EmploymentFilterSet(FilterSet): date = DateFilter(method="filter_date") - def filter_date(self, queryset, _, value): + 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)) diff --git a/timed/employment/permissions.py b/timed/employment/permissions.py index 7ab3d69b..f363e5c1 100644 --- a/timed/employment/permissions.py +++ b/timed/employment/permissions.py @@ -2,5 +2,5 @@ class NoReports(BasePermission): - def has_object_permission(self, _, __, obj): + def has_object_permission(self, _view, _request, obj): return not obj.reports.exists() diff --git a/timed/permissions.py b/timed/permissions.py index 2e9d41b3..54a26bd8 100644 --- a/timed/permissions.py +++ b/timed/permissions.py @@ -11,7 +11,7 @@ class IsUnverified(BasePermission): """Allows access only to verified objects.""" - def has_object_permission(self, _, __, obj): + def has_object_permission(self, _request, _view, obj): return obj.verified_by_id is None @@ -161,7 +161,7 @@ def has_object_permission(self, request, view, *args, **kwargs): class IsNotTransferred(IsAuthenticated): """Allows access only to not transferred objects.""" - def has_object_permission(self, _, __, obj): + def has_object_permission(self, _request, _view, obj): return not obj.transferred diff --git a/timed/projects/admin.py b/timed/projects/admin.py index d4677069..d0a552b5 100644 --- a/timed/projects/admin.py +++ b/timed/projects/admin.py @@ -37,7 +37,7 @@ class CustomerAdmin(admin.ModelAdmin): search_fields = ("name",) inlines = (CustomerAssigneeInline,) - def has_delete_permission(self, _, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.projects.exists() @@ -125,7 +125,7 @@ class ProjectAdmin(NestedModelAdmin): inlines = (TaskInline, RedmineProjectInline, ProjectAssigneeInline) - def has_delete_permission(self, _, obj=None): + def has_delete_permission(self, _request, obj=None): return obj and not obj.tasks.exists() diff --git a/timed/projects/filters.py b/timed/projects/filters.py index 99990b47..3e1a773d 100644 --- a/timed/projects/filters.py +++ b/timed/projects/filters.py @@ -36,7 +36,7 @@ class ProjectFilterSet(FilterSet): has_reviewer = NumberFilter(method="filter_has_reviewer") customer = NumberInFilter(field_name="customer") - def filter_has_manager(self, queryset, _, value): + def filter_has_manager(self, queryset, _name, value): if not value: # pragma: no cover return queryset return queryset.filter( @@ -52,7 +52,7 @@ def filter_has_manager(self, queryset, _, value): ) ) - def filter_has_reviewer(self, queryset, _, value): + def filter_has_reviewer(self, queryset, _name, value): if not value: # pragma: no cover return queryset return queryset.filter( diff --git a/timed/projects/serializers.py b/timed/projects/serializers.py index 96d347ea..b76ec501 100644 --- a/timed/projects/serializers.py +++ b/timed/projects/serializers.py @@ -64,7 +64,7 @@ class ProjectSerializer(ModelSerializer): "cost_center": "timed.projects.serializers.CostCenterSerializer", } - def get_root_meta(self, _, 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")) @@ -134,7 +134,7 @@ class TaskSerializer(ModelSerializer): "cost_center": "timed.projects.serializers.CostCenterSerializer", } - def get_root_meta(self, _, 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")) diff --git a/timed/reports/filters.py b/timed/reports/filters.py index 52726275..c5547c62 100644 --- a/timed/reports/filters.py +++ b/timed/reports/filters.py @@ -11,7 +11,7 @@ class StatisticFiltersetBase: - def filter_has_reviewer(self, queryset, _, value): + def filter_has_reviewer(self, queryset, _name, value): if not value: # pragma: no cover return queryset @@ -37,7 +37,7 @@ def filter_has_reviewer(self, queryset, _, value): ) return queryset.filter_aggregate(the_filter).filter_base(the_filter) - def filter_cost_center(self, queryset, _, value): + def filter_cost_center(self, queryset, _name, value): """Filter report by cost center. The filter behaves slightly different depending on what the diff --git a/timed/serializers.py b/timed/serializers.py index 152c508b..28a25563 100644 --- a/timed/serializers.py +++ b/timed/serializers.py @@ -7,7 +7,7 @@ class TotalTimeRootMetaMixin: duration_field = "duration" - def get_root_meta(self, _, 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"] diff --git a/timed/subscription/filters.py b/timed/subscription/filters.py index 50930757..b2ccbe54 100644 --- a/timed/subscription/filters.py +++ b/timed/subscription/filters.py @@ -8,7 +8,7 @@ class PackageFilter(FilterSet): customer = NumberFilter(method="filter_customer") - def filter_customer(self, queryset, _, 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) diff --git a/timed/tracking/filters.py b/timed/tracking/filters.py index 523439f0..7303f4a6 100644 --- a/timed/tracking/filters.py +++ b/timed/tracking/filters.py @@ -104,7 +104,7 @@ class ReportFilterSet(FilterSet): cost_center = NumberFilter(method="filter_cost_center") rejected = NumberFilter(field_name="rejected") - def filter_has_reviewer(self, queryset, _, value): + def filter_has_reviewer(self, queryset, _name, value): if not value: # pragma: no cover return queryset @@ -158,7 +158,7 @@ def filter_has_reviewer(self, queryset, _, value): | reports_task_assignee_is_reviewer ) - def filter_editable(self, queryset, _, 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 @@ -203,7 +203,7 @@ def filter_editable(self, queryset, _, value): return queryset.exclude(editable_filter) - def filter_cost_center(self, queryset, _, value): + def filter_cost_center(self, queryset, _name, value): """Filter report by cost center. Cost center on task has higher priority over project cost diff --git a/timed/tracking/tasks.py b/timed/tracking/tasks.py index 4cfbc6fa..3c55b93d 100644 --- a/timed/tracking/tasks.py +++ b/timed/tracking/tasks.py @@ -95,7 +95,7 @@ def notify_user_rejected_report(report, reviewer): _send_notification_emails([user_changes], reviewer, rejected=True) -def notify_user_rejected_reports(queryset, _, reviewer): +def notify_user_rejected_reports(queryset, _fields, reviewer): users = [report.user for report in queryset.order_by("user").distinct("user")] user_changes = [] From 04c15296c8376b87c75e4a70c0b977e45c41617d Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Mon, 22 Apr 2024 11:35:39 +0200 Subject: [PATCH 24/27] docs: move sphinx-doc to type annotations move __str__'s from sphinx-doc to type annotations move worktime_per_day and similar properties/lazy_attributes from sphinx-doc to annotations serializers: move validate, clean from sphinx-doc to type annotations --- timed/employment/admin.py | 6 +----- timed/employment/factories.py | 16 ++++------------ timed/employment/models.py | 32 ++++++++------------------------ timed/employment/serializers.py | 6 +----- timed/projects/models.py | 32 +++++++++----------------------- timed/tracking/factories.py | 8 ++------ timed/tracking/models.py | 32 ++++++++------------------------ timed/tracking/serializers.py | 5 +---- 8 files changed, 34 insertions(+), 103 deletions(-) diff --git a/timed/employment/admin.py b/timed/employment/admin.py index db6c2866..aa683f08 100644 --- a/timed/employment/admin.py +++ b/timed/employment/admin.py @@ -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() 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/models.py b/timed/employment/models.py index a8fb5dda..704be41a 100644 --- a/timed/employment/models.py +++ b/timed/employment/models.py @@ -30,12 +30,8 @@ class Location(models.Model): class Meta: ordering = ("name",) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return self.name @@ -58,12 +54,8 @@ class Meta: indexes = (models.Index(fields=["date"]),) ordering = ("date",) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return "{} {}".format(self.name, self.date.strftime("%Y")) @@ -80,12 +72,8 @@ class AbsenceType(models.Model): class Meta: ordering = ("name",) - def __str__(self): - """Represent the model as a string. - - :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): @@ -232,12 +220,8 @@ class Meta: indexes = (models.Index(fields=["start_date", "end_date"]),) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return "{} ({} - {})".format( self.user.username, self.start_date.strftime("%d.%m.%Y"), diff --git a/timed/employment/serializers.py b/timed/employment/serializers.py index e5c3d78f..fd097c9f 100644 --- a/timed/employment/serializers.py +++ b/timed/employment/serializers.py @@ -237,15 +237,11 @@ class EmploymentSerializer(ModelSerializer): "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) diff --git a/timed/projects/models.py b/timed/projects/models.py index 443909a4..92e3b920 100644 --- a/timed/projects/models.py +++ b/timed/projects/models.py @@ -37,11 +37,7 @@ class Meta: ordering = ("name",) def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + """Represent the model as a string.""" return self.name @@ -68,6 +64,7 @@ class Meta: ordering = ("name",) def __str__(self): + """Represent the model as a string.""" return self.name @@ -119,12 +116,8 @@ class Project(models.Model): class Meta: ordering = ("name",) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return f"{self.customer} > {self.name}" @@ -167,12 +160,8 @@ class Meta: ordering = ("name",) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return f"{self.project} > {self.name}" @@ -188,12 +177,8 @@ class TaskTemplate(models.Model): class Meta: ordering = ("name",) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return self.name @@ -217,6 +202,7 @@ class CustomerAssignee(models.Model): is_customer = models.BooleanField(default=False) def __str__(self) -> str: + """Represent the model as a string.""" return f"{self.user.username} {self.customer}" diff --git a/timed/tracking/factories.py b/timed/tracking/factories.py index ce537137..a4e8ba47 100644 --- a/timed/tracking/factories.py +++ b/timed/tracking/factories.py @@ -36,12 +36,8 @@ class ReportFactory(DjangoModelFactory): user = SubFactory("timed.employment.factories.UserFactory") @lazy_attribute - def duration(self): - """Generate a random duration between 0 and 5 hours. - - :return: The generated duration - :rtype: datetime.timedelta - """ + def duration(self) -> 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/models.py b/timed/tracking/models.py index e8c9b226..7bb1cf13 100644 --- a/timed/tracking/models.py +++ b/timed/tracking/models.py @@ -37,12 +37,8 @@ class Meta: verbose_name_plural = "activities" indexes = (models.Index(fields=["date"]),) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return f"{self.user}: {self.task}" @@ -61,12 +57,8 @@ 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 - """ + def __str__(self) -> str: + """Represent the model as a string.""" return "{}: {} {} - {}".format( self.user, self.date.strftime("%Y-%m-%d"), @@ -108,12 +100,8 @@ class Meta: indexes = (models.Index(fields=["date"]),) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return f"{self.user}: {self.task}" def save(self, *args, **kwargs): @@ -153,12 +141,8 @@ class Meta: "user", ) - def __str__(self): - """Represent the model as a string. - - :return: The string representation - :rtype: str - """ + def __str__(self) -> str: + """Represent the model as a string.""" return "{}: {} {}".format( self.user, self.date.strftime("%Y-%m-%d"), diff --git a/timed/tracking/serializers.py b/timed/tracking/serializers.py index e46b1417..41d7a129 100644 --- a/timed/tracking/serializers.py +++ b/timed/tracking/serializers.py @@ -434,13 +434,10 @@ 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) From 5c540495097dbe7d5e8d2ded0b2423c9d12d475c Mon Sep 17 00:00:00 2001 From: Arthur D <110528300+c0rydoras@users.noreply.github.com> Date: Mon, 22 Apr 2024 11:46:03 +0200 Subject: [PATCH 25/27] chore: move todo from docstring to comments Co-authored-by: David Vogt --- timed/projects/filters.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/timed/projects/filters.py b/timed/projects/filters.py index 3e1a773d..7c51c605 100644 --- a/timed/projects/filters.py +++ b/timed/projects/filters.py @@ -82,15 +82,11 @@ class Meta: 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. From db841554b8eedd0ccf5ff72d9e6e1e184717bab6 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Mon, 22 Apr 2024 14:37:31 +0200 Subject: [PATCH 26/27] refactor: move code after except ModelDoesNotExist outside of try block --- timed/employment/views.py | 37 ++++++++++++++++++------------------- timed/tracking/views.py | 30 +++++++++++++++--------------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/timed/employment/views.py b/timed/employment/views.py index 9f831585..7c316e96 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -67,23 +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)) except models.Employment.DoesNotExist: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): assigned_tasks = Task.objects.filter( @@ -98,8 +81,24 @@ def get_queryset(self): return queryset.filter(Q(reports__in=visible_reports) | Q(id=user.id)) msg = "User has no employment" raise exceptions.PermissionDenied(msg) from None - else: - return queryset + 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, *args, **kwargs): diff --git a/timed/tracking/views.py b/timed/tracking/views.py index 77949e08..aab2aa7b 100644 --- a/timed/tracking/views.py +++ b/timed/tracking/views.py @@ -138,21 +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, - ) - ) - return queryset.filter(Q(task__in=assigned_tasks) | Q(user=user)) except Employment.DoesNotExist: if CustomerAssignee.objects.filter(user=user, is_customer=True).exists(): return queryset.filter( @@ -163,6 +148,21 @@ def get_queryset(self): ) 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.""" From 4bcb7abe925bb4eb1bee7d795ece45d1d594fcb2 Mon Sep 17 00:00:00 2001 From: Arthur Deierlein Date: Tue, 23 Apr 2024 15:18:22 +0200 Subject: [PATCH 27/27] refactor: replace *args, **kwargs with _{argname} wherever possible while editing code to make ruff happy about unused variables i ended up replacing unused args/kwargs with *args, **kwargs, this commit reverts this to use the argname with an underscore wherever possible --- timed/authentication.py | 4 ++-- timed/employment/views.py | 4 ++-- timed/permissions.py | 24 ++++++++++++------------ timed/projects/admin.py | 2 +- timed/subscription/views.py | 4 ++-- timed/tracking/filters.py | 2 +- timed/tracking/views.py | 2 +- 7 files changed, 21 insertions(+), 21 deletions(-) diff --git a/timed/authentication.py b/timed/authentication.py index 0242d241..491299ef 100644 --- a/timed/authentication.py +++ b/timed/authentication.py @@ -12,7 +12,7 @@ class TimedOIDCAuthenticationBackend(OIDCAuthenticationBackend): - def get_introspection(self, access_token, *args, **kwargs): + 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() @@ -57,7 +57,7 @@ def get_userinfo_or_introspection(self, access_token): return claims raise AuthenticationFailed from exc - def get_or_create_user(self, access_token, *args, **kwargs): + 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) diff --git a/timed/employment/views.py b/timed/employment/views.py index 7c316e96..9548aae6 100644 --- a/timed/employment/views.py +++ b/timed/employment/views.py @@ -101,14 +101,14 @@ def get_queryset(self): return queryset @action(methods=["get"], detail=False) - def me(self, request, *args, **kwargs): + 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, *args, **kwargs): + 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 diff --git a/timed/permissions.py b/timed/permissions.py index 54a26bd8..a5d5b525 100644 --- a/timed/permissions.py +++ b/timed/permissions.py @@ -18,50 +18,50 @@ def has_object_permission(self, _request, _view, obj): class IsReadOnly(BasePermission): """Allows read only methods.""" - def has_permission(self, request, *args, **kwargs): + def has_permission(self, request, _view): return request.method in SAFE_METHODS - def has_object_permission(self, request, view, *args, **kwargs): + 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, *args, **kwargs): + def has_permission(self, request, _view): return request.method == "DELETE" - def has_object_permission(self, request, view, *args, **kwargs): + 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, *args, **kwargs): + def has_permission(self, request, _view): return request.method != "DELETE" - def has_object_permission(self, request, view, *args, **kwargs): + 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, *args, **kwargs): + def has_permission(self, request, _view): return request.method == "POST" - def has_object_permission(self, request, view, *args, **kwargs): + 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, *args, **kwargs): + def has_permission(self, request, _view): return request.method in ["PATCH", "PUT"] - def has_object_permission(self, request, view, *args, **kwargs): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) @@ -72,7 +72,7 @@ class IsAuthenticated(IsAuthenticated): operator. """ - def has_object_permission(self, request, view, *args, **kwargs): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) @@ -154,7 +154,7 @@ def has_permission(self, request, view): return request.user.is_superuser - def has_object_permission(self, request, view, *args, **kwargs): + def has_object_permission(self, request, view, _obj): return self.has_permission(request, view) diff --git a/timed/projects/admin.py b/timed/projects/admin.py index d0a552b5..41806155 100644 --- a/timed/projects/admin.py +++ b/timed/projects/admin.py @@ -95,7 +95,7 @@ class TaskInline(NestedStackedInline): extra = 0 inlines = (TaskAssigneeInline,) - def has_delete_permission(self, *args, **kwargs): + def has_delete_permission(self, _request, _obj): # for some reason obj is parent object and not task # so this doesn't work return False diff --git a/timed/subscription/views.py b/timed/subscription/views.py index e826df53..1cb08ec4 100644 --- a/timed/subscription/views.py +++ b/timed/subscription/views.py @@ -99,7 +99,7 @@ def create(self, request, *args, **kwargs): methods=["post"], permission_classes=[IsSuperUser | IsAccountant], ) - def confirm(self, request, *args, **kwargs): + def confirm(self, request, pk=None): # noqa: ARG002 """Confirm order. Only allowed by staff members @@ -114,7 +114,7 @@ def confirm(self, request, *args, **kwargs): def get_queryset(self): return models.Order.objects.select_related("project") - def destroy(self, *args, **kwargs): + def destroy(self, _request, pk=None): # noqa: ARG002 instance = self.get_object() if instance.acknowledged: # acknowledge orders may not be deleted diff --git a/timed/tracking/filters.py b/timed/tracking/filters.py index 7303f4a6..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, *args, **kwargs): + def filter(self, qs, _value): """Filter the queryset. :param QuerySet qs: The queryset to filter diff --git a/timed/tracking/views.py b/timed/tracking/views.py index aab2aa7b..366c0ea7 100644 --- a/timed/tracking/views.py +++ b/timed/tracking/views.py @@ -191,7 +191,7 @@ def update(self, request, *args, **kwargs): methods=["get"], serializer_class=serializers.ReportIntersectionSerializer, ) - def intersection(self, *args, **kwargs): + 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