This is kinda bad in terms of UI (It just makes a json of the thing and diffs that), but it's a start.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rP6dc30ecc8ef1: Drive Herald edits via transactions
edit rule, create rule, add/remove/edit conditions, actions
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
What I thought was a failing test is just strange validation error (It throws an exception if you submit a rule with no actions/conditions; Just reporting them as "errors" makes things break later; Maybe do this validation in the client side?).
Comment Actions
For consistency, I think TYPE_RENAME should just be TYPE_NAME. There's a special awkward case when creating a rule right now:
You could technically ignore that because it will be fixed by EditEngine, but maybe worth special casing it in the meantime since it could be a while before EditEngine arrives here.
In an ideal world it might be nice for this to be split into smaller pieces (repetition, match rule, conditions, actions?) but I think this is a big step forward and it's not particularly difficult to split apart later.
| src/applications/herald/storage/HeraldRuleTransaction.php | ||
|---|---|---|
| 80 | Maybe include a TYPE_EDIT here -- the default value that it's picking up is actually fairly reasonable, but can't be translated. | |
Comment Actions
- s/rename/name/
- change first "renamed" transaction to "created".
The first "Edited" tx still shows as "edited", but this is harder to test for.