Herald - Add "new" field to herald
ClosedPublic
Actions

Authored by btrahan on Feb 4 2014, 8:15 PM.

Details

Reviewers

epriestley

Maniphest Tasks

Commits

Summary

...and surface it in all adapters except commit adapters. Values are true or false. Ref T4294

Test Plan

made a herald rule to be cc'd on new tasks. was cc'd on new tasks and not cc'd on updates to existing tasks.

Repository

rP Phabricator

Branch

heraldnew

Lint

Lint Passed

Unit

No Test Coverage

src/applications/herald/adapter/HeraldAdapter.php
231	Not sure if this UI is really the best. i could do this in the subclasses so at least it says "new task". a mild improvement perhaps worth getting. this could be something like "object status" or something, but I think it collides with other "status" concepts rather than true / false, could have a "new" vs "updated" select, but that involves getting the "status" word correct.

Cool, looks great to me. Rule version in HeraldRule should get bumped too.

src/applications/herald/adapter/HeraldAdapter.php
105	Since this works a little differently than other fields (which are usually supplied internally), it might be nice to throw if the value isn't set to `true` or `false` (i.e., if you forgot to call setIsNewObject(), you get an obvious error instead of a silently broken field).
231	Hmm, maybe "Is newly created"? I don't think clarifying this is worth the bother of adding new conditions. I haven't seen too much user confusion about how fields/conditions work in Herald, and some of the other ones are a little not-quite-perfect-English already.
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
1557	This only covers Tasks and Mocks -- but not revisions -- I think? I'd expect there to need to be a separate setIsNewObject call somewhere for revisions.