Skip to content

Commit

Permalink
Multiple allowed users for PermitArea (HPH-36) (#276)
Browse files Browse the repository at this point in the history
Add support for allowing sharing of the PermitAreas between operators
and the enforcer.

Previously it was possible to use each PermitArea for just a single
enforcer or operator.
This was especially problematic, because the PermitArea codes were
unique within
enforcement domains so it was not even possible to create duplicate
areas with the
same identifiers.
  • Loading branch information
suutari-ai authored Nov 24, 2023
2 parents 1cefe0a + 7531956 commit d90797f
Show file tree
Hide file tree
Showing 18 changed files with 172 additions and 52 deletions.
5 changes: 4 additions & 1 deletion .sanitizerconfig
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ strategy:
identifier: null
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
Expand Down
2 changes: 1 addition & 1 deletion parkings/api/common_permit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion parkings/api/operator/permit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
17 changes: 9 additions & 8 deletions parkings/factories/permit.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,28 @@ 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]
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(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)
Expand All @@ -64,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


Expand Down Expand Up @@ -112,6 +113,6 @@ def create_permit(
areas=generate_areas(
domain=domain,
count=area_count,
permitted_user=owner,
allowed_user=owner,
),
)[0]
14 changes: 9 additions & 5 deletions parkings/importers/geojson_permit_areas.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,29 @@ 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=None):
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=None):
logger.info('Saving permit areas.')
user = get_user_model().objects.filter(username=permitted_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 = []
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)
if user is not None:
permit_area.allowed_users.add(user)
permit_area_ids.append(permit_area.pk)
count += 1
return count
6 changes: 3 additions & 3 deletions parkings/management/commands/import_geojson_permit_areas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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', 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, permitted_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, permitted_user)
importer.import_permit_areas(geojson_file_path, allowed_user)
39 changes: 39 additions & 0 deletions parkings/migrations/0046_permitarea_allowed_users.py
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
29 changes: 29 additions & 0 deletions parkings/migrations/0047_populate_permitarea_allowed_users.py
Original file line number Diff line number Diff line change
@@ -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,
),
]
16 changes: 16 additions & 0 deletions parkings/migrations/0048_remove_permitarea_permitted_user.py
Original file line number Diff line number Diff line change
@@ -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",
),
]
15 changes: 13 additions & 2 deletions parkings/models/permit.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
from .parking import Parking


class PermitAreaQuerySet(models.QuerySet):
def for_user(self, user):
return self.filter(allowed_users=user)


class PermitArea(TimestampedModelMixin):
name = models.CharField(max_length=40, verbose_name=_('name'))
domain = models.ForeignKey(
Expand All @@ -25,8 +30,14 @@ 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.PROTECT, 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()

class Meta:
unique_together = [('domain', 'identifier')]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down
11 changes: 6 additions & 5 deletions parkings/tests/api/enforcement/test_check_parking.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,15 @@
]


def create_permit_area(client=None, domain=None, permitted_user=None):
assert client or (domain and permitted_user)
return PermitArea.objects.create(
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",
permitted_user=(permitted_user or client.auth_user),
geom=create_area_geom())
area.allowed_users.add(allowed_user or client.auth_user)
return area


def create_area_geom(geom=GEOM_1):
Expand Down Expand Up @@ -326,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)
Expand Down
8 changes: 4 additions & 4 deletions parkings/tests/api/enforcement/test_permit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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),
},
]

Expand Down
12 changes: 6 additions & 6 deletions parkings/tests/api/operator/permit_area.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
Loading

0 comments on commit d90797f

Please sign in to comment.