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

Allow users to reject their own watch/blacklist PRs (v2: Now with cleaner commit history!) #11549

Merged
merged 3 commits into from
Sep 1, 2024

Conversation

Spevacus
Copy link
Contributor

This is #11285 but with a less messy commit history (and a small regex adjustment).

I'd have done rebase-foo on THAT PR instead, but my Git wizardry is... Novitiate. That's a word, right?


Implements feature request #8642

This PR allows users to reject/close (including -force) their own auto-created watch and blacklist PRs.

This was tested in this chatroom (where I removed SD ignoring its own messages when considering them for commands and some other changes, the full diff of which is available here where I mistakenly committed them to this branch and had to remove them).

One small change I made was making the payload argument for get_pull_request an optional arg with a blank string as its default. This change has no effect elsewhere in the project as get_pull_request isn't used anywhere aside from in this PR's changes, and you don't need to supply a payload when performing a GET request for a PR anyway.

I also took the liberty to remove the "so the user can learn from their mistakes" content from the required-rejection CmdException. I have a couple of reasons for this removal:

  • This wording makes no sense if the user is rejecting their own PR
  • We frequently reject duplicate PRs. In such a case, there is no mistake being made.

Known limitations:

This PR operates off of the chat messager's chat id, and so does submitting a watch PR. This can be different across chat servers, such as if a user submits a watch PR in SO chat and attempts to self-close it in Charcoal HQ. This will yield a "You need blacklist manager privileges to reject pull requests that aren't created by you." error.

Given that 95% of all watch/blacklist PRs occur in Charcoal HQ, I consider this an acceptable limitation.

@gparyani
Copy link
Contributor

gparyani commented Aug 1, 2024

FYI: this was already requested at #1639 before and was declined. stroopwafels - I confused that for this

@tripleee
Copy link
Member

tripleee commented Sep 1, 2024

@tripleee tripleee merged commit f878cb6 into Charcoal-SE:master Sep 1, 2024
3 checks passed
@tripleee
Copy link
Member

tripleee commented Sep 2, 2024

Sorry for the really long time this took. I'll try to plow through the backlog of pending PRs over the next few weeks.

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

Successfully merging this pull request may close these issues.

3 participants