ARCH-001 Phase 7: PaymentTransaction controller consolidation (Admin/Store) by KrzysztofPajak · Pull Request #797 · grandnode/grandnode2

KrzysztofPajak · 2026-08-26T13:42:55Z

Type: refactor

Issue

Grand.Web.Admin (471 lines) and Grand.Web.Store (524 lines) each shipped their own
PaymentTransactionController — the same class of duplication ARCH-001 already fixed for
Product (PR #790), Category (PR #792), Collection (PR #794), and Order (PR #795). No
Grand.Web.Vendor controller exists for this entity.

Full design: docs/superpowers/specs/2026-08-26-arch001-paymenttransaction-consolidation-design.md
Full plan: docs/superpowers/plans/2026-08-26-arch001-paymenttransaction-consolidation.md
(Both gitignored under docs/superpowers/, same as every prior phase — on disk in this branch's
history but not tracked by git.)

Different shape from every prior entity: only Admin and Store have a controller for
PaymentTransaction — no Vendor screen, and no separate combined-host controller either. That
ruled out the Vendor branch entirely, but did not rule out the routing-collision problem: the
combined Grand.Web host ProjectReferences both Grand.Web.Admin and Grand.Web.Store
directly, so both hosts' controllers and StartupApplication registrations load into the same
process. The first draft of the spec missed this and proposed per-host DI registration; caught
before implementation and corrected to a 2-branch RoutedPaymentTransactionDataScope
(fail-closed), same shape as RoutedCategoryDataScope/RoutedCollectionDataScope — Category and
Collection hit this identical shape despite also having no Vendor screen.

Solution

StorePaymentTransactionDataScope (bespoke — PaymentTransaction is a plain BaseEntity with a
scalar StoreId, not IStoreLinkEntity) + reuse of the existing generic
GlobalAdminDataScope<PaymentTransaction> for Admin (no restriction found in Admin's original
controller) + RoutedPaymentTransactionDataScope, all registered centrally in
Grand.Web.AdminShared/Startup/StartupApplication.cs.
Single-level BasePaymentTransactionController in Grand.Web.AdminShared — both hosts' action
surfaces were already an exact 1:1 match (List, GoToOrderNumber, Edit, Capture, MarkAsPaid,
Refund/RefundOffline, Void/VoidOffline, PartiallyRefundPopup ×2, PartiallyPaidPopup ×2, Delete —
15 actions total). A LoadAuthorizedPaymentTransaction helper replaces the repeated
"load, check StoreId != StaffStoreId, redirect if denied" boilerplate that Store's original
controller inlined at every single action site; Admin's original had no such check at all
(GlobalAdminDataScope.HasAccess is always true, so behavior is unchanged for Admin).
No view-model service existed on either host (both controllers built models inline) — nothing to
reconcile at that layer, unlike every prior phase.
Admin/Store PaymentTransactionControllers reduced to thin subclasses (each restating its own
host's [Area]/[Authorize*]/[AuthorizeMenu] attributes).
Views migrated to Grand.Web.AdminShared/Views/AdminShared/PaymentTransaction/...; hardcoded
Constants.AreaAdmin/Constants.AreaStore replaced with the existing
ViewContext.RouteData.Values["area"] idiom (Category/Edit.cshtml precedent). Widget zones
extracted into per-host WidgetZone.*.cshtml satellites.
7-task plan executed via superpowers:subagent-driven-development, task-by-task review ledger
available on request. Final whole-branch review (opus) found 0 Critical, 2 Important (see
Breaking changes below), several Minor — all addressed in one fix round, independently
re-verified clean.
Live smoke test run against a real Kestrel-hosted instance + this developer's own local
MongoDB (real order/payment-transaction data, no synthetic seeding needed): Admin
List/Edit render correctly with real data; the partial-paid popup's full write path exercised
end-to-end (AmountToPaid=50 submitted → PartiallyPaidOfflineCommand executed →
Status/Paid amount correctly recomputed on reload, confirming the RefreshPage fix below fires
a real page reload, not just a 200 response); Store's List correctly scoped to only its own
store's transactions; a direct cross-store access attempt via a known other-store transaction id
redirected to List rather than showing it (the direct regression check for
LoadAuthorizedPaymentTransaction's scope.HasAccess gate replacing the per-action inline
checks); Store's own transaction opened correctly under the /Store/ area.

Breaking changes

None to any public method signature. Three disclosed, deliberate behavior changes, all judged
safe and kept after final review:

RefreshPage bug fix. Admin's original PartiallyRefundPopup/PartiallyPaidPopup POST
actions set ViewBag.RefreshPage = true on success, but the view (identical on both hosts)
reads Model.RefreshPage — a dead write that meant Admin's popups never actually refreshed the
parent page after a successful partial refund/paid. Store's original already set
model.RefreshPage = true correctly. Unified onto Store's already-correct behavior; verified
live (see Testing).
Store's <vc:admin-widget> → <vc:store-widget> fix. Store's original 5 PaymentTransaction
views all called <vc:admin-widget>, a tag helper never registered in the Store app
(Grand.Web.Store.csproj has no ProjectReference to Grand.Web.Admin) — inert markup that
never rendered anything. Fixed to <vc:store-widget> to match every other consolidated entity's
convention; documented in-tree with a comment on each of the 5 new Store WidgetZone satellite
files. Can only add previously-absent widget-zone hooks on Store's PaymentTransaction pages,
never remove working behavior.
Store's Delete denial redirect target. Store's original Delete action redirected to
RedirectToAction("List", "MerchandiseReturn") on a store-mismatch denial — an evident
copy-paste bug from another controller. The consolidated version redirects to the
PaymentTransaction list, an unavoidable consequence of centralizing the denial path through
LoadAuthorizedPaymentTransaction. Both are list pages; no security or data-exposure delta.

Testing

dotnet build GrandNode.sln — 0 errors (4 pre-existing warnings, unrelated to this diff,
already present on develop).
dotnet test src/Tests/Grand.Web.Admin.Tests (unfiltered) — 565 passed, 0 failed.
dotnet test src/Tests/Grand.Web.Store.Tests (unfiltered) — 42 passed, 0 failed.
Live smoke test against a real running instance (see Solution above) — List/Edit render
correctly on both hosts with real data, full partial-paid write path verified end-to-end
including the RefreshPage fix's actual observable effect, cross-store denial confirmed via a
direct URL access attempt with a known other-store transaction id.
Executed via superpowers:subagent-driven-development: 7 plan tasks + one final-review fix
round (2 Important + 1 cheap Minor addressed: undocumented widget-tag fix now has an in-tree
comment on all 5 affected files; Admin's routing test strengthened to match Store's
[Area]/[AuthorizeAdmin] assertions per the plan's own spec; denied-branch tests added for
the two popup POST actions) — task-by-task review ledger available on request if useful for
review context.

🤖 Generated with Claude Code

…s (Store/Routed) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

…t region Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

…tTransactionController Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

…RefreshPage bug Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

…hin subclasses Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

…nShared, extract per-host WidgetZone satellites Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

…, strengthen Admin routing test, add popup denial tests - Add in-tree Razor comment to the 5 Store PaymentTransaction WidgetZone partials explaining the vc:store-widget vs vc:admin-widget fix (Finding 1) - Strengthen Admin's PaymentTransactionControllerRoutingTests to assert [Area(Constants.AreaAdmin)] and AuthorizeAdminAttribute, mirroring Store's equivalent test (Finding 2) - Add denied-branch tests for PartiallyRefundPopup and PartiallyPaidPopup POST overloads in BasePaymentTransactionControllerTests (Finding 3) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

+        var result = _controller.List() as ViewResult;
+
+        Assert.IsNotNull(result);
+        Assert.IsInstanceOfType(result.Model, typeof(PaymentTransactionListModel));


+        var result = await _controller.GoToOrderNumber(new PaymentTransactionListModel { OrderNumber = "123" }) as RedirectToActionResult;
+
+        Assert.IsNotNull(result);
+        Assert.AreEqual("List", result.ActionName);


+        var result = await _controller.GoToOrderNumber(new PaymentTransactionListModel { OrderNumber = "123" }) as RedirectToActionResult;
+
+        Assert.IsNotNull(result);
+        Assert.AreEqual("Edit", result.ActionName);


+        var result = await _controller.Edit("pt-1") as RedirectToActionResult;
+
+        Assert.IsNotNull(result);
+        Assert.AreEqual("List", result.ActionName);


+        Assert.IsNotNull(result);
+        var model = result.Model as PaymentTransactionModel;
+        Assert.IsNotNull(model);
+        Assert.AreEqual("pt-1", model.Id);


+        var result = await _controller.CapturePaymentTransaction("pt-1") as RedirectToActionResult;
+
+        Assert.IsNotNull(result);
+        Assert.AreEqual("List", result.ActionName);


+        var result = await _controller.CapturePaymentTransaction("pt-1") as RedirectToActionResult;
+
+        Assert.IsNotNull(result);
+        Assert.AreEqual("Edit", result.ActionName);


+
+        Assert.IsNotNull(result);
+        var resultModel = result.Model as PaymentTransactionModel;
+        Assert.IsTrue(resultModel.RefreshPage);


+
+        Assert.IsNotNull(result);
+        var resultModel = result.Model as PaymentTransactionModel;
+        Assert.IsFalse(resultModel.RefreshPage);


+        var result = await _controller.PartiallyRefundPopup("pt-1", false, model) as RedirectToActionResult;
+
+        Assert.IsNotNull(result);
+        Assert.AreEqual("List", result.ActionName);


…dation Resolves the expected additive conflict in Grand.Web.AdminShared/Startup/StartupApplication.cs between this phase's IAdminDataScope<PaymentTransaction> registration and Shipment's (PR #796, merged to develop) IAdminDataScope<Shipment> registration — both blocks kept, no logic changed on either side. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

- Add [AutoValidateAntiforgeryToken] to BasePaymentTransactionController itself, matching every other Base*Controller in this codebase (BaseCollectionController, BaseOrderController, BaseShipmentController). Runtime CSRF protection was already in place via the attribute restated on both concrete Admin/Store subclasses, so this is not a functional fix — but CodeQL's cross-project analysis doesn't follow the attribute across the base/derived project boundary, which is exactly the false-positive class BaseCollectionController's own doc comment already calls out and works around the same way. Resolves CodeQL alerts #1503-1513 (List, GoToOrderNumber, Capture/MarkAsPaid/Refund/RefundOffline/Void/VoidOffline, PartiallyRefundPopup, PartiallyPaidPopup, Delete). - Add null-forgiving (!) operators after Assert.IsNotNull(...) guards throughout BasePaymentTransactionControllerTests.cs. MSTest's Assert.IsNotNull narrows at runtime but CodeQL's C# nullable-flow analysis doesn't recognize it as a null-check, flagging the subsequent property access as a possible null dereference. Test-code-only, no behavior change; resolves 10 github-code-quality alerts plus 2 unreported instances of the identical pattern found by inspection. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

KrzysztofPajak and others added 7 commits August 26, 2026 08:34

feat(arch001): add IAdminDataScope<PaymentTransaction> implementation…

f7b0e3c

…s (Store/Routed) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

feat(arch001): add BasePaymentTransactionController skeleton with Lis…

8f549be

…t region Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

feat(arch001): add Edit + payment action buttons region to BasePaymen…

512620b

…tTransactionController Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

feat(arch001): add partial refund/paid popups and Delete region, fix …

81e79ce

…RefreshPage bug Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

feat(arch001): cut Admin/Store PaymentTransactionController over to t…

7f2c7a3

…hin subclasses Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

refactor(arch001): migrate PaymentTransaction views to Grand.Web.Admi…

0fa7b29

…nShared, extract per-host WidgetZone satellites Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013XGyrBT97Wr9zjxgDmG2KR

Copilot AI lite review requested due to automatic review settings August 26, 2026 13:42

Copilot AI reviewed Aug 26, 2026

github-code-quality Bot found potential problems Aug 26, 2026

View reviewed changes

github-advanced-security Bot found potential problems Aug 26, 2026

View reviewed changes

KrzysztofPajak and others added 2 commits August 26, 2026 17:16

KrzysztofPajak requested a lite review from Copilot August 26, 2026 15:49

Copilot AI reviewed Aug 26, 2026

KrzysztofPajak merged commit bfc80ff into develop Aug 26, 2026
1 check passed

KrzysztofPajak deleted the arch001/phase7-paymenttransaction-consolidation branch August 26, 2026 15:59

KrzysztofPajak mentioned this pull request Aug 27, 2026

ARCH-001 Phase 8: MerchandiseReturn controller/service consolidation (Admin/Store/Vendor) #799

Open

Navigation Menu

Uh oh!

ARCH-001 Phase 7: PaymentTransaction controller consolidation (Admin/Store) - #797

ARCH-001 Phase 7: PaymentTransaction controller consolidation (Admin/Store)#797
KrzysztofPajak merged 9 commits into
developfrom
arch001/phase7-paymenttransaction-consolidation

KrzysztofPajak commented Aug 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

KrzysztofPajak commented Aug 26, 2026

Issue

Solution

Breaking changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants