diff --git a/greg/admin.py b/greg/admin.py index 293f4973132a78dc527575442ff74ddf6ff097c1..809f5bfb29bf7be379a11a2aaade54fa0643e1ec 100644 --- a/greg/admin.py +++ b/greg/admin.py @@ -2,6 +2,7 @@ from django.contrib import admin from reversion.admin import VersionAdmin from greg.models import ( + OuIdentifier, Invitation, InvitationLink, Person, @@ -91,10 +92,21 @@ class ConsentTypeAdmin(VersionAdmin): readonly_fields = ("id", "created", "updated") +class OuIdentifierInline(admin.TabularInline): + model = OuIdentifier + extra = 1 + + +class IdentifierAdmin(VersionAdmin): + list_display = ("id", "name", "source", "value") + search_fields = ("id", "value") + + class OrganizationalUnitAdmin(VersionAdmin): - list_display = ("id", "orgreg_id", "name_en", "parent") + list_display = ("id", "name_en", "parent") readonly_fields = ("id", "created", "updated") - search_fields = ("name_en", "id", "orgreg_id") + search_fields = ("name_en", "id") + inlines = (OuIdentifierInline,) class OrganizationalUnitInline(admin.TabularInline): @@ -132,3 +144,4 @@ admin.site.register(Sponsor, SponsorAdmin) admin.site.register(SponsorOrganizationalUnit, SponsorOrganizationalUnitAdmin) admin.site.register(Invitation, InvitationAdmin) admin.site.register(InvitationLink, InvitationLinkAdmin) +admin.site.register(OuIdentifier, IdentifierAdmin) diff --git a/greg/api/serializers/organizational_unit.py b/greg/api/serializers/organizational_unit.py index 12251bffff79f9119d76e9e1f7ddcc58c2984b68..95258a1070bdb4324e0f5d1c288e03d8365794bf 100644 --- a/greg/api/serializers/organizational_unit.py +++ b/greg/api/serializers/organizational_unit.py @@ -1,9 +1,22 @@ from rest_framework.serializers import ModelSerializer +from greg.api.serializers.ouidentifier import OuIdentifierSerializer from greg.models import OrganizationalUnit class OrganizationalUnitSerializer(ModelSerializer): + identifiers = OuIdentifierSerializer(many=True) + class Meta: model = OrganizationalUnit - fields = "__all__" + fields = [ + "id", + "created", + "updated", + "name_nb", + "name_en", + "active", + "deleted", + "parent", + "identifiers", + ] diff --git a/greg/api/serializers/ouidentifier.py b/greg/api/serializers/ouidentifier.py new file mode 100644 index 0000000000000000000000000000000000000000..c1b9be570cfbc25ff34d9cfb7e3284e9e34a2c88 --- /dev/null +++ b/greg/api/serializers/ouidentifier.py @@ -0,0 +1,9 @@ +from rest_framework.serializers import ModelSerializer + +from greg.models import OuIdentifier + + +class OuIdentifierSerializer(ModelSerializer): + class Meta: + model = OuIdentifier + fields = ["id", "source", "name", "value"] diff --git a/greg/management/commands/import_from_orgreg.py b/greg/management/commands/import_from_orgreg.py index 438accf08ddd1c6fd76031171e90d3f4b5967d4f..732a68206ba4bb8599e3de7c7cc4d8972cff9367 100644 --- a/greg/management/commands/import_from_orgreg.py +++ b/greg/management/commands/import_from_orgreg.py @@ -1,6 +1,9 @@ """ Fetch all OUs from OrgReg and add the complete tree to Greg. +Ignores OrganizationalUnits without identifiers with source and name matching global +variables ORGREG_SOURCE and ORGREG_NAME + Assumes that the header used for authentication is of the type 'X-Gravitee-Api-Key': 'token'. @@ -16,7 +19,7 @@ from django.conf import settings from django.core.management.base import BaseCommand from orgreg_client import OrgUnit -from greg.models import OrganizationalUnit +from greg.models import OrganizationalUnit, OuIdentifier logger = logging.getLogger(__name__) @@ -26,13 +29,25 @@ class Command(BaseCommand): processed: Dict[int, OrganizationalUnit] = {} def _get_or_create_and_set_values( - self, ou, values: Mapping[str, Union[str, int, bool]] + self, ou: OrgUnit, values: Mapping[str, Union[str, int, bool]] ): """Upsert ou with latest values and store in processed dict.""" - - self.processed[ou.ou_id], created = OrganizationalUnit.objects.get_or_create( - orgreg_id=str(ou.ou_id) - ) + try: + identifier = OuIdentifier.objects.get( + name=settings.ORGREG_NAME, source=settings.ORGREG_SOURCE, value=ou.ou_id + ) + except OuIdentifier.DoesNotExist: + self.processed[ou.ou_id] = OrganizationalUnit.objects.create() + identifier = OuIdentifier.objects.create( + name=settings.ORGREG_NAME, + source=settings.ORGREG_SOURCE, + value=ou.ou_id, + orgunit=self.processed[ou.ou_id], + ) + created = True + else: + self.processed[ou.ou_id] = identifier.orgunit + created = False for k, v in values.items(): setattr(self.processed[ou.ou_id], k, v) self.processed[ou.ou_id].save() @@ -100,13 +115,17 @@ class Command(BaseCommand): client = orgreg_client.get_client(**settings.ORGREG_CLIENT) # Fetch already present OUs and those in OrgReg - current_ous = {int(i.orgreg_id): i for i in OrganizationalUnit.objects.all()} + current_ous = { + int(i.value): i.orgunit + for i in OuIdentifier.objects.filter( + source=settings.ORGREG_SOURCE, name=settings.ORGREG_NAME + ) + } logger.info("Fetch OUs from Orgreg...") orgreg_ous = {i.ou_id: i for i in client.get_ou()} - # Set deleted if an OU from Greg no longer exists in OrgReg and inactive if - # valid_to is set to a date before today + # Set deleted if an OU from Greg no longer exists in OrgReg logger.info("Set deleted tag on removed OUs...") for ou_id, ou in current_ous.items(): if ou_id not in orgreg_ous: diff --git a/greg/migrations/0012_ou_identifiers.py b/greg/migrations/0012_ou_identifiers.py new file mode 100644 index 0000000000000000000000000000000000000000..708422fca04e9e81529608d62d7dd352bc15f503 --- /dev/null +++ b/greg/migrations/0012_ou_identifiers.py @@ -0,0 +1,74 @@ +# Generated by Django 3.2.8 on 2021-10-28 11:44 + +import dirtyfields.dirtyfields +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +def move_orgreg_id_to_identifiers(apps, schema_editor): + OuIdentifier = apps.get_model("greg", "OuIdentifier") + OrganizationalUnit = apps.get_model("greg", "OrganizationalUnit") + for ou in OrganizationalUnit.objects.all(): + value = getattr(ou, "orgreg_id", None) + if value: + OuIdentifier.objects.create( + orgunit=ou, + name=settings.ORGREG_NAME, + source=settings.ORGREG_SOURCE, + value=value, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ("greg", "0011_role_remove_id_suffix"), + ] + + operations = [ + migrations.CreateModel( + name="OuIdentifier", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created", models.DateTimeField(auto_now_add=True)), + ("updated", models.DateTimeField(auto_now=True)), + ("name", models.CharField(max_length=256)), + ("source", models.CharField(max_length=256)), + ("value", models.CharField(max_length=256)), + ], + bases=(dirtyfields.dirtyfields.DirtyFieldsMixin, models.Model), + ), + migrations.AddField( + model_name="ouidentifier", + name="orgunit", + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="identifiers", + to="greg.organizationalunit", + ), + ), + migrations.RunPython(move_orgreg_id_to_identifiers), + migrations.RemoveConstraint( + model_name="organizationalunit", + name="unique_orgreg_id", + ), + migrations.RemoveField( + model_name="organizationalunit", + name="orgreg_id", + ), + migrations.AddConstraint( + model_name="ouidentifier", + constraint=models.UniqueConstraint( + fields=("name", "value"), name="unique_identifier" + ), + ), + ] diff --git a/greg/models.py b/greg/models.py index a57558b7d2c8f1e436aeaaba6b0418ffe6f287f2..8948adb47f0195d473717e8eb342eb21d88d2a02 100644 --- a/greg/models.py +++ b/greg/models.py @@ -364,12 +364,32 @@ class Consent(BaseModel): ) +class OuIdentifier(BaseModel): + """Generic identifier""" + + name = models.CharField(max_length=256) + source = models.CharField(max_length=256) + value = models.CharField(max_length=256) + orgunit = models.ForeignKey( + "OrganizationalUnit", on_delete=models.CASCADE, related_name="identifiers" + ) + + class Meta: + constraints = [ + models.UniqueConstraint(name="unique_identifier", fields=["name", "value"]) + ] + + def __repr__(self): + return "{}(id={!r}, name={!r}, value={!r})".format( + self.__class__.__name__, self.pk, self.name, self.value + ) + + class OrganizationalUnit(BaseModel): """ An organizational unit. Units can be organized in a hierarchical manner. """ - orgreg_id = models.CharField(max_length=256) name_nb = models.CharField(max_length=256) name_en = models.CharField(max_length=256) parent = models.ForeignKey("self", on_delete=models.PROTECT, null=True, blank=True) @@ -377,17 +397,12 @@ class OrganizationalUnit(BaseModel): deleted = models.BooleanField(default=False) def __repr__(self): - return "{}(id={!r}, orgreg_id={!r}, name_en={!r}, parent={!r})".format( - self.__class__.__name__, self.pk, self.orgreg_id, self.name_en, self.parent + return "{}(id={!r}, name_en={!r}, parent={!r})".format( + self.__class__.__name__, self.pk, self.name_en, self.parent ) def __str__(self): - return "{} ({})".format(str(self.name_en or self.name_nb), self.orgreg_id) - - class Meta: - constraints = [ - models.UniqueConstraint(name="unique_orgreg_id", fields=["orgreg_id"]) - ] + return "{}".format(str(self.name_en or self.name_nb)) class Sponsor(BaseModel): diff --git a/greg/tests/api/test_person.py b/greg/tests/api/test_person.py index 791bd9372de61a6f170d4fe41335576b62b93189..c616489ab67c6d3d23c487f1e3f41b9e6f464dbc 100644 --- a/greg/tests/api/test_person.py +++ b/greg/tests/api/test_person.py @@ -38,7 +38,7 @@ def role_type_visiting_professor() -> RoleType: @pytest.fixture def unit_human_resources() -> OrganizationalUnit: return OrganizationalUnit.objects.create( - orgreg_id="org_unit_1", name_nb="Personal", name_en="Human Resources" + name_nb="Personal", name_en="Human Resources" ) diff --git a/greg/tests/conftest.py b/greg/tests/conftest.py index 1650c63b2a715b0e0db8bc6385afa8b538cc2141..04dfe76cb87facce8fa4db2e09cd8b09c772c34d 100644 --- a/greg/tests/conftest.py +++ b/greg/tests/conftest.py @@ -10,6 +10,7 @@ import pytest from greg.models import ( Consent, Notification, + OuIdentifier, Person, Sponsor, SponsorOrganizationalUnit, @@ -136,10 +137,18 @@ def role_type_test_guest() -> RoleType: @pytest.fixture def unit_foo() -> OrganizationalUnit: - ou = OrganizationalUnit.objects.create(orgreg_id="12345", name_en="foo_unit") + ou = OrganizationalUnit.objects.create(name_en="foo_unit") return OrganizationalUnit.objects.get(id=ou.id) +@pytest.fixture +def ouidentifier_foo(unit_foo) -> OuIdentifier: + ouid = OuIdentifier.objcets.create( + source="orgreg", name="orgreg", value="12345", orgunit=unit_foo + ) + return OuIdentifier.objects.get(id=ouid.id) + + @pytest.fixture def role_person_foo( person_foo: Person, diff --git a/greg/tests/management/test_import_from_orgreg.py b/greg/tests/management/test_import_from_orgreg.py index 72d867bffc5b32dec03fb4aaedb161f56c12b03e..d7a39917ce00965d33fabc83ba004dd0664a49a3 100644 --- a/greg/tests/management/test_import_from_orgreg.py +++ b/greg/tests/management/test_import_from_orgreg.py @@ -1,16 +1,28 @@ import datetime import pytest +from django.conf import settings from django.core.management import call_command from orgreg_client import OrgUnit, OrgUnitList -from greg.models import OrganizationalUnit +from greg.models import OrganizationalUnit, OuIdentifier @pytest.fixture def old_unit(): - OrganizationalUnit.objects.create(orgreg_id="4", name_nb="a", name_en="b") - return OrganizationalUnit.objects.get(orgreg_id="4") + ou = OrganizationalUnit.objects.create(name_nb="a", name_en="b") + return OrganizationalUnit.objects.get(id=ou.id) + + +@pytest.fixture +def old_identifier(unit_foo): + ouid = OuIdentifier.objects.create( + orgunit=unit_foo, + name=settings.ORGREG_NAME, + source=settings.ORGREG_SOURCE, + value="4", + ) + return OuIdentifier.objects.get(id=ouid.id) @pytest.fixture @@ -44,7 +56,7 @@ def orgreg_response(): @pytest.mark.django_db -def test_command_ou_init(requests_mock, old_unit, orgreg_response): +def test_command_ou_init(requests_mock, old_identifier, orgreg_response): requests_mock.get("https://example.com/fake/ou/", text=orgreg_response.json()) assert OrganizationalUnit.objects.all().count() == 1 call_command("import_from_orgreg") @@ -53,11 +65,20 @@ def test_command_ou_init(requests_mock, old_unit, orgreg_response): assert OrganizationalUnit.objects.all().count() == 4 # Ensure tree is built correctly - assert OrganizationalUnit.objects.get(orgreg_id="3").parent.orgreg_id == "2" - assert OrganizationalUnit.objects.get(orgreg_id="3").parent.parent.orgreg_id == "1" + bottom = OrganizationalUnit.objects.get( + identifiers__source="orgreg", identifiers__value="3" + ) + assert bottom.name_nb == "bar" + assert bottom.parent.identifiers.get().value == "2" + assert bottom.parent.name_en == "baz" + assert bottom.parent.parent.identifiers.get().value == "1" + assert bottom.parent.parent.name_nb == "foo" # Ensure unknown org is marked deleted - assert OrganizationalUnit.objects.get(orgreg_id="4").deleted is True + old_unit = OrganizationalUnit.objects.get( + identifiers__source="orgreg", identifiers__value="4" + ) + assert old_unit.deleted @pytest.mark.django_db diff --git a/greg/tests/models/test_organizational_unit.py b/greg/tests/models/test_organizational_unit.py index cc318d824b87df964f095cc8f3bd43810f8e5802..ce2043065ff5b367bbb1975976d7173130b55385 100644 --- a/greg/tests/models/test_organizational_unit.py +++ b/greg/tests/models/test_organizational_unit.py @@ -5,19 +5,16 @@ from greg.models import OrganizationalUnit @pytest.mark.django_db def test_set_parent(): - parent = OrganizationalUnit.objects.create(orgreg_id="parent") - child = OrganizationalUnit.objects.create(orgreg_id="child", parent=parent) + parent = OrganizationalUnit.objects.create() + child = OrganizationalUnit.objects.create(parent=parent) assert list(OrganizationalUnit.objects.filter(parent__id=parent.id)) == [child] @pytest.mark.django_db def test_org_repr(unit_foo): - assert ( - repr(unit_foo) - == "OrganizationalUnit(id=1, orgreg_id='12345', name_en='foo_unit', parent=None)" - ) + assert repr(unit_foo) == "OrganizationalUnit(id=1, name_en='foo_unit', parent=None)" @pytest.mark.django_db def test_org_str(unit_foo): - assert str(unit_foo) == "foo_unit (12345)" + assert str(unit_foo) == "foo_unit" diff --git a/greg/tests/models/test_person.py b/greg/tests/models/test_person.py index 39be76df70748589eaaaf9752b2b6f84bc537ff1..dbf09316b7d40b08e81109afcbc16cf4ce14d399 100644 --- a/greg/tests/models/test_person.py +++ b/greg/tests/models/test_person.py @@ -94,7 +94,7 @@ def feide_verified(person: Person, feide_id: Identity) -> Person: def test_add_multiple_roles_to_person( person: Person, role_type_foo: RoleType, role_type_bar: RoleType ): - ou = OrganizationalUnit.objects.create(orgreg_id="12345", name_en="Test unit") + ou = OrganizationalUnit.objects.create(name_en="Test unit") role_with( person=person, type=role_type_foo, diff --git a/greg/tests/models/test_sponsor.py b/greg/tests/models/test_sponsor.py index 9f6db4f60574081b3069570a5c1632daed6d1f61..581111194aca42b87195e0cab985ff17aec618ef 100644 --- a/greg/tests/models/test_sponsor.py +++ b/greg/tests/models/test_sponsor.py @@ -27,12 +27,12 @@ def sponsor_bar() -> Sponsor: @pytest.fixture def unit1() -> OrganizationalUnit: - return OrganizationalUnit.objects.create(orgreg_id="1", name_en="First unit") + return OrganizationalUnit.objects.create(name_en="First unit") @pytest.fixture def unit2() -> OrganizationalUnit: - return OrganizationalUnit.objects.create(orgreg_id="2", name_en="Second unit") + return OrganizationalUnit.objects.create(name_en="Second unit") @pytest.mark.django_db diff --git a/greg/tests/models/test_sponsor_orgunit.py b/greg/tests/models/test_sponsor_orgunit.py index 317815f7d696142147e17e91628d8b2ca42e3de3..c05c635ccc20258a0c30ae7a5e4078d919a09406 100644 --- a/greg/tests/models/test_sponsor_orgunit.py +++ b/greg/tests/models/test_sponsor_orgunit.py @@ -5,5 +5,5 @@ import pytest def test_sponsor_org_repr(sponsor_org_unit): assert ( repr(sponsor_org_unit) - == "SponsorOrganizationalUnit(id=1, sponsor=Sponsor(id=1, feide_id='guy@example.org', first_name='Sponsor', last_name='Guy'), organizational_unit=OrganizationalUnit(id=1, orgreg_id='12345', name_en='foo_unit', parent=None), hierarchical_access=False)" + == "SponsorOrganizationalUnit(id=1, sponsor=Sponsor(id=1, feide_id='guy@example.org', first_name='Sponsor', last_name='Guy'), organizational_unit=OrganizationalUnit(id=1, name_en='foo_unit', parent=None), hierarchical_access=False)" ) diff --git a/greg/tests/populate_database.py b/greg/tests/populate_database.py index 3ac357ebcfb90b8c0a12d867f204fded6386b310..f4e103f1d569a766b3fcc2fb237cdb32bd289716 100644 --- a/greg/tests/populate_database.py +++ b/greg/tests/populate_database.py @@ -50,7 +50,7 @@ class DatabasePopulation: self.random = random.Random() def populate_database(self): - for i in range(10): + for _ in range(10): first_name = self.faker.first_name() last_name = self.faker.last_name() @@ -82,21 +82,19 @@ class DatabasePopulation: RoleType.objects.create(identifier=role_type, name_en=role_type) ) - for i in range(10): + for _ in range(10): self.units.append( - OrganizationalUnit.objects.create( - orgreg_id=f"12345{i}", name_en=self.faker.company() - ) + OrganizationalUnit.objects.create(name_en=self.faker.company()) ) - for i in range(5): + for _ in range(5): self.sponsors.append( Sponsor.objects.create( feide_id=self.faker.bothify(text="???####@uio.no") ) ) - for i in range(10): + for _ in range(10): self.consent_types.append( ConsentType.objects.create( identifier=self.faker.slug(), @@ -162,14 +160,14 @@ class DatabasePopulation: Role.objects.create( person=random.choice(self.persons), type=random.choice(self.role_types), - orgunit_id=random.choice(self.units), + orgunit=random.choice(self.units), start_date=self.faker.date_this_decade(), end_date=self.faker.date_this_decade( before_today=False, after_today=True ), contact_person_unit=self.faker.name(), available_in_search=self.random.random() > 0.5, - sponsor_id=random.choice(self.sponsors), + sponsor=random.choice(self.sponsors), ) person_role_count += 1 except IntegrityError: diff --git a/greg/tests/test_notifications.py b/greg/tests/test_notifications.py index 290fb5cf0142ff4416ff6acee9d2145cd3511db8..e5b68d0a6f4829681b3a3f663c2ba198131972cf 100644 --- a/greg/tests/test_notifications.py +++ b/greg/tests/test_notifications.py @@ -15,7 +15,7 @@ from greg.models import ( @pytest.fixture def org_unit_bar() -> OrganizationalUnit: - return OrganizationalUnit.objects.create(orgreg_id="bar_unit") + return OrganizationalUnit.objects.create() @pytest.fixture diff --git a/gregsite/settings/base.py b/gregsite/settings/base.py index b6dced4efb765fb3192c7c6456e8d5647520425a..7a618734d1a449a891d4e8078810d3064a4500e1 100644 --- a/gregsite/settings/base.py +++ b/gregsite/settings/base.py @@ -267,3 +267,7 @@ ENVIRONMENT = "unknown" INTERNAL_RK_PREFIX = "no.{instance}.greg".format(instance=INSTANCE_NAME) FEIDE_SOURCE = "feide" + +# Used by the OU import from orgreg to distinguish the OuIdentifiers from others +ORGREG_SOURCE = "orgreg" +ORGREG_NAME = "orgreg_id" diff --git a/gregui/api/serializers/ouidentifier.py b/gregui/api/serializers/ouidentifier.py new file mode 100644 index 0000000000000000000000000000000000000000..b1a60c2fdd87c4d32ef7ecbe59077bb449d9da0c --- /dev/null +++ b/gregui/api/serializers/ouidentifier.py @@ -0,0 +1,9 @@ +from rest_framework.serializers import ModelSerializer + +from greg.models import OuIdentifier + + +class IdentifierSerializer(ModelSerializer): + class Meta: + model = OuIdentifier + fields = ("id", "name", "source", "value") diff --git a/gregui/api/serializers/unit.py b/gregui/api/serializers/unit.py index c32945cbaf25cf3c927b57d8514f8801f9208555..565253ee02b4a3f76bbab69f1a1f7ff25efd6b62 100644 --- a/gregui/api/serializers/unit.py +++ b/gregui/api/serializers/unit.py @@ -1,9 +1,12 @@ from rest_framework.serializers import ModelSerializer from greg.models import OrganizationalUnit +from gregui.api.serializers.ouidentifier import IdentifierSerializer class UnitSerializerId(ModelSerializer): + identifiers = IdentifierSerializer(many=True, read_only=True) + class Meta: model = OrganizationalUnit - fields = ("orgreg_id", "name_nb", "name_en") + fields = ("name_nb", "name_en", "identifiers") diff --git a/gregui/api/views/invitation.py b/gregui/api/views/invitation.py index 2fd4485434a29cc01ceda0b9ee8337dd2f264b1c..5331d43fb6f13da8a8f30ed84da72ce8fc7673c6 100644 --- a/gregui/api/views/invitation.py +++ b/gregui/api/views/invitation.py @@ -66,7 +66,7 @@ class InvitationView(CreateAPIView, DestroyAPIView): invitationlink = InvitationLink.objects.filter( invitation__role__person_id=person.id, - invitation__role__sponsor_id=sponsor_user.sponsor_id, + invitation__role__sponsor_id=sponsor_user.sponsor.id, ) # TODO: send email to invited guest print(invitationlink) diff --git a/gregui/tests/conftest.py b/gregui/tests/conftest.py index 2e34c790b013086e4e3241c54fa396779a37e1b3..2ed152c3b9ed1e108772f712a9af67d8d0a207e9 100644 --- a/gregui/tests/conftest.py +++ b/gregui/tests/conftest.py @@ -129,9 +129,7 @@ def client() -> APIClient: @pytest.fixture def unit_foo() -> OrganizationalUnit: - ou = OrganizationalUnit.objects.create( - orgreg_id="12345", name_en="Foo EN", name_nb="Foo NB" - ) + ou = OrganizationalUnit.objects.create(name_en="Foo EN", name_nb="Foo NB") return OrganizationalUnit.objects.get(id=ou.id)