Pages Editor: add rules to "Add Task to Page" functionality

shaunanoordin commented 2 months ago

PR Overview

Part of: Pages Editor MVP project and FEM Lab super-project Follows #7075 Staging branch URL: https://pr-7079.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging

This PR adds special rules for the "Add Task to Page" functionality. (See #7065)

The overall rule is this: a Step can only have 1 branching task (single answer question task)

(New behaviour) The specific sub-rules are:

if a Step has a branching task, it can't have any other tasks. Screenshot: The "Add New Task to Page" button is disabled when (and only when) there's a branching ('Single'-type) Task.
if a Step already has at least one task, any added Question task must be a Multiple Answer Question Task. Screenshot: no visible changes, but clicking 'Question' will add a 'Multiple'-type Task, instead of a 'Single'-type Task
if a Step already has many tasks, any Multiple Answer Question Task can't be transformed into a Single Answer Question Task. Screenshot: 'Allow multiple' checkboxes are disabled for all Question tasks on pages with > 1 tasks.

Other changes in this PR:

Multiple Answer Question Task (aka "Multiple" Task) has been added.
- It shares many same code as the Single Answer Question Task (aka "Single"-type Task)
[design] For non-branching Steps/Pages, the "Next Step/Page now appears OUTSIDE the grey box. Old vs New:
[minor] functions in TasksPage have been refactored with better safety checks (e.g. if workflow doesn't exist, don't bother running "delete Task from workflow")

FAQ

Q: how do I visually tell the difference between a Single Answer Question Task (aka "Single"-type Task) and a Multiple Answer Question Task (aka "Multiple"-type Task) on the Pages Editor? A: check whether the "Allow Multiple" checkbox is ticked. Also, "Single"-type Tasks allows you to choose the next page for each answer, not for the whole page.

"Single"-type Task:

"Multiple"-type Task:

Testing Steps

Go to the testing URL: https://pr-7065.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging
Click on "Reset" in the debug panel to start a workflow from scratch.
Add a new Page with 1 Question Task ("Single"-type).
- You should not be able to add any further Tasks to this Page.
Click on the "Allow Multiple" button to change the Single Answer Question Task into a Multiple Answer Question Task.
- You should now be able to add additional Tasks to this Page
Add a new Text Task to the page.
- The "allow multiple" checkbox of the Question Task should now be disabled/locked
Add a new Question Task to the page.
- The newly added Question Task should have its "allow multiple" checkbox ticked and disabled/locked. (i.e. you can't change it to a "Single"-type.)
Delete the Text Task and one Question Task.
- The "allow multiple" button of the remaining Question Task should now be enabled again.

It's also worth running some quick basic tests to make sure the refactor works as expected:

Start with an empty workflow.
Add Tasks with new Pages.
- Add Text Tasks,
- Add Single Answer Question Tasks, and make sure the answers branch to different Tasks.
Add Tasks into existing Pages.
Edit Tasks in Pages.
Delete Tasks from Pages.
- Delete all Tasks from a Page. This should delete the Page.
Delete Pages.
- Any "next page => deleted Page" linkages should revert to the default "next page => submit".

Status

Ready for review. 👌

EDIT: ready to review from a functional standpoint Improvements I could/should add to this PR:

UI icons/tooltips to tell users why certain buttons are disabled. 🤔

coveralls commented 2 months ago

coverage: 56.955% (-0.02%) from 56.977% when pulling de56b91aa9c825fc0ffca1ff7232ef5e571e65f3 on pages-editor-pt20 into 9ca8f70f0d6e836ca931168915c9aed02edf8b1d on master.

goplayoutside3 commented 2 months ago

Looking good! I'm able to follow all of the testing steps above at the preview link: https://pr-7079.pfe-preview.zooniverse.org/lab/1982/workflows/editor/3711?env=staging (note that your Testing Steps section links to an older PR's staging, but I figured it out!)

A couple of questions for clarification:

While all of the conditions listed in the Testing Steps work in the UI, what is the reasoning behind disallowing the combo of Single Answer Question + more tasks in the same step? I understand the logic behind disabling buttons when branching is involved, but I want to make sure I understand why only "a" is disallowed in the following step scenarios:
- a) Single Answer Question + other task ❌
- b) Multiple Answer Question + other task ✅
- c) Multiple Answer Question + Multiple Answer Question + other task, etc ✅
I'm willing to bet ya'll discussed the decision at length during Pages Editor meetings, so I apologize for asking for receipts 😆 but please point me to documentation if it already exists.
When I add only one task (and therefore only one step) to a workflow, the dropdown below the step or the dropdown below each answer includes T0. Project teams should not be able to recursively select T0 when there's only one task, is that correct?

goplayoutside3 commented 2 months ago

Revisiting my comment above because we talked through the reasoning behind "each single answer question gets its own page" during the weekly call. It was initially implemented this way to accommodate the fact that Single Answer Question tasks can branch, but Multiple Answer Question tasks cannot.

@shaunanoordin do you plan to update this PR with different rules for adding question tasks to their own page, or should I go ahead with review as is?

shaunanoordin commented 2 months ago

@goplayoutside3 Thanks Delilah! 👍

re: "each single answer question gets its own page", your assessment is on point. 👌

re: "should a Page's/Answer's Next Page be able loop back to itself?" (i.e. can T0 go to T0?) That's a good question. Currently, the PFE/FEM Lab Project Builder does allow a Task T0 to set its next task as Task T0. This is true whether it's a branching/single-type Task or a non-branching Task. Does it make sense? Not really. I'll add this to the list of questions to ask, but once we get confirmation, implementing a "you can't loop back in on yourself" rule is fairly trivial.

Caveat: while a one-step loop of T0->T0 doesn't make sense, there is a case for a two-or-more-steps loop of T0->T1->T0. e.g. T0:"Do you see any more colours that haven't been identified?" Yes=> go to T1, No => submit and T1: "Type in the name of the colour you see" => go to T0

Next Steps:

I won't push any further commits to this PR, unless there's a major bug that needs fixing.
Additional functional changes will be added in a separate PR.
- Notably, this will include "Updated 'Add New Task' rule - only one Question Task per page, you can have non-Question Tasks added" rule discussed on Monday.
Please go ahead with review as is. 👍 Thanks again!

eatyourgreens commented 2 months ago

Re. tasks linking to themselves, check out:

eatyourgreens commented 2 months ago

if a Step already has at least one task, any added Question task must be a Multiple Answer Question Task.

Sorry, just a question out of curiosity but does this stop you from building transcription workflows, which combine a drawing task and a yes/no question into a single step?

shaunanoordin commented 2 months ago

Sorry, just a question out of curiosity but does this stop you from building transcription workflows, which combine a drawing task and a yes/no question into a single step?

Transcription Tasks are whole different kettle of fish: they fall into something called "Pre-defined Pages" which is something we're planning for phase 2.

zooniverse / Panoptes-Front-End