Page MenuHomePhabricator

Add email preferences to Pholio
ClosedPublic

Authored by chad on Jun 20 2014, 9:09 PM.
Tags
None
Referenced Files
F14033861: D9644.id23146.diff
Sat, Nov 9, 8:27 PM
F14033771: D9644.id.diff
Sat, Nov 9, 7:59 PM
F14033472: D9644.id23146.diff
Sat, Nov 9, 5:43 PM
F14033405: D9644.id.diff
Sat, Nov 9, 5:36 PM
F14032965: D9644.id23157.diff
Sat, Nov 9, 4:19 PM
F14032940: D9644.id23149.diff
Sat, Nov 9, 4:15 PM
F14032493: D9644.id23149.diff
Sat, Nov 9, 2:34 PM
F14032313: D9644.id23160.diff
Sat, Nov 9, 2:03 PM
Subscribers

Details

Summary

Fixes T5386, adds a base set of email preferences to Pholio

Test Plan

Turned on, tested and got email, turned off, tested and saw notifications.

Diff Detail

Repository
rP Phabricator
Branch
pholio-email
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1271
Build 1271: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

chad retitled this revision from to Add email preferences to Pholio.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.

This seems reasonably "Easy", I can do more in the future - they are good baby watching tasks.

  • Add Revision headers to Pholio History table

Ooops haha, added another diff. At least it's Pholio too.

undefined (403×901 px, 69 KB)

epriestley edited edge metadata.

The "revision" stuff has some issues that we should probably talk through separately, let's put it in a different diff and I'll gab there?

src/applications/pholio/storage/PholioTransaction.php
85

This should include PhabricatorTransactions::TYPE_COMMENT.

There are some additional shared types (TYPE_EDGE, TYPE_SUBSCRIBERS) but it's probably fine to ignore them so they end up in "Other" for now. Generally, this is something that could probably do with unification across applications.

That would also suggest unified tag types (e.g., a single "comment" type instead of a "pholio comment" type).

src/applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php
62

These checks should probably be isClassInstalledForViewer() now, but that's an existing issue / not a big deal either way.

271

Should be "other mock activity".

This revision now requires changes to proceed.Jun 21 2014, 5:02 PM
chad edited edge metadata.
  • Remove derp
chad edited edge metadata.
  • Address previous diff comments
epriestley edited edge metadata.

Two minor inlines.

src/applications/pholio/storage/PholioTransaction.php
66

(This is already handled by parent::getIcon().)

86

Oh, sorry -- this is actually PhabricatorTransactionType::TYPE_COMMENT (i.e., a single type used for all comments in every application).

src/applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php
199

These need a $user parameter.

This revision now requires changes to proceed.Jun 21 2014, 6:08 PM
chad edited edge metadata.
  • Update per comments
epriestley edited edge metadata.
This revision is now accepted and ready to land.Jun 21 2014, 6:55 PM
chad updated this revision to Diff 23160.

Closed by commit rPc9a195369f25 (authored by @chad).