ARCH-001 Phase 18: ProductReview controller/service consolidation (Admin/Store) - #813
Merged
KrzysztofPajak merged 6 commits intoSep 4, 2026
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ttribute regression tests Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…iews to AdminShared, keep List.cshtml as host-override Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…or ProductReview (fixes dead <vc:admin-widget> on Store) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t_buttons zone, CanFeatureOnHomepage comment Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The consolidation preserves the original action surface, adds explicit per-area scoping to maintain tenant isolation, and includes targeted regression/attribute tests for the new shared controller and scopes.
Pull request overview
This PR continues the ARCH-001 consolidation series by removing duplicated Admin/Store ProductReviewController implementations and centralizing the shared action surface in Grand.Web.AdminShared, while keeping only the genuinely host-specific controller attributes and view overrides in each host.
Changes:
- Introduced
BaseProductReviewController(AdminShared) to unify List/Edit/Delete/Approve/Disapprove/ProductSearchAutoComplete behavior across Admin + Store. - Added
IAdminDataScope<ProductReview>implementations (StoreProductReviewDataScope,RoutedProductReviewDataScope) and registered them in AdminShared startup to preserve correct per-area scoping in the combined host. - Moved shared views to
Grand.Web.AdminSharedand replaced dead/incompatible widget invocations on Store by routing widget zones through per-host partials (Admin:admin-widget, Store:store-widget+store_zone names).
File summaries
| File | Description |
|---|---|
| src/Web/Grand.Web.Store/Controllers/ProductReviewController.cs | Reduced to thin Store subclass with Store-specific attributes and DI wiring. |
| src/Web/Grand.Web.Admin/Controllers/ProductReviewController.cs | Reduced to thin Admin subclass with Admin-specific attributes and DI wiring. |
| src/Web/Grand.Web.AdminShared/Controllers/BaseProductReviewController.cs | New consolidated controller implementation for all shared actions and access gates. |
| src/Web/Grand.Web.AdminShared/Services/StoreProductReviewDataScope.cs | Store-specific access scope for ProductReview (strict store ownership). |
| src/Web/Grand.Web.AdminShared/Services/RoutedProductReviewDataScope.cs | Area-based resolver to select correct scope implementation per request. |
| src/Web/Grand.Web.AdminShared/Startup/StartupApplication.cs | DI registrations for ProductReview scopes (Global + Store + Routed). |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/ProductReview/Edit.cshtml | Shared Edit view updated to be area-agnostic and route widget zones via partials. |
| src/Web/Grand.Web.AdminShared/Views/AdminShared/ProductReview/Partials/CreateOrUpdate.cshtml | Shared CreateOrUpdate partial now delegates widget zones to per-host partials. |
| src/Web/Grand.Web.Store/Areas/Store/Views/ProductReview/List.cshtml | Store List override keeps host-specific UI; fixes widget zone to store-widget. |
| src/Web/Grand.Web.Store/Areas/Store/Views/ProductReview/Partials/WidgetZone.*.cshtml | New Store widget-zone partials using store-widget and store_ zone names. |
| src/Web/Grand.Web.Admin/Areas/Admin/Views/ProductReview/Partials/WidgetZone.*.cshtml | New Admin widget-zone partials using admin-widget and existing zone names. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/BaseProductReviewControllerTests.cs | New behavioral tests covering shared controller flows + scoping behavior. |
| src/Tests/Grand.Web.Admin.Tests/Controllers/ProductReviewTests.cs | New scope + routing + attribute tests for Admin host. |
| src/Tests/Grand.Web.Store.Tests/Controllers/ProductReviewControllerAttributeTests.cs | New attribute tests for Store thin subclass correctness. |
Review details
- Files reviewed: 22/22 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 result = await CreateController().List() as ViewResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreSame(listModel, result.Model); |
| var result = await CreateController().List() as ViewResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreSame(listModel, result.Model); |
| var result = await CreateController().Edit("missing") as RedirectToActionResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreEqual("List", result.ActionName); |
| var result = await CreateController().Edit("pr-1") as RedirectToActionResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreEqual("List", result.ActionName); |
| var result = await CreateController().Edit("pr-1") as ViewResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.IsInstanceOfType(result.Model, typeof(ProductReviewModel)); |
| var result = await CreateController().Edit(new ProductReviewModel { Id = "pr-1" }, false) as RedirectToActionResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreEqual("List", result.ActionName); |
| var result = await CreateController().Edit(new ProductReviewModel { Id = "pr-1" }, false) as RedirectToActionResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreEqual("List", result.ActionName); |
| var result = await CreateController().Edit(new ProductReviewModel { Id = "pr-1" }, true) as RedirectToActionResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreEqual("Edit", result.ActionName); |
| var result = await controller.Edit(model, false) as ViewResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreSame(model, result.Model); |
| var result = await CreateController().Delete("missing") as RedirectToActionResult; | ||
|
|
||
| Assert.IsNotNull(result); | ||
| Assert.AreEqual("List", result.ActionName); |
KrzysztofPajak
deleted the
arch001/phase18-productreview-consolidation
branch
September 4, 2026 16:09
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.Admin/Controllers/ProductReviewController.cs(171 lines) andGrand.Web.Store/Controllers/ProductReviewController.cs(186 lines) duplicated the same List/Edit/Delete/ApproveSelected/DisapproveSelected/ProductSearchAutoComplete action surface — part of ARCH-001 (architecture/maintainability), continuing the consolidation pattern shipped in PRs #790-#812.IProductReviewViewModelService/IProductReviewServicewere already fully shared; only the controller shell was duplicated. No VendorProductReviewControllerexists.Solution
Consolidated behind
BaseProductReviewControllerinGrand.Web.AdminShared, using this initiative's establishedIAdminDataScope<TEntity>strategy pattern:ProductReview : BaseEntityhas a flatStoreId(notIStoreLinkEntity), same shape family asOrder/GiftVoucher— required a bespokeStoreProductReviewDataScope/RoutedProductReviewDataScoperather than the genericStoreAdminDataScope<TEntity>.GiftVoucher,ProductReviewhas no "global" concept — every review belongs to exactly one store, soHasAccessandCanVieware identical throughout (no loose/strict split).ProductReviewControllerreduced to thinBaseProductReviewControllersubclasses (each restating[Area]/[Authorize*]/[AuthorizeMenu]/[AutoValidateAntiforgeryToken], since the shared base can't inherit either host's own base controller).CreateOrUpdate.cshtml(byte-identical between hosts) andEdit.cshtml(differed only by a hardcoded area string) moved toGrand.Web.AdminShared/Views/AdminShared/ProductReview/.List.cshtmlkept as a genuine per-host override (Admin has an extra store-filter dropdown; both hardcode area strings in inline JSUrl.Actioncalls).Real bug found and fixed during Task 4's review: the moved views carried forward 3
<vc:admin-widget>calls that were already dead literal HTML on Store (Grand.Web.Storedoesn't referenceGrand.Web.Admin, home ofAdminWidgetViewComponent) — a pre-existing bug, same class already fixed in this initiative's Category/Blog/News/Page phases. Fixed by extractingWidgetZone.*.cshtmlsatellite partials: the default (<vc:admin-widget>) lives inGrand.Web.Admin's own view tree (notAdminShared, which has no project reference reachingAdminWidgetViewComponent), and a new Store override uses<vc:store-widget>withstore_-prefixed zone names. A follow-up fix during final review found and closed the same gap on the one keptList.cshtmloverride (store_product_review_list_buttons), which this branch had made inconsistent (3 of 4 zones fixed, 1 still dead) relative to itself.Breaking changes
None. Pure consolidation — no behavior change beyond the two disclosed pre-existing-bug fixes above (both were already dead/inert code on Store, now live extension points; no plugin currently targets these new
store_product_review_*zone names, so nothing changes visibly today).Testing
dotnet build GrandNode.sln— 0 errors, 0 warnings.Grand.Web.Admin.Tests1168/1168,Grand.Web.Store.Tests99/99,Grand.Mapping.Tests234/234./Admin/ProductReview/Listand/Store/ProductReview/Listboth render correctly against 31 real seeded reviews (the specific regression class this initiative's Page-15 phase hit: a sharedList(GET)silently losing one host's model-prep call)./Admin/ProductReview/Edit/{id}and/Store/ProductReview/Edit/{id}(own-store review) render correctly via the sharedAdminSharedviews + newWidgetZonepartials.store1@store.comattemptingGET /Store/ProductReview/Edit/{id}on it was denied (redirected toList), confirmed zero mutation. Synthetic data deleted afterward, DB confirmed restored.Final whole-branch review (opus): Ready to merge: Yes (0 Critical, 1 Important + 3 Minor found and adjudicated — the Important finding and one Minor were fixed in a follow-up commit, verified by a scoped re-review; the remaining 2 Minors were parked as pre-existing/out-of-scope, disclosed above).