Repository navigation
Conversation
… names - Slice 651696 [vendor work]
| "groups": [ | ||
| { | ||
| "name": "AllExtensions", | ||
| "doNotInstall": true |
There was a problem hiding this comment.
[TODO] For me to check later during integration: confirm whether doNotInstall: true is needed for the test app in all five groups, and verify which pipeline installs and runs the tests. No action needed from the vendor now.
There was a problem hiding this comment.
Thanks. For context, doNotInstall is set because the test app depends on Permissions Mock. Sustainability Tests and Excise Taxes Tests have the same dependency and are installed in all five groups, in case that helps.
… names - Slice 651696 [vendor work]
|
A few changes in this push that didn't come from your comments:
None of this changes how files are named. I've also attached a short note on hardening that goes beyond your comments, mainly how the naming context is set and cleared, so you can push back on any of it. |
|
Here's what has changed since I wrote the PR description:
I've attached updated versions of the four documents linked in the description: Design Overview, Design, Technology, and Platform and Application Asks. Report-Filename-Manager-OnePager_20261014.pdf Report-Filename-Manager-Design_20261014.pdf |
|
Could you simplify the comments across the app and tests? The code and method names are mostly clear already. Please remove comments that repeat the code, explain obvious parameters, or record naming discussions and development history. Keep short comments for non-obvious behavior, compatibility constraints, and temporary workarounds. Longer investigation notes can stay in the design documents. |
… names - Slice 651696 [vendor work]
Done. I removed comments that repeated the code or restated parameter names, plus the naming discussions and development history, across the app and the tests. What's left are short notes on non-obvious behaviour, compatibility constraints (such as BaseApp 29 writing extensions upper case and 30 lower case), and the temporary workarounds, with the ask number where there is one. I also checked what remains against the BaseApp source and Microsoft Learn and corrected what didn't hold. These are comment changes only; all 169 tests still pass. |
Move application IDs to 43000-43099 and test IDs to 149500-149599. Update the Report Inbox extension field and manifest ranges without changing behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use Microsoft.Foundation.Reporting.FileNaming for the app and Microsoft.Test.Foundation.Reporting.FileNaming for tests. Add per-file namespace imports without changing object IDs or behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add the app and tests to the 22 country AL-Go configurations generated by Update-CountryProjectSettings.ps1, retaining existing formatting. All 23 country projects pass -Validate. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| "allowDownloadingSource": true, | ||
| "includeSourceInSymbolFile": true | ||
| }, | ||
| "target": "OnPrem", |
There was a problem hiding this comment.
The app manifest targets OnPrem, so this package cannot be submitted or deployed as an AppSource cloud app. Set target to Cloud and remove or replace any OnPrem-only APIs before submission.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.51.6
| local procedure ExpectExcelExportForgotten(var ReportFilenameContext: Codeunit "Report Filename Context"; Situation: Text) | ||
| begin | ||
| if ReportFilenameContext.IsExcelExportRunning() then | ||
| Error(ExcelStateErr, Situation + ', yet the export still counts as running'); |
There was a problem hiding this comment.
The Error call builds an argument by concatenating strings and values before passing it to the error template. Pass the format label and each value as separate Error parameters, using a label-backed format where needed, so translation and diagnostic analysis retain the message template.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.51.6
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
anordmann please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
| begin | ||
| foreach Value in Values do begin | ||
| if Rendered <> '' then | ||
| Rendered += Separator; |
There was a problem hiding this comment.
Joined repeatedly mutates a Text value while traversing the configured values list, creating a new text value for each separator and value. Use a TextBuilder and convert once after the loop to avoid repeated allocations as the number of values grows.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.51.6
|
|
||
| // Commit between proofs so a later failure does not roll back earlier log entries. | ||
| ProveRouteIndependence(); | ||
| Commit(); |
There was a problem hiding this comment.
The proof codeunit commits between its scenarios, but the Subtype = Test methods that invoke it do not declare TransactionModel::AutoCommit. Under the default AutoRollback transaction model, the first Commit can terminate the test instead of producing a business-logic verdict. Mark the invoking commit-driven tests AutoCommit and run them under a TestIsolation-enabled runner so the committed setup does not leak between tests.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.51.6
… names - Slice 651696 [vendor work]
What & why
Adds a new W1 app, Report Filename Manager, with its test app.
An administrator configures a file name pattern, for example
Invoice-[No.]-[Sell-to Customer No.], and one pattern names a report's output on every route: print, preview, save, download, email, Attach as PDF, Send to Disk, the electronic document and its PDF & electronic document zip, scheduled output, the Report Inbox and financial reports. Setup reads compiled metadata only, so it behaves the same in an empty company.The app changes no Base Application code. It reaches each naming point through platform and Base Application events. Where no suitable event exists, the app carries a clearly marked stand-in until Microsoft adds one (see below).
Linked work
AB#651696
No GitHub issue exists; the work is tracked in the ADO slice above.
How I validated this
What I tested and the outcome
base.ruleset.json. I have not yet compiled them against 30.0; this PR's CI is the first 30.0 build.TheMaximalPatternIsIdenticalOnEveryRoute,AnotherSubscribersNameIsNotOverridden,AnOrdinaryUserIsNamedByThePattern,AnApiPdfIsNamedByThePatternOfTheDocumentItRenders) are still being investigated.Account Categories overview-2026-09-28.xlsx; without it, asACC-CAT.xlsx.Stand-ins until Base Application adds an event
Each stand-in is marked in its object's header and is removed when the ask is met.
Report Filename Excel Sub.takes File Management's download over. BecauseDownloadHandlerisScope('OnPrem'), the app targets OnPrem; with the event it can target Cloud.What waits for the platform
[Report Name]can come out in a different language from a document whose language has no translation installed.Asks 4-7 are for the Base Application owners; asks 1-3 for the platform reporting owners. Background reading for review is attached below: Design Overview (one-pager), Design, Technology, and Microsoft Asks, with use cases and measurements.
Risk & compatibility
build/projects.json,build/groups.jsonand the W1 AL-Go settings..xlffiles, so they are not included, and those tests will fail in CI until translations are provided another way.Report Filename Manager Technology.pdf
Report Filename Manager Design Overview.pdf
Report Filename Manager Platform and Application Asks.pdf
Report Filename Manager Design.pdf