fix: ensure app version is returned for bundled apps [DHIS2-19138] #3236
+326
−111
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For apps that are bundled and don't have a manually installed version, api/apps doesn't return anything. This causes dashboard to think there isn't an installed version of the app/plugin and shows a warning in the dashboard item. The app version should be also looked up in the bundled apps list.
|
🚀 Deployed on https://pr-3236.dashboard.netlify.dhis2.org |
HendrikThePendric
requested changes
Mar 5, 2025
There was a problem hiding this comment.
I have some general suggestions for refactoring this for readability and getting rid of the ESLint warning, but I have also spotted a "real" problem with the current implementation:
- The
appVersionsstate is being initialised as an empty array - The final
appVersionsstate is computed with the results of a GET request - The children are being rendered immediately, which means that while the GET request is pending, the descending components are not able to read the app-versions
- So ideally, I guess this component requires a loading state (and perhaps also an error state). But at the very least, it should render the children conditionally, i.e.
{appVersions.length > 0 ? children : null}
Some more general suggestions:
- Consider consolidating the "all the app fetching logic" into one place. So this would mean removing the get for
appsfrom theCachedDataProviderand moving it to theInstalledAppVersionsProvider*. You could useuseDataQueryfor this. - Consider moving the fetch for bundled apps to dedicated hook:
useFetchBundledApps, which mimics theuseDataQuerybehaviour (i.e. it should return{ data, error, loading } - After implementing the suggestions above, it should be easy to clean up the provider component
[*] I realise that after doing this, we'll have to scan the full codebase and identify places where we read apps from the useCachedDataQuery hook, and then change it. But I don't think that would be too disruptive.
The handling for apps (both bundled and installed) will likely change and hopefully get simplified when/if api/apps will return all apps data.
Should silence some sonarqube complaint.
jenniferarnesen
added
the
e2e record
|
jenniferarnesen
changed the title
dashboards-app
|
||||||||||||||||||||||||||||
| Project |
dashboards-app
|
| Branch Review |
fix/bundled-app-versions
|
| Run status |
|
| Run duration | 01m 29s |
| Commit |
|
| Committer | Edoardo Sabadelli |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
| View all changes introduced in this branch ↗︎ | |
dhis2-bot
added a commit
that referenced
this pull request
Mar 5, 2025
## [101.0.4](v101.0.3...v101.0.4) (2025-03-05) ### Bug Fixes * retrieve the correct plugin versions for installed and bundled apps [DHIS2-19138] ([#3236](#3236)) ([4082158](4082158))
|
🎉 This PR is included in version 101.0.4 🎉 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements DHIS2-19138
Key features
api/appsDescription
For apps that are bundled and don't have a manually installed version,
api/appsreturns nothing.This causes dashboard to think there isn't an installed version of the app/plugin and shows a warning in the dashboard item. The app version should be looked up in the bundled apps list before returning "no version" (0.0.0).
TODO
Screenshots
Before.
For bundled apps with no manually installed version,
api/appsreturns nothing.A warning is shown in the dashboard item:
After.
The app version is looked up first in
api/appsand then indhis-web-apps/apps-bundle.json: