From a4bb163ea09e4e0519b7d2af4211c22952151ae4 Mon Sep 17 00:00:00 2001 From: Juha Louhiranta Date: Tue, 30 Jan 2024 16:22:06 +0200 Subject: [PATCH] fix: package update related fixes and formatting Refs: HP-2082 --- open_city_profile/decorators.py | 4 ++-- open_city_profile/graphene.py | 4 ++-- open_city_profile/tests/conftest.py | 10 ++++---- .../tests/graphql_test_helpers.py | 2 +- .../snapshots/snap_test_graphql_api_schema.py | 1 - .../tests/test_graphql_authentication.py | 4 ++-- open_city_profile/views.py | 3 +-- profiles/helpers.py | 5 ++++ profiles/schema.py | 24 +++++++++---------- .../test_gdpr_functions_with_multiple_idps.py | 6 ++--- .../test_gql_delete_my_profile_mutation.py | 5 +++- profiles/tests/profile_input_validation.py | 2 +- profiles/tests/test_audit_log_to_db.py | 2 +- profiles/tests/test_audit_log_to_logger.py | 2 +- .../tests/test_gql_claim_profile_mutation.py | 3 ++- .../tests/test_gql_claimable_profile_query.py | 2 +- .../tests/test_gql_create_profile_mutation.py | 22 +++++++++-------- profiles/tests/test_gql_federation.py | 3 ++- profiles/tests/test_gql_profiles_query.py | 5 ++-- ...l_service_connection_with_user_id_query.py | 3 ++- .../test_gql_update_my_profile_mutation.py | 2 +- .../tests/test_gql_update_profile_mutation.py | 2 +- .../tests/test_profile_changes_to_keycloak.py | 8 ++++--- services/enums.py | 6 ++--- services/exceptions.py | 2 +- utils/keycloak.py | 4 ++-- 26 files changed, 76 insertions(+), 60 deletions(-) create mode 100644 profiles/helpers.py diff --git a/open_city_profile/decorators.py b/open_city_profile/decorators.py index bb46abb5..5d317ed6 100644 --- a/open_city_profile/decorators.py +++ b/open_city_profile/decorators.py @@ -2,7 +2,7 @@ from django.core.exceptions import PermissionDenied from django.utils.translation import gettext_lazy as _ -from graphql.execution.base import ResolveInfo +from graphql.type import GraphQLResolveInfo from open_city_profile.exceptions import ServiceNotIdentifiedError @@ -13,7 +13,7 @@ def decorator(decorator_function): def wrapper(function): @wraps(function) def context_tester(*args, **kwargs): - info = next(arg for arg in args if isinstance(arg, ResolveInfo)) + info = next(arg for arg in args if isinstance(arg, GraphQLResolveInfo)) context = info.context for test_func in test_funcs: diff --git a/open_city_profile/graphene.py b/open_city_profile/graphene.py index c5737ff9..8ead93ec 100644 --- a/open_city_profile/graphene.py +++ b/open_city_profile/graphene.py @@ -151,7 +151,7 @@ def __init_subclass_with_meta__( interfaces=(), convert_choices_to_enum=True, _meta=None, - **options + **options, ): assert issubclass(model, TranslatableModel), ( 'You need to pass a valid Django Parler Model in {}.Meta, received "{}".' @@ -176,5 +176,5 @@ def __init_subclass_with_meta__( interfaces=interfaces, convert_choices_to_enum=convert_choices_to_enum, _meta=_meta, - **options + **options, ) diff --git a/open_city_profile/tests/conftest.py b/open_city_profile/tests/conftest.py index 4445252e..78df621b 100644 --- a/open_city_profile/tests/conftest.py +++ b/open_city_profile/tests/conftest.py @@ -10,7 +10,7 @@ from graphene.test import Client as GrapheneClient from graphene_django.settings import graphene_settings from graphene_django.views import instantiate_middleware -from graphql import build_client_schema, introspection_query +from graphql import build_client_schema, get_introspection_query from helusers.authz import UserAuthorization from open_city_profile.schema import schema @@ -33,7 +33,7 @@ def execute( auth_token_payload=None, service=_not_provided, context=None, - **kwargs + **kwargs, ): """ Custom execute method which adds all of the middlewares defined in the @@ -70,7 +70,7 @@ def execute( *args, context=context, middleware=list(instantiate_middleware(graphene_settings.MIDDLEWARE)), - **kwargs + **kwargs, ) @@ -179,7 +179,9 @@ def superuser_gql_client(): @pytest.fixture def gql_schema(anon_user_gql_client): - introspection = anon_user_gql_client.execute(introspection_query) + introspection = anon_user_gql_client.execute( + get_introspection_query(descriptions=False) + ) return build_client_schema(introspection["data"]) diff --git a/open_city_profile/tests/graphql_test_helpers.py b/open_city_profile/tests/graphql_test_helpers.py index 8a89cb81..d3183170 100644 --- a/open_city_profile/tests/graphql_test_helpers.py +++ b/open_city_profile/tests/graphql_test_helpers.py @@ -38,7 +38,7 @@ def generate_jwt_token(extra_claims=None): "iat": get_unix_timestamp_now() - 10, "aud": AUDIENCE, "sub": str(uuid.uuid4()), - "sid": get_random_string(), + "sid": get_random_string(12), "exp": get_unix_timestamp_now() + 120, } jwt_data.update(extra_claims) diff --git a/open_city_profile/tests/snapshots/snap_test_graphql_api_schema.py b/open_city_profile/tests/snapshots/snap_test_graphql_api_schema.py index 971a3173..3175e2bf 100644 --- a/open_city_profile/tests/snapshots/snap_test_graphql_api_schema.py +++ b/open_city_profile/tests/snapshots/snap_test_graphql_api_schema.py @@ -4,7 +4,6 @@ from snapshottest import Snapshot - snapshots = Snapshot() snapshots['test_graphql_schema_matches_the_reference 1'] = '''type Query { diff --git a/open_city_profile/tests/test_graphql_authentication.py b/open_city_profile/tests/test_graphql_authentication.py index 4f62488d..dbbe885c 100644 --- a/open_city_profile/tests/test_graphql_authentication.py +++ b/open_city_profile/tests/test_graphql_authentication.py @@ -28,7 +28,7 @@ def test_presenting_an_expired_access_token_with_any_operation_returns_jwt_authe assert len(errors) == 2 for path in ["myProfile", "_service"]: - assert data[path] is None + assert data is None error = next(err for err in errors if err["path"] == [path]) assert error["extensions"]["code"] == "JWT_AUTHENTICATION_ERROR" @@ -51,7 +51,7 @@ def test_presenting_a_logged_out_token_returns_jwt_authentication_error(live_ser assert len(errors) == 2 for path in ["myProfile", "_service"]: - assert data[path] is None + assert data is None error = next(err for err in errors if err["path"] == [path]) assert error["extensions"]["code"] == "JWT_AUTHENTICATION_ERROR" diff --git a/open_city_profile/views.py b/open_city_profile/views.py index d884e951..3648ec51 100644 --- a/open_city_profile/views.py +++ b/open_city_profile/views.py @@ -93,8 +93,7 @@ class GraphQLView(BaseGraphQLView): def execute_graphql_request(self, request, data, query, *args, **kwargs): """Extract any exceptions and send some of them to Sentry""" result = super().execute_graphql_request(request, data, query, *args, **kwargs) - # If 'invalid' is set, it's a bad request - if result and result.errors and not result.invalid: + if result and result.errors: errors = [ e for e in result.errors diff --git a/profiles/helpers.py b/profiles/helpers.py new file mode 100644 index 00000000..edaf796a --- /dev/null +++ b/profiles/helpers.py @@ -0,0 +1,5 @@ +from graphql_relay import to_global_id as relay_to_global_id + + +def to_global_id(type, id): + return relay_to_global_id(type_=type, id_=id) diff --git a/profiles/schema.py b/profiles/schema.py index 38af316a..33f3bb46 100644 --- a/profiles/schema.py +++ b/profiles/schema.py @@ -89,7 +89,7 @@ profile_updated = django.dispatch.Signal() -def get_claimable_profile(token=None): +def get_claimable_profile(token=None) -> Profile: claim_token = ClaimToken.objects.get(token=token) if claim_token.expires_at and claim_token.expires_at < timezone.now(): raise TokenExpiredError("Token for claiming this profile has expired") @@ -206,7 +206,7 @@ def update_sensitivedata(profile, sensitive_data): with override("en"): Language = graphene.Enum( - "Language", [(l[1].upper(), l[0]) for l in settings.LANGUAGES] + "Language", [(lang[1].upper(), lang[0]) for lang in settings.LANGUAGES] ) ContactMethod = graphene.Enum( "ContactMethod", [(cm[1].upper(), cm[0]) for cm in settings.CONTACT_METHODS] @@ -537,22 +537,22 @@ class Meta: language = Language() contact_method = ContactMethod() - def resolve_primary_email(self, info, **kwargs): + def resolve_primary_email(self: Profile, info, **kwargs): return info.context.primary_email_for_profile_loader.load(self.id) - def resolve_primary_phone(self, info, **kwargs): + def resolve_primary_phone(self: Profile, info, **kwargs): return info.context.primary_phone_for_profile_loader.load(self.id) - def resolve_primary_address(self, info, **kwargs): + def resolve_primary_address(self: Profile, info, **kwargs): return info.context.primary_address_for_profile_loader.load(self.id) - def resolve_emails(self, info, **kwargs): + def resolve_emails(self: Profile, info, **kwargs): return info.context.emails_by_profile_id_loader.load(self.id) - def resolve_phones(self, info, **kwargs): + def resolve_phones(self: Profile, info, **kwargs): return info.context.phones_by_profile_id_loader.load(self.id) - def resolve_addresses(self, info, **kwargs): + def resolve_addresses(self: Profile, info, **kwargs): return info.context.addresses_by_profile_id_loader.load(self.id) @@ -579,10 +579,10 @@ class Meta: "privileges to access this information.", ) - def resolve_service_connections(self, info, **kwargs): + def resolve_service_connections(self: Profile, info, **kwargs): return self.effective_service_connections_qs() - def resolve_sensitivedata(self, info, **kwargs): + def resolve_sensitivedata(self: Profile, info, **kwargs): service = info.context.service if info.context.user == self.user or info.context.user.has_perm( @@ -593,7 +593,7 @@ def resolve_sensitivedata(self, info, **kwargs): # TODO: We should return PermissionDenied as a partial error here. return None - def resolve_verified_personal_information(self, info, **kwargs): + def resolve_verified_personal_information(self: Profile, info, **kwargs): loa = info.context.user_auth.data.get("loa") if ( info.context.user == self.user and loa in ["substantial", "high"] @@ -608,7 +608,7 @@ def resolve_verified_personal_information(self, info, **kwargs): ) @login_and_service_required - def __resolve_reference(self, info, **kwargs): + def __resolve_reference(self: Profile, info, **kwargs): profile = graphene.Node.get_node_from_global_id( info, self.id, only_type=ProfileNode ) diff --git a/profiles/tests/gdpr/test_gdpr_functions_with_multiple_idps.py b/profiles/tests/gdpr/test_gdpr_functions_with_multiple_idps.py index 19b1f4a0..f584b934 100644 --- a/profiles/tests/gdpr/test_gdpr_functions_with_multiple_idps.py +++ b/profiles/tests/gdpr/test_gdpr_functions_with_multiple_idps.py @@ -128,9 +128,9 @@ def get_keycloak_token_response(token_request, context): for tunnistamo_service in tunnistamo_services: scope = tunnistamo_service.gdpr_query_scope api_audience = scope[: scope.rfind(".")] - tunnistamo_api_tokens[ - api_audience - ] = f"{tunnistamo_service.name}-{TUNNISTAMO_API_TOKEN_MARKER}" + tunnistamo_api_tokens[api_audience] = ( + f"{tunnistamo_service.name}-{TUNNISTAMO_API_TOKEN_MARKER}" + ) requests_mock.get( settings.TUNNISTAMO_API_TOKENS_URL, diff --git a/profiles/tests/gdpr/test_gql_delete_my_profile_mutation.py b/profiles/tests/gdpr/test_gql_delete_my_profile_mutation.py index 367196ee..cf10dbba 100644 --- a/profiles/tests/gdpr/test_gql_delete_my_profile_mutation.py +++ b/profiles/tests/gdpr/test_gql_delete_my_profile_mutation.py @@ -134,7 +134,10 @@ def test_user_can_delete_their_profile( assert Profile.objects.filter(pk=profile.pk).exists() -@pytest.mark.parametrize("should_fail", [False, True]) +@pytest.mark.parametrize( + "should_fail", + [pytest.param(False, id="should_not_fail"), pytest.param(True, id="should_fail")], +) def test_user_can_dry_run_profile_deletion( user_gql_client, service_1, diff --git a/profiles/tests/profile_input_validation.py b/profiles/tests/profile_input_validation.py index 6fa0f769..f1892b6e 100644 --- a/profiles/tests/profile_input_validation.py +++ b/profiles/tests/profile_input_validation.py @@ -1,10 +1,10 @@ import pytest -from graphql_relay import to_global_id from open_city_profile.tests import to_graphql_name, to_graphql_object from open_city_profile.tests.asserts import assert_match_error_code from profiles.models import Profile +from ..helpers import to_global_id from .factories import AddressFactory, EmailFactory, PhoneFactory diff --git a/profiles/tests/test_audit_log_to_db.py b/profiles/tests/test_audit_log_to_db.py index 7c408ba4..f8779820 100644 --- a/profiles/tests/test_audit_log_to_db.py +++ b/profiles/tests/test_audit_log_to_db.py @@ -4,7 +4,6 @@ from typing import Any, List, Optional import pytest -from graphql_relay import to_global_id from guardian.shortcuts import assign_perm from audit_log.models import LogEntry @@ -22,6 +21,7 @@ ) from services.tests.factories import ServiceConnectionFactory +from ..helpers import to_global_id from .factories import ( AddressFactory, EmailFactory, diff --git a/profiles/tests/test_audit_log_to_logger.py b/profiles/tests/test_audit_log_to_logger.py index 181d460f..1e1610d6 100644 --- a/profiles/tests/test_audit_log_to_logger.py +++ b/profiles/tests/test_audit_log_to_logger.py @@ -6,7 +6,6 @@ from typing import Any, List, Optional import pytest -from graphql_relay import to_global_id from guardian.shortcuts import assign_perm from open_city_profile.tests import to_graphql_name @@ -23,6 +22,7 @@ ) from services.tests.factories import ServiceConnectionFactory +from ..helpers import to_global_id from .factories import ( AddressFactory, EmailFactory, diff --git a/profiles/tests/test_gql_claim_profile_mutation.py b/profiles/tests/test_gql_claim_profile_mutation.py index 1a0647a3..415b471e 100644 --- a/profiles/tests/test_gql_claim_profile_mutation.py +++ b/profiles/tests/test_gql_claim_profile_mutation.py @@ -2,12 +2,12 @@ from string import Template from django.utils import timezone -from graphql_relay.node.node import to_global_id from open_city_profile.consts import API_NOT_IMPLEMENTED_ERROR, TOKEN_EXPIRED_ERROR from open_city_profile.tests.asserts import assert_match_error_code from profiles.models import Profile +from ..helpers import to_global_id from .factories import ( ClaimTokenFactory, EmailFactory, @@ -164,6 +164,7 @@ def test_changing_an_email_address_marks_it_unverified(user_gql_client): } executed = user_gql_client.execute(CLAIM_PROFILE_MUTATION, variables=variables) + assert "errors" not in executed assert executed["data"] == expected_data diff --git a/profiles/tests/test_gql_claimable_profile_query.py b/profiles/tests/test_gql_claimable_profile_query.py index 09bf0720..059d7037 100644 --- a/profiles/tests/test_gql_claimable_profile_query.py +++ b/profiles/tests/test_gql_claimable_profile_query.py @@ -2,11 +2,11 @@ from string import Template from django.utils import timezone -from graphql_relay.node.node import to_global_id from open_city_profile.consts import TOKEN_EXPIRED_ERROR from open_city_profile.tests.asserts import assert_match_error_code +from ..helpers import to_global_id from .factories import ClaimTokenFactory, ProfileFactory diff --git a/profiles/tests/test_gql_create_profile_mutation.py b/profiles/tests/test_gql_create_profile_mutation.py index 5a9d79b4..54538412 100644 --- a/profiles/tests/test_gql_create_profile_mutation.py +++ b/profiles/tests/test_gql_create_profile_mutation.py @@ -111,17 +111,19 @@ def test_staff_user_can_create_a_profile( ] }, "emails": { - "edges": [ - { - "node": { - "emailType": email_data["email_type"], - "email": email_data["email"], - "primary": True, + "edges": ( + [ + { + "node": { + "emailType": email_data["email_type"], + "email": email_data["email"], + "primary": True, + } } - } - ] - if with_email - else [] + ] + if with_email + else [] + ) }, "serviceConnections": { "edges": [ diff --git a/profiles/tests/test_gql_federation.py b/profiles/tests/test_gql_federation.py index 149dd90b..e2c1f500 100644 --- a/profiles/tests/test_gql_federation.py +++ b/profiles/tests/test_gql_federation.py @@ -1,5 +1,4 @@ import pytest -from graphql_relay import to_global_id from guardian.shortcuts import assign_perm from open_city_profile.tests.asserts import assert_match_error_code @@ -7,6 +6,8 @@ from profiles.tests.factories import AddressFactory, ProfileFactory from services.tests.factories import ServiceConnectionFactory +from ..helpers import to_global_id + GRAPHQL_SDL_QUERY = """ query { _service { diff --git a/profiles/tests/test_gql_profiles_query.py b/profiles/tests/test_gql_profiles_query.py index e5cee90d..bf87f65d 100644 --- a/profiles/tests/test_gql_profiles_query.py +++ b/profiles/tests/test_gql_profiles_query.py @@ -1,8 +1,9 @@ -import pytest import uuid +from string import Template + +import pytest from django.utils.translation import gettext_lazy as _ from guardian.shortcuts import assign_perm -from string import Template from open_city_profile.tests import to_graphql_name from open_city_profile.tests.asserts import assert_match_error_code diff --git a/profiles/tests/test_gql_service_connection_with_user_id_query.py b/profiles/tests/test_gql_service_connection_with_user_id_query.py index 8991aad3..00d4d9f2 100644 --- a/profiles/tests/test_gql_service_connection_with_user_id_query.py +++ b/profiles/tests/test_gql_service_connection_with_user_id_query.py @@ -1,11 +1,12 @@ import datetime import uuid -from graphql_relay import to_global_id from guardian.shortcuts import assign_perm from open_city_profile.tests.asserts import assert_match_error_code +from ..helpers import to_global_id + QUERY = """ query ($userId: UUID!, $serviceClientId: String!) { diff --git a/profiles/tests/test_gql_update_my_profile_mutation.py b/profiles/tests/test_gql_update_my_profile_mutation.py index d210aefc..a2f579fa 100644 --- a/profiles/tests/test_gql_update_my_profile_mutation.py +++ b/profiles/tests/test_gql_update_my_profile_mutation.py @@ -1,7 +1,6 @@ from string import Template import pytest -from graphql_relay.node.node import to_global_id from open_city_profile.tests.asserts import assert_match_error_code from profiles.models import Address, Email, Phone, Profile @@ -9,6 +8,7 @@ from services.tests.factories import ServiceConnectionFactory from utils import keycloak +from ..helpers import to_global_id from .factories import ( AddressFactory, EmailFactory, diff --git a/profiles/tests/test_gql_update_profile_mutation.py b/profiles/tests/test_gql_update_profile_mutation.py index 6963030e..d058573b 100644 --- a/profiles/tests/test_gql_update_profile_mutation.py +++ b/profiles/tests/test_gql_update_profile_mutation.py @@ -2,7 +2,6 @@ import pytest from django.utils.translation import gettext_lazy as _ -from graphql_relay.node.node import to_global_id from guardian.shortcuts import assign_perm from open_city_profile.tests.asserts import assert_match_error_code @@ -12,6 +11,7 @@ from services.tests.factories import ServiceConnectionFactory, ServiceFactory from utils import keycloak +from ..helpers import to_global_id from .factories import ( AddressFactory, EmailFactory, diff --git a/profiles/tests/test_profile_changes_to_keycloak.py b/profiles/tests/test_profile_changes_to_keycloak.py index cc453072..2b1b1855 100644 --- a/profiles/tests/test_profile_changes_to_keycloak.py +++ b/profiles/tests/test_profile_changes_to_keycloak.py @@ -93,9 +93,11 @@ def test_changing_email_causes_it_to_be_marked_unverified( mocked_send_verify_email = mocker.patch.object( keycloak.KeycloakAdminClient, "send_verify_email", - side_effect=None - if send_verify_email_succeeds - else Exception("send_verify_email failed"), + side_effect=( + None + if send_verify_email_succeeds + else Exception("send_verify_email failed") + ), ) profile = ProfileFactory( diff --git a/services/enums.py b/services/enums.py index bc241dfb..9ab937b6 100644 --- a/services/enums.py +++ b/services/enums.py @@ -1,8 +1,8 @@ +import enumfields from django.utils.translation import gettext_lazy as _ -from enumfields import Enum -class ServiceType(Enum): +class ServiceType(enumfields.Enum): HKI_MY_DATA = "hki_my_data" BERTH = "berth" YOUTH_MEMBERSHIP = "youth_membership" @@ -15,7 +15,7 @@ class Labels: GODCHILDREN_OF_CULTURE = _("Godchildren of Culture") -class ServiceIdp(Enum): +class ServiceIdp(enumfields.Enum): TUNNISTAMO = "tunnistamo" KEYCLOAK = "keycloak" diff --git a/services/exceptions.py b/services/exceptions.py index 46344d53..329f3500 100644 --- a/services/exceptions.py +++ b/services/exceptions.py @@ -1,2 +1,2 @@ -class MissingGDPRUrlException(Exception): +class MissingGDPRUrlException(Exception): # noqa: N818 """Indicate that Service is missing GDPR related URLs.""" diff --git a/utils/keycloak.py b/utils/keycloak.py index ee3fe867..afc2e466 100644 --- a/utils/keycloak.py +++ b/utils/keycloak.py @@ -77,7 +77,7 @@ def _get_auth(self, force_renew=False): ) if not result.ok: - raise AuthenticationError(f"Couldn't authenticate to Keycloak") + raise AuthenticationError("Couldn't authenticate to Keycloak") client_credentials = result.json() access_token = client_credentials["access_token"] @@ -137,7 +137,7 @@ def delete_user(self, user_id): def send_verify_email(self, user_id): url = self._single_user_url(user_id) - url += f"/send-verify-email" + url += "/send-verify-email" return self._handle_user_request( lambda auth: self._session.put(