Skip to content

ARCH-001: consolidate Admin/Store CategoryController into Grand.Web.AdminShared - #792

Merged
KrzysztofPajak merged 12 commits into
developfrom
arch001/phase3-category-consolidation
Aug 24, 2026
Merged

ARCH-001: consolidate Admin/Store CategoryController into Grand.Web.AdminShared#792
KrzysztofPajak merged 12 commits into
developfrom
arch001/phase3-category-consolidation

Conversation

@KrzysztofPajak

Copy link
Copy Markdown
Member

Type: refactor

Issue

Grand.Web.Admin and Grand.Web.Store each shipped their own CategoryController (349 / 365
lines, ~195-line diff after whitespace normalization), duplicating category CRUD, picture
handling, export/import, and category-product association screens plus every access check twice
over — the same class of duplication ARCH-001 (PR #790) already fixed for Product.

Full design: docs/superpowers/specs/2026-08-24-arch001-category-consolidation-design.md
Full plan: docs/superpowers/plans/2026-08-24-arch001-category-consolidation.md
(Both gitignored under docs/superpowers/, same as Product's — on disk in this branch's history
but not tracked by git; ask if you'd like them attached separately.)

Solution

Reuses the IAdminDataScope<TEntity> pattern proven by ARCH-001 Phase 1/2 (Product) — no new
interface members, no new bespoke scope classes:

  • New RoutedCategoryDataScope (2-branch: Admin/Store, fails closed on anything else including
    "Vendor" — Category has no Vendor screen at all, confirmed empty
    Grand.Web.Vendor/Controllers/) resolves the existing generic GlobalAdminDataScope<Category>/
    StoreAdminDataScope<Category> per request area. Registered centrally in
    Grand.Web.AdminShared/Startup/StartupApplication.cs, avoiding the DI last-registration-wins
    collision documented for Product's RoutedProductDataScope.
  • New BaseCategoryController in Grand.Web.AdminShared holds all 5 regions (List,
    Create/Edit/Delete, Picture, Export/Import, Products) migrated from both original controllers.
    ICategoryViewModelService/CategoryViewModelService needed no changes — unlike Product,
    it already lived solely in Grand.Web.AdminShared, correctly storeId-parameterized.
  • Admin/Store CategoryControllers reduced to thin BaseCategoryController subclasses (Admin 34
    lines, Store 47, vs. ~350 each before).
  • 7 of 10 shared views unified under Grand.Web.AdminShared/Views/AdminShared/Category/; 3 kept
    as genuine host-specific overrides (List, ProductAddPopup, CreateOrUpdate.TabInfo — each has a
    real Admin-only field/panel Store doesn't). Additionally found and fixed a pre-existing bug:
    Category's <vc:admin-widget> calls were dead literal HTML in Store (Store's
    _ViewImports.cshtml never registers Grand.Web.Admin's tag helper) — same bug class already
    documented and fixed for Product's widget zones. Extracted 20 new per-host WidgetZone.*.cshtml
    satellites (vc:store-widget/store_category_* naming) following the proven Product pattern.
  • Characterization tests written before/alongside the migration as a safety net (no
    CategoryController/CategoryViewModelService tests existed before this PR) —
    BaseCategoryControllerTests covers scope-based access on every action for both hosts.
  • Live cross-store isolation smoke test run against a real Kestrel-hosted instance + throwaway
    MongoDB: a category exclusively owned by "Store2", 10 action/method pairs attempted as a
    different store's user with a valid antiforgery token — denials correct on every applicable
    action, positive control succeeded, zero mutation confirmed post-hoc.
  • One real regression found and fixed by the final whole-branch review: List(POST) and
    ProductAddPopupList(POST) both unconditionally forced model.SearchStoreId = scope.DefaultStoreId, which for Admin (always null) silently overwrote the Admin user's own
    submitted store filter — Admin's original code never touched that field. Fixed with the same
    conditional guard BaseProductController already uses in ~12 places; covered by 2 new tests.
  • Also fixed, unrelated but discovered during branch setup: a pre-existing malformed
    .gitignore line on develop where .worktrees/ had been concatenated onto a thumbs-pattern
    glob with no separating newline, leaving .worktrees/ not actually ignored.

Deliberate, disclosed behavior differences from the two original hosts (both minor,
non-security):

  • Delete's access-denied redirect target unified to Edit (matching Store's actual original —
    Admin had no check at all before, so no precedent either way).
  • ExportXlsx/ImportFromXlsx are now reachable from Store (previously Admin-only) — same
    superset approach Product's Phase 1 used; Store's permission seed already grants
    ManageCategories with no per-action deny-list, so this is functional, not just code presence.

Not changed by this PR: ProductAddPopup GET's missing category-ownership check (verified
present in both original controllers — faithfully preserved, not introduced here); a pre-existing,
unrelated UserFields.cshtml NPE found during live testing (file never touched by this branch).
Both flagged as follow-up candidates, not blockers.

Breaking changes

None. ICategoryViewModelService's public contract is unchanged. This is a pure code-organization
refactor of two hosts' controllers/views into one shared base, plus the one disclosed
SearchStoreId-forcing bugfix and the two disclosed minor behavior normalizations above.

Testing

  1. dotnet build GrandNode.sln — 0 errors, 0 warnings.
  2. dotnet test src/Tests/Grand.Web.Admin.Tests (unfiltered) — 448 passed, 0 failed.
  3. dotnet test src/Tests/Grand.Web.Store.Tests (unfiltered) — 33 passed, 0 failed.
  4. dotnet test src/Tests/Grand.Mapping.Tests (unfiltered) — 234 passed, 0 failed.
  5. Live cross-store smoke test against a real running instance (see Solution above) — all
    applicable denials correct, positive control passed, no unauthorized mutation.
  6. Executed via superpowers:subagent-driven-development: 11 plan tasks, 2 task-level fix-loop
    rounds (both closing real test-coverage gaps, not correctness bugs), one final-whole-branch-review
    fix round (the SearchStoreId regression above) — full task-by-task ledger available on request
    if useful for review context.

🤖 Generated with Claude Code

KrzysztofPajak and others added 12 commits August 24, 2026 15:43
….worktrees/)

Commit 9dfbec5 appended '.worktrees/' onto the previous line because
src/Web/Grand.Web.Store/wwwroot/assets/images/thumbs/*.* had no trailing
newline, producing thumbs/*.**.worktrees/ - a single, useless pattern that
matched neither the thumbs glob nor .worktrees/. Split back into two lines.

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

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
- Implements Create(GET), Create(POST), Edit(GET), Edit(POST), Delete actions
- Delete denial redirects to Edit with ID (not List) per controller ruling, matching Store's original behavior and BaseProductController pattern
- All 9 tests passing (2 existing List tests + 7 new Create/Edit/Delete tests)
Verified Store's original controller had no Export/Import actions (grep, zero hits) - this is a
pure addition for Store, same superset approach ARCH-001 Phase 1 used for Product. Export is now
store-scoped for Store via scope.DefaultStoreId, unlike Admin's original always-global storeId: ''.

Permission-exposure verification: PermissionProvider.cs seeds no per-action deny-list entries at
all (grep for PermissionActionName.Export/Import: zero hits), and PermissionService.AuthorizeAction
implements a deny-list model (a PermissionAction DB record blocks a specific group+permission+action
combo; absence of a record means allowed once the coarse ManageCategories permission is granted).
StoreManager's DefaultPermission already grants ManageCategories. So once this code ships, Store
gains functional Export/Import buttons as a side effect by default (no seeded record excludes it) -
same outcome as Product's superset approach in Phase 1, and not a genuine code-level allow-list
excluding Store, so no follow-up note is needed for Task 9.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
…st coverage (task review fix)

- Add ProductAddPopupInsert_StoreScope_FiltersOutForeignProducts: verifies products from other stores are filtered
- Add ProductAddPopupInsert_StoreScope_AllProductsForeign_SkipsInsertEntirely: verifies InsertCategoryProductModel is skipped when all products are foreign
- Add ProductDelete_ProductNotOwnedByScopeStore_ReturnsKendoError: mirrors ProductUpdate denial test for Delete action
- Add ProductList_ScopeGrantsAccess_ReturnsData: happy path test for ProductList with access granted
- Remove dead Mock line from ProductUpdate test

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
…ared/Views/AdminShared/Category

Task 8 — diff-and-classify all 10 Admin/Store Category view pairs (List,
Create, Edit, ProductAddPopup, Partials/CreateOrUpdate + 5 tabs) before
moving anything, per the Product Phase 2 precedent.

Diff findings:
- List.cshtml: real diff (Admin-only import/export panel + SearchStoreId
  filter column + modal/script, ~65 lines) -> KEEP AS OVERRIDE, both hosts
  untouched.
- 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"` transform (same idiom already used by every unified
  Product view).
- ProductAddPopup.cshtml: real diff (Admin-only SearchStoreId/SearchVendorId
  filter fields, ~14 lines) -> KEEP AS OVERRIDE, both hosts untouched.
- 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: real diff (Admin-only
  CustomerGroups/Stores fields, ~14 lines) plus an area constant in the
  picture-popup URL -> KEEP AS OVERRIDE, both hosts untouched.
- Partials/CreateOrUpdate.TabProducts.cshtml: diff was only the area
  constant (4 occurrences) -> UNIFY with the same `var area` transform.
- Partials/CreateOrUpdate.TabSeo.cshtml: diff empty -> UNIFY.

Net: 6 of 10 files unified into Grand.Web.AdminShared/Views/AdminShared/
Category (Create, Edit, CreateOrUpdate, TabDiscounts, TabDocuments,
TabProducts, TabSeo — 7 files once WidgetZone extraction below is
counted; List/ProductAddPopup/TabInfo (3) kept as per-host overrides.

Widget-zone finding (not in the raw per-line diff, found by a targeted
grep before moving anything): every unify-candidate file calls
`<vc:admin-widget widget-zone="category_details_*" .../>` in BOTH the
Admin and the Store copy — Store's Category views never adapted this
copy-pasted call to its own `vc:store-widget`/`StoreWidgetViewComponent`,
so it has always been dead literal markup in Store (same pre-existing bug
class the Product Phase-2 postmortem found and fixed for Product's Store
views on 2026-08-18). Moving these files' literal `vc:admin-widget` calls
into AdminShared as-is would have reproduced the OTHER Product Phase-2
regression: AdminShared's own _ViewImports never registers Grand.Web.Admin's
tag helpers, so the call would compile as dead markup for Admin too.

Fixed by applying the same Partials/WidgetZone.<Name>.cshtml extraction
already used for Product: each of the 7 widget-zone call sites (
category_details_buttons, category_details_tabs, category_details_discounts_
{top,bottom}, category_details_documents_{top,bottom},
category_details_products_{top,bottom}, category_details_seo_{top,bottom})
now has a real, per-host partial (Admin keeps vc:admin-widget/original zone
name; Store gets a real vc:store-widget with a new store_category_details_*
zone name, mirroring the Product store_product_* convention) resolved via
the existing host-override-wins ViewLocationExpander precedence; the unified
AdminShared views call `<partial name="Partials/WidgetZone.X" .../>`
instead of the tag helper directly. This also fixes Store's pre-existing
dead-widget bug for Category (new store_category_details_* zones are not
yet targeted by any widget plugin, so no visible behavior change today).

Verified: dotnet build GrandNode.sln - 0 errors. Grand.Web.AdminShared.dll
(RCL, build-time Razor compilation) confirmed to contain the moved views
(21 "AdminShared/Category" + 33 "Admin.Catalog.Categories" string refs) and
zero literal "vc:" tag-helper strings (regression guard matching Product's
postmortem finding). Grand.Web.Admin/Grand.Web.Store compile Razor at
runtime in this project's configuration (Roslyn present in their own output,
no compiled-views assembly to string-inspect) so their own List/
ProductAddPopup/TabInfo overrides and the 10 new per-host WidgetZone.*
partials were verified by 0 build errors + reuse of tag-helper syntax
already proven working in each host's own Product views; full in-browser
smoke test of the Category admin screens is deferred to Task 10 per the
brief.
…filter isn't overwritten (final review fix)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
Copilot AI lite review requested due to automatic review settings August 24, 2026 15:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
Comment thread src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCategoryControllerTests.cs Dismissed
@KrzysztofPajak
KrzysztofPajak merged commit c1a7ead into develop Aug 24, 2026
6 checks passed
@KrzysztofPajak
KrzysztofPajak deleted the arch001/phase3-category-consolidation branch August 24, 2026 19:13
pull Bot pushed a commit to NicoJuicy/ecommerce-grandnode2 that referenced this pull request Aug 25, 2026
…Shared/Views/AdminShared/Collection

Diff-and-classify all 10 Admin/Store Collection view pairs before moving anything, per the
Product/Category precedent. 6 unified (Create, Edit, CreateOrUpdate + TabDiscounts/TabDocuments/
TabProducts/TabSeo), List/ProductAddPopup/TabInfo kept as host-specific overrides. Also found and
fixed the same dead-<vc:admin-widget>-in-Store tag-helper bug Category's Task 8 found: Store's
_ViewImports.cshtml never registers Grand.Web.Admin's tag helper, so all 13 collection_* widget
zone calls were silently dead literal HTML in Store - extracted per-host WidgetZone.*.cshtml
satellites (vc:store-widget/store_collection_* naming, one per zone name) for the 7 zones in the
unified files, and fixed the same bug inline in the 2 override files (List, TabInfo) that
Category's own shipped code (PR grandnode#792, develop@c1a7ead84) left unfixed - verified by reading
Category's merged Store-side List.cshtml/TabInfo.cshtml directly, not assumed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants