feat: deduce base url from hosted location [LIBS-580]#832
feat: deduce base url from hosted location [LIBS-580]#832KaiVandivier wants to merge 3 commits intomasterfrom
Conversation
amcgee
left a comment
amcgee
left a comment
There was a problem hiding this comment.
Looks good, great work Kai!
@janhenrikoverland @Philip-Larsen-Donnelly this will allow a single build of an app (i.e. Line Listing) to be served as EITHER bundled or installed app
I haven't tested this myself yet. We will need to also update the backend to not enforce coreApp declarations in the manifest when apps are installed.
There's a slight risk of misdetecting the baseUrl in some rare situations if DHIS2 is served at a path including /api/apps for example (or with dhis-web in installed app names) but I think this is probably ok...
| const testPaths = { | ||
| '/dev/api/apps/simple-app/index.html': '/dev/', | ||
| '/analytics_dev/dhis-web-maps/plugin.html': '/analytics_dev/', | ||
| '/dhis-web-line-listing/index.html': '/', | ||
| '/dhis-web-user-settings/': '/', | ||
| '/hmis/staging/v41/api/apps/WHO-Data-Quality-App/app.html': | ||
| '/hmis/staging/v41/', | ||
| } |
There was a problem hiding this comment.
Another ambiguous path we might need to consider...
| const testPaths = { | |
| '/dev/api/apps/simple-app/index.html': '/dev/', | |
| '/analytics_dev/dhis-web-maps/plugin.html': '/analytics_dev/', | |
| '/dhis-web-line-listing/index.html': '/', | |
| '/dhis-web-user-settings/': '/', | |
| '/hmis/staging/v41/api/apps/WHO-Data-Quality-App/app.html': | |
| '/hmis/staging/v41/', | |
| } | |
| const testPaths = { | |
| '/dev/api/apps/simple-app/index.html': '/dev/', | |
| '/analytics_dev/dhis-web-maps/plugin.html': '/analytics_dev/', | |
| '/dhis-web-line-listing/index.html': '/', | |
| '/dhis-web-user-settings/': '/', | |
| '/hmis/staging/v41/api/apps/WHO-Data-Quality-App/app.html': | |
| '/hmis/staging/v41/', | |
| '/api/apps/dhis-web-maps': '/api/apps', | |
| } |
An installed app could have dhis-web-maps as a name. We should probably block installation of apps with that prefix in core...
There was a problem hiding this comment.
Hm yeah that's an interesting case -- at /api/apps/dhis-web-maps/, we could either be at a weird custom app with the name 'dhis-web-maps' at a base URL of /, OR we could be at the core Maps app in an instance weirdly hosted with the base URL /api/apps/... I'm not sure if it's possible to figure out which case is true here hah
There was a problem hiding this comment.
It could ping the server to see which base URL works 🤷 or we could decide that this will always assume that's a custom app
There was a problem hiding this comment.
I think we could add enforcement in app hub and core app install logic to disallow custom apps with the dhis-web prefix, hopefully that would be sufficient to disambiguate these cases? And I don't think we need to cater to apps really wanting that specific URL slug...
There was a problem hiding this comment.
Cool, added that test case 🙂 👍
|
I'll test this before approving. |
|
Hm, I noticed that these new URLs end with a
Is one of these more correct than the other? 🤔 (Ending with a Screenshot |
|
@KaiVandivier they should always end with a |
|
2 participants
https://dhis2.atlassian.net/browse/LIBS-580
Computes a base URL from the location the app is hosted. This should be relatively stable, and there are only currently two possibilities (there may be more with the global shell work)
Tested out existing cases to make sure they still work by deploying a build of the simple example app (custom app) and by injecting this build of the Adapter into a core app and deploying that (with a console log added), but have not yet done a test of bundling a previously unbundled app