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

fix(graphCardSelectors): improve sync for multiple api calls #215

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

cdcabrera
Copy link
Member

@cdcabrera cdcabrera commented Feb 29, 2020

What's included

  • fix(graphCardSelectors): improve sync for multiple api calls
    • graphCard, require graphGranularity, relocate granularity to parent
    • graphCardSelectors, improve sync for multi api calls, aggregation
    • graphReducer, viewReducer, relocate granularity
    • i18n, test update
    • openShiftView, rhelView, relocate granularity
    • routerHelpers, routerTypes, test updates

Notes

  • Improves the sync graph display behavior when handling aggregated API calls. There is currently a scenario when one API call is significantly slower it can delay loading in various ways, such as causing the graph to display empty, then load after the fact. This set of updates aims to remove that, and reverts to the cached graph display more easily

How to test

Local run check, BEFORE syncing PR changes

  1. update the NPM packages with $ yarn
  2. $ yarn start
  3. Currently their is a 2 second delay baked into the mock server for the tally response.
    • Confirm selecting granularity back and forth quickly results in an expected display with various loading discrepancies, such as a missing graph, a severely delayed graph render, or blurred graph loading display.
    • To alter the delay change line 41 on ./src/services/rhsmServices.js to another delay in milliseconds, or remove the line entirely to remove the delay

Local run check, AFTER syncing PR changes

  1. update the NPM packages with $ yarn
  2. $ yarn start
  3. Currently their is a 2 second delay baked into the mock server for the tally response.
    • Confirm selecting granularity back and forth quickly results in an expected display without errors, and adverse display mechanics as mentioned in the before scenario testing
    • To alter the delay change line 41 on ./src/services/rhsmServices.js to another delay in milliseconds, or remove the line entirely to remove the delay

Example

...

Updates issue/story

Ongoing

@codecov-io
Copy link

Codecov Report

Merging #215 into ci will increase coverage by 0.3%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff            @@
##               ci     #215     +/-   ##
=========================================
+ Coverage   90.21%   90.52%   +0.3%     
=========================================
  Files          38       38             
  Lines         787      781      -6     
  Branches      198      194      -4     
=========================================
- Hits          710      707      -3     
+ Misses         70       68      -2     
+ Partials        7        6      -1
Impacted Files Coverage Δ
src/components/rhelView/rhelView.js 100% <100%> (ø) ⬆️
src/redux/reducers/graphReducer.js 100% <100%> (ø) ⬆️
src/redux/reducers/viewReducer.js 100% <100%> (ø) ⬆️
src/redux/selectors/graphCardSelectors.js 95.45% <100%> (+1.57%) ⬆️
src/components/openshiftView/openshiftView.js 100% <100%> (ø) ⬆️
src/components/graphCard/graphCard.js 68.75% <85.71%> (+1.44%) ⬆️

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 b8c4971...d911c40. Read the comment docs.

@cdcabrera cdcabrera changed the title WIP fix(updateGraphCardSelectors): improve sync for multiple api calls fix(updateGraphCardSelectors): improve sync for multiple api calls Mar 3, 2020
Copy link
Member Author

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

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

Minor optional change

src/redux/selectors/graphCardSelectors.js Show resolved Hide resolved
…nsights#215)

* graphCard, require graphGranularity, relocate granularity to parent
* graphCardSelectors, improve sync for multi api calls, aggregation
* graphReducer, viewReducer, relocate granularity
* i18n, test update
* openShiftView, rhelView, relocate granularity
* routerHelpers, routerTypes, test updates
@cdcabrera cdcabrera changed the title fix(updateGraphCardSelectors): improve sync for multiple api calls fix(graphCardSelectors): improve sync for multiple api calls Mar 4, 2020
@cdcabrera cdcabrera merged commit 6935cc9 into RedHatInsights:ci Mar 4, 2020
cdcabrera added a commit that referenced this pull request Mar 19, 2020
* graphCard, require graphGranularity, relocate granularity to parent
* graphCardSelectors, improve sync for multi api calls, aggregation
* graphReducer, viewReducer, relocate granularity
* i18n, test update
* openShiftView, rhelView, relocate granularity
* routerHelpers, routerTypes, test updates
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