Skip to content

feat(ui5-test-writer): Generate tests for Contact Cards - #4872

Open
sap-sebelao wants to merge 30 commits into
mainfrom
feat/ui5-test-writer/contact-cards-tests
Open

feat(ui5-test-writer): Generate tests for Contact Cards#4872
sap-sebelao wants to merge 30 commits into
mainfrom
feat/ui5-test-writer/contact-cards-tests

Conversation

@sap-sebelao

Copy link
Copy Markdown
Member
  • covers both LR and OP as there is a lot of shared code anyway
  • also contains some minor fixes
    -- column availability is now evaluated
    -- commented out test suggestion now suggests existing action

Internal issue
36742
Internal JIRAs
FXUBRQ34-12564
FXUBRQ34-12565
FXUBRQ34-12566
FXUBRQ34-12525
FXUBRQ34-12604

@changeset-bot

changeset-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 407d1e9

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

This PR includes changesets to release 6 packages
Name Type
@sap-ux/ui5-test-writer Minor
@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

@sap-sebelao
sap-sebelao marked this pull request as ready for review June 22, 2026 12:35
@sap-sebelao
sap-sebelao requested review from a team, docirl, dudajSAP and heimwege June 22, 2026 12:35

@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.

The PR introduces a substantive bug: extractFormFields pushes Contact-card entries (e.g. { property: "_Customer/Contact", targetAnnotation: "Contact" }) into the fields array, and the template's iCheckField loop iterates over all fields unconditionally — so each Contact-card entry generates both an invalid iCheckField({ property: "_Customer/Contact" }) call and the correct iClickLink call. The fix is to either exclude Contact-card entries from fields in extractFormFields, or guard against them in the template loop. The secondary comment about pickContactCardFields depending on the Contact entries being in fields would also need to be addressed if the extraction is fixed upstream. Please resolve these two coupled issues before merging.

PR Bot Information

Version: 1.26.4

  • LLM: anthropic--claude-4.6-sonnet
  • Event Trigger: pull_request.ready_for_review
  • Agent Instructions:
  • Correlation ID: af422319-47ae-4a87-a456-eed8438f7179
  • File Content Strategy: Full file content

Comment thread packages/ui5-test-writer/src/utils/objectPageUtils.ts
Comment thread packages/ui5-test-writer/src/utils/objectPageUtils.ts Outdated
Comment thread packages/ui5-test-writer/templates/v4/integration/ObjectPageJourney.js Outdated

@dudajSAP dudajSAP 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.

From my POV now the auto-gen header is missing on multiple places.

Comment thread packages/ui5-test-writer/templates/v4/integration/pages/ObjectPage.ts Outdated
Comment thread packages/ui5-test-writer/src/utils/objectPageUtils.ts Outdated
The expected fixture lagged behind the test-writer output by two
iCheckField lines for _ReferencedSalesOrder/SalesOrder and
_ReferencedSalesOrderItem/SalesOrderItem. Pull in the current fixture
version that matches the generator.
@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Summary

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


Description

This PR adds Contact Card OPA5 test generation to the @sap-ux/ui5-test-writer package, covering both List Report and Object Page templates. When DataFieldForAnnotation::<property>::Contact entries are detected in the spec model, the generator now emits iClickLink followed by iCheckContactDialog calls at all relevant locations:

  • OP header field groups – emits onHeader().iClickLink({ property: "<field>/Contact" })
  • OP body-section forms – emits onForm(...).iClickLink({ property: "<property>/Contact" })
  • OP body-section tables – emits onTable(...).iClickLink(0, "<DataFieldForAnnotation::...::Contact>")
  • LR tables – generates a new "Check contact card links" OPA test block

Additional bug fixes included in this PR:

  • Column availability check: Columns flagged as Adaptation or Hidden in the spec model are now excluded from generated iCheckColumns assertions.
  • @UI.ConnectedFields / @UI.FieldGroup wrappers: Body sub-section form fields wrapped in these annotations are now resolved to their inner property paths, each emitting one iCheckField with the appropriate connectedFields / fieldGroup qualifier on the FieldIdentifier.
  • getColumnIdentifier fallback: For annotation-driven columns without a Value schema key (e.g. Contact-card columns), the aggregation key is now used as the identifier — matching the MDC propertyKey.
  • The test model constant V4_WITH_SUB_OBJECT_PAGE was updated to include Contact-card columns in both LR and OP sections.
  • An integration test expected output file was updated to reflect corrected column ordering.

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?

  • New unit tests added for extractContactCardColumnsFromNode, pickContactCardFields, pickContactCardFieldsFromHeader, parseDataFieldForAnnotationName, and the isDefaultAvailableColumn availability filter.
  • Integration-level tests in fiori-elements.test.ts verify that generated journey files contain the expected iClickLink / iCheckContactDialog calls for both JS and TS templates.
  • Existing integration test expected outputs updated to match corrected column map ordering.

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

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

Version: 1.27.2

  • Event Trigger: pull_request.ready_for_review
  • Correlation ID: d78d5ba0-7b58-11f1-9e77-3b9078d611b8
  • Output Template: Repository PR Template
  • LLM: anthropic--claude-4.6-sonnet
  • Summary Prompt: Default Prompt
  • File Content Strategy: Full file content

@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.

Summary

The PR adds Contact Card OPA5 test generation across Object Page and List Report pages, plus handling for @UI.ConnectedFields and @UI.FieldGroup wrappers in body sub-section forms. The logic is generally sound and well-tested, but there are a few issues worth addressing:

  1. Missing navigationProperty guard in both EJS templates (ObjectPageJourney.js and ObjectPageJourney.ts): The new contactCardColumns block for body-level table sections emits onTable({ property: "<%- section.navigationProperty %>" }) without checking that section.navigationProperty is defined first — unlike the adjacent iCheckColumns block which already has this guard. This will generate broken onTable({ property: "undefined" }) calls for table sections with no navigation property. (Comments posted with fix suggestions.)

  2. Duplicate test suite in modelUtils.test.ts: The describe('parseDataFieldForAnnotationName()') block appears twice (lines 257–286 and lines 405–436) with identical test cases. The duplication adds no coverage and produces confusing test output. (Comment posted.)

  3. Intentional dual-emit design deserves a code comment in objectPageUtils.ts: Contact-card fields end up in both fields (generating an iCheckField call) and contactCardFields (generating iClickLink/iCheckContactDialog). Since this is intentional but non-obvious, a brief inline comment would prevent future maintainers from treating it as a bug and removing the duplicate.

PR Bot Information

Version: 1.27.2

  • Event Trigger: pull_request.ready_for_review
  • Correlation ID: d78d5ba0-7b58-11f1-9e77-3b9078d611b8
  • Agent Instructions:
  • LLM: anthropic--claude-4.6-sonnet
  • File Content Strategy: Full file content

Comment thread packages/ui5-test-writer/src/utils/objectPageUtils.ts
Comment thread packages/ui5-test-writer/src/utils/objectPageUtils.ts Outdated
Comment thread packages/ui5-test-writer/test/unit/utils/modelUtils.test.ts Outdated
heimwege
heimwege previously approved these changes Jul 16, 2026

@heimwege heimwege 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.

  • changeset ok
  • review comments addressed
  • coverage is excellent
  • did NOT test manually

Added one potential follow-up activity

Comment thread packages/ui5-test-writer/src/types.ts
Comment thread .changeset/ui5-test-writer-contact-cards.md
heimwege
heimwege previously approved these changes Jul 27, 2026

@heimwege heimwege 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.

re-approve

Comment thread packages/ui5-test-writer/templates/v4/integration/ObjectPageJourney.ts Outdated
heimwege
heimwege previously approved these changes Aug 4, 2026

@heimwege heimwege 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.

re-approve

devinea
devinea previously approved these changes Aug 6, 2026

@devinea devinea 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.

Code changes seem clear and well covered by tests.
did not test locally.

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui5-test-writer @sap-ux/ui5-test-writer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants