refactor(app): add heater shaker types #9577

sakibh · 2022-02-25T19:23:50Z

Overview

This PR adds the constants and types necessary for the heater shaker module in shared-data and redux.

Changelog

Update constants and types in shared-data and redux

Review requests

Check that all the necessary constants and types have been updates for the addition of the heater shaker module.

Risk assessment

low

This PR adds the constants and types necessary for the heater shaker module in shared-data and redux.

codecov · 2022-02-25T19:24:02Z

Codecov Report

Merging #9577 (2da92ef) into edge (8649379) will increase coverage by 0.42%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##             edge    #9577      +/-   ##
==========================================
+ Coverage   75.44%   75.87%   +0.42%     
==========================================
  Files        1908     1917       +9     
  Lines       50869    51476     +607     
  Branches     4892     4899       +7     
==========================================
+ Hits        38379    39057     +678     
+ Misses      11569    11496      -73     
- Partials      921      923       +2

Flag	Coverage Δ
api	`85.55% <0.00%> (+0.79%)`	⬆️
app	`73.10% <0.00%> (-0.04%)`	⬇️
components	`52.41% <ø> (ø)`
labware-library	`49.90% <ø> (ø)`
notify-server	`89.17% <ø> (ø)`
protocol-designer	`44.17% <83.33%> (+0.02%)`	⬆️
robot-server	`92.98% <ø> (+0.17%)`	⬆️
shared-data	`82.09% <100.00%> (+0.46%)`	⬆️
step-generation	`90.33% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...pentrons/hardware_control/modules/heater_shaker.py	`80.38% <0.00%> (+0.09%)`	⬆️
...rganisms/Devices/ModuleCard/ModuleOverflowMenu.tsx	`100.00% <ø> (ø)`
app/src/organisms/Devices/ModuleCard/index.tsx	`57.14% <0.00%> (-2.86%)`	⬇️
app/src/organisms/Devices/ModuleIcon.tsx	`100.00% <ø> (ø)`
...s/Robots/ModuleSettings/ModuleItem/ModuleImage.tsx	`0.00% <ø> (ø)`
...c/pages/Robots/ModuleSettings/ModuleItem/index.tsx	`0.00% <0.00%> (ø)`
app/src/redux/modules/constants.ts	`100.00% <ø> (ø)`
app/src/redux/modules/epic/fetchModulesEpic.ts	`82.75% <0.00%> (-2.96%)`	⬇️
components/src/icons/icon-data.ts	`100.00% <ø> (ø)`
...ts/LabwareSelectionModal/LabwareSelectionModal.tsx	`0.00% <ø> (ø)`
... and 153 more

shlokamin

thanks @sakibh LGTM!

jerader

thank you for adding these types! 🍰

sakibh · 2022-02-28T21:03:23Z

app/src/pages/Robots/ModuleSettings/ModuleItem/index.tsx

-            module={module}
-            controlDisabledReason={controlDisabledReason}
-          />
+          {module.type !== HEATERSHAKER_MODULE_TYPE && (


I gated this to not render ModuleControls when the module is heaterShakerModuleType because there are no controls for the heater shaker in this legacy component.

sakibh · 2022-02-28T21:12:18Z

protocol-designer/src/modules/moduleData.ts

@@ -31,6 +33,12 @@ export const SUPPORTED_MODULE_SLOTS: SupportedSlotMap = {
      value: SPAN7_8_10_11_SLOT,
    },
  ],
+  [HEATERSHAKER_MODULE_TYPE]: [
+    {
+      name: 'Slot 6 (supported)',


This needs to be revisited, I put in slot 6 as a placeholder.

jerader

I guess we should make sure we aren't gonna have a PD release before h/s support is added, because I see the heater shaker as an option on PD now. Or maybe there's a way to hide it.

jerader

LGTM! 🐳

shlokamin

code changes look LGTM, smoke tested PD and seems to look as expected (and since PD e2e tests are passing we know all critical flows still work 😄 )

shlokamin · 2022-03-02T20:54:44Z

components/src/icons/icon-data.ts

@@ -419,7 +419,7 @@ export const ICON_DATA_BY_NAME = {
    path:
      'M11.6 6V5c0-2-1.6-3.6-3.6-3.6S4.4 3 4.4 5v1H3v8h10V6h-1.4zM8 11c-.6 0-1-.4-1-1s.4-1 1-1 1 .4 1 1-.4 1-1 1zM5.6 6V5c0-1.3 1.1-2.4 2.4-2.4s2.4 1.1 2.4 2.4v1H5.6z',
  },
-  'heater-shaker': {
+  'ot-heater-shaker': {


shlokamin · 2022-03-02T22:24:41Z

protocol-designer/src/components/modals/FilePipettesModal/__tests__/ModuleFields.test.tsx

+  function render(props: ModuleFieldsProps) {
+    return mount(
+      <Provider store={store}>
+        <ModuleFields {...props} />
+      </Provider>
+    )
+  }
+


how come this change needed to be made?

cool with it, honestly just curious

Had to include this because of the getEnabledHeaterShaker feature flag in that component. It conditionally filters out the heater shaker after the modules end point is hit. When the flag is removed shall we revert the test as well?

oh sorry i meant the shallow vs mount

Ohh, for that I was following a similar pattern in another component/test called EditModuleCard.test.tsx which also tests with a feature flag (getDisableModuleRestrictions) and wraps the component with a Provider. From some digging around I gathered mount is needed here over shallow because it needs the full life cycle (componentDidMount) to the test the component.

refactor(app): add heater shaker types

d54f47c

This PR adds the constants and types necessary for the heater shaker module in shared-data and redux.

sakibh requested review from b-cooper, shlokamin and jerader February 25, 2022 19:23

sakibh requested review from a team as code owners February 25, 2022 19:23

sakibh self-assigned this Feb 25, 2022

sakibh requested a review from a team as a code owner February 25, 2022 19:23

sakibh removed request for a team February 25, 2022 19:23

sakibh added 2 commits February 25, 2022 14:25

update snapshot

976619a

fix icon test

dc31c38

shlokamin approved these changes Feb 25, 2022

View reviewed changes

jerader approved these changes Feb 25, 2022

View reviewed changes

fix typechecks

b89e0ad

sakibh commented Feb 28, 2022

View reviewed changes

jerader reviewed Feb 28, 2022

View reviewed changes

hide module behind ff

2da92ef

jerader approved these changes Mar 1, 2022

View reviewed changes

shlokamin approved these changes Mar 2, 2022

View reviewed changes

sakibh merged commit 4dc3670 into edge Mar 3, 2022

sakibh deleted the app_hs-update-types branch March 3, 2022 14:52

shlokamin restored the app_hs-update-types branch March 3, 2022 19:58

sakibh mentioned this pull request Mar 4, 2022

feat(protocol-designer): add heater shaker module #9626

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(app): add heater shaker types #9577

refactor(app): add heater shaker types #9577

sakibh commented Feb 25, 2022

codecov bot commented Feb 25, 2022 •

edited

Loading

shlokamin left a comment

jerader left a comment

sakibh Feb 28, 2022

sakibh Feb 28, 2022

jerader left a comment

jerader left a comment

shlokamin left a comment

shlokamin Mar 2, 2022

shlokamin Mar 2, 2022

shlokamin Mar 2, 2022

sakibh Mar 2, 2022 •

edited

Loading

shlokamin Mar 2, 2022

sakibh Mar 2, 2022

refactor(app): add heater shaker types #9577

refactor(app): add heater shaker types #9577

Conversation

sakibh commented Feb 25, 2022

Overview

Changelog

Review requests

Risk assessment

codecov bot commented Feb 25, 2022 • edited Loading

Codecov Report

shlokamin left a comment

Choose a reason for hiding this comment

jerader left a comment

Choose a reason for hiding this comment

sakibh Feb 28, 2022

Choose a reason for hiding this comment

sakibh Feb 28, 2022

Choose a reason for hiding this comment

jerader left a comment

Choose a reason for hiding this comment

jerader left a comment

Choose a reason for hiding this comment

shlokamin left a comment

Choose a reason for hiding this comment

shlokamin Mar 2, 2022

Choose a reason for hiding this comment

shlokamin Mar 2, 2022

Choose a reason for hiding this comment

shlokamin Mar 2, 2022

Choose a reason for hiding this comment

sakibh Mar 2, 2022 • edited Loading

Choose a reason for hiding this comment

shlokamin Mar 2, 2022

Choose a reason for hiding this comment

sakibh Mar 2, 2022

Choose a reason for hiding this comment

codecov bot commented Feb 25, 2022 •

edited

Loading

sakibh Mar 2, 2022 •

edited

Loading