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.

Lint

Lint Skipped

Unit

Tests Skipped

src/applications/herald/adapter/HeraldAdapter.php
235	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).
235	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.