Fix deprecation custom message issues (#7284)

Author: Scott Bommarito

src/NuGetGallery/Controllers/ManageDeprecationJsonApiController.cs

@@ -9,14 +9,16 @@
 using System.Web.Mvc;
 using NuGet.Services.Entities;
 using NuGet.Versioning;
-using NuGetGallery.Auditing;
 using NuGetGallery.Filters;
+using NuGetGallery.RequestModels;
 
 namespace NuGetGallery
 {
     public partial class ManageDeprecationJsonApiController
         : AppController
     {
+        private const int MaxCustomMessageLength = 1000;
+
         private readonly IPackageService _packageService;
         private readonly IPackageDeprecationService _deprecationService;
         private readonly IFeatureFlagService _featureFlagService;
@@ -50,28 +52,22 @@ public virtual JsonResult GetAlternatePackageVersions(string id)
         [RequiresAccountConfirmation("deprecate a package")]
         [ValidateAntiForgeryToken]
         public virtual async Task<JsonResult> Deprecate(
-            string id,
-            IEnumerable<string> versions,
-            bool isLegacy,
-            bool hasCriticalBugs,
-            bool isOther,
-            string alternatePackageId,
-            string alternatePackageVersion,
-            string customMessage)
+            DeprecatePackageRequest request)
         {
             var status = PackageDeprecationStatus.NotDeprecated;
 
-            if (isLegacy)
+            if (request.IsLegacy)
             {
                 status |= PackageDeprecationStatus.Legacy;
             }
 
-            if (hasCriticalBugs)
+            if (request.HasCriticalBugs)
             {
                 status |= PackageDeprecationStatus.CriticalBugs;
             }
 
-            if (isOther)
+            var customMessage = request.CustomMessage;
+            if (request.IsOther)
             {
                 if (string.IsNullOrWhiteSpace(customMessage))
                 {
@@ -81,19 +77,29 @@ public virtual async Task<JsonResult> Deprecate(
                 status |= PackageDeprecationStatus.Other;
             }
 
-            if (versions == null || !versions.Any())
+            if (customMessage != null)
+            {
+                if (customMessage.Length > MaxCustomMessageLength)
+                {
+                    return DeprecateErrorResponse(
+                        HttpStatusCode.BadRequest,
+                        string.Format(Strings.DeprecatePackage_CustomMessageTooLong, MaxCustomMessageLength));
+                }
+            }
+
+            if (request.Versions == null || !request.Versions.Any())
             {
                 return DeprecateErrorResponse(HttpStatusCode.BadRequest, Strings.DeprecatePackage_NoVersions);
             }
 
-            var packages = _packageService.FindPackagesById(id, PackageDeprecationFieldsToInclude.DeprecationAndRelationships);
+            var packages = _packageService.FindPackagesById(request.Id, PackageDeprecationFieldsToInclude.DeprecationAndRelationships);
             var registration = packages.FirstOrDefault()?.PackageRegistration;
             if (registration == null)
             {
                 // This should only happen if someone hacks the form or if the package is deleted while the user is filling out the form.
                 return DeprecateErrorResponse(
                     HttpStatusCode.NotFound,
-                    string.Format(Strings.DeprecatePackage_MissingRegistration, id));
+                    string.Format(Strings.DeprecatePackage_MissingRegistration, request.Id));
             }
 
             var currentUser = GetCurrentUser();
@@ -111,37 +117,37 @@ public virtual async Task<JsonResult> Deprecate(
             {
                 return DeprecateErrorResponse(
                     HttpStatusCode.Forbidden,
-                    string.Format(Strings.DeprecatePackage_Locked, id));
+                    string.Format(Strings.DeprecatePackage_Locked, request.Id));
             }
 
             PackageRegistration alternatePackageRegistration = null;
             Package alternatePackage = null;
-            if (!string.IsNullOrWhiteSpace(alternatePackageId))
+            if (!string.IsNullOrWhiteSpace(request.AlternatePackageId))
             {
-                if (!string.IsNullOrWhiteSpace(alternatePackageVersion))
+                if (!string.IsNullOrWhiteSpace(request.AlternatePackageVersion))
                 {
-                    alternatePackage = _packageService.FindPackageByIdAndVersionStrict(alternatePackageId, alternatePackageVersion);
+                    alternatePackage = _packageService.FindPackageByIdAndVersionStrict(request.AlternatePackageId, request.AlternatePackageVersion);
                     if (alternatePackage == null)
                     {
                         return DeprecateErrorResponse(
                             HttpStatusCode.NotFound,
-                            string.Format(Strings.DeprecatePackage_NoAlternatePackage, alternatePackageId, alternatePackageVersion));
+                            string.Format(Strings.DeprecatePackage_NoAlternatePackage, request.AlternatePackageId, request.AlternatePackageVersion));
                     }
                 }
                 else
                 {
-                    alternatePackageRegistration = _packageService.FindPackageRegistrationById(alternatePackageId);
+                    alternatePackageRegistration = _packageService.FindPackageRegistrationById(request.AlternatePackageId);
                     if (alternatePackageRegistration == null)
                     {
                         return DeprecateErrorResponse(
                             HttpStatusCode.NotFound,
-                            string.Format(Strings.DeprecatePackage_NoAlternatePackageRegistration, alternatePackageId));
+                            string.Format(Strings.DeprecatePackage_NoAlternatePackageRegistration, request.AlternatePackageId));
                     }
                 }
             }
 
             var packagesToUpdate = new List<Package>();
-            foreach (var version in versions)
+            foreach (var version in request.Versions)
             {
                 var normalizedVersion = NuGetVersionFormatter.Normalize(version);
                 var package = packages.SingleOrDefault(v => v.NormalizedVersion == normalizedVersion);
@@ -150,7 +156,7 @@ public virtual async Task<JsonResult> Deprecate(
                     // This should only happen if someone hacks the form or if a version of the package is deleted while the user is filling out the form.
                     return DeprecateErrorResponse(
                         HttpStatusCode.NotFound,
-                        string.Format(Strings.DeprecatePackage_MissingVersion, id));
+                        string.Format(Strings.DeprecatePackage_MissingVersion, request.Id));
                 }
                 else
                 {

src/NuGetGallery/NuGetGallery.csproj

@@ -252,6 +252,7 @@
     <Compile Include="Migrations\201905071526573_DevelopmentDependencyMetadata.Designer.cs">
       <DependentUpon>201905071526573_DevelopmentDependencyMetadata.cs</DependentUpon>
     </Compile>
+    <Compile Include="RequestModels\DeprecatePackageRequest.cs" />
     <Compile Include="Services\IPackageDeprecationService.cs" />
     <Compile Include="Queries\AutocompleteDatabasePackageIdsQuery.cs" />
     <Compile Include="Queries\AutocompleteDatabasePackageVersionsQuery.cs" />

src/NuGetGallery/RequestModels/DeprecatePackageRequest.cs

@@ -0,0 +1,22 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Web.Mvc;
+
+namespace NuGetGallery.RequestModels
+{
+    public class DeprecatePackageRequest
+    {
+        public string Id { get; set; }
+        public IEnumerable<string> Versions { get; set; }
+        public bool IsLegacy { get; set; }
+        public bool HasCriticalBugs { get; set; }
+        public bool IsOther { get; set; }
+        public string AlternatePackageId { get; set; }
+        public string AlternatePackageVersion { get; set; }
+
+        [AllowHtml]
+        public string CustomMessage { get; set; }
+    }
+}

src/NuGetGallery/Strings.Designer.cs

@@ -1121,4 +1121,7 @@ The {1} Team</value>
   <data name="UploadPackage_EmbeddedIconNotAccepted" xml:space="preserve">
     <value>The &lt;icon&gt; element is not currently supported.</value>
   </data>
+  <data name="DeprecatePackage_CustomMessageTooLong" xml:space="preserve">
+    <value>Your custom message is too long. It must be under {0} characters.</value>
+  </data>
 </root>

src/NuGetGallery/Strings.resx

@@ -63,7 +63,7 @@
             @if (!string.IsNullOrEmpty(Model.CustomMessage))
             {
                 <b>Additional Details</b>
-                <p>@Model.CustomMessage</p>
+                <p>@Html.PreFormattedText(Model.CustomMessage)</p>
             }
         </div>
     </div>