1667877 - Phabricator emails: when a freshly-created secure revision doesn't have a bug, an exception is thrown.

Status: RESOLVED FIXED

(Conduit :: Phabricator, defect)

Description

[Mitchell Hentges [:mhentges] 🦀]

See this Sentry ticket

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

Hmm, there was an assumption made that all secure revisions would have an associated bug.
However, each revision is marked as "secure" for a few seconds after creation until phab-bot confirms that it's able to be public.
If an email-generating event happens in this time frame (e.g.: for this revision in which necko-reviewers was added by Herald), then phab-emails has to handle a secure revision without a bug - which it's unprepared for.

Comment 2

[Mitchell Hentges [:mhentges] 🦀]

If an email-generating event happens in this time frame (e.g.: for this revision in which necko-reviewers was added by Herald), then phab-emails has to handle a secure revision without a bug - which it's unprepared for.

Thinking about this more, revisions are marked as "draft" when they're first created. Once processed by phab-bot (removing the "secure" property if they're public), they're moved out of "draft" state.
We shouldn't be sending emails for events that happen while the revision is still a "draft".

Comment 3

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1667877: Don't publish emails for draft revisions

When revisions are first created, they're marked as "draft" while
phab-bot processes them. Since there can be unexpected state at that
moment (revisions without bugs are temporarily marked as "secure"),
it's cleaner to wait until the processing finished. At that point, send
a single "revision created" email with all relevant information.

Depends on D91635

Comment 4

[Mitchell Hentges [:mhentges] 🦀]

Attachment: GitHub Pull Request