Make the store scope explicit on the category, brand and collection lists

[KrzysztofPajak]

Type: bugfix

Issue

GetAllCategories, GetAllBrands and GetAllCollections all declared string storeId = "", and AclService.Authorize returns true when the store is empty:

if (string.IsNullOrEmpty(storeId))
    //return true if no store specified/found
    return true;

So omitting the argument was not a compile error and silently turned tenant filtering off. That is not hypothetical — it had already happened:

The storefront search page listed collections from every store. GetSearchHandler builds the search filter dropdowns; the category block passes request.Store.Id, and ten lines below it the collection block did not:

var allCategories = await _categoryService.GetAllCategories(storeId: request.Store.Id, pageSize: 100);
...
var collections = await _collectionService.GetAllCollections(pageSize: 100);   // no store

To reproduce: run two stores, give store B a collection limited to B, and open the search page of store A — B's collection appears in the "collection" filter.

Solution

storeId cannot simply move to the front of the parameter list. All three methods take string parameters before it (parentId, categoryName, brandName, collectionName), so reordering would still compile at positional call sites while quietly swapping the arguments' meaning — the very class of bug this PR is about.

Instead, every parameter up to and including storeId became required, with the order untouched. The compiler then named all 27 call sites, and no call could change meaning without failing to build.

Twenty-six of those already passed the right scope and only became explicit — admin panel calls that are deliberately global now say storeId: "" in as many words. The twenty-seventh is the defect above and now passes the request's store.

AclServiceTest and the new AclMappingExtensionTests pin the fail-open, so changing it later has to be deliberate. The latter also records a difference nobody had written down: AclService.Authorize admits an entity shared with a second store, while AclMappingExtension.AccessToEntityByStore refuses it (Stores.Count == 1).

This is the first instalment of item 2.3 in the architecture audit. Roughly 150 further string storeId = "" declarations remain in the business layer, the CMS services (BlogService, NewsService, PageService) being the natural next batch.

Breaking changes

Yes, for third-party plugins. ICategoryService.GetAllCategories, IBrandService.GetAllBrands and ICollectionService.GetAllCollections no longer default their leading parameters. Plugin code calling them without those arguments will not compile until it passes them; parameter order and types are unchanged, so no call can silently change meaning. This is the point of the change — the scope has to be a decision, not an omission.

Storefront behaviour changes in exactly one place: the search page's collection filter now honours the current store, as the rest of the page already did.

Testing

1. dotnet build ./GrandNode.sln — clean, 0 warnings.
2. dotnet test on the affected projects: Grand.Business.Catalog.Tests (349), Grand.Business.Common.Tests (127), Grand.Web.Admin.Tests (55), Grand.Web.Store.Tests (17), Grand.Web.Tests (9), Grand.Business.Messages.Tests (33) — all pass.
3. Set up two stores. In the admin panel create a collection and limit it to store B only.
4. Open the search page of store A (/search) and expand the collection filter: store B's collection must not appear. Before this change it did.
5. Open the search page of store B: its collection appears.
6. Regression check on the calls that stayed global — admin panel Catalog → Categories / Brands / Collections lists, their export buttons, the admin search box, and the discount "add category/brand/collection" pickers all behave as before.

🤖 Generated with Claude Code

[Copilot]

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