Policy Transactions - add a details view for custom policy
ClosedPublic
Actions

Authored by btrahan on Apr 29 2014, 2:37 AM.

Details

Reviewers

epriestley
chad

Maniphest Tasks

T4738: Human readable Policy descriptions for transactions

Commits

Restricted Diffusion Commit
rP94a2cfbe4475: Policy Transactions - add a details view for custom policy

Summary

'cuz those can be complicated. Fixes T4738. I needed to do a fair amount of heavy lifting to get the policy stuff rendering correctly. For now, I made this end point very one purpose and tried to make that clear.

Test Plan

looked at some custom policies. see screenshots.

Diff Detail

Repository

rP Phabricator

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

btrahan updated this revision to Diff 21090.Apr 29 2014, 2:37 AM

btrahan added a task: T4738: Human readable Policy descriptions for transactions.

btrahan retitled this revision from to Policy Transactions - add a details view for custom policy.

btrahan updated this object.

btrahan edited the test plan for this revision. (Show Details)

btrahan added reviewers: epriestley, chad.

Herald added subscribers: Korvin, epriestley. · View Herald TranscriptApr 29 2014, 2:37 AM

Harbormaster completed remote builds in B110: Diff 21090.Apr 29 2014, 2:38 AM

Screen_Shot_2014-04-28_at_7.38.16_PM.png (1×1 px, 377 KB)

Screen_Shot_2014-04-28_at_7.38.02_PM.png (1×1 px, 375 KB)

Screen_Shot_2014-04-28_at_7.37.53_PM.png (1×1 px, 452 KB)

Screen_Shot_2014-04-28_at_7.38.10_PM.png (1×1 px, 368 KB)

Screen_Shot_2014-04-28_at_7.37.56_PM.png (1×1 px, 367 KB)

Easy to use new icons?

src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
115	yay icons. maybe fa-minus-circle for denied and fa-checked-circle for allow?

easily switch out icons as suggested =D

Harbormaster completed remote builds in B111: Diff 21092.Apr 29 2014, 2:57 AM

haha, awesome. I meant how was the icon experience? Everything you'd want/easy to understand?

I think its good. Scratchin' my head I can come up with:

setIconFont felt like setIconName to me, but then I figured this "font" stuff was just some design terminology I'm not hip with yet. Plus its "Font Awesome" so that's got to mean something.

I had some trouble finding good icons - searching for words like "approve" that just aren't there (duh) - but now that I've looked through it I have a much better sense of what's there.

Some minor inlines.

src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php
73	We should label this cancel button "Close".
82–96	Put this logic in the rules? e.g. `$rule_object->getRequiredHandlePHIDsForSummary()` or similar?
121–130	Move this into `$rule_object` too?