Skip to content

Commit

Permalink
Permissions rework - updates (#4649)
Browse files Browse the repository at this point in the history
* change migration to rename model instead of creating new one

* fix business_areas query on user

* fix unicef users migration for cases with roles in global
  • Loading branch information
pkujawa authored Feb 25, 2025
1 parent 870ec3e commit 1e55191
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 46 deletions.
75 changes: 52 additions & 23 deletions src/hct_mis_api/apps/account/migrations/0008_migration.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 3.2.25 on 2025-01-20 12:52
# Generated by Django 3.2.25 on 2025-02-20 13:40

from django.conf import settings
from django.db import migrations, models
Expand All @@ -10,36 +10,24 @@
class Migration(migrations.Migration):

dependencies = [
('program', '0002_migration'),
('core', '0005_migration'),
('auth', '0012_alter_user_first_name_max_length'),
('geo', '0002_migration'),
('program', '0003_migration'),
('auth', '0012_alter_user_first_name_max_length'),
('core', '0007_migration'),
('account', '0007_migration'),
]

operations = [
migrations.CreateModel(
name='AdminAreaLimitedTo',
fields=[
('id', model_utils.fields.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
('created_at', models.DateTimeField(auto_now_add=True, db_index=True)),
('updated_at', models.DateTimeField(auto_now=True, db_index=True)),
('areas', models.ManyToManyField(blank=True, related_name='admin_area_limits', to='geo.Area')),
('partner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='admin_area_limits', to='account.partner')),
('program', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='admin_area_limits', to='program.program')),
],
migrations.RenameModel(
old_name="UserRole",
new_name="RoleAssignment",
),
migrations.CreateModel(
name='RoleAssignment',
name='AdminAreaLimitedTo',
fields=[
('id', model_utils.fields.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
('created_at', models.DateTimeField(auto_now_add=True, db_index=True)),
('updated_at', models.DateTimeField(auto_now=True, db_index=True)),
('expiry_date', models.DateField(blank=True, help_text='After expiry date this Role Assignment will be inactive.', null=True)),
('business_area', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='core.businessarea')),
('group', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='auth.group')),
('partner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='account.partner')),
('program', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='program.program')),
],
),
migrations.AlterModelOptions(
Expand All @@ -56,19 +44,45 @@ class Migration(migrations.Migration):
name='is_visible_on_ui',
field=models.BooleanField(default=True),
),
migrations.DeleteModel(
name='UserRole',
migrations.AddField(
model_name='roleassignment',
name='group',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='auth.group'),
),
migrations.AddField(
model_name='roleassignment',
name='partner',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='account.partner'),
),
migrations.AddField(
model_name='roleassignment',
name='program',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='program.program'),
),
migrations.AlterField(
model_name='roleassignment',
name='business_area',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='core.businessarea'),
),
migrations.AlterField(
model_name='roleassignment',
name='expiry_date',
field=models.DateField(blank=True, help_text='After expiry date this Role Assignment will be inactive.', null=True),
),
migrations.AlterField(
model_name='roleassignment',
name='role',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to='account.role'),
),
migrations.AddField(
migrations.AlterField(
model_name='roleassignment',
name='user',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='role_assignments', to=settings.AUTH_USER_MODEL),
),
migrations.AlterUniqueTogether(
name='roleassignment',
unique_together=set(),
),
migrations.AddConstraint(
model_name='roleassignment',
constraint=models.CheckConstraint(check=models.Q(models.Q(('partner__isnull', True), ('user__isnull', False)), models.Q(('partner__isnull', False), ('user__isnull', True)), _connector='OR'), name='user_or_partner_not_both'),
Expand All @@ -89,6 +103,21 @@ class Migration(migrations.Migration):
model_name='roleassignment',
constraint=models.UniqueConstraint(condition=models.Q(('partner__isnull', False), ('program__isnull', True)), fields=('partner', 'role', 'business_area'), name='unique_partner_role_business_area_no_program'),
),
migrations.AddField(
model_name='adminarealimitedto',
name='areas',
field=models.ManyToManyField(blank=True, related_name='admin_area_limits', to='geo.Area'),
),
migrations.AddField(
model_name='adminarealimitedto',
name='partner',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='admin_area_limits', to='account.partner'),
),
migrations.AddField(
model_name='adminarealimitedto',
name='program',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='admin_area_limits', to='program.program'),
),
migrations.AlterUniqueTogether(
name='adminarealimitedto',
unique_together={('partner', 'program')},
Expand Down
25 changes: 12 additions & 13 deletions src/hct_mis_api/apps/account/migrations/0009_migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def migrate_unicef_partners(apps, schema_editor):
unicef_hq, _ = Partner.objects.get_or_create(name=settings.UNICEF_HQ_PARTNER)
unicef_hq.allowed_business_areas.set(BusinessArea.objects.all())

for business_area in BusinessArea.objects.all():
for business_area in BusinessArea.objects.exclude(slug="global"):
unicef_subpartner , _ = Partner.objects.get_or_create(
name=f"UNICEF Partner for {business_area.slug}"
)
Expand Down Expand Up @@ -294,35 +294,34 @@ def migrate_unicef_partners(apps, schema_editor):

# handle UNICEF users

# UNICEF users with roles in multiple Business Areas will be assigned to UNICEF HQ
# UNICEF users with no roles or role only in GLOBAL will be assigned to default empty partner
empty_partner, _ = Partner.objects.get_or_create(name=settings.DEFAULT_EMPTY_PARTNER)
User.objects.filter(partner=unicef_partner).annotate(
ba_count=Count("role_assignments__business_area", distinct=True)
).filter(ba_count__gt=1).update(partner=unicef_hq)
).filter(Q(ba_count=0) | Q(ba_count=1, role_assignments__business_area__slug="global")).update(partner=empty_partner)

# UNICEF users with roles in single Business Area will be assigned to UNICEF Sub-partner for that Business Area
unicef_users_in_single_ba = (
User.objects.filter(partner=unicef_partner)
.annotate(ba_count=Count("role_assignments__business_area", distinct=True))
.filter(ba_count=1)
.filter(Q(ba_count=1) | (Q(role_assignments__business_area__slug="global") & Q(ba_count=2)))
)

unicef_subpartners = {
ba.slug: Partner.objects.get(name=f"UNICEF Partner for {ba.slug}") for ba in BusinessArea.objects.all()
ba.slug: Partner.objects.get(name=f"UNICEF Partner for {ba.slug}") for ba in BusinessArea.objects.exclude(slug="global")
}

for ba in unicef_users_in_single_ba.values_list("role_assignments__business_area__slug", flat=True).distinct():
for ba in RoleAssignment.objects.filter(user__in=unicef_users_in_single_ba).exclude(business_area__slug="global").values_list("business_area__slug", flat=True).distinct():
unicef_users = unicef_users_in_single_ba.filter(role_assignments__business_area__slug=ba)
unicef_users.update(partner=unicef_subpartners[ba])

# UNICEF users with no roles will be assigned to default empty partner
empty_partner, _ = Partner.objects.get_or_create(name=settings.DEFAULT_EMPTY_PARTNER)
User.objects.filter(partner=unicef_partner).annotate(
ba_count=Count("role_assignments__business_area", distinct=True)
).filter(ba_count=0).update(partner=empty_partner)

unicef_subpartners_ids = [unicef_subpartner.id for unicef_subpartner in unicef_subpartners.values()]
Partner.objects.filter(id__in=[unicef_hq.id, *unicef_subpartners_ids]).update(parent=unicef_partner)

# UNICEF users with roles in multiple Business Areas will be assigned to UNICEF HQ
User.objects.filter(partner=unicef_partner).annotate(
ba_count=Count("role_assignments__business_area", distinct=True)
).filter(ba_count__gt=1).update(partner=unicef_hq)


class Migration(migrations.Migration):

Expand Down
8 changes: 3 additions & 5 deletions src/hct_mis_api/apps/account/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,10 @@ def permissions_in_business_area(self, business_area_slug: str, program_id: Unio

@property
def business_areas(self) -> QuerySet[BusinessArea]:
return (
BusinessArea.objects.filter(Q(role_assignments__user=self) | Q(role_assignments__partner__user=self))
.exclude(role_assignments__expiry_date__lt=timezone.now())
.exclude(active=False)
.distinct()
role_assignments = RoleAssignment.objects.filter(Q(user=self) | Q(partner__user=self)).exclude(
expiry_date__lt=timezone.now()
)
return BusinessArea.objects.filter(role_assignments__in=role_assignments).exclude(active=False).distinct()

@test_conditional(lru_cache())
def cached_role_assignments(self) -> QuerySet["RoleAssignment"]:
Expand Down
74 changes: 69 additions & 5 deletions tests/unit/apps/account/test_migration_of_roles_and_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ def setUpTestData(cls) -> None:

cls.business_area_afg = BusinessAreaFactory(name="Afghanistan", slug="afghanistan", code="AFG", active=True)
cls.business_area_ukr = BusinessAreaFactory(name="Ukraine", slug="ukraine", code="UKR", active=True)
cls.business_area_syria = BusinessAreaFactory(name="Syria", slug="syria", code="SYR", active=True)
cls.business_area_global = BusinessAreaFactory(name="Global", slug="global", code="GLOBAL", active=True)

# remove partners that were created in signal for creating BAs - to keep the data as it was before changes
Partner.objects.filter(parent__name="UNICEF").delete()
Expand All @@ -65,7 +67,11 @@ def setUpTestData(cls) -> None:
cls.user_unicef_in_afg = UserFactory(partner=cls.partner_unicef)
cls.user_unicef_in_ukr = UserFactory(partner=cls.partner_unicef)
cls.unicef_user_hq = UserFactory(partner=cls.partner_unicef)
# unicef_user_without_any_role - no role, partner UNICEF
cls.unicef_user_without_any_role = UserFactory(partner=cls.partner_unicef)
cls.user_unicef_only_global = UserFactory(partner=cls.partner_unicef)
cls.user_unicef_in_syria_and_global = UserFactory(partner=cls.partner_unicef)
cls.unicef_user_hq_with_global = UserFactory(partner=cls.partner_unicef)

# users under custom partners
cls.partner_1 = PartnerFactory(name="Partner 1")
Expand Down Expand Up @@ -111,7 +117,39 @@ def setUpTestData(cls) -> None:
user=cls.unicef_user_hq,
role=cls.role_2,
)
# unicef_user_without_any_role - no role, partner UNICEF
# user_unicef_only_global - role in Global, partner UNICEF
RoleAssignment.objects.create(
business_area=cls.business_area_global,
user=cls.user_unicef_only_global,
role=cls.role_1,
)
# user_unicef_in_syria_and_global - role in Syria and Global, partner UNICEF
RoleAssignment.objects.create(
business_area=cls.business_area_syria,
user=cls.user_unicef_in_syria_and_global,
role=cls.role_1,
)
RoleAssignment.objects.create(
business_area=cls.business_area_global,
user=cls.user_unicef_in_syria_and_global,
role=cls.role_1,
)
# unicef_user_hq_with_global - roles in Afghanistan, Ukraine and Global, partner UNICEF
RoleAssignment.objects.create(
business_area=cls.business_area_afg,
user=cls.unicef_user_hq_with_global,
role=cls.role_1,
)
RoleAssignment.objects.create(
business_area=cls.business_area_ukr,
user=cls.unicef_user_hq_with_global,
role=cls.role_2,
)
RoleAssignment.objects.create(
business_area=cls.business_area_global,
user=cls.unicef_user_hq_with_global,
role=cls.role_1,
)

# UNICEF has access to all programs
ProgramPartnerThrough.objects.create(
Expand Down Expand Up @@ -469,7 +507,7 @@ def test_partner_roles_migration(self) -> None:
self.assertEqual(self.partner_empty.role_assignments.count(), 0)

def test_unicef_partners_migration(self) -> None:
self.assertEqual(self.partner_unicef.user_set.count(), 4)
self.assertEqual(self.partner_unicef.user_set.count(), 7)

# call all 3 functions to check the final result
data_migration.migrate_user_roles(apps, None)
Expand All @@ -480,18 +518,26 @@ def test_unicef_partners_migration(self) -> None:
self.user_unicef_in_ukr.refresh_from_db()
self.unicef_user_hq.refresh_from_db()
self.unicef_user_without_any_role.refresh_from_db()
self.user_unicef_only_global.refresh_from_db()
self.user_unicef_in_syria_and_global.refresh_from_db()
self.unicef_user_hq_with_global.refresh_from_db()

# check UNICEF subpartners creation
self.assertEqual(Partner.objects.filter(parent=self.partner_unicef).count(), 3)
self.assertEqual(Partner.objects.filter(parent=self.partner_unicef).count(), 4)
self.assertEqual(self.partner_unicef.user_set.count(), 0)

unicef_in_afg = Partner.objects.filter(name="UNICEF Partner for afghanistan").first()
unicef_in_ukr = Partner.objects.filter(name="UNICEF Partner for ukraine").first()
unicef_in_syria = Partner.objects.filter(name="UNICEF Partner for syria").first()
unicef_hq = Partner.objects.filter(name="UNICEF HQ").first()
unicef_global = Partner.objects.filter(name="UNICEF Partner for global").first()

self.assertIsNotNone(unicef_in_afg)
self.assertIsNotNone(unicef_in_ukr)
self.assertIsNotNone(unicef_hq)
self.assertIsNotNone(unicef_in_syria)
# unicef_global should be created
self.assertIsNone(unicef_global)

self.assertEqual(
unicef_in_afg.parent,
Expand Down Expand Up @@ -532,6 +578,24 @@ def test_unicef_partners_migration(self) -> None:
self.partner_empty,
)

# user_unicef_only_global - has role only in Global -> should be under Default Empty Partner
self.assertEqual(
self.user_unicef_only_global.partner,
self.partner_empty,
)

# user_unicef_in_syria_and_global - has roles in Afg and Global -> should be under UNICEF Partner for Afg
self.assertEqual(
self.user_unicef_in_syria_and_global.partner,
unicef_in_syria,
)

# unicef_user_hq_with_global - has roles in Afg, Ukr and Global -> should be under UNICEF HQ
self.assertEqual(
self.unicef_user_hq_with_global.partner,
unicef_hq,
)

# UNICEF subpartners per BA should be allowed in specific BA; UNICEF HQ should be allowed in all
self.assertEqual(
unicef_in_afg.allowed_business_areas.count(),
Expand All @@ -553,7 +617,7 @@ def test_unicef_partners_migration(self) -> None:

self.assertEqual(
unicef_hq.allowed_business_areas.count(),
2,
4,
)
self.assertTrue(self.business_area_afg in unicef_hq.allowed_business_areas.all())
self.assertTrue(self.business_area_ukr in unicef_hq.allowed_business_areas.all())
Expand Down Expand Up @@ -585,7 +649,7 @@ def test_unicef_partners_migration(self) -> None:

self.assertEqual(
unicef_hq.role_assignments.count(),
2,
3,
)
self.assertEqual(
unicef_hq.role_assignments.all()[0].role,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import freezegun
import pytest
from flaky import flaky
from rest_framework import status
from rest_framework.reverse import reverse

Expand Down Expand Up @@ -324,6 +325,7 @@ def test_list_periodic_data_update_templates_caching(

assert etag_second_call == etag

@flaky(max_runs=3, min_passes=1)
@pytest.mark.parametrize(
"permissions, partner_permissions, access_to_program, expected_status",
[
Expand Down

0 comments on commit 1e55191

Please sign in to comment.