diff --git a/src/Tests/Grand.Web.Admin.Tests/Controllers/PaymentControllerTests.cs b/src/Tests/Grand.Web.Admin.Tests/Controllers/PaymentControllerTests.cs index 172c505fd8..6706d2872e 100644 --- a/src/Tests/Grand.Web.Admin.Tests/Controllers/PaymentControllerTests.cs +++ b/src/Tests/Grand.Web.Admin.Tests/Controllers/PaymentControllerTests.cs @@ -3,16 +3,15 @@ using Grand.Business.Core.Interfaces.Common.Configuration; using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; -using Grand.Business.Core.Interfaces.Common.Stores; using Grand.Domain.Directory; using Grand.Domain.Shipping; -using Grand.Domain.Stores; using Grand.Infrastructure; using Grand.Infrastructure.Mapper; using Grand.Mapping; using Grand.Web.Admin.Controllers; using Grand.Web.AdminShared.Mapper; using Grand.Web.AdminShared.Models.Payments; +using Grand.Web.Common.Helpers; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ViewFeatures; @@ -64,15 +63,14 @@ public void Setup() new Mock().Object, contextAccessorMock.Object); - // GetActiveStore resolves its services from HttpContext.RequestServices; - // a single store short-circuits the group/user-field lookups - var storeServiceMock = new Mock(); - storeServiceMock.Setup(s => s.GetAllStores()) - .ReturnsAsync(new List { new() { Id = StoreId } }); + // GetActiveStore resolves IAdminStoreService from HttpContext.RequestServices + // (ARCH-001 GetActiveStore() consolidation - BaseAdminController delegates to it instead of + // duplicating the store-scope lookup itself); a single store short-circuits to its own id. + var adminStoreServiceMock = new Mock(); + adminStoreServiceMock.Setup(s => s.GetActiveStore()).ReturnsAsync(StoreId); var requestServicesMock = new Mock(); - requestServicesMock.Setup(sp => sp.GetService(typeof(IStoreService))).Returns(storeServiceMock.Object); - requestServicesMock.Setup(sp => sp.GetService(typeof(IContextAccessor))) - .Returns(contextAccessorMock.Object); + requestServicesMock.Setup(sp => sp.GetService(typeof(IAdminStoreService))) + .Returns(adminStoreServiceMock.Object); requestServicesMock.Setup(sp => sp.GetService(typeof(IGroupService))) .Returns(new Mock().Object); diff --git a/src/Tests/Grand.Web.Common.Tests/Services/Admin/AdminStoreServiceTests.cs b/src/Tests/Grand.Web.Common.Tests/Services/Admin/AdminStoreServiceTests.cs index 668baff930..6134bad348 100644 --- a/src/Tests/Grand.Web.Common.Tests/Services/Admin/AdminStoreServiceTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Services/Admin/AdminStoreServiceTests.cs @@ -83,4 +83,40 @@ public async Task GetActiveStore_ShouldReturnEmptyString_WhenStoreFromContextDoe // Assert Assert.AreEqual(string.Empty, result); } + + [TestMethod] + public async Task GetActiveStore_ShouldReturnNull_WhenNoStoresExist() + { + // Arrange — ARCH-001 GetActiveStore() consolidation: the original implementation dereferenced + // FirstOrDefault() with the null-forgiving operator here, throwing a NullReferenceException on a + // zero-store install. Must degrade gracefully instead, matching BaseAdminController's original + // (already-safe) behavior. + _storeServiceMock.Setup(s => s.GetAllStores()).ReturnsAsync(new List()); + + // Act + var result = await _adminStoreService.GetActiveStore(); + + // Assert + Assert.IsNull(result); + } + + [TestMethod] + public async Task GetActiveStore_ShouldReturnEmptyString_WithoutLookup_WhenContextStoreIdIsEmpty() + { + // Arrange — ARCH-001 GetActiveStore() consolidation: an empty scope means "all stores"; must + // short-circuit before calling GetStoreById("") rather than relying on it happening to return + // null for an empty id. + var stores = new List { new Store { Id = "store1" }, new Store { Id = "store2" } }; + _storeServiceMock.Setup(s => s.GetAllStores()).ReturnsAsync(stores); + + var customer = new Customer { CustomerGuid = Guid.NewGuid() }; + _contextAccessorMock.Setup(c => c.WorkContext.CurrentCustomer).Returns(customer); + + // Act + var result = await _adminStoreService.GetActiveStore(); + + // Assert + Assert.AreEqual(string.Empty, result); + _storeServiceMock.Verify(s => s.GetStoreById(It.IsAny()), Times.Never); + } } \ No newline at end of file diff --git a/src/Web/Grand.Web.Admin/Controllers/BaseAdminController.cs b/src/Web/Grand.Web.Admin/Controllers/BaseAdminController.cs index 786c7e4289..dd85391300 100644 --- a/src/Web/Grand.Web.Admin/Controllers/BaseAdminController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/BaseAdminController.cs @@ -1,10 +1,7 @@ -using Grand.Business.Core.Interfaces.Common.Stores; -using Grand.Domain.Common; -using Grand.Domain.Customers; -using Grand.Infrastructure; -using Grand.Web.Admin.Extensions; +using Grand.Web.Admin.Extensions; using Grand.Web.Common.Controllers; using Grand.Web.Common.Filters; +using Grand.Web.Common.Helpers; using Microsoft.AspNetCore.Mvc; namespace Grand.Web.Admin.Controllers; @@ -19,25 +16,13 @@ public abstract class BaseAdminController : BaseController /// Get active store scope (for multi-store configuration mode) /// /// Store ID; 0 if we are in a shared mode - protected async Task GetActiveStore() + /// + /// ARCH-001 GetActiveStore() consolidation: delegates to the shared IAdminStoreService (also used by + /// BaseTaxCategoryController and StoreScope) instead of duplicating its logic. Resolved via the + /// service locator, not constructor injection, to avoid touching every Admin controller's constructor. + /// + protected Task GetActiveStore() { - var storeService = HttpContext.RequestServices.GetRequiredService(); - var workContext = HttpContext.RequestServices.GetRequiredService().WorkContext; - - var stores = await storeService.GetAllStores(); - if (stores.Count < 2) - return stores.FirstOrDefault()?.Id; - - var storeId = - workContext.CurrentCustomer.GetUserFieldFromEntity(SystemCustomerFieldNames - .AdminAreaStoreScopeConfiguration); - //empty scope means "all stores" - settings are loaded from/saved to the global scope - if (string.IsNullOrEmpty(storeId)) - { - return ""; - } - - var store = await storeService.GetStoreById(storeId); - return store != null ? store.Id : ""; + return HttpContext.RequestServices.GetRequiredService().GetActiveStore(); } } \ No newline at end of file diff --git a/src/Web/Grand.Web.Admin/Controllers/TaxController.cs b/src/Web/Grand.Web.Admin/Controllers/TaxController.cs index 05aa760a19..dae826550d 100644 --- a/src/Web/Grand.Web.Admin/Controllers/TaxController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/TaxController.cs @@ -3,11 +3,8 @@ using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Common.Localization; using Grand.Business.Core.Interfaces.Common.Stores; -using Grand.Domain.Common; -using Grand.Domain.Customers; using Grand.Domain.Directory; using Grand.Domain.Tax; -using Grand.Infrastructure; using Grand.Infrastructure.Caching; using Grand.Infrastructure.Plugins; using Grand.Web.Admin.Extensions; @@ -20,6 +17,7 @@ using Grand.Web.Common.DataSource; using Grand.Web.Common.Extensions; using Grand.Web.Common.Filters; +using Grand.Web.Common.Helpers; using Grand.Web.Common.Localization; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Rendering; @@ -43,34 +41,14 @@ public class TaxController( ICountryService countryService, IStoreService storeService, IEnumTranslationService enumTranslationService, - IContextAccessor contextAccessor, + IAdminStoreService adminStoreService, IAdminDataScope scope) : BaseTaxCategoryController(taxCategoryService, storeService, translationService, scope) { - /// - /// Get active store scope (for multi-store configuration mode) - /// - /// Store ID; 0 if we are in a shared mode - private async Task GetActiveStore() - { - var workContext = contextAccessor.WorkContext; - - var stores = await storeService.GetAllStores(); - if (stores.Count < 2) - return stores.FirstOrDefault()?.Id; - - var storeId = - workContext.CurrentCustomer.GetUserFieldFromEntity(SystemCustomerFieldNames - .AdminAreaStoreScopeConfiguration); - //empty scope means "all stores" - settings are loaded from/saved to the global scope - if (string.IsNullOrEmpty(storeId)) - { - return ""; - } - - var store = await storeService.GetStoreById(storeId); - return store != null ? store.Id : ""; - } + // ARCH-001 GetActiveStore() consolidation: was a hand-duplicated private copy of the same logic + // now in IAdminStoreService (also used by BaseAdminController and StoreScope) - BaseTaxCategoryController + // can't carry it itself, it must stay host-agnostic (Admin/Store/Vendor all extend it). + private Task GetActiveStore() => adminStoreService.GetActiveStore(); #region Tax Providers diff --git a/src/Web/Grand.Web.Common/Helpers/AdminStoreService.cs b/src/Web/Grand.Web.Common/Helpers/AdminStoreService.cs index 1e71c7dd50..eccf0beb4e 100644 --- a/src/Web/Grand.Web.Common/Helpers/AdminStoreService.cs +++ b/src/Web/Grand.Web.Common/Helpers/AdminStoreService.cs @@ -25,9 +25,13 @@ public async Task GetActiveStore() { var stores = await _storeService.GetAllStores(); if (stores.Count < 2) - return stores.FirstOrDefault()!.Id; + return stores.FirstOrDefault()?.Id; var storeId = _contextAccessor.WorkContext.CurrentCustomer.GetUserFieldFromEntity(SystemCustomerFieldNames.AdminAreaStoreScopeConfiguration); + //empty scope means "all stores" - settings are loaded from/saved to the global scope + if (string.IsNullOrEmpty(storeId)) + return ""; + var store = await _storeService.GetStoreById(storeId); return store != null ? store.Id : "";