Skip to content

[Master] - [Slice 639548] - Consolidate 17 country Expense Agent Demo Data into single W1 app - #10539

Open
v-rohangarg20 wants to merge 29 commits into
mainfrom
features/Slice-639548-Demo-data-for-new-countries-(October)-with-one-w1-app
Open

[Master] - [Slice 639548] - Consolidate 17 country Expense Agent Demo Data into single W1 app#10539
v-rohangarg20 wants to merge 29 commits into
mainfrom
features/Slice-639548-Demo-data-for-new-countries-(October)-with-one-w1-app

Conversation

@v-rohangarg20

@v-rohangarg20 v-rohangarg20 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Work item: AB#639548

What

Consolidate the per-country Expense Agent setup logic (17 countries) into the single W1 Expense Agent (Preview) app.

Changes

  • Move per-country G/L account and category resolution into the W1 Create Expense GL Account / Create Expense Categories codeunits using country-aware name/search/fallback logic (keyed off Company Information Country/Region Code).
  • Retire the per-country ExpenseAgent_<CC> apps from build registrations (build/groups.json, build/projects.json, each build/projects/Apps <CC>/.AL-Go/settings.json) and from app.json internalsVisibleTo; empty the per-country event subscribers.

Account-mapping fixes found during per-country UI testing

  • DK: employee prepayment 26400 (was resolving to 17400); other-refundable 05699.
  • AT: bank 2800, card 2830, travel 7310, rounding 8070 names.
  • NZ: employee prepayment 2510.
  • FR: bank paid 512100 (was 164100); travel refundable 625100 (was 625110).
  • Complete hard-coded fallbacks for the new countries (NL, BE, IT, CH, FI, CZ).

Testing

Verified per country by running each localized web client and checking the Employee/Expense Posting Groups. Create Expense GL Account compiles clean.

Rishukla90 and others added 28 commits August 7, 2026 07:46
…Bug646163-consolidate-country-demo-data-apps-into-one-app
Rename per-country demo data codeunits to Create/Update/Expense naming convention with country-code suffix, organize files into Setup Data/Master Data/Transactions/Historical subfolders, and update object references and variable names accordingly.
…idate-country-demo-data-apps-into-one-app

# Conflicts:
#	build/projects/Apps AT/.AL-Go/settings.json
#	build/projects/Apps AU/.AL-Go/settings.json
#	build/projects/Apps CA/.AL-Go/settings.json
#	build/projects/Apps DE/.AL-Go/settings.json
#	build/projects/Apps DK/.AL-Go/settings.json
#	build/projects/Apps ES/.AL-Go/settings.json
#	build/projects/Apps FR/.AL-Go/settings.json
#	build/projects/Apps GB/.AL-Go/settings.json
#	build/projects/Apps NZ/.AL-Go/settings.json
#	build/projects/Apps US/.AL-Go/settings.json
…Bug646163-consolidate-country-demo-data-apps-into-one-app

# Conflicts:
#	build/projects.json
Per-country demo data (GL accounts, posting/employee groups, categories, subcategories, rules, transactions, historical) for the 7 new countries, and register them in build/projects.json and per-country AL-Go settings. Restrict FindGLAccountByName to Posting accounts to avoid Begin-Total name collisions. Set the CZ Employee Posting Group Payables Account when the localized chart leaves it blank. Ignore the mandatory journal template name check when posting expense reports.
…res/Slice-639548-Demo-data-for-new-countries-(October)

# Conflicts:
#	build/projects/Apps CH/.AL-Go/settings.json
Bound Justification, Payment Method Code, Starting Point and Ending Point assignments with CopyStr to the target Expense field lengths (Text[100], Code[10], Text[50]). These pre-existing overflows surfaced as new warnings when the demo data began building for the newly added countries.
Replace the CopyStr bounding with correctly sized parameters (Justification Text[100], Payment Method Code Code[10], Starting/Ending Point Text[50]) to clear the AA0139 overflow warnings surfaced by building the demo data for the new countries.
Move per-country GL account/category resolution into the W1 Create Expense GL Account codeunit with country-aware name/search/fallback logic; retire per-country Expense Agent apps from build groups/projects/AL-Go settings and app.json internalsVisibleTo. Fix country account mappings surfaced during testing (DK prepayment 26400, DK other 05699, AT bank/card/travel/rounding names, NZ prepayment 2510, FR bank 512100, FR travel 625100, plus new-country fallbacks).
@v-rohangarg20
v-rohangarg20 requested review from a team August 23, 2026 08:13
@v-rohangarg20
v-rohangarg20 requested review from a team as code owners August 23, 2026 08:13
@github-actions github-actions Bot added Build: scripts & configs Build scripts and configuration files AL: Apps (W1) Add-on apps for W1 Finance GitHub request for Finance area labels Aug 23, 2026
@v-rohangarg20 v-rohangarg20 changed the title Consolidate 17 country Expense Agent setups into single W1 app Consolidate 17 country Expense Agent Demo Data into single W1 app Aug 23, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 23, 2026
@v-rohangarg20 v-rohangarg20 changed the title Consolidate 17 country Expense Agent Demo Data into single W1 app [Master] - [Slice 639548] - Consolidate 17 country Expense Agent Demo Data into single W1 app Aug 23, 2026
using Microsoft.Finance.GeneralLedger.Account;
using Microsoft.Foundation.Enums;

codeunit 8325 "Create Exp. GL Account AT"

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 think we should not duplicate same codeunits for each country but have standard structure for W1 and small adjustments for some countries. I expected to see single codeunit "Create Expense G/L Account" with some case statement for "per country" things instead of having 20 copies per each country which are very similar. This is not a consolidation but aggregation of several extension objects into one app. Maintenance of such design will require changes in 20+ places for every common change for all countries. Can we build common structure like for AT, but

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Accept with Suggestions

What this PR does

This is a Slice (AB#639548). It consolidates the 17 per-country Expense Agent apps into the single W1 Expense Agent (Preview) app. The per-country ExpenseAgent_<CC> apps are retired from build/groups.json, build/projects.json, the per-project .AL-Go/settings.json files, and app.json internalsVisibleTo; their event subscribers are emptied. The country logic moves into Create Expense GL Account and Create Expense Categories, keyed off Company Information Country/Region Code, plus per-country demo data.

The account-resolution design is sound. Each account resolver first calls FindExistingExpenseAccount (a case-insensitive name/search lookup against the live chart of accounts), then a case GetCountryCode() map of hard-coded numbers, and finally a trailing W1 default exit(...). GetCountryCode() returns an empty code when Company Information is missing, so an unmapped country falls through to the W1 default safely - there is no missing-fallback gap. The ES-only balance-account write is guarded with RecordRef.FieldExist(10700), so it does not create a hard dependency on the ES layer. CI builds are green for all 17 country apps (Verify App Changes and every Build Apps <CC> Default/Clean passed), which is the main guard for the build-registration retirement.

Suggestions

S1 - Journal template check now bypassed for all expense posting
ExpenseReportPost adds GenJnlPostLine.SetIgnoreJournalTemplNameMandatoryCheck(); with no condition, so it runs on every expense report post in the shipping app, not only for demo data. Please confirm this is intended for all expense posting. If it is, a short posting test with Journal Templ. Name Mandatory turned on in G/L Setup would lock the behavior in.

S2 - ES path skips G/L account indentation
For ES the code now calls UpdateIncomeStatementBalanceAccount() instead of GLAccountIndent.Indent();. This means the ES demo chart no longer gets re-indented. Please check that ES totals and indentation are still correct, or run Indent() as well as the balance-account update.

S3 - No automated test for the new country resolution
The country-aware account and category seeding was verified by hand per country. A small smoke test that runs Create Expense GL Account and Create Expense Categories for one or two countries and asserts the seeded accounts are not blank would protect this setup path from future regressions. This is non-blocking.

S4 - Check the "Subsitence" spelling
ExpenseSubsistenceLbl and its search label both use 'Subsitence' (missing an "s"). If this is meant to match an existing localized account name that has the same typo, keep it and add a short comment. If not, it should read Subsistence.

Risk assessment and necessity

Risk: The change is large (100 files) but the bulk is additive demo-data codeunits and build-registration removals, both low blast radius. The two behavior changes in product code are the S1 posting-check bypass (affects every expense post, not just demo) and the S2 ES indentation branch. The retirement of per-country apps from groups.json / projects.json / internalsVisibleTo is validated by the green multi-country CI build. One CI check, For Microsoft: Validate link to internal work items, reports FAILURE, but sibling copies of the same check pass and it carries no code-risk signal.

Necessity: Justified. Collapsing 17 country apps into one W1 app removes heavy duplication and is the right channel for a Slice. The scope is appropriate for a consolidation, and the additive demo data plus country map is a reasonable way to keep per-country behavior. No High-confidence blocker was found, so this is Accept with Suggestions.


[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10539 round=1 by=alexei-dobriansky at=2026-08-23T13:09:29Z lastSha=acfc0b6103356570ad7f674b1e3ae5d85c3aa4d6 reviewKey=763e5aa35a023bffceccc29daeec9b0d1ed0ca4da1c069053a77f3d5af966ef8 suggestions=S1@1dad0ca5,S2@32960275,S3@0f327380,S4@d3413573

CreateGLAccount ran GLAccountIndent.Indent() for any non-ES company, including uninitialized/test companies with no Country/Region Code (e.g. 'My Company'). In a localized container (ES) the localized G/L Account-Indent rejects the W1 default expense accounts (e.g. 10390) with 'Missing group', failing the PreviewPostingOfExpenseReportWhenDemoDataIsExecuted test. Only re-indent when a country code is set.
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Upgrade}$

This PR marks the country-specific ExpenseEventSubscriber<CC> codeunits ObsoleteState = Pending (ObsoleteTag 29.0) but, in the same PR, removes the corresponding Expense Agent (Preview) (<CC>) and Expense Agent Demo Data (<CC>) app projects entirely from build/projects.json and build/groups.json. If those app packages stop being built/shipped in this same release, tenants who already installed the old per-country preview/demo-data apps will never receive a package containing the staged Pending marker and cannot follow the normal Pending -> Removed upgrade path; they are left on the old binaries indefinitely or the extension disappears from their environment without an obsoletion-driven upgrade step.

Knowledge:

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 2

Recommendation: Request Changes

What this PR does

This is round 2 for Slice AB#639548. The new commit since round 1 only changes Create Expense GL Account, so blank-country companies no longer run GLAccountIndent.Indent().

The prior suggestions are not fixed by that change. The unconditional journal template bypass is still present, the ES branch still skips Indent(), no automated smoke test was added, and the Subsitence label spelling is still unchanged. While re-checking the current head, I also found one upgrade blocker in the full PR diff: the country-specific Expense Agent app projects are removed from build registration in the same PR that only marks their codeunits as ObsoleteState = Pending.

Status of previous suggestions
ID Title Status Author response
S1 Journal template check now bypassed for all expense posting Not addressed No author reply or matching code change.
S2 ES path skips G/L account indentation Not addressed The new commit changes the non-ES else branch only; ES still calls only UpdateIncomeStatementBalanceAccount().
S3 No automated test for the new country resolution Not addressed No new test files or smoke tests were added.
S4 Check the "Subsitence" spelling Not addressed The labels still use Subsitence.
New observations

S5 - Retired country apps skip obsolete staging
build/projects.json removes the country-specific Expense Agent app projects while those apps only mark their codeunits as ObsoleteState = Pending. Keep building and shipping a 29.0 package that contains the Pending markers, or use the normal app retirement path. Otherwise tenants with the old preview apps cannot get a normal Pending-to-Removed upgrade step.

Risk assessment and necessity

Risk: The new round-2 code change is small, but it does not resolve the earlier ES indentation concern. The larger current-head risk is upgrade compatibility: removing the retired country apps from build registration can stop existing installed preview apps from receiving the staged obsolete package. App build checks are mostly green or still running, but green builds do not prove the upgrade path is safe.

Necessity: The W1 consolidation is still justified and is the right direction for this Slice. The scope is reasonable, but the country app retirement needs a safe staging plan before merge. After that is fixed, the remaining prior suggestions are non-blocking.


[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10539 round=2 by=alexei-dobriansky at=2026-08-23T19:11:39Z lastSha=5a1499b32cabb66642da4e2f656aeab65dbe1668 reviewKey=d4c44376ccfafe757562c1f6c1f7c6abdc7457079e2dec62de7fc85b8ca8da28 suggestions=S1@1dad0ca5:notaddressed,S2@32960275:notaddressed,S3@0f327380:notaddressed,S4@d3413573:notaddressed,S5@bf4e80f3:new parentRound=1

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

Labels

AL: Apps (W1) Add-on apps for W1 Build: scripts & configs Build scripts and configuration files Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants