[ENG-10586] BE: Ensure only one ORCID ID is availalble in the UserModel.external_identity attribute#11661
Conversation
…D, the existing ORCID ID should be removed/overwritten
futa-ikeda
left a comment
futa-ikeda
left a comment
There was a problem hiding this comment.
Seems like a reasonable fix for now, but I will have @cslzchen weigh in on this, as he would have a bit more knowledge on this workflow.
cslzchen
left a comment
cslzchen
left a comment
There was a problem hiding this comment.
I am surprised that we have two set of the same/similar code ... one in api v1 and the other in api v2. And there seems to be slight differences. Please double check which is the actual code being used.
Usually if we have v2, it means we are using v2. But it may not be the case for ORCiD. Could the V2 could be some unfinished work from previous projects that never worked or tested at all?
This needs to be wider discussion, likely outside the project scope: if v1 is the one being used and has been fully tested and if v2 is untested/unfinished "future" work, should we maintain both code?
Please add/update unit tests.
framework/auth/views.py
Outdated
| external_identity[external_id_provider][external_id] = 'LINK' | ||
| if external_id_provider in user.external_identity: | ||
| user.external_identity[external_id_provider].update(external_identity[external_id_provider]) | ||
| if external_id_provider == 'orcid': |
futa-ikeda
merged commit c856700
into
CenterForOpenScience:feature/orcid-integration
3 participants
Ticket
Purpose
If a user has an existing ORCID ID, and then associates a new ORCID ID, the existing ORCID ID should be removed/overwritten
Changes
Side Effects
also maybe someone knows it better:
not confident whether we need to deal somehow with already existing in database values
not confident how it is /was possible to create several records for ORCID
maybe some backend workflow also will be needed for the stuff
QE Notes
CE Notes
Documentation