ARCH-001: consolidate Admin/Store CollectionController into Grand.Web.AdminShared - #794
Merged
Merged
Conversation
… region Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
Verified Store's original controller had no Export/Import actions (grep, zero hits) - pure addition for Store, same superset approach ARCH-001 Phase 1/3 used for Product/Category. Export is now store-scoped for Store via scope.DefaultStoreId, unlike Admin's original always-global storeId: ''. Permission-provider check: PermissionProvider.cs grants whole StandardPermission.ManageCollections to StoreManager (no PermissionActionName.Export/Import references exist in this file at all - grants are permission-level, not per-action) - so Store gaining Export/Import actions is the same superset exposure Category's Task 5 already accepted for the sibling entity; no per-action exclusion found, proceeding as planned. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
Normalizes ProductList's storeId argument to scope.DefaultStoreId (was StoreContext.CurrentStore.Id in Store's original code, inconsistent with every other call site in the file) - user-approved fix, not pure behavior preservation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ruWBUZPv3BnpPhjQVV8Xf
…seCollectionController subclasses
…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 #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
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This was referenced Aug 25, 2026
Merged
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
Grand.Web.AdminandGrand.Web.Storeeach shipped their ownCollectionController(389 / 404lines), duplicating collection CRUD, picture handling, export/import, and collection-product
association screens plus every access check twice over — the same class of duplication ARCH-001
already fixed for Product (PR #790) and Category (PR #792). Collection has no Vendor-panel screen
at all (confirmed empty
Grand.Web.Vendorequivalent), so this is a 2-host consolidation likeCategory, not 3.
Full design:
docs/superpowers/specs/2026-08-24-arch001-collection-consolidation-design.mdFull plan:
docs/superpowers/plans/2026-08-24-arch001-collection-consolidation.md(Both gitignored under
docs/superpowers/, same as Product's and Category's — on disk in thisbranch'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) and Phase 3(Category) — no new interface members, no new bespoke scope classes:
RoutedCollectionDataScope(2-branch: Admin/Store, fails closed on anything else) resolvesthe existing generic
GlobalAdminDataScope<Collection>/StoreAdminDataScope<Collection>perrequest area. Registered centrally in
Grand.Web.AdminShared/Startup/StartupApplication.cs.BaseCollectionControllerinGrand.Web.AdminShared(414 lines) holds all 5 regions (List,Create/Edit/Delete, Picture, Export/Import, Products) migrated from both original controllers.
ICollectionViewModelService/CollectionViewModelServiceneeded no changes.CollectionControllers reduced to thinBaseCollectionControllersubclasses (Admin35 lines, Store 48, vs. ~390-400 each before).
Grand.Web.AdminShared/Views/AdminShared/Collection/; 3 keptas genuine host-specific overrides (
List,ProductAddPopup,CreateOrUpdate.TabInfo— eachhas a real Admin-only field/panel Store doesn't). Fixed Collection's own instance of the
vc:admin-widgetdead-tag-helper bug in Store's 2 override files still calling the Admin-onlytag helper (Store's
_ViewImports.cshtmlnever registers it) — unlike Category, which left thisgap unfixed in its own equivalent override files (a known, separately-flagged issue on
Category). Extracted 18 new per-host
WidgetZone.*.cshtmlsatellites (9 zones × 2 hosts,vc:store-widget/store_collection_*naming), one satellite per zone name (not per call site),following the proven Category/Product pattern.
CollectionController/CollectionViewModelServicetests existed before this PR) —BaseCollectionControllerTestscovers scope-based access on every action for both hosts.MongoDB (built from this worktree, installed fresh, two real stores + two real store-manager
customers created via genuine Admin-UI form POSTs): a collection exclusively owned by "Store
Two", 11 action/method pairs (Edit GET/POST, Delete, PicturePopup GET/POST, ProductList,
ProductUpdate, ProductDelete, ProductAddPopup GET/POST, ProductAddPopupList) attempted as a
different store's user with a valid antiforgery token — denied on every applicable action;
positive control as the owning store's own user succeeded on all of them (including adding a
real product to the collection); post-hoc direct-DB read confirmed zero mutation slipped through
and
UpdatedOnUtconly ever moved on the legitimate edit.Deliberate, disclosed behavior differences from the two original hosts (both minor,
non-security, same shape as Category's disclosed differences):
ExportXlsx/ImportFromXlsxare now reachable from Store (previously Admin-only) — samesuperset approach Product's and Category's phases used; Store's permission seed already grants
ManageCollectionswith no per-action deny-list.Not changed by this PR:
ProductAddPopupGET's missing collection-ownership check andProductAddPopupList's caller-store-only filtering (verified present in both originalcontrollers — faithfully preserved, not introduced here, and neither discloses data the caller
couldn't already see); a pre-existing, unrelated
UserFields.cshtmlNullReferenceException foundduring live testing (
Grand.Web.Store/Areas/Store/Views/Shared/Partials/UserFields.cshtml:9,WorkContextSetter.GetStoreManagerkeys offCustomer.StoreIdinstead ofStaffStoreId— filenever touched by this branch, and the identical bug already exists on Category's merged code).
Both flagged as follow-up candidates, not blockers.
Breaking changes
None.
ICollectionViewModelService's public contract is unchanged. This is a purecode-organization refactor of two hosts' controllers/views into one shared base, plus the one
disclosed Store-side widget-zone bugfix and the one disclosed minor behavior normalization above.
Testing
dotnet build GrandNode.sln— 0 errors, 0 warnings.dotnet test src/Tests/Grand.Web.Admin.Tests(unfiltered) — 482 passed, 0 failed.dotnet test src/Tests/Grand.Web.Store.Tests(unfiltered) — 33 passed, 0 failed.dotnet test src/Tests/Grand.Mapping.Tests(unfiltered) — 234 passed, 0 failed.applicable denials correct, positive control passed, no unauthorized mutation.
superpowers:subagent-driven-development: 10 plan tasks, task-by-task reviewledger available on request if useful for review context.
🤖 Generated with Claude Code
Final whole-branch review notes (added post-review, before merge)
ImportFromXlsxon bothBaseCollectionControllerand the already-mergedBaseCategoryControllerresolves the importedentity by id and updates it regardless of the calling store's ownership, and never forces
LimitedToStores/Storeson newly-imported rows.PermissionService.AuthorizeActionisdeny-list, and
StoreManageralready holdsManageCollections/ManageCategories, so this is areal, POST-reachable (antiforgery-gated, no UI entry point on Store) cross-tenant write path on
both entities today. Tracked as a follow-up to fix Category and Collection together, not a block
on this branch.
Create.cshtml'sWidgetZone.DetailsButtonspartial call now implicitly passes the page'sCollectionModelinstead of the original's explicitadditional-data="null"(a partial withoutmodel=inherits the parent view's model). Same shape as Category's already-merged equivalent;a third-party widget zone consumer that null-checks
additional-dataon the create screen wouldnow see an empty model instead of null.
Edit(GET)no longer conditions the ownership warning/check onIGroupService.IsStoreManagerthe way the original did — a non-store-manager Store-area user(empty
StaffStoreId) is now denied a store-limited collection instead of seeing it unfiltered.Matches Category's already-merged behavior exactly; flagged as an undocumented-until-now
behavior change, safer direction, not a regression.
Partials/CreateOrUpdate.cshtmlandPartials/CreateOrUpdate.TabSeo.cshtmlas renamed from Store's original rather than Admin's(content was byte-identical either way) — cosmetic, noted so git-blame provenance doesn't
confuse a future reader.