Slice 641293: [Expense Agent] Expense Locations and Per-diem rules in configurationAdded more locations

[Bardur Knudsen (BardurKnudsen)]

Adds more locations (basically all country/regions) and corresponding expense rule headers and expense rule conditions.

Fixes AB#641293

[github-actions]

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

BuildRuleSeeds and BuildRuleConditionSeeds now delegate straight to the new seed-builder codeunits, so the shipped local integration events OnBeforeAddRuleSeed, OnBeforeInsertRuleSeed, and OnBeforeAddRuleConditionSeed are no longer raised on this code path. Dependent extensions can still subscribe to local publishers, so this refactor breaks the existing subscriber contract unless the new implementation preserves those event raises.

Knowledge:

• microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Events}$

The shipped [IntegrationEvent] OnBeforeAddCategorySeed changes its existing PaymentMethod parameter from Code[20] to Code[10]. Even though the publisher is local, subscribers still bind to the parameter's type, so this is a breaking event-contract change. Keep the event signature unchanged and handle the narrower storage requirement inside the publisher logic instead.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    local procedure OnBeforeAddCategorySeed(Code: Code[20]; Description: Text[250]; PostingDescription: Text[100]; ExpenseGroupCode: Code[20]; PostingGroupCode: Code[20]; PaymentMethod: Code[20]; IsRefundable: Boolean; IsPrepayment: Boolean; AttachmentEnforcement: Enum "Expense Attachment Enforcement"; DetailRequired: Enum "Expense Detail Needed"; var IsHandled: Boolean)

Knowledge:

• microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

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

This PR adds new setup codeunits — "Create Expense Location" (7104), "Create Expense Rule Condition" (7105), "Create Expense Rule Header" (7107) — which are invoked from "Create Expense Categories"'s setup/defaults flow, but the "Expense Agent - Objects" permission set (which the assignable Expense Agent role includes) grants execute permission for none of them (it only lists "Create Expense Report" among "Create *" codeunits). If any part of the setup/defaults flow runs under the agent user's permission set rather than SUPER, this produces a missing-permission failure. Verify whether these codeunits execute only under an installing/SUPER context; if the agent user can reach this flow, add execute permissions for the three new codeunits.

Knowledge:

• microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4}

[Bardur Knudsen (BardurKnudsen)]

Copilot resolve the merge conflicts in this pull request

[Copilot]

Copilot resolve the merge conflicts in this pull request

Resolved by merge commit 10889b3d9.

Copilot resolve the merge conflicts in this pull request

Resolved by merge commit 10889b3d9.

[alexei-dobriansky]

Good Sense Reviewer - Round 9

Recommendation: Request Changes

What this PR does

This expands the default expense-location catalog and seeds default management rules for per-diem locations. Since round 8, the branch merge brings the PR up to the current base and resolves conflicts in permissions, wizard, and test helper files.

Those merge changes look safe, but they do not change the default location and rule seeders. The current net diff still seeds blank per-diem currency with a fixed daily rate, can create rules for skipped locations, can add generic per-diem rules in AT setup, and can insert default conditions without matching headers.

Status of previous suggestions

ID	Title	Status	Author response
S1	Per-diem defaults lose their currencies and rates	Not addressed	None. Current seed rows still use blank currency and a fixed 100 daily rate for the default per-diem rules.
S2	Conflicting locations can break Apply default settings	Not addressed	None. The apply path still creates rule headers and conditions from all rule seeds after skipped locations are removed.
S3	AT per-diem overrides are now ignored	Not addressed	None. The AT setup still builds generic PER-DIEM rule seeds before adding the country-specific PERDIEMI and PERDIEMA seeds.
S4	Default limits lose their rule headers	Not addressed	None. The header seeder still exits for existing non-per-diem categories, but the condition seeder can still add their conditions.
S5	Posting skips invalid reimbursement lines	Addressed	Still addressed. The current code errors when Reimbursement Type is blank instead of allowing release.

New observations (commits since round 8)

None - the latest merge integrates current base changes and conflict resolutions. I did not find a new review finding in those hunks.

Risk assessment and necessity

Risk: The remaining issues can still seed wrong or incomplete management-rule data. This can make default expense setup validate per-diem expenses against wrong rates, create rules for locations that do not exist, leave localized per-diem setup mixed with generic rules, or create conditions without matching rule headers.

Necessity: The wider location catalog and default per-diem setup are useful feature work. The scope is still right, but the seeded locations, headers, conditions, currencies, rates, and country-specific defaults must stay consistent before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10155 round=9 by=alexei-dobriansky at=2026-09-09T13:55:57Z lastSha=10889b3d931d907198c9748934bf8a3c477f3258 reviewKey=950ba7e7733749a03c693b24ceb193263e30365bad7595b8d7f456778252608d suggestions=S1@7e91c4ab:notaddressed,S2@3b2d9f10:notaddressed,S3@8ac0de55:notaddressed,S4@855f4711:notaddressed,S5@fd7ed5cc:addressed parentRound=8

[Bardur Knudsen (BardurKnudsen)]

Good Sense Reviewer - Round 9

Recommendation: Request Changes

What this PR does
This expands the default expense-location catalog and seeds default management rules for per-diem locations. Since round 8, the branch merge brings the PR up to the current base and resolves conflicts in permissions, wizard, and test helper files.

Those merge changes look safe, but they do not change the default location and rule seeders. The current net diff still seeds blank per-diem currency with a fixed daily rate, can create rules for skipped locations, can add generic per-diem rules in AT setup, and can insert default conditions without matching headers.

Status of previous suggestions
ID Title Status Author response
S1 Per-diem defaults lose their currencies and rates Not addressed None. Current seed rows still use blank currency and a fixed 100 daily rate for the default per-diem rules.
S2 Conflicting locations can break Apply default settings Not addressed None. The apply path still creates rule headers and conditions from all rule seeds after skipped locations are removed.
S3 AT per-diem overrides are now ignored Not addressed None. The AT setup still builds generic PER-DIEM rule seeds before adding the country-specific PERDIEMI and PERDIEMA seeds.
S4 Default limits lose their rule headers Not addressed None. The header seeder still exits for existing non-per-diem categories, but the condition seeder can still add their conditions.
S5 Posting skips invalid reimbursement lines Addressed Still addressed. The current code errors when Reimbursement Type is blank instead of allowing release.
New observations (commits since round 8)
None - the latest merge integrates current base changes and conflict resolutions. I did not find a new review finding in those hunks.

Risk assessment and necessity
Risk: The remaining issues can still seed wrong or incomplete management-rule data. This can make default expense setup validate per-diem expenses against wrong rates, create rules for locations that do not exist, leave localized per-diem setup mixed with generic rules, or create conditions without matching rule headers.

Necessity: The wider location catalog and default per-diem setup are useful feature work. The scope is still right, but the seeded locations, headers, conditions, currencies, rates, and country-specific defaults must stay consistent before merge.

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10155 round=9 by=alexei-dobriansky at=2026-09-09T13:55:57Z lastSha=10889b3d931d907198c9748934bf8a3c477f3258 reviewKey=950ba7e7733749a03c693b24ceb193263e30365bad7595b8d7f456778252608d suggestions=S1@7e91c4ab:notaddressed,S2@3b2d9f10:notaddressed,S3@8ac0de55:notaddressed,S4@855f4711:notaddressed,S5@fd7ed5cc:addressed parentRound=8

Alexey (@alexey) Dobriansky: Can you please close this comment?

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Breaking\ Changes}$

The shipped "Contoso Expense Agent".InsertExpense contract was changed in place by narrowing ExpenseLocationCode from Code[30] to Code[20]. This internal procedure is called across app boundaries (e.g. the AT demo-data app's CreateExpenseAT/CreatePostedExpReportAT codeunits), so any dependent that holds a Code[30] value for this parameter loses the published contract. Keep the existing signature and introduce a new/overloaded entry point if the narrower type is required.

Knowledge:

• microsoft/knowledge/breaking-changes/do-not-change-published-procedure-signatures.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6}

[Bardur Knudsen (BardurKnudsen)]

Good Sense Reviewer - Round 9

Recommendation: Request Changes

What this PR does
This expands the default expense-location catalog and seeds default management rules for per-diem locations. Since round 8, the branch merge brings the PR up to the current base and resolves conflicts in permissions, wizard, and test helper files.

Those merge changes look safe, but they do not change the default location and rule seeders. The current net diff still seeds blank per-diem currency with a fixed daily rate, can create rules for skipped locations, can add generic per-diem rules in AT setup, and can insert default conditions without matching headers.

Status of previous suggestions
ID Title Status Author response
S1 Per-diem defaults lose their currencies and rates Not addressed None. Current seed rows still use blank currency and a fixed 100 daily rate for the default per-diem rules.
S2 Conflicting locations can break Apply default settings Not addressed None. The apply path still creates rule headers and conditions from all rule seeds after skipped locations are removed.
S3 AT per-diem overrides are now ignored Not addressed None. The AT setup still builds generic PER-DIEM rule seeds before adding the country-specific PERDIEMI and PERDIEMA seeds.
S4 Default limits lose their rule headers Not addressed None. The header seeder still exits for existing non-per-diem categories, but the condition seeder can still add their conditions.
S5 Posting skips invalid reimbursement lines Addressed Still addressed. The current code errors when Reimbursement Type is blank instead of allowing release.
New observations (commits since round 8)
None - the latest merge integrates current base changes and conflict resolutions. I did not find a new review finding in those hunks.

Risk assessment and necessity
Risk: The remaining issues can still seed wrong or incomplete management-rule data. This can make default expense setup validate per-diem expenses against wrong rates, create rules for locations that do not exist, leave localized per-diem setup mixed with generic rules, or create conditions without matching rule headers.

Necessity: The wider location catalog and default per-diem setup are useful feature work. The scope is still right, but the seeded locations, headers, conditions, currencies, rates, and country-specific defaults must stay consistent before merge.

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10155 round=9 by=alexei-dobriansky at=2026-09-09T13:55:57Z lastSha=10889b3d931d907198c9748934bf8a3c477f3258 reviewKey=950ba7e7733749a03c693b24ceb193263e30365bad7595b8d7f456778252608d suggestions=S1@7e91c4ab:notaddressed,S2@3b2d9f10:notaddressed,S3@8ac0de55:notaddressed,S4@855f4711:notaddressed,S5@fd7ed5cc:addressed parentRound=8

Dear agent:
S1: By diem defaults should be in local currency, regardless of where we travel.
S2: By design, as getting default locations is only done during initial setup
S3: AT Per diem override: By design. May be revisited later.
S4: Will be analyzed in a subsequent slice.

[alexei-dobriansky]

Good Sense Reviewer - Reply

Thanks for the clarification. I am closing the round-9 points.

• S1 - Agree. The default per-diem rules can use local currency. I withdraw this point.
• S2 - Agree. If this path is only for initial setup by design, I withdraw this point.
• S3 - Agree. I accept that the AT per-diem behavior is by design for this slice. I withdraw this point.
• S4 - Deferred. This will be handled in a later slice, so I will not keep it open on this PR.

The round-9 suggestions are closed.

---

[AI-PR-REPLY] version=1 promptVersion=4 system=github pr=10155 reviewRound=9 by=alexei-dobriansky at=2026-09-10T08:33:04Z respondsTo=2026-09-10T08:21:11Z reviewKey=950ba7e7733749a03c693b24ceb193263e30365bad7595b8d7f456778252608d