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

Entities loading #753

Merged
merged 17 commits into from
Jun 27, 2018
Merged

Entities loading #753

merged 17 commits into from
Jun 27, 2018

Conversation

bjoernricks
Copy link
Contributor

@bjoernricks bjoernricks commented Jun 27, 2018

  • Rename load to loadAll
  • Update entities store to put data in byId property
  • Allow to get a single entity from the store
  • Don't override byId and default with a filter
  • Add automatic test creation for entities

@bjoernricks bjoernricks requested a review from swaterkamp as a code owner June 27, 2018 12:50
Be more precice about the purpose of the function. It will try to load
all entities.
Add a testing helper function to create a rootState for entities tests.
When loading different lists only store the ids of the loaded entities
for the corresponding list and put all loaded entities in an byId
indexed object.

This allows to show already loaded entries (e.g. from a previous request
with a different filter).

Also this allows to get a single entity from the store.
Always defining the state simplifies accessing sub properties of the
state.
When accessing the list of entities always return an empty array.
By using the filter string directly it would have been possible to
overrride the byId and default state properties which MUST NOT be
possible at all.
Add tests to ensure byId and defaults can't be overriden by an filter
string in the entities state.
@codecov
Copy link

codecov bot commented Jun 27, 2018

Codecov Report

Merging #753 into master will increase coverage by 0.36%.
The diff coverage is 96.55%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #753      +/-   ##
=========================================
+ Coverage    5.69%   6.05%   +0.36%     
=========================================
  Files         776     777       +1     
  Lines       25680   25779      +99     
  Branches     5793    5799       +6     
=========================================
+ Hits         1462    1561      +99     
  Misses      21755   21755              
  Partials     2463    2463
Impacted Files Coverage Δ
gsa/src/web/store/entities/filters.js 100% <ø> (ø) ⬆️
...eb/components/dashboard/display/filterselection.js 0% <0%> (ø) ⬆️
gsa/src/web/entities/page.js 0% <0%> (ø) ⬆️
gsa/src/web/store/entities/utils/testing.js 100% <100%> (ø)
gsa/src/web/store/entities/utils/actions.js 100% <100%> (ø) ⬆️
gsa/src/web/store/entities/utils/selectors.js 100% <100%> (ø) ⬆️
gsa/src/web/store/entities/utils/main.js 100% <100%> (ø) ⬆️
gsa/src/web/store/entities/utils/reducers.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9108f97...034fd2e. Read the comment docs.

@swaterkamp swaterkamp merged commit ba36f56 into greenbone:master Jun 27, 2018
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.

2 participants