Skip to content

Commit 21a48f5

Browse files
KrzysztofPajakKrzysztofPajak
authored
ARCH-001: consolidate GetActiveStore() into IAdminStoreService (#831)
1 parent e07e8f7 commit 21a48f5

5 files changed

Lines changed: 64 additions & 63 deletions

File tree

‎src/Tests/Grand.Web.Admin.Tests/Controllers/PaymentControllerTests.cs‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,15 @@
33
using Grand.Business.Core.Interfaces.Common.Configuration;
44
using Grand.Business.Core.Interfaces.Common.Directory;
55
using Grand.Business.Core.Interfaces.Common.Localization;
6-
using Grand.Business.Core.Interfaces.Common.Stores;
76
using Grand.Domain.Directory;
87
using Grand.Domain.Shipping;
9-
using Grand.Domain.Stores;
108
using Grand.Infrastructure;
119
using Grand.Infrastructure.Mapper;
1210
using Grand.Mapping;
1311
using Grand.Web.Admin.Controllers;
1412
using Grand.Web.AdminShared.Mapper;
1513
using Grand.Web.AdminShared.Models.Payments;
14+
using Grand.Web.Common.Helpers;
1615
using Microsoft.AspNetCore.Http;
1716
using Microsoft.AspNetCore.Mvc;
1817
using Microsoft.AspNetCore.Mvc.ViewFeatures;
@@ -64,15 +63,14 @@ public void Setup()
6463
new Mock<IServiceProvider>().Object,
6564
contextAccessorMock.Object);
6665

67-
// GetActiveStore resolves its services from HttpContext.RequestServices;
68-
// a single store short-circuits the group/user-field lookups
69-
var storeServiceMock = new Mock<IStoreService>();
70-
storeServiceMock.Setup(s => s.GetAllStores())
71-
.ReturnsAsync(new List<Store> { new() { Id = StoreId } });
66+
// GetActiveStore resolves IAdminStoreService from HttpContext.RequestServices
67+
// (ARCH-001 GetActiveStore() consolidation - BaseAdminController delegates to it instead of
68+
// duplicating the store-scope lookup itself); a single store short-circuits to its own id.
69+
var adminStoreServiceMock = new Mock<IAdminStoreService>();
70+
adminStoreServiceMock.Setup(s => s.GetActiveStore()).ReturnsAsync(StoreId);
7271
var requestServicesMock = new Mock<IServiceProvider>();
73-
requestServicesMock.Setup(sp => sp.GetService(typeof(IStoreService))).Returns(storeServiceMock.Object);
74-
requestServicesMock.Setup(sp => sp.GetService(typeof(IContextAccessor)))
75-
.Returns(contextAccessorMock.Object);
72+
requestServicesMock.Setup(sp => sp.GetService(typeof(IAdminStoreService)))
73+
.Returns(adminStoreServiceMock.Object);
7674
requestServicesMock.Setup(sp => sp.GetService(typeof(IGroupService)))
7775
.Returns(new Mock<IGroupService>().Object);
7876

‎src/Tests/Grand.Web.Common.Tests/Services/Admin/AdminStoreServiceTests.cs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,40 @@ public async Task GetActiveStore_ShouldReturnEmptyString_WhenStoreFromContextDoe
8383
// Assert
8484
Assert.AreEqual(string.Empty, result);
8585
}
86+
87+
[TestMethod]
88+
public async Task GetActiveStore_ShouldReturnNull_WhenNoStoresExist()
89+
{
90+
// Arrange — ARCH-001 GetActiveStore() consolidation: the original implementation dereferenced
91+
// FirstOrDefault() with the null-forgiving operator here, throwing a NullReferenceException on a
92+
// zero-store install. Must degrade gracefully instead, matching BaseAdminController's original
93+
// (already-safe) behavior.
94+
_storeServiceMock.Setup(s => s.GetAllStores()).ReturnsAsync(new List<Store>());
95+
96+
// Act
97+
var result = await _adminStoreService.GetActiveStore();
98+
99+
// Assert
100+
Assert.IsNull(result);
101+
}
102+
103+
[TestMethod]
104+
public async Task GetActiveStore_ShouldReturnEmptyString_WithoutLookup_WhenContextStoreIdIsEmpty()
105+
{
106+
// Arrange — ARCH-001 GetActiveStore() consolidation: an empty scope means "all stores"; must
107+
// short-circuit before calling GetStoreById("") rather than relying on it happening to return
108+
// null for an empty id.
109+
var stores = new List<Store> { new Store { Id = "store1" }, new Store { Id = "store2" } };
110+
_storeServiceMock.Setup(s => s.GetAllStores()).ReturnsAsync(stores);
111+
112+
var customer = new Customer { CustomerGuid = Guid.NewGuid() };
113+
_contextAccessorMock.Setup(c => c.WorkContext.CurrentCustomer).Returns(customer);
114+
115+
// Act
116+
var result = await _adminStoreService.GetActiveStore();
117+
118+
// Assert
119+
Assert.AreEqual(string.Empty, result);
120+
_storeServiceMock.Verify(s => s.GetStoreById(It.IsAny<string>()), Times.Never);
121+
}
86122
}

‎src/Web/Grand.Web.Admin/Controllers/BaseAdminController.cs‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
using Grand.Business.Core.Interfaces.Common.Stores;
2-
using Grand.Domain.Common;
3-
using Grand.Domain.Customers;
4-
using Grand.Infrastructure;
5-
using Grand.Web.Admin.Extensions;
1+
using Grand.Web.Admin.Extensions;
62
using Grand.Web.Common.Controllers;
73
using Grand.Web.Common.Filters;
4+
using Grand.Web.Common.Helpers;
85
using Microsoft.AspNetCore.Mvc;
96

107
namespace Grand.Web.Admin.Controllers;
@@ -19,25 +16,13 @@ public abstract class BaseAdminController : BaseController
1916
/// Get active store scope (for multi-store configuration mode)
2017
/// </summary>
2118
/// <returns>Store ID; 0 if we are in a shared mode</returns>
22-
protected async Task<string> GetActiveStore()
19+
/// <remarks>
20+
/// ARCH-001 GetActiveStore() consolidation: delegates to the shared IAdminStoreService (also used by
21+
/// BaseTaxCategoryController and StoreScope) instead of duplicating its logic. Resolved via the
22+
/// service locator, not constructor injection, to avoid touching every Admin controller's constructor.
23+
/// </remarks>
24+
protected Task<string> GetActiveStore()
2325
{
24-
var storeService = HttpContext.RequestServices.GetRequiredService<IStoreService>();
25-
var workContext = HttpContext.RequestServices.GetRequiredService<IContextAccessor>().WorkContext;
26-
27-
var stores = await storeService.GetAllStores();
28-
if (stores.Count < 2)
29-
return stores.FirstOrDefault()?.Id;
30-
31-
var storeId =
32-
workContext.CurrentCustomer.GetUserFieldFromEntity<string>(SystemCustomerFieldNames
33-
.AdminAreaStoreScopeConfiguration);
34-
//empty scope means "all stores" - settings are loaded from/saved to the global scope
35-
if (string.IsNullOrEmpty(storeId))
36-
{
37-
return "";
38-
}
39-
40-
var store = await storeService.GetStoreById(storeId);
41-
return store != null ? store.Id : "";
26+
return HttpContext.RequestServices.GetRequiredService<IAdminStoreService>().GetActiveStore();
4227
}
4328
}

‎src/Web/Grand.Web.Admin/Controllers/TaxController.cs‎

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,8 @@
33
using Grand.Business.Core.Interfaces.Common.Directory;
44
using Grand.Business.Core.Interfaces.Common.Localization;
55
using Grand.Business.Core.Interfaces.Common.Stores;
6-
using Grand.Domain.Common;
7-
using Grand.Domain.Customers;
86
using Grand.Domain.Directory;
97
using Grand.Domain.Tax;
10-
using Grand.Infrastructure;
118
using Grand.Infrastructure.Caching;
129
using Grand.Infrastructure.Plugins;
1310
using Grand.Web.Admin.Extensions;
@@ -20,6 +17,7 @@
2017
using Grand.Web.Common.DataSource;
2118
using Grand.Web.Common.Extensions;
2219
using Grand.Web.Common.Filters;
20+
using Grand.Web.Common.Helpers;
2321
using Grand.Web.Common.Localization;
2422
using Microsoft.AspNetCore.Mvc;
2523
using Microsoft.AspNetCore.Mvc.Rendering;
@@ -43,34 +41,14 @@ public class TaxController(
4341
ICountryService countryService,
4442
IStoreService storeService,
4543
IEnumTranslationService enumTranslationService,
46-
IContextAccessor contextAccessor,
44+
IAdminStoreService adminStoreService,
4745
IAdminDataScope<TaxCategory> scope)
4846
: BaseTaxCategoryController(taxCategoryService, storeService, translationService, scope)
4947
{
50-
/// <summary>
51-
/// Get active store scope (for multi-store configuration mode)
52-
/// </summary>
53-
/// <returns>Store ID; 0 if we are in a shared mode</returns>
54-
private async Task<string> GetActiveStore()
55-
{
56-
var workContext = contextAccessor.WorkContext;
57-
58-
var stores = await storeService.GetAllStores();
59-
if (stores.Count < 2)
60-
return stores.FirstOrDefault()?.Id;
61-
62-
var storeId =
63-
workContext.CurrentCustomer.GetUserFieldFromEntity<string>(SystemCustomerFieldNames
64-
.AdminAreaStoreScopeConfiguration);
65-
//empty scope means "all stores" - settings are loaded from/saved to the global scope
66-
if (string.IsNullOrEmpty(storeId))
67-
{
68-
return "";
69-
}
70-
71-
var store = await storeService.GetStoreById(storeId);
72-
return store != null ? store.Id : "";
73-
}
48+
// ARCH-001 GetActiveStore() consolidation: was a hand-duplicated private copy of the same logic
49+
// now in IAdminStoreService (also used by BaseAdminController and StoreScope) - BaseTaxCategoryController
50+
// can't carry it itself, it must stay host-agnostic (Admin/Store/Vendor all extend it).
51+
private Task<string> GetActiveStore() => adminStoreService.GetActiveStore();
7452

7553
#region Tax Providers
7654

‎src/Web/Grand.Web.Common/Helpers/AdminStoreService.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,13 @@ public async Task<string> GetActiveStore()
2525
{
2626
var stores = await _storeService.GetAllStores();
2727
if (stores.Count < 2)
28-
return stores.FirstOrDefault()!.Id;
28+
return stores.FirstOrDefault()?.Id;
2929

3030
var storeId = _contextAccessor.WorkContext.CurrentCustomer.GetUserFieldFromEntity<string>(SystemCustomerFieldNames.AdminAreaStoreScopeConfiguration);
31+
//empty scope means "all stores" - settings are loaded from/saved to the global scope
32+
if (string.IsNullOrEmpty(storeId))
33+
return "";
34+
3135
var store = await _storeService.GetStoreById(storeId);
3236

3337
return store != null ? store.Id : "";

0 commit comments

Comments
 (0)