When resolving multiple jobs in a reviewer action, record activity & notify owners once #23459
Conversation
diox
force-pushed
the
reviewer-action-dont-duplicate-activities-or-notifications
branch
from
359fccb to
bd62d47
Compare
| for version in self.target_versions: | ||
| now = datetime.now() |
There was a problem hiding this comment.
did you intend to move this out of the loop? It doesn't change anything here.
There was a problem hiding this comment.
Yes, minor detail but I wanted to guarantee all the versions got the same datestatuschanged to make tracking any issues easier.
| # executed completely and log an activity, the rest are no-op that | ||
| # we just need for record-keeping purposes. We will only notify the | ||
| # owners for that "complete" one. | ||
| notify_owners = action_completed |
There was a problem hiding this comment.
what's the thinking about defaulting notify_owners to action_completed? I don't understand the connection - it would mean all actions that aren't migrated over to ContentAction currently would send multiple emails.
There was a problem hiding this comment.
I can't guarantee non-migrated actions behave like the ContentAction ones and don't do anything/don't log anything when not necessary. In fact I know this isn't the case. I did this to ensure those actions would have the exact same behavior as before out of caution.
Fixes mozilla/addons#15495
Context
While Cinder consolidates multiple reports into the same job, we can still have multiple jobs for a given add-on because of escalations. When that happens, reviewers can resolve them all at the same time, but we don't want to duplicate activities or emails.
Testing
Repeat with a different add-on, this time rejecting multiple versions instead of disabling the add-on.