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: remove d2 #2125

Draft
wants to merge 36 commits into
base: master
Choose a base branch
from
Draft

fix: remove d2 #2125

wants to merge 36 commits into from

Conversation

edoardo
Copy link
Member

@edoardo edoardo commented Jul 21, 2022

Implements DHIS2-XXXX


Key features

  1. prepare to get rid of d2 entirely

Description

This branch has changes to remove the need for d2.
There is still one place where d2 is needed and we don't have an alternative at the moment, the user data store.


TODO

  • figure out how to use the user data store without d2
  • lots of regression testing
  • fix tests, particularly the App.js tests, but before touching them we should figure out how we want to test, what should
    be tested in Cypress and what in unit tests
  • look for and clean up more code not needed
  • see if it's possible to avoid fetching all user authorities to check for the ALL, do we really need the isSuperuser flag?

Known issues

  • the user data store is used to store the visualization object when switching visualization from DV to Maps. Currently it relies on d2 as it's using some classes defined in that package.
  • now that with this PR DV uses the app service data for getting/setting the current AO in the user data store, Maps should be updated in order to read the AO from the same key, since the app service stores the key under settings. Fixed here.

Screenshots

Not very meaningful but this is the app running on this branch:
Screenshot 2022-07-21 at 15 16 34

@edoardo edoardo marked this pull request as draft July 21, 2022 13:20
@dhis2-bot
Copy link
Contributor

dhis2-bot commented Jul 21, 2022

@cypress
Copy link

cypress bot commented Nov 4, 2022

22 failed tests on run #1624 ↗︎

22 569 0 0 Flakiness 0

Details:

fix: set displayProperty from user settings required by Visualization
Project: Data Visualizer App Commit: 2840c2761c
Status: Failed Duration: 08:07 💡
Started: Feb 9, 2023 11:22 AM Ended: Feb 9, 2023 11:30 AM
Failed  open.cy.js • 14 failed tests

View Output Video

Test
opening a saved AO > Column > resets to a new AO Screenshot
opening a saved AO > Bar > resets to a new AO Screenshot
opening a saved AO > Stacked column > resets to a new AO Screenshot
opening a saved AO > Line > resets to a new AO Screenshot
opening a saved AO > Pie > resets to a new AO Screenshot
opening a saved AO > Gauge > resets to a new AO Screenshot
opening a saved AO > Radar > resets to a new AO Screenshot
opening a saved AO > Year over year (line) > resets to a new AO Screenshot
opening a saved AO > Pivot table > resets to a new AO Screenshot
opening a saved AO > Scatter > resets to a new AO Screenshot
The first 10 failed tests are shown, see all 14 tests in Cypress Cloud.
Failed  dimensions/period.cy.js • 3 failed tests

View Output Video

Test
Period dimension > initial state > period 'Last 3 months' is selected Screenshot
Period dimension > initial state > a period can be unselected by double click Screenshot
Period dimension > initial state > a period can be unselected by button Screenshot
Failed  visTypes/scatter.cy.js • 1 failed test

View Output Video

Test
using a Scatter chart > saves and displays items in the correct places Screenshot
Failed  confirmLeave.cy.js • 4 failed tests

View Output Video

Test
confirm leave modal > tries to open a new AO Screenshot
confirm leave modal > cancels leave Screenshot
confirm leave modal > tries to open a new AO Screenshot
confirm leave modal > confirms leave Screenshot

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Nov 4, 2022

@dhis2-bot dhis2-bot temporarily deployed to netlify November 4, 2022 14:53 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 7, 2022 09:34 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 9, 2022 09:50 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify November 18, 2022 14:32 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 9, 2022 09:21 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify December 12, 2022 13:22 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify January 16, 2023 10:43 Inactive
This is to avoid the double Redux store issue.
The wrapper where the Redux store is created cannot use effects that can
cause the component to rerender as this will cause a second Redux store
to be created.
This code might be needed later after more refactoring, so leaving it
commented for now.
The new interpretation modal is used now.
* Remove uiLocale prop as it's not used.
* Fix filtering of authorities to set the superuser flag
* Adjusted tests
This eventually can be skipped once all components are converted into
functional components and the hooks can be used everywhere.
Only compute it when the selector is used (apparently never).
The coverage is provided by Cypress for opening various AOs and the
currentAnalyticalObject
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