Classifier: refactor subject set store and allow for undefined workfl…

…ows (#2040) * Check workflowID before selecting workflow After the classifier mounts, or updates, check for a workflow ID before running the selectWorkflow action. * Refactor the subject set store Subject sets can be shared across workflows, but identifiers canonly appear once in the state tree. This moves the subject set store up to the root store. Each set is stored in one place, then referenced by any workflow that uses it.
zooniverse · Feb 17, 2021 · 1cc8ca1 · 1cc8ca1
1 parent 78b017f
commit 1cc8ca1
Show file tree

Hide file tree

Showing 11 changed files with 43 additions and 42 deletions.
diff --git a/packages/lib-classifier/src/components/Classifier/Classifier.js b/packages/lib-classifier/src/components/Classifier/Classifier.js
@@ -66,7 +66,7 @@ export default class Classifier extends React.Component {
     this.classifierStore.classifications.setOnComplete(onCompleteClassification)
     this.classifierStore.setOnToggleFavourite(onToggleFavourite)
     if (workflowID) {
-      this.classifierStore.workflows.selectWorkflow(workflowID, subjectSetID)
+      this.selectWorkflow(workflowID, subjectSetID)
     }
   }
 
@@ -77,11 +77,11 @@ export default class Classifier extends React.Component {
     }
 
     if (workflowID !== prevProps.workflowID) {
-      this.classifierStore.workflows.selectWorkflow(workflowID, subjectSetID)
+      this.selectWorkflow(workflowID, subjectSetID)
     }
 
     if (subjectSetID !== prevProps.subjectSetID) {
-      this.classifierStore.workflows.selectWorkflow(workflowID, subjectSetID)
+      this.selectWorkflow(workflowID, subjectSetID)
     }
 
     if (authClient) {
@@ -94,6 +94,14 @@ export default class Classifier extends React.Component {
     this.classifierStore.projects.setActive(project.id)
   }
 
+  selectWorkflow() {
+    const { classifierStore } = this
+    const { subjectSetID, workflowID } = this.props
+    if (workflowID) {
+      classifierStore.workflows.selectWorkflow(workflowID, subjectSetID)
+    }
+  }
+
   render () {
     return (
       <Provider classifierStore={this.classifierStore}>

diff --git a/packages/lib-classifier/src/store/RootStore.js b/packages/lib-classifier/src/store/RootStore.js
@@ -14,6 +14,7 @@ import FeedbackStore from './FeedbackStore'
 import FieldGuideStore from './FieldGuideStore'
 import ProjectStore from './ProjectStore'
 import SubjectStore from './SubjectStore'
+import SubjectSetStore from './SubjectSetStore'
 import SubjectViewerStore from './SubjectViewerStore'
 import TutorialStore from './TutorialStore'
 import WorkflowStore from './WorkflowStore'
@@ -27,6 +28,7 @@ const RootStore = types
     fieldGuide: types.optional(FieldGuideStore, () => FieldGuideStore.create({})),
     projects: types.optional(ProjectStore, () => ProjectStore.create({})),
     subjects: types.optional(SubjectStore, () => SubjectStore.create({})),
+    subjectSets: types.optional(SubjectSetStore, () => SubjectSetStore.create({})),
     subjectViewer: types.optional(SubjectViewerStore, () => SubjectViewerStore.create({})),
     tutorials: types.optional(TutorialStore, () => TutorialStore.create({})),
     workflows: types.optional(WorkflowStore, () => WorkflowStore.create({})),

diff --git a/...c/store/Workflow/SubjectSet/SubjectSet.js → ...sifier/src/store/SubjectSet/SubjectSet.js b/...c/store/Workflow/SubjectSet/SubjectSet.js → ...sifier/src/store/SubjectSet/SubjectSet.js
@@ -1,5 +1,5 @@
 import { types } from 'mobx-state-tree'
-import Resource from '../../Resource'
+import Resource from '../Resource'
 
 const SubjectSet = types
   .model('SubjectSet', {

diff --git a/...re/Workflow/SubjectSet/SubjectSet.spec.js → ...r/src/store/SubjectSet/SubjectSet.spec.js b/...re/Workflow/SubjectSet/SubjectSet.spec.js → ...r/src/store/SubjectSet/SubjectSet.spec.js
diff --git a/...er/src/store/Workflow/SubjectSet/index.js → ...-classifier/src/store/SubjectSet/index.js b/...er/src/store/Workflow/SubjectSet/index.js → ...-classifier/src/store/SubjectSet/index.js
diff --git a/...rkflow/SubjectSetStore/SubjectSetStore.js → .../store/SubjectSetStore/SubjectSetStore.js b/...rkflow/SubjectSetStore/SubjectSetStore.js → .../store/SubjectSetStore/SubjectSetStore.js
@@ -1,6 +1,6 @@
 import { types } from 'mobx-state-tree'
 import SubjectSet from '../SubjectSet'
-import ResourceStore from '../../ResourceStore'
+import ResourceStore from '../ResourceStore'
 
 const SubjectSetStore = types
   .model('SubjectSetStore', {

diff --git a/...w/SubjectSetStore/SubjectSetStore.spec.js → ...e/SubjectSetStore/SubjectSetStore.spec.js b/...w/SubjectSetStore/SubjectSetStore.spec.js → ...e/SubjectSetStore/SubjectSetStore.spec.js
diff --git a/...c/store/Workflow/SubjectSetStore/index.js → ...sifier/src/store/SubjectSetStore/index.js b/...c/store/Workflow/SubjectSetStore/index.js → ...sifier/src/store/SubjectSetStore/index.js
diff --git a/packages/lib-classifier/src/store/Workflow/Workflow.js b/packages/lib-classifier/src/store/Workflow/Workflow.js
@@ -1,6 +1,6 @@
-import { flow, tryReference, types } from 'mobx-state-tree'
+import { flow, getRoot, tryReference, types } from 'mobx-state-tree'
 import Resource from '../Resource'
-import SubjectSetStore from './SubjectSetStore'
+import SubjectSet from '../SubjectSet'
 import WorkflowConfiguration from './WorkflowConfiguration'
 
 // The db type for steps is jsonb which is being serialized as an empty object when not defined.
@@ -25,14 +25,14 @@ const Workflow = types
     grouped: types.optional(types.boolean, false),
     links: types.frozen({}),
     steps: types.array(WorkflowStep),
-    subjectSets: types.optional(SubjectSetStore, () => SubjectSetStore.create({})),
+    subjectSet: types.safeReference(SubjectSet),
     tasks: types.maybe(types.frozen()),
     version: types.string
   })
 
   .views(self => ({
     get subjectSetId () {
-      const activeSet = tryReference(() => self.subjectSets.active)
+      const activeSet = tryReference(() => self.subjectSet)
       return activeSet?.id
     },
 
@@ -49,8 +49,10 @@ const Workflow = types
     function * selectSubjectSet(id) {
       const validSets = self.links.subject_sets || []
       if (validSets.indexOf(id) > -1) {
-        yield self.subjectSets.setActive(id)
-        return tryReference(() => self.subjectSets.active)
+        const { subjectSets } = getRoot(self)
+        const subjectSet = yield subjectSets.getResource(id)
+        self.subjectSet = subjectSet.id
+        return subjectSet
       }
       throw new Error(`No subject set ${id} for workflow ${self.id}`)
     }

diff --git a/packages/lib-classifier/src/store/Workflow/Workflow.spec.js b/packages/lib-classifier/src/store/Workflow/Workflow.spec.js
@@ -1,6 +1,7 @@
 import { WorkflowFactory } from '@test/factories'
 import { Factory } from 'rosie'
 import sinon from 'sinon'
+import RootStore from '../RootStore'
 import Workflow from './Workflow'
 
 describe('Model > Workflow', function () {
@@ -102,44 +103,40 @@ describe('Model > Workflow', function () {
   })
 
   describe('Actions > selectSubjectSet', function () {
+    let rootStore
     let workflow
 
     beforeEach(function () {
+      rootStore = RootStore.create();
       const subjectSets = Factory.buildList('subject_set', 5)
-      const subjectSetMap = {}
-      subjectSets.forEach(subjectSet => {
-        subjectSetMap[subjectSet.id] = subjectSet
-      })
+      rootStore.subjectSets.setResources(subjectSets)
       const workflowSnapshot = WorkflowFactory.build({
         id: 'workflow1',
         display_name: 'A test workflow',
         links: {
-          subject_sets: Object.keys(subjectSetMap)
-        },
-        subjectSets: {
-          resources: subjectSetMap
+          subject_sets: subjectSets.map(subjectSet => subjectSet.id)
         },
         version: '0.0'
       })
       workflow = Workflow.create(workflowSnapshot)
+      rootStore.workflows.setResources([workflow])
     })
 
     describe('with a valid subject set', function () {
 
-      it('should set the active subject set', async function () {
+      it('should select the subject set', async function () {
         expect(workflow.subjectSetId).to.be.undefined()
         const subjectSetID = workflow.links.subject_sets[1]
         const subjectSet = await workflow.selectSubjectSet(subjectSetID)
         expect(subjectSet.id).to.equal(subjectSetID)
-        expect(subjectSet).to.deep.equal(workflow.subjectSets.active)
       })
     })
 
     describe('with an invalid subject set', function () {
 
       it('should throw an error', async function () {
         let errorThrown = false
-        sinon.stub(workflow.subjectSets, 'fetchResource').callsFake(async () => undefined)
+        sinon.stub(rootStore.subjectSets, 'fetchResource').callsFake(async () => undefined)
         expect(workflow.subjectSetId).to.be.undefined()
         try {
           const subjectSet = await workflow.selectSubjectSet('abcdefg')
@@ -148,32 +145,29 @@ describe('Model > Workflow', function () {
           expect(e.message).to.equal('No subject set abcdefg for workflow workflow1')
         }
         expect(errorThrown).to.be.true()
-        workflow.subjectSets.fetchResource.restore()
+        rootStore.subjectSets.fetchResource.restore()
       })
     })
   })
 
   describe('Views > subjectSetId', function () {
+    let rootStore
     let workflow
 
     beforeEach(function () {
+      rootStore = RootStore.create();
       const subjectSets = Factory.buildList('subject_set', 5)
-      const subjectSetMap = {}
-      subjectSets.forEach(subjectSet => {
-        subjectSetMap[subjectSet.id] = subjectSet
-      })
+      rootStore.subjectSets.setResources(subjectSets)
       const workflowSnapshot = WorkflowFactory.build({
         id: 'workflow1',
         display_name: 'A test workflow',
         links: {
-          subject_sets: Object.keys(subjectSetMap)
-        },
-        subjectSets: {
-          resources: subjectSetMap
+          subject_sets: subjectSets.map(subjectSet => subjectSet.id)
         },
         version: '0.0'
       })
       workflow = Workflow.create(workflowSnapshot)
+      rootStore.workflows.setResources([workflow])
     })
 
     describe('with no selected subject set', function () {
@@ -185,7 +179,7 @@ describe('Model > Workflow', function () {
 
     describe('with a selected subject set', function () {
 
-      it('should return the active subject set', async function () {
+      it('should return the selected subject set ID', async function () {
         expect(workflow.subjectSetId).to.be.undefined()
         const subjectSetID = workflow.links.subject_sets[1]
         await workflow.selectSubjectSet(subjectSetID)
@@ -197,7 +191,7 @@ describe('Model > Workflow', function () {
 
       it('should error', async function () {
         let errorThrown = false
-        sinon.stub(workflow.subjectSets, 'fetchResource').callsFake(async () => undefined)
+        sinon.stub(rootStore.subjectSets, 'fetchResource').callsFake(async () => undefined)
         expect(workflow.subjectSetId).to.be.undefined()
         try {
           await workflow.selectSubjectSet('abcdefg')
@@ -206,7 +200,7 @@ describe('Model > Workflow', function () {
           expect(e.message).to.equal('No subject set abcdefg for workflow workflow1')
         }
         expect(errorThrown).to.be.true()
-        workflow.subjectSets.fetchResource.restore()
+        rootStore.subjectSets.fetchResource.restore()
       })
     })
   })

diff --git a/packages/lib-classifier/src/store/WorkflowStore/WorkflowStore.spec.js b/packages/lib-classifier/src/store/WorkflowStore/WorkflowStore.spec.js
@@ -25,6 +25,7 @@ describe('Model > WorkflowStore', function () {
       feedback: {},
       fieldGuide: {},
       subjects: {},
+      subjectSets: {},
       subjectViewer: {},
       tutorials: {},
       workflowSteps: {},
@@ -125,18 +126,11 @@ describe('Model > WorkflowStore', function () {
 
       before(async function () {
         const subjectSets = Factory.buildList('subject_set', 5)
-        const subjectSetMap = {}
-        subjectSets.forEach(subjectSet => {
-          subjectSetMap[subjectSet.id] = subjectSet
-        })
         const workflowSnapshot = WorkflowFactory.build({
           id: workflow.id,
           display_name: 'A test workflow',
           links: {
-            subject_sets: Object.keys(subjectSetMap)
-          },
-          subjectSets: {
-            resources: subjectSetMap
+            subject_sets: subjectSets.map(subjectSet => subjectSet.id)
           },
           version: '0.0'
         })
@@ -150,6 +144,7 @@ describe('Model > WorkflowStore', function () {
 
         rootStore = setupStores(panoptesClientStub, projectWithoutDefault)
         rootStore.workflows.reset()
+        rootStore.subjectSets.setResources(subjectSets)
         rootStore.workflows.setResources([workflowWithSubjectSets])
         await rootStore.workflows.selectWorkflow(workflowID, subjectSetID)
       })