Don't show update-pr-from-base-branch when there are conflicts
#3879
Conversation
| if (!canMerge && !isDraftPR) { | ||
| const hasConflicts = select.exists('.js-merge-pr a[href$="/conflicts"]'); | ||
|
|
||
| if ((!canMerge && !isDraftPR) || hasConflicts) { |
There was a problem hiding this comment.
The logic overlaps, maybe the whole block should be be rewritten because the core logic is "if has conflicts or can not merge, return"
We already have hasConflicts but can we determine whether "it can merge" in a single selector? Without also checking specifically for draft PRs.
There are 8 situations to be tested, combinations of:
- PR state: regular or draft
- user: collaborator or third party
- conflicts: yes or no
Note: these are situations to be tested, not conditions that should appear in the code
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Why are we checking if its a draft PR?
That's what I'm saying. We shouldn't.
the core logic is "if has conflicts or can not merge, return"
Without also checking specifically for draft PRs.
There was a problem hiding this comment.
This doesn't work on draft PRs at all now because the button is always :disabled there
|
I think our We should replace it with
Maybe the selector is this? |
|
I don't see "Update branch" here: #3839 I think the logic suggested above would fix it |
|
| // Button exists when the current user can merge the PR. | ||
| // Button is disabled when there are conflicts (there's already a native "Resolve conflicts" button) |
even if it means one more select
|
Not sure if this PR did it. But #3904 has update base branch 3 times |
2 participants
LINKED ISSUES:
Fixes
update-pr-from-base-branchshouldn't appear when there's a "Resolve conflicts" button #3861TEST URLS:
Drop
select-domin favor of extending the prototype #3855SCREENSHOT:
None