Skip to content

Conversation

@trz42
Copy link
Contributor

@trz42 trz42 commented Feb 26, 2024

This let the bot only respond to comments which contain a bot command. If so it will either process the command if the sender is authorised to send commands to the bot or it will just respond with a note that the sender is not authorised.

The PR has been tested by letting two different accounts (one authorised, one not authorised) send comments with and without bot commands. See comment trz42/software-layer#72 (comment) and below.

The PR essentially adds a new function contains_any_bot_command and rearranges the processing in handle_issue_comment_event by first checking if the comment was newly created (only newly created comments are considered for processing bot commands), then checking if it contains any bot command, then proceeding with the processing of the commands (incl checking if the sender has permission to send a bot command).

Copy link
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Apart from the very small change the CI brings up I can't think of anything else to change. +1 for testing it "in the wild"


# check if comment does not contain a bot command
if not contains_any_bot_command(comment_received):
self.log(f"comment does not contain a bot comment; not processing it further")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for an f string here, a plain string will do.
Can't claim credit, the CI clued me in for this one 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot. Changed this with e7e69b6

Copy link
Member

@Neves-P Neves-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! 👍

@trz42 trz42 merged commit 7f45435 into EESSI:develop Feb 27, 2024
This was referenced Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants