Support Herald rules for Maniphest
Closed, ResolvedPublic

Assigned To
btrahan
Priority
Normal
Author
epriestley
Blocks
Restricted Task
Restricted Task
Blocked By
Restricted Task
Restricted Task
Differential Revisions
Restricted Revision
Commits
Restricted Revision / rPfb4c9b63453c: Maniphest + Herald - add support for assigning tasks and adding projects
Restricted Revision / rP477d4e9db19a: Herald - add support for "content source" conditions
Subscribers
jdloft, qgil, roylou and 10 others
Projects
Tokens
"Like" token, awarded by svemir.
Description

We should extend Herald support to Maniphest (and maybe to Phriction and Ponder).

epriestley triaged this task as "Normal" priority.Via WebAug 10 2012, 3:37 PM
epriestley added subscribers: epriestley, cpiro, jeremyb.
epriestley added projects: Herald, Maniphest.
jeremyb added a project: Wikimedia.Via WebAug 10 2012, 6:37 PM
chengyin added a subscriber: chengyin.Via WebOct 18 2012, 9:23 PM
epriestley added a subscriber: ChrisNewman.Via Old WorldJan 14 2013, 9:20 PM

◀ Merged tasks: T2317.

epriestley edited this Task.Via Old WorldMar 19 2013, 3:46 PM
adityar7 added a comment.Via WebApr 6 2013, 7:59 AM

Supporting Phriction would be really useful for some organizations.

We have general Phriction documents like coding guidelines, policies, etc. and ideally, updating these would notify everyone automatically.

adityar7 added a subscriber: adityar7.Via WebApr 6 2013, 7:59 AM
epriestley added a comment.Via WebApr 6 2013, 8:01 AM

T1514 or some similar followup will also add normal CCs to Phriction, at which point you could CC a mailing list, as a possible alternative approach.

adityar7 added a comment.Via WebApr 11 2013, 1:13 PM

Is there currently a way to get around this to some extent i.e. for an email to be sent to a mailing list when a new task is created?

Without being able to do so, wouldn't it basically be impossible to keep track of tasks in a team unless one refreshes the "All Tasks" list periodically or is there some other way this can be done?

epriestley added a comment.Via WebApr 11 2013, 1:21 PM

You can install an event listener today, but it's a high-power / high-complexity solution:

http://www.phabricator.com/docs/phabricator/article/Events_User_Guide_Installing_Event_Listeners.html

Basically, we have good coverage on the low end (look at "All Tasks" every so often) and the high end (install an event listener) but nothing in the middle. Herald will fill that gap.

chengyin added a comment.Via WebApr 11 2013, 8:42 PM

@adityar7, currently we added process in our organization for this - all the bugs simply go to one person and this person reassign to whoever should take the task. I believe this is a common practice.

adityar7 added a comment.Via WebApr 12 2013, 5:36 AM

Thanks for the suggestions. We'll try to create an event listener for now. Generalizing Herald to everything would be a great improvement overall, though. If Herald is built on event listeners, then obviously there's scope for making it really customizable too.

@chengyin: that could work in some cases, but would require a huge overhead if the organization has more than a couple of developers. Add a QA department (which we don't have yet but most places do) and it becomes infeasible.

AnhNhan edited this Task.Via Old WorldApr 27 2013, 10:03 PM
richard.path added a subscriber: richard.path.Via Old WorldJun 18 2013, 1:04 AM
epriestley edited this Task.Via Old WorldJul 9 2013, 3:04 AM
epriestley edited this Task.Via Old WorldSep 4 2013, 3:26 PM
btrahan edited this Task.Via LegacySep 25 2013, 6:46 PM
btrahan claimed this task.Via WebSep 25 2013, 6:49 PM
btrahan added a comment.Via WebSep 25 2013, 11:30 PM

@epriestley - I was trying to add a herald rule for content source. I've hit a quasi-roadblock and wanted to check what to do. Basically PhabricatorContentSource seems to need PHIDs and plug into all that infrastructure real nice. I was thinking it should be somewhat of a hack job with a few static methods on the main class containing data. eg PhabricatorContentSource might have something like

public static function getPHIDs() {
  return array(
     self::CONTENT_SOURCE => 'PHID-CS-'.md5(self::CONTENT_SOURCE), // pretend this is the right length

...and then the PhabricatorPHIDType class would use this and other methods to generate the data it needs.

btrahan added a comment.Via WebSep 25 2013, 11:36 PM

...the alternative to getting PhabricatorContentSource on the phid infrastructure is to cut up the herald infrastructure a bit to assume not everything is a handle so readily. This was proving confusing enough though I prolly just need to take another look later.

epriestley added a comment.Via WebSep 25 2013, 11:49 PM

I think we should avoid PHID'ing them, since the rules probably only care about the type (conduit, email, web), not the entire content source, and content source objects can have more fields than just the type (remote IP, sender's email address, etc).

Instead, I think we can just use a <select /> without TOO much trouble. It would look like:

[Content source] [is     v] [web     v]
                 [is not  ] [email    ]
                            [conduit  ]

This can mostly be modeled on the existing "Add a flag..." action, which uses a <select /> of flag colors.

To do this, I think it looks roughly like this:

  • In HeraldRuleEditor._buildInput (JS), add a 'contentsource' branch, similar to the existing 'flagcolor' branch.
  • In HeraldRuleController->setupEditorBehavior() (PHP), add a template and default in the Javelin::initBehavior(...) call at the bottom.
  • Add HeraldAdapter::VALUE_CONTENT_SOURCE.
  • Add HeraldAdapater::CONDITION_CONTENT_SOURCE.
  • Map it in HeraldAdapter::getValueTypeForFieldAndCondition() (and add other mappings, like condition -> (is, is not)).

I think that does it? Slightly messy but shouldn't be too bad.

I also wonder if we should call it something other than "Content source", but don't have any ideas immediately.

epriestley added a comment.Via WebSep 25 2013, 11:50 PM

Oh, also bump the herald rule version.

btrahan edited this Task.Via LegacySep 26 2013, 9:12 PM
btrahan edited this Task.Via LegacySep 26 2013, 9:19 PM
btrahan edited this Task.Via LegacySep 26 2013, 9:58 PM
btrahan edited this Task.Via LegacySep 26 2013, 10:03 PM
btrahan closed this task as "Resolved".Via WebSep 27 2013, 7:14 PM

This task got a little bit big, so here's what's been done via the above diffs:

  • can now create herald rules for maniphest tasks
    • actions are cc, flag, assign to user, and add project
  • can now make herald conditions based on content source (web, email, conduit, etc)
    • only deployed in maniphest
    • note that "web" is a bit confusing as mobile and tablet are also web technically and really are just how wide your viewport was at the time, though practically speaking you were probably on mobile / tablet

Overall, I / we could do a lot more and add tons of little features. That said, I think this covers what's been asked for so far and I think we should roll out new features as they are asked for. This is a cheap way to also not have to worry about the overall UI too much which I think can get a bit bloated pretty quickly if we just add every option ever.

Definitely appreciate feedback / ideas! Feel free to file new tasks or comment on this one.

roylou added a subscriber: roylou.Via WebDec 26 2013, 8:04 AM
chengyin added a subscriber: btrahan.Via WebJan 27 2014, 6:46 PM

@btrahan

I tried the feature today, and I was very happy about its existence! I didn't even realize it was there already.

Two more sub-features I see useful for us. I'm not sure if they would be valuable in your scope, I'd be more than happy to open individual tickets for them:

  1. Instead of specifying who to notify, can it read from a member of project? The idea is to notify all the project member when a task is created into the project.
  2. Can we include priorities for the condition?

Once again, thanks!

epriestley added a comment.Via WebJan 27 2014, 6:55 PM

(D7436 discusses project mail; it's complicated because it runs into a lot of mailing list issues.)

AMonk changed the visibility of this Task from "All Users" to "Public (No Login Required)".Via WebSat, Apr 11, 8:56 PM
Herald added subscribers: qgil, jdloft. · View Herald TranscriptVia HeraldSat, Apr 11, 8:56 PM

Add Comment