Fixes T4666, add Herald rules to Phriction Documents
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T4666: Support Herald in Phriction
- Commits
- Restricted Diffusion Commit
rP8e1a4eef0400: T4666, Support Herald in Phriction
add Herald rule to flag if title contains "xyz", create Phriction Document with title "xyz". Phriction Document should be flagged.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- phrictionheraldadapter
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 3017 Build 3021: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
Some mostly-stylistic inlines, this looks functionally correct to me.
| src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php | ||
|---|---|---|
| 15 | Let's call these "wiki documents", vs "wikis" -- I think we do that elsewhere in the UI, and it's vaguely possible we might some day let you have multiple top-level wikis (which each contain a bunch of documents). | |
| 66 | It would be nice to implement this, did you take a shot at it? Doing it in a followup diff would be cool, too. | |
| 78 | If these show up in this order in the UI, let's put EMAIL right under ADD_CC? I think that's consistent with other applications, and leaving "do nothing" as the last option feels vaguely nice. | |
| 97–99 | Wxxx is dashboard widgets -- Phriction pages don't have a monogram. I think this is just used in the transcript view. You could do something like: pht('Wiki Document %d', $this->getDocument()->getID());...or: pht('Wiki Document "%s"', $this->getDocument()->getContent()->getTitle());...I think. | |
| 112 | Does FIELD_IS_NEW_OBJECT work automatically? I think it does but don't remember. | |
| src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php | ||
|---|---|---|
| 112 | It works (because parent adapter adds it always?) | |