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: embedded dashboards configuration error ui #3247

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

HendrikThePendric
Copy link
Contributor

@HendrikThePendric HendrikThePendric commented Mar 20, 2025

Implements DHIS2-19275


Key features

  1. App does not crash when Superset Gateway is not available
  2. All other functionality remains identical. This means users can still see all embedded dashboards in the list and navigate to them. They can also create new ones and view and edit them.
  3. However, instead of actually showing any dashboard content, the user will be presented with the standard error UI, with a specific message.

Description

Apart from the points mentioned in the key features, this PR also addresses a bug 3c5fe12 and removes a redundant selector function 924d4c8


Test instructions

To test the "happy path" i.e. where everything is working as it should be, just use the Netlify link and connect to https://debug.dhis2.org/dev. Here you should be seeing the Superset dashboards load correctly. So this would be regression testing.

To test the changes implemented here, use the same netlify link and connect to another v42 test instance, for example https://test.e2e.dhis2.org/analytics-dev. Now check the "Enable embedded dashboards" checkbox in system settings. To see the implemented error view, just create an embedded dashboard and try to view it.

Screenshot

Screenshot 2025-03-24 at 12 19 58

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Mar 20, 2025

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

@dhis2-bot dhis2-bot temporarily deployed to netlify March 20, 2025 16:48 Inactive
@jenniferarnesen
Copy link
Collaborator

jenniferarnesen commented Mar 21, 2025

I think we should consider a different solution. With this implementation, you are basically stuck and can't use the dashboard app at all, right? Instead of SystemSettingProvider returning a Notice component, it should just return the value of "hasConfigurationError" and then let the ViewDashboard decide how to handle it. It should still be possible to view other dashboards, create regular dashboards, etc. Just that the option to create embedded dashboards is removed and existing embedded dashboards could show an error, with a link to go to the dashboard start page.

Copy link
Collaborator

@jenniferarnesen jenniferarnesen left a comment

Choose a reason for hiding this comment

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

See other comment

@dhis2-bot dhis2-bot temporarily deployed to netlify March 24, 2025 11:13 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify March 24, 2025 15:17 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify March 25, 2025 09:04 Inactive
constructor() {
super(
i18n.t(
'Could not load dashboard content because Superset Gateway is unavailable'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there was a suggestion for a different error message in the feature channel.

@dhis2-bot dhis2-bot temporarily deployed to netlify March 25, 2025 09:22 Inactive
@HendrikThePendric HendrikThePendric force-pushed the fix/error-ui-for-superset-gateway-config-error branch from d4fc875 to 8f4ed52 Compare March 26, 2025 11:46
@dhis2-bot dhis2-bot temporarily deployed to netlify March 26, 2025 11:48 Inactive
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.

3 participants