From b6a6348990c70751eb2a927159097bd586017fc7 Mon Sep 17 00:00:00 2001
From: mcbouslog <mcbouslog@gmail.com>
Date: Fri, 15 Mar 2019 00:19:16 -0500
Subject: [PATCH 01/15] Init add feedback modal

---
 .../components/Feedback/FeedbackModal.js      | 76 +++++++++++++++++++
 .../components/Feedback/FeedbackModal.spec.js | 16 ++++
 .../Classifier/components/Feedback/index.js   |  1 +
 .../components/Feedback/locales/en.json       |  6 ++
 .../components/DefaultLayout/DefaultLayout.js |  2 +
 .../TaskArea/components/Tasks/Tasks.js        |  2 -
 .../lib-classifier/src/store/FeedbackStore.js | 29 ++++++-
 .../src/store/FeedbackStore.spec.js           | 16 +++-
 .../lib-classifier/src/store/SubjectStore.js  |  9 ++-
 9 files changed, 150 insertions(+), 7 deletions(-)
 create mode 100644 packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
 create mode 100644 packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
 create mode 100644 packages/lib-classifier/src/components/Classifier/components/Feedback/index.js
 create mode 100644 packages/lib-classifier/src/components/Classifier/components/Feedback/locales/en.json

diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
new file mode 100644
index 0000000000..5480dab9be
--- /dev/null
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
@@ -0,0 +1,76 @@
+import React from 'react'
+import PropTypes from 'prop-types'
+import { inject, observer } from 'mobx-react'
+import { Button, Box } from 'grommet'
+import { Modal } from '@zooniverse/react-components'
+import counterpart from 'counterpart'
+import en from './locales/en'
+
+import SubjectViewer from '../SubjectViewer'
+
+counterpart.registerTranslations('en', en)
+
+function storeMapper(stores) {
+  const { 
+    hideFeedback,
+    messages,
+    showModal
+  } = stores.classifierStore.feedback
+  return {
+    hideFeedback,
+    messages,
+    showModal
+  }
+}
+
+@inject(storeMapper)
+@observer
+class FeedbackModal extends React.Component {
+  render () {
+    const label = counterpart('FeedbackModal.label')
+    const { hideFeedback, messages, showModal } = this.props
+
+    if (showModal) {
+      return (
+        <Modal
+          active={showModal}
+          closeFn={() => hideFeedback()}
+          title={label}
+        >
+          <>
+            <Box
+              height="medium"
+              overflow="auto"
+            >
+              <SubjectViewer />
+              <ul>
+                {messages.map(message =>
+                  <li key={Math.random()}>
+                    {message}
+                  </li>
+                )}
+              </ul>
+            </Box>
+            <Box pad={{ top: 'small' }}>
+              <Button
+                onClick={() => hideFeedback()}
+                label={counterpart('FeedbackModal.close')}
+                primary={true}
+              />
+            </Box>
+          </>
+        </Modal>
+      )
+    }
+
+    return null
+  }
+}
+
+FeedbackModal.wrappedComponent.propTypes = {
+  hideFeedback: PropTypes.func,
+  messages: PropTypes.arrayOf(PropTypes.string),
+  showModal: PropTypes.bool
+}
+
+export default FeedbackModal
diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
new file mode 100644
index 0000000000..38cf3994d1
--- /dev/null
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
@@ -0,0 +1,16 @@
+import React from 'react'
+import { shallow } from 'enzyme'
+import { expect } from 'chai'
+import FeedbackModal from './FeedbackModal'
+
+describe('FeedbackModal', function () {
+  it('should render without crashing', function () {
+    const wrapper = shallow(<FeedbackModal.wrappedComponent />)
+    expect(wrapper).to.be.ok
+  })
+
+  xit('should return null if showModal false', function () {
+    const wrapper = shallow(<FeedbackModal.wrappedComponent showModal={false} />)
+    expect(wrapper).to.equal(null)
+  })
+})
diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/index.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/index.js
new file mode 100644
index 0000000000..72ddfb2b11
--- /dev/null
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/index.js
@@ -0,0 +1 @@
+export { default } from './FeedbackModal'
diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/locales/en.json b/packages/lib-classifier/src/components/Classifier/components/Feedback/locales/en.json
new file mode 100644
index 0000000000..a6eb26a592
--- /dev/null
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/locales/en.json
@@ -0,0 +1,6 @@
+{
+  "FeedbackModal": {
+    "close": "Close",
+    "label": "Feedback" 
+  }
+}
\ No newline at end of file
diff --git a/packages/lib-classifier/src/components/Classifier/components/Layout/components/DefaultLayout/DefaultLayout.js b/packages/lib-classifier/src/components/Classifier/components/Layout/components/DefaultLayout/DefaultLayout.js
index c6bc0d2ae6..877c1f4d24 100644
--- a/packages/lib-classifier/src/components/Classifier/components/Layout/components/DefaultLayout/DefaultLayout.js
+++ b/packages/lib-classifier/src/components/Classifier/components/Layout/components/DefaultLayout/DefaultLayout.js
@@ -2,6 +2,7 @@ import React from 'react'
 import styled from 'styled-components'
 import { pxToRem } from '@zooniverse/react-components'
 
+import FeedbackModal from '../../../Feedback'
 import ImageToolbar from '../../../ImageToolbar'
 import MetaTools from '../../../MetaTools'
 import SubjectViewer from '../../../SubjectViewer'
@@ -56,6 +57,7 @@ function DefaultLayout () {
         <StyledMetaTools />
       </ViewerGrid>
       <StyledTaskArea />
+      <FeedbackModal />
     </ContainerGrid>
   )
 }
diff --git a/packages/lib-classifier/src/components/Classifier/components/TaskArea/components/Tasks/Tasks.js b/packages/lib-classifier/src/components/Classifier/components/TaskArea/components/Tasks/Tasks.js
index 6c220fb07e..7daa818d15 100644
--- a/packages/lib-classifier/src/components/Classifier/components/TaskArea/components/Tasks/Tasks.js
+++ b/packages/lib-classifier/src/components/Classifier/components/TaskArea/components/Tasks/Tasks.js
@@ -10,8 +10,6 @@ import TaskHelp from './components/TaskHelp'
 import { default as TaskNavButtons } from './components/TaskNavButtons'
 
 function storeMapper (stores) {
-  // TODO remove feedback store, added so FeedbackStore afterAttach would run during initial store development
-  const { isActive } = stores.classifierStore.feedback
   const { loadingState } = stores.classifierStore.workflows
   const { active: step } = stores.classifierStore.workflowSteps
   const tasks = stores.classifierStore.workflowSteps.activeStepTasks
diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 198386d347..2468ef36a5 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -7,8 +7,24 @@ import strategies from './feedback/strategies'
 const FeedbackStore = types
   .model('FeedbackStore', {
     isActive: types.optional(types.boolean, false),
-    rules: types.map(types.frozen({}))
+    rules: types.map(types.frozen({})),
+    showModal: types.optional(types.boolean, false)
   })
+  .views(self => ({
+    get messages () {
+      const messages = []
+      self.rules.forEach(([item]) => {
+        let message = false
+        if (item.success && item.successEnabled) {
+          message = item.successMessage
+        } else if (!item.success && item.failureEnabled) {
+          message = item.failureMessage
+        }
+        if (message) messages.push(message)
+      })
+      return messages
+    }
+  }))
   .actions(self => {
     function afterAttach () {
       createSubjectObserver()
@@ -51,6 +67,15 @@ const FeedbackStore = types
       }
     }
 
+    function showFeedback () {
+      self.showModal = true
+    }
+
+    function hideFeedback () {
+      self.showModal = false
+      getRoot(self).subjects.advance()
+    }
+
     function update (annotation) {
       const { task, value } = annotation
       const taskRules = self.rules.get(task) || []
@@ -69,6 +94,8 @@ const FeedbackStore = types
     return {
       afterAttach,
       createRules,
+      showFeedback,
+      hideFeedback,
       update,
       reset
     }
diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index ded9f6f33c..288de84842 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -3,7 +3,6 @@ import sinon from 'sinon'
 import FeedbackStore from './FeedbackStore'
 import strategies from './feedback/strategies'
 import helpers from './feedback/helpers'
-console.log(helpers)
 
 describe('Model > FeedbackStore', function () {
   let feedback
@@ -16,13 +15,14 @@ describe('Model > FeedbackStore', function () {
         T0: [{
           id: 'testRule',
           answer: '0',
-          strategy: "testStrategy",
+          strategy: 'testStrategy',
           successEnabled: true,
           successMessage: 'Yay!',
           failureEnabled: true,
           failureMessage: 'No!'
         }]
-      }
+      },
+      showModal: false
     }
     sinon.stub(helpers, 'isFeedbackActive').callsFake(() => feedbackStub.isActive)
     sinon.stub(helpers, 'generateRules').callsFake(() => feedbackStub.rules)
@@ -92,4 +92,14 @@ describe('Model > FeedbackStore', function () {
       expect(feedback.rules).to.be.empty
     })
   })
+
+  describe('messages', function () {
+    xit('should return an array of feedback messages', function () {
+      expect(feedback.messages).to.equal(['Yay!'])
+    })
+
+    xit('should return an empty array if no messages applicable', function () {
+      expect(feedback.messages).to.equal([])
+    })
+  })
 })
diff --git a/packages/lib-classifier/src/store/SubjectStore.js b/packages/lib-classifier/src/store/SubjectStore.js
index c43b47720e..bfd0a6caed 100644
--- a/packages/lib-classifier/src/store/SubjectStore.js
+++ b/packages/lib-classifier/src/store/SubjectStore.js
@@ -59,7 +59,14 @@ const SubjectStore = types
       const classificationDisposer = autorun(() => {
         onPatch(getRoot(self), (patch) => {
           const { path, value } = patch
-          if (path === '/classifications/loadingState' && value === 'posting') self.advance()
+          if (path === '/classifications/loadingState' && value === 'posting') {
+            const feedback = getRoot(self).feedback
+            if (feedback.isActive && feedback.messages.length > 0) {
+              feedback.showFeedback()
+            } else {
+              self.advance()
+            }
+          }
         })
       })
       addDisposer(self, classificationDisposer)

From 1fa0c8234b9ac35a2d1b63cf4d10ed4743346acb Mon Sep 17 00:00:00 2001
From: mcbouslog <mcbouslog@gmail.com>
Date: Mon, 18 Mar 2019 01:26:59 -0500
Subject: [PATCH 02/15] Add hideSubjectViewer functionality

---
 .../components/Feedback/FeedbackModal.js      |  6 ++++--
 .../components/Feedback/FeedbackModal.spec.js |  2 +-
 .../lib-classifier/src/store/FeedbackStore.js | 19 ++++++++++---------
 .../src/store/FeedbackStore.spec.js           | 16 ++++++++++++++--
 4 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
index 5480dab9be..24721ad8c6 100644
--- a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
@@ -13,11 +13,13 @@ counterpart.registerTranslations('en', en)
 function storeMapper(stores) {
   const { 
     hideFeedback,
+    hideSubjectViewer,
     messages,
     showModal
   } = stores.classifierStore.feedback
   return {
     hideFeedback,
+    hideSubjectViewer,
     messages,
     showModal
   }
@@ -28,7 +30,7 @@ function storeMapper(stores) {
 class FeedbackModal extends React.Component {
   render () {
     const label = counterpart('FeedbackModal.label')
-    const { hideFeedback, messages, showModal } = this.props
+    const { hideFeedback, hideSubjectViewer, messages, showModal } = this.props
 
     if (showModal) {
       return (
@@ -42,7 +44,7 @@ class FeedbackModal extends React.Component {
               height="medium"
               overflow="auto"
             >
-              <SubjectViewer />
+              {!hideSubjectViewer && <SubjectViewer />}
               <ul>
                 {messages.map(message =>
                   <li key={Math.random()}>
diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
index 38cf3994d1..3a7b9016ce 100644
--- a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
@@ -9,7 +9,7 @@ describe('FeedbackModal', function () {
     expect(wrapper).to.be.ok
   })
 
-  xit('should return null if showModal false', function () {
+  it('should return null if showModal false', function () {
     const wrapper = shallow(<FeedbackModal.wrappedComponent showModal={false} />)
     expect(wrapper).to.equal(null)
   })
diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 2468ef36a5..8d058fc332 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -11,18 +11,19 @@ const FeedbackStore = types
     showModal: types.optional(types.boolean, false)
   })
   .views(self => ({
+    get hideSubjectViewer () {
+      return flatten(Array.from(self.rules.values()))
+        .some(rule => rule.hideSubjectViewer)
+    },
     get messages () {
-      const messages = []
-      self.rules.forEach(([item]) => {
-        let message = false
-        if (item.success && item.successEnabled) {
-          message = item.successMessage
-        } else if (!item.success && item.failureEnabled) {
-          message = item.failureMessage
+      return flatten(Array.from(self.rules.values()))
+        .map(rule => {
+          if (rule.success && rule.successEnabled) {
+            return rule.successMessage
+          } else if (!rule.success && rule.failureEnabled) {
+            return rule.failureMessage
         }
-        if (message) messages.push(message)
       })
-      return messages
     }
   }))
   .actions(self => {
diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index 288de84842..f4e7984e9a 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -94,12 +94,24 @@ describe('Model > FeedbackStore', function () {
   })
 
   describe('messages', function () {
-    xit('should return an array of feedback messages', function () {
+    // TODO set feedback.rules['T0'] to array of 2 rules, both successEnabled => true, one failureEnabled => false and test lengths of returned messages array
+    
+    it('should return an array of success feedback messages', function () {
       expect(feedback.messages).to.equal(['Yay!'])
     })
 
-    xit('should return an empty array if no messages applicable', function () {
+    it('should return an array of failure messages', function () {
       expect(feedback.messages).to.equal([])
     })
   })
+
+  describe('hideSubjectViewer', function () {
+    it('should return true if any rule hides subject viewer', function () {
+      expect(feedback.hideSubjectViewer).to.equal(true)
+    })
+
+    it('should return false if no rule hides subject viewer', function () {
+      expect(feedback.hideSubjectViewer).to.equal(false)
+    })
+  })
 })

From ea9b70a4e31b63029a6aa82a07c52cfcfbdecd9c Mon Sep 17 00:00:00 2001
From: mcbouslog <mcbouslog@gmail.com>
Date: Mon, 18 Mar 2019 01:27:41 -0500
Subject: [PATCH 03/15] Refactor to addMiddleware for subject advance

---
 .../lib-classifier/src/store/FeedbackStore.js | 55 ++++++++++++-------
 .../src/store/FeedbackStore.spec.js           | 31 +++++++++++
 .../lib-classifier/src/store/SubjectStore.js  |  9 +--
 3 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 8d058fc332..2feb0475a8 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -1,5 +1,6 @@
 import { autorun } from 'mobx'
-import { addDisposer, getRoot, onAction, types } from 'mobx-state-tree'
+import { addDisposer, addMiddleware, getRoot, onAction, types } from 'mobx-state-tree'
+import { flatten } from 'lodash'
 
 import helpers from './feedback/helpers'
 import strategies from './feedback/strategies'
@@ -8,7 +9,8 @@ const FeedbackStore = types
   .model('FeedbackStore', {
     isActive: types.optional(types.boolean, false),
     rules: types.map(types.frozen({})),
-    showModal: types.optional(types.boolean, false)
+    showModal: types.optional(types.boolean, false),
+    allowSubjectAdvance: types.optional(types.boolean, false)
   })
   .views(self => ({
     get hideSubjectViewer () {
@@ -22,25 +24,15 @@ const FeedbackStore = types
             return rule.successMessage
           } else if (!rule.success && rule.failureEnabled) {
             return rule.failureMessage
-        }
-      })
+          }
+        })
     }
   }))
   .actions(self => {
     function afterAttach () {
-      createSubjectObserver()
       createClassificationObserver()
-    }
-
-    function createSubjectObserver () {
-      const subjectDisposer = autorun(() => {
-        const subject = getRoot(self).subjects.active
-        if (subject) {
-          self.reset()
-          self.createRules(subject)
-        }
-      })
-      addDisposer(self, subjectDisposer)
+      createSubjectMiddleware()
+      createSubjectObserver()
     }
 
     function createClassificationObserver () {
@@ -48,15 +40,37 @@ const FeedbackStore = types
         onAction(getRoot(self).classifications, (call) => {
           if (call.name === 'completeClassification') {
             const annotations = getRoot(self).classifications.currentAnnotations
-            for (const value of annotations.values()) {
-              self.update(value)
-            }
+            annotations.forEach(annotation => self.update(annotation))
           }
         })
       })
       addDisposer(self, classificationDisposer)
     }
 
+    function createSubjectMiddleware () {
+      const subjectMiddleware = autorun(() => {
+        addMiddleware(getRoot(self).subjects, (call, next, abort) => {
+          if (call.name === 'advance' && self.isActive && self.messages.length && !self.allowSubjectAdvance) {
+            self.showFeedback()
+            return abort()
+          }
+          next(call)
+        })
+      })
+      addDisposer(self, subjectMiddleware)
+    }
+
+    function createSubjectObserver () {
+      const subjectDisposer = autorun(() => {
+        const subject = getRoot(self).subjects.active
+        if (subject) {
+          self.reset()
+          self.createRules(subject)
+        }
+      })
+      addDisposer(self, subjectDisposer)
+    }
+
     function createRules (subject) {
       const project = getRoot(self).projects.active
       const workflow = getRoot(self).workflows.active
@@ -74,6 +88,7 @@ const FeedbackStore = types
 
     function hideFeedback () {
       self.showModal = false
+      self.allowSubjectAdvance = true
       getRoot(self).subjects.advance()
     }
 
@@ -90,6 +105,8 @@ const FeedbackStore = types
     function reset () {
       self.isActive = false
       self.rules.clear()
+      self.showModal = false
+      self.allowSubjectAdvance = false
     }
 
     return {
diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index f4e7984e9a..78e1ccc979 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -91,8 +91,39 @@ describe('Model > FeedbackStore', function () {
     it('should reset feedback rules', function () {
       expect(feedback.rules).to.be.empty
     })
+
+    it('should reset showModal state', function () {
+      expect(feedback.showModal).to.be.false
+    })
+
+    it('should reset allowSubjectAdvance state', function () {
+      expect(feedback.allowSubjectAdvance).to.be.false
+    })
+  })
+
+  describe('showFeedback', function () {
+    it('should set showModal state to true', function () {
+      feedback.showFeedback()
+      expect(feedback.showModal).to.be.true
+    })
   })
 
+  describe('hideFeedback', function () {
+    // TODO stub getRoot(self).subjects.advance()
+
+    // before(function () {
+    //   feedback.hideFeedback()
+    // })
+
+    it('should set showModal state to false', function () {
+      expect(feedback.showModal).to.be.false
+    })
+
+    it('should set allowSubjectAdvance state to true', function () {
+      expect(feedback.allowSubjectAdvance).to.be.true
+    })
+  })
+  
   describe('messages', function () {
     // TODO set feedback.rules['T0'] to array of 2 rules, both successEnabled => true, one failureEnabled => false and test lengths of returned messages array
     
diff --git a/packages/lib-classifier/src/store/SubjectStore.js b/packages/lib-classifier/src/store/SubjectStore.js
index bfd0a6caed..c43b47720e 100644
--- a/packages/lib-classifier/src/store/SubjectStore.js
+++ b/packages/lib-classifier/src/store/SubjectStore.js
@@ -59,14 +59,7 @@ const SubjectStore = types
       const classificationDisposer = autorun(() => {
         onPatch(getRoot(self), (patch) => {
           const { path, value } = patch
-          if (path === '/classifications/loadingState' && value === 'posting') {
-            const feedback = getRoot(self).feedback
-            if (feedback.isActive && feedback.messages.length > 0) {
-              feedback.showFeedback()
-            } else {
-              self.advance()
-            }
-          }
+          if (path === '/classifications/loadingState' && value === 'posting') self.advance()
         })
       })
       addDisposer(self, classificationDisposer)

From f404e28a04f8db96c0fa7a63f81d69a03cfa9420 Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Mon, 18 Mar 2019 14:59:20 +0000
Subject: [PATCH 04/15] Update tests Set up a fresh feedback store before each
 test. Update tests to reset initial state correctly between tests.

---
 .../src/store/FeedbackStore.spec.js           | 46 ++++++++++++-------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index 78e1ccc979..5194b27764 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -9,6 +9,14 @@ describe('Model > FeedbackStore', function () {
   let feedbackStub
 
   before(function () {
+    sinon.stub(helpers, 'isFeedbackActive').callsFake(() => feedbackStub.isActive)
+    sinon.stub(helpers, 'generateRules').callsFake(() => feedbackStub.rules)
+    strategies.testStrategy = {
+      reducer: sinon.stub().callsFake(rule => rule)
+    }
+  })
+  
+  beforeEach(function () {
     feedbackStub = {
       isActive: true,
       rules: {
@@ -16,19 +24,25 @@ describe('Model > FeedbackStore', function () {
           id: 'testRule',
           answer: '0',
           strategy: 'testStrategy',
+          success: true,
           successEnabled: true,
           successMessage: 'Yay!',
           failureEnabled: true,
           failureMessage: 'No!'
+        }],
+        T1: [{
+          id: 'testRule',
+          answer: '0',
+          strategy: 'testStrategy',
+          success: false,
+          successEnabled: true,
+          successMessage: 'Yippee!',
+          failureEnabled: true,
+          failureMessage: 'Nope!'
         }]
       },
       showModal: false
     }
-    sinon.stub(helpers, 'isFeedbackActive').callsFake(() => feedbackStub.isActive)
-    sinon.stub(helpers, 'generateRules').callsFake(() => feedbackStub.rules)
-    strategies.testStrategy = {
-      reducer: sinon.stub().callsFake(rule => rule)
-    }
     feedback = FeedbackStore.create(feedbackStub)
   })
 
@@ -48,7 +62,7 @@ describe('Model > FeedbackStore', function () {
       id: '3'
     }
 
-    before(function () {
+    beforeEach(function () {
       feedback.projects = {
         active: project
       }
@@ -58,6 +72,11 @@ describe('Model > FeedbackStore', function () {
       feedback.createRules(subject)
     })
 
+    afterEach(function () {
+      helpers.isFeedbackActive.resetHistory()
+      helpers.generateRules.resetHistory()
+    })
+
     it('should set active state', function () {
       expect(helpers.isFeedbackActive).to.have.been.calledOnceWith(project, subject, workflow)
     })
@@ -68,7 +87,7 @@ describe('Model > FeedbackStore', function () {
   })
 
   describe('update', function () {
-    before(function () {
+    beforeEach(function () {
       const annotation = { task: 'T0', value: 0 }
       feedback.update(annotation)
     })
@@ -80,7 +99,7 @@ describe('Model > FeedbackStore', function () {
   })
 
   describe('reset', function () {
-    before(function () {
+    beforeEach(function () {
       feedback.reset()
     })
 
@@ -89,7 +108,7 @@ describe('Model > FeedbackStore', function () {
     })
 
     it('should reset feedback rules', function () {
-      expect(feedback.rules).to.be.empty
+      expect(feedback.rules.toJSON()).to.be.empty
     })
 
     it('should reset showModal state', function () {
@@ -125,14 +144,9 @@ describe('Model > FeedbackStore', function () {
   })
   
   describe('messages', function () {
-    // TODO set feedback.rules['T0'] to array of 2 rules, both successEnabled => true, one failureEnabled => false and test lengths of returned messages array
     
-    it('should return an array of success feedback messages', function () {
-      expect(feedback.messages).to.equal(['Yay!'])
-    })
-
-    it('should return an array of failure messages', function () {
-      expect(feedback.messages).to.equal([])
+    it('should return an array of feedback messages', function () {
+      expect(feedback.messages).to.eql(['Yay!', 'Nope!'])
     })
   })
 

From 4bcf0ab4839137b38e7ac1bf5a6e2f683a4d1d5a Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Mon, 18 Mar 2019 15:08:17 +0000
Subject: [PATCH 05/15] Add hideSubjectViewer to test rules Update tests to use
 the rules map too.

---
 packages/lib-classifier/src/store/FeedbackStore.spec.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index 5194b27764..d3a27a0022 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -22,6 +22,7 @@ describe('Model > FeedbackStore', function () {
       rules: {
         T0: [{
           id: 'testRule',
+          hideSubjectViewer: true,
           answer: '0',
           strategy: 'testStrategy',
           success: true,
@@ -32,6 +33,7 @@ describe('Model > FeedbackStore', function () {
         }],
         T1: [{
           id: 'testRule',
+          hideSubjectViewer: false,
           answer: '0',
           strategy: 'testStrategy',
           success: false,
@@ -156,6 +158,8 @@ describe('Model > FeedbackStore', function () {
     })
 
     it('should return false if no rule hides subject viewer', function () {
+      const [ rule ] = feedback.rules.get('T0')
+      rule.hideSubjectViewer = false
       expect(feedback.hideSubjectViewer).to.equal(false)
     })
   })

From f2fb12f8afb538ba5d8d191c3cae8d905ca443b7 Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Mon, 18 Mar 2019 15:15:54 +0000
Subject: [PATCH 06/15] Test hideFeedback Stub out root.subjects before each
 test.

---
 .../lib-classifier/src/store/FeedbackStore.spec.js    | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index d3a27a0022..a951449ab6 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -130,11 +130,14 @@ describe('Model > FeedbackStore', function () {
   })
 
   describe('hideFeedback', function () {
-    // TODO stub getRoot(self).subjects.advance()
+    
 
-    // before(function () {
-    //   feedback.hideFeedback()
-    // })
+    beforeEach(function () {
+      feedback.subjects = {
+        advance: () => true
+      }
+      feedback.hideFeedback()
+    })
 
     it('should set showModal state to false', function () {
       expect(feedback.showModal).to.be.false

From a350fd47af1a1c188511f6e196440b10b48e4b12 Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Mon, 18 Mar 2019 15:21:27 +0000
Subject: [PATCH 07/15] Fix feedback modal tests Expect the rendered HTML to be
 null when the component is hidden.

---
 .../Classifier/components/Feedback/FeedbackModal.spec.js      | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
index 3a7b9016ce..8766e3953d 100644
--- a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
@@ -9,8 +9,8 @@ describe('FeedbackModal', function () {
     expect(wrapper).to.be.ok
   })
 
-  it('should return null if showModal false', function () {
+  it('should not render if showModal false', function () {
     const wrapper = shallow(<FeedbackModal.wrappedComponent showModal={false} />)
-    expect(wrapper).to.equal(null)
+    expect(wrapper.html()).to.be.null
   })
 })

From dd79a1f0a8ced893a8cf527baa15e07cd19a4f68 Mon Sep 17 00:00:00 2001
From: mcbouslog <mcbouslog@gmail.com>
Date: Tue, 19 Mar 2019 02:49:34 -0500
Subject: [PATCH 08/15] Feedback modal cleanup

---
 .../components/Feedback/FeedbackModal.js          |  4 ++--
 .../components/Feedback/FeedbackModal.spec.js     | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
index 24721ad8c6..53ab90da08 100644
--- a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
@@ -36,7 +36,7 @@ class FeedbackModal extends React.Component {
       return (
         <Modal
           active={showModal}
-          closeFn={() => hideFeedback()}
+          closeFn={hideFeedback}
           title={label}
         >
           <>
@@ -55,7 +55,7 @@ class FeedbackModal extends React.Component {
             </Box>
             <Box pad={{ top: 'small' }}>
               <Button
-                onClick={() => hideFeedback()}
+                onClick={hideFeedback}
                 label={counterpart('FeedbackModal.close')}
                 primary={true}
               />
diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
index 8766e3953d..c010084ca3 100644
--- a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.spec.js
@@ -1,6 +1,8 @@
 import React from 'react'
 import { shallow } from 'enzyme'
 import { expect } from 'chai'
+import sinon from 'sinon'
+import { Button } from 'grommet'
 import FeedbackModal from './FeedbackModal'
 
 describe('FeedbackModal', function () {
@@ -13,4 +15,17 @@ describe('FeedbackModal', function () {
     const wrapper = shallow(<FeedbackModal.wrappedComponent showModal={false} />)
     expect(wrapper.html()).to.be.null
   })
+
+  it('should show messages', function () {
+    const wrapper = shallow(<FeedbackModal.wrappedComponent showModal messages={['Yay!', 'Good Job', 'Try Again']} />)
+    const list = wrapper.find('li')
+    expect(list).to.have.lengthOf(3)
+  })
+
+  it('should call hideFeedback on close', function () {
+    const hideFeedbackStub = sinon.stub()
+    const wrapper = shallow(<FeedbackModal.wrappedComponent showModal messages={['Yay!', 'Good Job', 'Try Again']} hideFeedback={hideFeedbackStub} />)
+    wrapper.find(Button).simulate('click')
+    expect(hideFeedbackStub).to.have.been.called
+  })
 })

From e3ea7d6c877cadbd2bc4e17723a1f1917032e3e1 Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Tue, 19 Mar 2019 12:52:48 +0000
Subject: [PATCH 09/15] Remove allowSubjectAdvance allowSubjectAdvance just
 seems to track showModal, so try using that instead.

---
 packages/lib-classifier/src/store/FeedbackStore.js      | 9 +++------
 packages/lib-classifier/src/store/FeedbackStore.spec.js | 8 --------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 2feb0475a8..0629c3aaa2 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -9,8 +9,7 @@ const FeedbackStore = types
   .model('FeedbackStore', {
     isActive: types.optional(types.boolean, false),
     rules: types.map(types.frozen({})),
-    showModal: types.optional(types.boolean, false),
-    allowSubjectAdvance: types.optional(types.boolean, false)
+    showModal: types.optional(types.boolean, false)
   })
   .views(self => ({
     get hideSubjectViewer () {
@@ -50,7 +49,7 @@ const FeedbackStore = types
     function createSubjectMiddleware () {
       const subjectMiddleware = autorun(() => {
         addMiddleware(getRoot(self).subjects, (call, next, abort) => {
-          if (call.name === 'advance' && self.isActive && self.messages.length && !self.allowSubjectAdvance) {
+          if (call.name === 'advance' && self.isActive && self.messages.length && !self.showModal) {
             self.showFeedback()
             return abort()
           }
@@ -87,9 +86,8 @@ const FeedbackStore = types
     }
 
     function hideFeedback () {
-      self.showModal = false
-      self.allowSubjectAdvance = true
       getRoot(self).subjects.advance()
+      self.showModal = false
     }
 
     function update (annotation) {
@@ -106,7 +104,6 @@ const FeedbackStore = types
       self.isActive = false
       self.rules.clear()
       self.showModal = false
-      self.allowSubjectAdvance = false
     }
 
     return {
diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index a951449ab6..d5e95484d6 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -116,10 +116,6 @@ describe('Model > FeedbackStore', function () {
     it('should reset showModal state', function () {
       expect(feedback.showModal).to.be.false
     })
-
-    it('should reset allowSubjectAdvance state', function () {
-      expect(feedback.allowSubjectAdvance).to.be.false
-    })
   })
 
   describe('showFeedback', function () {
@@ -142,10 +138,6 @@ describe('Model > FeedbackStore', function () {
     it('should set showModal state to false', function () {
       expect(feedback.showModal).to.be.false
     })
-
-    it('should set allowSubjectAdvance state to true', function () {
-      expect(feedback.allowSubjectAdvance).to.be.true
-    })
   })
   
   describe('messages', function () {

From 38123703b3ac917b19d99b121165be72b9a1d880 Mon Sep 17 00:00:00 2001
From: mcbouslog <mcbouslog@gmail.com>
Date: Tue, 19 Mar 2019 08:51:31 -0500
Subject: [PATCH 10/15] Add Boolean filter to feedback messages

---
 packages/lib-classifier/src/store/FeedbackStore.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 0629c3aaa2..8e4c1db0a8 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -24,7 +24,7 @@ const FeedbackStore = types
           } else if (!rule.success && rule.failureEnabled) {
             return rule.failureMessage
           }
-        })
+        }).filter(Boolean)
     }
   }))
   .actions(self => {

From fb1f30d1bca1d3341b730eb12a63c8d70f973e4a Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Wed, 20 Mar 2019 12:21:25 +0000
Subject: [PATCH 11/15] Add onHide callback Add a callback that's invoked when
 the modal closes. Set it to subjects.advance when we open the modal.

---
 .../lib-classifier/src/store/FeedbackStore.js   | 17 ++++++++++++++---
 .../src/store/FeedbackStore.spec.js             | 12 +++++++-----
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 8e4c1db0a8..05130c7006 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -11,6 +11,9 @@ const FeedbackStore = types
     rules: types.map(types.frozen({})),
     showModal: types.optional(types.boolean, false)
   })
+  .volatile(self => ({
+    onHide: () => true
+  }))
   .views(self => ({
     get hideSubjectViewer () {
       return flatten(Array.from(self.rules.values()))
@@ -28,6 +31,10 @@ const FeedbackStore = types
     }
   }))
   .actions(self => {
+    function setOnHide (onHide) {
+      self.onHide = onHide
+    }
+
     function afterAttach () {
       createClassificationObserver()
       createSubjectMiddleware()
@@ -50,10 +57,13 @@ const FeedbackStore = types
       const subjectMiddleware = autorun(() => {
         addMiddleware(getRoot(self).subjects, (call, next, abort) => {
           if (call.name === 'advance' && self.isActive && self.messages.length && !self.showModal) {
+            abort()
+            const onHide = getRoot(self).subjects.advance
+            self.setOnHide(onHide)
             self.showFeedback()
-            return abort()
+          } else {
+            next(call)
           }
-          next(call)
         })
       })
       addDisposer(self, subjectMiddleware)
@@ -86,7 +96,7 @@ const FeedbackStore = types
     }
 
     function hideFeedback () {
-      getRoot(self).subjects.advance()
+      self.onHide()
       self.showModal = false
     }
 
@@ -109,6 +119,7 @@ const FeedbackStore = types
     return {
       afterAttach,
       createRules,
+      setOnHide,
       showFeedback,
       hideFeedback,
       update,
diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index d5e95484d6..bbc70ea32c 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -15,7 +15,7 @@ describe('Model > FeedbackStore', function () {
       reducer: sinon.stub().callsFake(rule => rule)
     }
   })
-  
+
   beforeEach(function () {
     feedbackStub = {
       isActive: true,
@@ -126,22 +126,24 @@ describe('Model > FeedbackStore', function () {
   })
 
   describe('hideFeedback', function () {
-    
-
     beforeEach(function () {
       feedback.subjects = {
         advance: () => true
       }
+      feedback.setOnHide(sinon.stub())
       feedback.hideFeedback()
     })
 
     it('should set showModal state to false', function () {
       expect(feedback.showModal).to.be.false
     })
+
+    it('should call the onHide callback', function () {
+      expect(feedback.onHide).to.have.been.calledOnce
+    })
   })
-  
+
   describe('messages', function () {
-    
     it('should return an array of feedback messages', function () {
       expect(feedback.messages).to.eql(['Yay!', 'Nope!'])
     })

From c4af485d867fe9c86fb7b4a959c4a5df75e067c3 Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Wed, 20 Mar 2019 12:30:03 +0000
Subject: [PATCH 12/15] lint the modal

---
 .../Classifier/components/Feedback/FeedbackModal.js    | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
index 53ab90da08..7f28a35a09 100644
--- a/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
+++ b/packages/lib-classifier/src/components/Classifier/components/Feedback/FeedbackModal.js
@@ -10,8 +10,8 @@ import SubjectViewer from '../SubjectViewer'
 
 counterpart.registerTranslations('en', en)
 
-function storeMapper(stores) {
-  const { 
+function storeMapper (stores) {
+  const {
     hideFeedback,
     hideSubjectViewer,
     messages,
@@ -41,8 +41,8 @@ class FeedbackModal extends React.Component {
         >
           <>
             <Box
-              height="medium"
-              overflow="auto"
+              height='medium'
+              overflow='auto'
             >
               {!hideSubjectViewer && <SubjectViewer />}
               <ul>
@@ -57,7 +57,7 @@ class FeedbackModal extends React.Component {
               <Button
                 onClick={hideFeedback}
                 label={counterpart('FeedbackModal.close')}
-                primary={true}
+                primary
               />
             </Box>
           </>

From d2cd8ad3b192edbdf7e36227edd3137fcf4f5017 Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Wed, 20 Mar 2019 12:34:26 +0000
Subject: [PATCH 13/15] Add shouldShowFeedback Declare a boolean
 shouldShowFeedback flag to clarify the conditional check when we're
 interrupting the subject advance.

---
 packages/lib-classifier/src/store/FeedbackStore.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 05130c7006..3c8a5ce8ba 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -56,7 +56,8 @@ const FeedbackStore = types
     function createSubjectMiddleware () {
       const subjectMiddleware = autorun(() => {
         addMiddleware(getRoot(self).subjects, (call, next, abort) => {
-          if (call.name === 'advance' && self.isActive && self.messages.length && !self.showModal) {
+          const shouldShowFeedback = self.isActive && self.messages.length && !self.showModal
+          if (call.name === 'advance' && shouldShowFeedback) {
             abort()
             const onHide = getRoot(self).subjects.advance
             self.setOnHide(onHide)

From 8a33b93883e97c323aecfe3d2d415dcc2432987d Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Wed, 20 Mar 2019 12:53:14 +0000
Subject: [PATCH 14/15] Add onSubjectAdvance Move middleware code into
 onSubjectAdvance function for clarity.

---
 .../lib-classifier/src/store/FeedbackStore.js | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index 3c8a5ce8ba..d34255d1dc 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -53,15 +53,23 @@ const FeedbackStore = types
       addDisposer(self, classificationDisposer)
     }
 
+    function onSubjectAdvance (call, next, abort) {
+      const shouldShowFeedback = self.isActive && self.messages.length && !self.showModal
+      if (shouldShowFeedback) {
+        abort()
+        const onHide = getRoot(self).subjects.advance
+        self.setOnHide(onHide)
+        self.showFeedback()
+      } else {
+        next(call)
+      }
+    }
+
     function createSubjectMiddleware () {
       const subjectMiddleware = autorun(() => {
         addMiddleware(getRoot(self).subjects, (call, next, abort) => {
-          const shouldShowFeedback = self.isActive && self.messages.length && !self.showModal
-          if (call.name === 'advance' && shouldShowFeedback) {
-            abort()
-            const onHide = getRoot(self).subjects.advance
-            self.setOnHide(onHide)
-            self.showFeedback()
+          if (call.name === 'advance') {
+            onSubjectAdvance(call, next, abort)
           } else {
             next(call)
           }

From aeaefc72d539d2a217f4f8669fa8dcf8ef64aa2d Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <jim@zooniverse.org>
Date: Wed, 20 Mar 2019 15:24:02 +0000
Subject: [PATCH 15/15] Add tests for onSubjectAdvance middleware Override the
 mock feedback store with stores that have active and inactive feedback. Test
 the middleware for both cases.

---
 .../lib-classifier/src/store/FeedbackStore.js |  1 +
 .../src/store/FeedbackStore.spec.js           | 45 +++++++++++++++++--
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/packages/lib-classifier/src/store/FeedbackStore.js b/packages/lib-classifier/src/store/FeedbackStore.js
index d34255d1dc..dc4ef78e0b 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.js
@@ -131,6 +131,7 @@ const FeedbackStore = types
       setOnHide,
       showFeedback,
       hideFeedback,
+      onSubjectAdvance,
       update,
       reset
     }
diff --git a/packages/lib-classifier/src/store/FeedbackStore.spec.js b/packages/lib-classifier/src/store/FeedbackStore.spec.js
index bbc70ea32c..6c9c60a078 100644
--- a/packages/lib-classifier/src/store/FeedbackStore.spec.js
+++ b/packages/lib-classifier/src/store/FeedbackStore.spec.js
@@ -127,9 +127,6 @@ describe('Model > FeedbackStore', function () {
 
   describe('hideFeedback', function () {
     beforeEach(function () {
-      feedback.subjects = {
-        advance: () => true
-      }
       feedback.setOnHide(sinon.stub())
       feedback.hideFeedback()
     })
@@ -160,4 +157,46 @@ describe('Model > FeedbackStore', function () {
       expect(feedback.hideSubjectViewer).to.equal(false)
     })
   })
+
+  describe('when the subject queue advances', function () {
+    const call = {
+      name: 'advance',
+      args: []
+    }
+    const next = sinon.stub()
+    const abort = sinon.stub()
+
+    describe('when feedback is inactive', function () {
+      beforeEach(function () {
+        feedback = FeedbackStore.create({ isActive: false })
+        feedback.onSubjectAdvance(call, next, abort)
+      })
+
+      it('should continue the action', function () {
+        expect(next.withArgs(call)).to.have.been.calledOnce
+      })
+    })
+
+    describe('when feedback is active', function () {
+      beforeEach(function () {
+        feedback = FeedbackStore.create(feedbackStub)
+        feedback.subjects = {
+          advance: sinon.stub()
+        }
+        feedback.onSubjectAdvance(call, next, abort)
+      })
+
+      it('should abort the action', function () {
+        expect(abort).to.have.been.calledOnce
+      })
+
+      it('should show feedback', function () {
+        expect(feedback.showModal).to.be.true
+      })
+
+      it('should set the onHide callback', function () {
+        expect(feedback.onHide).to.equal(feedback.subjects.advance)
+      })
+    })
+  })
 })