Description
Revisions and Commits
rP Phabricator | |||
D14299 | rP0e8ed0c61684 Desactivate subtask when logged out. |
Related Objects
- Mentioned In
- T9593: Counterexample header is incorrectly colored in email
- Mentioned Here
- T8227: Why not just add an option/setting/preference?
Event Timeline
I will purpose a diff too !
$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).
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 ?