Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENG-2111] Unregistered Contributor Claim on Registration also claims on Project #9472

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

UdayVarkhedkar
Copy link
Contributor

Purpose

Currently, when an unregistered contributor is claimed by a registered user, the unregistered contributor is not also claimed on the project the registration is based upon.

Changes

  • If the node the unregistered contributor is being claimed upon is a registration, in addition to the contributor being replaced on the registration, they are also replaced on the origin project.
  • Adds a test

QA Notes

This can be tested through the UI. Follow the same workflow specified in the JIRA ticket expecting the contributor to be modified on the project as well.

Documentation

N/A

Side Effects

N/A

Ticket

JIRA Ticket

Copy link
Contributor

@Johnetordoff Johnetordoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, good use of code commenting

Copy link
Collaborator

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. So this is definitely doing what the ticket asked for, but it's a bit odd because it should already be happening. If I make two projects, add the unregistered contributor (with email address) to both, then claim one of those, I claim both of them. It's supposed to search for all unregistered contributors on any project and claim them.

Could you add a test case where you have a second node, unrelated to the one you're registering, that has that unclaimed user as well, and see if it picks up both nodes when the user claims on the registration? If so, this should be fine. If not, we might need to do a bit more digging to see what's going on.

@@ -733,6 +733,9 @@ def claim_user_registered(auth, node, **kwargs):
if should_claim:
node.replace_contributor(old=unreg_user, new=current_user)
node.save()
if isinstance(node, Registration):
project = node.registered_from
project.replace_contributor(old=unreg_user, new=current_user)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the user is removed from the project before they have a chance to claim on the registration?

@UdayVarkhedkar
Copy link
Contributor Author

I added that test case and it fails. Based off the test and the code here, I don't think that claiming an unregistered contributor on a given project claims the unregistered contributor on all nodes, currently. I can add that logic in this PR, I'm thinking of iterating through the unclaimed records on the unregistered contributor when they are claimed to replace the unregisterd contributor with the claiming contributor. Sound good/in line with your expectations @brianjgeiger?

@brianjgeiger
Copy link
Collaborator

Give it a test on a staging environment. Set up two projects, invite someone by email address, and have them claim on one project. When I did it, both projects were claimed. I think it works already for projects, but I'm not sure why it doesn't for registrations.

@UdayVarkhedkar
Copy link
Contributor Author

I just tested it on staging and the unclaimed contributor is only claimed on the project they explicitly claimed, not on other projects the unclaimed contributor is a contributor.

@brianjgeiger
Copy link
Collaborator

Hmmmm.

@UdayVarkhedkar
Copy link
Contributor Author

UdayVarkhedkar commented Oct 7, 2020

I retested this on staging.osf.io and experienced the same results while testing the registered contributor claiming the unregistered contributor initially on the second project the unregistered contributor was added to (rather than the first which I did last time). It has been my continued experience that a registered contributor claiming an unregistered contributor solely claims the unregistered contributor on the specific project the action was made.

Creating a new ticket to fix this issue or expanding the scope of this ticket to fix that issue appear to be the best ways forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants