Page MenuHomePhabricator

Fix some visibility issues with inline comments in Diffusion
ClosedPublic

Authored by epriestley on Mar 5 2016, 9:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 28, 3:19 PM
Unknown Object (File)
Sat, Dec 21, 12:45 PM
Unknown Object (File)
Wed, Dec 18, 6:21 AM
Unknown Object (File)
Sat, Dec 7, 8:50 AM
Unknown Object (File)
Mon, Dec 2, 6:12 PM
Unknown Object (File)
Nov 2 2024, 4:00 AM
Unknown Object (File)
Nov 2 2024, 3:22 AM
Unknown Object (File)
Nov 2 2024, 1:27 AM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Branch
dx1mail
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 11007
Build 13603: Run Core Tests
Build 13602: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Fix some visibility issues with inline comments in Diffusion.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.
This revision is now accepted and ready to land.Mar 5 2016, 10:08 PM
This revision was automatically updated to reflect the committed changes.