Defer setting classify props until ready

Wait until canClassify is true before setting workflowID, subjectSetID or subjectID in the classifier. Otherwise, subject queries can be sent with incomplete URLs.
zooniverse · Jun 11, 2021 · 375f722 · 375f722
1 parent 72116f2
commit 375f722
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 3 deletions.
diff --git a/packages/app-project/src/screens/ClassifyPage/ClassifyPage.js b/packages/app-project/src/screens/ClassifyPage/ClassifyPage.js
@@ -43,6 +43,15 @@ function ClassifyPage ({
   // indexed subject sets require a subject
   canClassify = subjectSetFromUrl?.isIndexed ? !!subjectID : canClassify 
 
+  let classifierProps = {}
+  if (canClassify) {
+    classifierProps = {
+      workflowID,
+      subjectSetID,
+      subjectID
+    }
+  }
+
   return (
     <StandardLayout>
 
@@ -64,9 +73,7 @@ function ClassifyPage ({
             <ProjectName />
             <ClassifierWrapper
               onAddToCollection={addToCollection}
-              subjectID={subjectID}
-              subjectSetID={subjectSetID}
-              workflowID={workflowID}
+              {...classifierProps}
             />
             <ThemeModeToggle />
           </Grid>

diff --git a/packages/app-project/src/screens/ClassifyPage/ClassifyPage.spec.js b/packages/app-project/src/screens/ClassifyPage/ClassifyPage.spec.js
@@ -10,6 +10,9 @@ import YourStats from './components/YourStats'
 import ConnectWithProject from '@shared/components/ConnectWithProject'
 import ProjectStatistics from '@shared/components/ProjectStatistics'
 
+// TODO: is there a better way to find the dynamically imported classifier?
+const ClassifierDisplayName = 'ForwardRef(LoadableComponent)'
+
 describe('Component > ClassifyPage', function () {
   let wrapper
 
@@ -79,11 +82,17 @@ describe('Component > ClassifyPage', function () {
 
       before(function () {
         wrapper = shallow(<ClassifyPage workflowID='1234' workflows={workflows} />)
+        console.log(wrapper.debug())
       })
 
       it('should show a workflow menu', function () {
         expect(wrapper.find(WorkflowMenu)).to.have.lengthOf(1)
       })
+
+      it('should not pass the workflow ID to the classifier', function () {
+        const classifier = wrapper.find(ClassifierDisplayName)
+        expect(classifier.prop('workflowID')).to.be.undefined()
+      })
     })
 
     describe('with a subject set', function () {
@@ -100,6 +109,16 @@ describe('Component > ClassifyPage', function () {
       it('should not show a workflow menu', function () {
         expect(wrapper.find(WorkflowMenu)).to.have.lengthOf(0)
       })
+
+      it('should pass the workflow ID to the classifier', function () {
+        const classifier = wrapper.find(ClassifierDisplayName)
+        expect(classifier.prop('workflowID')).to.equal('1234')
+      })
+
+      it('should pass the subject set ID to the classifier', function () {
+        const classifier = wrapper.find(ClassifierDisplayName)
+        expect(classifier.prop('subjectSetID')).to.equal('3456')
+      })
     })
 
     describe('with an indexed subject set', function () {
@@ -129,6 +148,16 @@ describe('Component > ClassifyPage', function () {
         it('should show a workflow menu', function () {
           expect(wrapper.find(WorkflowMenu)).to.have.lengthOf(1)
         })
+
+        it('should not pass the workflow ID to the classifier', function () {
+          const classifier = wrapper.find(ClassifierDisplayName)
+          expect(classifier.prop('workflowID')).to.be.undefined()
+        })
+
+        it('should not pass the subject set ID to the classifier', function () {
+          const classifier = wrapper.find(ClassifierDisplayName)
+          expect(classifier.prop('subjectSetID')).to.be.undefined()
+        })
       })
 
       describe('with a subject', function () {
@@ -148,6 +177,21 @@ describe('Component > ClassifyPage', function () {
         it('should not show a workflow menu', function () {
           expect(wrapper.find(WorkflowMenu)).to.have.lengthOf(0)
         })
+
+        it('should pass the workflow ID to the classifier', function () {
+          const classifier = wrapper.find(ClassifierDisplayName)
+          expect(classifier.prop('workflowID')).to.equal('1234')
+        })
+
+        it('should pass the subject set ID to the classifier', function () {
+          const classifier = wrapper.find(ClassifierDisplayName)
+          expect(classifier.prop('subjectSetID')).to.equal('3456')
+        })
+
+        it('should pass the subject ID to the classifier', function () {
+          const classifier = wrapper.find(ClassifierDisplayName)
+          expect(classifier.prop('subjectID')).to.equal('5678')
+        })
       })
     })
   })