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

chore: remove d2, and Event Charts/Reports auth header used in development mode #3204

Merged
merged 18 commits into from
Feb 10, 2025

Conversation

jenniferarnesen
Copy link
Collaborator

@jenniferarnesen jenniferarnesen commented Feb 4, 2025

The setting of the authentication header has caused a lot of headaches when testing
with non admin users as the app switches back to admin after login.
This is most likely only something happening during in-house development, but nevertheless,
the potential for it to impact actual installations is real if they were to have
REACT_APP_DHIS2_AUTHORIZATION defined in their environment.

This header is no longer needed now that Event Charts and Event Reports use the withCredentials, which allows CORS requests.

Other changes:

  • All remaining uses of d2 are now migrated to app-platform dataEngine.
    ** The userDataStore which is used when setting the show/hide description
    ** The utility function generateUid was copied from d2 into this repo.
    ** The api request fetchVisualization has been migrated.
  • The show/hide description cypress test was reimplemented as a standard non-cucumber test, as that is where we are headed. Thus, we need side-by-side configurations for regular tests and cucumber tests. So, all the cucumber tests were moved to a e2e_cucumber subfolder, and non-cucumber config was added.

Using localhost with Event Reports on analytics-dev:
image

Using netlify with Event Reports on analytics-dev:
image

In production (sanity check)
image

userDataStore requests to GET dashboard
image

userDataStore request to PUT showDescription
image

userDataStore requests to POST showDescription
image

Confirmed that visualization requests are identical.

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Feb 4, 2025

🚀 Deployed on https://pr-3204.dashboard.netlify.dhis2.org

@dhis2-bot dhis2-bot temporarily deployed to netlify February 4, 2025 09:32 Inactive
* chore: add excludeByVersionTag to cypress config and add tagify dependency

* chore: allow mixing cucumber and non-cucumber specs

* chore: temporarily disable all cypress-cucumber stuff

* test: add cypress test for creating a superset embedded dashboard

* chore: update dependencies so that cypress-tags could run

* chore: disable exclude by version tags and enable cucumber preprocessor

* chore: detect version during run and skip tests if below 42

* chore: keep cucumber specs separate from js specs to prevent

This prevents the js spec from being included in each cucumber feature.
badeball/cypress-cucumber-preprocessor#1262

* fix: close more-menu after toggling dashboard starred state

* fix: ensure dashboard iframe is reloaded when dashboard embed data is updated

* test: finalize e2e test suite for superset embedded dashboard feature

* chore: ensure the before callback has a this scope

* chore: update yarn.lock

* chore: upgrade node version in all workflows

Ensure it  matches my local version where all tests pass

* chore: adjust cypress spec path

* chore: adjust test matrix so it includes both cucumber and regular cypress specs
@dhis2-bot dhis2-bot temporarily deployed to netlify February 4, 2025 10:32 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 4, 2025 10:44 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 4, 2025 10:51 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 5, 2025 13:17 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 6, 2025 13:07 Inactive
@jenniferarnesen
Copy link
Collaborator Author

I think we're going to have to live with the SonarQube code smell report for now.

Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

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

Code changes LGTM

Copy link
Contributor

@HendrikThePendric HendrikThePendric left a comment

Choose a reason for hiding this comment

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

One thing I am considering for your PR and mine (#3205)....
Instead of having "vanilla cypress tests" in cypress/e2e and moving all of the existing stuff to cypress/e2e_cucumber, should we maybe keep all the old stuff where it is and move the new files to something like cypress/e2e_plain?
Maybe less disruptive that way?

@jenniferarnesen
Copy link
Collaborator Author

One thing I am considering for your PR and mine (#3205).... Instead of having "vanilla cypress tests" in cypress/e2e and moving all of the existing stuff to cypress/e2e_cucumber, should we maybe keep all the old stuff where it is and move the new files to something like cypress/e2e_plain? Maybe less disruptive that way?

Possibly. The only thing is that when we have migrated all the cucumber, then we'll probably want to rename e2e_plain to e2e which effectively shows the same changeset.

@dhis2-bot dhis2-bot temporarily deployed to netlify February 7, 2025 14:43 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify February 7, 2025 14:47 Inactive
@jenniferarnesen jenniferarnesen changed the title chore: remove d2 and only use auth header when in development mode chore: remove d2, and Event Charts/Reports auth header used in development mode Feb 10, 2025
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
14 New issues
14 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@dhis2-bot dhis2-bot temporarily deployed to netlify February 10, 2025 10:30 Inactive
@jenniferarnesen jenniferarnesen merged commit 3f9c897 into master Feb 10, 2025
28 of 29 checks passed
@jenniferarnesen jenniferarnesen deleted the chore/remove-d2 branch February 10, 2025 13:58
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 101.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants