Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Engaging Crowds: Allow completed workflows to be fetched by ID #2580

Merged
merged 3 commits into from
Dec 14, 2021

Conversation

eatyourgreens
Copy link
Contributor

When a workflow ID is specified in a page URL, fetch that workflow even if it's been completed.

Try it out with a finished workflow from HMS NHS:
https://local.zooniverse.org:3000/projects/msalmon/hms-nhs-the-nautical-health-service/classify/workflow/18618?env=production

Workflow 7: Port Sailed Out Of, showing a list of completed subject sets to choose from.

This fixes a bug where returning to a completed workflow, eg. by bookmarking the URL, takes you to a blank page with an empty classifier, because the Classify page doesn't load completed workflows.
https://www.zooniverse.org/projects/msalmon/hms-nhs-the-nautical-health-service/classify/workflow/18618

Package:
app-project

Review Checklist

General

  • Are the tests passing locally and on Travis?
  • Is the documentation up to date?

Components

Apps

  • Does it work in all major browsers: Firefox, Chrome, Edge, Safari?
  • Does it work on mobile?
  • Can you yarn panic && yarn bootstrap or docker-compose up --build and app works as expected?

Publishing

  • Is the changelog updated?
  • Are the dependencies updated for apps and libraries that are using the newly published library?

Post-merging

@eatyourgreens eatyourgreens added the enhancement New feature or request label Dec 13, 2021
@eatyourgreens eatyourgreens requested a review from a team December 13, 2021 16:30
@eatyourgreens eatyourgreens force-pushed the fetch-completed-workflows branch from 2cea1ca to f50c48a Compare December 13, 2021 17:17
When a workflow ID is specified in a page URL, fetch that workflow even if it's been completed.
@eatyourgreens eatyourgreens force-pushed the fetch-completed-workflows branch from 96bd299 to 5534905 Compare December 14, 2021 12:11
Copy link
Member

@shaunanoordin shaunanoordin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Package: lib-classifier

This PR modifies the Classify page's behaviour in regards to 100% completed Workflows

Current behaviour: if WF 1234 is 100% classified (completed), accessing the WF URL (e.g. /projects/foo/bar/classify/workflow/1234) causes the workflow to not load properly.

  • for a grouped Workflow with selectable Subject Sets, instead of the SSet selection menu, the Classify page loads with no tasks and no Subjects.
  • ❔ (Unknown: not sure what happens for completed non-grouped Workflows, but presumably the same.)

New behaviour: if WF 1234 is 100% classified, accessing the WF URL causes the Workflow to load properly.

  • for a grouped Workflow, the SSet selection menu opens as expected.
  • ❔ (Untested: for completed non-grouped workflows, presumable the Classify page loads as normal.)

Consideration: is there any reason why we wouldn't want to allow users access to an active but completed workflow?
Answer: not really, no? There's no reason to not allow access to an active WF.

  • Our main concerns are 1. we don't allow bad classifications to be submitted (won't be the case here), and 2. we don't waste volunteer's time. (see below)
  • If a WF has selectable Subject Sets, the SSet selection menu will clearly show that all selectable subject sets are completed, so volunteers won't need to waste their time.
  • If a WF has no selectable SSets, the Classify interface shows immediately, and this will indicate that "this Subject is finished", again so volunteers won't need to waste their time.

Testing

Tested on localhost and on production, with the HMS NHS project, using macOS + Chrome 96

Dev Notes

  • Check out ADR 34 for notes on the removal of explicit default workflows.

Status

LGTM 👍

@@ -67,13 +84,34 @@ function orderWorkflows(workflows, order) {
return workflowsInOrder
}

async function fetchWorkflowsHelper(language = 'en', activeWorkflows, defaultWorkflow, workflowOrder = [], env) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making a note that we're removing the concept of explicit default workflows here, (i.e. project.default_workflow) which is in line with ADR 34

@github-actions github-actions bot added the approved This PR is approved for merging label Dec 14, 2021
@eatyourgreens eatyourgreens merged commit c094ae5 into master Dec 14, 2021
@eatyourgreens eatyourgreens deleted the fetch-completed-workflows branch December 14, 2021 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved for merging enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants