Add reminder functionality to the autonag bot

[tomrittervg]

For several purposes, it would be good to be able to have a bot come back and place a reminder on a bug. This can be used for sec-approval to land a test or to come back and remove a preference after letting it ride the trains.

Checklist

• Type annotations added to new functions - No. I'm not sure these are needed?
• Docs added to functions touched in main classes - n/a
• Dry-run produced the expected results.

[marco-c]

Quick comment, haven't had time to look at the code yet.

For several purposes, it would be good to be able to have a bot come back and place a reminder on a bug. This can be used for sec-approval to land a test

We have #978 for that which we plan to implement soon. Were you thinking of this "reminder" functionality as a replacement or in addition to that?

or to come back and remove a preference after letting it ride the trains.

We are thinking of defining a process for expiring test variants (https://docs.google.com/document/d/1U17fEx9D1lRjJMfjNrGPjjbQCOy1qQjjuj9gufsXW9c/edit), which we will likely enforce with autonag. We were thinking of doing something similar in the future about preferences. Might be worth formalizing this instead of relying on a generic reminder functionality to ensure people don't forget to add reminders :)

That said, this generic reminder functionality is a neat idea!

[mozfreddyb]

I believe this patch would be useful and I'm OK if this supersede the work in #978. Tom tells me that the in-testsuite? flag is used inconsistently and there are bugs with this flag that do not even have a test attached.

Our main requirement is that we can get people to remember to check in tests, this here would support that use case.

[marco-c]

I believe this patch would be useful and I'm OK if this supersede the work in #978. Tom tells me that the in-testsuite? flag is used inconsistently and there are bugs with this flag that do not even have a test attached.

Our main requirement is that we can get people to remember to check in tests, this here would support that use case.

The main drawback is that it becomes very hard to perform statistical analyses if it gets used for multiple things (stupid example: if you want to know how many sec bugs had tests to land). IDK if we will ever need it, but you never know what you might need in the future so it could be good to make it potentially possible.

What if we define a sub-whiteboard to specify what the reminder is about? E.g. "[reminder-test DATE]" or "[reminder-pref DATE]".

[tomrittervg]

What if we define a sub-whiteboard to specify what the reminder is about? E.g. "[reminder-test DATE]" or "[reminder-pref DATE]".

Updated it, so you can now put [reminder-foo if you want. :)

[suhaibmujahid]

Thank you for working on this, @tomrittervg! Please let me know if you need clarification or help.

[marco-c]

I believe this patch would be useful and I'm OK if this supersede the work in #978. Tom tells me that the in-testsuite? flag is used inconsistently and there are bugs with this flag that do not even have a test attached.

Our main requirement is that we can get people to remember to check in tests, this here would support that use case.

Maybe stupid idea, but what if we detect sec bugs that have tests attached but not landed and nag on them? This way we totally circumvent the problem and people don't have to remember to add reminders or keywords or flags.

[mozfreddyb]

I believe this patch would be useful and I'm OK if this supersede the work in #978. Tom tells me that the in-testsuite? flag is used inconsistently and there are bugs with this flag that do not even have a test attached.
Our main requirement is that we can get people to remember to check in tests, this here would support that use case.

Maybe stupid idea, but what if we detect sec bugs that have tests attached but not landed and nag on them? This way we totally circumvent the problem and people don't have to remember to add reminders or keywords or flags.

Yes. if that would take care of the backlog. Don't mind doing that at all, if we want that functionality on top.

[marco-c]

Yes. if that would take care of the backlog. Don't mind doing that at all, if we want that functionality on top.

We could apply the same solution for both the backlog and future bugs. We also have a feature in the bot to set a maximum number of needinfos per person, so we don't risk overwhelming people with needinfos because of a potentially large backlog.

[tomrittervg]

@suhaibmujahid - I've made a bunch of edits, and I tested it locally to ensure it runs. The output indicates that this is working now.

[suhaibmujahid]

Thank you for the nice work, @tomrittervg! Please see my comments.

[tomrittervg]

@suhaibmujahid okay, think I've addressed all those.

[suhaibmujahid]

LGTM. Thank you, just minor comments.

[marco-c]

Looks good to me, we should just add a check so only people with editbugs (or canconfirm) privileges can set reminders up. You can add groups to the include_fields when you retrieve users, and then for each user check if there is a group which is "editbugs" (or "canconfirm").

[tomrittervg]

Looks good to me, we should just add a check so only people with editbugs (or canconfirm) privileges can set reminders up. You can add groups to the include_fields when you retrieve users, and then for each user check if there is a group which is "editbugs" (or "canconfirm").

This is done.

[suhaibmujahid]

LGTM