Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DATAP-1208 Convert reducers to RTK slices #482

Draft
wants to merge 95 commits into
base: main
Choose a base branch
from

Conversation

wwhorton
Copy link
Contributor

@wwhorton wwhorton commented Nov 9, 2023

Redux Toolkit divides redux state into "slices", which make managing state easier and more efficient. Explorer was converted to use slices, but CCDB hadn't yet. This is a PR converting all the reducers to slices, updating tests to accommodate the changes, and refactoring code where necessary, mainly in dispatches. Most of the content of the actions directory was removed since it's handled in the extraReducers piece of the slices, but where it seemed to make more sense to leave them as is, I did.

Removals

  • boilerplate action creators
  • boilerplate reducer setup
  • snapshot tests
  • redux-mock-store

Changes

  • numerous, but, broadly speaking, all of the reducers are now slices and have been converted in place
  • the store was updated to use the newer RTK format
  • added testing-library tests instead of snapshot tests
  • added test setup using real reducers and actions
  • added logic from Complaint Explorer
  • split off logic from Query reducer into Filters, Map, and Trends reducers
  • added middleware to bring together parameters to query the api
  • added middleware logic to only query the aggregations endpoint only when required instead of currently querying both results and aggregations no matter what
  • added React strict mode to catch more errors during development

Testing

  • yarn test: All tests should pass
  • Since redux touches all parts of the app, it would probably be a good idea to go through manually and make sure that everything looks as it should and works as expected.

Notes

  • The names of the tests should probably be updated to reflect the new reducer action names. It didn't occur to me until pretty late in the process, but I don't want to hold up the PR to make those updates when they could be a separate one.
  • Something unexpected I learned was that when state updates in actions relied on a return value from enforceValues() they didn't actually update with the new value if it was within a returned object, e.g. { ...state, newValue: enforceValues(x,'y') }. It had to be written in the "state.value = enforceValues(x, 'y')" form. I think this might have something to do with Immer, but it's probably a good idea for all the state updates to take that form. Again, probably ought to be another PR to update that in the places it wasn't necessary for this PR.

Todos

  • Go through and prune away tests that are no longer required, and add tests to meet coverage requirements. Because the conversion alone results in an enormous PR I thought it would be a good idea to do this part as a separate PR.
  • Optimize the slices and the action directory. Again, to avoid complicating what was already a huge PR, I didn't do this, preferring to set it aside as a separate PR.

Checklist

  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the front end playbook
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged
  • Visually tested in supported browsers and devices
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

@wwhorton wwhorton marked this pull request as ready for review March 4, 2024 17:46
fix bug, shoudl be actions

refactoring a lot of stuff

adding new dependencies updates, and breaking a lot of tests

save off some work

save some work

fixed routes.js logic so it fires correctly on page load

renaming files to correct slice convention

renaming lintin broken references

make dates strings, fixing data lens action

rename query action

save off some more work

update tests

updating more unit tests, fixing missing logic

save off more work

fixing broken map trends chart, updating references, fixing modal

hide button in print mode

save work

fixing references

fixing more unit tests

filterSlice tests

cleanup fix zip typeahead querystring

save off work, fixing queryslice tests

fixing data export unit tests, adding filters to query string export

fixing trendsslicetest

format date fix, update configurestore

adding proxy middleware

make app.js like complaint explorer, migrating over to new react18 strict mode, fixing bug with tour asking to exit onExit handler

update app spec with realstore

clean up tests, fixing percapita

trends work

action bar test update

convert has narrative to s functional

Save work

trend depth toggle to functional

save work

update trend depth toggle

update tests for company receivd filter

save off product / agg branch work
@flacoman91 flacoman91 force-pushed the WW-DATAP-1208-reducers-use-slices branch from 94edfff to 4429c20 Compare April 10, 2024 17:11
@flacoman91 flacoman91 changed the title Ww datap 1208 reducers use slices DATAP-1208 Convert reducers to RTK slices Apr 12, 2024
…-1208-reducers-use-slices

# Conflicts:
#	package.json
#	src/components/Filters/Company.js
#	src/components/Filters/Company.spec.js
#	src/components/Filters/FilterPanel.js
#	src/components/Filters/HasNarrative.js
#	src/components/Filters/HasNarrative.spec.js
#	src/components/Filters/__tests__/__snapshots__/CompanyReceivedFilter.spec.js.snap
#	src/components/Search/SearchComponents.js
#	src/components/Search/SearchPanel.js
#	src/components/Trends/TrendsPanel.less
#	src/components/__tests__/FilterPanel.spec.js
#	src/components/__tests__/Warning.spec.js
#	src/reducers/query/selectors.js
#	src/reducers/trends/selectors.js
# Conflicts:
#	src/actions/url.js
#	src/components/Filters/FilterPanel.js
#	src/components/Filters/Product.js
#	src/components/Filters/__tests__/Product.spec.js
#	src/reducers/query/query.js
#	src/reducers/query/selectors.js

/**
* Helper function generate and sort options
*
* @param {Array} aggsProducts - Products array from aggs reducer
* @param {Array} queryProduct - Products array from query reducer. TBD this will move to new filters reducer in the future
* @param {Array} filtersProducts - Products array from query reducer. TBD this will move to new filters reducer in the future
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self, fix this comment.

@flacoman91 flacoman91 marked this pull request as draft April 30, 2024 14:12
# Conflicts:
#	src/App.spec.js
#	src/components/Filters/__tests__/__snapshots__/CompanyReceivedFilter.spec.js.snap
#	src/components/Search/PillPanel.js
#	src/components/Search/SearchComponents.js
#	src/components/Trends/ExternalTooltip.js
#	src/components/Trends/FocusHeader.js
#	src/components/Trends/LensTabs.js
#	src/components/Trends/TrendDepthToggle.js
#	src/components/__tests__/FocusHeader.spec.js
#	src/components/__tests__/LensTabs.spec.js
#	src/components/__tests__/Warning.spec.js
#	src/components/__tests__/__snapshots__/TrendDepthToggle.spec.js.snap
# Conflicts:
#	src/actions/filter.js
#	src/actions/view.js
#	src/components/Filters/FilterPanel.js
#	src/components/Filters/FilterPanel.spec.js
#	src/components/Filters/FilterPanelToggle.js
#	src/components/List/ListPanel/ListPanel.js
#	src/components/Map/MapPanel.js
#	src/components/Trends/ExternalTooltip/ExternalTooltip.js
#	src/components/Trends/ExternalTooltip/ExternalTooltip.spec.js
#	src/components/Trends/ExternalTooltip/TooltipRow.js
#	src/components/Trends/TrendsPanel.js
#	src/components/Warnings/Warning.spec.js
#	src/reducers/trends/selectors.js
# Conflicts:
#	src/actions/filter.js
#	src/actions/view.js
#	src/components/Filters/FilterPanel.js
#	src/components/Filters/FilterPanel.spec.js
#	src/components/Filters/FilterPanelToggle.js
#	src/components/List/ListPanel/ListPanel.js
#	src/components/Map/MapPanel.js
#	src/components/Trends/ExternalTooltip/ExternalTooltip.js
#	src/components/Trends/ExternalTooltip/ExternalTooltip.spec.js
#	src/components/Trends/ExternalTooltip/TooltipRow.js
#	src/components/Trends/TrendsPanel.js
#	src/components/Warnings/Warning.spec.js
#	src/reducers/trends/selectors.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants