From 1cc8ca111bc0ea0b345ce8c544cb98f0b8cb74fc Mon Sep 17 00:00:00 2001 From: Jim O'Donnell Date: Wed, 17 Feb 2021 09:20:17 +0000 Subject: [PATCH] Classifier: refactor subject set store and allow for undefined workflows (#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. --- .../src/components/Classifier/Classifier.js | 14 +++++-- .../lib-classifier/src/store/RootStore.js | 2 + .../{Workflow => }/SubjectSet/SubjectSet.js | 2 +- .../SubjectSet/SubjectSet.spec.js | 0 .../store/{Workflow => }/SubjectSet/index.js | 0 .../SubjectSetStore/SubjectSetStore.js | 2 +- .../SubjectSetStore/SubjectSetStore.spec.js | 0 .../{Workflow => }/SubjectSetStore/index.js | 0 .../src/store/Workflow/Workflow.js | 14 ++++--- .../src/store/Workflow/Workflow.spec.js | 40 ++++++++----------- .../store/WorkflowStore/WorkflowStore.spec.js | 11 ++--- 11 files changed, 43 insertions(+), 42 deletions(-) rename packages/lib-classifier/src/store/{Workflow => }/SubjectSet/SubjectSet.js (87%) rename packages/lib-classifier/src/store/{Workflow => }/SubjectSet/SubjectSet.spec.js (100%) rename packages/lib-classifier/src/store/{Workflow => }/SubjectSet/index.js (100%) rename packages/lib-classifier/src/store/{Workflow => }/SubjectSetStore/SubjectSetStore.js (88%) rename packages/lib-classifier/src/store/{Workflow => }/SubjectSetStore/SubjectSetStore.spec.js (100%) rename packages/lib-classifier/src/store/{Workflow => }/SubjectSetStore/index.js (100%) diff --git a/packages/lib-classifier/src/components/Classifier/Classifier.js b/packages/lib-classifier/src/components/Classifier/Classifier.js index 9b65ffb02e..e36a647141 100644 --- 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 ( diff --git a/packages/lib-classifier/src/store/RootStore.js b/packages/lib-classifier/src/store/RootStore.js index c06c95b4e8..6de5604c31 100644 --- 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/packages/lib-classifier/src/store/Workflow/SubjectSet/SubjectSet.js b/packages/lib-classifier/src/store/SubjectSet/SubjectSet.js similarity index 87% rename from packages/lib-classifier/src/store/Workflow/SubjectSet/SubjectSet.js rename to packages/lib-classifier/src/store/SubjectSet/SubjectSet.js index 68e09a3e96..1a174953c0 100644 --- a/packages/lib-classifier/src/store/Workflow/SubjectSet/SubjectSet.js +++ b/packages/lib-classifier/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/packages/lib-classifier/src/store/Workflow/SubjectSet/SubjectSet.spec.js b/packages/lib-classifier/src/store/SubjectSet/SubjectSet.spec.js similarity index 100% rename from packages/lib-classifier/src/store/Workflow/SubjectSet/SubjectSet.spec.js rename to packages/lib-classifier/src/store/SubjectSet/SubjectSet.spec.js diff --git a/packages/lib-classifier/src/store/Workflow/SubjectSet/index.js b/packages/lib-classifier/src/store/SubjectSet/index.js similarity index 100% rename from packages/lib-classifier/src/store/Workflow/SubjectSet/index.js rename to packages/lib-classifier/src/store/SubjectSet/index.js diff --git a/packages/lib-classifier/src/store/Workflow/SubjectSetStore/SubjectSetStore.js b/packages/lib-classifier/src/store/SubjectSetStore/SubjectSetStore.js similarity index 88% rename from packages/lib-classifier/src/store/Workflow/SubjectSetStore/SubjectSetStore.js rename to packages/lib-classifier/src/store/SubjectSetStore/SubjectSetStore.js index 877e425410..b353c7148d 100644 --- a/packages/lib-classifier/src/store/Workflow/SubjectSetStore/SubjectSetStore.js +++ b/packages/lib-classifier/src/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/packages/lib-classifier/src/store/Workflow/SubjectSetStore/SubjectSetStore.spec.js b/packages/lib-classifier/src/store/SubjectSetStore/SubjectSetStore.spec.js similarity index 100% rename from packages/lib-classifier/src/store/Workflow/SubjectSetStore/SubjectSetStore.spec.js rename to packages/lib-classifier/src/store/SubjectSetStore/SubjectSetStore.spec.js diff --git a/packages/lib-classifier/src/store/Workflow/SubjectSetStore/index.js b/packages/lib-classifier/src/store/SubjectSetStore/index.js similarity index 100% rename from packages/lib-classifier/src/store/Workflow/SubjectSetStore/index.js rename to packages/lib-classifier/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 index 93589259b6..075aa063e9 100644 --- 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 index dbd51650e9..cb8e1395c2 100644 --- 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,36 +103,32 @@ 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) }) }) @@ -139,7 +136,7 @@ describe('Model > Workflow', 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 index d61ff084aa..e8261bbfcf 100644 --- 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) })