feat(@sap-ux/create): Adds update service-metadata command - #4987
feat(@sap-ux/create): Adds update service-metadata command#4987IainSAP wants to merge 40 commits into
Conversation
🦋 Changeset detectedLatest commit: a29d6c2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 34 packages
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 |
|
Back to draft - not clear what the behaviour should be where there are backend annotations under the same foplder structure as the value help. The command is |
Hm. But for CAP this is the same because all backend annotations are part of the |
On second thought no - backend annotations and metadata should be in sync. Also its been suggested that the datasource entry in the manifest should also be re-written but not the yamls. ...but, if new value helps appear (where there were none previously - edge case ) they do need yaml updates. |
SummaryThe following content is AI-generated and provides a summary of the pull request: DescriptionAdds a new Key capabilities of
Bug fixes in
Documentation and MCP skill references ( Type of change
How have you tested?
Checklist:
PR Bot InformationVersion:
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR introduces a well-structured update service command with good test coverage, but has two recurring bugs: backendConfig.url is used instead of the resolved backendUrl in both the providerConfig.baseURL and the createForAbapOnCloud call, meaning any system with a connectPath will make requests to the wrong base URL. The TLS patch check should also be moved after the early return for cloud auth types to avoid operating on a provider config that is never used. Additionally, the service name validation order should be swapped to guard against invalid explicit names before resolving the default, and the troubleshooting table in the MCP skill doc references an error string that doesn't match what the code actually emits.
heimwege
left a comment
heimwege
left a comment
There was a problem hiding this comment.
the current implementation is fine for EDMXBackend but I'm wondering about CAP apps:
- CAP apps have no fiori-tools-proxy backend entry in ui5.yaml. CAP apps use cds-plugin-ui5 for local serving - no backend: section under fiori-tools-proxy.
- getBackendConfigsFromFioriToolsProxyMiddleware() returns [] when there's no backend entry (it catches any parse error and returns empty).
- backendConfig is undefined - this hits the guard at service.ts:350: No backend configuration found in 'ui5.yaml' for app at '/path/to/app'
- And exits cleanly.
But the command description is: Refresh the local OData service metadata.xml from the live backend for a Fiori application.
I see two options. Either we restrict to EDMXBacken only (maybe giving a description how to update the metadata for a cap app via .cdsrc and manifest.json) or we add proper handling of CAP apps.
Reject CAP applications early with a clear error and document the EDMX-backend-only limitation in the command description and README so it flows into the fiori-mcp-server docs embeddings.
Will add a guard for edmx and and update docs (readme) |
heimwege
left a comment
heimwege
left a comment
There was a problem hiding this comment.
- changeset ok
- review comments addressed
- coverage excellent
- did NOT test manually
|
5 participants
Summary
Test plan