Skip to content

Update navigation handling for multiple properties targeting the same entity set - #5176

Open
marcovieth wants to merge 4 commits into
mainfrom
fix/39359/update-navigation-handling
Open

marcovieth wants to merge 4 commits into
mainfrom
fix/39359/update-navigation-handling

Conversation

@marcovieth

Copy link
Copy Markdown
Contributor

Summary from AI

In FE V4 apps, the "Add Subpage" quick action was incorrectly disabled when multiple navigation properties all target the same entity set and at least one of them already has a subpage configured
Root cause: prepareNavigationData called isPageExists(targetEntitySet) which only matched by entity set name — so a page existing for _Subtype1 → Child01 would block _ZZSubtype13 → Child01 as well
Fix: replace the entity-set check with a route-pattern check — a nav property is only skipped if the manifest already contains a route whose pattern includes /{navProperty}(
Changes

packages/preview-middleware-client/src/adp/quick-actions/fe-v4/add-new-subpage.ts — prepareNavigationData now checks manifest routes instead of entity set names; nav properties that already have a route are filtered out, all others are offered
packages/preview-middleware-client/test/unit/adp/quick-actions/fe-v4.test.ts — updated isNewPageUnavailable object-page test case to include the _BookSupplement route (a target page must have a route — the test was incomplete); added a dedicated regression test for the multi-nav-prop-to-same-entity-set scenario
Test plan

pnpm --filter @sap-ux-private/preview-middleware-client test — all 63 tests pass
Manually verified with hcm.fio.myfamilymemberss1 on QKZ: "Add Subpage" is now enabled and offers _ZZSubtype13 as an option

@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

👋 Hi — I'm PR Bot, your SAP code review assistant.

I'll automatically review your pull requests for code quality, security, and SAP compliance. Get an overview of what I do →

What I do

  • Summarize your pull request changes
  • Review code for quality, correctness, and reliability
  • Suggest fixes when a pipeline job fails

Key commands

Command Description
/review [--all] Trigger a code review. Add --all to include files excluded by excluded_paths.
/summarize Generate a PR summary
/ask <question> Ask about the current changes
/help See all available commands

*This introduction message will be shown to you only once, you will not see it in future PRs.

@changeset-bot

changeset-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 32f727d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@sap-ux/preview-middleware Patch
@sap-ux-private/preview-middleware-client Patch
@sap-ux/create Patch
@sap-ux/ui5-test-writer Patch
@sap-ux/fiori-elements-writer Patch
@sap-ux/fiori-freestyle-writer Patch
@sap-ux/fiori-app-sub-generator Patch
@sap-ux/repo-app-import-sub-generator Patch
@sap-ux/generator-simple-fe Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vadson71
vadson71 marked this pull request as ready for review September 11, 2026 13:50
@vadson71
vadson71 requested a review from a team as a code owner September 11, 2026 13:50
@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Summary

The following content is AI-generated and provides a summary of the pull request:


Title: Fix Add Subpage handling for shared target entity sets

Description

Fixes FE V4 Add Subpage quick action handling when multiple navigation properties target the same entity set.

Previously, a subpage already configured for one navigation property could incorrectly disable or hide other navigation properties that point to the same target entity set. This change now checks existing manifest routes by navigation property route pattern instead of only by target entity set.

Key updates:

  • Filters navigation options based on whether a route already exists for the specific navigation property.
  • Generates target page IDs using navProperty|entitySetObjectPage to avoid collisions between navigation properties targeting the same entity set.
  • Adds regression test coverage for multiple navigation properties pointing to the same entity set.
  • Updates existing Add Subpage tests and adds patch changesets for the affected packages.

Type of change

  • Bug (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a new feature)
  • Breaking change (Bug or New feature that would cause existing functionality/consumers to not work as expected)
  • Non-Breaking chores (Changes to tools, libraries, build process, documentation, etc)
  • None of the above (Reviewers might ask for more clarification)

How have you tested?

  • Ran pnpm --filter @sap-ux-private/preview-middleware-client test
  • Verified all preview middleware client unit tests pass.
  • Manually verified in an FE V4 app scenario where multiple navigation properties target the same entity set: Add Subpage is enabled and offers only navigation properties without an existing route.

Checklist:

  • The code conforms to the general development principles
  • Supplied as many details as possible on this change
  • The code is easy to read and maintainable by others
  • Corresponding changes to the documentation has been done
  • Already existing and new unit tests pass locally
  • I have reviewed and addressed all Hyperspace bot findings (or explicitly explained dismissals)
  • I have done an Agentic review

  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.31.30

  • File Content Strategy: Full file content
  • Event Trigger: pull_request.ready_for_review
  • LLM: gpt-5.5
  • Output Template: Repository PR Template
  • Summary Prompt: Default Prompt
  • Correlation ID: b91e7e30-ade7-11f1-8ca0-76bc92662e8d

@hyperspace-pr-bot hyperspace-pr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found one substantive concern: the route-existence check appears to scan all manifest routes by navigation property only, which can suppress valid subpage options when the same navigation property name exists under another page. I wasn’t able to post the inline comment before tool access ended, so please verify that the check is scoped to the current page’s generated route pattern rather than any route containing /${navigationProperty}(.

PR Bot Information

Version: 1.31.30

  • File Content Strategy: Full file content
  • Event Trigger: pull_request.ready_for_review
  • LLM: gpt-5.5
  • Agent Instructions:
  • Correlation ID: b91e7e30-ade7-11f1-8ca0-76bc92662e8d

@sonarqubecloud

Copy link
Copy Markdown

@Jimmy-Joseph19 Jimmy-Joseph19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes looks good.
Changeset found.
Test coverage ok.
Tested with int test https://github.com/SAP/open-ux-tools/actions/runs/34827724531

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