diff --git a/frontend/src/hooks/useOus/index.tsx b/frontend/src/hooks/useOus/index.tsx index 2c4b8615b9431206697079b5c829fb4d88e3513b..a0c0d8a46bfc7e9999607e3ca3d873d45d68cd81 100644 --- a/frontend/src/hooks/useOus/index.tsx +++ b/frontend/src/hooks/useOus/index.tsx @@ -6,6 +6,8 @@ type OuData = { nb: string en: string orgreg_id?: string + identifier_1?: string + identifier_2?: string acronym_nob?: string acronym_nno?: string acronym_eng?: string diff --git a/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx b/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx index 97e35b086876b26ef4626bc6072867f6b0c7f8f9..585a3da6ee1c888f33b5fe099d71f8ab1c19e8f7 100644 --- a/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx +++ b/frontend/src/routes/sponsor/guest/newGuestRole/index.tsx @@ -177,7 +177,9 @@ function NewGuestRole({ guest, reloadGuestInfo }: NewGuestRoleProps) { } return ( <MenuItem key={ou.id.toString()} value={ou.id}> - {name} ({ou.orgreg_id}) + {name} + {ou.identifier_1 ? ` (${ou.identifier_1})` : ''} + {ou.identifier_2 ? ` (${ou.identifier_2})` : ''} </MenuItem> ) } diff --git a/frontend/src/routes/sponsor/register/stepPersonForm.tsx b/frontend/src/routes/sponsor/register/stepPersonForm.tsx index 3002319e6e7aa8e2240ada9a3aaabd5399b75b94..4b2c63aa678241dd93452e3b70ef4d95c563c6e1 100644 --- a/frontend/src/routes/sponsor/register/stepPersonForm.tsx +++ b/frontend/src/routes/sponsor/register/stepPersonForm.tsx @@ -151,24 +151,20 @@ const StepPersonForm = forwardRef( function getFullOptionLabel(ouData: OuData) { switch (i18n.language) { case 'en': - return `${ouData.en} (${ - ouData.acronym_eng ?? ouData.acronym_nob - ? ouData.acronym_eng ?? ouData.acronym_nob - : '' - }) (${ouData.orgreg_id})` + return `${ouData.en}${ + ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' + }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` case 'nn': - return `${ouData.nb} (${ - ouData.acronym_nno ?? ouData.acronym_nob - ? ouData.acronym_nno ?? ouData.acronym_nob - : '' - }) (${ouData.orgreg_id})` + return `${ouData.nb}${ + ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' + }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` default: // There should always be a Norwegian bokmaal acronym set - return `${ouData.nb} (${ - ouData.acronym_nob ? ouData.acronym_nob : '' - }) (${ouData.orgreg_id})` + return `${ouData.nb}${ + ouData.identifier_1 ? ` (${ouData.identifier_1})` : '' + }${ouData.identifier_2 ? ` (${ouData.identifier_2})` : ''}` } } diff --git a/greg/api/serializers/organizational_unit.py b/greg/api/serializers/organizational_unit.py index e5de710e904a857025e3e82acb2d572f33829cc6..3e9420ae0ac75fc6c6f3050fbcaf86f901111425 100644 --- a/greg/api/serializers/organizational_unit.py +++ b/greg/api/serializers/organizational_unit.py @@ -34,6 +34,8 @@ class SponsorOrgUnitsSerializer(ModelSerializer): "nb", "en", "orgreg_id", + "identifier_1", + "identifier_2", "acronym_nob", "acronym_nno", "acronym_eng", diff --git a/greg/importers/orgreg.py b/greg/importers/orgreg.py index 242da61819d1e211f6ce06a7eab3d0036109c15c..18af60511b00379825717f7795470acdf384fc38 100644 --- a/greg/importers/orgreg.py +++ b/greg/importers/orgreg.py @@ -29,14 +29,40 @@ class OrgregImporter: def _upsert_extra_identities(self, ou: OrgUnit): """Upsert any configured extra IDs from orgreg.""" for extra_id in settings.ORGREG_EXTRA_IDS: - identity_in_orgreg = self._get_external_key_from_ou(extra_id, ou) - if identity_in_orgreg is not None: - self._upsert_identifier( - extra_id["source"], - extra_id["type"], - identity_in_orgreg.value, - ou.ou_id, - ) + if "type" in extra_id.keys(): + identity_in_orgreg = self._get_external_key_from_ou(extra_id, ou) + if identity_in_orgreg is not None: + self._upsert_identifier( + extra_id["source"], + extra_id["type"], + identity_in_orgreg.value, + ou.ou_id, + ) + elif "fieldname" in extra_id.keys(): + if hasattr(ou, extra_id["fieldname"]): + field = getattr(ou, extra_id["fieldname"]) + # Subfields are typically the language, but can be other things + subfield = extra_id.get("subfield", None) + if subfield and hasattr(field, subfield): + value = getattr(field, subfield) + # Give nob if nno/eng don't exist + elif ( + subfield + and subfield in ["nno", "eng"] + and hasattr(field, "nob") + ): + value = getattr(field, "nob") + else: + value = field + if value is not None and isinstance(value, str): + self._upsert_identifier( + settings.ORGREG_SOURCE, + extra_id["fieldname"], + value, + ou.ou_id, + ) + else: + logger.warning("Unsupported key value %s", extra_id.keys()[0]) # Acronyms can also be used as identifiers for acronym in settings.ORGREG_ACRONYMS: @@ -138,6 +164,7 @@ class OrgregImporter: self, ou: OrgUnit, values: Mapping[str, Union[str, int, bool]] ): """Upsert ou with latest values and store in processed dict.""" + try: identifier = OuIdentifier.objects.get( name=settings.ORGREG_NAME, source=settings.ORGREG_SOURCE, value=ou.ou_id diff --git a/greg/management/commands/populate_test_data.py b/greg/management/commands/populate_test_data.py index 10272594405874d0009c00976a3edcef6aa62803..0d93d9c4e5c7bedcb6defe416b1c76e1ef81622d 100644 --- a/greg/management/commands/populate_test_data.py +++ b/greg/management/commands/populate_test_data.py @@ -144,6 +144,18 @@ class DatabasePopulation: value="2", orgunit=earth, ) + OuIdentifier.objects.create( + name="shortname", + source=settings.ORGREG_SOURCE, + value="UiJ", + orgunit=earth, + ) + OuIdentifier.objects.create( + name="legacy_stedkode", + source="sapuio", + value="1000", + orgunit=earth, + ) europe = OrganizationalUnit.objects.create( name_nb="Europa", name_en=OU_EUROPE_NAME_EN, parent=earth ) @@ -153,9 +165,33 @@ class DatabasePopulation: value="3", orgunit=europe, ) + OuIdentifier.objects.create( + name="shortname", + source=settings.ORGREG_SOURCE, + value="EU", + orgunit=europe, + ) + OuIdentifier.objects.create( + name="legacy_stedkode", + source="sapuio", + value="1100", + orgunit=europe, + ) america = OrganizationalUnit.objects.create( name_nb="Amerika", name_en="America", parent=earth ) + OuIdentifier.objects.create( + name="shortname", + source=settings.ORGREG_SOURCE, + value="AM", + orgunit=america, + ) + OuIdentifier.objects.create( + name="legacy_stedkode", + source="sapuio", + value="1200", + orgunit=america, + ) OuIdentifier.objects.create( name=settings.ORGREG_NAME, source=settings.ORGREG_SOURCE, @@ -171,6 +207,18 @@ class DatabasePopulation: value="5", orgunit=norway, ) + OuIdentifier.objects.create( + name="shortname", + source=settings.ORGREG_SOURCE, + value="EU-NOR", + orgunit=norway, + ) + OuIdentifier.objects.create( + name="legacy_stedkode", + source="sapuio", + value="1110", + orgunit=norway, + ) def _add_roletypes(self): RoleType.objects.create( diff --git a/greg/models.py b/greg/models.py index 42531bd00e925874eb4407eb982708dd17fb922a..af7aabad73322295442996e6fc586186e292547b 100644 --- a/greg/models.py +++ b/greg/models.py @@ -535,13 +535,49 @@ class OrganizationalUnit(BaseModel): .first() ) + @property + def identifier_1(self) -> Optional[str]: + """Gets the first extra field to show when listing units""" + if len(settings.ORGREG_EXTRA_IDS) > 0: + if "type" in settings.ORGREG_EXTRA_IDS[0].keys(): + identifier = settings.ORGREG_EXTRA_IDS[0]["type"] + elif "fieldname" in settings.ORGREG_EXTRA_IDS[0].keys(): + identifier = settings.ORGREG_EXTRA_IDS[0]["fieldname"] + else: + return None + else: + return None + return ( + self.identifiers.filter(name=identifier) + .values_list("value", flat=True) + .first() + ) + + @property + def identifier_2(self) -> Optional[str]: + """Gets the first extra field to show when listing units""" + if len(settings.ORGREG_EXTRA_IDS) > 1: + if "type" in settings.ORGREG_EXTRA_IDS[1].keys(): + identifier = settings.ORGREG_EXTRA_IDS[1]["type"] + elif "fieldname" in settings.ORGREG_EXTRA_IDS[1].keys(): + identifier = settings.ORGREG_EXTRA_IDS[1]["fieldname"] + else: + return None + else: + return None + return ( + self.identifiers.filter(name=identifier) + .values_list("value", flat=True) + .first() + ) + def __repr__(self) -> str: return "{}(id={!r}, name_en={!r}, parent={!r})".format( self.__class__.__name__, self.pk, self.name_en, self.parent ) def __str__(self) -> str: - return f"{self.name_nb} ({self.name_en})" + return f"{self.name_nb} ({self.name_en}) ({self.identifier_1}) ({self.identifier_2})" def fetch_tree(self): """ diff --git a/greg/tests/models/test_organizational_unit.py b/greg/tests/models/test_organizational_unit.py index 186b2bb501603d2acde3f42a9c8080a50485d5ab..ac15e90209290cfba7d5b2175868e545d77e0120 100644 --- a/greg/tests/models/test_organizational_unit.py +++ b/greg/tests/models/test_organizational_unit.py @@ -17,7 +17,7 @@ def test_org_repr(unit_foo): @pytest.mark.django_db def test_org_str(unit_foo): - assert str(unit_foo) == " (foo_unit)" + assert str(unit_foo) == " (foo_unit) (None) (12345)" @pytest.mark.django_db diff --git a/gregsite/settings/base.py b/gregsite/settings/base.py index 90fc3c108e2aec61e37540660f8f02e68b6252d4..63fca433646be6ee397ed287116f5610dc4a5057 100644 --- a/gregsite/settings/base.py +++ b/gregsite/settings/base.py @@ -303,8 +303,10 @@ INVITATION_DURATION = 30 ORGREG_SOURCE = "orgreg" ORGREG_NAME = "orgreg_id" -# Extra ids to be imported from orgreg, list of dict with source/type. -# [{"source": "sapuio", "type": "legacy_stedkode"}] +# Extra ids to be imported from orgreg. This also is used for fields +# wanted to show when listing units. List of dict with source/type or fieldname/subfield. +# Order is important where the first two fields will be shown in that order +# [{"fieldname": "shortname", "subfield": "nob"},{"source": "sapuio", "type": "legacy_stedkode"}] ORGREG_EXTRA_IDS = [] # Acronyms to be imported as identifiers from orgreg. List of acronym values, supported ones are: nob, nno, eng diff --git a/gregsite/settings/testing.py b/gregsite/settings/testing.py index 0c2c8bed286b0cdc9eba628f83d861d88a00a7a3..0cff0dc5a51520eb84e28183bbe8df1a66394661 100644 --- a/gregsite/settings/testing.py +++ b/gregsite/settings/testing.py @@ -22,5 +22,8 @@ ORGREG_CLIENT = { "endpoints": {"base_url": "https://example.com/orgreg/"}, "headers": {"X-Gravitee-Api-Key": "foo"}, } -ORGREG_EXTRA_IDS = [] +ORGREG_EXTRA_IDS = [ + {"fieldname": "shortname", "subfield": "nob"}, + {"type": "legacy_stedkode", "source": "sapuio"}, +] ORGREG_ACRONYMS = [] diff --git a/gregui/tests/api/views/test_ous.py b/gregui/tests/api/views/test_ous.py index b512e105439a1e096f26495cafb6e91501ab256f..ff091c0f89127af7c11f182384977dfaea838178 100644 --- a/gregui/tests/api/views/test_ous.py +++ b/gregui/tests/api/views/test_ous.py @@ -60,6 +60,24 @@ def test_acronym_nob_field_empty_if_identifier_not_present( resp = client.get(reverse("gregui-v1:ou-list")) data = resp.json() - assert len(data) == 1 assert data[0]["acronym_nob"] is None + + +@pytest.mark.django_db +def test_org_str3(unit_foo3): + assert str(unit_foo3) == " (foo_unit3) (FOO) (123456)" + + +@pytest.mark.django_db +def test_unit_fields_populated_if_identifier_present( + unit_foo3, client, log_in, user_sponsor_foobar +): + log_in(user_sponsor_foobar) + + resp = client.get(reverse("gregui-v1:ou-list")) + data = resp.json() + orgunit = data[0] + assert orgunit["identifier_1"] == "FOO" # shortname + assert orgunit["identifier_2"] == "123456" + assert orgunit["orgreg_id"] == "1" diff --git a/gregui/tests/conftest.py b/gregui/tests/conftest.py index b999205c2e6096ba9efa2234434cdfb168c88d04..c77fe560cbabe1bc9ae4d1ea2745d0deeb084a85 100644 --- a/gregui/tests/conftest.py +++ b/gregui/tests/conftest.py @@ -19,6 +19,7 @@ from greg.models import ( Invitation, InvitationLink, OrganizationalUnit, + OuIdentifier, Person, Role, RoleType, @@ -80,6 +81,21 @@ def unit_bar() -> OrganizationalUnit: return OrganizationalUnit.objects.get(id=ou.id) +@pytest.fixture +def unit_foo3() -> OrganizationalUnit: + ou = OrganizationalUnit.objects.create(name_en="foo_unit3") + OuIdentifier.objects.create( + source="orgreg", name="shortname", value="FOO", orgunit=ou + ) + OuIdentifier.objects.create( + source="sapuio", name="legacy_stedkode", value="123456", orgunit=ou + ) + OuIdentifier.objects.create( + source="orgreg", name="orgreg_id", value="1", orgunit=ou + ) + return OrganizationalUnit.objects.get(id=ou.id) + + @pytest.fixture def role_type_foo() -> RoleType: rt = RoleType.objects.create( @@ -130,6 +146,13 @@ def sponsor_bar(unit_bar: OrganizationalUnit, create_sponsor) -> Sponsor: ) +@pytest.fixture +def sponsor_foobar(unit_foo3: OrganizationalUnit, create_sponsor) -> Sponsor: + return create_sponsor( + feide_id="foo3@example.com", first_name="Foo", last_name="3", unit=unit_foo3 + ) + + @pytest.fixture def create_user() -> Callable[[str, str, str, str], UserModel]: user_model = get_user_model() @@ -195,6 +218,22 @@ def user_sponsor_bar(sponsor_bar: Sponsor, create_user) -> User: return user_model.objects.get(id=user.id) +@pytest.fixture +def user_sponsor_foobar(sponsor_foobar: Sponsor, create_user) -> User: + user_model = get_user_model() + + user = create_user( + username="test_sponsor_foobar", + email="foo3@example.org", + first_name="Foo", + last_name="3", + ) + GregUserProfile.objects.create(user=user, sponsor=sponsor_foobar) + + # This user is a sponsor for unit_foo3 + return user_model.objects.get(id=user.id) + + @pytest.fixture def user_person(person_foo: Sponsor, create_user) -> User: user_model = get_user_model()