From cc344a9f96ddab45fac99da1a00abcc8a42ead1b Mon Sep 17 00:00:00 2001 From: Tuomas Suutari Date: Thu, 23 Nov 2023 19:44:57 +0200 Subject: [PATCH 1/6] Add and use PermitAreaQueryset.for_user Add a QuerySet method named "for_user" for PermitAreas and use it instead of direct filtering by the field name. This allows changing the field name without changes to these filters. Refs HPH-36 --- parkings/api/common_permit.py | 2 +- parkings/api/operator/permit.py | 2 +- parkings/models/permit.py | 7 +++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/parkings/api/common_permit.py b/parkings/api/common_permit.py index 5167b2cb..749fb5df 100644 --- a/parkings/api/common_permit.py +++ b/parkings/api/common_permit.py @@ -98,7 +98,7 @@ def validate(self, attrs): def _validate_areas(self, attrs): user = self.context['request'].user domain = self.instance.domain if self.instance else attrs['domain'] - areas = PermitArea.objects.filter(permitted_user=user, domain=domain) + areas = PermitArea.objects.for_user(user).filter(domain=domain) area_identifiers = set(x['area'] for x in attrs.get('areas', [])) found = areas.filter(identifier__in=area_identifiers) if found.count() != len(area_identifiers): diff --git a/parkings/api/operator/permit.py b/parkings/api/operator/permit.py index c2af3bcd..73993fe4 100644 --- a/parkings/api/operator/permit.py +++ b/parkings/api/operator/permit.py @@ -73,4 +73,4 @@ class OperatorPermittedPermitAreaViewSet(mixins.ListModelMixin, GenericViewSet): serializer_class = OperatorPermitAreaSerializer def get_queryset(self): - return PermitArea.objects.filter(permitted_user=self.request.user) + return PermitArea.objects.for_user(self.request.user) diff --git a/parkings/models/permit.py b/parkings/models/permit.py index c9caaf60..4b3cc4b9 100644 --- a/parkings/models/permit.py +++ b/parkings/models/permit.py @@ -17,6 +17,11 @@ from .parking import Parking +class PermitAreaQuerySet(models.QuerySet): + def for_user(self, user): + return self.filter(permitted_user=user) + + class PermitArea(TimestampedModelMixin): name = models.CharField(max_length=40, verbose_name=_('name')) domain = models.ForeignKey( @@ -28,6 +33,8 @@ class PermitArea(TimestampedModelMixin): permitted_user = models.ForeignKey( settings.AUTH_USER_MODEL, on_delete=models.PROTECT, verbose_name=_("permitted_user")) + objects = PermitAreaQuerySet.as_manager() + class Meta: unique_together = [('domain', 'identifier')] ordering = ('identifier',) From 5adff5627af3da57061c2a8460a4b3d464a837de Mon Sep 17 00:00:00 2001 From: Tuomas Suutari Date: Thu, 23 Nov 2023 20:12:44 +0200 Subject: [PATCH 2/6] Add and populate PermitArea.allowed_users Add a new many-2-many field named "allowed_users" to PermitArea and populate it with the user object from the "permitted_user" field. This allows multiple operators/enforcers to use the same PermitAreas for their permits. also make the permitted_user field nullable for now to allow migrations to be reversed. Refs HPH-36 --- .sanitizerconfig | 4 ++ .../0046_permitarea_allowed_users.py | 39 +++++++++++++++++++ .../0047_populate_permitarea_allowed_users.py | 29 ++++++++++++++ parkings/models/permit.py | 11 +++++- 4 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 parkings/migrations/0046_permitarea_allowed_users.py create mode 100644 parkings/migrations/0047_populate_permitarea_allowed_users.py diff --git a/.sanitizerconfig b/.sanitizerconfig index 723fabf1..c150c7a1 100644 --- a/.sanitizerconfig +++ b/.sanitizerconfig @@ -181,6 +181,10 @@ strategy: modified_at: null name: null permitted_user_id: null + parkings_permitarea_allowed_users: + id: null + permitarea_id: null + user_id: null parkings_permitareaitem: area_id: null end_time: null diff --git a/parkings/migrations/0046_permitarea_allowed_users.py b/parkings/migrations/0046_permitarea_allowed_users.py new file mode 100644 index 00000000..821c7e16 --- /dev/null +++ b/parkings/migrations/0046_permitarea_allowed_users.py @@ -0,0 +1,39 @@ +# Generated by Django 2.2.15 on 2023-11-23 18:03 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("parkings", "0045_parkingarea_name"), + ] + + operations = [ + # Allow null values temporarily to make the migrations + # reversible (the field will be removed in the following + # migrations so it doesn't matter for the forward direction) + migrations.AlterField( + model_name="permitarea", + name="permitted_user", + field=models.ForeignKey( + on_delete=models.SET_NULL, + null=True, + to=settings.AUTH_USER_MODEL, + verbose_name="permitted_user", + ), + ), + + # Add the new field + migrations.AddField( + model_name="permitarea", + name="allowed_users", + field=models.ManyToManyField( + help_text="Users who are allowed to create permits to this area.", + related_name="allowed_permit_areas", + to=settings.AUTH_USER_MODEL, + verbose_name="allowed users", + ), + ), + ] diff --git a/parkings/migrations/0047_populate_permitarea_allowed_users.py b/parkings/migrations/0047_populate_permitarea_allowed_users.py new file mode 100644 index 00000000..1583a20a --- /dev/null +++ b/parkings/migrations/0047_populate_permitarea_allowed_users.py @@ -0,0 +1,29 @@ +# Migration to populate the PermitArea.allowed_users field + +from django.db import migrations + + +def populate_allowed_users_from_permitted_user(apps, schema_editor): + permit_area_model = apps.get_model("parkings", "PermitArea") + for permit_area in permit_area_model.objects.all(): + permit_area.allowed_users.add(permit_area.permitted_user) + + +def populate_permitted_user_from_allowed_users(apps, schema_editor): + permit_area_model = apps.get_model("parkings", "PermitArea") + for permit_area in permit_area_model.objects.all(): + permit_area.permitted_user = permit_area.allowed_users.first() + permit_area.save() + + +class Migration(migrations.Migration): + dependencies = [ + ("parkings", "0046_permitarea_allowed_users"), + ] + + operations = [ + migrations.RunPython( + populate_permitted_user_from_allowed_users, + populate_permitted_user_from_allowed_users, + ), + ] diff --git a/parkings/models/permit.py b/parkings/models/permit.py index 4b3cc4b9..9da5e873 100644 --- a/parkings/models/permit.py +++ b/parkings/models/permit.py @@ -19,7 +19,7 @@ class PermitAreaQuerySet(models.QuerySet): def for_user(self, user): - return self.filter(permitted_user=user) + return self.filter(models.Q(permitted_user=user) | models.Q(allowed_users=user)) class PermitArea(TimestampedModelMixin): @@ -31,7 +31,14 @@ class PermitArea(TimestampedModelMixin): geom = gis_models.MultiPolygonField( srid=GK25FIN_SRID, verbose_name=_('geometry')) permitted_user = models.ForeignKey( - settings.AUTH_USER_MODEL, on_delete=models.PROTECT, verbose_name=_("permitted_user")) + settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, + verbose_name=_("permitted_user")) + allowed_users = models.ManyToManyField( + settings.AUTH_USER_MODEL, + verbose_name=_("allowed users"), + related_name="allowed_permit_areas", + help_text=_("Users who are allowed to create permits to this area."), + ) objects = PermitAreaQuerySet.as_manager() From 0c3c668dd76c8429a0674476a31415279a524da5 Mon Sep 17 00:00:00 2001 From: Tuomas Suutari Date: Thu, 23 Nov 2023 20:16:29 +0200 Subject: [PATCH 3/6] Remove PermitArea.permitted_user Remove the "permitted_user" field from the PermitArea model and update all code to use the "allowed_users" instead, because the field is now redundant. Refs HPH-36 --- .sanitizerconfig | 1 - parkings/factories/permit.py | 5 +++-- parkings/importers/geojson_permit_areas.py | 2 +- .../0048_remove_permitarea_permitted_user.py | 16 ++++++++++++++++ parkings/models/permit.py | 5 +---- .../tests/api/enforcement/test_check_parking.py | 5 +++-- parkings/tests/api/operator/permit_area.py | 12 ++++++------ parkings/tests/api/operator/test_permit.py | 7 +++++-- parkkihubi_hel/importers/permit_areas.py | 3 ++- 9 files changed, 37 insertions(+), 19 deletions(-) create mode 100644 parkings/migrations/0048_remove_permitarea_permitted_user.py diff --git a/.sanitizerconfig b/.sanitizerconfig index c150c7a1..5365e06f 100644 --- a/.sanitizerconfig +++ b/.sanitizerconfig @@ -180,7 +180,6 @@ strategy: identifier: null modified_at: null name: null - permitted_user_id: null parkings_permitarea_allowed_users: id: null permitarea_id: null diff --git a/parkings/factories/permit.py b/parkings/factories/permit.py index 1e232db8..9c375ce7 100644 --- a/parkings/factories/permit.py +++ b/parkings/factories/permit.py @@ -44,15 +44,16 @@ def create_permit_area(identifier, domain=None, permitted_user=None): )[0] if domain is None: domain = EnforcementDomain.get_default_domain() - PermitArea.objects.get_or_create( + (area, created) = PermitArea.objects.get_or_create( identifier=identifier, domain=domain, defaults={ 'name': "Kamppi", 'geom': geom, - 'permitted_user': permitted_user, } ) + if created: + area.allowed_users.add(permitted_user) def generate_areas(domain=None, count=1, permitted_user=None): diff --git a/parkings/importers/geojson_permit_areas.py b/parkings/importers/geojson_permit_areas.py index bc7bdf4a..f0002618 100644 --- a/parkings/importers/geojson_permit_areas.py +++ b/parkings/importers/geojson_permit_areas.py @@ -30,11 +30,11 @@ def _save_permit_areas(self, permit_areas_dict, permitted_user): permit_area_ids = [] for area_dict in permit_areas_dict: domain = area_dict.pop('domain', default_domain) - area_dict.setdefault('permitted_user', user) permit_area, _ = PermitArea.objects.update_or_create( identifier=area_dict['identifier'], domain=domain, defaults=area_dict) + permit_area.allowed_users.add(user) permit_area_ids.append(permit_area.pk) count += 1 return count diff --git a/parkings/migrations/0048_remove_permitarea_permitted_user.py b/parkings/migrations/0048_remove_permitarea_permitted_user.py new file mode 100644 index 00000000..755b267a --- /dev/null +++ b/parkings/migrations/0048_remove_permitarea_permitted_user.py @@ -0,0 +1,16 @@ +# Generated by Django 2.2.15 on 2023-11-23 18:14 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("parkings", "0047_populate_permitarea_allowed_users"), + ] + + operations = [ + migrations.RemoveField( + model_name="permitarea", + name="permitted_user", + ), + ] diff --git a/parkings/models/permit.py b/parkings/models/permit.py index 9da5e873..554377b5 100644 --- a/parkings/models/permit.py +++ b/parkings/models/permit.py @@ -19,7 +19,7 @@ class PermitAreaQuerySet(models.QuerySet): def for_user(self, user): - return self.filter(models.Q(permitted_user=user) | models.Q(allowed_users=user)) + return self.filter(allowed_users=user) class PermitArea(TimestampedModelMixin): @@ -30,9 +30,6 @@ class PermitArea(TimestampedModelMixin): identifier = models.CharField(max_length=10, verbose_name=_('identifier')) geom = gis_models.MultiPolygonField( srid=GK25FIN_SRID, verbose_name=_('geometry')) - permitted_user = models.ForeignKey( - settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, - verbose_name=_("permitted_user")) allowed_users = models.ManyToManyField( settings.AUTH_USER_MODEL, verbose_name=_("allowed users"), diff --git a/parkings/tests/api/enforcement/test_check_parking.py b/parkings/tests/api/enforcement/test_check_parking.py index d032da83..8da7b23d 100644 --- a/parkings/tests/api/enforcement/test_check_parking.py +++ b/parkings/tests/api/enforcement/test_check_parking.py @@ -49,12 +49,13 @@ def create_permit_area(client=None, domain=None, permitted_user=None): assert client or (domain and permitted_user) - return PermitArea.objects.create( + area = PermitArea.objects.create( domain=(domain or client.enforcer.enforced_domain), identifier="A", name="Kamppi", - permitted_user=(permitted_user or client.auth_user), geom=create_area_geom()) + area.allowed_users.add(permitted_user or client.auth_user) + return area def create_area_geom(geom=GEOM_1): diff --git a/parkings/tests/api/operator/permit_area.py b/parkings/tests/api/operator/permit_area.py index ec56bb99..5875af54 100644 --- a/parkings/tests/api/operator/permit_area.py +++ b/parkings/tests/api/operator/permit_area.py @@ -10,12 +10,12 @@ def test_endpoint_returns_list_of_permitareas(operator_api_client, operator): domain = EnforcementDomain.objects.create(code='ESP', name='Espoo') - PermitArea.objects.create( + area = PermitArea.objects.create( name='AreaOne', geom=create_area_geom(), identifier='A', - permitted_user=operator.user, domain=domain ) + area.allowed_users.add(operator.user) response = operator_api_client.get(list_url) json_response = response.json() @@ -33,18 +33,18 @@ def test_endpoint_returns_only_the_list_of_permitted_permitareas( operator_api_client, operator, staff_user ): domain = EnforcementDomain.objects.create(code='ESP', name='Espoo') - PermitArea.objects.create( + area_a = PermitArea.objects.create( name='AreaOne', geom=create_area_geom(), identifier='A', - permitted_user=operator.user, domain=domain ) - PermitArea.objects.create( + area_a.allowed_users.add(operator.user) + area_b = PermitArea.objects.create( name='AreaTwo', geom=create_area_geom(), identifier='B', - permitted_user=staff_user, domain=domain ) + area_b.allowed_users.add(staff_user) response = operator_api_client.get(list_url) json_response = response.json() diff --git a/parkings/tests/api/operator/test_permit.py b/parkings/tests/api/operator/test_permit.py index 44778fe6..aed95d0d 100644 --- a/parkings/tests/api/operator/test_permit.py +++ b/parkings/tests/api/operator/test_permit.py @@ -208,13 +208,16 @@ def test_area_restriction( 'properties': {'permit_type': 'Asukaspysäköintitunnus'}, } - PermitArea.objects.create( + permit_area = PermitArea.objects.create( name='Kamppi', identifier='AR3A', geom=create_area_geom(), - permitted_user=(operator.user if allowed == 'allowed' else staff_user), domain=domain, ) + if allowed == "allowed": + permit_area.allowed_users.add(operator.user) + else: + permit_area.allowed_users.add(staff_user) data = permit_data if mode == 'single' else [permit_data] diff --git a/parkkihubi_hel/importers/permit_areas.py b/parkkihubi_hel/importers/permit_areas.py index 3e950185..5fd5bd3b 100644 --- a/parkkihubi_hel/importers/permit_areas.py +++ b/parkkihubi_hel/importers/permit_areas.py @@ -26,12 +26,13 @@ def _save_permit_areas(self, permit_areas_dict, permitted_user): logger.info('Saving permit areas.') count = 0 permit_area_ids = [] + user = get_user_model().objects.filter(username=permitted_user).get() for area_dict in permit_areas_dict: permit_area, _ = PermitArea.objects.update_or_create( domain=EnforcementDomain.get_default_domain(), identifier=area_dict['identifier'], - permitted_user=get_user_model().objects.filter(username=permitted_user).get(), defaults=area_dict) + permit_area.allowed_users.add(user) permit_area_ids.append(permit_area.pk) count += 1 PermitArea.objects.exclude(pk__in=permit_area_ids).delete() From 82bf553af93e074811fc69f01d355b5f03be310c Mon Sep 17 00:00:00 2001 From: Tuomas Suutari Date: Fri, 24 Nov 2023 07:11:44 +0200 Subject: [PATCH 4/6] Rename "permitted_user" to "allowed_user" Rename all occurences of "permitted_user" to "allowed_user" so that code matches the new naming of the field. The tests are still utilizing just a single allowed user per area, but that should be fine for now. Refs HPH-36 --- parkings/factories/permit.py | 14 +++++++------- parkings/importers/geojson_permit_areas.py | 8 ++++---- .../commands/import_geojson_permit_areas.py | 6 +++--- .../test_active_permit_by_external_id.py | 6 +++--- .../tests/api/enforcement/test_check_parking.py | 8 ++++---- parkings/tests/api/enforcement/test_permit.py | 8 ++++---- .../operator/test_active_permit_by_external_id.py | 2 +- parkings/tests/api/operator/test_permit.py | 2 +- parkkihubi_hel/importers/permit_areas.py | 8 ++++---- .../management/commands/import_permit_areas.py | 7 +++---- 10 files changed, 34 insertions(+), 35 deletions(-) diff --git a/parkings/factories/permit.py b/parkings/factories/permit.py index 9c375ce7..30aaf8f8 100644 --- a/parkings/factories/permit.py +++ b/parkings/factories/permit.py @@ -35,10 +35,10 @@ def generate_subjects(count=1): return subjects -def create_permit_area(identifier, domain=None, permitted_user=None): +def create_permit_area(identifier, domain=None, allowed_user=None): geom = generate_multi_polygon() - if permitted_user is None: - permitted_user = get_user_model().objects.get_or_create( + if allowed_user is None: + allowed_user = get_user_model().objects.get_or_create( username='TEST_STAFF', defaults={'is_staff': True} )[0] @@ -53,10 +53,10 @@ def create_permit_area(identifier, domain=None, permitted_user=None): } ) if created: - area.allowed_users.add(permitted_user) + area.allowed_users.add(allowed_user) -def generate_areas(domain=None, count=1, permitted_user=None): +def generate_areas(domain=None, count=1, allowed_user=None): areas = [] for c in range(count): identifier = fake.random.choice(CAPITAL_LETTERS) @@ -65,7 +65,7 @@ def generate_areas(domain=None, count=1, permitted_user=None): 'end_time': generate_timestamp_string('+1h', '+2h'), 'area': identifier, }) - create_permit_area(identifier, domain, permitted_user) + create_permit_area(identifier, domain, allowed_user) return areas @@ -113,6 +113,6 @@ def create_permit( areas=generate_areas( domain=domain, count=area_count, - permitted_user=owner, + allowed_user=owner, ), )[0] diff --git a/parkings/importers/geojson_permit_areas.py b/parkings/importers/geojson_permit_areas.py index f0002618..05218a4f 100644 --- a/parkings/importers/geojson_permit_areas.py +++ b/parkings/importers/geojson_permit_areas.py @@ -16,15 +16,15 @@ class PermitAreaImporter(GeoJsonImporter): Imports permit area data """ - def import_permit_areas(self, geojson_file_path, permitted_user): + def import_permit_areas(self, geojson_file_path, allowed_user): permit_area_dicts = self.read_and_parse(geojson_file_path) - count = self._save_permit_areas(permit_area_dicts, permitted_user) + count = self._save_permit_areas(permit_area_dicts, allowed_user) logger.info('Created or updated {} permit areas'.format(count)) @transaction.atomic - def _save_permit_areas(self, permit_areas_dict, permitted_user): + def _save_permit_areas(self, permit_areas_dict, allowed_user): logger.info('Saving permit areas.') - user = get_user_model().objects.filter(username=permitted_user).get() + user = get_user_model().objects.filter(username=allowed_user).get() default_domain = self.get_default_domain() count = 0 permit_area_ids = [] diff --git a/parkings/management/commands/import_geojson_permit_areas.py b/parkings/management/commands/import_geojson_permit_areas.py index bfde75d3..41d5a065 100644 --- a/parkings/management/commands/import_geojson_permit_areas.py +++ b/parkings/management/commands/import_geojson_permit_areas.py @@ -8,11 +8,11 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument('geojson_file_path') - parser.add_argument('permitted_user') + parser.add_argument('allowed_user') parser.add_argument('--srid', '-s', type=int, default=None) parser.add_argument('--domain', '-d', type=str, default=None) - def handle(self, *, geojson_file_path, permitted_user, + def handle(self, *, geojson_file_path, allowed_user, srid=None, domain=None, **kwargs): importer = PermitAreaImporter(srid=srid, default_domain_code=domain) - importer.import_permit_areas(geojson_file_path, permitted_user) + importer.import_permit_areas(geojson_file_path, allowed_user) diff --git a/parkings/tests/api/enforcement/test_active_permit_by_external_id.py b/parkings/tests/api/enforcement/test_active_permit_by_external_id.py index eb47291d..c16e8dd9 100644 --- a/parkings/tests/api/enforcement/test_active_permit_by_external_id.py +++ b/parkings/tests/api/enforcement/test_active_permit_by_external_id.py @@ -25,8 +25,8 @@ def create_active_permit(client): def generate_areas_data(client): user = client.auth_user domain = client.enforcer.enforced_domain - areas = generate_areas(domain=domain, permitted_user=user, count=2) - create_permit_area('X1', domain=domain, permitted_user=user) + areas = generate_areas(domain=domain, allowed_user=user, count=2) + create_permit_area('X1', domain=domain, allowed_user=user) areas[0]['area'] = 'X1' return areas @@ -38,7 +38,7 @@ def test_post_active_permit_by_external_id(enforcer_api_client, enforcer): 'external_id': generate_external_ids(), 'subjects': generate_subjects(), 'areas': generate_areas( - domain=enforcer.enforced_domain, permitted_user=enforcer.user), + domain=enforcer.enforced_domain, allowed_user=enforcer.user), } response = enforcer_api_client.post(list_url, data=data) diff --git a/parkings/tests/api/enforcement/test_check_parking.py b/parkings/tests/api/enforcement/test_check_parking.py index 8da7b23d..0dbb97f2 100644 --- a/parkings/tests/api/enforcement/test_check_parking.py +++ b/parkings/tests/api/enforcement/test_check_parking.py @@ -47,14 +47,14 @@ ] -def create_permit_area(client=None, domain=None, permitted_user=None): - assert client or (domain and permitted_user) +def create_permit_area(client=None, domain=None, allowed_user=None): + assert client or (domain and allowed_user) area = PermitArea.objects.create( domain=(domain or client.enforcer.enforced_domain), identifier="A", name="Kamppi", geom=create_area_geom()) - area.allowed_users.add(permitted_user or client.auth_user) + area.allowed_users.add(allowed_user or client.auth_user) return area @@ -327,7 +327,7 @@ def test_enforcer_can_view_only_own_permit(enforcer_api_client, staff_user, enfo EnforcerFactory(user=staff_user) domain = staff_user.enforcer.enforced_domain - create_permit_area(domain=domain, permitted_user=staff_user) + create_permit_area(domain=domain, allowed_user=staff_user) create_permit(domain=domain) response = enforcer_api_client.post(list_url, data=PARKING_DATA) diff --git a/parkings/tests/api/enforcement/test_permit.py b/parkings/tests/api/enforcement/test_permit.py index 4c30fa2d..c9bc2a01 100644 --- a/parkings/tests/api/enforcement/test_permit.py +++ b/parkings/tests/api/enforcement/test_permit.py @@ -15,7 +15,7 @@ def generate_areas_data(client): return generate_areas( domain=client.enforcer.enforced_domain, - permitted_user=client.auth_user) + allowed_user=client.auth_user) @pytest.mark.parametrize( @@ -190,7 +190,7 @@ def test_permit_creation_normalizes_timestamps( domain = enforcer_api_client.enforcer.enforced_domain user = enforcer_api_client.auth_user area_identifier = 'area name' - create_permit_area(area_identifier, domain=domain, permitted_user=user) + create_permit_area(area_identifier, domain=domain, allowed_user=user) permit_data = { 'series': create_permit_series(owner=enforcer_api_client.auth_user).id, 'external_id': 'E123', @@ -281,13 +281,13 @@ def test_permit_bulk_create_creates_lookup_items(enforcer_api_client): "series": permit_series.id, "external_id": "E1", "subjects": generate_subjects(), - "areas": generate_areas(domain=domain, permitted_user=user), + "areas": generate_areas(domain=domain, allowed_user=user), }, { "series": permit_series.id, "external_id": "E2", "subjects": generate_subjects(), - "areas": generate_areas(domain=domain, permitted_user=user), + "areas": generate_areas(domain=domain, allowed_user=user), }, ] diff --git a/parkings/tests/api/operator/test_active_permit_by_external_id.py b/parkings/tests/api/operator/test_active_permit_by_external_id.py index 180fe104..ee4f330d 100644 --- a/parkings/tests/api/operator/test_active_permit_by_external_id.py +++ b/parkings/tests/api/operator/test_active_permit_by_external_id.py @@ -26,7 +26,7 @@ def test_operator_can_post_active_permit_by_external_id(operator_api_client): data = { 'external_id': generate_external_ids(), 'subjects': generate_subjects(), - 'areas': generate_areas(active_permit.domain, permitted_user=user), + 'areas': generate_areas(active_permit.domain, allowed_user=user), 'domain': active_permit.domain.code, } diff --git a/parkings/tests/api/operator/test_permit.py b/parkings/tests/api/operator/test_permit.py index aed95d0d..6bbbf411 100644 --- a/parkings/tests/api/operator/test_permit.py +++ b/parkings/tests/api/operator/test_permit.py @@ -48,7 +48,7 @@ def test_operator_can_create_permit_with_valid_post_data( 'external_id': generate_external_ids(), 'domain': 'TSTDOM', 'subjects': generate_subjects(), - 'areas': generate_areas(domain, permitted_user=operator.user), + 'areas': generate_areas(domain, allowed_user=operator.user), } response = operator_api_client.post(list_url, data=permit_data) diff --git a/parkkihubi_hel/importers/permit_areas.py b/parkkihubi_hel/importers/permit_areas.py index 5fd5bd3b..96a5f670 100644 --- a/parkkihubi_hel/importers/permit_areas.py +++ b/parkkihubi_hel/importers/permit_areas.py @@ -16,17 +16,17 @@ class PermitAreaImporter(WfsImporter): """ wfs_typename = 'Asukas_ja_yrityspysakointivyohykkeet_alue' - def import_permit_areas(self, permitted_user): + def import_permit_areas(self, allowed_user): permit_area_dicts = self.download_and_parse() - count = self._save_permit_areas(permit_area_dicts, permitted_user) + count = self._save_permit_areas(permit_area_dicts, allowed_user) logger.info('Created or updated {} permit areas'.format(count)) @transaction.atomic - def _save_permit_areas(self, permit_areas_dict, permitted_user): + def _save_permit_areas(self, permit_areas_dict, allowed_user): logger.info('Saving permit areas.') count = 0 permit_area_ids = [] - user = get_user_model().objects.filter(username=permitted_user).get() + user = get_user_model().objects.filter(username=allowed_user).get() for area_dict in permit_areas_dict: permit_area, _ = PermitArea.objects.update_or_create( domain=EnforcementDomain.get_default_domain(), diff --git a/parkkihubi_hel/management/commands/import_permit_areas.py b/parkkihubi_hel/management/commands/import_permit_areas.py index 2c465caa..de7f21b6 100644 --- a/parkkihubi_hel/management/commands/import_permit_areas.py +++ b/parkkihubi_hel/management/commands/import_permit_areas.py @@ -7,8 +7,7 @@ class Command(BaseCommand): help = 'Uses the PermitAreaImporter to create permit areas' def add_arguments(self, parser): - parser.add_argument('permitted-user') + parser.add_argument('allowed_user') - def handle(self, *args, **options): - permitted_user = options.get('permitted-user', None) - PermitAreaImporter().import_permit_areas(permitted_user) + def handle(self, *args, allowed_user=None, **options): + PermitAreaImporter().import_permit_areas(allowed_user) From 32b9faff79df5bcec4d330816cd2c9111cf174a5 Mon Sep 17 00:00:00 2001 From: Tuomas Suutari Date: Fri, 24 Nov 2023 07:33:28 +0200 Subject: [PATCH 5/6] Management commands: Make "allowed_user" optional Make the "allowed_user" parameter optional, since now it's possible to create the PermitAreas without any allowed users and it could make sense to import new areas without assigning them to any users at import time. Refs HPH-36 --- parkings/importers/geojson_permit_areas.py | 12 ++++++++---- .../commands/import_geojson_permit_areas.py | 4 ++-- parkkihubi_hel/importers/permit_areas.py | 12 ++++++++---- .../management/commands/import_permit_areas.py | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/parkings/importers/geojson_permit_areas.py b/parkings/importers/geojson_permit_areas.py index 05218a4f..4bfb2294 100644 --- a/parkings/importers/geojson_permit_areas.py +++ b/parkings/importers/geojson_permit_areas.py @@ -16,15 +16,18 @@ class PermitAreaImporter(GeoJsonImporter): Imports permit area data """ - def import_permit_areas(self, geojson_file_path, allowed_user): + def import_permit_areas(self, geojson_file_path, allowed_user=None): permit_area_dicts = self.read_and_parse(geojson_file_path) count = self._save_permit_areas(permit_area_dicts, allowed_user) logger.info('Created or updated {} permit areas'.format(count)) @transaction.atomic - def _save_permit_areas(self, permit_areas_dict, allowed_user): + def _save_permit_areas(self, permit_areas_dict, allowed_user=None): logger.info('Saving permit areas.') - user = get_user_model().objects.filter(username=allowed_user).get() + if allowed_user is not None: + user = get_user_model().objects.filter(username=allowed_user).get() + else: + user = None default_domain = self.get_default_domain() count = 0 permit_area_ids = [] @@ -34,7 +37,8 @@ def _save_permit_areas(self, permit_areas_dict, allowed_user): identifier=area_dict['identifier'], domain=domain, defaults=area_dict) - permit_area.allowed_users.add(user) + if user is not None: + permit_area.allowed_users.add(user) permit_area_ids.append(permit_area.pk) count += 1 return count diff --git a/parkings/management/commands/import_geojson_permit_areas.py b/parkings/management/commands/import_geojson_permit_areas.py index 41d5a065..d7fbc971 100644 --- a/parkings/management/commands/import_geojson_permit_areas.py +++ b/parkings/management/commands/import_geojson_permit_areas.py @@ -8,11 +8,11 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument('geojson_file_path') - parser.add_argument('allowed_user') + parser.add_argument('allowed_user', nargs='?', type=str, default=None) parser.add_argument('--srid', '-s', type=int, default=None) parser.add_argument('--domain', '-d', type=str, default=None) - def handle(self, *, geojson_file_path, allowed_user, + def handle(self, *, geojson_file_path, allowed_user=None, srid=None, domain=None, **kwargs): importer = PermitAreaImporter(srid=srid, default_domain_code=domain) importer.import_permit_areas(geojson_file_path, allowed_user) diff --git a/parkkihubi_hel/importers/permit_areas.py b/parkkihubi_hel/importers/permit_areas.py index 96a5f670..b3dd7297 100644 --- a/parkkihubi_hel/importers/permit_areas.py +++ b/parkkihubi_hel/importers/permit_areas.py @@ -16,23 +16,27 @@ class PermitAreaImporter(WfsImporter): """ wfs_typename = 'Asukas_ja_yrityspysakointivyohykkeet_alue' - def import_permit_areas(self, allowed_user): + def import_permit_areas(self, allowed_user=None): permit_area_dicts = self.download_and_parse() count = self._save_permit_areas(permit_area_dicts, allowed_user) logger.info('Created or updated {} permit areas'.format(count)) @transaction.atomic - def _save_permit_areas(self, permit_areas_dict, allowed_user): + def _save_permit_areas(self, permit_areas_dict, allowed_user=None): logger.info('Saving permit areas.') count = 0 permit_area_ids = [] - user = get_user_model().objects.filter(username=allowed_user).get() + if allowed_user is not None: + user = get_user_model().objects.filter(username=allowed_user).get() + else: + user = None for area_dict in permit_areas_dict: permit_area, _ = PermitArea.objects.update_or_create( domain=EnforcementDomain.get_default_domain(), identifier=area_dict['identifier'], defaults=area_dict) - permit_area.allowed_users.add(user) + if user is not None: + permit_area.allowed_users.add(user) permit_area_ids.append(permit_area.pk) count += 1 PermitArea.objects.exclude(pk__in=permit_area_ids).delete() diff --git a/parkkihubi_hel/management/commands/import_permit_areas.py b/parkkihubi_hel/management/commands/import_permit_areas.py index de7f21b6..1cac3d41 100644 --- a/parkkihubi_hel/management/commands/import_permit_areas.py +++ b/parkkihubi_hel/management/commands/import_permit_areas.py @@ -7,7 +7,7 @@ class Command(BaseCommand): help = 'Uses the PermitAreaImporter to create permit areas' def add_arguments(self, parser): - parser.add_argument('allowed_user') + parser.add_argument('allowed_user', nargs='?', type=str, default=None) def handle(self, *args, allowed_user=None, **options): PermitAreaImporter().import_permit_areas(allowed_user) From 7531956b45cd8582a1b388b99cea229962d52a4a Mon Sep 17 00:00:00 2001 From: Tuomas Suutari Date: Fri, 24 Nov 2023 08:44:39 +0200 Subject: [PATCH 6/6] Operator API: Test creation with more allowed_users Add a few test cases that make sure that the creation of permits via the Operator API works correctly even when the PermitArea has more than one allowed user. Refs HPH-36 --- parkings/tests/api/operator/test_permit.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/parkings/tests/api/operator/test_permit.py b/parkings/tests/api/operator/test_permit.py index 6bbbf411..4d32065d 100644 --- a/parkings/tests/api/operator/test_permit.py +++ b/parkings/tests/api/operator/test_permit.py @@ -186,9 +186,16 @@ def test_operator_and_enforcers_cannot_see_each_others_permit( @pytest.mark.parametrize('allowed', ['allowed', 'denied']) +@pytest.mark.parametrize('number_of_allowed_users', [1, 2]) @pytest.mark.parametrize('mode', ['single', 'bulk']) def test_area_restriction( - allowed, mode, operator_api_client, operator, staff_user + allowed, + number_of_allowed_users, + mode, + operator_api_client, + operator, + staff_user, + enforcer, ): area = { 'start_time': '2020-01-01T00:00:00+00:00', @@ -214,6 +221,8 @@ def test_area_restriction( geom=create_area_geom(), domain=domain, ) + if number_of_allowed_users == 2: + permit_area.allowed_users.add(enforcer.user) if allowed == "allowed": permit_area.allowed_users.add(operator.user) else: