From 929253393291c32a721fc94f20ce191bc20cf522 Mon Sep 17 00:00:00 2001 From: Jim O'Donnell Date: Sat, 17 Aug 2024 09:25:49 +0100 Subject: [PATCH 1/2] feat(app-project): remember current workflow in a cookie - Store your selected workflow in a `workflow_id` session cookie on the `zooniverse.org` domain, scoped to the current project. - Set that cookie when you choose a workflow with the `WorkflowSelectButton` component. - Read the cookie in the Project model, and store it as `project.selectedWorkflow`. Fall back to `project.defaultWorkflow` for backwards compatibility. - Use `project.defaultWorkflow` on the Classify page, if there's no workflow in the URL. - Use `props.workflowID` on the Classify page, if there is a workflow in the URL. - Add the project context to tests. --- .../ProjectHeader/components/Nav/Nav.js | 3 +- .../ClassifyPage/ClassifyPageConnector.js | 15 ++- .../WorkflowAssignmentModal.js | 15 ++- .../WorkflowAssignmentModal.stories.js | 1 + .../WorkflowSelector/WorkflowSelector.spec.js | 88 ++++++++------ .../WorkflowSelectButton.js | 20 ++-- .../WorkflowSelectButton.spec.js | 111 +++++++++++------- .../WorkflowSelectButtons.spec.js | 48 +++++--- packages/app-project/stores/Project.js | 22 +++- packages/app-project/stores/Project.spec.js | 8 +- 10 files changed, 218 insertions(+), 113 deletions(-) diff --git a/packages/app-project/src/components/ProjectHeader/components/Nav/Nav.js b/packages/app-project/src/components/ProjectHeader/components/Nav/Nav.js index 467a7e9f40..303c5f5179 100644 --- a/packages/app-project/src/components/ProjectHeader/components/Nav/Nav.js +++ b/packages/app-project/src/components/ProjectHeader/components/Nav/Nav.js @@ -1,5 +1,6 @@ import { SpacedText } from '@zooniverse/react-components' import { Anchor, Box } from 'grommet' +import { observer } from 'mobx-react' import { useRouter } from 'next/router' import { bool } from 'prop-types' import styled, { css } from 'styled-components' @@ -84,4 +85,4 @@ Nav.propTypes = { adminMode: bool } -export default Nav +export default observer(Nav) diff --git a/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js b/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js index d1cf47b138..f1015364f4 100644 --- a/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js +++ b/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js @@ -6,7 +6,9 @@ function useStore(store) { const { appLoadingState, project: { - experimental_tools + experimental_tools, + defaultWorkflow, + setSelectedWorkflow }, user: { personalization: { projectPreferences } } } = store @@ -14,6 +16,8 @@ function useStore(store) { return { appLoadingState, projectPreferences, + defaultWorkflow, + setSelectedWorkflow, workflowAssignmentEnabled: experimental_tools.includes('workflow assignment') } } @@ -23,16 +27,23 @@ function ClassifyPageConnector(props) { const { appLoadingState, projectPreferences, + defaultWorkflow, + setSelectedWorkflow, workflowAssignmentEnabled = false } = useStore(store) + if (props.workflowID && props.workflowID !== defaultWorkflow) { + setSelectedWorkflow(props.workflowID) + } + return ( ) } diff --git a/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.js b/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.js index a1e5c992a9..e9aa21afd5 100644 --- a/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.js +++ b/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.js @@ -3,7 +3,6 @@ import PropTypes from 'prop-types' import { MobXProviderContext, observer } from 'mobx-react' import { Button, Box, CheckBox } from 'grommet' import { Modal, PrimaryButton, SpacedText } from '@zooniverse/react-components' -import { useRouter } from 'next/router' import { useTranslation } from 'next-i18next' import Link from 'next/link' import addQueryParams from '@helpers/addQueryParams' @@ -12,6 +11,8 @@ function useStore() { const { store } = useContext(MobXProviderContext) return { + /** the current project */ + project: store.project, /** assignedWorkflowID is fetched every 5 classifications per user session */ assignedWorkflowID: store.user.personalization.projectPreferences.settings?.workflow_id, /** This function determines if the user has an assigned workflow and verifies that workflow is active in panoptes */ @@ -20,13 +21,10 @@ function useStore() { } function WorkflowAssignmentModal({ currentWorkflowID = '' }) { - const { assignedWorkflowID, promptAssignment } = useStore() + const { project, assignedWorkflowID, promptAssignment } = useStore() const { t } = useTranslation('screens') - const router = useRouter() - const owner = router?.query?.owner - const project = router?.query?.project - const url = `/${owner}/${project}/classify/workflow/${assignedWorkflowID}` + const url = `/${project.slug}/classify/workflow/${assignedWorkflowID}` /** Check if user has dismissed the modal, but only in the browser */ const isBrowser = typeof window !== 'undefined' @@ -66,6 +64,10 @@ function WorkflowAssignmentModal({ currentWorkflowID = '' }) { setActive(false) } + function onClick() { + project.setSelectedWorkflow(assignedWorkflowID) + } + return ( diff --git a/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.stories.js b/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.stories.js index 1b31635f37..f472f05010 100644 --- a/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.stories.js +++ b/packages/app-project/src/screens/ClassifyPage/components/WorkflowAssignmentModal/WorkflowAssignmentModal.stories.js @@ -22,6 +22,7 @@ const mockRouter = { const snapshot = { project: { + slug: 'zooniverse/snapshot-serengeti', strings: { display_name: 'Snapshot Serengeti', }, diff --git a/packages/app-project/src/shared/components/WorkflowSelector/WorkflowSelector.spec.js b/packages/app-project/src/shared/components/WorkflowSelector/WorkflowSelector.spec.js index 491c96b083..d268ddd7a9 100644 --- a/packages/app-project/src/shared/components/WorkflowSelector/WorkflowSelector.spec.js +++ b/packages/app-project/src/shared/components/WorkflowSelector/WorkflowSelector.spec.js @@ -1,12 +1,16 @@ import asyncStates from '@zooniverse/async-states' import { mount } from 'enzyme' +import { Provider } from 'mobx-react' import { RouterContext } from 'next/dist/shared/lib/router-context.shared-runtime' +import initStore from '@stores' import WorkflowSelector from './WorkflowSelector' import WorkflowSelectButtons from './components/WorkflowSelectButtons' import { expect } from 'chai' describe('Component > WorkflowSelector', function () { + let store + const mockRouter = { asPath: '/zooniverse/snapshot-serengeti/about/team', basePath: '/projects', @@ -42,22 +46,13 @@ describe('Component > WorkflowSelector', function () { const DEFAULT_WORKFLOW_DESCRIPTION = 'WorkflowSelector.message' /** The translation function will simply return keys in a testing env */ - it('should render without crashing', function () { - const wrapper = mount( - - - - ) - expect(wrapper).to.be.ok() + this.beforeEach(function () { + store = initStore(true) }) - describe('workflow description', function () { - it('should use the `workflowDescription` prop if available', function () { - const wrapper = mount( + it('should render without crashing', function () { + const wrapper = mount( + WorkflowSelector', function () { workflowDescription={WORKFLOW_DESCRIPTION} /> + + ) + expect(wrapper).to.be.ok() + }) + + describe('workflow description', function () { + it('should use the `workflowDescription` prop if available', function () { + const wrapper = mount( + + + + + ) expect(wrapper.contains(WORKFLOW_DESCRIPTION)).to.be.true() }) it('should use the default message if the `workflowDescription` prop is unset', function () { const wrapper = mount( - - - + + + + + ) expect(wrapper.contains(DEFAULT_WORKFLOW_DESCRIPTION)).to.be.true() }) it('should use the default message if the `workflowDescription` prop is an empty string', function () { const wrapper = mount( - - - + + + + + ) expect(wrapper.contains(DEFAULT_WORKFLOW_DESCRIPTION)).to.be.true() }) @@ -98,14 +114,16 @@ describe('Component > WorkflowSelector', function () { describe('when successfully loaded the user state and loaded the user project preferences', function () { it('should render workflow select buttons', function () { const wrapper = mount( - - - + + + + + ) expect(wrapper.find(WorkflowSelectButtons)).to.have.lengthOf(1) }) diff --git a/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.js b/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.js index 17b6fc64b5..4eb357922b 100644 --- a/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.js +++ b/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.js @@ -3,28 +3,33 @@ import withThemeContext from '@zooniverse/react-components/helpers/withThemeCont import { Button } from 'grommet' import { Next } from 'grommet-icons' import Link from 'next/link' -import { useRouter } from 'next/router' import { bool, number, object, shape, string } from 'prop-types' import { useTranslation } from 'next-i18next' +import { useCallback, useContext } from 'react' +import { MobXProviderContext } from 'mobx-react' import addQueryParams from '@helpers/addQueryParams' import theme from './theme' export const ThemedButton = withThemeContext(Button, theme) +function useProject() { + const stores = useContext(MobXProviderContext) + const { project } = stores.store + return project +} function WorkflowSelectButton ({ disabled = false, - router, workflow, ...rest }) { const { t } = useTranslation('components') - const nextRouter = useRouter() - router = router || nextRouter - const owner = router?.query?.owner - const project = router?.query?.project + const project = useProject() - const url = `/${owner}/${project}/classify/workflow/${workflow.id}` + const url = `/${project.slug}/classify/workflow/${workflow.id}` + const onClick = useCallback(() => { + project.setSelectedWorkflow(workflow.id) + }, [project, workflow.id]) const href = addQueryParams(url) const completeness = parseInt(workflow.completeness * 100, 10) @@ -68,6 +73,7 @@ function WorkflowSelectButton ({ icon={} reverse label={label} + onClick={onClick} primary {...rest} /> diff --git a/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.spec.js b/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.spec.js index 522e3eea4d..0e44cecfc3 100644 --- a/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.spec.js +++ b/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButton/WorkflowSelectButton.spec.js @@ -3,12 +3,17 @@ import { Grommet } from 'grommet' import { RouterContext } from 'next/dist/shared/lib/router-context.shared-runtime' import sinon from 'sinon' import zooTheme from '@zooniverse/grommet-theme' +import { Provider } from 'mobx-react' +import { applySnapshot } from 'mobx-state-tree' import WorkflowSelectButton, { ThemedButton } from './WorkflowSelectButton' +import initStore from '@stores' import Link from 'next/link' import { expect } from 'chai' describe('Component > WorkflowSelector > WorkflowSelectButton', function () { + let store + const mockRouter = { asPath: '/zooniverse/snapshot-serengeti/about/team', basePath: '/projects', @@ -36,24 +41,36 @@ describe('Component > WorkflowSelector > WorkflowSelectButton', function () { } } + beforeEach(function () { + store = initStore(true) + applySnapshot(store.project, { + id: '1', + slug: 'foo/bar' + }) + }) + it('should render without crashing', function () { const wrapper = mount( - - - - - + + + + + + + ) expect(wrapper).to.be.ok() }) it('should not add "set selection" to the label', function () { const wrapper = mount( - - - - - + + + + + + + ) const label = shallow(wrapper.find(ThemedButton).prop('label')).render() expect(label.text()).to.equal('WorkflowSelector.WorkflowSelectButton.completeWorkflow name') @@ -63,18 +80,20 @@ describe('Component > WorkflowSelector > WorkflowSelectButton', function () { describe('when used with a default workflow', function () { it('should be a link pointing to `/classify/workflow/:workflow_id`', function () { const wrapper = mount( - - - - - + + + + + + + ) expect(wrapper.find(Link).prop('href')).to.equal(`${router.asPath}/classify/workflow/${WORKFLOW.id}`) }) @@ -83,11 +102,13 @@ describe('Component > WorkflowSelector > WorkflowSelectButton', function () { describe('when used with a non-default workflow', function () { it('should be a link pointing to `/classify/workflow/:workflow_id`', function () { const wrapper = mount( - - - - - + + + + + + + ).find(WorkflowSelectButton) expect(wrapper.find(Link).prop('href')).to.equal(`${router.asPath}/classify/workflow/${WORKFLOW.id}`) }) @@ -102,11 +123,13 @@ describe('Component > WorkflowSelector > WorkflowSelectButton', function () { grouped: true } wrapper = mount( - - - - - + + + + + + + ) }) @@ -120,22 +143,26 @@ describe('Component > WorkflowSelector > WorkflowSelectButton', function () { describe('when disabled', function () { it('should not have an href', function () { const wrapper = mount( - - - - - + + + + + + + ).find(WorkflowSelectButton) expect(wrapper.prop('href')).to.be.undefined() }) it('should not wrap the button with Link', function () { const wrapper = mount( - - - - - + + + + + + + ) expect(wrapper.find(Link)).to.have.lengthOf(0) }) diff --git a/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButtons/WorkflowSelectButtons.spec.js b/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButtons/WorkflowSelectButtons.spec.js index 2f60c1331e..5c72f979e9 100644 --- a/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButtons/WorkflowSelectButtons.spec.js +++ b/packages/app-project/src/shared/components/WorkflowSelector/components/WorkflowSelectButtons/WorkflowSelectButtons.spec.js @@ -1,9 +1,13 @@ import { render } from '@testing-library/react' import { expect } from 'chai' +import { Provider } from 'mobx-react' import { RouterContext } from 'next/dist/shared/lib/router-context.shared-runtime' +import initStore from '@stores' import WorkflowSelectButtons from './WorkflowSelectButtons' describe('Component > WorkflowSelector > WorkflowSelectorButtons', function () { + let store + const mockRouter = { asPath: '/zooniverse/snapshot-serengeti', basePath: '/projects', @@ -41,12 +45,18 @@ describe('Component > WorkflowSelector > WorkflowSelectorButtons', function () { } ] + this.beforeEach(function () { + store = initStore(true) + }) + describe('when workflow assignment is not enabled', function () { it('should render a workflow link for each workflow', function () { const { getAllByRole } = render( - - - + + + + + ) expect(getAllByRole('link')).to.have.lengthOf(workflows.length) }) @@ -57,18 +67,22 @@ describe('Component > WorkflowSelector > WorkflowSelectorButtons', function () { describe.skip('when there is an assigned workflow', function () { it('should only render links for unlocked workflows', function () { const { getAllByRole } = render( - - - + + + + + ) expect(getAllByRole('link')).to.have.lengthOf(2) }) it('should render other workflows as just text', function () { const { getByText } = render( - - - + + + + + ) expect(getByText('workflow 3')).to.exist() }) @@ -77,9 +91,11 @@ describe('Component > WorkflowSelector > WorkflowSelectorButtons', function () { describe('when there is not an assigned workflow', function () { it('should only render the first level workflow as unlocked', function () { const { getAllByRole, getByRole } = render( - - - + + + + + ) expect(getByRole('link', { href: '/projects/undefined/undefined/classify/workflow/1' })).to.exist() expect(getAllByRole('link')).to.have.lengthOf(1) @@ -88,9 +104,11 @@ describe('Component > WorkflowSelector > WorkflowSelectorButtons', function () { it('should render other workflows as just text', function () { const { getByText } = render( - - - + + + + + ) expect(getByText('workflow 2')).to.exist() expect(getByText('workflow 3')).to.exist() diff --git a/packages/app-project/stores/Project.js b/packages/app-project/stores/Project.js index ed2dc5846c..c4d1e01955 100644 --- a/packages/app-project/stores/Project.js +++ b/packages/app-project/stores/Project.js @@ -2,6 +2,7 @@ import asyncStates from '@zooniverse/async-states' import { getRoot, types } from 'mobx-state-tree' import numberString from './types/numberString' +import { getCookie } from '@helpers' const TranslationStrings = types.map(types.maybeNull(types.string)) @@ -26,6 +27,7 @@ const Project = types primary_language: types.optional(types.string, 'en'), owners: types.frozen([]), retired_subjects_count: types.optional(types.number, 0), + selectedWorkflow: types.optional(types.string, ''), slug: types.optional(types.string, ''), strings: TranslationStrings, subjects_count: types.optional(types.number, 0), @@ -43,7 +45,7 @@ const Project = types if (activeWorkflows.length === 1) { [singleActiveWorkflow] = self.links['active_workflows'] } - return singleActiveWorkflow + return singleActiveWorkflow || self.selectedWorkflow }, get description () { @@ -90,5 +92,23 @@ const Project = types return self.links['active_workflows'].includes(workflowId) } })) + .preProcessSnapshot(snapshot => { + let workflowFromCookie = '' + if (typeof document !== 'undefined') { + workflowFromCookie = getCookie('workflow_id') + } + return { + ...snapshot, + selectedWorkflow: workflowFromCookie + } + }) + .actions(self => ({ + setSelectedWorkflow(workflowId) { + if (typeof document !== 'undefined') { + self.selectedWorkflow = workflowId + document.cookie = `workflow_id=${workflowId}; path=/projects/${self.slug}; domain=zooniverse.org; SameSite=Strict` + } + } + })) export default Project diff --git a/packages/app-project/stores/Project.spec.js b/packages/app-project/stores/Project.spec.js index ce481d77c4..e22dcd3e34 100644 --- a/packages/app-project/stores/Project.spec.js +++ b/packages/app-project/stores/Project.spec.js @@ -147,8 +147,8 @@ describe('Stores > Project', function () { project = rootStore.project }) - it('should be undefined', function () { - expect(project.defaultWorkflow).to.be.undefined() + it('should be empty', function () { + expect(project.defaultWorkflow).to.be.empty() }) }) @@ -167,8 +167,8 @@ describe('Stores > Project', function () { project = rootStore.project }) - it('should be undefined', function () { - expect(project.defaultWorkflow).to.be.undefined() + it('should be empty', function () { + expect(project.defaultWorkflow).to.be.empty() }) }) }) From af2eedfa38ef88465e1532e5824691d94a8bd1a6 Mon Sep 17 00:00:00 2001 From: Jim O'Donnell Date: Sun, 18 Aug 2024 08:51:55 +0100 Subject: [PATCH 2/2] Client-side redirect /classify to /classify/workflow/[workflowID] --- .../ClassifyPage/ClassifyPageConnector.js | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js b/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js index f1015364f4..1b613ab9b3 100644 --- a/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js +++ b/packages/app-project/src/screens/ClassifyPage/ClassifyPageConnector.js @@ -1,4 +1,5 @@ import { MobXProviderContext, observer } from 'mobx-react' +import { useRouter } from 'next/router' import { useContext } from 'react' import ClassifyPageContainer from './ClassifyPageContainer' @@ -22,7 +23,12 @@ function useStore(store) { } } -function ClassifyPageConnector(props) { +function ClassifyPageConnector({ + // workflow ID from the page URL + workflowID, + ...props +}) { + const router = useRouter() const { store } = useContext(MobXProviderContext) const { appLoadingState, @@ -32,8 +38,15 @@ function ClassifyPageConnector(props) { workflowAssignmentEnabled = false } = useStore(store) - if (props.workflowID && props.workflowID !== defaultWorkflow) { - setSelectedWorkflow(props.workflowID) + // store the workflow from the URL, if it isn't already stored + if (workflowID && workflowID !== defaultWorkflow) { + setSelectedWorkflow(workflowID) + } + + // client-side redirect if there's no workflow in the URL + if (!workflowID && defaultWorkflow) { + const newPath = router.asPath.replace('/classify', `/classify/workflow/${defaultWorkflow}`) + router.replace(newPath, newPath, { shallow: true }) } return ( @@ -43,7 +56,7 @@ function ClassifyPageConnector(props) { assignedWorkflowID={projectPreferences?.settings?.workflow_id} projectPreferences={projectPreferences} workflowAssignmentEnabled={workflowAssignmentEnabled} - workflowID={props.workflowID || defaultWorkflow} + workflowID={workflowID || defaultWorkflow} /> ) }