Properly return a 400 JSON result instead of 500 when trying to remove the last owner of a package (#6715)

Author: Scott Bommarito

src/NuGetGallery/Controllers/JsonApiController.cs

@@ -206,7 +206,7 @@ public async Task<JsonResult> RemovePackageOwner(string id, string username)
                 {
                     if (model.Package.Owners.Count == 1 && model.User == model.Package.Owners.Single())
                     {
-                        throw new InvalidOperationException("You can't remove the only owner from a package.");
+                        return Json(new { success = false, message = "You can't remove the only owner from a package." }, JsonRequestBehavior.AllowGet);
                     }
 
                     await _packageOwnershipManagementService.RemovePackageOwnerAsync(model.Package, model.CurrentUser, model.User, commitAsTransaction: true);

tests/NuGetGallery.Facts/Controllers/JsonApiControllerFacts.cs

@@ -588,6 +588,54 @@ public async Task RemovesPackageOwnerRequest(Func<Fakes, User> getCurrentUser, F
                                 false));
                     }
 
+                    [Theory]
+                    [MemberData(nameof(AllCanManagePackageOwnersPairedWithCanBeRemoved_Data))]
+                    public async Task FailsIfAttemptingToRemoveLastOwner(Func<Fakes, User> getCurrentUser, Func<Fakes, User> getUserToRemove)
+                    {
+                        // Arrange
+                        var fakes = Get<Fakes>();
+                        var currentUser = getCurrentUser(fakes);
+                        var userToRemove = getUserToRemove(fakes);
+                        var package = fakes.Package;
+                        var controller = GetController<JsonApiController>();
+                        controller.SetCurrentUser(currentUser);
+
+                        foreach (var owner in package.Owners.Where(o => !o.MatchesUser(userToRemove)).ToList())
+                        {
+                            package.Owners.Remove(owner);
+                        }
+
+                        var packageOwnershipManagementService = GetMock<IPackageOwnershipManagementService>();
+
+                        packageOwnershipManagementService
+                            .Setup(x => x.GetPackageOwnershipRequests(package, null, userToRemove))
+                            .Returns(Enumerable.Empty<PackageOwnerRequest>());
+
+                        // Act
+                        var result = await controller.RemovePackageOwner(package.Id, userToRemove.Username);
+                        dynamic data = result.Data;
+
+                        // Assert
+                        Assert.False(data.success);
+
+                        packageOwnershipManagementService
+                            .Verify(
+                                x => x.RemovePackageOwnerAsync(package, currentUser, userToRemove, It.IsAny<bool>()),
+                                Times.Never());
+
+                        GetMock<IMessageService>()
+                            .Verify(
+                                x => x.SendMessageAsync(
+                                    It.Is<PackageOwnerRemovedMessage>(
+                                        msg =>
+                                        msg.FromUser == currentUser
+                                        && msg.ToUser == userToRemove
+                                        && msg.PackageRegistration == package),
+                                    false,
+                                    false),
+                                Times.Never());
+                    }
+
                     [Theory]
                     [MemberData(nameof(AllCanManagePackageOwnersPairedWithCanBeRemoved_Data))]
                     public async Task RemovesExistingOwner(Func<Fakes, User> getCurrentUser, Func<Fakes, User> getUserToRemove)