Repository: btcpayserver/btcpayserver (csharp)

Issue:
Adding products with the same ID breaks the POS
# EDIT - REASON:

I figured out how I broke it, if the Product has the same `ID`, it triggers this error? Set ID = 2 for two of them.

----

I added 4 items to my Products list, added them all via POS cart, and it kept triggering this error:

Screenshots, because copy/pasting the log via the browser wasn't formatting well. Hope this is helpful.

<img width="1788" alt="Screen Shot 2024-09-16 at 4 13 03 PM" src="https://github.com/user-attachments/assets/e428fc07-fd86-4481-99d3-9b35c535c2ba">
<img width="1790" alt="Screen Shot 2024-09-16 at 4 13 14 PM" src="https://github.com/user-attachments/assets/a529849a-3b7a-466d-a0a7-79fbfb0b6088">

Pull request:
Validates missing and duplicate IDs on the edit actions and when creating/updating apps via the API. Fails gracefully by excluding existing items without ID or with duplicate ID for the rest of the cases.

Fixes #6227.

Patch:
diff --git a/BTCPayServer/Controllers/GreenField/GreenfieldAppsController.cs b/BTCPayServer/Controllers/GreenField/GreenfieldAppsController.cs
index fe477403eb..e17a7fb165 100644
--- a/BTCPayServer/Controllers/GreenField/GreenfieldAppsController.cs
+++ b/BTCPayServer/Controllers/GreenField/GreenfieldAppsController.cs
@@ -402,11 +402,11 @@ private void ValidatePOSAppRequest(PointOfSaleAppRequest request)
                 try
                 {
                     // Just checking if we can serialize
-                    AppService.SerializeTemplate(AppService.Parse(request.Template));
+                    AppService.SerializeTemplate(AppService.Parse(request.Template, true, true));
                 }
-                catch
+                catch (Exception ex)
                 {
-                    ModelState.AddModelError(nameof(request.Template), "Invalid template");
+                    ModelState.AddModelError(nameof(request.Template), ex.Message);
                 }
             }
         }
@@ -486,11 +486,11 @@ private void ValidateCrowdfundAppRequest(CrowdfundAppRequest request)
                 try
                 {
                     // Just checking if we can serialize
-                    AppService.SerializeTemplate(AppService.Parse(request.PerksTemplate));
+                    AppService.SerializeTemplate(AppService.Parse(request.PerksTemplate, true, true));
                 }
-                catch
+                catch (Exception ex)
                 {
-                    ModelState.AddModelError(nameof(request.PerksTemplate), "Invalid template");
+                    ModelState.AddModelError(nameof(request.PerksTemplate), $"Invalid template: {ex.Message}");
                 }
             }
 
diff --git a/BTCPayServer/Plugins/Crowdfund/Controllers/UICrowdfundController.cs b/BTCPayServer/Plugins/Crowdfund/Controllers/UICrowdfundController.cs
index a1edaa088b..321eab3923 100644
--- a/BTCPayServer/Plugins/Crowdfund/Controllers/UICrowdfundController.cs
+++ b/BTCPayServer/Plugins/Crowdfund/Controllers/UICrowdfundController.cs
@@ -447,11 +447,11 @@ public async Task<IActionResult> UpdateCrowdfund(string appId, UpdateCrowdfundVi
 
             try
             {
-                vm.PerksTemplate = AppService.SerializeTemplate(AppService.Parse(vm.PerksTemplate));
+                vm.PerksTemplate = AppService.SerializeTemplate(AppService.Parse(vm.PerksTemplate, true, true));
             }
-            catch
+            catch (Exception ex)
             {
-                ModelState.AddModelError(nameof(vm.PerksTemplate), "Invalid template");
+                ModelState.AddModelError(nameof(vm.PerksTemplate), $"Invalid template: {ex.Message}");
             }
             if (vm.TargetAmount is decimal v && v == 0.0m)
             {
diff --git a/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs b/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs
index 27202be977..7f5cae7f39 100644
--- a/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs
+++ b/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs
@@ -647,11 +647,11 @@ public async Task<IActionResult> UpdatePointOfSale(string appId, UpdatePointOfSa
                 ModelState.AddModelError(nameof(vm.Currency), "Invalid currency");
             try
             {
-                vm.Template = AppService.SerializeTemplate(AppService.Parse(vm.Template));
+                vm.Template = AppService.SerializeTemplate(AppService.Parse(vm.Template, true, true));
             }
-            catch
+            catch (Exception ex)
             {
-                ModelState.AddModelError(nameof(vm.Template), "Invalid template");
+                ModelState.AddModelError(nameof(vm.Template), $"Invalid template: {ex.Message}");
             }
             if (!ModelState.IsValid)
             {
diff --git a/BTCPayServer/Services/Apps/AppService.cs b/BTCPayServer/Services/Apps/AppService.cs
index c4fd79f8f6..b10229a911 100644
--- a/BTCPayServer/Services/Apps/AppService.cs
+++ b/BTCPayServer/Services/Apps/AppService.cs
@@ -348,12 +348,17 @@ public static string SerializeTemplate(ViewPointOfSaleViewModel.Item[] items)
         {
                return JsonConvert.SerializeObject(items, Formatting.Indented, _defaultSerializer);
         }
-        public static ViewPointOfSaleViewModel.Item[] Parse(string template, bool includeDisabled = true)
+        public static ViewPointOfSaleViewModel.Item[] Parse(string template, bool includeDisabled = true, bool throws = false)
         {
-            if (string.IsNullOrWhiteSpace(template))
-                return Array.Empty<ViewPointOfSaleViewModel.Item>();
-
-            return  JsonConvert.DeserializeObject<ViewPointOfSaleViewModel.Item[]>(template, _defaultSerializer)!.Where(item => includeDisabled || !item.Disabled).ToArray();
+            if (string.IsNullOrWhiteSpace(template)) return [];
+            var allItems = JsonConvert.DeserializeObject<ViewPointOfSaleViewModel.Item[]>(template, _defaultSerializer)!;
+            // ensure all items have an id, which is also unique
+            var itemsWithoutId = allItems.Where(i => string.IsNullOrEmpty(i.Id)).ToList();
+            if (itemsWithoutId.Any() && throws) throw new ArgumentException($"Missing ID for item \"{itemsWithoutId.First().Title}\".");
+            // find items with duplicate IDs
+            var duplicateIds = allItems.GroupBy(i => i.Id).Where(g => g.Count() > 1).Select(g => g.Key).ToList();
+            if (duplicateIds.Any() && throws) throw new ArgumentException($"Duplicate ID \"{duplicateIds.First()}\".");
+            return allItems.Where(item => (includeDisabled || !item.Disabled) && !itemsWithoutId.Contains(item) && !duplicateIds.Contains(item.Id)).ToArray();
         }
 #nullable restore
 #nullable enable
diff --git a/BTCPayServer/Views/Shared/Crowdfund/UpdateCrowdfund.cshtml b/BTCPayServer/Views/Shared/Crowdfund/UpdateCrowdfund.cshtml
index 203d133006..3bf9c2b01f 100644
--- a/BTCPayServer/Views/Shared/Crowdfund/UpdateCrowdfund.cshtml
+++ b/BTCPayServer/Views/Shared/Crowdfund/UpdateCrowdfund.cshtml
@@ -48,7 +48,7 @@
 
     @if (!ViewContext.ModelState.IsValid)
     {
-        <div asp-validation-summary="ModelOnly"></div>
+        <div asp-validation-summary="All" class="@(ViewContext.ModelState.ErrorCount.Equals(1) ? "no-marker" : "")"></div>
     }
     <div class="row">
         <div class="col-sm-10 col-md-9 col-xl-7 col-xxl-6">
diff --git a/BTCPayServer/Views/Shared/PointOfSale/UpdatePointOfSale.cshtml b/BTCPayServer/Views/Shared/PointOfSale/UpdatePointOfSale.cshtml
index 9dbda45247..16c5958b85 100644
--- a/BTCPayServer/Views/Shared/PointOfSale/UpdatePointOfSale.cshtml
+++ b/BTCPayServer/Views/Shared/PointOfSale/UpdatePointOfSale.cshtml
@@ -61,7 +61,7 @@
 
     @if (!ViewContext.ModelState.IsValid)
     {
-        <div asp-validation-summary="ModelOnly"></div>
+        <div asp-validation-summary="All" class="@(ViewContext.ModelState.ErrorCount.Equals(1) ? "no-marker" : "")"></div>
     }
     <div class="row">
         <div class="col-sm-10 col-md-9 col-xl-7 col-xxl-6">
diff --git a/BTCPayServer/Views/Shared/TemplateEditor.cshtml b/BTCPayServer/Views/Shared/TemplateEditor.cshtml
index e542805d4e..567d286ddb 100644
--- a/BTCPayServer/Views/Shared/TemplateEditor.cshtml
+++ b/BTCPayServer/Views/Shared/TemplateEditor.cshtml
@@ -118,13 +118,12 @@
 </template>
 
 <div id="TemplateEditor" class="editor" v-cloak>
-    <h3 class="mt-5 mb-4" v-pre>@Model.title</h3>
+    <h3 class="mt-5 mb-3" v-pre>@Model.title</h3>
     @if (ViewContext.ViewData.ModelState.TryGetValue(Model.templateId, out var errors))
     {
         foreach (var error in errors.Errors)
         {
-            <br />
-            <span class="text-danger" v-pre>@error.ErrorMessage</span>
+            <p class="text-danger" v-pre>@error.ErrorMessage</p>
         }
     }
     <div class="d-flex flex-wrap align-items-end justify-content-between gap-3 mb-3">


Test patch:
diff --git a/BTCPayServer.Tests/GreenfieldAPITests.cs b/BTCPayServer.Tests/GreenfieldAPITests.cs
index d7ae0e2701..77b44af6e4 100644
--- a/BTCPayServer.Tests/GreenfieldAPITests.cs
+++ b/BTCPayServer.Tests/GreenfieldAPITests.cs
@@ -367,6 +367,27 @@ await AssertValidationError(new[] { "AppName", "Currency", "Template" },
                     }
                 )
             );
+            var template = @"[
+              {
+                ""description"": ""Lovely, fresh and tender, Meng Ding Gan Lu ('sweet dew') is grown in the lush Meng Ding Mountains of the southwestern province of Sichuan where it has been cultivated for over a thousand years."",
+                ""id"": ""green-tea"",
+                ""image"": ""~/img/pos-sample/green-tea.jpg"",
+                ""priceType"": ""Fixed"",
+                ""price"": ""1"",
+                ""title"": ""Green Tea"",
+                ""disabled"": false
+              }
+            ]";
+            await AssertValidationError(new[] { "Template" },
+                async () => await client.CreatePointOfSaleApp(
+                    user.StoreId,
+                    new PointOfSaleAppRequest
+                    {
+                        AppName = "good name",
+                        Template = template.Replace(@"""id"": ""green-tea"",", "")
+                    }
+                )
+            );
 
             // Test creating a POS app successfully
             var app = await client.CreatePointOfSaleApp(
@@ -375,7 +396,8 @@ await AssertValidationError(new[] { "AppName", "Currency", "Template" },
                 {
                     AppName = "test app from API",
                     Currency = "JPY",
-                    Title = "test app title"
+                    Title = "test app title",
+                    Template = template
                 }
             );
             Assert.Equal("test app from API", app.AppName);
@@ -558,6 +580,27 @@ await AssertValidationError(new[] { "EndDate" },
                     }
                 )
             );
+            var template = @"[
+              {
+                ""description"": ""Lovely, fresh and tender, Meng Ding Gan Lu ('sweet dew') is grown in the lush Meng Ding Mountains of the southwestern province of Sichuan where it has been cultivated for over a thousand years."",
+                ""id"": ""green-tea"",
+                ""image"": ""~/img/pos-sample/green-tea.jpg"",
+                ""priceType"": ""Fixed"",
+                ""price"": ""1"",
+                ""title"": ""Green Tea"",
+                ""disabled"": false
+              }
+            ]";
+            await AssertValidationError(new[] { "PerksTemplate" },
+                async () => await client.CreateCrowdfundApp(
+                    user.StoreId,
+                    new CrowdfundAppRequest
+                    {
+                        AppName = "good name",
+                        PerksTemplate = template.Replace(@"""id"": ""green-tea"",", "")
+                    }
+                )
+            );
 
             // Test creating a crowdfund app
             var app = await client.CreateCrowdfundApp(
@@ -565,7 +608,8 @@ await AssertValidationError(new[] { "EndDate" },
                 new CrowdfundAppRequest
                 {
                     AppName = "test app from API",
-                    Title = "test app title"
+                    Title = "test app title",
+                    PerksTemplate = template
                 }
             );
             Assert.Equal("test app from API", app.AppName);
diff --git a/BTCPayServer.Tests/POSTests.cs b/BTCPayServer.Tests/POSTests.cs
index 5e4a4b8c53..b4a1b447c2 100644
--- a/BTCPayServer.Tests/POSTests.cs
+++ b/BTCPayServer.Tests/POSTests.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Threading.Tasks;
 using BTCPayServer.Client;
 using BTCPayServer.Controllers;
@@ -90,6 +91,54 @@ public void CanParseOldYmlCorrectly()
         Assert.Null( parsedDefault[4].AdditionalData);
         Assert.Null( parsedDefault[4].PaymentMethods);
         }
+
+        [Fact]
+        [Trait("Fast", "Fast")]
+        public void CanParseAppTemplate()
+        {
+            var template = @"[
+              {
+                ""description"": ""Lovely, fresh and tender, Meng Ding Gan Lu ('sweet dew') is grown in the lush Meng Ding Mountains of the southwestern province of Sichuan where it has been cultivated for over a thousand years."",
+                ""id"": ""green-tea"",
+                ""image"": ""~/img/pos-sample/green-tea.jpg"",
+                ""priceType"": ""Fixed"",
+                ""price"": ""1"",
+                ""title"": ""Green Tea"",
+                ""disabled"": false
+              },
+              {
+                ""description"": ""Tian Jian Tian Jian means 'heavenly tippy tea' in Chinese, and it describes the finest grade of dark tea. Our Tian Jian dark tea is from Hunan province which is famous for making some of the best dark teas available."",
+                ""id"": ""black-tea"",
+                ""image"": ""~/img/pos-sample/black-tea.jpg"",
+                ""priceType"": ""Fixed"",
+                ""price"": ""1"",
+                ""title"": ""Black Tea"",
+                ""disabled"": false
+              }
+            ]";
+
+            var items = AppService.Parse(template);
+            Assert.Equal(2, items.Length);
+            Assert.Equal("green-tea", items[0].Id);
+            Assert.Equal("black-tea", items[1].Id);
+
+            // Fails gracefully for missing ID
+            var missingId = template.Replace(@"""id"": ""green-tea"",", "");
+            items = AppService.Parse(missingId);
+            Assert.Single(items);
+            Assert.Equal("black-tea", items[0].Id);
+            
+            // Throws for missing ID
+            Assert.Throws<ArgumentException>(() => AppService.Parse(missingId, true, true));
+
+            // Fails gracefully for duplicate IDs
+            var duplicateId = template.Replace(@"""id"": ""green-tea"",", @"""id"": ""black-tea"",");
+            items = AppService.Parse(duplicateId);
+            Assert.Empty(items);
+            
+            // Throws for duplicate IDs
+            Assert.Throws<ArgumentException>(() => AppService.Parse(duplicateId, true, true));
+        }
         
         [Fact(Timeout = LongRunningTestTimeout)]
         [Trait("Integration", "Integration")]
diff --git a/BTCPayServer.Tests/SeleniumTests.cs b/BTCPayServer.Tests/SeleniumTests.cs
index ea94e2603c..117020d6b1 100644
--- a/BTCPayServer.Tests/SeleniumTests.cs
+++ b/BTCPayServer.Tests/SeleniumTests.cs
@@ -1253,6 +1253,15 @@ public async Task CanCreateAppPoS()
 
             s.ClickPagePrimary();
             Assert.Contains("App updated", s.FindAlertMessage().Text);
+            
+            s.Driver.ScrollTo(By.Id("CodeTabButton"));
+            s.Driver.FindElement(By.Id("CodeTabButton")).Click();
+            template = s.Driver.FindElement(By.Id("TemplateConfig")).GetAttribute("value");
+            s.Driver.FindElement(By.Id("TemplateConfig")).Clear();
+            s.Driver.FindElement(By.Id("TemplateConfig")).SendKeys(template.Replace(@"""id"": ""green-tea"",", ""));
+
+            s.ClickPagePrimary();
+            Assert.Contains("Invalid template: Missing ID for item \"Green Tea\".", s.Driver.FindElement(By.CssSelector(".validation-summary-errors")).Text);
 
             s.Driver.FindElement(By.Id("ViewApp")).Click();
             var windows = s.Driver.WindowHandles;