Conversation
Port the extensibility customization guide from the Orders/bookshop sample to XTravels: model-only extension (x_priority + x_CostCenter), multi-repo workspace setup, local multitenancy, and tenant subscription. Regenerate all four screenshots against a running XTravels tenant: - base Travels list - extension project readme - Fiori preview with Priority and Cost Center columns - deployed tenant UI with Priority and Cost Center columns Remove the superseded Orders screenshots.
|
👋 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
Key commands
*This introduction message will be shown to you only once, you will not see it in future PRs. |
Control PanelHi, I'm an AI-powered Review Bot that helps you with summarizing and reviewing pull requests.
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
I posted several focused review comments on changed lines covering a correctness issue in the verification flow, a misleading CodeList statement, a typo, and accessibility improvements for alt text and sensory-dependent instructions. The guide is generally coherent after the XTravels port, but these fixes should improve accuracy and reader usability before merge.
PR Bot Information
Version: 1.31.51
- Review Focus Files:
- LLM:
gpt-5.5 - File Content Strategy: Full file content
- Correlation ID:
ec556a80-b755-11f1-9702-c9dd4a88854f - Event Trigger:
issue_comment.edited
| 2. Verify that it worked by opening the [XTravels Fiori UI](http://localhost:4004/travels/webapp/index.html) in a **new private browser window** and log in as `carol`, which is assigned to tenant `t1`. | ||
|
|
||
| {.mute-dark} | ||
| {.mute-dark} |
There was a problem hiding this comment.
Best Practices: The alt text describes the image type instead of the image content
Alt text should summarize the useful information and avoid phrases such as “A screenshot” and “It shows.” Consider making the table content the subject of the alt text.
| {.mute-dark} | |
| {.mute-dark} |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| ``` | ||
|
|
||
| {.ignore-dark} | ||
| {.ignore-dark} |
There was a problem hiding this comment.
Best Practices: The alt text does not summarize the informative content
Alt text should describe the purpose or key message of the screenshot rather than referring to the screenshot itself. Consider naming the README content that the reader should take away.
| {.ignore-dark} | |
| {.ignore-dark} |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| #### Verify the Extension | ||
|
|
||
| Verify your extensions are applied correctly by opening the [Orders Fiori Preview](http://localhost:4006/$fiori-preview/OrdersService/Orders#preview-app) in a **new private browser window**, log in as `bob`, and see columns _Priority_ and _Sales Region_ filled as in the following screenshot: | ||
| Verify your extensions are applied correctly by opening the [Travels Fiori Preview](http://localhost:4006/$fiori-preview/TravelService/Travels#preview-app) in a **new private browser window**, log in as `bob`, and see columns _Priority_ and _Cost Center_ filled as in the following screenshot: |
There was a problem hiding this comment.
Best Practices: The instruction relies on sensory language and combines several actions
“See columns” is less accessible and the sentence is hard to follow. Consider using “confirm” and keeping the expected result clear.
| Verify your extensions are applied correctly by opening the [Travels Fiori Preview](http://localhost:4006/$fiori-preview/TravelService/Travels#preview-app) in a **new private browser window**, log in as `bob`, and see columns _Priority_ and _Cost Center_ filled as in the following screenshot: | |
| Verify your extensions are applied correctly by opening the [Travels Fiori Preview](http://localhost:4006/$fiori-preview/TravelService/Travels#preview-app) in a **new private browser window**, log in as `bob`, and confirm that the _Priority_ and _Cost Center_ columns contain values, as in the following screenshot: |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| Verify your extensions are applied correctly by opening the [Travels Fiori Preview](http://localhost:4006/$fiori-preview/TravelService/Travels#preview-app) in a **new private browser window**, log in as `bob`, and see columns _Priority_ and _Cost Center_ filled as in the following screenshot: | ||
|
|
||
| {.mute-dark} | ||
| {.mute-dark} |
There was a problem hiding this comment.
Best Practices: The alt text starts with screenshot boilerplate
Alt text should convey the relevant state directly and avoid “A screenshot” or “shows.” Consider focusing on the Fiori preview and extension column values.
| {.mute-dark} | |
| {.mute-dark} |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| #### Verify the Extension {#test-extension } | ||
|
|
||
| Verify your extensions are applied correctly by opening the [Order Management UI](http://localhost:4004/orders/index.html#manage-orders) in a **new private browser window**, log in as `bob`, and check that columns _Priority_ and _Sales Region_ are displayed as in the following screenshot. Also, check that there's content with a proper label in the _Sales Region_ column. | ||
| Verify your extensions are applied correctly by opening the [XTravels UI](http://localhost:4004/travels/webapp/index.html) in a **new private browser window**, log in as `bob`, and check that columns _Priority_ and _Cost Center_ are displayed as in the following screenshot. Also, check that there's content with a proper label in the _Cost Center_ column. |
There was a problem hiding this comment.
Logic Error: This verification expects Cost Center data before the guide adds it
The next step says the Cost Center column is not filled until users copy initial data to db/data/, so readers who follow the steps in order will see a blank column here. Consider moving the Add Data step before this verification, or change this verification and screenshot to check only that the columns exist.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| ### Extending the Service Model | ||
|
|
||
| In the existing in `OrdersService`, the new entities `x_CustomerPostalAddresses` and `x_Remarks` are automatically included since they are targets of the corresponding _compositions_. | ||
| In the existing in `TravelService`, the new entities `x_ApproverPostalAddresses` and `x_Notes` are automatically included since they are targets of the corresponding _compositions_. |
There was a problem hiding this comment.
Typo: The sentence has an extra “in”
“In the existing in TravelService” is grammatically incorrect and interrupts the explanation. Consider making TravelService the location of the auto-included entities.
| In the existing in `TravelService`, the new entities `x_ApproverPostalAddresses` and `x_Notes` are automatically included since they are targets of the corresponding _compositions_. | |
| In `TravelService`, the new entities `x_ApproverPostalAddresses` and `x_Notes` are automatically included since they are targets of the corresponding _compositions_. |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| In the existing in `TravelService`, the new entities `x_ApproverPostalAddresses` and `x_Notes` are automatically included since they are targets of the corresponding _compositions_. | ||
|
|
||
| The new entities `x_Customers` and `x_SalesRegion` are [autoexposed](../services/providing-services#auto-exposed-entities) in a read-only way as [CodeLists](../../cds/common#aspect-codelist). Only if wanted to _change_ it, you would need to expose them explicitly: | ||
| The new entities `x_Approvers` and `x_CostCenters` are [autoexposed](../services/providing-services#auto-exposed-entities) in a read-only way as [CodeLists](../../cds/common#aspect-codelist). Only if wanted to _change_ it, you would need to expose them explicitly: |
There was a problem hiding this comment.
Bug: The text incorrectly says both entities are CodeLists
x_Approvers is defined with cuid, managed, while only x_CostCenters extends CodeList. Consider separating the autoexposure statement from the CodeList statement so readers do not model x_Approvers as a code list.
| The new entities `x_Approvers` and `x_CostCenters` are [autoexposed](../services/providing-services#auto-exposed-entities) in a read-only way as [CodeLists](../../cds/common#aspect-codelist). Only if wanted to _change_ it, you would need to expose them explicitly: | |
| The new entities `x_Approvers` and `x_CostCenters` are [autoexposed](../services/providing-services#auto-exposed-entities) in a read-only way. Only `x_CostCenters` is a [CodeList](../../cds/common#aspect-codelist). If you want to change how they are exposed, expose them explicitly: |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
Co-authored-by: hyperspace-pr-bot[bot] <209611008+hyperspace-pr-bot[bot]@users.noreply.github.com>
- Add Preferred Project Layout, Ship a Pre-Filled Base Model, and Make It Browsable subsections so a cloned template runs with npm install + cds watch. - Add Seed External & Federated Data (@capire/common, S4 customers, flights): explain the minified base model, trim sap.common-Currencies.csv to code;symbol;name;descr, and don't seed Languages/Regions (no target table). - Split reference wiring into its own section (Provide Reference Logic for Local Runs); renumber Extension Guides/Deploy accordingly. - Purge comments from JSON code blocks; use VitePress line-highlight instead. - Regenerate xtravels-deployed-ext and xtravels-ext-readme screenshots.
Extract the @sap/cds-oyster code-extension sandbox reference material into a dedicated Code Extension Reference page (code-extension.md), mirroring the multitenancy index.md/mtxs.md how+reference split. business-logic.md and business-logic-advanced.md keep their walkthroughs and best practices, with moved blocks replaced by short orientation stubs linking into the reference. Also applies a house-style pass across all three pages (pre-defined, -ize spelling, bare ## Introduction headings, no & in headings) and adds the new page to _menu.md.
…ox debugging business-logic.md: - Document the generated JSDoc handler stub and how it drives code completion - Note the mocked sandbox is debuggable in-process (cds watch --debug) business-logic-advanced.md: - Reframe before-CREATE around the verified declarative-vs-handler boundary: @mandatory/@assert.range/@assert.notNull/@readonly work in extensions only on added, defaulted fields; provider fields and @Assert:(case ...) constraints are rejected at push. Keep only the cross-record checks (customer lookup, budget). - Replace the hand-rolled TravelLog audit trio with a pointer to @cap-js/change-tracking and a single after-CREATE emit('TravelCreated') example - Swap TravelLog for CustomerPolicies + a TravelCreated event in the opened surface code-extension.md: - Add a Debugging in the mocked sandbox section and a troubleshooting row
This branch has not been deployed
1 participant
What
Ports the extensibility customization guide from the Orders/bookshop sample to XTravels.
Changes
x_priority(enum high/medium/low) andx_CostCenter(association to a newx_CostCenterscode list), extending only app-localsap.capire.travelsentities.Notes