zooniverse / front-end-monorepo

A rebuild of the front-end for zooniverse.org
https://www.zooniverse.org
Apache License 2.0
104 stars 30 forks source link

Subject set selection improperly displaying #3256

Closed snblickhan closed 2 years ago

snblickhan commented 2 years ago

FEM

Describe the bug

For workflow 21362 (set selection + sequential transcription) the correct subject sets aren't loading, or they're doing so sporadically. Often the wrong one will load (usually the previous set I had viewed), or the metadata won't display in the green banner, or I’ll get an Application error: a client-side exception has occurred (see the browser console for more information).

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://www.zooniverse.org/projects/pmlogan/poets-and-lovers
  2. Click on Journal Entries
  3. Choose a subject set
  4. Return to the subject set selection screen and choose a new subject set
  5. See what happens (based on testing this is where it shouldn't work properly)

Applicable Panoptes resource IDs (project, workflow, etc) to demonstrate the issue: Project 15597 Workflow 21362

Expected behavior

The correct subject set should load in the classifier each time it is selected from the menu.

Screenshots

If applicable, add screenshots to help explain your problem.

Screen Shot 2022-06-03 at 10 41 58 AM

Screen Shot 2022-06-03 at 10 42 36 AM

Screen Shot 2022-06-03 at 10 43 00 AM

sentry-io[bot] commented 2 years ago

Sentry issue: FEM-APP-PROJECT-1EG

snblickhan commented 2 years ago

Delilah noted on Slack that this is maybe related to https://github.com/zooniverse/front-end-monorepo/pull/2918#issuecomment-1068431604

goplayoutside3 commented 2 years ago

Poets & Lovers subject set selection works before commit https://github.com/zooniverse/front-end-monorepo/commit/ad72d0134822990e0a5d483af7489b0c4db4977d.

Also noting here that HMS NHS subject selection currently has the same bug when selecting a subject set within the same workflow. Visiting HMS NHS for the first time and selecting new subject sets in different workflows each time, does work as expected.

eatyourgreens commented 2 years ago

3239 may fix it, by restricting the translations effect hook to changing workflow.strings. At the moment, it overwrites the whole workflow.

eatyourgreens commented 2 years ago

3219 also removes some unnecessary rendering, which might affect the selected subject set ID.

eatyourgreens commented 2 years ago

I can reliably crash the classifier by loading this page directly (no need to go through choosing a workflow and set.) https://www.zooniverse.org/projects/pmlogan/poets-and-lovers/classify/workflow/21363/subject-set/103998

The error message tells us that the subject set ID has been set in the store (from the URL) before the subject set has loaded from the Panoptes API. So we get an invalid reference error (a reference pointing to a subject set that doesn't exist yet.)

Uncaught (in promise) Error: [mobx-state-tree] Failed to resolve reference '103998' to type 'SubjectSetResource' (from node: /workflows/resources/21363/subjectSet)

Deleting the fem-classifier-15997 key from session storage fixes the crash, so it's triggered by translation strings (#3145) but only in projects that use session storage, which is projects that have prioritized workflows. https://github.com/zooniverse/front-end-monorepo/blob/8310df5ae324f354560efab124d610dbf4c7d8d8/packages/app-project/src/screens/ClassifyPage/ClassifyPage.js#L34-L37

The error is thrown after this useEffect hook runs, so #3219 might fix it. I'm not sure. https://github.com/zooniverse/front-end-monorepo/blob/8310df5ae324f354560efab124d610dbf4c7d8d8/packages/lib-classifier/src/components/Classifier/Classifier.js#L64-L79

eatyourgreens commented 2 years ago

After testing in the dev classifier, with session caching turned on for all workflows, I can fix the bug by removing this line from the classifier. https://github.com/zooniverse/front-end-monorepo/blob/8310df5ae324f354560efab124d610dbf4c7d8d8/packages/lib-classifier/src/components/Classifier/Classifier.js#L59

That line remembers which subject set you're working on, for a project like Poets and Lovers. It's also removed in #3239. When I run Poets and Lovers in the dev classifier, on the branch for #3239 and with session caching turned on, I don't see this bug, so #3239 might fix it.

EDIT: #3219 removes that line too, so I'll also try that branch.

eatyourgreens commented 2 years ago

After checking out both #3239 and #3219, I've updated #3219 to include a fix for this.