ARCH-001 Phase 20: Customer controller/service consolidation (Admin/Store) - #819
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…pe<Customer> Task 3 of ARCH-001 Phase 20 (Customer consolidation): routes to AdminCustomerDataScope or StoreCustomerDataScope based on the request's area route value, failing closed for Vendor/unrecognized/missing areas (Customer has no Vendor screen). Registers the two concrete scopes plus the routed IAdminDataScope<Customer> in StartupApplication.cs, inserted directly after the MessageTemplate scope block without touching any other entity's registration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…List/Create, fix Store List() CustomerSettings gap Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…llable per plan spec Enables #nullable on BaseCustomerController.cs (matching sibling BaseReportsController.cs/BaseFullReportsController.cs) and changes CheckTwoFactorEnabledWarning(Customer existingCustomer, ...) to Customer? existingCustomer, as mandated by the plan's Global Constraints (the brief's own code sample had this non-nullable, which was incorrect). Also annotates the CustomerList() local 'tagIds' as string[]? since it is genuinely assigned null under Store scope — the only other warning enabling #nullable surfaced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…ns with LoadAuthorizedCustomer helper Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…via flow analysis Replaces 'if (denied != null) return denied;' followed by up to three customer! suppressions per method with 'if (customer is null) return denied!;', which lets the compiler narrow customer to non-null for the rest of each method. Removes all 13 customer! suppressions in the file, down to one denied! suppression per call site (7 total), justified by LoadAuthorizedCustomer's own invariant: it returns either (null, denied) or (customer, null), never both non-null or both null. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…s regions Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
Ported missing Deleted check from original Admin CustomerController that prevented access to soft-deleted customers. Without this check, deleted customers are silently accessible/editable again via the Admin panel. - Add entity.Deleted check to HasAccess (deny if deleted) - Add test cases for deleted customer (both sales-manager and non-sales-manager) - Verify no regression: 7 AdminCustomerDataScopeTests pass, 20 BaseCustomerControllerTests pass Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…t personalize/price, out-of-stock), add missing [HttpPost] CSRF hardening
Adds the final block of BaseCustomerController's shared action surface:
ProductsPrice, PersonalizedProducts, ProductAddPopup (GET/POST),
ProductAddPopupList, UpdateProductPrice, DeleteProductPrice,
UpdatePersonalizedProduct, DeletePersonalizedProduct,
OutOfStockSubscriptionList, plus a new HasAccessToCustomer helper
(non-redirecting ownership check for actions that soft-deny with an
empty JsonResult instead of a redirect).
Security fix (disclosed): Admin's originals for UpdateProductPrice,
DeleteProductPrice, UpdatePersonalizedProduct, and
DeletePersonalizedProduct never carried [HttpPost] (only
PermissionAuthorizeAction), unlike Store's already-correct versions.
Since [AutoValidateAntiforgeryToken] only validates unsafe HTTP verbs,
these were reachable via GET, bypassing CSRF protection. All four now
carry [HttpPost], matching Store's convention.
Ownership-check asymmetry preserved, not fixed: Admin's originals for
every action in this region never checked customer ownership; Store's
did. Ported via 'if (scope.DefaultStoreId is not null) { ...check... }'
branches rather than a uniform check, mirroring GetCartList's
already-noted asymmetry from Task 7.
DeletePersonalizedProduct's global-scope branch calls
ICustomerViewModelService.DeletePersonalizedProduct(id) directly,
matching Admin's original convenience-method call, rather than a
two-step Get+Delete.
This is the last task adding to BaseCustomerController's shared action
surface; Task 9 starts the Admin-only BaseCustomerManagementController.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…ctions Task 9 of ARCH-001 Phase 20 (Customer consolidation): adds BaseCustomerManagementController, an abstract subclass of BaseCustomerController holding Admin-only actions (Impersonate, RemoveAffiliate, DeleteSelected, ExportExcelAll/Selected, CustomerNotesSelect/Add/Delete, ContactFormList) plus the real override of the CheckTwoFactorEnabledWarning hook. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…omerManagementController subclass Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo Task 10 of ARCH-001 Phase 20 (Customer consolidation). Resumed from a prior implementer's uncommitted work; verified it against the task-10 brief before committing. - CustomerController now a thin subclass of BaseCustomerManagementController, restating [AuthorizeAdmin]/[AutoValidateAntiforgeryToken]/[Area]/[AuthorizeMenu] (previously arrived transitively via BaseAdminController, which the new shared base cannot inherit). - Added CustomerControllerAttributeTests.cs regression guard, verified by inspection to genuinely fail against the pre-cutover controller body (commit ed1d8b3): BaseAdminController carried those four attributes, and CustomerController itself carried none of its own, so inherit:false lookups return empty against the old body. - No pre-existing CustomerControllerTests.cs existed in this repo to trim/delete (brief's Step 4 was moot here).
…omerController subclass with ApplyPostConstraints override Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…ed, add store_ widget-zone satellites Moves the 9 shared tab partials + CreateOrUpdateAddress.cshtml + SendEmail.cshtml + the CreateOrUpdate.cshtml tabstrip shell into Grand.Web.AdminShared. Every <vc:admin-widget> call is replaced by a Partials/WidgetZone.*.cshtml per-area-partial (default in Grand.Web.Admin backed by vc:admin-widget, override in Grand.Web.Store backed by vc:store-widget with a new store_-prefixed zone name), per the established GiftVoucher Phase 17 pattern - no <vc:...> literal lives in any AdminShared view. Step 1's diff was not empty for 10 of 11 files (contradicting the brief's assumption); resolved per coordinator ruling using idioms already established elsewhere in this AdminShared tree: - Constants.AreaAdmin/Constants.AreaStore literals -> `var area = ViewContext.RouteData.Values["area"]?.ToString();` (matches Category/Page/CustomerAttribute's own AdminShared views; Constants isn't even resolvable in AdminShared, which imports neither host's Extensions namespace). - AdminAreaSettings.HideStoreColumn-gated grid columns (TabCurrentShoppingCart/TabCurrentWishlist/ TabOrders/TabOutOfStockSubscriptions/TabLoyaltyPoints) and TabLoyaltyPoints' extra AddLoyaltyPointsStoreId field/JS -> wrapped in `@if (area == "Admin" && ...)`, reproducing both hosts' current behavior by construction (Store's UI is always single-store, so it never rendered these regardless of the setting). - CreateOrUpdateAddress.cshtml moved as-is (BOM-only diff). The tabstrip shell adds a local `@inject IAdminDataScope<Customer> Scope` (shadowing the tree-wide Product-typed one in _ViewImports.cshtml, matching Order/Shipment/VendorReview's own local overrides) and gates the three Admin-only tabs (CustomerNotes, ActivityLog, Documents) on `Scope.DefaultStoreId is null`. Those three tabs' own partials stay in Grand.Web.Admin's tree (Store never had them) alongside CreateOrUpdate.TabInfo.cshtml, untouched per Task 13's scope. Does not touch Partials/CreateOrUpdate.TabInfo.cshtml - Task 13's own field-by-field audit.
…by-field audit Merged the last unmigrated Customer partial into AdminShared after a full field-by-field diff (Admin 487 lines vs Store 406). Every ADMIN-ONLY diff block accounted for: - AJAX area literal (Constants.AreaAdmin/AreaStore) -> the established `var area = ViewContext.RouteData.Values["area"]?.ToString()` idiom from Task 12. - customer_details_info_top/bottom widget zone calls -> per-area satellite partials (WidgetZone.InfoTop/InfoBottom.cshtml), Admin default + store_-prefixed Store override, same pattern as every other Customer tab. - CustomerGroups/VendorId/StaffStoreId/StoreId/Owner/SeId fields: forced or blanked by BaseCustomerController.ApplyPostConstraints (Store-only override) -- confirmed against its source, which also documents StoreId as forced (the brief's own field list omitted it). Gated on `Scope.DefaultStoreId is null`. - CustomerTags field: not forced by ApplyPostConstraints but never exposed by Store's panel; PrepareCustomerModel populates it unconditionally for both hosts, so this is an Admin-only UI capability, not a data constraint. Same Scope gate. - AffiliateId block: not in the brief's worked examples. Its asp-action="RemoveAffiliate" only exists on BaseCustomerManagementController, which only Admin's CustomerController inherits -- Store's controller doesn't have the action at all, so rendering the block unconditionally would post to a nonexistent route. Same Scope gate, combined with the existing AffiliateId null-check. - LastIpAddress/CreatedOn/LastActivityDate: Admin wraps each field in its own @if, Store wraps all three in one @if with identical conditions and identical inner markup -- cosmetic only, kept Admin's three-block form. - BOM, comment-only lines, blank lines: whitespace-class, ignored. Added two model-preparation characterization tests to CustomerViewModelServiceTests.cs proving PrepareCustomerModel populates every Scope-gated field for a global-scoped call, and that a store-scoped existing-customer call still maps StoreId/VendorId from the entity (the Store-only blanking happens in ApplyPostConstraints on POST, not here). Build: dotnet build GrandNode.sln -> 0 errors. Tests: Grand.Web.Admin.Tests 1264/1264 passed (was 1262 baseline + 2 new). Grand.Web.Store.Tests 111/111 passed (unchanged).
… gates Fixes findings from the final whole-branch review of the Customer consolidation: - ProductsPrice/PersonalizedProducts: restore Store's dropped ownership gate (cross-tenant PII read) using the LoadAuthorizedCustomer pattern already established by GetCartList/OutOfStockSubscriptionList. - ReviewList: restore the customer-ownership half of its original two-part gate; the store-id filter passed into GetAllProductReviews was already correct and is left untouched. - List(): add back [PermissionAuthorizeAction(PermissionActionName.List)], dropped from Store's original during the merge. Confirmed Admin's seeded ManageCustomers permission already grants List by default, so this cannot regress Admin's own access. - Edit(POST)/Delete/SendEmail: widen catch (GrandException) back to catch (Exception), restoring Admin's original catch-all behavior that was silently narrowed by taking Store's catch clause during the merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
…ates Adds coverage for the review fixes in BaseCustomerController.cs: - ProductsPrice/PersonalizedProducts: Store-scope denial (empty result, service never called) and global-scope pass-through pairs. - ReviewList: same Store-scope/global-scope pair. - DeleteProductPrice: renamed the misnamed DeleteProductPrice_StoreScope_DeniedOwnership_NoOpsSilently to DeleteProductPrice_StoreScope_PriceNotFound_NoOpsSilently (it exercised the not-found branch, not ownership denial) and added a new test for the real ownership-denial branch using a CustomerProductPrice that exists but belongs to a customer outside the caller's store. Exposes CustomerProductServiceMock as a test-class field so individual tests can control GetCustomerProductPriceById. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two mutating Admin actions in the new shared management controller remain GET-reachable (missing [HttpPost]), bypassing antiforgery validation and expanding CSRF exposure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors Customer management to eliminate duplicated Admin/Store controllers by moving the shared action surface into Grand.Web.AdminShared base controllers, using a routed IAdminDataScope<Customer> to enforce per-host access rules, and migrating most Customer views into AdminShared with host-specific overrides and new Store widget-zone extension points.
Changes:
- Consolidated Admin + Store customer actions behind
BaseCustomerController/BaseCustomerManagementController, with Store reduced to a thin controller plus store-only gate + post-constraint enforcement. - Introduced
IAdminDataScope<Customer>implementations (AdminCustomerDataScope,StoreCustomerDataScope) and a routed resolver (RoutedCustomerDataScope) wired via DI. - Migrated/reworked Customer UI partials into
AdminShared, adding host-specific WidgetZone partials and expanding test coverage for the new shared surface and scope routing.
File summaries
| File | Description |
|---|---|
| src/Web/Grand.Web.Store/Controllers/CustomerController.cs | Store controller becomes thin subclass of shared base; keeps store-only gate + POST constraints. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.WishlistTop.cshtml | Adds Store widget zone hook for wishlist tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.WishlistBottom.cshtml | Adds Store widget zone hook for wishlist tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ShoppingCartTop.cshtml | Adds Store widget zone hook for shopping cart tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ShoppingCartBottom.cshtml | Adds Store widget zone hook for shopping cart tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ReviewsTop.cshtml | Adds Store widget zone hook for reviews tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ReviewsBottom.cshtml | Adds Store widget zone hook for reviews tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ProductTop.cshtml | Adds Store widget zone hook for personalized products tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ProductBottom.cshtml | Adds Store widget zone hook for personalized products tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ProductPriceTop.cshtml | Adds Store widget zone hook for product price tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.ProductPriceBottom.cshtml | Adds Store widget zone hook for product price tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.OrdersTop.cshtml | Adds Store widget zone hook for orders tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.OrdersBottom.cshtml | Adds Store widget zone hook for orders tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.LoyaltyPointsTop.cshtml | Adds Store widget zone hook for loyalty points tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.LoyaltyPointsBottom.cshtml | Adds Store widget zone hook for loyalty points tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.InfoTop.cshtml | Adds Store widget zone hook for customer info tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.InfoBottom.cshtml | Adds Store widget zone hook for customer info tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.DetailsTabs.cshtml | Adds Store widget zone hook for customer details tabs. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.BackInStockTop.cshtml | Adds Store widget zone hook for back-in-stock tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.BackInStockBottom.cshtml | Adds Store widget zone hook for back-in-stock tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.AddressesTop.cshtml | Adds Store widget zone hook for addresses tab (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.AddressesBottom.cshtml | Adds Store widget zone hook for addresses tab (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.AddressDetailsTop.cshtml | Adds Store widget zone hook for address edit/create (top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/WidgetZone.AddressDetailsBottom.cshtml | Adds Store widget zone hook for address edit/create (bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabReviews.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabProductPrice.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabProduct.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabOutOfStockSubscriptions.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabOrders.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabLoyaltyPoints.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabInfo.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabCurrentWishlist.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.TabCurrentShoppingCart.cshtml | Removes Store-specific implementation (now shared). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Customer/Partials/CreateOrUpdate.cshtml | Removes Store-specific wrapper (now shared/overridden as needed). |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/SendEmail.cshtml | Makes SendEmail partial area-aware so it works for Admin and Store. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdateAddress.cshtml | Adds widget-zone partial hooks around address editor. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabReviews.cshtml | Uses area-aware routes + host widget-zone partials for reviews tab. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabProductPrice.cshtml | Uses area-aware routes + host widget-zone partials for product price tab. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabProduct.cshtml | Uses area-aware routes + host widget-zone partials for personalized products tab. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabOutOfStockSubscriptions.cshtml | Uses area-aware routes + gates store-column to Admin only. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabOrders.cshtml | Uses area-aware routes + gates store-column to Admin only. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabLoyaltyPoints.cshtml | Uses area-aware routes + gates store selector fields to Admin only. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabInfo.cshtml | Gates Admin-only fields based on Scope.DefaultStoreId and makes URLs area-aware. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabCurrentWishlist.cshtml | Uses area-aware routes + gates store-column to Admin only + host widget zones. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabCurrentShoppingCart.cshtml | Uses area-aware routes + gates store-column to Admin only + host widget zones. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.TabAddresses.cshtml | Uses area-aware routes and adds host widget-zone hooks for addresses tab. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Customer/Partials/CreateOrUpdate.cshtml | Makes wrapper area-aware; gates notes/activity/documents tabs for Admin only; adds host widget-zone hook. |
| src/Web/Grand.Web.AdminShared/Startup/StartupApplication.cs | Registers Customer data scopes + routed resolver in DI. |
| src/Web/Grand.Web.AdminShared/Services/StoreCustomerDataScope.cs | Adds store-scoped customer access rules (store + registered-only). |
| src/Web/Grand.Web.AdminShared/Services/RoutedCustomerDataScope.cs | Resolves correct Customer scope at request-time based on area route value. |
| src/Web/Grand.Web.AdminShared/Services/AdminCustomerDataScope.cs | Adds Admin customer access rules (Sales Manager gating + deleted check). |
| src/Web/Grand.Web.AdminShared/Controllers/BaseCustomerManagementController.cs | Introduces Admin-only customer management surface (impersonation, notes, export, etc.). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.WishlistTop.cshtml | Adds Admin widget zone partial (wishlist top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.WishlistBottom.cshtml | Adds Admin widget zone partial (wishlist bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ShoppingCartTop.cshtml | Adds Admin widget zone partial (shopping cart top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ShoppingCartBottom.cshtml | Adds Admin widget zone partial (shopping cart bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ReviewsTop.cshtml | Adds Admin widget zone partial (reviews top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ReviewsBottom.cshtml | Adds Admin widget zone partial (reviews bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ProductTop.cshtml | Adds Admin widget zone partial (product top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ProductPriceTop.cshtml | Adds Admin widget zone partial (product price top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ProductPriceBottom.cshtml | Adds Admin widget zone partial (product price bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.ProductBottom.cshtml | Adds Admin widget zone partial (product bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.OrdersTop.cshtml | Adds Admin widget zone partial (orders top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.OrdersBottom.cshtml | Adds Admin widget zone partial (orders bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.LoyaltyPointsTop.cshtml | Adds Admin widget zone partial (loyalty points top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.LoyaltyPointsBottom.cshtml | Adds Admin widget zone partial (loyalty points bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.InfoTop.cshtml | Adds Admin widget zone partial (info top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.InfoBottom.cshtml | Adds Admin widget zone partial (info bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.DetailsTabs.cshtml | Adds Admin widget zone partial (details tabs) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.BackInStockTop.cshtml | Adds Admin widget zone partial (back-in-stock top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.BackInStockBottom.cshtml | Adds Admin widget zone partial (back-in-stock bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.AddressesTop.cshtml | Adds Admin widget zone partial (addresses top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.AddressesBottom.cshtml | Adds Admin widget zone partial (addresses bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.AddressDetailsTop.cshtml | Adds Admin widget zone partial (address details top) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/WidgetZone.AddressDetailsBottom.cshtml | Adds Admin widget zone partial (address details bottom) consumed by shared views. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/SendEmail.cshtml | Removes Admin-local SendEmail partial (now shared). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/CreateOrUpdateAddress.cshtml | Removes Admin-local address partial (now shared). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Customer/Partials/CreateOrUpdate.TabAddresses.cshtml | Removes Admin-local addresses tab partial (now shared). |
| src/Tests/Grand.Web.Store.Tests/Controllers/CustomerControllerTests.cs | Updates Store controller tests to cover only store-specific behavior + anti-smuggling constraints. |
| src/Tests/Grand.Web.Store.Tests/Controllers/CustomerControllerAttributeTests.cs | Adds Store controller attribute/regression tests for the thin subclass. |
| src/Tests/Grand.Web.Admin.Tests/Services/CustomerViewModelServiceTests.cs | Extends service tests for fields now conditionally rendered via scope-gated views. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/StoreCustomerDataScopeTests.cs | Adds unit tests for Store customer scope ownership + registered-only rules. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/RoutedCustomerDataScopeTests.cs | Adds unit tests ensuring correct per-area scope resolution and fail-closed behavior. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/CustomerControllerAttributeTests.cs | Adds Admin controller attribute/regression tests verifying thin subclass + base inheritance. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCustomerManagementControllerTests.cs | Adds tests for admin-only management actions (impersonation + 2FA warning hook). |
| src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCustomerControllerTests.cs | Adds broad shared-surface regression tests (list model prep, ownership gates, etc.). |
| src/Tests/Grand.Web.Admin.Tests/Controllers/AdminCustomerDataScopeTests.cs | Adds unit tests for Admin customer scope (Sales Manager gating + deleted handling). |
Review details
Suppressed comments (1)
src/Web/Grand.Web.AdminShared/Controllers/BaseCustomerManagementController.cs:142
- CustomerNoteAdd mutates state but is missing [HttpPost], so it can be reached via GET and won’t trigger antiforgery validation. The Admin view posts to this endpoint already, so adding [HttpPost] should be compatible while closing the GET/CSRF surface.
- Files reviewed: 89/89 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| var view = result as ViewResult; | ||
| Assert.IsNotNull(view); | ||
| var model = view.Model as CustomerListModel; | ||
| Assert.IsTrue(model.UsernamesEnabled); |
|
|
||
| var view = result as ViewResult; | ||
| var model = view.Model as CustomerModel; | ||
| Assert.IsTrue(model.Active); |
| Assert.IsNull(customer); | ||
| var redirect = denied as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("List", redirect.ActionName); |
| Assert.IsNull(result); | ||
| var redirect = denied as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("List", redirect.ActionName); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("List", redirect.ActionName); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("List", redirect.ActionName); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("AddressEdit", redirect.ActionName); |
|
|
||
| var json = result as JsonResult; | ||
| var data = json.Value as DataSourceResult; | ||
| Assert.AreEqual(0, data.Total); |
|
|
||
| var json = result as JsonResult; | ||
| var data = json.Value as DataSourceResult; | ||
| Assert.AreEqual(0, data.Total); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("List", redirect.ActionName); |
|
…tion Admin's pre-consolidation Create(POST) required PermissionActionName.Edit while every other Create action (and Store's original Create) required .Create - same defect class already disclosed and fixed for GiftVoucher. The consolidation harmonized on .Create but this went undisclosed in the PR description and had no regression test. Adds the disclosing comment (mirrors BaseGiftVoucherController's) and a CreatePost_RequiresCreatePermission test mirroring GiftVoucherControllerAttributeTests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nm3A9doUBAUWVM3FKHb45
…stomer-consolidation # Conflicts: # src/Web/Grand.Web.AdminShared/Startup/StartupApplication.cs
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…dation' into arch001/phase20-customer-consolidation
…leteSelected The prior 'Copilot Autofix' commit (5d3e60b) added [HttpPost] to DeleteSelected but dropped the method's opening brace in the process, which would not compile. Restored it; the [HttpPost] addition itself is correct and matches the disclosed 'GET-reachable mutators' finding in the PR description. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nm3A9doUBAUWVM3FKHb45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type: refactor
Issue
ARCH-001 Phase 20:
Grand.Web.Admin.Controllers.CustomerController(1006 lines) andGrand.Web.Store.Controllers.CustomerController(906 lines) duplicated nearly the entire Customermanagement surface instead of sharing it via
Grand.Web.AdminShared, following the pattern alreadyshipped for Product, Category, Collection, Order, Shipment, PaymentTransaction, MerchandiseReturn,
Reports, VendorReview, the attribute family, Discount, Brand, Blog, Page, News, GiftVoucher,
ProductReview, and MessageTemplate.
Solution
Merged both controllers behind
BaseCustomerController/BaseCustomerManagementControllerinGrand.Web.AdminShared, driven by a newIAdminDataScope<Customer>— two bespoke, non-genericscope classes (
AdminCustomerDataScope: Sales-Manager-aware;StoreCustomerDataScope:store+registered-only), since Customer's ownership rules have no parallel among already-shipped
entities. Admin's concrete controller is a fully-empty thin subclass carrying the complete action
surface; Store's inherits the shared base directly (a genuine subset — Impersonate/Export/
CustomerNote*/ContactForm stay Admin-only) plus its own per-store feature gate,
PerStoreDisabledpage, and
ApplyPostConstraintsoverride forcing ownership fields on every POST. All Customer viewsmigrated into
AdminSharedexceptEdit/Create/List(genuine per-host overrides), withstore_-prefixed widget-zone satellites added throughout (Store's Customer views had zero prior<vc:>calls — all net-new extension points, not dead-code fixes).Design spec:
docs/superpowers/specs/2026-09-05-arch001-customer-consolidation-design.mdImplementation plan:
docs/superpowers/plans/2026-09-05-arch001-customer-consolidation.md(both gitignored, referenced here for anyone continuing this initiative)
Disclosed, intentional behavior changes (not pure refactor)
List()now populatesCustomerSettings-driven column-visibility flags for Store. Store'soriginal silently returned
new CustomerListModel(), always hiding the Username/Company/Phone/ZipPostalCode optional grid columns regardless of what the store owner configured. Real,
user-visible bug fix.
[HttpPost]added to 4 previously GET-reachable mutating actions(
UpdateProductPrice/DeleteProductPrice/UpdatePersonalizedProduct/DeletePersonalizedProduct)on Admin — they never carried it, meaning
[AutoValidateAntiforgeryToken](which only validatesnon-safe verbs) never protected them. Store's originals already had
[HttpPost]. Closes a realCSRF-bypass gap.
AdminCustomerDataScope.HasAccesswas missing aDeletedcheck present throughout Admin'soriginal controller (
customer.Deletedwas checked inline before nearly every action). Foundmid-implementation (surfaced as a minor note during one task's review, confirmed load-bearing),
fixed at the scope-class level before later work consumed it — soft-deleted customers are once
again inaccessible via Admin, matching the original exactly.
Create(POST) requiredPermissionActionName.Editinstead of.Create— theonly Admin Create action gated on the wrong permission (every other Create action, and Store's
original Create, correctly required
.Create). Same defect class already disclosed and fixedfor GiftVoucher's consolidation. Harmonized on
.Create; a user grantedCreatebut notEditnow gains customer-creation access they lacked before, and vice versa for
Edit-only users.Found in post-merge review of this PR. Regression test:
CustomerControllerAttributeTests.CreatePost_RequiresCreatePermission.Final whole-branch review — 2 Critical + 3 Important, all fixed
The final review (dispatched on the most capable available model, independently re-deriving every
access-control gate from the original pre-consolidation controllers rather than trusting per-task
review verdicts) found:
ProductsPriceandPersonalizedProductssilently lost Store's ownership gateduring the task that added them — Store's originals both opened with a
GetStoreCustomer-stylecheck denying access to another store's customer, dropped in the merge. Any store-panel user with
Customer-preview permission could POST an arbitrary
customerIdand read that customer'snegotiated prices / personalized-product assignments, including customers belonging to other
stores. Same defect class this initiative has been bitten by before (News Phase 16's Comments/
CommentDelete leak): the task's own framing named the mutating sibling actions
(
UpdateProductPrice/DeleteProductPrice, which got correct gates), and the adjacent readactions one region away were missed by both the implementer and its task reviewer.
ReviewListlost the customer-ownership half of its original two-part gate (thestore-id filter half survived) — bounded impact (results stay within the caller's own store), but
a store manager could enumerate which foreign-store customers reviewed on their store, which the
original denied.
List()lost[PermissionAuthorizeAction(PermissionActionName.List)],present on Store's original. A store user whose
Listpermission was revoked could still load thepage shell (no data exposure —
CustomerListPOST retained its own gate).Edit/Delete/SendEmailnarrowed Admin's originalcatch (Exception)tocatch (GrandException)(Store's original, narrower form) — an undisclosed behavior change forAdmin: a non-
GrandExceptionfailure now propagates as an unhandled 500 instead of being caughtand surfaced via
Error(exc.Message)with the form redisplayed.All 5 fixed in one wave (regression tests added for the two Critical fixes and
ReviewList;List()'s permission grant independently verified against the realCustomerspermission seedbefore adding; all three
catchsites widened back toException), then independentlyre-verified in a scoped re-review — all 5 confirmed ADDRESSED, no new breakage. 1271/1271
Grand.Web.Admin.Testspassing after the fix wave.Minor findings, disclosed not fixed (triaged as non-blocking by the final reviewer)
store_customer_details_buttons/store_customer_list_buttonswidget-zone satellites were notadded (spec called for them on the kept-host-specific
Edit/Create/Listviews) — two missingextension points, not a regression.
ViewContext.RouteData.Values["area"]andScope.DefaultStoreId is null) are both used, sometimes in the same file — both are sanctioned byViews/AdminShared/_ViewImports.cshtml's own documentation; picking one per file would readbetter.
slots even when hidden for Store — self-healing on next save, cosmetic.
List()perf micro-optimization (skip group/tag lookups for Store) wasn'timplemented — both hosts pay for the full
PrepareCustomerListModel()call. Two extra reads perpage-load, acceptable.
AddressCreate/AddressEdit/ProductAddPopupviews remain duplicated per host (expected tounify pending a task-time diff; the diff turned out non-trivial and this was descoped, not
silently dropped).
SendEmail,LoyaltyPointsHistoryAdd,CustomerNoteAdd,DeleteSelected) remain without[HttpPost], faithfully preserved from both originals — fixing4 of ~8 in one PR was a defensible scope line, not a gap introduced here.
Live smoke test — partial, honestly disclosed gap
Verified live against a real dev DB (Admin login,
/Admin/Customer/Listand/Admin/Customer/Editrender correctly with the full action surface and all 11 tabs — confirms the attribute-cutover
regression class this initiative has hit repeatedly did not recur here, and confirms the
TabInfo.cshtmlmerge — this plan's own single biggest flagged risk — renders correctly live, notjust via build+unit-tests). Store login and the per-store gate (
PerStoreDisabledredirect) alsoconfirmed live on Store's host.
Not verified live: the deeper Store-side flows (cross-store
Editdenial, theList()CustomerSettings-column fix, the two Critical
ProductsPrice/PersonalizedProductsfixes, and theApplyPostConstraintsanti-smuggling proof) — all requireCustomer:RegisterCustomersPerStore=truein the test environment, which is
falsethere (Store's Customer panel is disabled site-wide).Attempting to flip it in an isolated throwaway config confirmed the setting's own documented
warning: it broke login resolution for the existing seeded accounts (global-unique vs.
per-store-unique lookup semantics). Reverted immediately, verified zero net change. These specific
fixes were independently re-derived from source twice already (the final review's own re-derivation,
plus its scoped re-review's independent test execution) but never round-tripped through an actual
live HTTP request as a Store user. Flagged here rather than silently omitted.
Breaking changes
None for installations at default settings. See "Disclosed, intentional behavior changes" above —
all are fixes to existing gaps, not new restrictions, except the narrower/wider exception-catch
adjustment (restored to original width, not a new narrowing).
Testing
dotnet build GrandNode.sln— 0 errors, 0 warnings.run one project at a time):
Grand.Web.Admin.Tests1271/1271,Grand.Web.Store.Tests111/111,Grand.Mapping.Tests234/234./Adminand/Store, confirm the Customer list/edit screens render andexisting customer data displays correctly on both hosts.
Customer:RegisterCustomersPerStore=truein your environment: additionally verify astore-panel user cannot view/edit another store's customer, cannot smuggle
Owner/VendorId/StaffStoreId/SeId/CustomerGroupsvia a crafted POST, and thatProductsPrice/PersonalizedProductscorrectly deny cross-store access — this is the one class of check this PRitself could not complete live (see "Live smoke test" above).