ARCH-001 Phase 13: Brand controller/service consolidation (Admin/Store) - #806
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…randController subclasses Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…act Store WidgetZone satellites Task 7 - diff-and-classify all 8 Admin/Store Brand view pairs before moving anything, per the Category/Product precedent. Diff findings: - Create.cshtml / Edit.cshtml: diff was only the asp-area constant (Constants.AreaAdmin vs Constants.AreaStore) -> UNIFY, using the established `var area = ViewContext.RouteData.Values["area"]?.ToString();` + `asp-area="@area"` idiom already used by Category/Product. - List.cshtml: GENUINE DIVERGENCE (Admin-only SearchStoreId filter column + Export/Import Excel panel/modal/script, ~65 lines) -> KEPT AS OVERRIDE in each host's own Areas/*/Views/Brand, matching the Category precedent exactly (commit fa46d59). - Partials/CreateOrUpdate.cshtml: diff empty -> UNIFY. - Partials/CreateOrUpdate.TabDiscounts.cshtml: diff empty -> UNIFY. - Partials/CreateOrUpdate.TabDocuments.cshtml: diff was only the area constant (3 occurrences) -> UNIFY with the same `var area` transform. - Partials/CreateOrUpdate.TabInfo.cshtml: GENUINE DIVERGENCE (Admin-only CustomerGroups/Stores fields, ~14 lines) plus an area constant in the picture-popup URL -> KEPT AS OVERRIDE, matching Category's own TabInfo divergence (same fields, same precedent commit). - Partials/CreateOrUpdate.TabSeo.cshtml: diff empty -> UNIFY. Net: 6 of 8 files unified into Grand.Web.AdminShared/Views/AdminShared/Brand (Create, Edit, CreateOrUpdate, TabDiscounts, TabDocuments, TabSeo); List.cshtml and TabInfo.cshtml (2) kept as per-host overrides. Widget-zone finding: every one of the 8 Admin Brand views calls `<vc:admin-widget widget-zone="brand_*" .../>`, and Store's copies carried the identical literal call - Store's _ViewImports.cshtml never registers Grand.Web.Admin's tag helper, so these have always been dead literal markup in Store (same bug class already fixed for Category/Collection). Unlike the Category precedent, this task's brief explicitly scoped the fix to all 11 call sites including the two kept-as-override files (List.cshtml's brand_list_buttons, TabInfo.cshtml's brand_details_info_top/bottom), not just the 8 zones inside the 6 unified files. Fixed by extracting one Partials/WidgetZone.<Name>.cshtml satellite per call site (11 total, since brand_details_buttons appears twice - Create.cshtml with additional-data=null, Edit.cshtml with additional-data=Model - and needs two distinct satellites to preserve that distinction): - 8 zones used by the 6 unified files get a same-named satellite in BOTH Grand.Web.Admin/.../Brand/Partials and Grand.Web.Store/.../Brand/Partials (Admin's calls vc:admin-widget with the unchanged zone name; Store's calls vc:store-widget with a new store_brand_* zone name), resolved from the unified AdminShared view via the relative `<partial name="Partials/ WidgetZone.X" .../>` + per-host search-path fallback mechanism already proven by Category's shipped WidgetZone.* satellites (verified via `grep -rn "WidgetZone\." .../AdminShared/Category/` before writing any Brand satellite). - The 3 zones used only by the two kept-as-override files get a Store-only satellite (brand_list_buttons -> WidgetZone.ListButtons.cshtml; brand_details_info_top/bottom -> WidgetZone.Info.Top/Bottom.cshtml); Admin's own List.cshtml/TabInfo.cshtml keep their direct vc:admin-widget calls unchanged since those were never dead. This fixes Store's pre-existing dead-widget bug for Brand (new store_brand_* zones are not yet targeted by any widget plugin, so no visible behavior change today). Verified: dotnet build GrandNode.sln - 0 errors (20 pre-existing warnings, unrelated). Every <partial name="...WidgetZone..."> reference in the moved/ edited views confirmed via grep to resolve to a WidgetZone.*.cshtml file that exists at that exact relative path in the expected host(s). Every moved/ edited view's @model directive confirmed unchanged (BrandModel/ BrandListModel, matching BaseBrandController's View(model) calls). Live Kestrel render check (spec step 5) deferred to the controller session per task instructions - this environment does not drive a browser against a running Kestrel instance. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…overage Finding 1 (Critical): BaseBrandController.ExportXlsx always exported all brands across all stores regardless of caller scope. Fixed to match the BaseCategoryController/BaseCollectionController pattern: storeId: scope.DefaultStoreId ?? "". Added ExportXlsx_StoreScoped_ScopesExportToDefaultStoreId to BaseBrandControllerTests to lock in the store-scoped call. Finding 2 (Important): Store's BrandController.EditWarningCheck override (the one hand-ported, non-mechanical piece of business logic in the Brand consolidation) had zero direct test coverage. Added BrandControllerTests.EditWarningCheckTests covering all three branches of the condition via the public Edit(GET) action against a real BrandController instance.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The consolidation matches established Category/Collection patterns (including routed scopes and widget-zone satellites) and is backed by targeted regression/characterization tests for the key scoping and warning behaviors.
Pull request overview
Consolidates Admin + Store Brand admin-screens into a shared BaseBrandController in Grand.Web.AdminShared, using the existing IAdminDataScope<TEntity> pattern to preserve per-host access behavior while reducing duplication.
Changes:
- Added
RoutedBrandDataScopeand DI registrations soIAdminDataScope<Brand>resolves correctly per-request area (Admin vs Store) in the combined host. - Introduced
BaseBrandControllerand reduced both hostBrandControllerclasses to thin DI/attribute wiring (Store also overrides the warning hook). - Reworked Brand widget-zone injection to use host-specific partial “satellites” (
vc:admin-widgetvsvc:store-widget), aligning with the already-shipped Category/Collection precedent; added characterization/regression tests around scoping + warning logic.
File summaries
| File | Description |
|---|---|
| src/Web/Grand.Web.Store/Controllers/BrandController.cs | Becomes a thin subclass of BaseBrandController; retains Store-specific EditWarningCheck. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.Tabs.cshtml | Store widget-zone satellite partial (tabs). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.SEO.Top.cshtml | Store widget-zone satellite partial (SEO top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.ListButtons.cshtml | Store widget-zone satellite partial (list buttons). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.Info.Top.cshtml | Store widget-zone satellite partial (info top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.Info.Bottom.cshtml | Store widget-zone satellite partial (info bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.Documents.Top.cshtml | Store widget-zone satellite partial (documents top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.Documents.Bottom.cshtml | Store widget-zone satellite partial (documents bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.Discounts.Top.cshtml | Store widget-zone satellite partial (discounts top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.Discounts.Bottom.cshtml | Store widget-zone satellite partial (discounts bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.DetailsButtonsCreate.cshtml | Store widget-zone satellite partial (create buttons). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/WidgetZone.DetailsButtons.cshtml | Store widget-zone satellite partial (edit buttons). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/CreateOrUpdate.TabInfo.cshtml | Replaces dead vc:admin-widget calls with Store widget-zone partials. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Partials/CreateOrUpdate.cshtml | Removed (now served from shared AdminShared view set). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/List.cshtml | Uses Store widget-zone partial for list buttons. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Edit.cshtml | Removed (now served from shared AdminShared view set). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Brand/Create.cshtml | Removed (now served from shared AdminShared view set). |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Brand/Partials/CreateOrUpdate.TabSeo.cshtml | Switches widget injection to partial-based satellites. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Brand/Partials/CreateOrUpdate.TabDocuments.cshtml | Makes Document URLs area-aware + switches widget injection to satellites. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Brand/Partials/CreateOrUpdate.TabDiscounts.cshtml | Switches widget injection to partial-based satellites. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Brand/Partials/CreateOrUpdate.cshtml | Uses widget-zone tabs satellite partial. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Brand/Edit.cshtml | Makes the form’s asp-area dynamic + uses details-buttons satellite. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Brand/Create.cshtml | Makes the form’s asp-area dynamic + uses create-buttons satellite. |
| src/Web/Grand.Web.AdminShared/Startup/StartupApplication.cs | Registers global/store scopes + routes IAdminDataScope<Brand> via RoutedBrandDataScope. |
| src/Web/Grand.Web.AdminShared/Services/RoutedBrandDataScope.cs | New per-area resolver for Brand scope; fails closed for unrecognized areas. |
| src/Web/Grand.Web.AdminShared/Controllers/BaseBrandController.cs | New shared controller implementing list/create/edit/delete/picture/export/import with scoped access decisions. |
| src/Web/Grand.Web.Admin/Controllers/BrandController.cs | Becomes a thin subclass of BaseBrandController with Admin host attributes. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.Tabs.cshtml | Admin widget-zone satellite partial (tabs). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.SEO.Top.cshtml | Admin widget-zone satellite partial (SEO top). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.Documents.Top.cshtml | Admin widget-zone satellite partial (documents top). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.Documents.Bottom.cshtml | Admin widget-zone satellite partial (documents bottom). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.Discounts.Top.cshtml | Admin widget-zone satellite partial (discounts top). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.Discounts.Bottom.cshtml | Admin widget-zone satellite partial (discounts bottom). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.DetailsButtonsCreate.cshtml | Admin widget-zone satellite partial (create buttons). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/WidgetZone.DetailsButtons.cshtml | Admin widget-zone satellite partial (edit buttons). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/CreateOrUpdate.TabSeo.cshtml | Removed (now served from shared AdminShared view set). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/CreateOrUpdate.TabDocuments.cshtml | Removed (now served from shared AdminShared view set). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Brand/Partials/CreateOrUpdate.TabDiscounts.cshtml | Removed (now served from shared AdminShared view set). |
| src/Tests/Grand.Web.Store.Tests/Controllers/BrandControllerTests.cs | Adds Store controller attribute + warning-hook characterization tests. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/RoutedBrandDataScopeTests.cs | Adds tests ensuring area-based scope routing fails closed outside Admin/Store. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/BrandControllerTests.cs | Adds Admin controller attribute + base-class inheritance tests. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/BaseBrandControllerTests.cs | Adds characterization/regression tests for access gating and store-scoped export behavior. |
Review details
- Files reviewed: 42/42 changed files
- Comments generated: 0
- 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 = (BrandListModel)view.Model; |
|
|
||
| var view = result as ViewResult; | ||
| Assert.IsNotNull(view); | ||
| var model = (BrandListModel)view.Model; |
|
|
||
| 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 view = result as ViewResult; | ||
| Assert.IsNotNull(view); | ||
| Assert.AreEqual("Acme", ((BrandModel)view.Model).Name); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("Edit", redirect.ActionName); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("Edit", redirect.ActionName); |
|
|
||
| var content = result as ContentResult; | ||
| Assert.IsNotNull(content); | ||
| Assert.AreEqual("This is not your brand", content.Content); |
|
|
||
| var content = result as ContentResult; | ||
| Assert.IsNotNull(content); | ||
| Assert.AreEqual("Picture not exist", content.Content); |
|
|
||
| var content = result as ContentResult; | ||
| Assert.IsNotNull(content); | ||
| Assert.AreEqual("Brand not exist", content.Content); |
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.
Summary
Merges Grand.Web.Admin.Controllers.BrandController and Grand.Web.Store.Controllers.BrandController
into one shared BaseBrandController in Grand.Web.AdminShared, following the same
IAdminDataScope pattern already shipped for Product/Category/Collection/Order/Shipment/
PaymentTransaction/MerchandiseReturn/Reports/VendorReview/the attribute family/Discount.
No Vendor work (no VendorBrandController exists). No new IAdminDataScope members, no
bespoke scope classes - Brand reuses the existing generic Global/StoreAdminDataScope
exactly like Category/Collection.
What changed
List.cshtml and CreateOrUpdate.TabInfo.cshtml - have genuine per-host content divergence and were
kept as host-specific overrides instead, matching an identical-shape precedent already shipped
for Category).
Store's ViewImports.cshtml never registered Grand.Web.Admin's tag helper, so all 11 calls were
silently dead literal HTML. Extracted real per-host WidgetZone.*.cshtml satellites
(vc:store-widget/store_brand* naming) instead of reproducing the dead code - same fix already
shipped for Category/Collection/Product/PaymentTransaction.
Category/Collection siblings (which use
storeId: scope.DefaultStoreId ?? ""). Because Store'soriginal BrandController never had an ExportXlsx action at all, this consolidation would otherwise
have newly exposed an unscoped cross-store brand export on the Store host. Fixed to match the
sibling pattern, with a regression test asserting store-scoping.
Verification
Grand.Web.Store.Tests 56/56 (plus additions), Grand.Mapping.Tests 234/234 - all green.
both hosts' thin BrandController subclasses were independently cross-checked, attribute-by-
attribute and using-line-by-using-line, against the already-shipped CategoryController AND
CollectionController in both hosts - exact match confirmed twice (once during task review, once
during the final whole-branch review).
PR was opened). Built this branch and pointed a real Kestrel instance at the developer's own local
dev database (
grand20260825, MongoDB running as a Windows service) on port 44360. Logged in asadmin@yourstore.comandstore1@store.comvia a real browser session (no synthetic requests).Admin: List/Create/Edit all render correctly against real data (3 pre-existing brands plus a new
test brand); created a brand limited exclusively to "Store2"; Edit(GET) showed the correct
"Limited to stores: STORE2" state, all 4 tabs (Info/SEO/Discounts/Documents) rendered without
error, confirming the widget-zone satellites resolve correctly for Admin too. Store (as store1,
whose own store is not Store2):
Edit(GET)on the Store2-exclusive brand correctly redirected toList(CanView deny); the List grid correctly excluded that brand entirely (store-scoped List(POST)filtering);
PicturePopupreturned the exact expected "This is not your brand" deny message(HasAccess check). Positive control: store1 created and opened its own brand successfully, forced
onto its own store, no warning, all tabs rendering (Info/SEO/Discounts/UserFields - Store's expected
tab set, no Documents tab). Zero routing 404s encountered on either host, confirming the
attribute-cutover risk (this initiative's most common failure mode) did not recur here. All test
data cleaned up via crafted antiforgery-tokened deletes;
Admin/Brand/Listconfirmed back toexactly its original 3 brands afterward. This closes the one verification gap the PR previously
disclosed as outstanding.
🤖 Generated with Claude Code