ARCH-001 Phase 15: Page controller/service consolidation (Admin/Store) - #809
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… Copy/List actions Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t.cshtml as host-override, extract widget-zone satellites
…ll in List() GET (Admin's store-filter dropdown was rendering against a null model) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Only minor test helper cleanup was noted; the core refactor appears consistent with existing patterns and is covered by added tests.
Pull request overview
Consolidates duplicated Admin/Store CMS Page CRUD logic by introducing a shared BasePageController (in Grand.Web.AdminShared) and routing host-specific data scoping through a new RoutedPageDataScope. Store retains the intentionally Store-only actions (Copy, StorePagesList, GlobalPagesList) and the per-host List.cshtml views remain divergent as described.
Changes:
- Introduced
BasePageController+RoutedPageDataScopeto centralize shared controller behavior and per-area scoping. - Reduced Admin/Store
PageControllerimplementations to thin subclasses; Store keeps Store-only actions and theEditWarningCheckoverride. - Moved/shared Razor views into
Grand.Web.AdminSharedand added per-host widget-zone partials; added/updated unit tests for the new shared surface.
File summaries
| File | Description |
|---|---|
| src/Web/Grand.Web.Store/Controllers/PageController.cs | Refactored Store controller into thin BasePageController subclass + Store-only actions. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/List.cshtml | Replaced direct widget component call with host partial indirection for list buttons. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.Tabs.cshtml | Store widget-zone satellite partial for tab injection. |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.Seo.Top.cshtml | Store widget-zone satellite partial (SEO top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.Seo.Bottom.cshtml | Store widget-zone satellite partial (SEO bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.ListButtons.cshtml | Store widget-zone satellite partial (list buttons). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.Info.Top.cshtml | Store widget-zone satellite partial (info top). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.Info.Bottom.cshtml | Store widget-zone satellite partial (info bottom). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.DetailsButtonsCreate.cshtml | Store widget-zone satellite partial (create-screen buttons). |
| src/Web/Grand.Web.Store/Areas/Store/Views/Page/Partials/WidgetZone.DetailsButtons.cshtml | Store widget-zone satellite partial (edit-screen buttons). |
| src/Web/Grand.Web.AdminShared/Controllers/BasePageController.cs | New shared controller implementing List/Create/Edit/Delete with scoped access checks. |
| src/Web/Grand.Web.AdminShared/Services/RoutedPageDataScope.cs | New per-area IAdminDataScope<Page> router for combined-host DI. |
| src/Web/Grand.Web.AdminShared/Startup/StartupApplication.cs | Registers global/store Page scopes and the routed scope implementation. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Page/Create.cshtml | Shared Create view updated to use current route area and per-host widget satellites. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Page/Edit.cshtml | Shared Edit view updated to be area-aware (Copy button Store-only) + widget satellites. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Page/Partials/CreateOrUpdate.cshtml | Shared edit form now conditionally shows Admin-only tabs/sections + widget satellites. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Page/Partials/CreateOrUpdate.TabInfo.cshtml | Shared “Info” tab updated for host-specific fields and widget satellites. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/Page/Partials/CreateOrUpdate.TabSeo.cshtml | Shared “SEO” tab updated to include widget satellites. |
| src/Web/Grand.Web.Admin/Controllers/PageController.cs | Refactored Admin controller into thin BasePageController subclass with host attributes. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/List.cshtml | Replaced direct widget component call with host partial indirection for list buttons. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.Tabs.cshtml | Admin widget-zone satellite partial for tab injection. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.Seo.Top.cshtml | Admin widget-zone satellite partial (SEO top). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.Seo.Bottom.cshtml | Admin widget-zone satellite partial (SEO bottom). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.ListButtons.cshtml | Admin widget-zone satellite partial (list buttons). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.Info.Top.cshtml | Admin widget-zone satellite partial (info top). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.Info.Bottom.cshtml | Admin widget-zone satellite partial (info bottom). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.DetailsButtonsCreate.cshtml | Admin widget-zone satellite partial (create-screen buttons). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/WidgetZone.DetailsButtons.cshtml | Admin widget-zone satellite partial (edit-screen buttons). |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Create.cshtml | Removed in favor of shared Create view in Grand.Web.AdminShared. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Edit.cshtml | Removed in favor of shared Edit view in Grand.Web.AdminShared. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/CreateOrUpdate.cshtml | Removed in favor of shared partial in Grand.Web.AdminShared. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/CreateOrUpdate.TabInfo.cshtml | Removed in favor of shared partial in Grand.Web.AdminShared. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/Page/Partials/CreateOrUpdate.TabSeo.cshtml | Removed in favor of shared partial in Grand.Web.AdminShared. |
| src/Tests/Grand.Web.Store.Tests/Controllers/PageControllerTests.cs | New tests covering Store-only actions and Store host attributes. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/BasePageControllerTests.cs | New characterization tests for shared controller behavior and scoping. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/PageControllerTests.cs | New tests ensuring Admin controller is a thin subclass with required host attributes. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/RoutedPageDataScopeTests.cs | New tests covering area-based scope resolution and fail-closed behavior. |
Review details
Suppressed comments (4)
src/Tests/Grand.Web.Admin.Tests/Controllers/RoutedPageDataScopeTests.cs:43
- If Build(...) is simplified to not take unused out parameters, this call should be updated accordingly.
var routed = Build("Admin", out _, out _);
Assert.IsNull(routed.DefaultStoreId);
src/Tests/Grand.Web.Admin.Tests/Controllers/RoutedPageDataScopeTests.cs:58
- If Build(...) is simplified to not take unused out parameters, this call should be updated accordingly.
var routed = Build("Vendor", out _, out _);
Assert.ThrowsExactly<InvalidOperationException>(() => _ = routed.DefaultStoreId);
}
src/Tests/Grand.Web.Admin.Tests/Controllers/RoutedPageDataScopeTests.cs:65
- If Build(...) is simplified to not take unused out parameters, this call should be updated accordingly.
var routed = Build(null, out _, out _);
Assert.ThrowsExactly<InvalidOperationException>(() => _ = routed.DefaultStoreId);
}
src/Tests/Grand.Web.Admin.Tests/Controllers/RoutedPageDataScopeTests.cs:51
- If Build(...) is simplified to not take unused out parameters, this call should be updated accordingly.
var routed = Build("Store", out _, out _);
Assert.AreEqual("store-1", routed.DefaultStoreId);
}
- Files reviewed: 37/37 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.
Comment on lines
+14
to
+28
| private static RoutedPageDataScope Build(string area, out Mock<IAdminDataScope<Page>> globalMock, out Mock<IAdminDataScope<Page>> storeMock) | ||
| { | ||
| var httpContext = new DefaultHttpContext(); | ||
| if (area != null) | ||
| httpContext.Request.RouteValues = new RouteValueDictionary { ["area"] = area }; | ||
|
|
||
| var httpContextAccessorMock = new Mock<IHttpContextAccessor>(); | ||
| httpContextAccessorMock.Setup(a => a.HttpContext).Returns(httpContext); | ||
|
|
||
| var global = new GlobalAdminDataScope<Page>(); | ||
| var store = new StoreAdminDataScope<Page>(BuildContextAccessor()); | ||
| globalMock = null; | ||
| storeMock = null; | ||
| return new RoutedPageDataScope(httpContextAccessorMock.Object, global, store); | ||
| } |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| var view = result as ViewResult; | ||
| Assert.IsNotNull(view); | ||
| Assert.IsTrue(((PageModel)view.Model).Published); |
|
|
||
| var view = result as ViewResult; | ||
| Assert.IsNotNull(view); | ||
| Assert.IsFalse(((PageModel)view.Model).Published); |
|
|
||
| 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.IsTrue(((PageModel)view.Model).ShowCopyButton); |
| var result = await _controller.Edit("p1"); | ||
|
|
||
| var view = result as ViewResult; | ||
| Assert.IsTrue(((PageModel)view.Model).ShowCopyButton); |
| var result = await _controller.Edit("p1"); | ||
|
|
||
| var view = result as ViewResult; | ||
| Assert.IsFalse(((PageModel)view.Model).ShowCopyButton); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("Edit", redirect.ActionName); |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("List", redirect.ActionName); |
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
…ge-consolidation # Conflicts: # src/Web/Grand.Web.AdminShared/Startup/StartupApplication.cs Co-authored-by: KrzysztofPajak <16772986+KrzysztofPajak@users.noreply.github.com>
Contributor
Resolved. I merged |
|
|
||
| var redirect = result as RedirectToActionResult; | ||
| Assert.IsNotNull(redirect); | ||
| Assert.AreEqual("List", redirect.ActionName); |
…er.EditWarningCheck CodeQL flagged the inner 'page.LimitedToStores &&' in the OR's second branch as always true, since it's only evaluated when the first branch's !page.LimitedToStores was already false. Pre-existing legacy idiom copied from Category/Blog's EditWarningCheck; removing the dead re-check does not change behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRztBQVfXimBPMFAioLabo
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.
Resolves ARCH-001 Phase 15
Type: refactor
Issue
Grand.Web.AdminandGrand.Web.Storeeach carried their ownPageController(200 / 281 lines, noVendor equivalent) duplicating CRUD and access-control logic for
Page(GrandNode's CMS page entity)instead of sharing it through
Grand.Web.AdminShared. Same class of drag and security-gap riskdocumented for every prior ARCH-001 phase.
Solution
Consolidated both hosts behind one shared
BasePageControllerinGrand.Web.AdminShared, driven by anew
RoutedPageDataScope(IAdminDataScope<Page>). No Vendor work needed —Grand.Web.Vendorhas noPageControllerat all.This phase is structurally different from every prior one: Store keeps three genuinely Store-only
actions —
Copy(forks a shared/global page into store-exclusive ownership — a new action class forthis initiative),
StorePagesList, andGlobalPagesList(backing Store's own 2-tab list UI). These areNOT shared into the base — Admin is already global, so "copy into my store" and the two-tab split are
Store-specific workflow decisions, not security-scope differences.
List.cshtmlalso stays a genuineper-host override view on both hosts — the most divergent view file this initiative has hit (Admin: a
single Kendo grid with a store-filter dropdown; Store: a 2-tab
admin-tabstripwith twoindependently-sourced grids).
BasePageControllerholds the shared surface:List,Create,Edit,Delete.PageControllers are thin subclasses supplying DI wiring and host attributes. Store alsokeeps
EditWarningCheck(the same proven re-derived condition already used by Category/Blog) plus thethree actions above.
Grand.Web.AdminShared/Views/AdminShared/Page/...;List.cshtmlkeptper-host. 7 widget zones extracted into real per-host satellite partials (Store's
<vc:admin-widget>calls were dead literal HTML before this change, the same bug class found and fixed in every prior
phase —
page_list_buttons' Store-side satellite is a genuinely new call, since Store'sList.cshtmlnever had a dead one to convert).
Full design rationale:
docs/superpowers/specs/2026-09-03-arch001-page-consolidation-design.md(spec)and
docs/superpowers/plans/2026-09-03-arch001-page-consolidation.md(plan) — both kept on disk only(gitignored), summarized here.
Disclosed behavior changes (intentional, not regressions)
Urlnow usesRequest.Schemefor both hosts, instead of Admin's original hardcoded"http"literal — a strict, low-risk improvement (Store's original already did this).Edit.cshtmlis now gated on!string.IsNullOrEmpty(Model.SeName)for bothhosts — Admin's original rendered it unconditionally, which would have produced a broken link for an
empty
SeName. Store's original already had this guard.SeName, notGetSeName(workingLanguageId)— both originals resolved the localized slug for the current workinglanguage; the shared implementation resolves the default one. For a page with a localized SeName,
the Edit-screen preview link now points at the default-language slug instead. Both slugs resolve
correctly through the slug table, so this is cosmetic-at-worst, not a broken link — flagged because it
wasn't an originally-planned change; the simplification avoids adding an
IContextAccessordependencyback into the shared base.
Deletelost an inertModelState.IsValid/Error(ModelState)wrapper — the action bindsonly a
string id, soModelStatewas always valid in practice; this unifies onto Store's simpleroriginal shape with no observable behavior change.
could match Admin's existing button ordering — purely cosmetic.
Final whole-branch review
Dispatched on the most capable model per this initiative's process. Verdict: "Ready to merge: With
fixes" — 1 Critical, 3 Important (2 of which are the disclosed-behavior-change items above, not code
defects), fixed in one round:
List()GET actions were"confirmed identical" bare
View()calls with no model. That was wrong — Admin's original calledPreparePageListModel()to populate its store-filter dropdown; the shared implementation dropped thatcall, which would have thrown a
NullReferenceExceptionrendering/Admin/Page/List(Admin's keptList.cshtmldereferencesModel.AvailableStores). This slipped through every per-task review becauseit lived in the seam between the controller task and the views task — exactly the kind of defect only a
whole-branch review or a live render check can catch. Fixed:
List()GET now callsPreparePageListModel()and passes the model for both hosts (Store's view never dereferences it, sothis is harmless there). Live-verified after the fix (see Testing below).
Scoped re-review confirmed the fix ADDRESSED, no new breakage.
Breaking changes
None for storefront/API consumers. See "Disclosed behavior changes" above for the small set of
admin/store-panel behavior differences this consolidation surfaced or corrected.
Testing
Automated (all green):
dotnet build GrandNode.sln— 0 errors.dotnet test src/Tests/Grand.Web.Admin.Tests— full suite green (1024+ tests).dotnet test src/Tests/Grand.Web.Store.Tests— full suite green (70+ tests, incl.Copy's 3 accessbranches and
StorePagesList/GlobalPagesListfiltering).dotnet test src/Tests/Grand.Mapping.Tests— 234/234.Live smoke test (run directly against a real dev DB, not sandboxed — specifically targeting the page that
had the Critical regression):
/Admin/Page/Listrenders correctly with the store-filter dropdown populated (theexact screen the Critical finding broke) — direct proof the fix works, not just that it compiles.
Created a page limited exclusively to a second store ("Store2"); confirmed
Createrenders correctlyincluding the Admin-only User Fields tab and CustomerGroups/Stores fields.
Edit(GET) on the Store2-only page denied (redirect toList); crafted antiforgery-tokenedCopyandDeletePOSTs both denied (redirect toList), zeromutation (verified via a pre/post Admin re-read); confirmed
StorePagesListreturns 0 andGlobalPagesListreturns all 15 real seeded global pages, with the Store2-only page correctly absentfrom both.
Editsucceeded with correct field visibility(no User Fields tab, no CustomerGroups/Stores fields);
Copy— forked a real seeded global page(
ContactUs) into a store2-exclusive copy, title/body preserved,SeNamecorrectly de-duplicated(
contactus-1), new copy's ownShowCopyButtoncorrectly computedfalse(single-store-exclusive).List.cshtmlrendered correctly on both tabs, backed by liveStorePagesList/GlobalPagesListdata.[Area]/[Authorize*]omission that has hit roughly half ofthis initiative's phases did not recur here (caught clean on first attempt).
ContactUscopy) deleted afterward; DB confirmed restoredto its original 15-page state.
🤖 Generated with Claude Code