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 dashboard class undefined #18617

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cconard96
Copy link
Contributor

@cconard96 cconard96 commented Dec 28, 2024

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.

Description

On main, sometimes dashboards are not loading and the GLPIDashboard JS global is undefined at the time the constructor tries being called. Added bare-minimum E2E test to check that dashboards are loading on each of the 4 included dashboard locations.

@AdrienClairembault
Copy link
Contributor

Seems like it causes the search/display_preferences.cy.js test to fail, even after multiple tries it does not go green.

@cconard96
Copy link
Contributor Author

Seems like it causes the search/display_preferences.cy.js test to fail, even after multiple tries it does not go green.

I noticed, even if I don't understand how they are related at all. This test seems to fail a lot even if it manages to pass after a few tries in other PRs. In this run:

  1. Failed because it couldn't find #tabspanel-select
  2. No failure noted. Should have been a pass?
  3. Couldn't find Pending reason option (probably because it was already in the shown items list)
  4. Same as failure 3

For the first failure, it is probably a matter of waiting for a network request using waitForNetworkIdle. For the last 2, I don't see the relation with this PR.

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