diff --git a/greg/management/commands/import_guests.py b/greg/management/commands/import_guests.py index 352c45a0c77772d22e0bb1a1a335ce01997392ed..cf7456191232e08bb0d742460a9c911e241139c4 100644 --- a/greg/management/commands/import_guests.py +++ b/greg/management/commands/import_guests.py @@ -49,7 +49,7 @@ class Command(BaseCommand): super().__init__(*args, **kwargs) self._orgunit_id_type = "legacy_stedkode" self._end_date = datetime.datetime.today() + datetime.timedelta(days=100) - self._ignore_id_req = False + self._update_existing = False def add_arguments(self, parser: CommandParser) -> None: parser.add_argument( @@ -68,10 +68,10 @@ class Command(BaseCommand): help="End date of roles. Default: today + 100 days", ) parser.add_argument( - "--ignore-id-req", + "--update-existing", default=False, action="store_true", - help="Imports people without required ids if set", + help="Force update of already imported information", ) def _find_person_from_ids(self, ids: dict) -> Optional[Person]: @@ -182,10 +182,11 @@ class Command(BaseCommand): person=person, type=role_type, ) - role.comments = role_data["comment"] - role.end_date = self._end_date - role.save() - logger.info("updating_existing_role", role=role, sponsor=role.sponsor) + if self._update_existing: + role.comments = role_data["comment"] + role.end_date = self._end_date + role.save() + logger.info("updating_existing_role", role=role, sponsor=role.sponsor) except Role.DoesNotExist: sponsor = self._find_orgunit_sponsor(orgunit) @@ -269,20 +270,21 @@ class Command(BaseCommand): def upsert_person(self, external_id: str, data: dict) -> Optional[Person]: """Add or update person""" - if not self._ignore_id_req and not self._has_required_id(data["ids"]): + person = self._find_person_from_ids(data["ids"]) + if not (person or self._has_required_id(data["ids"])): logger.error("missing_required_id", external_id=external_id) return None - person = self._find_person_from_ids(data["ids"]) if person: - # Update data - person.first_name = data["first_name"] - person.last_name = data["last_name"] - person.date_of_birth = data["date_of_birth"] - person.gender = data["gender"] - person.registration_completed_date = datetime.date.today() - person.save() - logger.info("person_updated", person=person) + if self._update_existing: + # Update data + person.first_name = data["first_name"] + person.last_name = data["last_name"] + person.date_of_birth = data["date_of_birth"] + person.gender = data["gender"] + person.registration_completed_date = datetime.date.today() + person.save() + logger.info("person_updated", person=person) else: # Create new person person = Person.objects.create( @@ -308,7 +310,7 @@ class Command(BaseCommand): self._orgunit_id_type = options["orgunit_type"] self._end_date = options["end_date"] - self._ignore_id_req = options["ignore_id_req"] + self._update_existing = options["update_existing"] with open(options["file"], "r", encoding="UTF-8") as fp: persons = json.load(fp) diff --git a/greg/tests/conftest.py b/greg/tests/conftest.py index d5ab2cfdd773d201216b47e72648dc20645e45d3..1ceabf4b1b0e432c9cccc1bd1a8dd39c69bb176d 100644 --- a/greg/tests/conftest.py +++ b/greg/tests/conftest.py @@ -159,7 +159,7 @@ def role_type_test_guest() -> RoleType: def unit_foo() -> OrganizationalUnit: ou = OrganizationalUnit.objects.create(name_en="foo_unit") OuIdentifier.objects.create( - source="orgreg", name="legacy_stedkode", value="12345", orgunit=ou + source="orgreg", name="legacy_stedkode", value="123456", orgunit=ou ) return OrganizationalUnit.objects.get(id=ou.id) @@ -168,7 +168,7 @@ def unit_foo() -> OrganizationalUnit: def unit_foo2() -> OrganizationalUnit: ou = OrganizationalUnit.objects.create(name_en="foo_unit2") OuIdentifier.objects.create( - source="orgreg", name="legacy_stedkode", value="23456", orgunit=ou + source="orgreg", name="legacy_stedkode", value="234567", orgunit=ou ) return OrganizationalUnit.objects.get(id=ou.id) diff --git a/greg/tests/management/fixtures/guests.json b/greg/tests/management/fixtures/guests.json new file mode 100644 index 0000000000000000000000000000000000000000..f9731fbbffcb8dd524b327465710e0cba9a2f009 --- /dev/null +++ b/greg/tests/management/fixtures/guests.json @@ -0,0 +1,79 @@ +{ + "11111": { + "first_name": "Foo", + "last_name": "New name to be ignored unless specified", + "gender": "male", + "ids": [ + { + "source_system": "Manual", + "external_id": "+4792929292", + "id_type": "private_mobile" + }, + { + "source_system": "cerebrum", + "external_id": "foo@example.com", + "id_type": "feide_email" + }, + { + "source_system": "cerebrum", + "external_id": "foo@uio.no", + "id_type": "feide_id" + }, + { + "source_system": "cerebrum", + "external_id": "11111", + "id_type": "migration_id" + } + ], + "date_of_birth": "1975-06-05", + "role": [ + { + "comment": "Comment here", + "start_date": "2021-12-01", + "role_type": "role_foo", + "orgunit": "123456" + } + ] + }, + "3333333": { + "first_name": "Test", + "last_name": "Surname", + "gender": "female", + "ids": [ + { + "source_system": "Manual", + "external_id": "NO22AB22333", + "id_type": "passport_number" + }, + { + "source_system": "Manual", + "external_id": "+4793939393", + "id_type": "private_mobile" + }, + { + "source_system": "cerebrum", + "external_id": "test.sum@example.com", + "id_type": "feide_email" + }, + { + "source_system": "cerebrum", + "external_id": "testsum@uio.no", + "id_type": "feide_id" + }, + { + "source_system": "cerebrum", + "external_id": "3333333", + "id_type": "migration_id" + } + ], + "date_of_birth": "1988-04-05", + "role": [ + { + "comment": "First comment", + "start_date": "2021-12-01", + "role_type": "role_bar", + "orgunit": "123456" + } + ] + } +} \ No newline at end of file diff --git a/greg/tests/management/fixtures/guests2.json b/greg/tests/management/fixtures/guests2.json new file mode 100644 index 0000000000000000000000000000000000000000..68494cd31de7fddb4de1d0412328d3d2c2511b5b --- /dev/null +++ b/greg/tests/management/fixtures/guests2.json @@ -0,0 +1,54 @@ +{ + "10402": { + "first_name": "Foo", + "last_name": "Bar", + "gender": "male", + "ids": [ + { + "source_system": "DFO_SAP", + "external_id": "05067500360", + "id_type": "norwegian_national_id_number" + }, + { + "source_system": "Manual", + "external_id": "+4755555555", + "id_type": "private_mobile" + }, + { + "source_system": "cerebrum", + "external_id": "11111", + "id_type": "migration_id" + } + ], + "date_of_birth": "1975-03-26", + "role": [ + { + "comment": "Comment here", + "start_date": "2021-12-01", + "role_type": "role_bar", + "orgunit": "123456" + } + ] + }, + "251494": { + "first_name": "Test", + "last_name": "Surname", + "gender": "female", + "ids": [ + { + "source_system": "Manual", + "external_id": "NO22AB22333", + "id_type": "passport_number" + } + ], + "date_of_birth": "1988-04-05", + "role": [ + { + "comment": "Updated comment", + "start_date": "2021-12-01", + "role_type": "role_bar", + "orgunit": "123456" + } + ] + } +} \ No newline at end of file diff --git a/greg/tests/management/test_import_guests.py b/greg/tests/management/test_import_guests.py new file mode 100644 index 0000000000000000000000000000000000000000..e02c1bccdeaa7eef3098feedf2f746a2870e17f3 --- /dev/null +++ b/greg/tests/management/test_import_guests.py @@ -0,0 +1,71 @@ +import pytest + +from django.core.management import call_command + +from greg.models import Person, Role + + +@pytest.mark.django_db +def test_import_guests(sponsor_org_unit, role_type_foo, role_type_bar): + """Test import of two persons from two files""" + + assert Person.objects.count() == 0 + assert Role.objects.count() == 0 + + # One guest with missing required id, one okay + call_command("import_guests", "./greg/tests/management/fixtures/guests.json") + assert Person.objects.count() == 1 + assert Role.objects.count() == 1 + + # Two guests with ok ids + call_command("import_guests", "./greg/tests/management/fixtures/guests2.json") + assert Person.objects.count() == 2 + assert Role.objects.count() == 2 + + # First file again, but a matching id so we import the new role we missed the first + # time + call_command("import_guests", "./greg/tests/management/fixtures/guests.json") + assert Person.objects.count() == 2 + assert Role.objects.count() == 3 + + +@pytest.mark.django_db +def test_import_guests_update_existing(sponsor_org_unit, role_type_foo, role_type_bar): + """Verify that update existing works correctly""" + + assert Person.objects.count() == 0 + assert Role.objects.count() == 0 + + # One guest with missing required id, one okay + call_command( + "import_guests", + "./greg/tests/management/fixtures/guests.json", + "--update-existing", + ) + assert Person.objects.count() == 1 + assert Role.objects.count() == 1 + + # Two guests with ok ids + call_command( + "import_guests", + "./greg/tests/management/fixtures/guests2.json", + "--update-existing", + ) + assert Person.objects.count() == 2 + assert Role.objects.count() == 2 + pe = Person.objects.get(first_name="Foo") + assert pe.last_name == "Bar" + pe = Person.objects.get(first_name="Test") + assert pe.roles.first().comments == "Updated comment" + # First file again, but a matching id so we import the new role + call_command( + "import_guests", + "./greg/tests/management/fixtures/guests.json", + "--update-existing", + ) + assert Person.objects.count() == 2 + assert Role.objects.count() == 3 + pe = Person.objects.get(first_name="Foo") + assert pe.last_name == "New name to be ignored unless specified" + pe = Person.objects.get(first_name="Test") + assert pe.roles.first().comments == "First comment" diff --git a/greg/tests/management/test_import_sponsors_from_cerebrum.py b/greg/tests/management/test_import_sponsors_from_cerebrum.py index 2bafed211ec5b18a3b015a5b31e3e490d715442f..f548a7ea789b0e50ccbf3693c9ff5191fe83e4bd 100644 --- a/greg/tests/management/test_import_sponsors_from_cerebrum.py +++ b/greg/tests/management/test_import_sponsors_from_cerebrum.py @@ -109,13 +109,13 @@ def test_import_sponsors_from_cerebrum( "url": "http://example.com/cerebrum/", "headers": {"X-Gravitee-Api-Key": "fake-key"}, } - settings.CEREBRUM_MANUAL_SPONSOR_UNITS = ["12345"] + settings.CEREBRUM_MANUAL_SPONSOR_UNITS = ["123456"] requests_mock.get( - "http://example.com/cerebrum/v1/groups/adm-leder-12345", + "http://example.com/cerebrum/v1/groups/adm-leder-123456", text=group_response.json(), ) requests_mock.get( - "http://example.com/cerebrum/v1/groups/adm-leder-12345/members/", + "http://example.com/cerebrum/v1/groups/adm-leder-123456/members/", text=group_members_response.json(), ) requests_mock.get( @@ -128,11 +128,11 @@ def test_import_sponsors_from_cerebrum( ) requests_mock.get( - "http://example.com/cerebrum/v1/groups/adm-leder-23456", + "http://example.com/cerebrum/v1/groups/adm-leder-234567", text=group_response.json(), ) requests_mock.get( - "http://example.com/cerebrum/v1/groups/adm-leder-23456/members/", + "http://example.com/cerebrum/v1/groups/adm-leder-234567/members/", text=group_members_response2.json(), ) requests_mock.get( diff --git a/greg/tests/models/test_organizational_unit.py b/greg/tests/models/test_organizational_unit.py index ac15e90209290cfba7d5b2175868e545d77e0120..0d9c2d04d525d7f86200e787f4b424d610ee1ba8 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) (None) (12345)" + assert str(unit_foo) == " (foo_unit) (None) (123456)" @pytest.mark.django_db