Page MenuHomePhabricator

Allow Differential Mail to accept multiple comments for one mail
ClosedPublic

Authored by epriestley on Feb 11 2014, 9:22 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 6:13 PM
Unknown Object (File)
Sun, Dec 8, 6:00 PM
Unknown Object (File)
Wed, Dec 4, 8:01 PM
Unknown Object (File)
Sat, Nov 30, 7:47 AM
Unknown Object (File)
Tue, Nov 26, 3:56 PM
Unknown Object (File)
Tue, Nov 26, 3:56 PM
Unknown Object (File)
Tue, Nov 26, 3:56 PM
Unknown Object (File)
Tue, Nov 26, 3:56 PM
Subscribers

Details

Summary

Ref T2222. Currently, one DifferentialComment can do a lot of things (add ccs, reviewers, comments, inline comments, and perform state changes). In the future, each ApplicationTransaction does only one thing. This is the primary piece of complexity which makes the upcoming migration risky, because each comment needs to migrate into multiple transactions.

I want to mitigate this complexity as much as possible before the migration itself happens. One approach I'm going to use to do that is to start writing one comment per effect now, so the mapping is more direct when the migration itself happens and the write code can be straightforward (one row per save()) after the migration.

This tackles a small piece of that, which is the mail Differential sends. Currently, Differential mail acts on a single comment. Instead, allow it to act on a list of comments, but always give it one comment for now. In the future, we can hand it several comments instead and still get the expected behavior.

This change should have no impact on any application behaviors.

Test Plan
  • Commented;
  • commented with inline;
  • added reviewers;
  • added CCs;
  • added CCs via mentions;
  • updated revision;
  • looked at all the mail, all of which seemed sane.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

src/applications/differential/editor/DifferentialRevisionEditor.php
389โ€“391

There are no functional changes here, this is just a simplification. $comment is never used. The old code is the same as the new code.

src/applications/differential/mail/DifferentialReviewRequestMail.php
7โ€“18

This method meant (and continues to mean) "set the body text of the mail", and does not refer to comment objects.