Repository navigation
[API tests] Assign a number series to API vendor payment journals - #12681
Open
Prangshuman Das (t-prda) wants to merge 1 commit into
Open
Prangshuman Das (t-prda) wants to merge 1 commit into
Prangshuman Das (t-prda) wants to merge 1 commit into
Conversation
This was referenced Oct 12, 2026
github-actions
Bot
added
the
Team: Integrations
Prangshuman Das (t-prda)
changed the base branch from
features/646383-stack-d4-quote-dates
to
main
October 12, 2026 11:13
github-actions
Bot
added
Build: Automation
Prangshuman Das (t-prda)
force-pushed
the
features/646383-stack-d5-journal-numbering
branch
from
October 12, 2026 11:28
d530f68 to
169f50c
Compare
Prangshuman Das (t-prda)
changed the base branch from
main
to
features/646383-stack-d4-quote-dates
October 12, 2026 12:29
Prangshuman Das (t-prda)
added this pull request to stack #12697
October 12, 2026 15:38
Prangshuman Das (t-prda)
marked this pull request as ready for review
October 13, 2026 08:58
Prangshuman Das (t-prda)
force-pushed
the
features/646383-stack-d5-journal-numbering
branch
from
October 14, 2026 09:27
169f50c to
74d4f2e
Compare
github-actions
Bot
removed
Build: Automation
Prangshuman Das (t-prda)
force-pushed
the
features/646383-stack-d5-journal-numbering
branch
from
October 14, 2026 09:48
74d4f2e to
761f655
Compare
Prangshuman Das (t-prda)
force-pushed
the
features/646383-stack-d5-journal-numbering
branch
from
October 14, 2026 10:30
761f655 to
4c8e2aa
Compare
Prangshuman Das (t-prda)
force-pushed
the
features/646383-stack-d5-journal-numbering
branch
from
October 14, 2026 14:04
4c8e2aa to
e7891c9
Compare
Contributor
Good Sense Reviewer - Round 2Recommendation: AcceptWhat this PR doesThe change assigns a number series when a generated vendor-payment journal batch has none, then re-enables the two affected API tests. The batch exists before it is read, existing configuration is preserved, and the created series supports automatic document numbering without changing production behavior. Status of previous suggestionsNo suggestions were raised in round 1. New observations (commits since round 1)None. The full current diff was reviewed because the earlier commit is no longer reachable. Risk assessment and necessityRisk: The change is limited to test infrastructure and the disabled-test list. It affects all callers of the shared vendor-payment journal helper, but only fills missing number-series configuration and preserves existing values. Necessity: A number series is required for API-created vendor payments with a blank document number. The change restores deterministic setup and returns the two regression scenarios to normal execution.
|
Prangshuman Das (t-prda)
removed this pull request from stack #12697
October 15, 2026 08:59
Prangshuman Das (t-prda)
added this pull request to stack #12891
October 15, 2026 09:00
Prangshuman Das (t-prda)
force-pushed
the
features/646383-stack-d5-journal-numbering
branch
from
October 15, 2026 09:02
e7891c9 to
5195b56
Compare
Give the vendor payments batch a number series when it has none and re-enable the vendor payment and apply vendor entry tests. AB#646383 AB#653393 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
Prangshuman Das (t-prda)
force-pushed
the
features/646383-stack-d5-journal-numbering
branch
from
October 15, 2026 12:56
5195b56 to
f0799f2
Compare
Contributor
Good Sense Reviewer - Round 3Recommendation: AcceptWhat this PR doesThe change assigns a number series when a generated vendor-payment journal batch has none, then re-enables the two affected API tests. The full current diff was reviewed because the earlier reviewed commit is no longer reachable; the replacement commit keeps the same targeted behavior, preserves existing batch configuration, and supplies automatic document numbering only where it is missing. Status of previous suggestionsNo suggestions were raised in round 2. New observations (commits since round 2)None. The full current diff was reviewed because the earlier commit is no longer reachable. Risk assessment and necessityRisk: The change is limited to shared test setup and the disabled-test list. It affects all tests that use the vendor-payment journal helper, but it only fills an empty number-series field and preserves any existing value. Necessity: Automatic document numbering is required when the API creates a vendor payment without a document number. This setup fix restores that prerequisite and returns both affected regression scenarios to normal execution.
|
dominicstarkl
pushed a commit
to dominicstarkl/BCApps
that referenced
this pull request
Oct 15, 2026
Review focus: ``MicrosoftTestAuthProvider`` (about 60 lines) and the enum default; the ``build/`` changes are deletions plus the anti-SSRF line in ``NewBcContainer.ps1``. Defaults API test authentication (`Library - Graph Mgt`) to the current user's web service key. API tests no longer need per-app auth uptake or a container password. - `API Test Authentication` enum: `DefaultImplementation` = `Microsoft Test Auth Provider`. Removes `No API Test Auth Provider` and the explicit `Microsoft Test Environment` value. - `Microsoft Test Auth Provider`: on-premises with NavUserPassword, sends Basic auth with `UserId()` and the web service key. When the key is missing or expired, a background session (`API Test Web Service Key`, CU 131025) creates it with a 24h expiry and commits it, because the API request runs in a separate server session. The caller's transaction is never committed; the provider waits up to 60 s for the session and then re-reads the key. - `Library - Graph Mgt`: adds the `SetLicenseSafeWorkDate` and `AddFieldToIgnoreIfExists` helpers used by later layers. - Build: removes the container password and Key Vault credential plumbing (`ApiTestCredential.psm1`, `Remove-ApiTestPassword.ps1`, `PipelineFinalize` hooks and their tests). Keeps the clean-tenant execution logic. Sets `NavHttpClientAntiSSRFEnabled=false` in the test container, as NAV `RunALTests.ps1` does: since microsoft#12323, API tests call the container's own endpoint through AL `HttpClient`, which anti-SSRF blocks. - CU 139494 `API Test Auth Provider Tests`: covers key creation and renewal. Declares `TestHttpRequestPolicy = AllowAllOutboundRequests` (NAV blocks outbound requests by default; microsoft#12694 covered the other API tests). Related: [AB#646383](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/646383), [AB#653393](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/653393) ### Stack (each PR shows only its own layer; merge bottom first) 1. microsoft#12674 A: web service key auth (base: `main`) 2. microsoft#12675 B: Expense API tests + clean test codeunit execution 3. microsoft#12676 C: re-enable APIV1/APIV2 API tests 4. microsoft#12890 Review fixes for A: simplify web service key handling 5. Area fixes: microsoft#12677 D1 VAT fixtures → microsoft#12678 D2 cancellation reasons → microsoft#12679 D3 localized discounts → microsoft#12680 D4 quote dates → microsoft#12681 D5 journal numbering → microsoft#12682 D6 field comparison → microsoft#12683 D7 PDF documents → microsoft#12684 D8 APIV1 reliability Experiments (on D8, do not merge): microsoft#12685 E single tenant, microsoft#12686 F no clean execution. Combined validation (do not merge): microsoft#12687, same tree as D8. Platform: `main` already pins BCPlatform 30.0.56112.0, which includes the SQL connection-pool fix ([AB#653457](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/653457)). Note for CI: AL-Go's new-warning check needs a CI/CD baseline from the base branch, and `features/*` bases have none, so a PR build there fails with a 404. The current CI runs were triggered while these PRs targeted `main`, so their results are valid for the cumulative change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
dominicstarkl
pushed a commit
to dominicstarkl/BCApps
that referenced
this pull request
Oct 15, 2026
Review focus: the Expense test changes and the auth-helper deletion; the ``enableCleanTestCodeunitExecution`` line. The large JSON diffs are temporary disables, undone in C. Re-enables the Expense Agent API tests on top of the default web service key auth. No Key Vault password and no auth subscriber. - Deletes `Expense API Test Auth Helper` and its usages. - Re-applies the Expense fixes from microsoft#12325 (reverted in microsoft#12514): `TravelRequestApproval`, Expense permission and spend-request tests, the `ExpDetailTest` permission set, and `SpendRequestTests`. - Turns on `enableCleanTestCodeunitExecution`, as microsoft#12325 did, so `RequiredTestIsolation = Disabled` codeunits run in clean-tenant lanes. - Expense disabled tests: keeps only the entries NAV master also disables (12). - Temporarily disables the unrelated APIV1/APIV2 tests that the clean lanes start executing, the same entries microsoft#12325 added. They are re-enabled in the next layers. Related: [AB#646383](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/646383), [AB#653393](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/653393) ### Stack (each PR shows only its own layer; merge bottom first) 1. microsoft#12674 A: web service key auth (base: `main`) 2. microsoft#12675 B: Expense API tests + clean test codeunit execution 3. microsoft#12676 C: re-enable APIV1/APIV2 API tests 4. microsoft#12890 Review fixes for A: simplify web service key handling 5. Area fixes: microsoft#12677 D1 VAT fixtures → microsoft#12678 D2 cancellation reasons → microsoft#12679 D3 localized discounts → microsoft#12680 D4 quote dates → microsoft#12681 D5 journal numbering → microsoft#12682 D6 field comparison → microsoft#12683 D7 PDF documents → microsoft#12684 D8 APIV1 reliability Experiments (on D8, do not merge): microsoft#12685 E single tenant, microsoft#12686 F no clean execution. Combined validation (do not merge): microsoft#12687, same tree as D8. Platform: `main` already pins BCPlatform 30.0.56112.0, which includes the SQL connection-pool fix ([AB#653457](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/653457)). Note for CI: AL-Go's new-warning check needs a CI/CD baseline from the base branch, and `features/*` bases have none, so a PR build there fails with a 404. The current CI runs were triggered while these PRs targeted `main`, so their results are valid for the cumulative change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
dominicstarkl
pushed a commit
to dominicstarkl/BCApps
that referenced
this pull request
Oct 15, 2026
Review focus: mostly mechanical. ``Initialize`` sets a license-safe work date in each API test, and the JSON deletions re-enable tests. A few files have small fixture or isolation tweaks. Re-enables the APIV1, APIV2, IRS 1099 and E-Document API tests that pass without area-specific fixes. Aligned with NAV master: only tests that NAV master also runs are re-enabled. - API test `Initialize` sets a license-safe work date. Adds minimal fixture and isolation adjustments. - Removes the APIV1/APIV2 exclusions, keeping only the entries fixed in the following area PRs and the two `139806` methods NAV master disables. Removes E-Doc `135575`. Related: [AB#646383](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/646383), [AB#653393](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/653393) ### Stack (each PR shows only its own layer; merge bottom first) 1. microsoft#12674 A: web service key auth (base: `main`) 2. microsoft#12675 B: Expense API tests + clean test codeunit execution 3. microsoft#12676 C: re-enable APIV1/APIV2 API tests 4. microsoft#12890 Review fixes for A: simplify web service key handling 5. Area fixes: microsoft#12677 D1 VAT fixtures → microsoft#12678 D2 cancellation reasons → microsoft#12679 D3 localized discounts → microsoft#12680 D4 quote dates → microsoft#12681 D5 journal numbering → microsoft#12682 D6 field comparison → microsoft#12683 D7 PDF documents → microsoft#12684 D8 APIV1 reliability Experiments (on D8, do not merge): microsoft#12685 E single tenant, microsoft#12686 F no clean execution. Combined validation (do not merge): microsoft#12687, same tree as D8. Platform: `main` already pins BCPlatform 30.0.56112.0, which includes the SQL connection-pool fix ([AB#653457](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/653457)). Note for CI: AL-Go's new-warning check needs a CI/CD baseline from the base branch, and `features/*` bases have none, so a PR build there fails with a 404. The current CI runs were triggered while these PRs targeted `main`, so their results are valid for the cumulative change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review focus: the number series in
LibraryGraphJournalLines.Area fix (previously #11228): ensures a vendor-payment journal number series, then re-enables the affected tests.
Related: AB#646383, AB#653393
Stack (each PR shows only its own layer; merge bottom first)
main)Experiments (on D8, do not merge): #12685 E single tenant, #12686 F no clean execution. Combined validation (do not merge): #12687, same tree as D8.
Platform:
mainalready pins BCPlatform 30.0.56112.0, which includes the SQL connection-pool fix (AB#653457).Note for CI: AL-Go's new-warning check needs a CI/CD baseline from the base branch, and
features/*bases have none, so a PR build there fails with a 404. The current CI runs were triggered while these PRs targetedmain, so their results are valid for the cumulative change.