Skip to content

[API tests] Align sales quote document and posting dates - #11227

Draft
Prangshuman Das (t-prda) wants to merge 5 commits into
prdas/646383-split-italian-discountfrom
prdas/646383-split-quote-dates
Draft

Prangshuman Das (t-prda) wants to merge 5 commits into
prdas/646383-split-italian-discountfrom
prdas/646383-split-quote-dates

Conversation

@t-prda

@t-prda Prangshuman Das (t-prda) commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Fixes AB#653124

Scope

Validate Posting Date and Document Date against WorkDate in the shared V1/V2 sales-quote fixtures. This is separate from the license-safe WorkDate helper now inherited from Expense-first #12325.

Related umbrella646383.

Evidence and limits

Two observed IT failures in saved result set 2: CU139723/139823.TestActionMakeOrderFromQuote, with Document Date greater than Posting Date. The helper has five test callers per codeunit; do not equate those ten callers with ten observed failures.

Only this layer's original fix/exclusion patch is reviewed here (4 paths); the patch is unchanged by Expense-first restructuring.

Exact head: 71e2190c672484cc3ceb6c85d9700687a8ff4e21; tree: 32624cadd6af63614106b4e04050dc1f51326a90.

Expense-first sequencing using existing exclusions

The same 193 method-specific temporary exclusions (135 APIV1 +58 APIV2 across12 codeunits) now live in the existing src/DisabledTests/_Exclude_APIV1__Tests/_Exclude_APIV1__Tests.DisabledTest.json and src/DisabledTests/_Exclude_APIV2__Tests/_Exclude_APIV2__Tests.DisabledTest.json. No separate sequencing files remain. Original entries retain their order/style/content; no new duplicate or wildcard/codeunit-wide exclusion is introduced. Unrelated pre-existing APIV2 duplicates are preserved rather than mixed with this change.

#11860 removes the temporary additions from these same existing manifests alongside its already-reviewed authentication uptake. All193 temporary stage entries are removed in #11860, but it makes 192/193 target methods eligible: the independent 139739::TestDeleteInUse exclusion remains until the VAT fixture fix PR #11224 removes it. From #11224 onward all193 are eligible; eligibility is not runtime success. All general/downstream cumulative trees are exactly byte-identical to the preceding checkpoint; the full checkpoint has none of these193 methods excluded. No app-name allowlist, selection setting, runner/authentication/test/production change or Logiq exclusion is added.

Why these methods started executing: they already required Disabled isolation and were IntegrationTest codeunits. Ordinary typed selection in TestSuiteMgt332–352 selects None|Codeunit; the old extra Disabled pass in RunTestsInBcContainer was UnitTest-only. The clean-execution switch newly reaches Disabled IntegrationTest codeunits and still honors the existing JSON exclusions. These193 methods were not listed in those exclusions. This is a pre-existing selection gap, not a newly introduced product auth bug or proof they never ran in any historical configuration. A complete67-codeunit audit preserves existing UnitTest/Legacy behavior and the ten independently handled Logiq integration tests.

**Expense scope is unchanged:**51 API reenables, nine non-API exclusions, baseline six cases and all consolidated regressions. The two legacy Spend Requests methods remain conditional on not CLEAN30. Focused #12325 review:21 files, including the two existing exclusion manifests. General #11860 review:159 files.

The official NAV Disable-NAVALTest helper was inspected: it has no destination/app-file parameter, writes NAV's App/DisabledTests using per-codeunit filenames and sorts entries. It cannot safely preserve these BCApps files. A bounded JSON relocation preserved original prefixes/order and checked exact identities, then exercised the unchanged real loader. No new shared helper/framework or NAV selector change was made.

Validation and presentation evidence

Pester117/117 passed independently at the new Expense/general/full heads. Actual existing-loader checks confirm identical effective193-method selection after relocation and preserved51/9 Expense scope. All nine downstream Git tree hashes remain exactly unchanged. Fresh exact-head CI is pending; old-head successes are not substituted for current runtime evidence.

Historical run37330893173 at head88881be reached193 general methods:171 genuine401 failures and22 nominal passes (17 bare-ASSERTERROR cases can accept the wrong error; five local fixtures). Separately, all51 Expense methods passed in13 inspected countries (663 results), and all63 touched API methods yielded819 results; Activity coverage was117/198 at that checkpoint. These are bounded historical results, not a full/current matrix pass. The preceding stage-fix runs hit50 hosted-runner acquisition cancellations; other jobs were active, so this was not a claimed global outage. New pushes schedule fresh CI, not an AL retry. The general SQL-pool NRE and missing warning-reference artifacts remain separate unresolved limitations.

Durable session presentation notes: api-test-enablement-presentation-notes.md, with source-pinned selection proof, fix/coverage inventory, helper limitations and historical-vs-current evidence boundaries. Fresh runtime proof must still establish the51 Expense cases and actual193-case suppression at Expense stage.

Related umbrella646383. Native stack #12327 remains #12325 → #11860 → #11224 → #11225 → #11226 → #11227 → #11228 → #11229 → #11230 → #11322. Protected merged #10085/#11862/#11891 and validation #11892 are untouched; validation-only #11861/#11455 remain Do Not Merge. No new main integration, native-group/base change, PR merge, queue operation, Actions cancellation or manual retry was made. All prior heads remain ancestors; source publication used backups and an atomic forward-only push with explicit leases.

Exact current head: 71e2190c672484cc3ceb6c85d9700687a8ff4e21; source tree: 32624cadd6af63614106b4e04050dc1f51326a90.

@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Sep 8, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 8, 2026
@github-actions github-actions Bot added the Team: Integrations GitHub request for Integrations area label Sep 8, 2026
@t-prda
Prangshuman Das (t-prda) changed the base branch from main to prdas/646383-split-italian-discount September 8, 2026 17:06
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

@t-prda
Prangshuman Das (t-prda) removed this pull request from stack #11232 September 24, 2026 15:27
@t-prda
Prangshuman Das (t-prda) added this pull request to stack #11863 September 24, 2026 15:27
@t-prda
Prangshuman Das (t-prda) removed this pull request from stack #11863 September 25, 2026 08:46
@t-prda
Prangshuman Das (t-prda) added this pull request to stack #11893 September 25, 2026 08:47
@github-actions

Copy link
Copy Markdown
Contributor

Issue #11561 is not valid. Please make sure you link an issue that exists, is open and is approved.

@t-prda Prangshuman Das (t-prda) added the Team: SCM GitHub request for SCM area label Sep 28, 2026
@github-actions github-actions Bot removed the Team: SCM GitHub request for SCM area label Sep 28, 2026
@t-prda Prangshuman Das (t-prda) added the Team: SCM GitHub request for SCM area label Sep 28, 2026
@t-prda
Prangshuman Das (t-prda) marked this pull request as ready for review September 28, 2026 11:03
@t-prda
Prangshuman Das (t-prda) requested a review from a team September 28, 2026 11:03
@t-prda
Prangshuman Das (t-prda) requested a review from a team as a code owner September 28, 2026 11:03
@github-actions github-actions Bot removed the Team: SCM GitHub request for SCM area label Sep 28, 2026
@t-prda Prangshuman Das (t-prda) added the Team: SCM GitHub request for SCM area label Sep 28, 2026
@github-actions github-actions Bot removed the Team: SCM GitHub request for SCM area label Sep 28, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept

What this PR does

This change aligns the posting and document dates with WorkDate() in the shared V1 and V2 sales-quote fixtures, then re-enables the two make-order tests. Both fixtures set and persist the dates before the API action, which prevents country-specific validation from rejecting a quote whose document date is later than its posting date. Production behavior is unchanged.

Problem-solution fit

Fit: Strong

The change directly corrects the invalid test setup that caused the two make-order scenarios to fail. The V1 and V2 implementations remain aligned, and the removed exclusions match the corrected scenarios.

Suggestions

No suggestions.

Risk assessment and necessity

Risk: Low. The change is limited to test fixtures and exclusion metadata. The shared fixture also supports other quote tests, but none depend on different posting and document dates, and the completed validation passed without failures.

Necessity: The date alignment is required to keep the fixture valid under country-specific sales-document rules. Removing only the two related exclusions keeps the scope precise.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11227 round=1 by=alexei-dobriansky at=2026-09-28T22:21:35Z lastSha=45dafcb71dc550a919cc20fa744773aaffdba81f reviewKey=d77ec6b6d0124d2f36b8325ef9a699b9947deb8de4a8dcb7b68133896a33db67 suggestions=

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Accept

What this PR does

Since round 1, the branch only integrated changes from its stacked parent and base. The owned added and removed spans remain unchanged: the shared V1 and V2 fixtures align both dates with WorkDate(), and the two matching make-order tests are re-enabled. The date validations remain correctly ordered and persisted before the API action.

Status of previous suggestions

No previous suggestions.

New observations (commits since round 1)

None - the new commits only reconcile the stacked parent and base; no new owned code spans were introduced.

Risk assessment and necessity

Risk: Low. The change remains limited to test fixtures and exclusion metadata. The shared fixture is used by five test procedures, but using valid matching dates is consistent across those scenarios. Current-head CI was still running, so no new runtime result was available.

Necessity: The alignment keeps the quote fixture valid for localized date validation and re-enables exactly the two affected test methods without changing production behavior.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11227 round=2 by=alexei-dobriansky at=2026-10-02T22:27:30Z lastSha=98f7cde6da49a05fd596c48ba7a64257fec182e4 reviewKey=c963092289006476b795a3dd035ea3612b34e8582c965cce25976d936d0213d7 suggestions=none parentRound=1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session:3952f078-a881-4da8-ad96-13b727e48a91
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Issue #11224 is not valid. Please make sure you link an issue that exists, is open and is approved.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 3

Recommendation: Accept

What this PR does

Since round 2, the stacked branch was rewritten to inherit updated parent sequencing. The net change remains the same four paths: the shared V1 and V2 sales-quote fixtures align posting and document dates with WorkDate(), and the two matching make-order tests are removed from the exclusion manifests.

Status of previous suggestions

No previous suggestions.

New observations (commits since round 2)

None - the rewritten stack preserves the previously reviewed fix. The posting and document date validations are safe for quote records, and the exclusions still remove exactly the two corrected test methods.

Risk assessment and necessity

Risk: Low. The change remains limited to test setup and exclusion metadata. The shared fixture affects several quote scenarios, but assigning equal valid dates is compatible with those scenarios and does not change production behavior. Current-head CI is still in progress; the visible failure is the issue-link validation rather than a code failure.

Necessity: The date alignment prevents localized conversion validation from rejecting quotes whose document date is later than the posting date. Removing the two exclusions restores coverage for the affected V1 and V2 make-order scenarios.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11227 round=3 by=alexei-dobriansky at=2026-10-05T22:55:29Z lastSha=71e2190c672484cc3ceb6c85d9700687a8ff4e21 reviewKey=d857b74da2dcf0ca5c3412ccfb6e4585d1a5d08fe493a693514ac346b465488b suggestions=none parentRound=2

melnikbo pushed a commit to melnikbo/BCApps that referenced this pull request Oct 7, 2026
…microsoft#12325)

## Scope

Related authentication and API enablement:
[AB#646383](https://dynamicssmb2.visualstudio.com/Dynamics%20SMB/_workitems/edit/646383)
(link only; this PR does not resolve the umbrella item).

Fixes
[AB#653119](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/653119)

Separate test defect: [653119 - Policy snapshot API URL composition and
response
isolation](https://dynamicssmb2.visualstudio.com/Dynamics%20SMB/_workitems/edit/653119).
This tracks the policy-snapshot test corrections, not production access
permissions or unrelated runtime failures.

Expense-first adoption directly against main after the external squash
merge of workflow microsoft#11891; remaining general APIs follow in microsoft#11860.

- Migrate the 11 reviewed Expense API/helper paths to shared
authentication and remove exactly 51 existing Expense API exclusions.
Preserve exactly nine non-API Expense exclusions; all 19 previously
retained API exclusions are now removed.
- Enable the existing `enableCleanTestCodeunitExecution` boolean. Use
existing DisabledTests selectors throughout discovery, ordinary
execution and clean-codeunit execution/reruns. No app-name allowlist,
new selection setting or runner implementation/test change.
- Include the six-line license-safe WorkDate helper needed by PerDiem,
five query-safe policy URL compositions, four independent response
clears, and the existing unlimited-approval fixture. Preserve all nine
Activity Log tests and upstream assertions.
- Consolidate microsoft#11453's exact assigned-user filter (no cross-table range
compression), true fallback fixture and regressions. Upstream wildcard
quoting alone did not fix the range-gap case.
- Consolidate microsoft#11451's unique denied-approval error/permission capture
and microsoft#11454's existing repeated-delta, missing-header-permission,
Released-status and real deletion-total regressions, including its
internal test-only permission set.
- Do not reintroduce upstream repairs from microsoft#11654 (three permission-role
tests and posted zero-amount fixture), the obsolete date test removed by
microsoft#12074, or the production indirect-Modify change already supplied by
microsoft#11333. microsoft#11452's duplicate deletion helper is absent; existing callers
use the upstream shared helper.
- All seven artificial Expense exclusions introduced by the previous
uptake are absent here and at every general checkpoint. This is not
blanket re-enablement of Expense tests.

This focused review diff has21 paths, using the two existing API
exclusion manifests. No NAV selectors, local NST, BC-ExpenseAgent
changes, unrelated new test scope or production permission changes.

Exact head: `bd3bbc04e13965f967a26eef435bdbf0cbc09ad7`; tree:
`2b8c47b138846f63bd49d47188f54ea7e86c1f24`.

## Expense-first sequencing using existing exclusions

The same **193 method-specific temporary exclusions (135 APIV1 +58 APIV2
across12 codeunits)** now live in the existing
`src/DisabledTests/_Exclude_APIV1__Tests/_Exclude_APIV1__Tests.DisabledTest.json`
and
`src/DisabledTests/_Exclude_APIV2__Tests/_Exclude_APIV2__Tests.DisabledTest.json`.
No separate sequencing files remain. Original entries retain their
order/style/content; no new duplicate or wildcard/codeunit-wide
exclusion is introduced. Unrelated pre-existing APIV2 duplicates are
preserved rather than mixed with this change.

microsoft#11860 removes the temporary additions from these same existing
manifests alongside its already-reviewed authentication uptake. All193
temporary stage entries are removed in microsoft#11860, but it makes **192/193
target methods eligible**: the independent `139739::TestDeleteInUse`
exclusion remains until the VAT fixture correction in PR
[microsoft#11224](microsoft#11224) removes it.
From microsoft#11224 onward all193 are eligible; eligibility is not runtime
success. All general/downstream cumulative trees are **exactly
byte-identical** to the preceding checkpoint; the full checkpoint has
none of these193 methods excluded. No app-name allowlist, selection
setting, runner/authentication/test/production change or Logiq exclusion
is added.

**Why these methods started executing:** they already required Disabled
isolation and were IntegrationTest codeunits. Ordinary typed selection
in TestSuiteMgt332–352 selects None|Codeunit; the old extra Disabled
pass in RunTestsInBcContainer was UnitTest-only. The clean-execution
switch newly reaches Disabled IntegrationTest codeunits and still honors
the existing JSON exclusions. These193 methods were not listed in those
exclusions. This is a pre-existing selection gap, not a newly introduced
product auth bug or proof they never ran in any historical
configuration. A complete67-codeunit audit preserves existing
UnitTest/Legacy behavior and the ten independently handled Logiq
integration tests. Historical baseline run37215976231 at
head69df41756d918a516a3c868ca66f45cbfd320428 independently corroborates
this: zero of the exact193 methods appear across five inspected W1
result artifacts containing37,626 testcases (Integration, both Legacy
buckets, default/unit and Uncategorized). This evidence is limited to
that W1 baseline, not every country or historical run. The completed
alternate-path audit found no other ordinary configured baseline BCApps
lane: APIV1/APIV2 are outside Legacy buckets, Disabled-unit fallback
retains UnitTest filtering, discovery skips test procedures, and
ordinary PR/CI/CD/rerun routes do not bypass those constraints. Manual
or explicitly untyped execution remains possible; no universal
historical or NAV absence is claimed.

**Expense scope is unchanged:**51 API reenables, nine non-API
exclusions, baseline six cases and all consolidated regressions. The two
legacy Spend Requests methods remain conditional on not CLEAN30. Focused
microsoft#12325 review:21 files, including the two existing exclusion manifests.
General microsoft#11860 review:159 files.

The official NAV `Disable-NAVALTest` helper was inspected: it has no
destination/app-file parameter, writes NAV's App/DisabledTests using
per-codeunit filenames and sorts entries. It cannot safely preserve
these BCApps files. A bounded JSON relocation preserved original
prefixes/order and checked exact identities, then exercised the
unchanged real loader. No new shared helper/framework or NAV selector
change was made.

## Validation and presentation evidence

**Pester117/117 passed independently at the new Expense/general/full
heads.** Actual existing-loader checks confirm identical
effective193-method selection after relocation and preserved51/9 Expense
scope. All nine downstream Git tree hashes remain exactly unchanged.
Fresh exact-head CI is pending; old-head successes are not substituted
for current runtime evidence.

Historical run37330893173 at head88881be reached193 general methods:171
genuine401 failures and22 nominal passes (17 bare-ASSERTERROR cases can
accept the wrong error; five local fixtures). Separately, all51 Expense
methods passed in13 inspected countries (663 results), and all63 touched
API methods yielded819 results; Activity coverage was117/198 at that
checkpoint. These are bounded historical results, not a full/current
matrix pass. The preceding stage-fix runs hit50 hosted-runner
acquisition cancellations; other jobs were active, so this was not a
claimed global outage. New pushes schedule fresh CI, not an AL retry.
The general SQL-pool NRE and missing warning-reference artifacts remain
separate unresolved limitations.

Durable session presentation notes:
**api-test-enablement-presentation-notes.md**, with source-pinned
selection proof, fix/coverage inventory, helper limitations and
historical-vs-current evidence boundaries. Fresh runtime proof must
still establish the51 Expense cases and actual193-case suppression at
Expense stage.


[AB#646383](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/646383)
(link only). Native stack #12327 remains microsoft#12325 → microsoft#11860 → microsoft#11224 →
microsoft#11225 → microsoft#11226 → microsoft#11227 → microsoft#11228 → microsoft#11229 → microsoft#11230 → microsoft#11322. Protected
merged microsoft#10085/microsoft#11862/microsoft#11891 and validation microsoft#11892 are untouched;
validation-only microsoft#11861/microsoft#11455 remain Do Not Merge. No new main
integration, native-group/base change, PR merge, queue operation,
Actions cancellation or manual retry was made. All prior heads remain
ancestors; source publication used backups and an atomic forward-only
push with explicit leases.

Exact current head: `bd3bbc04e13965f967a26eef435bdbf0cbc09ad7`; source
tree: `2b8c47b138846f63bd49d47188f54ea7e86c1f24`.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
@t-prda
Prangshuman Das (t-prda) marked this pull request as draft October 7, 2026 13:12

This branch was successfully deployed

1 active (outdated) deployment
triage — 45dafcb7 Deployed Sep 28, 2026 by t-prda via Classify team ownership #5945
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 Team: Integrations GitHub request for Integrations area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants