From b31ac43f7520f09ee0a99fe2a1bf94a1b35bf8b5 Mon Sep 17 00:00:00 2001 From: jrcastro2 Date: Thu, 19 Dec 2024 14:39:31 +0100 Subject: [PATCH] names: remove internal id from sync * Always sync CERN name, if orcid is present tag CERN name as unlisted * closes https://github.com/CERNDocumentServer/cds-rdm/issues/284 --- site/cds_rdm/tasks.py | 8 ++++++-- site/cds_rdm/utils.py | 22 ++++------------------ site/tests/test_merge_names.py | 9 ++++++--- site/tests/test_tasks.py | 10 +++++++--- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/site/cds_rdm/tasks.py b/site/cds_rdm/tasks.py index 9df943c..a989a02 100644 --- a/site/cds_rdm/tasks.py +++ b/site/cds_rdm/tasks.py @@ -113,7 +113,7 @@ def sync_local_accounts_to_names(since=None, user_id=None): orcid = user.user_profile.get("orcid") try: # 1. Prefer ORCID: if the user has an ORCID, we update the ORCID name with the - # CERN user info, and we unlist any previous CERN-only name + # CERN user info, and we also update and unlist any previous CERN-only name current_app.logger.debug( f"Names sync | Fetching ORCID name for user {user.id}." ) @@ -125,7 +125,11 @@ def sync_local_accounts_to_names(since=None, user_id=None): f"Names sync | Unlisting CERN name for user {user.id}." ) names_utils.update_name(user, cern_name, unlist=True, uow=uow) - + else: + current_app.logger.debug( + f"Names sync | No CERN name found for user {user.id}. Creating new unlisted name." + ) + names_utils.create_new_name(user, unlist=True, uow=uow) current_app.logger.debug( f"Names sync | Updating ORCID name for user {user.id}." ) diff --git a/site/cds_rdm/utils.py b/site/cds_rdm/utils.py index 7873a55..c34f32e 100644 --- a/site/cds_rdm/utils.py +++ b/site/cds_rdm/utils.py @@ -108,21 +108,6 @@ def add_or_update_email(self, user, name, updated_name, updated=False): updated = True return updated - def add_person_id(self, user, name, updated_name, updated=False): - """Adds the person id to the name. - - :param user: The user object. - :param name: The name dictionary. - :param updated_name: The updated name dictionary. - :param updated: If the name has already been updated. - :return: Boolean indicating if the name was updated. - """ - person_id = user.user_profile.get("person_id") - if person_id and not name.get("internal_id"): - updated_name["internal_id"] = person_id - updated = True - return updated - def check_if_update_needed(self, user, name, unlist=False): """Check if the name needs to be updated. @@ -148,7 +133,6 @@ def check_if_update_needed(self, user, name, unlist=False): update_functions = [ self.add_affiliations, self.add_orcid, - self.add_person_id, self.add_or_update_props, self.add_or_update_email, ] @@ -182,7 +166,7 @@ def update_name(self, user, name_dict, unlist=False, uow=None): except ValidationError as e: current_app.logger.error(f"Error updating name for user {user.id}: {e}") - def create_new_name(self, user, uow=None): + def create_new_name(self, user, unlist=False, uow=None): """Creates a new name for the user. :param user: The user object. @@ -191,7 +175,6 @@ def create_new_name(self, user, uow=None): default_props = self.get_default_props(user.id) name = { "id": str(user.user_profile["person_id"]), - "internal_id": str(user.user_profile["person_id"]), "props": default_props, "identifiers": [], } @@ -217,6 +200,9 @@ def create_new_name(self, user, uow=None): if user.email: name["props"]["email"] = user.email + if unlist: + name["tags"] = ["unlisted"] + try: self.service.create(system_identity, name, uow=uow) except ValidationError as e: diff --git a/site/tests/test_merge_names.py b/site/tests/test_merge_names.py index 3f8150c..e80cb30 100644 --- a/site/tests/test_merge_names.py +++ b/site/tests/test_merge_names.py @@ -31,7 +31,11 @@ def test_sync_name_with_existing_orcid(app, database, user_3, name_user_3): Name.index.refresh() names = service.scan(system_identity) - assert len(list(names.hits)) == 1 + # Creates the CDS name unlisted and the ORCID name + assert len(list(names.hits)) == 2 + + cds_name = service.read(system_identity, user_3.user_profile["person_id"]) + assert cds_name.data["tags"] == ["unlisted"] filter = dsl.Q( "bool", @@ -40,9 +44,8 @@ def test_sync_name_with_existing_orcid(app, database, user_3, name_user_3): ], ) - # Since the ORCID value is present no CDS name is created but the user data is merged to the ORCID one os_name = service.search(system_identity, extra_filter=filter) - assert os_name.total == 1 + assert os_name.total == 2 name = service.read(system_identity, id_) diff --git a/site/tests/test_tasks.py b/site/tests/test_tasks.py index d4c1432..7f0f1cf 100644 --- a/site/tests/test_tasks.py +++ b/site/tests/test_tasks.py @@ -76,7 +76,8 @@ def test_sync_name_with_existing_orcid(app, database, user_3, name_user_3): Name.index.refresh() names = service.scan(system_identity) - assert len(list(names.hits)) == 4 + # There should be user 1, user 1 orcid value, user 2, user 3 and user 3 orcid value + assert len(list(names.hits)) == 5 filter = dsl.Q( "bool", @@ -85,9 +86,12 @@ def test_sync_name_with_existing_orcid(app, database, user_3, name_user_3): ], ) - # Since the ORCID value is present no CDS name is created but the user data is merged to the ORCID one + # The ORCID value is present but the CDS name is created anyway unlisted os_name = service.search(system_identity, extra_filter=filter) - assert os_name.total == 1 + assert os_name.total == 2 + + cds_name = service.read(system_identity, user_3.user_profile["person_id"]) + assert cds_name.data["tags"] == ["unlisted"] name = service.read(system_identity, id_)