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

Bugfix: Consider old style CLA documents without userId #298

Merged
merged 1 commit into from
Jan 3, 2018

Conversation

KharitonOff
Copy link
Contributor

@MichaelTsengLZ could you review my changes, please? Are you ok with it?

problem 1: old CLA documents don't match the query with userId -> user is requested to sign the CLA again, this doesn't work either, because of index of the document, which doesn't consider the userId.

solution: extend query with search parameter, considering nonexistend userId

problem 2: method isSignificantPullRequest expects user token as mandatory attribute, in case user clicks the "recheck" link in the PR comment this method is also called, but there is no token provided

solution: use token of the linked item if there is no user token

problem #1: old CLA documents don't match the query with userId -> user is requested to sign the CLA again, this doesn't work either, because of index of the document, which doesn't consider the userId.

solution: extend query with search parameter, considering nonexistend userId

problem #2: method isSignificantPullRequest expects user token as mandatory attribute, in case user clicks the "recheck" link in the PR comment this method is also called, but there is no token provided

solution: use token of the linked item if there is no user token
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 91.592% when pulling 55a85ae on reviewNeeded into 25375ff on master.

@MichaelTsengLZ
Copy link
Contributor

Good catch. I should consider the compatibility and the case that contributor will click 'recheck' link. 👍 👍 👍

@KharitonOff KharitonOff merged commit 06abaf0 into master Jan 3, 2018
@KharitonOff KharitonOff deleted the reviewNeeded branch June 11, 2018 08:35
KharitonOff added a commit that referenced this pull request Feb 18, 2020
Bugfix: Consider old style CLA documents without userId
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