Fix handling of notifications with project members
ClosedPublic
Actions

Authored by epriestley on Mar 24 2015, 1:40 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T7377: Project reviewer not getting diff notification

Commits

Summary

Fixes T7377. We don't expand projects into members when sending notifications right now. Instead, expand them.

Test Plan

Added a project as a reviewer to a revision, made a comment, saw project members receive a read notification + email (with appropriate preferences).
There's meaningful test coverage on the core mail stuff.

Repository

rP Phabricator

Branch

projnotif

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 4958
Build 4976: [Placeholder Plan] Wait for 30 Seconds

epriestley retitled this revision from to Fix handling of notifications with project members.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

src/applications/feed/PhabricatorFeedStoryPublisher.php
195	This is the actual change.
src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php
62–66	This allows us to implement executeExpansion() easily and remove some code.
79–81	This lets us remove a bunch of redundant code.
src/applications/metamta/storage/PhabricatorMetaMTAMail.php
784	This is a behavioral change. Previously, members who were recipients via projects would not be returned, because this method did not expand projects. Now, we expand projects and return those recipients. The old behavior was incorrect, because these members do receive mail. However, the bug it would have caused is "notifications received by project members are incorrectly delivered unread when they should be delivered read", and that bug was masked by the root bug here (those notifications not being delivered at all). So I fixed the "no notifications" bug, and hit this bug (incorrectly marked "unread" notifications), and fixed it with this change.