-
Notifications
You must be signed in to change notification settings - Fork 356
Fix CHANGE_AUTHOR and CHANGE_AUTHOR_DISPLAY_NAME #325
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
Fix CHANGE_AUTHOR and CHANGE_AUTHOR_DISPLAY_NAME #325
Conversation
...va/com/cloudbees/jenkins/plugins/bitbucket/client/events/BitbucketCloudPullRequestEvent.java
Outdated
Show resolved
Hide resolved
public Author(String username) { | ||
this.username = username; | ||
} | ||
|
||
public String getUsername() { | ||
return username; | ||
} | ||
|
||
public void setUsername(String username) { | ||
this.username = username; | ||
} | ||
|
||
public String getIdentifier() { | ||
return identifier; | ||
} | ||
|
||
public void setIndentifier(String identifier) { | ||
public void setIdentifier(String identifier) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (this.pullRequest.getSource().getRepository().getOwnerName().equals(this.pullRequest.getAuthorLogin())) { | ||
if (!this.pullRequest.getSource().getRepository().getOwnerName().equals(repository.getOwnerName())) { // i.e., a fork | ||
BitbucketCloudRepositoryOwner owner = new BitbucketCloudRepositoryOwner(); | ||
owner.setUsername(this.pullRequest.getAuthorLogin()); | ||
owner.setDisplayName(this.pullRequest.getAuthorDisplayName()); | ||
owner.setUsername(this.pullRequest.getSource().getRepository().getOwnerName()); | ||
owner.setDisplayName(this.pullRequest.getAuthorLogin()); | ||
if (repository.isPrivate()) { | ||
this.pullRequest.getSource().getRepository().setPrivate(repository.isPrivate()); | ||
} | ||
this.pullRequest.getSource().getRepository().setOwner(owner); | ||
} else if (this.pullRequest.getSource().getRepository().getOwnerName().equals(repository.getOwnerName())) { | ||
} else { // origin branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly fixes a bug with the handling of webhooks for forked PRs in BB Cloud, though I am not sure how to test that outside a mock context (discovered due to unit test failures): I cannot find any non-test usages of BitbucketRepository.getOwner
except for code in this reconstructMissingData
method which sets an owner
field. Perhaps @stephenc can clarify if he still remembers anything about it (but this was over three years ago).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok to me. The changes to the value of CHANGE_AUTHOR
for Bitbucket Server should probably be called out in the release notes as a potential compatibility issue.
Fixes #195, which is not a plugin regression, but a cloud API regression to satisfy GDPR. There is apparently no way to set
CHANGE_AUTHOR
to the previous value. (CHANGE_FORK
, or its first component if/
-separated, is probably the same thing, though this is not necessarily the author.) Nor isCHANGE_AUTHOR_EMAIL
possible.Also, for Server, the display name was mistakenly being bound to the variable intended for the (code) username.
Example variables with BB Cloud, before:
after:
With BB Server 7, before:
and after: