ARCH-001 Phase 5: Order controller/service consolidation (Admin/Store/Vendor)

[KrzysztofPajak]

Type: refactor

Issue

Grand.Web.Admin (875 lines), Grand.Web.Store (858 lines), and Grand.Web.Vendor (220 lines)
each shipped their own OrderController — the same class of duplication ARCH-001 already fixed
for Product (PR #790), Category (PR #792), and Collection (PR #794). Vendor also carried its own
drifted duplicate IOrderViewModelService/OrderViewModelService (613 lines vs AdminShared's
1297), the same problem Product had before Phase 1.

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

Order does not mirror Category/Collection's shape — it is closer to, and in places more
complex than, Product:

• Admin has its own access restriction orthogonal to store/vendor scope: a Sales Manager may only
  act on orders whose SeId matches their own (CheckSalesManager in the original code). Neither
  Store nor Vendor has this. Reusing the generic always-true GlobalAdminDataScope<Order> for
  Admin would have silently dropped this restriction — a bespoke AdminOrderDataScope was
  required instead, the first entity in ARCH-001 where Admin itself needs non-trivial scope logic.
• Order is a plain BaseEntity with a single StoreId field, not IStoreLinkEntity — the
  generic StoreAdminDataScope<TEntity> doesn't apply; a bespoke StoreOrderDataScope was needed.
• Vendor's ownership check is over a child collection (OrderItems.Any(i => i.VendorId == ...)),
  not a flat field, and Vendor also filters line items inside a shared model-preparation method
  (a vendor viewing a mixed-vendor order sees only its own lines) — a new interface member,
  IAdminDataScope<TEntity>.FilterOrderItems, was added for this.
• Vendor's action surface is a genuine subset (list/view/PDF only, no mutation at all), not just a
  differently-scoped copy of the same surface — the first entity needing a two-level base
  controller (BaseOrderController for all three hosts, BaseOrderManagementController : BaseOrderController for Admin+Store only).
• Excel export / bulk delete deliberately stay Admin-exclusive, not shared — Store already holds
  the underlying Export/Delete permission grants (used today by its PDF actions), so sharing
  those three actions would have silently handed Store new working Excel-export/bulk-delete
  routes as a side effect of consolidation.

Solution

• Three bespoke IAdminDataScope<Order> implementations (none reusing the generic Global/Store
  scopes — a first for ARCH-001): AdminOrderDataScope (Sales-Manager-aware), StoreOrderDataScope,
  VendorOrderDataScope (ownership over OrderItems, plus FilterOrderItems). Routed by a new
  3-branch RoutedOrderDataScope (fails closed) — the first 3-branch routed scope in this effort;
  prior entities were 2-branch.
• BaseOrderController (List/read/PDF, all three hosts) + BaseOrderManagementController : BaseOrderController (every mutating action, Admin+Store only) in Grand.Web.AdminShared. A
  LoadAuthorizedOrder helper replaces ~20x-duplicated load-and-check logic per host. Every
  denial redirects to "List" (Order's own original controllers never redirected to "Edit" on
  denial, unlike Category/Collection).
• Admin/Store/Vendor OrderControllers reduced to thin subclasses (each restating its own host's
  [Area]/[Authorize*]/[AuthorizeMenu] attributes, since the shared base classes are
  host-agnostic by design). ExportExcelAll/ExportExcelSelected/DeleteSelected stay declared
  only on Admin's concrete controller.
• Order notes' denial response is a genuine, preserved host divergence, not unified: Admin's
  original throws ArgumentException for both not-found and Sales-Manager denial; Store's
  original throws only for not-found and soft-denies (Content("")/Json(false)) for
  store-mismatch. Both preserved exactly.
• Vendor's own drifted IOrderViewModelService/OrderViewModelService deleted; AdminShared's
  service gained scope-based FilterOrderItems wiring plus vendor field-gating (Vendor never
  saw CustomerIp, AffiliateId, SalesEmployeeId, OrderTags, OrderStatuses, or the entire
  order-totals block in its original pre-consolidation service — all now gated behind
  scope.DefaultVendorId is null, closing a latent exposure risk of showing merchant Profit to
  a vendor via any future shared view).
• Order views migrated to Grand.Web.AdminShared/Views/AdminShared/Order/...; fixed the same
  dead-<vc:admin-widget>-in-Store tag-helper bug already found and fixed in the Product and
  Category phases (Store's _ViewImports.cshtml never registers Admin's tag helper).
• 19-task plan executed via superpowers:subagent-driven-development, task-by-task review ledger
  available on request. Final whole-branch review (opus) found 2 Critical + 3 Important issues
  that no per-task review caught — all fixed in one round, independently re-verified:
  • Vendor's Areas/Vendor/Views/_ViewImports.cshtml still imported the now-dead
    Grand.Web.Vendor.Models.Orders instead of Grand.Web.AdminShared.Models.Orders — every
    Vendor Order screen would have thrown a model-type InvalidOperationException at runtime
    despite a clean build and 813 passing unit tests. Fixed with a scoped type-alias import
    (a blanket import would have made ShipmentModel/MerchandiseReturnModel/*ReportModel
    ambiguous across Vendor's other, not-yet-consolidated views).
  • PdfInvoiceSelected dropped the vendor-id argument its two sibling PDF actions correctly
    pass, leaking other vendors' line items into a vendor's exported PDF for mixed-vendor orders.
  • RoutedOrderDataScope didn't forward CanView (unlike the sibling routed scopes).
  • Zero test coverage existed on the three PDF actions and GoToOrderId — exactly the class of
    action where a scope value threads into a third-party service argument, and exactly why the
    PDF leak slipped through five rounds of task review. Added.
• Live cross-tenant smoke test run against a real Kestrel-hosted instance + this developer's
  existing local MongoDB (built from this branch, three synthetic test orders inserted directly
  and removed afterward, no git-tracked files touched): a store-exclusive order denied to the
  other store's manager (direct URL and PDF) and permitted to its own store's manager; a
  mixed-vendor order permitted to a vendor whose Products tab correctly showed only that vendor's
  own line item; a zero-overlap order denied entirely, including from the vendor's own list; and
  — the direct regression check for the PDF-leak fix above — the vendor's downloaded PDF invoice
  for the mixed order, extracted with pdftotext, contained only that vendor's own product line,
  confirmed byte-for-byte.

Breaking changes

None to IOrderViewModelService's method signatures beyond internal scope wiring. Order notes'
per-host denial-response shapes, the Sales-Manager restriction, and every other original behavior
are preserved exactly, with three disclosed, deliberate exceptions:

1. PdfInvoiceSelected now applies scope.HasAccess uniformly, which for Admin is a
   security-positive behavior change (closes a pre-existing gap where a Sales Manager could
   previously export any order id via crafted selectedIds, bypassing the Sales-Manager scoping
   every other Admin action already enforced).
2. Vendor's PrepareOrderDetailsModel now (correctly) never populates order-totals/financial
   fields, matching what Vendor's original pre-consolidation service already did — this closes a
   gap this branch itself introduced mid-flight (final-review finding), not a change from
   pre-branch develop behavior.
3. AddressEdit's dead if (address == null) throw (never fires; address is initialized via
   new Address(), never null) and a PascalCase-vs-camelCase route-value casing quirk are
   preserved byte-for-byte from both original Admin and Store controllers — verified against
   develop's pre-branch source, not introduced by this branch, and deliberately left as-is
   rather than silently "fixed" mid-consolidation.

Testing

1. dotnet build GrandNode.sln — 0 errors, 0 warnings.
2. dotnet test src/Tests/Grand.Web.Admin.Tests (unfiltered) — 538 passed, 0 failed.
3. dotnet test src/Tests/Grand.Web.Store.Tests (unfiltered) — 38 passed, 0 failed.
4. dotnet test src/Tests/Grand.Web.Vendor.Tests (unfiltered) — 12 passed, 0 failed.
5. dotnet test src/Tests/Grand.Mapping.Tests (unfiltered) — 234 passed, 0 failed.
6. Live cross-tenant smoke test against a real running instance (see Solution above) — all
   applicable denials correct, positive controls passed, PDF-export fix confirmed against the
   actual generated PDF's extracted text, no unauthorized data exposure found.
7. Executed via superpowers:subagent-driven-development: 19 plan tasks + one final-review fix
   round, task-by-task review ledger available on request if useful for review context.

🤖 Generated with Claude Code

[Copilot]

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