Disabled amount/currency update for payment request with active invoices (#4390)

* Disabled amount/currency update for payment request with active invoices

close #4241

* Check amount isn't changed in backend

* Add test case

* Update BTCPayServer/Controllers/GreenField/GreenfieldPaymentRequestsController.cs

Co-authored-by: d11n <mail@dennisreimann.de>

* Update BTCPayServer/Controllers/UIPaymentRequestController.cs

Co-authored-by: d11n <mail@dennisreimann.de>

* Improve wording

Co-authored-by: nicolas.dorier <nicolas.dorier@gmail.com>
Co-authored-by: Dennis Reimann <mail@dennisreimann.de>

BTCPayServer.Tests/GreenfieldAPITests.cs

@@ -1409,6 +1409,12 @@ namespace BTCPayServer.Tests
             var invoiceData = await client.PayPaymentRequest(user.StoreId, paymentTestPaymentRequest.Id, new PayPaymentRequestRequest());
             await Pay(invoiceData.Id);
 
+            // Can't update amount once invoice has been created
+            await AssertValidationError(new[] { "Amount" }, () => client.UpdatePaymentRequest(user.StoreId, paymentTestPaymentRequest.Id, new UpdatePaymentRequestRequest()
+            {
+                Amount = 294m
+            }));
+
             // Let's tests some unhappy path
             paymentTestPaymentRequest = await client.CreatePaymentRequest(user.StoreId,
                 new CreatePaymentRequestRequest() { Amount = 0.1m, AllowCustomPaymentAmounts = false, Currency = "BTC", Title = "Payment test title" });

BTCPayServer.Tests/PaymentRequestTests.cs

@@ -53,7 +53,7 @@ namespace BTCPayServer.Tests
 
             // Permission guard for guests editing 
             Assert
-                .IsType<NotFoundResult>(guestpaymentRequestController.EditPaymentRequest(user.StoreId, id));
+                .IsType<NotFoundResult>(await guestpaymentRequestController.EditPaymentRequest(user.StoreId, id));
 
             request.Title = "update";
             Assert.IsType<RedirectToActionResult>(await paymentRequestController.EditPaymentRequest(id, request));

BTCPayServer.Tests/SeleniumTests.cs

@@ -942,6 +942,11 @@ namespace BTCPayServer.Tests
             s.Driver.FindElement(By.Id("ClearExpiryDate")).Click();
             s.Driver.FindElement(By.Id("SaveButton")).Click();
             s.Driver.FindElement(By.XPath("//a[starts-with(@id, 'Edit-')]")).Click();
+
+            // amount and currency should be editable, because no invoice exists
+            s.GoToUrl(editUrl);
+            Assert.True(s.Driver.FindElement(By.Id("Amount")).Enabled);
+            Assert.True(s.Driver.FindElement(By.Id("Currency")).Enabled);
             
             s.GoToUrl(viewUrl);
             s.Driver.AssertElementNotFound(By.CssSelector("[data-test='status']"));
@@ -953,8 +958,12 @@ namespace BTCPayServer.Tests
             s.Driver.WaitForElement(By.CssSelector("invoice"));
             Assert.Contains("Awaiting Payment", s.Driver.PageSource);
             
-            // archive (from details page)
+            // amount and currency should not be editable, because invoice exists
             s.GoToUrl(editUrl);
+            Assert.False(s.Driver.FindElement(By.Id("Amount")).Enabled);
+            Assert.False(s.Driver.FindElement(By.Id("Currency")).Enabled);
+            
+            // archive (from details page)
             var payReqId = s.Driver.Url.Split('/').Last();
             s.Driver.FindElement(By.Id("ArchivePaymentRequest")).Click();
             Assert.Contains("The payment request has been archived", s.FindAlertMessage().Text);

BTCPayServer/Controllers/GreenField/GreenfieldPaymentRequestsController.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Reflection.Metadata;
 using System.Threading;
 using System.Threading.Tasks;
 using BTCPayServer.Abstractions.Constants;
@@ -15,6 +16,7 @@ using BTCPayServer.Services.PaymentRequests;
 using BTCPayServer.Services.Rates;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Cors;
+using Microsoft.AspNetCore.Identity;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Routing;
 using PaymentRequestData = BTCPayServer.Data.PaymentRequestData;
@@ -30,6 +32,7 @@ namespace BTCPayServer.Controllers.Greenfield
         private readonly UIInvoiceController _invoiceController;
         private readonly PaymentRequestRepository _paymentRequestRepository;
         private readonly CurrencyNameTable _currencyNameTable;
+        private readonly UserManager<ApplicationUser> _userManager;
         private readonly LinkGenerator _linkGenerator;
 
         public GreenfieldPaymentRequestsController(
@@ -38,6 +41,7 @@ namespace BTCPayServer.Controllers.Greenfield
             PaymentRequestRepository paymentRequestRepository,
             PaymentRequestService paymentRequestService,
             CurrencyNameTable currencyNameTable,
+            UserManager<ApplicationUser> userManager,
             LinkGenerator linkGenerator)
         {
             _InvoiceRepository = invoiceRepository;
@@ -45,6 +49,7 @@ namespace BTCPayServer.Controllers.Greenfield
             _paymentRequestRepository = paymentRequestRepository;
             PaymentRequestService = paymentRequestService;
             _currencyNameTable = currencyNameTable;
+            _userManager = userManager;
             _linkGenerator = linkGenerator;
         }
 
@@ -152,7 +157,7 @@ namespace BTCPayServer.Controllers.Greenfield
         public async Task<IActionResult> CreatePaymentRequest(string storeId,
             CreatePaymentRequestRequest request)
         {
-            var validationResult = Validate(request);
+            var validationResult = await Validate(null, request);
             if (validationResult != null)
             {
                 return validationResult;
@@ -178,7 +183,7 @@ namespace BTCPayServer.Controllers.Greenfield
         public async Task<IActionResult> UpdatePaymentRequest(string storeId,
             string paymentRequestId, [FromBody] UpdatePaymentRequestRequest request)
         {
-            var validationResult = Validate(request);
+            var validationResult = await Validate(paymentRequestId, request);
             if (validationResult != null)
             {
                 return validationResult;
@@ -196,11 +201,22 @@ namespace BTCPayServer.Controllers.Greenfield
 
             return Ok(FromModel(await _paymentRequestRepository.CreateOrUpdatePaymentRequest(updatedPr)));
         }
+        private string GetUserId() => _userManager.GetUserId(User);
 
-        private IActionResult Validate(PaymentRequestBaseData data)
+        private async Task<IActionResult> Validate(string id, PaymentRequestBaseData data)
         {
             if (data is null)
                 return BadRequest();
+
+            if (id != null)
+            {
+                var pr = await this.PaymentRequestService.GetPaymentRequest(id, GetUserId());
+                if (pr.Amount != data.Amount)
+                {
+                    if (pr.Invoices.Any())
+                        ModelState.AddModelError(nameof(data.Amount), "Amount and currency are not editable once payment request has invoices");
+                }
+            }
             if (data.Amount <= 0)
             {
                 ModelState.AddModelError(nameof(data.Amount), "Please provide an amount greater than 0");

BTCPayServer/Controllers/UIPaymentRequestController.cs

@@ -93,7 +93,7 @@ namespace BTCPayServer.Controllers
         }
 
         [HttpGet("/stores/{storeId}/payment-requests/edit/{payReqId?}")]
-        public IActionResult EditPaymentRequest(string storeId, string payReqId)
+        public async Task<IActionResult> EditPaymentRequest(string storeId, string payReqId)
         {
             var store = GetCurrentStore();
             var paymentRequest = GetCurrentPaymentRequest();
@@ -102,9 +102,11 @@ namespace BTCPayServer.Controllers
                 return NotFound();
             }
 
+            var prInvoices = payReqId is null ? null : (await _PaymentRequestService.GetPaymentRequest(payReqId, GetUserId())).Invoices;
             var vm = new UpdatePaymentRequestViewModel(paymentRequest)
             {
-                StoreId = store.Id
+                StoreId = store.Id,
+                AmountAndCurrencyEditable = payReqId is null || !prInvoices.Any()
             };
 
             vm.Currency ??= store.GetStoreBlob().DefaultCurrency;
@@ -131,17 +133,24 @@ namespace BTCPayServer.Controllers
             {
                 ModelState.AddModelError(string.Empty, "You cannot edit an archived payment request.");
             }
+            var data = paymentRequest ?? new PaymentRequestData();
+            data.StoreDataId = viewModel.StoreId;
+            data.Archived = viewModel.Archived;
+            var blob = data.GetBlob();
+
+            if (blob.Amount != viewModel.Amount && payReqId != null)
+            {
+                var prInvoices = (await _PaymentRequestService.GetPaymentRequest(payReqId, GetUserId())).Invoices;
+                if (prInvoices.Any())
+                    ModelState.AddModelError(nameof(viewModel.Amount), "Amount and currency are not editable once payment request has invoices");
+            }
 
             if (!ModelState.IsValid)
             {
                 return View(nameof(EditPaymentRequest), viewModel);
             }
 
-            var data = paymentRequest ?? new PaymentRequestData();
+            
-            data.StoreDataId = viewModel.StoreId;
-            data.Archived = viewModel.Archived;
-
-            var blob = data.GetBlob();
             blob.Title = viewModel.Title;
             blob.Email = viewModel.Email;
             blob.Description = viewModel.Description;
@@ -343,10 +352,10 @@ namespace BTCPayServer.Controllers
         }
 
         [HttpGet("{payReqId}/clone")]
-        public IActionResult ClonePaymentRequest(string payReqId)
+        public async Task<IActionResult> ClonePaymentRequest(string payReqId)
         {
             var store = GetCurrentStore();
-            var result = EditPaymentRequest(store.Id, payReqId);
+            var result = await EditPaymentRequest(store.Id, payReqId);
             if (result is ViewResult viewResult)
             {
                 var model = (UpdatePaymentRequestViewModel)viewResult.Model;
@@ -364,7 +373,7 @@ namespace BTCPayServer.Controllers
         public async Task<IActionResult> TogglePaymentRequestArchival(string payReqId)
         {
             var store = GetCurrentStore();
-            var result = EditPaymentRequest(store.Id, payReqId);
+            var result = await EditPaymentRequest(store.Id, payReqId);
             if (result is ViewResult viewResult)
             {
                 var model = (UpdatePaymentRequestViewModel)viewResult.Model;

BTCPayServer/Models/PaymentRequestViewModels/ListPaymentRequestsViewModel.cs

@@ -87,6 +87,7 @@ namespace BTCPayServer.Models.PaymentRequestViewModels
         public bool AllowCustomPaymentAmounts { get; set; }
 
         public Dictionary<string, object> FormResponse { get; set; }
+        public bool AmountAndCurrencyEditable { get; set; } = true;
     }
 
     public class ViewPaymentRequestViewModel

BTCPayServer/PaymentRequest/PaymentRequestService.cs

@@ -72,7 +72,7 @@ namespace BTCPayServer.PaymentRequest
 
         public async Task<ViewPaymentRequestViewModel> GetPaymentRequest(string id, string userId = null)
         {
-            var pr = await _PaymentRequestRepository.FindPaymentRequest(id, null);
+            var pr = await _PaymentRequestRepository.FindPaymentRequest(id, userId);
             if (pr == null)
             {
                 return null;

BTCPayServer/Views/UIPaymentRequest/EditPaymentRequest.cshtml

@@ -49,12 +49,16 @@
             <div class="d-flex justify-content-between">
                 <div class="form-group flex-fill me-4">
                     <label asp-for="Amount" class="form-label" data-required></label>
-                    <input type="number" inputmode="decimal" step="any" asp-for="Amount" class="form-control" required />
+                    <input type="number" inputmode="decimal" step="any" asp-for="Amount" class="form-control" required disabled="@(!Model.AmountAndCurrencyEditable)" />
                     <span asp-validation-for="Amount" class="text-danger"></span>
+                    @if (!Model.AmountAndCurrencyEditable)
+                    {
+                        <p class="text-warning mb-0 mt-2">Amount and currency are not editable once payment request has invoices</p>
+                    }
                 </div>
                 <div class="form-group">
                     <label asp-for="Currency" class="form-label"></label>
-                    <input asp-for="Currency" class="form-control w-auto" currency-selection />
+                    <input asp-for="Currency" class="form-control w-auto" currency-selection disabled="@(!Model.AmountAndCurrencyEditable)" />
                     <span asp-validation-for="Currency" class="text-danger"></span>
                 </div>
             </div>