HomePhabricator

Fix some visibility issues with inline comments in Diffusion

Description

Fix some visibility issues with inline comments in Diffusion

Summary:
Fixes T10519. Two issues:

First, the acting user wasn't explicitly included in the mail. This usually didn't matter, but could matter if you unsubscribed and then interacted.

Second, we had some logic which tried to hide redundant "added inline comment" transactions, but could hide them inappropriately. In particular, if another action (like a subscribe) was present in the same group, we could hide the inlines because of that other transaction, then also hide the subscribe. This particular issue is likely an unintended consequence of hiding self-subscribes.

Instead of hiding inlines if anything else happened, hide them only if:

  • there is another "added a comment" transaction; or
  • there is another "added an inline comment" transaction.

This prevents the root issue in T10519 (incorrectly hiding every transaction, and thus not sending the mail) and should generally make behavior a little more consistent and future-proof.

Test Plan:

  • Submitted only an inline comment on a commit I had not previously interacted with.
  • Before patch: no mail was generated (entire mail was improperly hidden).
  • After patch: got some mail with my comment.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10519

Differential Revision: https://secure.phabricator.com/D15407

Details

Provenance
epriestleyAuthored on Mar 5 2016, 9:49 PM
epriestleyPushed on Mar 5 2016, 10:18 PM
Reviewer
chad
Differential Revision
D15407: Fix some visibility issues with inline comments in Diffusion
Parents
rP090252a89e2b: Clean up Macro view page
Branches
Unknown
Tags
Unknown
Tasks
T10519: no email sent for inline comments on a Diffusion commit
Build Status
Buildable 11008
Build 13604: Run Core Tests