From 714a366167e607d2453905ea5e6df0b99edb4def Mon Sep 17 00:00:00 2001 From: Jethary Alcid <66035149+jerader@users.noreply.github.com> Date: Mon, 16 Dec 2024 14:55:29 -0500 Subject: [PATCH 1/6] fix(protocol-designer): analytics opt in modal fixes (#17106) closes AUTH-1142 --- .eslintignore | 1 + components/src/modals/Modal.tsx | 4 +- protocol-designer/cypress/e2e/createNew.cy.ts | 1 + protocol-designer/cypress/e2e/import.cy.ts | 1 + .../cypress/e2e/migrations.cy.ts | 1 + protocol-designer/cypress/e2e/settings.cy.ts | 7 +- protocol-designer/cypress/support/commands.ts | 9 +++ protocol-designer/src/ProtocolRoutes.tsx | 5 +- protocol-designer/src/analytics/actions.ts | 17 +++-- protocol-designer/src/analytics/middleware.ts | 2 +- protocol-designer/src/analytics/mixpanel.ts | 64 ++++++++++--------- protocol-designer/src/analytics/reducers.ts | 10 ++- protocol-designer/src/analytics/selectors.ts | 3 +- .../src/assets/localization/en/shared.json | 2 +- .../components/SettingsPage/SettingsApp.tsx | 13 ++-- .../src/organisms/GateModal/index.tsx | 20 ++---- .../pages/Landing/__tests__/Landing.test.tsx | 5 +- protocol-designer/src/pages/Landing/index.tsx | 2 +- .../Settings/__tests__/Settings.test.tsx | 5 +- .../src/pages/Settings/index.tsx | 22 ++++--- protocol-designer/src/persist.ts | 5 +- 21 files changed, 122 insertions(+), 77 deletions(-) diff --git a/.eslintignore b/.eslintignore index 5d9406d522a..5535673df8f 100644 --- a/.eslintignore +++ b/.eslintignore @@ -12,6 +12,7 @@ **/CHANGELOG.md !api/release-notes.md !app-shell/build/release-notes.md +**/.yarn-cache/** # components library storybook-static diff --git a/components/src/modals/Modal.tsx b/components/src/modals/Modal.tsx index 7be1ca06340..2a622ad083f 100644 --- a/components/src/modals/Modal.tsx +++ b/components/src/modals/Modal.tsx @@ -24,6 +24,7 @@ export interface ModalProps extends StyleProps { zIndexOverlay?: number showOverlay?: boolean position?: Position + hasHeader?: boolean } /** @@ -43,6 +44,7 @@ export const Modal = (props: ModalProps): JSX.Element => { zIndexOverlay, position, showOverlay, + hasHeader = true, ...styleProps } = props @@ -83,7 +85,7 @@ export const Modal = (props: ModalProps): JSX.Element => { showOverlay={showOverlay} zIndexOverlay={zIndexOverlay} width={styleProps.width ?? '31.25rem'} - header={modalHeader} + header={hasHeader ? modalHeader : undefined} onOutsideClick={closeOnOutsideClick ?? false ? onClose : undefined} // center within viewport aside from nav marginLeft={styleProps.marginLeft ?? '5.656rem'} diff --git a/protocol-designer/cypress/e2e/createNew.cy.ts b/protocol-designer/cypress/e2e/createNew.cy.ts index be578415bee..fb1c70470b3 100644 --- a/protocol-designer/cypress/e2e/createNew.cy.ts +++ b/protocol-designer/cypress/e2e/createNew.cy.ts @@ -12,6 +12,7 @@ describe('The Redesigned Create Protocol Landing Page', () => { }) it('content and step 1 flow works', () => { + cy.closeAnalyticsModal() cy.clickCreateNew() cy.verifyCreateNewHeader() verifyCreateProtocolPage() diff --git a/protocol-designer/cypress/e2e/import.cy.ts b/protocol-designer/cypress/e2e/import.cy.ts index 83ddaf0577d..8001f44f7d6 100644 --- a/protocol-designer/cypress/e2e/import.cy.ts +++ b/protocol-designer/cypress/e2e/import.cy.ts @@ -7,6 +7,7 @@ import { describe('The Import Page', () => { beforeEach(() => { cy.visit('/') + cy.closeAnalyticsModal() }) it('successfully loads a protocol exported on a previous version', () => { diff --git a/protocol-designer/cypress/e2e/migrations.cy.ts b/protocol-designer/cypress/e2e/migrations.cy.ts index 61470962bad..eb93db7ed51 100644 --- a/protocol-designer/cypress/e2e/migrations.cy.ts +++ b/protocol-designer/cypress/e2e/migrations.cy.ts @@ -6,6 +6,7 @@ import { TestFilePath } from '../support/testFiles' describe('Protocol fixtures migrate and match snapshots', () => { beforeEach(() => { cy.visit('/') + cy.closeAnalyticsModal() }) const testCases: MigrateTestCase[] = [ diff --git a/protocol-designer/cypress/e2e/settings.cy.ts b/protocol-designer/cypress/e2e/settings.cy.ts index 5ce896aa883..a9802484d89 100644 --- a/protocol-designer/cypress/e2e/settings.cy.ts +++ b/protocol-designer/cypress/e2e/settings.cy.ts @@ -1,6 +1,7 @@ describe('The Settings Page', () => { before(() => { cy.visit('/') + cy.closeAnalyticsModal() }) it('content and toggle state', () => { @@ -19,19 +20,19 @@ describe('The Settings Page', () => { cy.getByTestId('analyticsToggle') .should('exist') .should('be.visible') - .find('path[aria-roledescription="ot-toggle-input-off"]') + .find('path[aria-roledescription="ot-toggle-input-on"]') .should('exist') // Toggle the share sessions with Opentrons setting cy.getByTestId('analyticsToggle').click() cy.getByTestId('analyticsToggle') - .find('path[aria-roledescription="ot-toggle-input-on"]') + .find('path[aria-roledescription="ot-toggle-input-off"]') .should('exist') // Navigate away from the settings page // Then return to see privacy toggle remains toggled on cy.visit('/') cy.openSettingsPage() cy.getByTestId('analyticsToggle').find( - 'path[aria-roledescription="ot-toggle-input-on"]' + 'path[aria-roledescription="ot-toggle-input-off"]' ) // Toggle off editing timeline tips // Navigate away from the settings page diff --git a/protocol-designer/cypress/support/commands.ts b/protocol-designer/cypress/support/commands.ts index 3f9ffd8ddd8..5a40d7762cb 100644 --- a/protocol-designer/cypress/support/commands.ts +++ b/protocol-designer/cypress/support/commands.ts @@ -9,6 +9,7 @@ declare global { verifyFullHeader: () => Cypress.Chainable verifyCreateNewHeader: () => Cypress.Chainable clickCreateNew: () => Cypress.Chainable + closeAnalyticsModal: () => Cypress.Chainable closeAnnouncementModal: () => Cypress.Chainable verifyHomePage: () => Cypress.Chainable importProtocol: (protocolFile: string) => Cypress.Chainable @@ -61,6 +62,7 @@ export const locators = { eula: 'a[href="https://opentrons.com/eula"]', privacyToggle: 'Settings_hotKeys', analyticsToggleTestId: 'analyticsToggle', + confirm: 'Confirm', } // General Custom Commands @@ -111,6 +113,13 @@ Cypress.Commands.add('clickCreateNew', () => { cy.contains(locators.createProtocol).click() }) +Cypress.Commands.add('closeAnalyticsModal', () => { + cy.get('button') + .contains(locators.confirm) + .should('be.visible') + .click({ force: true }) +}) + // Header Import Cypress.Commands.add('importProtocol', (protocolFilePath: string) => { cy.contains(locators.import).click() diff --git a/protocol-designer/src/ProtocolRoutes.tsx b/protocol-designer/src/ProtocolRoutes.tsx index 7350aa0a8da..1f9c4864ed2 100644 --- a/protocol-designer/src/ProtocolRoutes.tsx +++ b/protocol-designer/src/ProtocolRoutes.tsx @@ -59,9 +59,6 @@ export function ProtocolRoutes(): JSX.Element { path: '/', } const allRoutes: RouteProps[] = [...pdRoutes, landingPage] - const showGateModal = - process.env.NODE_ENV === 'production' || process.env.OT_PD_SHOW_GATE - const navigate = useNavigate() const handleReset = (): void => { navigate('/', { replace: true }) @@ -75,7 +72,7 @@ export function ProtocolRoutes(): JSX.Element { - {showGateModal ? : null} + diff --git a/protocol-designer/src/analytics/actions.ts b/protocol-designer/src/analytics/actions.ts index a6fa4fc18cf..d1357dcfc05 100644 --- a/protocol-designer/src/analytics/actions.ts +++ b/protocol-designer/src/analytics/actions.ts @@ -1,9 +1,10 @@ +import { OLDEST_MIGRATEABLE_VERSION } from '../load-file/migration' import { setMixpanelTracking } from './mixpanel' import type { AnalyticsEvent } from './mixpanel' export interface SetOptIn { type: 'SET_OPT_IN' - payload: boolean + payload: { hasOptedIn: boolean; appVersion: string } } const _setOptIn = (payload: SetOptIn['payload']): SetOptIn => { @@ -16,12 +17,20 @@ const _setOptIn = (payload: SetOptIn['payload']): SetOptIn => { return { type: 'SET_OPT_IN', - payload, + payload: { hasOptedIn: payload.hasOptedIn, appVersion: payload.appVersion }, } } -export const optIn = (): SetOptIn => _setOptIn(true) -export const optOut = (): SetOptIn => _setOptIn(false) +export const optIn = (): SetOptIn => + _setOptIn({ + hasOptedIn: true, + appVersion: process.env.OT_PD_VERSION || OLDEST_MIGRATEABLE_VERSION, + }) +export const optOut = (): SetOptIn => + _setOptIn({ + hasOptedIn: false, + appVersion: process.env.OT_PD_VERSION || OLDEST_MIGRATEABLE_VERSION, + }) export interface AnalyticsEventAction { type: 'ANALYTICS_EVENT' payload: AnalyticsEvent diff --git a/protocol-designer/src/analytics/middleware.ts b/protocol-designer/src/analytics/middleware.ts index 6c798e353ff..29cc66f8a11 100644 --- a/protocol-designer/src/analytics/middleware.ts +++ b/protocol-designer/src/analytics/middleware.ts @@ -499,7 +499,7 @@ export const trackEventMiddleware: Middleware = ({ // NOTE: this is the Redux state AFTER the action has been fully dispatched const state = getState() - const optedIn = getHasOptedIn(state as BaseState) ?? false + const optedIn = getHasOptedIn(state as BaseState)?.hasOptedIn ?? false const event = reduxActionToAnalyticsEvent(state as BaseState, action) if (event != null) { diff --git a/protocol-designer/src/analytics/mixpanel.ts b/protocol-designer/src/analytics/mixpanel.ts index 6304753d8c7..c15c95a8f51 100644 --- a/protocol-designer/src/analytics/mixpanel.ts +++ b/protocol-designer/src/analytics/mixpanel.ts @@ -1,10 +1,8 @@ -// TODO(IL, 2020-09-09): reconcile with app/src/analytics/mixpanel.js, which this is derived from import mixpanel from 'mixpanel-browser' import { getIsProduction } from '../networking/opentronsWebApi' import { getHasOptedIn } from './selectors' import type { BaseState } from '../types' -// TODO(IL, 2020-09-09): AnalyticsEvent type copied from app/src/analytics/types.js, consider merging export type AnalyticsEvent = | { name: string @@ -19,18 +17,20 @@ const MIXPANEL_ID = getIsProduction() : process.env.OT_PD_MIXPANEL_DEV_ID const MIXPANEL_OPTS = { - // opt out by default opt_out_tracking_by_default: true, } export function initializeMixpanel(state: BaseState): void { - const optedIn = getHasOptedIn(state) ?? false + const optedIn = getHasOptedIn(state)?.hasOptedIn ?? false if (MIXPANEL_ID != null) { - console.debug('Initializing Mixpanel', { optedIn }) - - mixpanel.init(MIXPANEL_ID, MIXPANEL_OPTS) - setMixpanelTracking(optedIn) - trackEvent({ name: 'appOpen', properties: {} }, optedIn) // TODO IMMEDIATELY: do we want this? + try { + console.debug('Initializing Mixpanel', { optedIn }) + mixpanel.init(MIXPANEL_ID, MIXPANEL_OPTS) + setMixpanelTracking(optedIn) + trackEvent({ name: 'appOpen', properties: {} }, optedIn) + } catch (error) { + console.error('Error initializing Mixpanel:', error) + } } else { console.warn('MIXPANEL_ID not found; this is a bug if build is production') } @@ -40,32 +40,38 @@ export function initializeMixpanel(state: BaseState): void { export function trackEvent(event: AnalyticsEvent, optedIn: boolean): void { console.debug('Trackable event', { event, optedIn }) if (MIXPANEL_ID != null && optedIn) { - if ('superProperties' in event && event.superProperties != null) { - mixpanel.register(event.superProperties) - } - if ('name' in event && event.name != null) { - mixpanel.track(event.name, event.properties) + try { + if ('superProperties' in event && event.superProperties != null) { + mixpanel.register(event.superProperties) + } + if ('name' in event && event.name != null) { + mixpanel.track(event.name, event.properties) + } + } catch (error) { + console.error('Error tracking event:', error) } } } export function setMixpanelTracking(optedIn: boolean): void { if (MIXPANEL_ID != null) { - if (optedIn) { - console.debug('User has opted into analytics; tracking with Mixpanel') - mixpanel.opt_in_tracking() - // Register "super properties" which are included with all events - mixpanel.register({ - appVersion: process.env.OT_PD_VERSION, - // NOTE(IL, 2020): Since PD may be in the same Mixpanel project as other OT web apps, this 'appName' property is intended to distinguish it - appName: 'protocolDesigner', - }) - } else { - console.debug( - 'User has opted out of analytics; stopping Mixpanel tracking' - ) - mixpanel.opt_out_tracking() - mixpanel.reset() + try { + if (optedIn) { + console.debug('User has opted into analytics; tracking with Mixpanel') + mixpanel.opt_in_tracking() + mixpanel.register({ + appVersion: process.env.OT_PD_VERSION, + appName: 'protocolDesigner', + }) + } else { + console.debug( + 'User has opted out of analytics; stopping Mixpanel tracking' + ) + mixpanel.opt_out_tracking() + mixpanel.reset() + } + } catch (error) { + console.error('Error setting Mixpanel tracking:', error) } } } diff --git a/protocol-designer/src/analytics/reducers.ts b/protocol-designer/src/analytics/reducers.ts index 6d4f71ebaa6..9c2427f603a 100644 --- a/protocol-designer/src/analytics/reducers.ts +++ b/protocol-designer/src/analytics/reducers.ts @@ -4,8 +4,14 @@ import type { Reducer } from 'redux' import type { Action } from '../types' import type { SetOptIn } from './actions' import type { RehydratePersistedAction } from '../persist' -type OptInState = boolean | null -const optInInitialState = null +export interface OptInState { + hasOptedIn: boolean | null + appVersion?: string +} +const optInInitialState = { + hasOptedIn: null, +} + // @ts-expect-error(sb, 2021-6-17): cannot use string literals as action type // TODO IMMEDIATELY: refactor this to the old fashioned way if we cannot have type safety: https://github.com/redux-utilities/redux-actions/issues/282#issuecomment-595163081 const hasOptedIn: Reducer = handleActions( diff --git a/protocol-designer/src/analytics/selectors.ts b/protocol-designer/src/analytics/selectors.ts index 77a37bbfcb1..e98c11e3ab7 100644 --- a/protocol-designer/src/analytics/selectors.ts +++ b/protocol-designer/src/analytics/selectors.ts @@ -1,3 +1,4 @@ import type { BaseState } from '../types' -export const getHasOptedIn = (state: BaseState): boolean | null => +import type { OptInState } from './reducers' +export const getHasOptedIn = (state: BaseState): OptInState => state.analytics.hasOptedIn diff --git a/protocol-designer/src/assets/localization/en/shared.json b/protocol-designer/src/assets/localization/en/shared.json index 0d9342713c4..212ae62bf05 100644 --- a/protocol-designer/src/assets/localization/en/shared.json +++ b/protocol-designer/src/assets/localization/en/shared.json @@ -102,7 +102,7 @@ "only_tiprack": "Incompatible file type", "opentrons_flex": "Opentrons Flex", "opentrons": "Opentrons", - "opentrons_collects_data": "In order to improve our products, Opentrons would like to collect data related to your use of Protocol Designer. With your consent, Opentrons will collect and store analytics and session data, including through the use of cookies and similar technologies, solely for the purpose enhancing our products.", + "opentrons_collects_data": "In order to improve our products, Opentrons would like to collect data related to your use of Protocol Designer. Opentrons will collect and store analytics and session data, including through the use of cookies and similar technologies, solely for the purpose enhancing our products.", "ot2": "Opentrons OT-2", "overwrite_labware": "Overwrite labware", "overwrite": "Click Overwrite to replace the existing labware with the new labware.", diff --git a/protocol-designer/src/components/SettingsPage/SettingsApp.tsx b/protocol-designer/src/components/SettingsPage/SettingsApp.tsx index 20a31121010..f3433b9787c 100644 --- a/protocol-designer/src/components/SettingsPage/SettingsApp.tsx +++ b/protocol-designer/src/components/SettingsPage/SettingsApp.tsx @@ -20,13 +20,10 @@ import { FeatureFlagCard } from './FeatureFlagCard/FeatureFlagCard' export function SettingsApp(): JSX.Element { const dispatch = useDispatch() - const hasOptedIn = useSelector(analyticsSelectors.getHasOptedIn) + const { hasOptedIn } = useSelector(analyticsSelectors.getHasOptedIn) const canClearHintDismissals = useSelector( tutorialSelectors.getCanClearHintDismissals ) - const _toggleOptedIn = hasOptedIn - ? analyticsActions.optOut - : analyticsActions.optIn const { t } = useTranslation(['card', 'application', 'button']) return ( @@ -73,7 +70,13 @@ export function SettingsApp(): JSX.Element { dispatch(_toggleOptedIn())} + onClick={() => + dispatch( + hasOptedIn + ? analyticsActions.optOut() + : analyticsActions.optIn() + ) + } /> diff --git a/protocol-designer/src/organisms/GateModal/index.tsx b/protocol-designer/src/organisms/GateModal/index.tsx index cfe35b1b24a..c97db1d5898 100644 --- a/protocol-designer/src/organisms/GateModal/index.tsx +++ b/protocol-designer/src/organisms/GateModal/index.tsx @@ -9,7 +9,6 @@ import { Modal, PrimaryButton, SPACING, - SecondaryButton, StyledText, } from '@opentrons/components' import { @@ -22,12 +21,15 @@ const EULA_URL = 'https://opentrons.com/eula' export function GateModal(): JSX.Element | null { const { t } = useTranslation('shared') - const hasOptedIn = useSelector(analyticsSelectors.getHasOptedIn) + const { appVersion, hasOptedIn } = useSelector( + analyticsSelectors.getHasOptedIn + ) const dispatch = useDispatch() - if (hasOptedIn == null) { + if (appVersion == null || hasOptedIn == null) { return ( - dispatch(analyticsActions.optOut())} - > - - {t('reject')} - - dispatch(analyticsActions.optIn())}> - {t('agree')} + {t('confirm')} @@ -85,9 +80,6 @@ export function GateModal(): JSX.Element | null { }} /> - - {t('analytics_tracking')} - ) diff --git a/protocol-designer/src/pages/Landing/__tests__/Landing.test.tsx b/protocol-designer/src/pages/Landing/__tests__/Landing.test.tsx index 4ca8430796f..c064a66dfb2 100644 --- a/protocol-designer/src/pages/Landing/__tests__/Landing.test.tsx +++ b/protocol-designer/src/pages/Landing/__tests__/Landing.test.tsx @@ -35,7 +35,10 @@ const render = () => { describe('Landing', () => { beforeEach(() => { - vi.mocked(getHasOptedIn).mockReturnValue(false) + vi.mocked(getHasOptedIn).mockReturnValue({ + hasOptedIn: false, + appVersion: '8.2.1', + }) vi.mocked(getFileMetadata).mockReturnValue({}) vi.mocked(loadProtocolFile).mockReturnValue(vi.fn()) vi.mocked(useAnnouncements).mockReturnValue({} as any) diff --git a/protocol-designer/src/pages/Landing/index.tsx b/protocol-designer/src/pages/Landing/index.tsx index 3a9ea55bfd3..0e66adaa435 100644 --- a/protocol-designer/src/pages/Landing/index.tsx +++ b/protocol-designer/src/pages/Landing/index.tsx @@ -38,7 +38,7 @@ export function Landing(): JSX.Element { const [showAnnouncementModal, setShowAnnouncementModal] = useState( false ) - const hasOptedIn = useSelector(getHasOptedIn) + const { hasOptedIn } = useSelector(getHasOptedIn) const { bakeToast, eatToast } = useKitchen() const announcements = useAnnouncements() const lastAnnouncement = announcements[announcements.length - 1] diff --git a/protocol-designer/src/pages/Settings/__tests__/Settings.test.tsx b/protocol-designer/src/pages/Settings/__tests__/Settings.test.tsx index 8a1b948e953..671a31ccfc4 100644 --- a/protocol-designer/src/pages/Settings/__tests__/Settings.test.tsx +++ b/protocol-designer/src/pages/Settings/__tests__/Settings.test.tsx @@ -32,7 +32,10 @@ const render = () => { describe('Settings', () => { beforeEach(() => { - vi.mocked(getHasOptedIn).mockReturnValue(false) + vi.mocked(getHasOptedIn).mockReturnValue({ + hasOptedIn: false, + appVersion: '8.2.1', + }) vi.mocked(getFeatureFlagData).mockReturnValue({}) vi.mocked(getCanClearHintDismissals).mockReturnValue(true) }) diff --git a/protocol-designer/src/pages/Settings/index.tsx b/protocol-designer/src/pages/Settings/index.tsx index b678327adb8..7165b5d68a8 100644 --- a/protocol-designer/src/pages/Settings/index.tsx +++ b/protocol-designer/src/pages/Settings/index.tsx @@ -42,18 +42,16 @@ export function Settings(): JSX.Element { const [showAnnouncementModal, setShowAnnouncementModal] = useState( false ) - const hasOptedIn = useSelector(analyticsSelectors.getHasOptedIn) + const analytics = useSelector(analyticsSelectors.getHasOptedIn) const flags = useSelector(getFeatureFlagData) const canClearHintDismissals = useSelector( tutorialSelectors.getCanClearHintDismissals ) - const _toggleOptedIn = hasOptedIn - ? analyticsActions.optOut - : analyticsActions.optIn - const prereleaseModeEnabled = flags.PRERELEASE_MODE === true const pdVersion = process.env.OT_PD_VERSION + const prereleaseModeEnabled = flags.PRERELEASE_MODE === true + const allFlags = Object.keys(flags) as FlagTypes[] const getDescription = (flag: FlagTypes): string => { @@ -276,15 +274,23 @@ export function Settings(): JSX.Element { data-testid="analyticsToggle" size="2rem" css={ - Boolean(hasOptedIn) + Boolean(analytics.hasOptedIn) ? TOGGLE_ENABLED_STYLES : TOGGLE_DISABLED_STYLES } - onClick={() => dispatch(_toggleOptedIn())} + onClick={() => + dispatch( + analytics.hasOptedIn + ? analyticsActions.optOut() + : analyticsActions.optIn() + ) + } > diff --git a/protocol-designer/src/persist.ts b/protocol-designer/src/persist.ts index 693346e77b0..576ad937cc2 100644 --- a/protocol-designer/src/persist.ts +++ b/protocol-designer/src/persist.ts @@ -8,7 +8,10 @@ export interface RehydratePersistedAction { payload: { 'tutorial.dismissedHints'?: Record 'featureFlags.flags'?: Record - 'analytics.hasOptedIn'?: boolean | null + 'analytics.hasOptedIn'?: { + hasOptedIn: boolean | null + appVersion?: string + } } } export const getLocalStorageItem = (path: string): unknown => { From 4446eafa6354b4ffbc3b7925ce54541128a5e8ff Mon Sep 17 00:00:00 2001 From: Jethary Alcid <66035149+jerader@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:49:12 -0500 Subject: [PATCH 2/6] =?UTF-8?q?fix(protocol-designer):=20heater=20shaker?= =?UTF-8?q?=20timer=20field=20is=20a=20boolean=20instea=E2=80=A6=20(#17119?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …d of boolean string closes RQA-3794 --- .../src/molecules/ToggleExpandStepFormField/index.tsx | 6 +----- .../StepForm/StepTools/HeaterShakerTools/index.tsx | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/protocol-designer/src/molecules/ToggleExpandStepFormField/index.tsx b/protocol-designer/src/molecules/ToggleExpandStepFormField/index.tsx index 7412afb3c2a..a99f6547f7a 100644 --- a/protocol-designer/src/molecules/ToggleExpandStepFormField/index.tsx +++ b/protocol-designer/src/molecules/ToggleExpandStepFormField/index.tsx @@ -62,11 +62,7 @@ export function ToggleExpandStepFormField( resetFieldValue() } } else if (toggleValue == null) { - toggleUpdateValue( - name === 'targetTemperature' || name === 'heaterShakerTimer' - ? 'true' - : true - ) + toggleUpdateValue(name === 'targetTemperature' ? 'true' : true) } else { toggleUpdateValue(!toggleValue) if (toggleValue) { diff --git a/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/HeaterShakerTools/index.tsx b/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/HeaterShakerTools/index.tsx index 1577db5da8c..3447de54069 100644 --- a/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/HeaterShakerTools/index.tsx +++ b/protocol-designer/src/pages/Designer/ProtocolSteps/StepForm/StepTools/HeaterShakerTools/index.tsx @@ -103,7 +103,7 @@ export function HeaterShakerTools(props: StepFormProps): JSX.Element { 'form:step_edit_form.field.heaterShaker.timer.heaterShakerSetTimer' )} fieldTitle={t('form:step_edit_form.field.heaterShaker.duration')} - isSelected={formData.heaterShakerSetTimer === 'true'} + isSelected={formData.heaterShakerSetTimer === true} units={t('application:units.time')} toggleElement="checkbox" formLevelError={getFormLevelError( From e236cb6e4155404a83830d7c03c1d5826edec18c Mon Sep 17 00:00:00 2001 From: koji Date: Mon, 16 Dec 2024 16:37:06 -0500 Subject: [PATCH 3/6] fix(protocol-designer) remove hardcoded pd version from release notes modal (#17121) * fix(protocol-designer) remove hardcoded pd version from release notes modal --- protocol-designer/src/assets/localization/en/modal.json | 2 +- .../src/organisms/AnnouncementModal/announcements.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/protocol-designer/src/assets/localization/en/modal.json b/protocol-designer/src/assets/localization/en/modal.json index 2c065e2d6b6..8530f61a28a 100644 --- a/protocol-designer/src/assets/localization/en/modal.json +++ b/protocol-designer/src/assets/localization/en/modal.json @@ -52,7 +52,7 @@ "body6": "All protocols require {{app}} version 7.3.0 or later to run." }, "redesign": { - "body1": "Welcome to Protocol Designer 8.2.0!", + "body1": "Welcome to Protocol Designer {{version}}!", "body2": "We’re excited to release the new Opentrons Protocol Designer, now with a fresh redesign! Enjoy the same functionality with the added ability to:", "body3": "Add multiple Heater-Shaker Modules and Magnetic Blocks to the deck (Flex only).", "body4": "All protocols now require Opentrons App version 8.2.0+ to run.", diff --git a/protocol-designer/src/organisms/AnnouncementModal/announcements.tsx b/protocol-designer/src/organisms/AnnouncementModal/announcements.tsx index edbc801351a..76eda9f25ea 100644 --- a/protocol-designer/src/organisms/AnnouncementModal/announcements.tsx +++ b/protocol-designer/src/organisms/AnnouncementModal/announcements.tsx @@ -47,6 +47,7 @@ const OPENTRONS_PD = 'Opentrons Protocol Designer' export const useAnnouncements = (): Announcement[] => { const { t } = useTranslation('modal') + const pdVersion = process.env.OT_PD_VERSION return [ { @@ -307,7 +308,7 @@ export const useAnnouncements = (): Announcement[] => { { announcementKey: 'redesign8.2', image: , - heading: t('announcements.redesign.body1'), + heading: t('announcements.redesign.body1', { version: pdVersion }), message: ( From 4bb6a9c6fc1f05f8298063b168d121199a599575 Mon Sep 17 00:00:00 2001 From: Jethary Alcid <66035149+jerader@users.noreply.github.com> Date: Tue, 17 Dec 2024 10:55:12 -0500 Subject: [PATCH 4/6] =?UTF-8?q?fix(protocol-designer):=20make=208=5F2=5F2?= =?UTF-8?q?=20migration=20to=20migrate=20HS=20set=20timer=20=E2=80=A6=20(#?= =?UTF-8?q?17124)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …field The `heaterShakerSetTimer` field's type is `heaterShakerSetTimer: 'true' | 'false' | null` - this was written 3 years ago. However, despite this, the field when checking the checkbox in the form in PD would populate it as `true` or `false` booleans rather than the true/false strings. 2 weeks ago, right before 8.2.0 was released, there was a bug checking the timer checkbox for the temperature module and it was because of this boolean vs boolean strings issue. So I extended it onto the heater-shaker set timer field as well because of its type. Unfortunately, that resulted in this current error: - protocols created in 8.2.0 and 8.2.1 have the `heaterShakerSetTimer` field set to either `'true'` or `'false'` strings - protocols created prior to 8.2.0 have the `heaterShakerSetTimer` field set to `true` or `false` boolean - my initial fix was to update the form field to check for `true` or `false` boolean but that didn't fix any protocols created in 8.2.0 and 8.2.1. - the extended fix unfortunately required a migration so now if `'true'` or `'false'` strings are used in the field, they get migrated to `true` or `false` boolean -- this i believe is the only way to fix all protocol versions --- .../fixtures/protocol/8/doItAllV8.json | 2 +- .../8/newAdvancedSettingsAndMultiTemp.json | 2 +- .../8/ninetySixChannelFullAndColumn.json | 2 +- protocol-designer/src/form-types.ts | 2 +- .../src/load-file/migration/8_2_2.ts | 51 +++++++++++++++++++ .../src/load-file/migration/index.ts | 4 ++ .../test/heaterShakerFormToArgs.test.ts | 4 +- 7 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 protocol-designer/src/load-file/migration/8_2_2.ts diff --git a/protocol-designer/fixtures/protocol/8/doItAllV8.json b/protocol-designer/fixtures/protocol/8/doItAllV8.json index 5618fde0e9b..9206bdcf267 100644 --- a/protocol-designer/fixtures/protocol/8/doItAllV8.json +++ b/protocol-designer/fixtures/protocol/8/doItAllV8.json @@ -13,7 +13,7 @@ }, "designerApplication": { "name": "opentrons/protocol-designer", - "version": "8.2.0", + "version": "8.2.2", "data": { "_internalAppBuildDate": "Mon, 11 Nov 2024 20:18:16 GMT", "defaultValues": { diff --git a/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json b/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json index 6fe09bdbd02..9c6d24abaca 100644 --- a/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json +++ b/protocol-designer/fixtures/protocol/8/newAdvancedSettingsAndMultiTemp.json @@ -13,7 +13,7 @@ }, "designerApplication": { "name": "opentrons/protocol-designer", - "version": "8.2.0", + "version": "8.2.2", "data": { "_internalAppBuildDate": "Mon, 11 Nov 2024 20:10:44 GMT", "defaultValues": { diff --git a/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json b/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json index cc871cd6917..fe6d08df65c 100644 --- a/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json +++ b/protocol-designer/fixtures/protocol/8/ninetySixChannelFullAndColumn.json @@ -13,7 +13,7 @@ }, "designerApplication": { "name": "opentrons/protocol-designer", - "version": "8.2.0", + "version": "8.2.2", "data": { "_internalAppBuildDate": "Wed, 01 May 2024 13:32:34 GMT", "defaultValues": { diff --git a/protocol-designer/src/form-types.ts b/protocol-designer/src/form-types.ts index 1a9962ae582..8e86d72a426 100644 --- a/protocol-designer/src/form-types.ts +++ b/protocol-designer/src/form-types.ts @@ -360,7 +360,7 @@ export interface HydratedTemperatureFormData { targetTemperature: string | null } export interface HydratedHeaterShakerFormData { - heaterShakerSetTimer: 'true' | 'false' | null + heaterShakerSetTimer: boolean | null heaterShakerTimer: string | null id: string latchOpen: boolean diff --git a/protocol-designer/src/load-file/migration/8_2_2.ts b/protocol-designer/src/load-file/migration/8_2_2.ts new file mode 100644 index 00000000000..007aa288787 --- /dev/null +++ b/protocol-designer/src/load-file/migration/8_2_2.ts @@ -0,0 +1,51 @@ +import type { ProtocolFile } from '@opentrons/shared-data' +import type { DesignerApplicationData } from './utils/getLoadLiquidCommands' + +export const migrateFile = ( + appData: ProtocolFile +): ProtocolFile => { + const { designerApplication } = appData + + if (designerApplication == null || designerApplication?.data == null) { + throw Error('The designerApplication key in your file is corrupt.') + } + const savedStepForms = designerApplication.data + ?.savedStepForms as DesignerApplicationData['savedStepForms'] + + const savedStepsWithUpdatedHeaterShakerTimerField = Object.values( + savedStepForms + ).reduce((acc, form) => { + if (form.stepType === 'heaterShaker') { + const { id, heaterShakerSetTimer } = form + let newSetTimer = heaterShakerSetTimer + + if (heaterShakerSetTimer === 'false') { + newSetTimer = false + } else if (heaterShakerSetTimer === 'true') { + newSetTimer = true + } + return { + ...acc, + [id]: { + ...form, + heaterShakerSetTimer: newSetTimer, + }, + } + } + return acc + }, {}) + + return { + ...appData, + designerApplication: { + ...designerApplication, + data: { + ...designerApplication.data, + savedStepForms: { + ...designerApplication.data.savedStepForms, + ...savedStepsWithUpdatedHeaterShakerTimerField, + }, + }, + }, + } +} diff --git a/protocol-designer/src/load-file/migration/index.ts b/protocol-designer/src/load-file/migration/index.ts index 1ef3f346153..27afb8eb132 100644 --- a/protocol-designer/src/load-file/migration/index.ts +++ b/protocol-designer/src/load-file/migration/index.ts @@ -12,6 +12,8 @@ import { migrateFile as migrateFileSeven } from './7_0_0' import { migrateFile as migrateFileEight } from './8_0_0' import { migrateFile as migrateFileEightOne } from './8_1_0' import { migrateFile as migrateFileEightTwo } from './8_2_0' +import { migrateFile as migrateFileEightTwoPointTwo } from './8_2_2' + import type { PDProtocolFile } from '../../file-types' export const OLDEST_MIGRATEABLE_VERSION = '1.0.0' @@ -54,6 +56,8 @@ const allMigrationsByVersion: MigrationsByVersion = { '8.1.0': migrateFileEightOne, // @ts-expect-error '8.2.0': migrateFileEightTwo, + // @ts-expect-error + '8.2.2': migrateFileEightTwoPointTwo, } export const migration = ( file: any diff --git a/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/heaterShakerFormToArgs.test.ts b/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/heaterShakerFormToArgs.test.ts index 4329e6ca4f6..2a664085113 100644 --- a/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/heaterShakerFormToArgs.test.ts +++ b/protocol-designer/src/steplist/formLevel/stepFormToArgs/test/heaterShakerFormToArgs.test.ts @@ -9,7 +9,7 @@ describe('heaterShakerFormToArgs', () => { id: 'id', stepDetails: 'step details', moduleId: 'moduleId', - heaterShakerSetTimer: 'true', + heaterShakerSetTimer: true, setHeaterShakerTemperature: true, setShake: true, latchOpen: false, @@ -35,7 +35,7 @@ describe('heaterShakerFormToArgs', () => { id: 'id', stepDetails: 'step details', moduleId: 'moduleId', - heaterShakerSetTimer: 'false', + heaterShakerSetTimer: false, setHeaterShakerTemperature: true, setShake: false, latchOpen: false, From f38bb0b6c0cd4cc51d6844f7c09abf8b44b10d61 Mon Sep 17 00:00:00 2001 From: koji Date: Wed, 18 Dec 2024 08:43:00 -0500 Subject: [PATCH 5/6] fix(protocol-designer): change initial value from null to true (#17133) * fix(protocol-designer): change initial value from null to true --- protocol-designer/src/analytics/reducers.ts | 4 ++-- protocol-designer/src/pages/Settings/index.tsx | 10 ++++------ protocol-designer/src/persist.ts | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/protocol-designer/src/analytics/reducers.ts b/protocol-designer/src/analytics/reducers.ts index 9c2427f603a..c894c6be3ce 100644 --- a/protocol-designer/src/analytics/reducers.ts +++ b/protocol-designer/src/analytics/reducers.ts @@ -5,11 +5,11 @@ import type { Action } from '../types' import type { SetOptIn } from './actions' import type { RehydratePersistedAction } from '../persist' export interface OptInState { - hasOptedIn: boolean | null + hasOptedIn: boolean appVersion?: string } const optInInitialState = { - hasOptedIn: null, + hasOptedIn: true, } // @ts-expect-error(sb, 2021-6-17): cannot use string literals as action type diff --git a/protocol-designer/src/pages/Settings/index.tsx b/protocol-designer/src/pages/Settings/index.tsx index 7165b5d68a8..63519226a92 100644 --- a/protocol-designer/src/pages/Settings/index.tsx +++ b/protocol-designer/src/pages/Settings/index.tsx @@ -42,7 +42,7 @@ export function Settings(): JSX.Element { const [showAnnouncementModal, setShowAnnouncementModal] = useState( false ) - const analytics = useSelector(analyticsSelectors.getHasOptedIn) + const { hasOptedIn } = useSelector(analyticsSelectors.getHasOptedIn) const flags = useSelector(getFeatureFlagData) const canClearHintDismissals = useSelector( tutorialSelectors.getCanClearHintDismissals @@ -274,13 +274,13 @@ export function Settings(): JSX.Element { data-testid="analyticsToggle" size="2rem" css={ - Boolean(analytics.hasOptedIn) + Boolean(hasOptedIn) ? TOGGLE_ENABLED_STYLES : TOGGLE_DISABLED_STYLES } onClick={() => dispatch( - analytics.hasOptedIn + hasOptedIn ? analyticsActions.optOut() : analyticsActions.optIn() ) @@ -288,9 +288,7 @@ export function Settings(): JSX.Element { > diff --git a/protocol-designer/src/persist.ts b/protocol-designer/src/persist.ts index 576ad937cc2..4c0bb6a1ed3 100644 --- a/protocol-designer/src/persist.ts +++ b/protocol-designer/src/persist.ts @@ -9,7 +9,7 @@ export interface RehydratePersistedAction { 'tutorial.dismissedHints'?: Record 'featureFlags.flags'?: Record 'analytics.hasOptedIn'?: { - hasOptedIn: boolean | null + hasOptedIn: boolean appVersion?: string } } From aa57afca31e21f5aa3e20f0651b2ae56931de45f Mon Sep 17 00:00:00 2001 From: Jethary Alcid <66035149+jerader@users.noreply.github.com> Date: Wed, 18 Dec 2024 15:29:25 -0500 Subject: [PATCH 6/6] fix(protocol-designer): refine logic for persisted state (#17142) --- protocol-designer/src/analytics/reducers.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/protocol-designer/src/analytics/reducers.ts b/protocol-designer/src/analytics/reducers.ts index c894c6be3ce..6aff3b00a84 100644 --- a/protocol-designer/src/analytics/reducers.ts +++ b/protocol-designer/src/analytics/reducers.ts @@ -8,7 +8,7 @@ export interface OptInState { hasOptedIn: boolean appVersion?: string } -const optInInitialState = { +const optInInitialState: OptInState = { hasOptedIn: true, } @@ -23,7 +23,11 @@ const hasOptedIn: Reducer = handleActions( action: RehydratePersistedAction ) => { const persistedState = action.payload?.['analytics.hasOptedIn'] - return persistedState !== undefined ? persistedState : optInInitialState + if (persistedState == null || persistedState?.hasOptedIn == null) { + return optInInitialState + } else { + return persistedState + } }, }, optInInitialState