[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

Automatic-posting setup

• Adds Post Automatically, Auto-Post Jnl. Template, and Auto-Post Jnl. Batch to payment-method mappings.
• Requires the journal template and batch before automatic posting can be enabled.
• Requires a balancing account on the configured batch.
• Keeps the Auto-Post Enabled transaction FlowField aligned with the complete setup requirements.

Posting and filtering

• Automatically posts successful, unused Capture, Sale, and Refund transactions after the related sales invoice or credit memo is posted.
• Uses one shared eligibility implementation for automatic posting and the postable-transactions filter, including partial-invoice/refund deferral.
• Makes the selected filter end date inclusive.

Posting safety

• Posts each transaction through a dedicated, single-use journal batch, so unrelated lines in the configured batch are never posted.
• Commits document-link updates before starting isolated Boolean Codeunit.Run operations.
• Defers inventory-pick/put-away automatic posting until the warehouse posting transaction is committed.
• Checks the invoking user's journal permissions before creating or posting journal data.
• Runs cleanup and skipped-record persistence in isolated, trappable operations so secondary failures do not escape document posting.
• Emits telemetry for posting, cleanup, and failure-logging errors.
• Skips previews and caller-owned suppressed-commit transactions.

Tests

The automatic-posting test suite covers setup validation, Sale/Capture/Refund posting, multiple and mixed transactions, unrelated journal lines, partial invoices and credit memos, document-link transaction boundaries, future posting dates, suppressed commits, preview, job-queue setup, failure handling, parameter propagation, and shared postable eligibility.

The Shopify app builds successfully with the AL MCP server. The full local test-project build is currently blocked by the existing MockAzureKeyVaultSecretProvider environment dependency; CI provides the full test matrix.

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).

[alexei-dobriansky]

Good Sense Reviewer - Round 3

Recommendation: Request Changes

What this PR does

This change adds automatic posting for Shopify sale, capture, and refund transactions when the related sales invoice or credit memo is posted. It also adds setup fields, a postable-transaction filter, isolated journal batches, failure logging, and regression tests.

The approach is mostly aligned with the requested feature: it uses a shared eligibility codeunit, avoids posting unrelated journal lines, skips previews and suppressed-commit posting, and defers partial documents. Two issues still block merge: the current build fails on new analyzer warnings, and refund posting can still apply one refund transaction across other refunds on the same Shopify order.

Problem-solution fit

Fit: Partial

The feature goal is clear and the implementation covers the main sale, capture, and refund flows. The fit is not strong yet because the current head does not pass the build and one refund edge case can post the wrong application.

Since last review

ID	Title	Status	Author response
S1	Align the postable filter with refund posting readiness	Addressed	The shared eligibility code now checks refund readiness by refund id.
S2	Remove unused using directives	Addressed	The unused directives called out earlier were removed.

New observations

S3 (🔴 High): Filter refund posting to the current refund
When posting one refund transaction, GetOrderTransactions still loops through every refund for the same Shopify order. Filter RefundHeader by OrderTransaction."Refund Id" before applying credit memo entries, so one refund cannot pay a different refund's credit memo.

S4 (🔴 High): Rename the shadowing test variables
The current build does not pass because the new test code declares local PaymentMethodMapping variables with the same name as the global variable. Rename those locals or use the global record consistently; otherwise the PR stays blocked by AA0198 warnings.

Risk assessment and necessity

Risk: This is a financial posting path. An incorrect refund match can apply customer ledger entries to the wrong credit memo, and a build failure blocks all validation of the Shopify app changes.

Necessity: The feature is useful and the scope is appropriate: merchants need Shopify payments and refunds to follow posted documents without manual journal work. The blockers should be fixed before merge because they affect build health and refund posting correctness.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9525 round=3 by=alexei-dobriansky at=2026-08-28T22:20:42Z lastSha=7e2fbbc34ea308cbf09b620eab3a9b61dbcdf1b5 reviewKey=e490261ce4ba67d3ef753472e095300af4df224ceb6fe5303351ab10dc0e6c44 suggestions=S1@d755f4d7:addressed,S2@9c615153:addressed,S3@54639714:new,S4@8b7a5440:new parentRound=2

[Onat Buyukakkus (onbuyuka)]

Addressed the two actionable Round 3 findings in ecef587:

• S3: refund application now keeps RefundHeader scoped to OrderTransaction."Refund Id", with regression coverage for two refunds on the same order.
• S4: renamed the four local PaymentMethodMapping variables that introduced AA0198 warnings and blocked the build matrix.

The remaining automated inline recommendations were assessed, replied to, and resolved individually.

[github-actions]

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

The new refund path reads OrderTransaction."Refund Id" in GetOrderTransactions(), but OnPreDataItem still partial-loads "Shpfy Order Transaction" without that field. Each refund row will therefore fall back to a just-in-time field load, which undermines the report's partial-record optimization. Add "Refund Id" to the preloaded field set before iteration starts.

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

                SetLoadFields(Amount, "Rounding Amount", Type, "Shopify Order Id", "Shopify Transaction Id", Gateway, "Gift Card Id", "Payment Method", "Refund Id");

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md
• microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.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}

[alexei-dobriansky]

Good Sense Reviewer - Round 4

Recommendation: Accept with Suggestions

What this PR does

This change adds automatic posting for Shopify sale, capture, and refund transactions when the related sales invoice or credit memo is posted. It also adds setup fields, a shared eligibility check, isolated journal batches, failure logging, and regression tests.

The latest commit addresses the open refund correctness issue by limiting refund application to the current refund id and adds a regression test for two refunds on the same order. It also fixes the analyzer warning caused by shadowing test variables. The remaining concern is non-blocking: the new refund-id read should be included in the report's partial-record field list.

Status of previous suggestions

ID	Title	Status	Author response
S1	Align the postable filter with refund posting readiness	Addressed	Already addressed before this round.
S2	Remove unused using directives	Addressed	Already addressed before this round.
S3	Filter refund posting to the current refund	Addressed	The refund path now filters by the current refund id and has same-order regression coverage.
S4	Rename the shadowing test variables	Addressed	The local test variables were renamed so they no longer shadow the global record.

New observations (commits since round 3)

S5 (🟠 Moderate): Preload Refund Id for refund report filtering
GetOrderTransactions now reads OrderTransaction."Refund Id", but the report dataitem still does not include that field in SetLoadFields. Add "Refund Id" there so refund rows do not fall back to extra just-in-time reads.

Risk assessment and necessity

Risk: This remains a financial posting path. The prior wrong-refund risk is addressed, and the remaining issue is limited to report performance on refund transaction iteration.

Necessity: The feature is justified because it removes manual journal work for Shopify payments and refunds while preserving setup and posting safeguards. The latest round is small and directly targets the previous blockers.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9525 round=4 by=alexei-dobriansky at=2026-08-29T22:12:56Z lastSha=ecef5878e34a03c916efb31d7c567eb4a5a09121 reviewKey=d67cbe02268972ead8a4d302062c32b6044d3c2de30ba05ff74c2893033f5934 suggestions=S1@d755f4d7:addressed,S2@9c615153:addressed,S3@54639714:addressed,S4@8b7a5440:addressed,S5@dd247f25:new parentRound=3

[github-actions]

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

IgnorePostedTransactionsLbl is used as the prompt for Confirm() on line 199, but the label-suffix guidance reserves Qst for confirmation questions. Keeping the Lbl suffix hides that this text is a question prompt and makes the call-site contract harder to scan. Rename it to IgnorePostedTransactionsQst and update the Confirm(...) call so the suffix matches the actual usage.

Knowledge:

• microsoft/knowledge/style/label-suffix-approved-list.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}

[alexei-dobriansky]

Good Sense Reviewer - Round 5

Recommendation: Accept with Suggestions

What this PR does

This change adds automatic posting for Shopify sale, capture, and refund transactions when the related sales invoice or credit memo is posted. It also adds setup fields, a shared eligibility check, isolated journal batches, failure logging, and regression tests.

The latest commit preserves the manual refund suggestion fallback when a refund transaction has no refund id, while keeping exact refund-id filtering for transactions that do have one. It also adjusts the refund regression test to use the posted credit memo total and fixes the future-posting-date test setup. The remaining concern is still non-blocking: the report reads the refund id but does not include it in the dataitem load field list.

Status of previous suggestions

ID	Title	Status	Author response
S1	Align the postable filter with refund posting readiness	Addressed	Fix still present in the current diff.
S2	Remove unused using directives	Addressed	Fix still present in the current diff.
S3	Filter refund posting to the current refund	Addressed	The nonzero refund-id path still filters to the matching refund and has same-order regression coverage.
S4	Rename the shadowing test variables	Addressed	Fix still present in the current diff.
S5	Preload Refund Id for refund report filtering	Not addressed	The latest commit did not add "Refund Id" to the report dataitem SetLoadFields call.

New observations (commits since round 4)

None - the latest commit only adjusts the refund fallback and tests. The remaining open item is tracked above.

Risk assessment and necessity

Risk: This remains a sensitive financial posting path because it creates and posts journal lines for Shopify payments and refunds. The latest refund-id change keeps exact matching when the transaction has a refund id, and only uses the broader order-level fallback for legacy rows without one. The remaining issue is limited to extra database reads during report iteration.

Necessity: The feature is justified because it removes manual payment and refund journal work while keeping setup, permission, posting, and failure-handling safeguards. The latest commit is narrow and supports existing legacy refund data behavior without changing the automatic posting filter for exact refund ids.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9525 round=5 by=alexei-dobriansky at=2026-08-30T22:11:17Z lastSha=18a55b34860e354a74d4cdfd782ee2d34aace637 reviewKey=75ef290aadb36a4e6642644451d12680eb67c821ce701f117cb1ef4f4f3c4c35 suggestions=S1@d755f4d7:addressed,S2@9c615153:addressed,S3@54639714:addressed,S4@8b7a5440:addressed,S5@dd247f25:notaddressed parentRound=4

[github-actions]

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

The Suggest Shopify Payments action on the Shpfy Transactions page still uses CurrPage.SetSelectionFilter(OrderTransaction) without falling back to the full page filter when nothing is explicitly multi-selected. If a user applies a filter (e.g. Postable Transactions) and invokes the action from the current row without multi-selecting, SetSelectionFilter can collapse the scope to that single record instead of the visible filtered set, silently posting/suggesting only part of the intended workload.

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

                    CurrPage.SetSelectionFilter(OrderTransaction);
                    if not OrderTransaction.MarkedOnly then
                        OrderTransaction.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

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

[alexei-dobriansky]

Good Sense Reviewer - Round 6

Recommendation: Accept with Suggestions

What this PR does

This change adds automatic posting for Shopify sale, capture, and refund transactions when the related sales invoice or credit memo is posted. It also adds setup fields, a shared eligibility check, isolated journal batches, failure logging, and regression tests.

The new commit only makes tests safer across localizations: it resets posted shipment and invoice number-series date usage before scenarios affected by a future posting date, and it unlinks an open sales document instead of deleting it. These changes do not alter the runtime posting flow. The remaining non-blocking concern is still that the payment suggestion report reads the refund id but does not include it in the dataitem load field list.

Status of previous suggestions

ID	Title	Status	Author response
S1	Align the postable filter with refund posting readiness	Addressed	The shared eligibility code still checks refund readiness through posted credit memos by refund id.
S2	Remove unused using directives	Addressed	The current diff keeps the needed namespace imports for the referenced records.
S3	Filter refund posting to the current refund	Addressed	The automatic refund path still filters refund transactions by the posted credit memo refund id.
S4	Rename the shadowing test variables	Addressed	The local eligibility test record keeps a distinct name.
S5	Preload Refund Id for refund report filtering	Not addressed	The report dataitem SetLoadFields call still omits "Refund Id" while later code reads OrderTransaction."Refund Id".

New observations (commits since round 5)

None - the latest commit only adjusts tests for localization and number-series behavior. The remaining open item is tracked above.

Risk assessment and necessity

Risk: This remains a sensitive financial posting path because it creates and posts journal lines for Shopify payments and refunds. The latest commit is test-only, so it does not add posting risk. The remaining issue is limited to extra database reads during refund transaction iteration in the suggestion report.

Necessity: The feature is justified because it removes manual payment and refund journal work while keeping setup, permission, posting, and failure-handling safeguards. The latest commit is narrow and helps the automatic-posting test suite run consistently across localizations.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9525 round=6 by=alexei-dobriansky at=2026-09-02T22:17:18Z lastSha=31e5d042adaba02df05915692c079d12bc42b1f5 reviewKey=b64c7f2e549376528b653f0298d733b35080e750d129423f2db4e4092a197411 suggestions=S1@d755f4d7:addressed,S2@9c615153:addressed,S3@54639714:addressed,S4@8b7a5440:addressed,S5@dd247f25:notaddressed parentRound=5

[github-actions]

⚠️ 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.