[Shopify] Automatic Transaction Posting

[Onat Buyukakkus (onbuyuka)]

Summary

Introduces automatic posting of Shopify order/refund payment transactions as general journal lines when the related invoice or credit memo is posted in Business Central. This is a reworked, hardened version of the feature originally proposed in #6515.

Changes

New functionality

• Automatic-posting setup on Shpfy Payment Method Mapping (table + page): Post Automatically, Auto-Post Jnl. Template, Auto-Post Jnl. Batch (validated to require a balancing account on the batch).
• Visibility: Auto-Post Enabled flow field on Shpfy Order Transaction (table + page).
• Automatic posting: when a sales invoice/credit memo linked to a Shopify order/refund is posted, the related successful, not-yet-posted transactions are posted as customer payments if their payment method mapping is set up for automatic posting.
• Filtering: Filter Postable Transactions / Clear Filter actions on the Shopify Transactions list.

Prerequisites to post automatically

• On the Shpfy Payment Method Mapping: Post Automatically = true and Auto-Post Jnl. Template / Auto-Post Jnl. Batch filled in (the batch must have a balancing account).
• The transaction relates to a mapping that is set up for automatic posting.

Design notes (safety around the posting routine)

• Posting runs from Sales-Post.OnAfterPostSalesDoc, which fires after the document has been committed. No broad COMMIT is added to the posting routine.
• PreviewMode and CommitIsSuppressed postings are respected — automatic posting is skipped in those cases so previews and caller-controlled transactions are never disturbed.
• Payment posting is synchronous (via Gen. Jnl.-Post Batch), independent of the Post with Job Queue setting.
• Posting is best-effort: a failure is logged as a Shopify skipped record and never blocks, reverses, or errors the document posting, and never leaves an orphaned journal line behind.

Tests

Added ShpfyAutoPostTransTest.Codeunit.al (ID 139627) covering:

• Auto-Post Jnl. Batch validation with / without a balancing account and with an empty value.
• Auto-posting an order transaction when Post Automatically is true, and not posting when it is false.
• Auto-posting multiple transactions of an order.
• Posting only transactions linked to an auto-post mapping (and skipping non-linked ones).
• Auto-posting a refund transaction on credit memo posting.
• Synchronous posting even when Post with Job Queue is enabled.
• Best-effort behavior: on a payment-posting failure the document still posts, no ledger entry is created, no journal line is left behind, and a skipped record is logged.
• Preview posting does not auto-post and does not break the preview.

Fixes AB#620951

[github-actions]

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

The new ShowPostableTransactions and ClearFilter actions are promoted into the Related group, but Related is reserved for record-linked navigation (e.g., Customer Ledger Entries) while view-filter actions like these fit the standard Process group instead.

Knowledge:

• microsoft/knowledge/ui/promoted-action-groups.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}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

The best-effort auto-post path only traps AutoGenJnlPost.Run(...) and GenJnlPostBatch.Run(...) via their boolean return values. The surrounding RemoveJournalLines(...), the post-build Commit(), and LogFailureAndCommit(...) still raise normally on failure, so an exception there would escape OnAfterPostSalesDoc even though the whole feature is designed to never interrupt document posting. Additionally, if an exception occurs after BindSubscription(AutoGenJnlPost) but before the corresponding UnbindSubscription call (e.g. inside RemoveJournalLines before Run, or inside the post-build Commit before GenJnlPostBatch.Run), the manual event subscriber instance is left bound for later, unrelated journal postings in the same session. Wrap the whole attempt so cleanup/commit/logging cannot itself abort the caller, and guarantee UnbindSubscription runs on every exit path (including exceptional ones).

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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}

[github-actions]

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

PostTransactions iterates Shopify order/refund transactions with FindSet/repeat and, for each row whose payment method mapping enables auto-posting, calls PostTransaction which itself issues Commit() (once to establish a rollback boundary before the first payment, again after building each journal line before batch posting, and again in LogFailureAndCommit on failure). When an invoice or credit memo carries multiple transactions, this produces one journal batch posting (and one or more commits) per transaction instead of one combined operation, which is the per-row commit anti-pattern this article documents. The design intentionally isolates a failed payment posting from already-succeeded ones and from the underlying document post, which is a legitimate trade-off, but it is worth the author confirming the extra commit/posting-batch overhead per transaction is acceptable for orders with many line-item transactions.

Knowledge:

• microsoft/knowledge/performance/avoid-commit-inside-loops.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}

[github-actions]

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

MarkPostableTransactions filters Shpfy Order Transaction by Shop, Gateway, and Credit Card Company, but the table's only keys are Shopify Transaction Id (clustered), Gift Card Id, Created At, and Type — none start with Shop/Gateway/Credit Card Company. FilterPostableTransactions calls this once per auto-post-enabled payment mapping (in a repeat/until loop), so each call performs a filtered scan with no supporting key, and the cost multiplies by the number of configured mappings.

Knowledge:

• microsoft/knowledge/performance/setcurrentkey-aligns-key-with-filters.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}

[github-actions]

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

UnitTestAutoPostJnlBatchValidateWithoutBalAccountNo uses a bare asserterror ShpfyPaymentMethodMapping.Validate("Auto-Post Jnl. Batch", GenJournalBatch.Name); without following it with Assert.ExpectedError/ExpectedErrorCode. The test only proves some error occurred, not that it was the missing-balancing-account TestField failure; a typo or unrelated setup error would also make it pass.

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.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}

[github-actions]

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

The PR adds the "Auto-Post Enabled" transactions-page field plus the new Filter Postable Transactions/Clear Filter action flow, but there is no page test that opens Shpfy Transactions, runs the filter dialog, and asserts which records remain marked. Add a UI test covering the gateway/date filters and the Clear Filter action so regressions in this new filtering surface are caught.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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}

[Predrag Maricic (PredragMaricic)]

Request changes:

S1 - Automatic posting can post unrelated journal lines

Shpfy Auto Post Transactions filters the journal line record to the configured template and batch, then explicitly clears the Shpfy Transaction Id filter before calling Gen. Jnl.-Post Batch. This posts the entire configured batch, including unrelated pre-existing manual journal lines. The setup does not require or enforce a dedicated empty batch, so posting a sales invoice can unexpectedly post entries the user did not intend to post.

Please isolate automatic lines in a dedicated batch or use a posting path that is scoped to only the generated transaction lines. Add a regression test that places an unrelated line in the configured batch and verifies it remains unposted.

S2 - Partial invoicing can consume the full Shopify transaction too early

Automatic posting runs after each invoice is posted, while Shpfy Suggest Payments distributes the full order transaction over invoices that exist at that moment and creates a G/L residual for any remaining amount. For a split or partially invoiced Shopify order, the first invoice can therefore consume and mark the whole transaction as used before later invoices are posted, leaving later invoices unpaid or misallocating the remainder.

Please add split/partial-invoice coverage and ensure the first invoice does not consume the portion belonging to invoices that have not yet been posted.

S3 - The “postable transactions” filter does not match posting eligibility

The filter checks only Used = false, a posted invoice number, and a mapping with Post Automatically = true. It does not enforce the automatic-posting routine's Status = Success, supported transaction type, or non-empty journal template/batch requirements, so pending, failed, authorization, or incompletely configured transactions can be shown as postable. The end-date range also ends at 00:00, excluding nearly the entire selected end date.

Please align the UI filter with the actual posting predicates and make the selected end date inclusive.

[Onat Buyukakkus (onbuyuka)]

Round 2 — review feedback addressed (commit 34eab3e)

Thanks for the detailed review. Summary of the changes.

Predrag Maricic (@PredragMaricic)

S1 — automatic posting could post unrelated journal lines. Each transaction is now posted through a dedicated, single-use batch (SHPFY#####) cloned from the configured template/batch, then deleted. Gen. Jnl.-Post Batch still posts the whole batch, but that batch only ever contains this transaction's generated lines, so pre-existing lines in the configured batch are never touched. Regression test UnitTestAutoPostDoesNotPostUnrelatedBatchLines parks an unrelated line in the configured batch and asserts it stays unposted.

S2 — partial invoicing consuming the full transaction too early. Auto-posting now defers while any not-yet-posted sales document exists for the same Shopify order/refund (OpenSalesDocumentExistsForOrder/ForRefund), so the transaction is applied only once the order is fully invoiced. UnitTestAutoPostDefersWhilePartialInvoiceOpen covers the split scenario: no posting while a second invoice is still open; posting happens once both are posted.

S3 — filter vs. posting eligibility + end date. The "Filter Postable Transactions" list now enforces the same predicates as the posting routine (Status = Success, supported Type, mapping configured with a non-empty template/batch, and a posted invoice or credit memo). The selected end date is now inclusive (end-of-day).

AL review agent — inline threads (resolved)

• Data Modeling — the journal template's OnValidate now clears the batch on any template change, so a stale template+batch combination can't persist.
• Testing (parameter propagation) — added UnitTestSetJournalParametersPropagatesToGeneratedLine, asserting the generated line uses the mapped template, batch, posting date and applies-to document.
• Upgrade (event signature) — false positive: AL binds event-subscriber parameters by name, not by position; an invalid binding would be a compile error, the app builds clean, and the 15 auto-post tests only pass because this OnAfterPostSalesDoc subscriber fires.

AL review agent — general comments

• Accessibility — ShowPostableTransactions/ClearFilter moved from Related to Process.
• Testing (bare asserterror) — now asserts ExpectedError('Bal. Account No.') + ExpectedErrorCode('TestField').
• Error handling — the posting attempt is trapped (Codeunit.Run for line building + GenJnlPostBatch.Run for posting); any failure is logged to a Skipped Record, and cleanup/logging run after Sales-Post has already committed the document, so they can never reverse the posted invoice/credit memo.
• Performance (commit per transaction) — an intentional consequence of isolating each transaction in its own batch (S1); the commit count is bounded by the small number of payment transactions per document.
• Performance (MarkPostableTransactions key) — marking is now a single pass over the pre-filtered set instead of a per-mapping re-filter.
• Page test — the filter eligibility is exercised through the posting tests; happy to add a dedicated TestPage test for the filter dialog if preferred.

All tests green: 15/15 auto-post + 9/9 Suggest Payment regression. App builds clean (0 errors / 0 warnings).

[github-actions]

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

CreateJournalBatch (and the near-identical CreateFailingPaymentMethodMapping, plus CreateNoSeries) hand-roll Gen. Journal Template, Gen. Journal Batch, and No. Series/No. Series Line fixtures with Init()/direct field assignment/Insert() instead of using the existing test library helpers Library - ERM.CreateGenJournalTemplate, Library - ERM.CreateGenJournalBatch, and Library - ERM.CreateNoSeriesCode (or LibraryUtility.CreateNoSeries/CreateNoSeriesLine). None of the Validate calls the library performs (e.g. Description, Force Doc. Balance) run, so the fixture bypasses the setup and field-validation logic production code assumes and can silently break as the schema evolves.

Knowledge:

• microsoft/knowledge/testing/use-library-codeunits-for-test-fixtures.md

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

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

[github-actions]

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

MarkPostableTransactions only reads Shop, Gateway, Credit Card Company, Type, Refund Id, and the Posted Invoice No. FlowField, but FindSet() still loads the full (wide) Shpfy Order Transaction row for every candidate scanned. Add SetLoadFields before the scan so the filter/marking pass does not transfer unused columns.

Suggested change

	// Mark rows whose mapping is auto-post enabled and whose invoice/credit memo is posted.
	Rec.SetAutoCalcFields("Posted Invoice No.");
	if not Rec.FindSet() then
	// Mark rows whose mapping is auto-post enabled and whose invoice/credit memo is posted.
	Rec.SetLoadFields(Shop, Gateway, "Credit Card Company", Type, "Refund Id");
	Rec.SetAutoCalcFields("Posted Invoice No.");
	if not Rec.FindSet() then

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md

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

[github-actions]

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

The new setup fields on Shpfy Payment Method Mapping (Post Automatically, Auto-Post Jnl. Template, Auto-Post Jnl. Batch) only hold internal posting configuration, not customer/business content, but they inherit a table-level classification that overstates their sensitivity. These fields should be classified SystemMetadata rather than CustomerContent.

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

        field(7; "Post Automatically"; Boolean)
        {
            Caption = 'Post Automatically';
            DataClassification = SystemMetadata;
        }
        field(8; "Auto-Post Jnl. Template"; Code[10])
        {
            Caption = 'Auto-Post Journal Template';
            DataClassification = SystemMetadata;
            TableRelation = "Gen. Journal Template" where(Type = const("Cash Receipts"));

            trigger OnValidate()
            begin
                // Clear the batch on any template change so a stale template+batch can't persist.
                if "Auto-Post Jnl. Template" <> xRec."Auto-Post Jnl. Template" then
                    Clear("Auto-Post Jnl. Batch");
            end;
        }
        field(9; "Auto-Post Jnl. Batch"; Code[10])
        {
            Caption = 'Auto-Post Journal Batch';
            DataClassification = SystemMetadata;
            TableRelation = "Gen. Journal Batch".Name where("Journal Template Name" = field("Auto-Post Jnl. Template"));

            trigger OnValidate()
            var
                GenJournalBatch: Record "Gen. Journal Batch";
            begin
                if "Auto-Post Jnl. Batch" = '' then
                    exit;
                TestField("Auto-Post Jnl. Template");
                GenJournalBatch.Get("Auto-Post Jnl. Template", "Auto-Post Jnl. Batch");
                GenJournalBatch.TestField("Bal. Account No.");
            end;
        }

Knowledge:

• microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md
• microsoft/knowledge/privacy/table-level-data-classification-cascades.md

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

[github-actions]

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

The new auto-post test scenarios exercise Shpfy Auto Post Transactions, which now calls Commit() internally (via PostTransaction/RemoveIsolatedBatch/LogFailure), but the test methods in ShpfyAutoPostTransTest.Codeunit.al do not declare [TransactionModel(TransactionModel::AutoCommit)]. Under the platform's default test transaction model, a Commit() inside the code under test can fail the test run with an infrastructure error before the test's own assertions are reached.

Knowledge:

• microsoft/knowledge/testing/transactionmodel-attribute-governs-test-transactions.md

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

[github-actions]

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

The test suite adds coverage for SetJournalParameters but always passes WorkDate(), so it never exercises the new PreconfirmWorkingDateOnBeforeCheckLine subscriber branch that suppresses the posting-after-working-date confirmation dialog. Add a test that posts with a future working date to prove the auto-post flow stays headless (no dialog) when that prompt would normally fire.

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

[github-actions]

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

This PR adds more transaction-specific data flow inside case OrderTransaction.Type of: the Capture/Sale and Refund arms now both need coordinated edits to thread OrderTransaction through helper calls. Move the transaction-type-specific posting behavior behind an interface-backed implementation on Shpfy Transaction Type so adding or changing a transaction kind does not require editing this dispatch block and its helper pipeline together.

Knowledge:

• microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md

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

[github-actions]

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

UnitTestAutoPostJnlBatchValidateWithoutBalAccountNo hardcodes the expected error text fragment and error code after asserterror instead of using Library Assert's dedicated TestField helper. Replace the literal checks with ExpectedTestFieldError so the negative test stays pinned to the intended field failure without duplicating caption and code knowledge.

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

asserterror ShpfyPaymentMethodMapping.Validate("Auto-Post Jnl. Batch", GenJournalBatch.Name);
LibraryAssert.ExpectedTestFieldError(GenJournalBatch.FieldCaption("Bal. Account No."), '');

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md

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

[github-actions]

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

UnitTestPostSalesOrderWithAutoPostTransaction starts a group of new auto-post-enabled posting tests that drive LibrarySales.PostSalesDocument into Shpfy Auto Post Transactions, where the new implementation executes Commit(), but those tests do not opt into [TransactionModel(TransactionModel::AutoCommit)]. Add TransactionModel::AutoCommit to the auto-post commit-path tests so the transaction model matches the behavior they verify.

Knowledge:

• microsoft/knowledge/testing/transactionmodel-attribute-governs-test-transactions.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

The best-effort auto-post failure path is not itself protected. After AutoGenJnlPost.Run(...) or GenJnlPostBatch.Run(...) fails, RemoveIsolatedBatch(...) and LogFailure(...) still perform ordinary deletes/inserts and Commit() calls; if one of those secondary operations errors, that exception propagates out of OnAfterPostSalesDoc and can turn a recoverable payment-posting failure into a hard sales-posting failure. Isolate cleanup/logging behind protected error handling (for example a dedicated Codeunit.Run/TryFunction wrapper) so auto-post failures remain non-blocking even when logging or batch cleanup also goes wrong.

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

[github-actions]

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

OnAfterSalesPosting first calls CreateDocLinksToBCDocs, which Inserts "Shpfy Doc. Link To Doc." rows whenever a Shopify document link already exists for the posted document (the normal case for orders/refunds that originated in Shopify) — this opens a new write transaction. Immediately afterward, when CommitIsSuppressed is false, it calls ShpfyAutoPostTransactions.AutoPostTransactions, which reaches PostTransaction and invokes AutoGenJnlPost.Run(OrderTransaction) (a Codeunit.Run call) with no intervening Commit(). Per platform rules, Codeunit.Run cannot nest inside an already-open write transaction; the runtime raises an error the first time this path executes with a pending Doc. Link insert. Because if not AutoGenJnlPost.Run(...) then traps the error and only calls LogFailure, this failure is swallowed silently — auto-posting will never actually succeed for any transaction whose invoice/credit memo has a pre-existing Shopify document link, which is the common real-world case. The new test codeunit does not create any "Shpfy Doc. Link To Doc." rows in its fixtures (see CreateShopifyOrder/CreateOrderTransaction), so this path is not exercised by the added tests, which is why it was not caught before merge. Add an explicit Commit() before AutoGenJnlPost.Run(OrderTransaction) (or restructure CreateDocLinksToBCDocs's writes to happen after the auto-post attempt) so the sub-operation is not silently rejected by the runtime.

Knowledge:

• microsoft/knowledge/performance/codeunit-run-requires-prior-commit-inside-transaction.md

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

[github-actions]

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

The new OnAfterPostSalesDoc subscriber calls codeunit "Shpfy Auto Post Transactions" for every posted Shopify invoice or credit memo, and that internal codeunit carries direct RIMD rights on "Gen. Journal Batch" and "Gen. Journal Line". This lets any user who can post the sales document transitively create and post journal data without a separate authorization check; Access = Internal only hides the API at compile time and does not enforce runtime authorization. Add an explicit authorization boundary for auto-posting that is independent of sales-document posting.

Knowledge:

• microsoft/knowledge/security/internal-access-is-not-a-security-boundary.md

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

[github-actions]

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

The fixture helpers hand-roll standard Gen. Journal Template, Gen. Journal Batch, and No. Series records with Init/Insert instead of using the existing test libraries (Library - ERM / Library - Utility). That bypasses the maintained fixture setup and makes these tests brittle when required fields or validation logic change.

Knowledge:

• microsoft/knowledge/testing/use-library-codeunits-for-test-fixtures.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

The new page "Shpfy Filter Transactions" is a helper dialog used only from "Shpfy Transactions", but it omits Access = Internal; and therefore ships as a public object that dependent extensions can bind to. Mark it internal unless you intend to support it as a stable external contract.

Suggested change

	page 30176 "Shpfy Filter Transactions"
	{
	Caption = 'Filter Postable Transactions';
	PageType = StandardDialog;
	ApplicationArea = All;
	page 30176 "Shpfy Filter Transactions"
	{
	Access = Internal;
	Caption = 'Filter Postable Transactions';
	PageType = StandardDialog;
	ApplicationArea = All;

Knowledge:

• microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md

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

[github-actions]

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

The new auto-post flow performs privileged general-journal posting inside an internal codeunit without an independent authorization check. "Shpfy Auto Post Transactions" creates and posts cash-receipt/refund journal lines with direct "Gen. Journal Batch"/"Gen. Journal Line" rights, and the changed sales-post subscriber invokes it automatically for posted Shopify invoices and credit memos. Combined with the new payment-method-mapping fields that let Shopify configuration choose the journal template and batch, this lets a user who can manage Shopify mappings and post sales documents indirectly execute finance posting they may not be allowed to run directly. Guard this path with an explicit journal-posting authorization boundary instead of relying on Access = Internal and object permissions.

Knowledge:

• microsoft/knowledge/security/internal-access-is-not-a-security-boundary.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

The new "Auto-Post Enabled" FlowField looks up only "Post Automatically", but the posting code treats a mapping as enabled only when both "Auto-Post Jnl. Template" and "Auto-Post Jnl. Batch" are also populated. That lets a transaction show auto-post enabled even when it can never be auto-posted. Make the FlowField reflect the full posting prerequisite, or rename it to the narrower meaning.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

The mapping table stores "Credit Card Company" as Text[30], but both "Shpfy Order Transaction" and "Shpfy Credit Card Company" use 50 characters. The new auto-post lookups use this field as part of an exact key, so any value longer than 30 characters is truncated when the mapping row is created and later no longer matches the transaction. Widen the mapping field to Text[50] so the key can round-trip the full external value.

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

        field(3; "Credit Card Company"; Text[50])
        {
            Caption = 'Credit Card Company';
            DataClassification = CustomerContent;
            TableRelation = "Shpfy Credit Card Company";
        }

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

When a payment mapping is marked for automatic posting but either journal field is blank, GetAutoPostMapping just returns false and the transaction is skipped with no error or failure log. Validate the required journal setup when "Post Automatically" is enabled, or emit a concrete failure record when auto-posting is skipped because the setup is incomplete.

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

[github-actions]

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

The new automatic payment-posting flow in codeunit "Shpfy Auto Post Transactions" is a core posting routine, but it ships with no surrounding OnBefore/OnAfter integration events. That leaves extensions no supported seam to adjust eligibility or replace the default auto-post behavior without copying this subscriber-driven flow. Add thin publishers around the posting pipeline, using an overridable OnBefore event where custom posting must be able to skip the default path.

Knowledge:

• microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md

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

[github-actions]

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

The new automatic payment-posting flow catches posting failures at both Codeunit.Run and Gen. Jnl.-Post Batch.Run and only persists them through Shpfy Skipped Record. Add explicit telemetry for these failure branches with stable event IDs and correlation dimensions (for example transaction/shop/batch identifiers, plus error text/call stack) so best-effort auto-post failures can be monitored outside the UI.

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

[github-actions]

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

The new auto-posting logic explicitly handles Type::Capture alongside Type::Sale, but the added payment tests exercise only Sale and Refund. Add at least one capture-based posting scenario (and ideally a suggest-payments scenario) so regressions in the new Capture branch are caught.

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

[alexei-dobriansky]

AI PR Review — Round 1

Recommendation: Accept with Suggestions

Risk assessment: The automatic posting path is isolated and best-effort, but the new postable-transaction filter does not mirror the refund flow.

Findings / suggestions

🟠 S1 — Align the postable filter with refund posting readiness

FilterPostableTransactions() filters on Used = false and Posted Invoice No. <> ''. Refund auto-posting is driven from a posted sales credit memo (PostRefundTransactions(SalesCrMemoHeaderNo)), while the transaction table only exposes a posted-invoice lookup by Shopify order id. That means refund transactions can be shown as postable merely because the order has an invoice, and refund transactions whose readiness is only the posted credit memo are not represented accurately. Consider adding/using a posted credit memo relation for refund transactions, or limit this filter to Capture/Sale transactions so the list matches the actual auto-post trigger.

---

Review mode: autonomous conversation comment (interactive:false). A human should make the final approve/request-changes decision.

[AI-PR-REVIEW] by=alexei-dobriansky round=1 sha=7d9d4bbdbc3fc11a7e87485c934328974c6f51fc promptVersion=good-sense-reviewer-2026-08 reviewKey=d41fc9f2c5a9525 suggestions=[S1@d755f4d7]