-
Notifications
You must be signed in to change notification settings - Fork 209
[MNT] issue-assign-bot - only users with write access should be able to assign others #2739
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for contributing to
|
…into issue-assign-bot-1
@MatthewMiddlehurst, I've resolved the merge conflicts and tested the feature here - shinymack#7 |
if user != commenter and commenter_permission not in ["admin", "write"]: | ||
comment_msg = ( | ||
f"@{commenter}, you cannot assign @{user}" | ||
" because you lack write access.\n" | ||
"Only users with write access can assign others." | ||
) | ||
issue.create_comment(comment_msg) |
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.
I don't think a message should be written for each invalid user, just a single one for all.
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.
@MatthewMiddlehurst Should i use two arrays for valid and invalid users, then comment for the invalid users and then move onto the 2 issue limit code for valid users.
also the "assign me" statement doesn't check for the 2 issue limit, should i also fix that?
and for the code conflicting with the below one, do you mean that even when an admin is assigning someone it should check the 2 issue limit?
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.
Just have a single printout if they try to assign someone else after the for loop. No need to @ them for each user they try to add.
If you continue the loop the code below wont run. there is lots of duplication going on im pretty sure
elif user != commenter and commenter_permission in ["admin", "write"]: | ||
issue.add_to_assignees(user) | ||
continue |
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.
this conflicts with the code below.
Reference Issues/PRs
Contributes to feature 4 of #2697
What does this implement/fix? Explain your changes.
Users not having write-access to the repository cannot assign other users.
If a user with no write access tries to assign someone else the issue-assign bot will post a warning comment.
f"@{commenter}, you cannot assign @{user}" " because you lack write access.\n" "Only users with write access can assign others."
Does your contribution introduce a new dependency? If yes, which one?
No
Any other comments?
No
PR checklist
For all contributions
For new estimators and functions
__maintainer__
at the top of relevant files and want to be contacted regarding its maintenance. Unmaintained files may be removed. This is for the full file, and you should not add yourself if you are just making minor changes or do not want to help maintain its contents.For developers with write access