diff --git a/src/DevBetterWeb.Web/Pages/Admin/User.cshtml b/src/DevBetterWeb.Web/Pages/Admin/User.cshtml index 012b135c..767fb0e1 100644 --- a/src/DevBetterWeb.Web/Pages/Admin/User.cshtml +++ b/src/DevBetterWeb.Web/Pages/Admin/User.cshtml @@ -119,7 +119,7 @@
- +
Personal @@ -128,6 +128,7 @@
+
@@ -213,7 +214,7 @@
- +
Links diff --git a/src/DevBetterWeb.Web/Pages/Admin/User.cshtml.cs b/src/DevBetterWeb.Web/Pages/Admin/User.cshtml.cs index bf9e48aa..33f33c2c 100644 --- a/src/DevBetterWeb.Web/Pages/Admin/User.cshtml.cs +++ b/src/DevBetterWeb.Web/Pages/Admin/User.cshtml.cs @@ -69,6 +69,9 @@ public UserModel(ILogger logger, } + // Set from the route before anything else loads so the page's forms always post a userId, + // even if a later part of OnGetAsync (e.g. the Stripe invoice lookup) fails. + public string UserId { get; set; } = string.Empty; public IdentityUser? IdentityUser { get; set; } public List Roles { get; set; } = new List(); public List RolesNotAssignedToUser { get; set; } = new List(); @@ -81,6 +84,8 @@ public UserModel(ILogger logger, public async Task OnGetAsync(string userId) { + UserId = userId; + try { if (string.IsNullOrEmpty(userId)) @@ -95,8 +100,19 @@ public async Task OnGetAsync(string userId) return BadRequest(); } - var invoices = await _invoiceHandlerListService.SearchByEmailAsync(currentUser!.Email!); - Invoices = _mapper.Map>(invoices); + try + { + var invoices = await _invoiceHandlerListService.SearchByEmailAsync(currentUser!.Email!); + Invoices = _mapper.Map>(invoices); + } + catch (InvalidOperationException exception) + { + _logger.LogError(exception, "Unable to load Stripe invoices for userId {UserId}", userId); + } + catch (DbUpdateException exception) + { + _logger.LogError(exception, "Unable to load Stripe invoices for userId {UserId}", SanitizeForLog(userId)); + } var roles = await _roleManager.Roles.ToListAsync(); @@ -249,6 +265,11 @@ public async Task OnPostEditSubscriptionAsync(string userId, int return RedirectToPage("./User", new { userId = userId }); } + private static string SanitizeForLog(string? value) + { + return value?.Replace("\r", string.Empty).Replace("\n", string.Empty) ?? string.Empty; + } + public async Task OnPostUpdateEmailConfirmationAsync(string userId, bool isEmailConfirmed) { await _userEmailConfirmationService.UpdateUserEmailConfirmationAsync(userId, !isEmailConfirmed); @@ -258,21 +279,38 @@ public async Task OnPostUpdateEmailConfirmationAsync(string userI public async Task OnPostUpdatePersonalInfoAsync(string userId) { + // Admins often edit members who have not entered a shipping address yet, so the address + // fields are only required once any of them is filled in. + bool hasShippingAddress = HasAnyShippingAddressField(UserPersonalUpdateModel); + if (!hasShippingAddress) + { + foreach (var field in ShippingAddressFields) + { + ModelState.Remove($"{nameof(UserPersonalUpdateModel)}.{field}"); + } + } + if (!ModelState.IsValid) { - ModelState.AddModelError("InvalidUserId", "Bad Data"); - return BadRequest(ModelState); + return await RedisplayPersonalInfoFormAsync(userId); } var spec = new MemberByUserIdSpec(userId); var member = await _memberRepository.FirstOrDefaultAsync(spec); - if (member is null) throw new MemberNotFoundException(userId); + if (member is null) + { + ModelState.AddModelError(string.Empty, $"No member record exists for user {userId}."); + return await RedisplayPersonalInfoFormAsync(userId); + } member.UpdateName(UserPersonalUpdateModel.FirstName, UserPersonalUpdateModel.LastName, false); member.UpdatePEInfo(UserPersonalUpdateModel.PEFriendCode, UserPersonalUpdateModel.PEUsername, false); member.UpdateAboutInfo(UserPersonalUpdateModel.AboutInfo, false); member.UpdateAddress(UserPersonalUpdateModel.Address, false); - member.UpdateShippingAddress(UserPersonalUpdateModel.Address!, UserPersonalUpdateModel.City!, UserPersonalUpdateModel.State!, UserPersonalUpdateModel.PostalCode!, UserPersonalUpdateModel.Country!, false); + if (hasShippingAddress) + { + member.UpdateShippingAddress(UserPersonalUpdateModel.Address!, UserPersonalUpdateModel.City!, UserPersonalUpdateModel.State!, UserPersonalUpdateModel.PostalCode!, UserPersonalUpdateModel.Country!, false); + } member.UpdateDiscord(UserPersonalUpdateModel.DiscordUsername, false); member.UpdateEmail(UserPersonalUpdateModel.Email, false); @@ -287,6 +325,32 @@ public async Task OnPostUpdatePersonalInfoAsync(string userId) return RedirectToPage("./User", new { userId }); } + + private static readonly string[] ShippingAddressFields = + { + nameof(UserPersonalUpdateModel.Address), + nameof(UserPersonalUpdateModel.City), + nameof(UserPersonalUpdateModel.State), + nameof(UserPersonalUpdateModel.Country), + nameof(UserPersonalUpdateModel.PostalCode), + }; + + private static bool HasAnyShippingAddressField(UserPersonalUpdateModel model) => + !string.IsNullOrWhiteSpace(model.Address) || + !string.IsNullOrWhiteSpace(model.City) || + !string.IsNullOrWhiteSpace(model.State) || + !string.IsNullOrWhiteSpace(model.Country) || + !string.IsNullOrWhiteSpace(model.PostalCode); + + // Reloads the page data but keeps the admin's submitted values so validation errors show next to the fields. + private async Task RedisplayPersonalInfoFormAsync(string userId) + { + var submitted = UserPersonalUpdateModel; + var result = await OnGetAsync(userId); + UserPersonalUpdateModel = submitted; + return result; + } + public async Task OnPostUpdateLinksAsync(string userId) { var spec = new MemberByUserIdSpec(userId); diff --git a/tests/DevBetterWeb.Tests/Pages/AdminUserModelTests/OnPostUpdatePersonalInfoAsync.cs b/tests/DevBetterWeb.Tests/Pages/AdminUserModelTests/OnPostUpdatePersonalInfoAsync.cs new file mode 100644 index 00000000..73aee4f7 --- /dev/null +++ b/tests/DevBetterWeb.Tests/Pages/AdminUserModelTests/OnPostUpdatePersonalInfoAsync.cs @@ -0,0 +1,127 @@ +using System.Threading; +using System.Threading.Tasks; +using AutoMapper; +using DevBetterWeb.Core.Entities; +using DevBetterWeb.Core.Interfaces; +using DevBetterWeb.Core.Specs; +using DevBetterWeb.Infrastructure.Identity.Data; +using DevBetterWeb.Infrastructure.Interfaces; +using DevBetterWeb.Web.Pages.Admin; +using DevBetterWeb.Web.Pages.User; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Xunit; + +namespace DevBetterWeb.Tests.Pages.AdminUserModelTests; + +public class OnPostUpdatePersonalInfoAsync +{ + private const string UserId = "user-123"; + + private readonly IRepository _memberRepository = Substitute.For>(); + private readonly UserManager _userManager = UserManagerHelpers.CreateSubstitute(); + private readonly Member _member = MemberHelpers.CreateWithInternalConstructor(); + private readonly UserModel _pageModel; + + public OnPostUpdatePersonalInfoAsync() + { + _memberRepository.FirstOrDefaultAsync(Arg.Any(), Arg.Any()) + .Returns(_member); + _userManager.FindByIdAsync(UserId).Returns(new ApplicationUser { Id = UserId }); + + var roleManager = Substitute.For>( + Substitute.For>(), null!, null!, null!, null!); + + _pageModel = new UserModel(NullLogger.Instance, + _userManager, + roleManager, + Substitute.For(), + Substitute.For(), + _memberRepository, + Substitute.For>(), + Substitute.For>(), + Substitute.For(), + Substitute.For(), + Substitute.For()); + _pageModel.PageContext = new PageContext { HttpContext = new DefaultHttpContext() }; + } + + private void GivenBindingErrorsForEmptyAddressFields() + { + // Mirrors what model binding reports for the [Required] address fields when they are left blank. + foreach (var field in new[] { "Address", "City", "Country", "PostalCode" }) + { + _pageModel.ModelState.AddModelError($"UserPersonalUpdateModel.{field}", $"The {field} field is required."); + } + } + + [Fact] + public async Task SavesNameAndEmailGivenNoShippingAddress() + { + _pageModel.UserPersonalUpdateModel = new UserPersonalUpdateModel + { + FirstName = "Kajan", + LastName = "Smith", + Email = "kajan@example.com" + }; + GivenBindingErrorsForEmptyAddressFields(); + + var result = await _pageModel.OnPostUpdatePersonalInfoAsync(UserId); + + Assert.IsType(result); + Assert.Equal("Kajan", _member.FirstName); + Assert.Equal("kajan@example.com", _member.Email); + Assert.Null(_member.ShippingAddress); + await _memberRepository.Received(1).UpdateAsync(_member, Arg.Any()); + await _userManager.Received(1).UpdateAsync(Arg.Is(u => u.Email == "kajan@example.com")); + } + + [Fact] + public async Task RedisplaysFormWithSubmittedValuesGivenPartialShippingAddress() + { + _pageModel.UserPersonalUpdateModel = new UserPersonalUpdateModel + { + FirstName = "Kajan", + LastName = "Smith", + City = "Vavuniya" + }; + GivenBindingErrorsForEmptyAddressFields(); + + var result = await _pageModel.OnPostUpdatePersonalInfoAsync(UserId); + + Assert.IsType(result); + Assert.False(_pageModel.ModelState.IsValid); + Assert.Equal("Vavuniya", _pageModel.UserPersonalUpdateModel.City); + Assert.Equal(UserId, _pageModel.UserId); + await _memberRepository.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + } + + [Fact] + public async Task RedisplaysFormInsteadOfBadRequestGivenMissingLastName() + { + _pageModel.UserPersonalUpdateModel = new UserPersonalUpdateModel { FirstName = "Kajan" }; + _pageModel.ModelState.AddModelError("UserPersonalUpdateModel.LastName", "The LastName field is required."); + + var result = await _pageModel.OnPostUpdatePersonalInfoAsync(UserId); + + Assert.IsType(result); + await _memberRepository.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default); + } + + [Fact] + public async Task RedisplaysFormWithErrorGivenNoMemberRecord() + { + _memberRepository.FirstOrDefaultAsync(Arg.Any(), Arg.Any()) + .Returns(null); + _pageModel.UserPersonalUpdateModel = new UserPersonalUpdateModel { FirstName = "Kajan", LastName = "Smith" }; + + var result = await _pageModel.OnPostUpdatePersonalInfoAsync(UserId); + + Assert.IsType(result); + Assert.False(_pageModel.ModelState.IsValid); + } +}