Page MenuHomePhabricator

"Create subtask" is not greyed out for logged-out users
Closed, ResolvedPublic

Description

FireShot Capture 1 - ⚓ T9408 Upgrading_ `dot` (Graphviz) sup_ - https___secure.phabricator.com_T9408.png (237×200 px, 13 KB)

Revisions and Commits

Event Timeline

joshuaspence raised the priority of this task from to Needs Triage.
joshuaspence updated the task description. (Show Details)
joshuaspence added a project: Maniphest.
joshuaspence added a subscriber: joshuaspence.

I will purpose a diff too !

ManiphestTaskDetailController.php
$view->addAction(
      id(new PhabricatorActionView())
        ->setName(pht('Create Subtask'))
        ->setHref($this->getApplicationURI("/task/create/?parent={$id}"))
        ->setIcon('fa-level-down'));

should have a ->setDisabled(!$can_edit), @epriestley right.

Extra question : how does ->setWorflow(!$can_edit) : looking at source, I can't reeally found what this is for. I thing I have to include this as well ?

That fix is close, but probably not quite right, because a user might be able to create a subtask without being able to edit the original task.

For example, if I create a task with "Can Edit: epriestley", you are still allowed to create a subtask of it.

I think the actual check is just:

$can_create = $user->isLoggedIn();

setWorkflow() is used if we expect a dialog in the response instead of a normal web page. When you setWorkflow(true), the UI will use AJAX for the request if possible, so the dialog pops up on the same page instead of a different page. So you should do that here:

->setDisabled(!$can_create)
->setWorkflow(!$can_create)

...because when a user is logged out we expect clicking the button to show a "You can not take this action, log in to continue" dialog, but when the user is logged in we expect a full "Create Task" page.

Oki this is much more clear so Worflow here is a javelin workflow.
Greping workflow in phabricator get me so much different workflow type that i was lost.

Ok so I hava done what you advices me and im gonna submited this.
However to make it more clear, won't it be better to just add a new can_create policy.
I guess it should be able to say that admin are only willing "Moderator" members to create a new task but then the others users may comment it (I guess, you are probably gonna want that once Nuance is out).

maniphest_policy.png (414×552 px, 39 KB)

It thus add one new option thus I'm wondering if it falls under T8227: Why not just add an option/setting/preference?. Any thought @epriestley ?