Skip to content

fix: use / for app module delimiter#1428

Merged
KaiVandivier merged 4 commits intomasterfrom
DHIS2-19308_UI
Apr 8, 2025
Merged

fix: use / for app module delimiter#1428
KaiVandivier merged 4 commits intomasterfrom
DHIS2-19308_UI

Conversation

@netroms
Copy link
Contributor

@netroms netroms commented Mar 25, 2025

No description provided.

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Mar 25, 2025

🚀 Deployed on https://pr-1428--dhis2-settings.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify March 25, 2025 15:09 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify March 25, 2025 15:34 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify March 26, 2025 09:23 Inactive
Copy link
Contributor

@KaiVandivier KaiVandivier left a comment

Choose a reason for hiding this comment

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

Quick question:

src/App.jsx Outdated
module.defaultAction.substr(0, 3) === '../'
? module.name
: `app:${module.name}`,
: `apps${appsModuleDelimiter}${module.name}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Right now this is apps: (plural) or apps/ -- shouldn't it be app: (singular) for the < 42?

Maybe this approach:

    const appsModulePrefix= apiVersion >= 42 ? 'apps/' : 'app:'
    const startModules = (data.apps.modules || []).map((module) => ({
        id:
            module.defaultAction.substr(0, 3) === '../'
                ? module.name
                : appsModulePrefix + module.name,

Copy link
Member

Choose a reason for hiding this comment

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

thanks for the catch! for whatever reason, it worked with apps: in v41 when I tested, but I confirmed with Morten that it should yes be:

  • app: for v41 and prior
  • apps/ for v42 and later

@dhis2-bot dhis2-bot temporarily deployed to netlify April 2, 2025 15:00 Inactive
@tomzemp tomzemp requested a review from KaiVandivier April 2, 2025 15:24
@KaiVandivier KaiVandivier merged commit 2e4ed77 into master Apr 8, 2025
6 checks passed
@KaiVandivier KaiVandivier deleted the DHIS2-19308_UI branch April 8, 2025 13:52
dhis2-bot added a commit that referenced this pull request Apr 8, 2025
## [100.7.3](v100.7.2...v100.7.3) (2025-04-08)

### Bug Fixes

* use correct app module prefix for right version ([#1428](#1428)) ([2e4ed77](2e4ed77))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 100.7.3 🎉

The release is available on:

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.

4 participants