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..f6ab0600 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) # pramga: no cover class IsAccountant(IsAuthenticated): diff --git a/timed/projects/admin.py b/timed/projects/admin.py index a1de3a66..296a0a86 100644 --- a/timed/projects/admin.py +++ b/timed/projects/admin.py @@ -13,19 +13,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 @@ -34,9 +34,12 @@ class TaskAssigneeInline(NestedStackedInline): class CustomerAdmin(admin.ModelAdmin): """Customer admin view.""" - list_display = ["name"] - search_fields = ["name"] - inlines = [CustomerPasswordInline, CustomerAssigneeInline] + list_display = ("name",) + search_fields = ("name",) + inlines = ( + CustomerPasswordInline, + CustomerAssigneeInline, + ) def has_delete_permission(self, request, obj=None): return obj and not obj.projects.exists() @@ -44,19 +47,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. """ @@ -92,7 +97,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 @@ -113,11 +118,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() @@ -127,4 +135,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 63a4adaa..8e4b3ac5 100644 --- a/timed/subscription/admin.py +++ b/timed/subscription/admin.py @@ -16,7 +16,7 @@ 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/models.py b/timed/subscription/models.py index 6c40bcc1..2da56e78 100644 --- a/timed/subscription/models.py +++ b/timed/subscription/models.py @@ -43,8 +43,7 @@ class Order(models.Model): class CustomerPassword(models.Model): - """ - Password per customer used for login into SySupport portal. + """Password per customer used for login into SySupport portal. Password are only hashed with md5. This model will be obsolete once customer center will go live. 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``.