-
Notifications
You must be signed in to change notification settings - Fork 75
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
Feat/normalizing state #882
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
talyssonoc
reviewed
Oct 18, 2023
talyssonoc
reviewed
Oct 24, 2023
...cripts/components/story/expanded_story/expanded_story_default/expanded_story_default_spec.js
Outdated
Show resolved
Hide resolved
Guilherme-NL
force-pushed
the
feat/normalizing-state
branch
from
October 25, 2023 16:06
d118957
to
a28e5fc
Compare
kabernardes
approved these changes
Oct 26, 2023
talyssonoc
reviewed
Oct 30, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some comments about using selectors
app/assets/javascripts/components/story/CollapsedStory/index.jsx
Outdated
Show resolved
Hide resolved
gabrielcicero45
approved these changes
Oct 30, 2023
…ewStory and move denormalization outside the haveSearch function
Guilherme-NL
force-pushed
the
feat/normalizing-state
branch
from
November 1, 2023 17:53
49f21c1
to
cf02560
Compare
…m models to reducers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Normalized state
In this PR, we normalized the stories and pastIteration in the Redux state.
Stories
The data was in the format below:
Considering this, for now, we implemented a simpler normalization:
PastIterations
The data was in the format below:
Normalized data example:
normalize and denormalize
I created functions to handle normalization and denormalization. I believe this was the biggest challenge of this task. After calling normalization and denormalization in the correct places, it is easy to make changes to the structure of the normalized object, since the function is isolated and no longer interferes with the functioning of the application
To ensure that everything was working as before, I applied normalization only in the reducer. And I denormalized for functions that used the state as input.
Addition multiple new stories:
We changed the default id of new stories to a Symbol (
new-${Date.now()
}, this way we can uniquely identify each new story, and add multiple new stories at the same time.