Skip to content

[Purchases] Use posted return-shipment report selection for PDF - #11230

Open
Prangshuman Das (t-prda) wants to merge 39 commits into
prdas/646383-split-field-comparisonfrom
prdas/646383-split-return-shipment-pdf
Open

Prangshuman Das (t-prda) wants to merge 39 commits into
prdas/646383-split-field-comparisonfrom
prdas/646383-split-return-shipment-pdf

Conversation

@t-prda

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

Copy link
Copy Markdown
Contributor

Scope

Change Return Shpt. PDF Doc.Handler.GeneratePdfBlobWithDocumentType from P.Return to P.Ret.Shpt. This is the only production-code change in the seven-way split and affects report selection/attachment naming.

AB#646383 — follow-up split from PR #10085; link only, not an instruction to resolve the umbrella work item.

Evidence and limits

No corresponding PDF failure appears in saved result sets 2 or 4. The existing APIV2 purchase-return-shipment suite exclusion is unchanged. A targeted posted-return-shipment PDF regression run is still required.

Native GitHub stack #11893

This draft is stacked above #11229. #10085 is the bottom API-authentication fix. The review diff contains only this layer's fix plus removal of its 0 temporary method exclusions. Earlier layers are inherited, not repeated in this diff.

Existing PR/branch history is preserved; no force-push or merge into main was performed. NAV selection/company/native exclusions and existing out-of-scope Expense exclusions remain untouched.

Methods re-enabled by this layer

None added. The existing purchase-return-shipment suite exclusion remains unchanged.

Validation

Exact parent ancestry, file scope, whitespace and exclusion ownership checks pass. This layer's added/removed source lines are unchanged relative to its refreshed parent. Upstream changes are inherited through the main merge, not reverted. Earlier exact-head cumulative artifact evidence is historical after this parent reconciliation; fresh new-head runtime validation is pending. Existing excluded PDF, IN/RU gaps and tolerated-native distinctions remain; no absent/excluded case is claimed passing.

Validation limits

The previous UserPassword HTTP 401/200/401 result is historical after this reconciliation. CU139496 MicrosoftAuthenticationRespectsServerAuthMode still executes all three requests; Windows200/200/200 runtime remains unverified. The workflow clean-codeunit gate stays default-off and uptake stays explicitly enabled. No provider, authentication contract, new public event, NAV selector or foreign NST change was introduced. Excluded PDF cases and absent/excluded country/native cases remain unverified; prior tolerated-native results are not universal passes. The 59 owned re-enabled methods cover the reviewed fixes, not59 distinct product defects. Validation drafts remain Do Not Merge, outside native stack #11893.

Current checkpoint

Head 004bf02871e12fec89243265b7bb7aaa34823828, tree 9f20c013cde4b7b549d6425fb43af472b29123c4; parent 196d33070b9aa62583e6fcefa69e149020503543.

Forward-integrated captured main bb7111877ff786951b86a1a0f80d8b39b8f5dacd, including upstream CLEAN27 removal 82b11d26c073de93df3aab17434069f72feab640 (PR12066), to align the direct stacked-PR warning gate. The prior direct uptake checkout retained obsolete source while warning-reference run37020063048 used main73d5794e; RU and CH therefore each reported51 additional warnings (36 AA0244,15 AA0218). Main-targeted RU validations already passed with cleaned source. No warning suppression, parameter rename, partial cherry-pick, or comparator change was made.

Local Pester:117 passed, zero failed/skipped at exact workflow, uptake and full heads. Every layer retains its exact owned patch; all changed baseline blobs equal captured main, and all remaining blobs—including exclusions—are unchanged. The23 committed wrappers/shared success-only finalizer, generator removal, current-process credential identity, ACLs, buffer clearing and narrowed platform classifier remain intact; ordinary configured reruns are unchanged.

Accepted cleanup limitation: failed/cancelled runs rely on normal container teardown; no hard-runner-loss guarantee. Normal CI uses per-run disposable credentials, but supplied credentials may differ. Hook logs prove invocation, not necessarily explicit deletion if teardown already removed the file.

Uptake retains the five query-safe URL compositions in CU148343 StandardSubmissionExposesPolicySnapshot, all assertions,8 methods, setup restoration and the single unlimited-approval fixture. Workflow remains default-off; uptake enables clean execution. Auth visibility/provider contracts and HTTP scenario are preserved. Upstream shared Spend Request zero-amount UnitTest semantics and permission cleanup from PR11561 remain unchanged. CU139806 TestGetCompanyAndEnvironmentDescriptions stays excluded pending its original rationale/current NAV verification (PR11741).

Fresh exact-head GitHub CI is pending, not passed. Previous runs are historical for these new commits. Verify all8 activity methods across22 countries if present; Windows runtime and excluded PDF/native coverage remain unverified. Only this captured main was integrated—no repeated baseline chasing. Merged prefix/native stack #11893 and draft/ready states remain unchanged; validation drafts remain Do Not Merge.

Split from the preserved PR10085 snapshot. AB#646383

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve published branch history and re-enable only the 0 methods covered by this layer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
@t-prda
Prangshuman Das (t-prda) changed the base branch from main to prdas/646383-split-field-comparison September 8, 2026 17:06
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
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
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
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

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.

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

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
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept with Suggestions

What this PR does

This changes the posted purchase return-shipment PDF handler to use the report-selection usage for posted return shipments instead of purchase return orders. The new value matches the record type passed to the report and the usage already used when printing posted return shipments, so it selects the correct report and keeps attachment naming aligned.

Problem-solution fit

Fit: Strong

The changed value directly corrects the mismatch between a posted return shipment and the return-order report selection. The scope is narrow and addresses the reported PDF behavior without changing posting or stored document data.

Suggestions

S1 (🟠 Moderate): Add targeted posted return shipment PDF coverage
Add a regression test for GeneratePdfBlobWithDocumentType on a posted return shipment. Verify that it uses the P.Ret.Shpt. report selection and produces the expected attachment name.

Risk assessment and necessity

Risk: Low. The one-line change affects PDF report selection and attachment naming for posted purchase return shipments. It does not change posting, ledger entries, amounts, persisted records, or public APIs.

Necessity: The change is required because P.Return selects purchase return-order configuration, while this handler processes a posted return shipment. Without it, the wrong configured report can be used for the generated PDF.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11230 round=1 by=alexei-dobriansky at=2026-09-28T12:22:20.2138883Z lastSha=e494103c9652cbd3260756d7091a9689fc31170c reviewKey=40f8e15effe2f91c283f6631e7988a5e92864789ff0f3d816117256f7f56e29b suggestions=S1@f5a9305b

pull Bot pushed a commit to CarstenMertes/BCApps that referenced this pull request Sep 29, 2026
## Purpose


[AB#646383](https://dynamicssmb2.visualstudio.com/Dynamics%20SMB/_workitems/edit/646383)

Add opt-in authentication to `Library - Graph Mgt.` without API
re-enablement or pipeline changes.

## Design

- Default `None` preserves existing behavior. The public enum/interface
and public `API Test Auth Context` retain external provider
extensibility.
- The Microsoft provider stays `Internal`; context `Apply` stays
internal. Internal visibility is API encapsulation, not authorization.
- Credential precedence remains container file, then existing Key Vault
only if the file is absent. Invalid configured credentials fail;
successful passwords remain instance-scoped `SecretText`.
- File credentials now use a private file-mapping provider directly,
without replacing or clearing the session-wide Azure Key Vault
provider/cache. No extra AL plaintext copy is introduced.
- `OnAfterInitializeWebRequestWithURL` remains last. The pre-existing
empty Graph `OnRun` is restored.

The trust boundary is admitted OnPrem test code and environment
credential access, not `Internal` visibility or a destination-URL
restriction.

## Tests and scope

All six AL contracts remain: default None, event ordering, provider
reuse, instance scoping, same-provider reselection and deselection.
The dedicated recorder is removed. The non-SingleInstance internal mock
publishes the **test-only** `OnAfterConfigureAuthentication` event; the
same manually bound test-codeunit instance owns `Library - Variable
Storage` for both recording and verification. Initialization clears that
instance's queue after prior failures; each contract drains it and calls
`AssertEmpty`.

The six source-pattern PowerShell checks are removed, not replaced by
other AL-text assertions. The real 401/200/401 HTTP scenario is owned by
uptake microsoft#11860 after credential provisioning; core has no dependency on
that future workflow. This is not a claim of a complete provider-branch
matrix.

No caller migration, URL repair, credential provisioning, scheduling,
exclusion or work-date rollout belongs to this core.

Native stack #11893: **microsoft#10085 auth core -> microsoft#11862 URL/fixture
prerequisites -> microsoft#11891 workflow infrastructure -> microsoft#11860 AL uptake ->
microsoft#11224–microsoft#11230 -> microsoft#11322 -> microsoft#11451–microsoft#11454**.

## Current checkpoint

Head `4a76bb71851c158083dbe4d22e84bf40a0577842`, tree
`12739039e502a3feeea9e98694fb360ea959c606`.

Merged main baseline remains `0a602a2481c93ebfe7b1d27d6016fb9926c42111`
(permission cleanup from PR 11561). No additional main merge or code
changes were made during finalization. Old `b195c7a`/`4eef8ac`
tree-equality claims remain obsolete after the intentional baseline/auth
changes.

[Verified AL compile/publish/test run
36131334134](https://github.com/microsoft/BCApps/actions/runs/36131334134)
**succeeded, attempt 2**, at exact validation head
`4a76bb71851c158083dbe4d22e84bf40a0577842`. **113/113 test jobs and
113/113 cleanup steps succeeded.** Core has no credential-file
provisioning/removal step (expected).

W1 artifacts verify all **6 auth contracts**.

Local Pester at the applicable checkpoint: **28 passed, zero
failed/skipped**. Core local Pester: **28 passed**; no unsupported claim
about a separate root PowerShell workflow.

## Validation limits

For uptake/full, CU139496
`MicrosoftAuthenticationRespectsServerAuthMode` is verified in the
actual UserPassword fixture (**401/200/401**). The Windows
**200/200/200** expectations are implemented but **Windows runtime
remains unverified**; no foreign local NST was used. Successful GitHub
runs do not establish universal native NAV coverage. Excluded PDF cases,
country-specific absent/excluded cases and tolerated-native distinctions
are not claimed passing. No PR has been merged or auto-merged;
validation drafts remain Do Not Merge outside stack #11893.

---------

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
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Accept with Suggestions

What this PR does

The net PR change still updates posted purchase return-shipment PDF generation to use P.Ret.Shpt. instead of P.Return. The commits since round 1 only reconcile the stacked parent and do not alter this hunk; the change still matches the posted document type and the standard report-selection usage.

Status of previous suggestions
ID Title Status Author response
S1 Add targeted posted return shipment PDF coverage Not addressed —
New observations (commits since round 1)

None - commits since round 1 add no attributable net change to this PR.

Risk assessment and necessity

Risk: Low. The one-line change affects only API PDF report selection and attachment naming for posted purchase return shipments. No new net code change was added after round 1, but targeted runtime coverage is still missing.

Necessity: The change is required because P.Return uses purchase return-order configuration, while this handler processes a posted return shipment. Without the change, PDF generation can select the wrong configured report and attachment name.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11230 round=2 by=alexei-dobriansky at=2026-09-30T18:13:38.7734192Z lastSha=130808d8c99adb871201d0b36dda259bd2b13ef5 reviewKey=dbd19a31246a78791041846cc2696142f4fafbba7d9dbf13191eaf7b4e80a608 suggestions=S1@f5a9305b:notaddressed parentRound=1

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

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

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 3

Recommendation: Accept with Suggestions

What this PR does

The net PR change still updates posted purchase return-shipment PDF generation to use P.Ret.Shpt. instead of P.Return. The commits since round 2 only reconcile the stacked parent and refreshed base; they do not alter the reviewed hunk. The selected usage matches the posted record type, the purchase report-selection setup, and the equivalent posted-document handlers.

Status of previous suggestions
ID Title Status Author response
S1 Add targeted posted return shipment PDF coverage Not addressed —
New observations (commits since round 2)

None - commits since round 2 add no attributable net change to this PR.

Risk assessment and necessity

Risk: Low. The one-line change affects PDF report selection and attachment naming for posted purchase return shipments. The existing API test only checks that the expanded PDF value exists and remains excluded, so it does not protect the exact report-usage and filename behavior.

Necessity: The change is required because P.Return selects purchase return-order configuration, while this handler processes a posted return shipment. Without the change, PDF generation can select the wrong configured report and attachment name.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11230 round=3 by=alexei-dobriansky at=2026-10-02T07:19:28.9062628Z lastSha=24b0760ed61b737d387c798d71a82a6226208f6e reviewKey=d71291e67bee2d622db973b3cafbd02bc0beeda785c084b608155cb57f4be5cd suggestions=S1@f5a9305b:notaddressed parentRound=2

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
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 4

Recommendation: Accept with Suggestions

What this PR does

The net PR change still updates posted purchase return-shipment PDF generation to use P.Ret.Shpt. instead of P.Return. The commits since round 3 only reconcile stacked ancestry and do not alter this hunk; inherited workflow changes are already in the current base. The selected usage matches the posted record type, the standard return-shipment print path, and the equivalent posted-document handler pattern.

Status of previous suggestions
ID Title Status Author response
S1 Add targeted posted return shipment PDF coverage Not addressed —
New observations (commits since round 3)

None - commits since round 3 add no attributable net change to this PR.

Risk assessment and necessity

Risk: Low. The one-line change affects PDF report selection and attachment naming for posted purchase return shipments. The exact handler path still lacks targeted regression coverage, but the change does not affect posting, ledger entries, amounts, persisted document data, or public APIs.

Necessity: The change is required because P.Return selects purchase return-order configuration, while this handler processes a posted return shipment. Without the change, PDF generation can select the wrong configured report and attachment name.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11230 round=4 by=alexei-dobriansky at=2026-10-02T12:35:42.3658332Z lastSha=66e9530b09e90282c2f46d248b1a641fe6e9fd05 reviewKey=a1fdee69b8e116ca7e7e3022fdb63d3817046f6e4e5d1747f37a09500620f132 suggestions=S1@f5a9305b:notaddressed parentRound=3

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
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 5

Recommendation: Accept with Suggestions

What this PR does

The net PR change still updates posted purchase return-shipment PDF generation to use P.Ret.Shpt. instead of P.Return. The commits since round 4 only refresh stacked ancestry and the shared warning baseline; they do not alter the reviewed hunk. The selected usage matches the posted document type, the standard return-shipment print path, and the equivalent posted-document handler pattern.

Status of previous suggestions
ID Title Status Author response
S1 Add targeted posted return shipment PDF coverage Not addressed —
New observations (commits since round 4)

None - commits since round 4 add no attributable net change to this PR.

Risk assessment and necessity

Risk: Low. The one-line change affects PDF report selection and attachment naming for posted purchase return shipments. Fresh exact-head builds and targeted PDF runtime coverage are still pending, but the change does not affect posting, ledger entries, persisted data, public APIs, or events.

Necessity: The change is required because P.Return selects purchase return-order configuration, while this handler processes a posted return shipment. Without the change, PDF generation can select the wrong configured report and attachment name.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11230 round=5 by=alexei-dobriansky at=2026-10-02T18:41:32.9087791Z lastSha=004bf02871e12fec89243265b7bb7aaa34823828 reviewKey=551e55ba8a465eb6794c299c01ab0e9362c0906b0558d126aef85e4dfc7e553d suggestions=S1@f5a9305b:notaddressed parentRound=4

This branch was successfully deployed

1 active (outdated) deployment
triage — e494103c Deployed Sep 28, 2026 by t-prda via Classify team ownership #5949
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team: SCM GitHub request for SCM area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants