Page MenuHomePhabricator

"status" field should not be copied from template when creating new tasks
Open, Needs TriagePublic

Description

When creating new task from template (e.g. somedomain.com/maniphest/task/edit/form/2/?template=123), status field should not be copied.

Usually user do not want to keep template task open because there is nothing to do with that task.
But when creating a new task usually people want to leave it as "Open".

Currently value of "status" field is copied from template.

Event Timeline

Pawka created this task.Apr 4 2016, 11:21 AM

This is intentional, so you can use a task as a template to create other tasks if you have, say, several different "open" statuses. The "Create Subtask" action specifically forces the status to the default open status, but I think the current behavior is correct for bare ?template= which has been entered manually.

One possible workaround is to close the task, then use ?template=X&status=Y to explicitly override the closed status.

But it sounds like the root problem here is really that you want a way to hide template tasks? If that's a problem, I'd be inclined to attack it directly, rather than just sort of half-hiding these tasks by closing them and losing the ability to copy statuses.

Pawka added a comment.Apr 5 2016, 6:50 AM

I see your point with different "open" statuses.

And yes, one of purposes is to hide template tasks. Main problem with making status field copyable - breaking all existing templates which were created before this adjustment.

Do you think it would be worth to include some configuration (on form editor or somewhere else) if field should be copyable on templates?

If we did add configuration, I think there are three ways we could do it:

Global. This would probably be via T6030, and maniphest.fields would get checkboxes for [X] Copy this field when creating a task from a template.

One downside to this is that it's not very flexible: you'd only get one setting that would apply to everything.

An upside is that there's basically already a precedent for this (the copy property in maniphest.custom-field-definitions) and it would generally be consistent with that. Another upside is that it's fairly simple and easy to understand.

Per-Task. The template task itself could conceivably specify which fields get copied when it is used as a template.

This seems like a lot of UI and complexity, and it's hard for me to imagine many use cases for this. Although I can imagine possibly wanting more than 1 set of "which fields get copied?" rules, I can't imagine wanting 50.

This also wouldn't really help with existing tasks, since you'd still have to hunt them all down and configure them.

Per-Form. ?template=1 implicitly selects an edit form, and /task/edit/form/xxx/?template=1 explicitly selects one. So we could let each form choose which fields it copies.

An upside is that this feels better to me in terms of flexibility than global or per-task.

Some downsides are that this isn't very obvious, and the implicit form selection depends on the viewer, so ?template=1 may not give users the same form. If you're using a custom URL anyway, you could just add &status=x.


Some alternate approaches:

Mark Task as Template: We could let you mark tasks as template tasks, and hide them by default.

This doesn't help with existing tasks and I think it creates a lot of UI burden so I'm hesitant to pursue it.

Named Templates: We could possibly do something like:

  • Add a "Save task as template..." option.
  • You name the template, and select which fields it will include. You can choose some subset of fields.
  • Then you use ?template=bug instead of ?template=123, and could close/delete the original task without affecting the template. (Or maybe there's no original task at all and you just "Create template..." and get a "template" variant of the edit form.)

This isn't unreasonable and could be a pathway toward more powerful templating in the future. We do a soft version of this already with form defaults, and it could probably share some code.

I like it better than hiding template tasks, but it's a lot of work. I'd like more clarity on T10222 before moving forward, too.

And this doesn't help with existing tasks.

Extension Hook: Copyability is runtime-mutable, and we could give PhabricatorEditEngineExtension a hook callback like didBuildFields($object, array $fields). Then you could implement an extension like:

// ...

public function didBuildFields($object, array $fields) {
  $fields['status']->setIsCopyable(false);
}

// ...

This is basically forking the behavior without needing to actually fork. Conceivably, you could even implement a rule like "disable status copying if the template is older than T12345; allow it if the template is newer", so all the old tasks would have the old behavior and the new ones would have the new behavior.

We already have this extension point class and this additional hook generally isn't unreasonable (an earlier version of EditEngine even had it, although I ended up not needing it and removed it), but this behavior seems like a fairly flimsy justification for adding a callback hook. If we had a few other flimsy use cases too (or, ideally, at least one good use case) this would be an easy call, but I can't recall any offhand and I'm really resistant to upstreaming changes which affect only one install.

Change the field copyability rule: We could use rules like these instead:

  • Status is copyable only if more than one "open" status is configured.
  • Status is copyable only if the template's status is an open status.
  • Status is copyable only if the status itself is configured to be copyable (like how "auto-assign on close" is now configurable per-status after T10343).

These have varying implementation difficulty. (In particular, other template rules do not currently depend on the actual value being copied, and this isn't wholly straightforward to implement.)

I worry these rules would generally add complexity without adding much value, and also likely affect only one install. T10343 was a very borderline use case, and the use case here feels flimsier to me.


Some workarounds:

  • Also specify &status=x in the URL (doesn't help with existing tasks).
  • Fork and apply D15605 locally -- the change is safe/correct, I just think copying status is the better upstream default now that status is configurable and multiple "open" statuses are supported (at the time the original copy rule was implemented, I believe status was not user-configurable).
  • Uh, search for ?template=x in the access log and go open all those tasks? Kinda junk. I don't think there's any other source you can look at to identify the affected tasks.

Upshot:

  • Of config-based options, global config seems most viable to me, but the pathway forward is a relatively long one, via T6030.
  • Of alternate approaches:
    • I'm open to exploring "named templates" in the future but this is probably some ways away and doesn't solve existing tasks.
    • I'm open to adding an extension point if we can find more use cases to justify its existence, so this isn't a "custom feature for one install" sort of thing.
    • I'm much less enthusiastic about a template flag or making the rule more complex.

All of this probably has a fairly long timeline, so your best bet in the meantime might be maintaining a fork until T6030 happens or more use cases for the hook show up.

@epriestley When merging task which is clearly speaking about something else (although related), please update the task title and description to contain the data from merged task so they won't get lost. Thank you.

pasik added a subscriber: pasik.May 12 2018, 1:57 PM