Skip to content

[29.0] bug 643971 - Enhance vendor and customer checks in advance letters and update blocked status validation - #10950

Merged
David Holuša (DavidHolusa) merged 2 commits into
releases/29.0from
bugs/643971-290-BlockedCustAndVendInAdvances2
Sep 4, 2026
Merged

[29.0] bug 643971 - Enhance vendor and customer checks in advance letters and update blocked status validation#10950
David Holuša (DavidHolusa) merged 2 commits into
releases/29.0from
bugs/643971-290-BlockedCustAndVendInAdvances2

Conversation

@DavidHolusa

@DavidHolusa David Holuša (DavidHolusa) commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What & why

When creating advance letters from sales or purchase orders, the system did not validate whether the customer/vendor was blocked before proceeding. Additionally, the customer blocked-status check in CheckBlockedCustOnAdvanceLettersCZZ was too broad — it rejected customers with Blocked = Ship, which is irrelevant for advance letters (invoicing). This change:

  1. Adds upfront blocked-customer/vendor validation in the Create Sales Adv. Letter and Create Purch. Adv. Letter reports (SetSalesHeader / SetPurchHeader) so blocked entities are caught early.
  2. Narrows the customer blocked check from Blocked <> Blocked::" " to Blocked in [Blocked::All, Blocked::Invoice], allowing customers blocked only for shipping to still have advance letters created.

Linked work

Fixes AB#643971

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)

  • Created a sales advance letter from a sales order for a customer with Blocked = Ship — advance letter created successfully (previously errored).
  • Created a sales advance letter from a sales order for a customer with Blocked = All — correctly blocked with an error.
  • Created a sales advance letter from a sales order for a customer with Blocked = Invoice — correctly blocked with an error.
  • Created a purchase advance letter from a purchase order for a blocked vendor (Blocked = All) — correctly blocked with an error.
  • Created advance letters for non-blocked customer/vendor — both succeeded as expected.

Risk & compatibility

  • Behavior change: Customers with Blocked = Ship are now allowed to have advance letters created, whereas before they were blocked. This is intentional — the Ship block is not relevant to advance payment invoicing.
  • The new upfront validation in SetPurchHeader/SetSalesHeader may surface errors earlier in flows that previously failed later (or silently succeeded). No data migration or schema changes involved.

@DavidHolusa
David Holuša (DavidHolusa) requested a review from a team September 2, 2026 11:46
@DavidHolusa
David Holuša (DavidHolusa) requested a review from a team as a code owner September 2, 2026 11:46
@github-actions github-actions Bot added the Team: Finance GitHub request for Finance area label Sep 2, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Sep 2, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This PR adds blocked customer and vendor checks before creating advance letters from sales and purchase orders. It also changes the customer advance-letter check so Blocked = Ship is allowed, while Invoice, All, and privacy-blocked customers still fail.

The new checks run before the reports copy fields into the advance-letter headers, so they target the right entry point. The narrowed customer rule also matches the intended invoicing behavior. The main gap is test coverage: the new report path is not covered, and existing sales blocked-status tests still describe the old Ship behavior.

Problem-solution fit

Fit: Strong

The bug says blocked customer and vendor values can be used when creating advance letters from orders. The diff targets that path and also aligns the Ship customer status with advance-letter invoicing behavior, so the code change fits the problem. It is not complete until the regression tests cover the same paths and rules.

Suggestions

S1 (🔴 High): Add order-based blocked-party tests
Add tests that create sales and purchase advance letters from orders with blocked customer and vendor values. The changed checks run in SetSalesHeader and SetPurchHeader, while the existing tests cover direct advance-letter header entry. This is a financial document path, so the exact report path should be covered.

S2 (🟠 Moderate): Update stale Blocked Ship expectations
The existing sales advance tests still expect Blocked = Ship to fail for direct creation and payment posting. Update those tests so Ship is allowed, and keep coverage for Invoice, All, and privacy blocked. Otherwise the test app keeps the old behavior as the expected result.

Risk assessment and necessity

Risk: This is in CZ advance payment creation and payment posting. An incorrect blocked-status rule can either create financial advance letters for blocked parties or block valid Ship customers. There is no schema change or public event change, and the performance impact is negligible.

Necessity: The change is needed because report-based creation copies customer and vendor fields without relying on the advance-letter header field validation. The scope is small and focused, but the tests must be aligned before merge.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10950 round=1 by=alexei-dobriansky at=2026-09-02T13:16:55Z lastSha=4423e7330bf83e27dec14d7350faaeffe15286e7 reviewKey=7a82db23056e1268dae0acdb99220b65c6c67edd19161f5ffd6412f1ba469cbb suggestions=S1@b34a69f3,S2@c4c86d21

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

The PR adds blocked customer and vendor checks before creating advance letters from sales and purchase orders. It also narrows the customer advance-letter check so Blocked = Ship is allowed, while Invoice, All, and privacy-blocked customers still fail.

The new commit only changes tests. It updates the existing sales direct-create and payment-posting tests so the Ship case now runs without asserterror. That addresses the stale expected-error checks, but the changed report entry points still have no blocked-party regression tests.

Status of previous suggestions
ID Title Status Author response
S1 Add order-based blocked-party tests Not addressed No reply. The current diff still has no sales or purchase order-based blocked-party tests for SetSalesHeader or SetPurchHeader.
S2 Update stale Blocked Ship expectations Addressed Fixed in 0f356e631d; the Ship sales create and payment-posting cases now run the operation and skip expected-error checks.
New observations (commits since round 1)

None - the new commit only updates the stale sales test expectations.

Risk assessment and necessity

Risk: The affected path creates financial advance letters from sales and purchase orders. A wrong blocked-party rule can either create documents for blocked parties or block valid Ship customers. The round-2 code does not add new functional risk, but the report path that was changed remains uncovered.

Necessity: The blocked checks are needed because order-based creation calls the report setup procedures before it creates the advance letter. The test update is also needed because Ship is now valid for these advance-letter flows, but the missing order-based tests still leave the main bug path unverified.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10950 round=2 by=alexei-dobriansky at=2026-09-03T07:14:31Z lastSha=0f356e631df2003851603b9fc021fd6ea86a97a3 reviewKey=f39dd1c86b2b6e930c88092da461b3101d50746b74d2b3f475c4a3ddc0ffddae suggestions=S1@b34a69f3:notaddressed,S2@c4c86d21:addressed parentRound=1

@DavidHolusa
David Holuša (DavidHolusa) merged commit 52c4967 into releases/29.0 Sep 4, 2026
168 checks passed
@DavidHolusa
David Holuša (DavidHolusa) deleted the bugs/643971-290-BlockedCustAndVendInAdvances2 branch September 4, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants