Move the product read-only flag from ViewBag to the model and remove unread ViewBag values

src/Tests/Grand.Mapping.Tests/AdminShared/CatalogProductMappingTests.Product_ToModel.verified.txt

@@ -1,5 +1,6 @@
 {
   Id: prod-001,
+  IsReadOnly: false,
   AuctionEnded: false,
   VisibleIndividually: true,
   ProductLayoutId: pl-001,

src/Tests/Grand.Mapping.Tests/AdminShared/CatalogProductMappingTests.Product_ToModel_WithCalendarData.verified.txt

@@ -1,5 +1,6 @@
 {
   Id: prod-002,
+  IsReadOnly: false,
   AuctionEnded: false,
   VisibleIndividually: false,
   Name: Calendar Product,

src/Tests/Grand.Web.Admin.Tests/Controllers/BaseAddressAttributeControllerTests.cs

@@ -37,6 +37,7 @@ private class TestAddressAttributeController(
     private Mock<IAddressAttributeViewModelService> _vmService = null!;
     private Mock<IAddressAttributeService> _service = null!;
     private Mock<IAdminDataScope<AddressAttribute>> _scope = null!;
+    private Mock<ILanguageService> _languageService = null!;
     private TestAddressAttributeController _sut = null!;
 
     [TestInitialize]
@@ -45,9 +46,10 @@ public void Setup()
         _vmService = new Mock<IAddressAttributeViewModelService>();
         _service = new Mock<IAddressAttributeService>();
         _scope = new Mock<IAdminDataScope<AddressAttribute>>();
+        _languageService = new Mock<ILanguageService>();
         _sut = new TestAddressAttributeController(
             _service.Object, _vmService.Object,
-            Mock.Of<ILanguageService>(), Mock.Of<IRequestTranslationService>(), _scope.Object);
+            _languageService.Object, Mock.Of<IRequestTranslationService>(), _scope.Object);
 
         var httpContext = new DefaultHttpContext();
         var loggerFactoryMock = new Mock<ILoggerFactory>();
@@ -62,6 +64,26 @@ public void Setup()
         _sut.TempData = new TempDataDictionary(httpContext, new Mock<ITempDataProvider>().Object);
     }
 
+    [TestMethod]
+    public async Task EditPost_InvalidModel_KeepsThePostedLocales()
+    {
+        var entity = new AddressAttribute { Id = "1" };
+        _service.Setup(x => x.GetAddressAttributeById("1")).ReturnsAsync(entity);
+        _scope.Setup(x => x.HasAccess(entity)).ReturnsAsync(true);
+        _languageService.Setup(l => l.GetAllLanguages(true, It.IsAny<string>()))
+            .ReturnsAsync(new List<Grand.Domain.Localization.Language> { new() { Id = "lang-1" } });
+        _sut.ModelState.AddModelError("Name", "required");
+        var model = new AddressAttributeModel {
+            Id = "1",
+            Locales = [new AddressAttributeLocalizedModel { LanguageId = "lang-1", Name = "typed" }]
+        };
+
+        var result = await _sut.Edit(model, false);
+
+        Assert.IsInstanceOfType<ViewResult>(result);
+        Assert.AreEqual("typed", model.Locales.Single().Name);
+    }
+
     [TestMethod]
     public async Task List_GlobalScope_ReturnsAllAttributesUnfiltered()
     {

src/Tests/Grand.Web.Admin.Tests/Controllers/BaseBlogControllerTests.cs

@@ -46,6 +46,7 @@ private class TestBlogController(
     private Mock<IRequestTranslationService> _translationServiceMock;
     private Mock<IAdminDataScope<BlogPost>> _postScopeMock;
     private Mock<IAdminDataScope<BlogCategory>> _categoryScopeMock;
+    private Mock<ILanguageService> _languageServiceMock;
 
     [TestInitialize]
     public void Setup()
@@ -67,14 +68,14 @@ public void Setup()
         _categoryScopeMock = new Mock<IAdminDataScope<BlogCategory>>();
         _categoryScopeMock.Setup(s => s.DefaultStoreId).Returns((string)null);
 
-        var languageServiceMock = new Mock<ILanguageService>();
-        languageServiceMock.Setup(l => l.GetAllLanguages(true, It.IsAny<string>()))
+        _languageServiceMock = new Mock<ILanguageService>();
+        _languageServiceMock.Setup(l => l.GetAllLanguages(true, It.IsAny<string>()))
             .ReturnsAsync(new List<Grand.Domain.Localization.Language>());
 
         _controller = new TestBlogController(
             _blogServiceMock.Object,
             _blogViewModelServiceMock.Object,
-            languageServiceMock.Object,
+            _languageServiceMock.Object,
             _translationServiceMock.Object,
             new Mock<IStoreService>().Object,
             new Mock<IDateTimeService>().Object,
@@ -152,6 +153,63 @@ public async Task EditPost_ScopeDeniesAccess_RedirectsToEdit()
         _blogViewModelServiceMock.Verify(v => v.UpdateBlogPostModel(It.IsAny<BlogPostModel>(), It.IsAny<BlogPost>()), Times.Never);
     }
 
+    [TestMethod]
+    public async Task EditPost_InvalidModel_KeepsThePostedLocales()
+    {
+        var post = new BlogPost { Id = "p1" };
+        _blogServiceMock.Setup(b => b.GetBlogPostById("p1")).ReturnsAsync(post);
+        _postScopeMock.Setup(s => s.HasAccess(post)).ReturnsAsync(true);
+        SetOneLanguage();
+        _controller.ModelState.AddModelError("Title", "required");
+        var model = new BlogPostModel {
+            Id = "p1",
+            Locales = [new BlogLocalizedModel { LanguageId = "lang-1", Title = "typed" }]
+        };
+
+        var result = await _controller.Edit(model, false);
+
+        Assert.IsInstanceOfType<ViewResult>(result);
+        Assert.AreEqual("typed", model.Locales.Single().Title);
+    }
+
+    [TestMethod]
+    public async Task CategoryCreatePost_InvalidModel_KeepsThePostedLocales()
+    {
+        SetOneLanguage();
+        _controller.ModelState.AddModelError("Name", "required");
+        var model = new BlogCategoryModel {
+            Locales = [new BlogCategoryLocalizedModel { LanguageId = "lang-1", Name = "typed" }]
+        };
+
+        var result = await _controller.CategoryCreate(model, false);
+
+        Assert.IsInstanceOfType<ViewResult>(result);
+        Assert.AreEqual("typed", model.Locales.Single().Name);
+    }
+
+    [TestMethod]
+    public async Task CategoryEditPost_InvalidModel_KeepsThePostedLocales()
+    {
+        var category = new BlogCategory { Id = "c1" };
+        _blogServiceMock.Setup(b => b.GetBlogCategoryById("c1")).ReturnsAsync(category);
+        _categoryScopeMock.Setup(s => s.HasAccess(category)).ReturnsAsync(true);
+        SetOneLanguage();
+        _controller.ModelState.AddModelError("Name", "required");
+        var model = new BlogCategoryModel {
+            Id = "c1",
+            Locales = [new BlogCategoryLocalizedModel { LanguageId = "lang-1", Name = "typed" }]
+        };
+
+        var result = await _controller.CategoryEdit(model, false);
+
+        Assert.IsInstanceOfType<ViewResult>(result);
+        Assert.AreEqual("typed", model.Locales.Single().Name);
+    }
+
+    private void SetOneLanguage() =>
+        _languageServiceMock.Setup(l => l.GetAllLanguages(true, It.IsAny<string>()))
+            .ReturnsAsync(new List<Grand.Domain.Localization.Language> { new() { Id = "lang-1" } });
+
     [TestMethod]
     public async Task Delete_ScopeDeniesAccess_RedirectsToEditWithoutDeleting()
     {

src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCustomerAttributeControllerTests.cs

@@ -34,6 +34,7 @@ private class TestCustomerAttributeController(
     private Mock<ICustomerAttributeService> _service = null!;
     private Mock<ICustomerAttributeViewModelService> _vmService = null!;
     private Mock<IAdminDataScope<CustomerAttribute>> _scope = null!;
+    private Mock<ILanguageService> _languageService = null!;
     private TestCustomerAttributeController _sut = null!;
 
     [TestInitialize]
@@ -42,8 +43,9 @@ public void Setup()
         _service = new Mock<ICustomerAttributeService>();
         _vmService = new Mock<ICustomerAttributeViewModelService>();
         _scope = new Mock<IAdminDataScope<CustomerAttribute>>();
+        _languageService = new Mock<ILanguageService>();
         _sut = new TestCustomerAttributeController(_service.Object, _vmService.Object,
-            Mock.Of<ILanguageService>(), Mock.Of<IRequestTranslationService>(), _scope.Object);
+            _languageService.Object, Mock.Of<IRequestTranslationService>(), _scope.Object);
 
         var httpContext = new DefaultHttpContext();
         var loggerFactoryMock = new Mock<ILoggerFactory>();
@@ -58,6 +60,26 @@ public void Setup()
         _sut.TempData = new TempDataDictionary(httpContext, new Mock<ITempDataProvider>().Object);
     }
 
+    [TestMethod]
+    public async Task EditPost_InvalidModel_KeepsThePostedLocales()
+    {
+        var entity = new CustomerAttribute { Id = "1" };
+        _service.Setup(x => x.GetCustomerAttributeById("1")).ReturnsAsync(entity);
+        _scope.Setup(x => x.HasAccess(entity)).ReturnsAsync(true);
+        _languageService.Setup(l => l.GetAllLanguages(true, It.IsAny<string>()))
+            .ReturnsAsync(new List<Grand.Domain.Localization.Language> { new() { Id = "lang-1" } });
+        _sut.ModelState.AddModelError("Name", "required");
+        var model = new CustomerAttributeModel {
+            Id = "1",
+            Locales = [new CustomerAttributeLocalizedModel { LanguageId = "lang-1", Name = "typed" }]
+        };
+
+        var result = await _sut.Edit(model, false);
+
+        Assert.IsInstanceOfType<ViewResult>(result);
+        Assert.AreEqual("typed", model.Locales.Single().Name);
+    }
+
     [TestMethod]
     public async Task Delete_UsesViewModelServiceNotRawService()
     {

src/Tests/Grand.Web.Admin.Tests/Controllers/BaseNewsControllerTests.cs

@@ -213,28 +213,6 @@ public async Task CreatePost_GlobalScoped_LeavesModelStoresUntouched()
         await _controller.Create(submitted, false);
     }
 
-    [TestMethod]
-    public async Task CreateGet_PopulatesAllLanguages()
-    {
-        var languageServiceMock = new Mock<ILanguageService>();
-        languageServiceMock.Setup(l => l.GetAllLanguages(true, It.IsAny<string>()))
-            .ReturnsAsync(new List<Grand.Domain.Localization.Language> { new() { Id = "l1" } });
-        var controller = new TestNewsController(
-            _newsViewModelServiceMock.Object, _newsServiceMock.Object, languageServiceMock.Object,
-            _translationServiceMock.Object, new Mock<IStoreService>().Object, new Mock<IDateTimeService>().Object,
-            _scopeMock.Object);
-        controller.ControllerContext = _controller.ControllerContext;
-        controller.TempData = _controller.TempData;
-
-        await controller.Create();
-
-        // Confirms the value assigned to ViewBag.AllLanguages is the resolved list, not an unawaited
-        // Task object (the pre-existing bug this task fixes) - would throw InvalidCastException or
-        // fail this assertion if the bug were reintroduced.
-        var allLanguages = (IList<Grand.Domain.Localization.Language>)controller.ViewBag.AllLanguages;
-        Assert.AreEqual(1, allLanguages.Count);
-    }
-
     [TestMethod]
     public async Task EditGet_NewsItemNotFound_RedirectsToList()
     {

src/Tests/Grand.Web.Admin.Tests/Controllers/BaseProductAttributeControllerTests.cs

@@ -38,6 +38,7 @@ private class TestProductAttributeController(
     private Mock<IProductService> _productService = null!;
     private Mock<IProductAttributeService> _attributeService = null!;
     private Mock<IAdminDataScope<ProductAttribute>> _scope = null!;
+    private Mock<ILanguageService> _languageService = null!;
     private TestProductAttributeController _sut = null!;
 
     [TestInitialize]
@@ -46,8 +47,9 @@ public void Setup()
         _productService = new Mock<IProductService>();
         _attributeService = new Mock<IProductAttributeService>();
         _scope = new Mock<IAdminDataScope<ProductAttribute>>();
+        _languageService = new Mock<ILanguageService>();
         _sut = new TestProductAttributeController(_productService.Object, _attributeService.Object,
-            Mock.Of<ILanguageService>(), Mock.Of<IRequestTranslationService>(), new SeoSettings(), _scope.Object);
+            _languageService.Object, Mock.Of<IRequestTranslationService>(), new SeoSettings(), _scope.Object);
 
         var httpContext = new DefaultHttpContext();
         var loggerFactoryMock = new Mock<ILoggerFactory>();
@@ -62,6 +64,26 @@ public void Setup()
         _sut.TempData = new TempDataDictionary(httpContext, new Mock<ITempDataProvider>().Object);
     }
 
+    [TestMethod]
+    public async Task EditPost_InvalidModel_KeepsThePostedLocales()
+    {
+        var entity = new ProductAttribute { Id = "1" };
+        _attributeService.Setup(x => x.GetProductAttributeById("1")).ReturnsAsync(entity);
+        _scope.Setup(x => x.HasAccess(entity)).ReturnsAsync(true);
+        _languageService.Setup(l => l.GetAllLanguages(true, It.IsAny<string>()))
+            .ReturnsAsync(new List<Grand.Domain.Localization.Language> { new() { Id = "lang-1" } });
+        _sut.ModelState.AddModelError("Name", "required");
+        var model = new Grand.Web.AdminShared.Models.Catalog.ProductAttributeModel {
+            Id = "1",
+            Locales = [new Grand.Web.AdminShared.Models.Catalog.ProductAttributeLocalizedModel { LanguageId = "lang-1", Name = "typed" }]
+        };
+
+        var result = await _sut.Edit(model, false);
+
+        Assert.IsInstanceOfType<ViewResult>(result);
+        Assert.AreEqual("typed", model.Locales.Single().Name);
+    }
+
     [TestMethod]
     public async Task List_PassesScopeStoreIdToService()
     {

src/Tests/Grand.Web.Admin.Tests/Controllers/BaseProductControllerTests.cs

@@ -17,11 +17,17 @@
 using Grand.Web.Common.Security.Authorization;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.AspNetCore.Mvc.ViewFeatures;
+using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Primitives;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
+using System.Globalization;
 using System.Linq.Expressions;
 
 namespace Grand.Web.Admin.Tests.Controllers;
@@ -140,7 +146,7 @@ public async Task EditGet_UsesCanViewNotHasAccess_LooserRuleWins()
 
         //shown, but read-only: HasAccess no longer gates the page, it only takes the buttons away
         Assert.IsInstanceOfType<ViewResult>(result);
-        Assert.AreEqual(true, ((ViewResult)result).ViewData["IsReadOnly"]);
+        Assert.IsTrue(((ProductModel)((ViewResult)result).Model).IsReadOnly);
     }
 
     [TestMethod]
@@ -154,7 +160,36 @@ public async Task EditGet_OwnedProduct_IsNotReadOnly()
         var result = await _controller.Edit("p1");
 
         Assert.IsInstanceOfType<ViewResult>(result);
-        Assert.AreEqual(false, ((ViewResult)result).ViewData["IsReadOnly"]);
+        Assert.IsFalse(((ProductModel)((ViewResult)result).Model).IsReadOnly);
+    }
+
+    [TestMethod]
+    public async Task IsReadOnly_IsNeverBoundFromTheRequest()
+    {
+        // The server decides it in Edit(GET); a posted value must not reach the re-rendered form.
+        var services = new ServiceCollection().AddLogging();
+        services.AddMvcCore();
+        await using var provider = services.BuildServiceProvider();
+        var metadata = provider.GetRequiredService<IModelMetadataProvider>().GetMetadataForType(typeof(ProductModel));
+        var binder = provider.GetRequiredService<IModelBinderFactory>().CreateBinder(
+            new ModelBinderFactoryContext { Metadata = metadata, BindingInfo = new BindingInfo() });
+        var form = new FormCollection(new Dictionary<string, StringValues> {
+            ["Name"] = "posted",
+            ["IsReadOnly"] = "true"
+        });
+
+        var result = await provider.GetRequiredService<ParameterBinder>().BindModelAsync(
+            new ActionContext(new DefaultHttpContext { RequestServices = provider }, new RouteData(), new ActionDescriptor()),
+            binder,
+            new FormValueProvider(BindingSource.Form, form, CultureInfo.InvariantCulture),
+            new ParameterDescriptor { Name = "model", ParameterType = typeof(ProductModel) },
+            metadata,
+            value: null,
+            container: null);
+
+        var model = (ProductModel)result.Model;
+        Assert.AreEqual("posted", model.Name);
+        Assert.IsFalse(model.IsReadOnly);
     }
 
     // --- Edit (POST) -------------------------------------------------------------------------------
@@ -473,11 +508,7 @@ public async Task LoadProductFriendlyNames_ScopeDeniesAccess_SkipsProduct()
 
         var json = result as JsonResult;
         Assert.IsNotNull(json);
-        // Faithful port of Store/Vendor's original index-based comma logic: it decides whether to
-        // append ", " from the loop index (i != products.Count - 1), not from whether a name was
-        // actually appended, so skipping the last product still leaves a trailing ", ". Pre-existing
-        // quirk in both original hosts, not introduced by this migration - characterized, not fixed.
-        Assert.AreEqual("Allowed, ", GetTextProperty(json.Value));
+        Assert.AreEqual("Allowed", GetTextProperty(json.Value));
     }
 
     [TestMethod]
@@ -508,11 +539,10 @@ public async Task RequiredProductAddPopup_CallsPrepareAddRequiredProductModel()
         _productAddPopupViewModelServiceMock.Setup(s => s.PrepareAddRequiredProductModel())
             .ReturnsAsync(new ProductModel.AddRequiredProductModel());
 
-        var result = await _controller.RequiredProductAddPopup("input1") as ViewResult;
+        var result = await _controller.RequiredProductAddPopup() as ViewResult;
 
         Assert.IsNotNull(result);
         _productAddPopupViewModelServiceMock.Verify(s => s.PrepareAddRequiredProductModel(), Times.Once);
-        Assert.AreEqual("input1", _controller.ViewBag.productIdsInput);
     }
 
     // --- RequiredProductAddPopupList -------------------------------------------------------------------