Upload page UX polish (#9281)

lyndaidaii · web-flow · commit a3ccd3943009 · 2022-10-31T10:37:05.000-07:00
diff --git a/src/AccountDeleter/EmptyFeatureFlagService.cs b/src/AccountDeleter/EmptyFeatureFlagService.cs
@@ -290,5 +290,10 @@ public bool ProxyGravatarEnSubdomain()
         {
             throw new NotImplementedException();
         }
+
+        public bool IsDisplayUploadWarningV2Enabled(User user)
+        {
+            throw new NotImplementedException();
+        }
     }
 }
diff --git a/src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs b/src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs
@@ -130,6 +130,11 @@ public bool IsMarkdigMdSyntaxHighlightEnabled()
             throw new NotImplementedException();
         }
 
+        public bool IsDisplayUploadWarningV2Enabled(User user)
+        {
+            throw new NotImplementedException();
+        }
+
         public bool IsODataDatabaseReadOnlyEnabled()
         {
             throw new NotImplementedException();
diff --git a/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs b/src/NuGetGallery.Services/Configuration/FeatureFlagService.cs
@@ -47,6 +47,7 @@ public class FeatureFlagService : IFeatureFlagService
         private const string LicenseMdRenderingFlightName = GalleryPrefix + "LicenseMdRendering";
         private const string MarkdigMdRenderingFlightName = GalleryPrefix + "MarkdigMdRendering";
         private const string MarkdigMdSyntaxHighlightFlightName = GalleryPrefix + "MarkdigMdSyntaxHighlight";
+        private const string DisplayUploadWarningV2FlightName = GalleryPrefix + "DisplayUploadWarningV2";
         private const string DeletePackageApiFlightName = GalleryPrefix + "DeletePackageApi";
         private const string ImageAllowlistFlightName = GalleryPrefix + "ImageAllowlist";
         private const string DisplayBannerFlightName = GalleryPrefix + "Banner";
@@ -341,6 +342,11 @@ public bool IsMarkdigMdSyntaxHighlightEnabled()
             return _client.IsEnabled(MarkdigMdSyntaxHighlightFlightName, defaultValue: false);
         }
 
+        public bool IsDisplayUploadWarningV2Enabled(User user)
+        {
+            return _client.IsEnabled(DisplayUploadWarningV2FlightName, user, defaultValue: false);
+        }
+
         public bool IsDeletePackageApiEnabled(User user)
         {
             return _client.IsEnabled(DeletePackageApiFlightName, user, defaultValue: false);
diff --git a/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs b/src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs
@@ -268,6 +268,11 @@ public interface IFeatureFlagService
         /// </summary>
         bool IsMarkdigMdSyntaxHighlightEnabled();
 
+        /// <summary>
+        /// Whether the new warning of the verfiy metadata when upload package is enabled.
+        /// </summary>
+        bool IsDisplayUploadWarningV2Enabled(User user);
+
         /// <summary>
         /// Whether or not the user can delete a package through the API.
         /// </summary>
diff --git a/src/NuGetGallery/App_Data/Files/Content/flags.json b/src/NuGetGallery/App_Data/Files/Content/flags.json
@@ -108,6 +108,12 @@
       "SiteAdmins": false,
       "Accounts": [],
       "Domains": []
+    },
+    "NuGetGallery.DisplayUploadWarningV2": {
+      "All": true,
+      "SiteAdmins": false,
+      "Accounts": [],
+      "Domains": []
     }
   }
 }
diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs
@@ -375,6 +375,7 @@ private async Task<ActionResult> UploadPackageInternal(SubmitPackageRequest mode
             verifyRequest.LicenseExpressionSegments = GetLicenseExpressionSegmentsOrNull(packageMetadata.LicenseMetadata);
             verifyRequest.ReadmeFileContents = await GetReadmeFileContentsOrNullAsync(packageMetadata, packageArchiveReader);
             verifyRequest.IsMarkdigMdSyntaxHighlightEnabled = _featureFlagService.IsMarkdigMdSyntaxHighlightEnabled();
+            verifyRequest.IsDisplayUploadWarningV2Enabled = _featureFlagService.IsDisplayUploadWarningV2Enabled(currentUser);
 
             model.InProgressUpload = verifyRequest;
             return View(model);
@@ -658,6 +659,7 @@ private async Task<JsonResult> GetVerifyPackageView(User currentUser,
             model.IsSymbolsPackage = isSymbolsPackageUpload;
             model.HasExistingAvailableSymbols = hasExistingSymbolsPackageAvailable;
             model.IsMarkdigMdSyntaxHighlightEnabled = _featureFlagService.IsMarkdigMdSyntaxHighlightEnabled();
+            model.IsDisplayUploadWarningV2Enabled = _featureFlagService.IsDisplayUploadWarningV2Enabled(currentUser);
             model.Warnings.AddRange(packageContentData.Warnings.Select(w => new JsonValidationMessage(w)));
             model.LicenseFileContents = packageContentData.LicenseFileContents;
 
diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj
@@ -333,6 +333,8 @@
     <Compile Include="Modules\CookieComplianceHttpModule.cs" />
     <Compile Include="RequestModels\DeletePackagesApiRequest.cs" />
     <Compile Include="RequestModels\UpdateListedRequest.cs" />
+    <Compile Include="Services\MissingLicenseValidationMessageV2.cs" />
+    <Compile Include="Services\UploadPackageMissingReadme.cs" />
     <Compile Include="Services\MissingLicenseValidationMessage.cs" />
     <Compile Include="Services\UploadPackageIdNamespaceConflict.cs" />
     <Compile Include="Services\ImageDomainValidator.cs" />
@@ -635,6 +637,11 @@
     <Compile Include="Services\TyposquattingStringNormalization.cs" />
     <Compile Include="Services\UpdateDeprecationError.cs" />
     <Compile Include="Services\ValidationService.cs" />
+    <Compile Include="Strings.Designer.cs">
+      <AutoGen>True</AutoGen>
+      <DesignTime>True</DesignTime>
+      <DependentUpon>Strings.resx</DependentUpon>
+    </Compile>
     <Compile Include="Telemetry\ClientInformationTelemetryEnricher.cs" />
     <Compile Include="Telemetry\ClientTelemetryPIIProcessor.cs" />
     <Compile Include="Telemetry\CustomerResourceIdEnricher.cs" />
@@ -1483,11 +1490,6 @@
     <Compile Include="Services\IFormsAuthenticationService.cs" />
     <Compile Include="Services\IPackageFileService.cs" />
     <Compile Include="RequestModels\DisplayPackageRequest.cs" />
-    <Compile Include="Strings.Designer.cs">
-      <DependentUpon>Strings.resx</DependentUpon>
-      <AutoGen>True</AutoGen>
-      <DesignTime>True</DesignTime>
-    </Compile>
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Properties\AssemblyInfo.*.cs" />
     <Compile Include="RequestModels\SubmitPackageRequest.cs" />
@@ -2058,8 +2060,8 @@
     </EmbeddedResource>
     <EmbeddedResource Include="Strings.resx">
       <Generator>PublicResXFileCodeGenerator</Generator>
-      <LastGenOutput>Strings.Designer.cs</LastGenOutput>
       <SubType>Designer</SubType>
+      <LastGenOutput>Strings.Designer.cs</LastGenOutput>
     </EmbeddedResource>
   </ItemGroup>
   <ItemGroup>
diff --git a/src/NuGetGallery/RequestModels/SubmitPackageRequest.cs b/src/NuGetGallery/RequestModels/SubmitPackageRequest.cs
@@ -14,5 +14,7 @@ public class SubmitPackageRequest
         public bool IsUserLocked { get; set; }
 
         public bool IsMarkdigMdSyntaxHighlightEnabled { get; set; }
+
+        public bool IsDisplayUploadWarningV2Enabled { get; set;  }
     }
 }
diff --git a/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs b/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs
@@ -119,6 +119,7 @@ public VerifyPackageRequest(PackageMetadata packageMetadata, IEnumerable<User> p
         public bool IsSymbolsPackage { get; set; }
         public bool HasExistingAvailableSymbols { get; set; }
         public bool IsMarkdigMdSyntaxHighlightEnabled { get; set; }
+        public bool IsDisplayUploadWarningV2Enabled { get; set; }
 
         public List<JsonValidationMessage> Warnings { get; set; } = new List<JsonValidationMessage>();
 
diff --git a/src/NuGetGallery/Scripts/gallery/async-file-upload.js b/src/NuGetGallery/Scripts/gallery/async-file-upload.js
@@ -303,6 +303,11 @@
                 $("#verify-collapser-container").removeClass("hidden");
                 $("#submit-collapser-container").removeClass("hidden");
 
+                if (model != null && model.IsDisplayUploadWarningV2Enabled) {
+                    $('#upload-package-form').collapse('hide');
+                    $('#warning-container').addClass('hidden');
+                }
+ 
                 window.nuget.configureExpanderHeading("verify-package-section");
                 window.nuget.configureExpanderHeading("submit-package-form");
             }
diff --git a/src/NuGetGallery/Services/MissingLicenseValidationMessageV2.cs b/src/NuGetGallery/Services/MissingLicenseValidationMessageV2.cs
@@ -0,0 +1,38 @@
+// 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;
+using System.Globalization;
+using System.Web;
+
+namespace NuGetGallery
+{
+    /// <summary>
+    /// A warning for packages that are missing licensing metadata.
+    /// </summary>
+    public class MissingLicenseValidationMessageV2 : IValidationMessage
+    {
+        private string DocumentationLink => $"<a href=\"https://aka.ms/nuget/authoring-best-practices#licensing\" aria-label=\"{Strings.UploadPackage_LearnMore_PackagingLicenseV2}\">{Strings.UploadPackage_LearnMore_PackagingLicenseV2}</a>.";
+
+        private readonly string _baseMessage;
+
+        public MissingLicenseValidationMessageV2(string basePlainTextMessage)
+        {
+            if (string.IsNullOrWhiteSpace(basePlainTextMessage))
+            {
+                throw new ArgumentException(
+                    string.Format(CultureInfo.CurrentCulture, Strings.ParameterCannotBeNullOrEmpty, nameof(basePlainTextMessage)), nameof(basePlainTextMessage));
+            }
+
+            _baseMessage = basePlainTextMessage;
+            PlainTextMessage = $"{_baseMessage} {Strings.UploadPackage_LearnMore_PackagingLicenseV2}: https://aka.ms/nuget/authoring-best-practices#licensing.";
+        }
+
+        public string PlainTextMessage { get; }
+
+        public bool HasRawHtmlRepresentation => true;
+
+        public string RawHtmlMessage
+            => Strings.UploadPackage_LicenseShouldBeSpecifiedHtmlV2 + " " + DocumentationLink;
+    }
+}
diff --git a/src/NuGetGallery/Services/PackageMetadataValidationService.cs b/src/NuGetGallery/Services/PackageMetadataValidationService.cs
@@ -152,7 +152,7 @@ public async Task<PackageValidationResult> ValidateMetadataBeforeUploadAsync(
                 return result;
             }
 
-            result = await CheckReadmeMetadataAsync(nuGetPackage, currentUser);
+            result = await CheckReadmeMetadataAsync(nuGetPackage, warnings, currentUser);
             if (result != null)
             {
                 return result;
@@ -211,7 +211,14 @@ private async Task<PackageValidationResult> CheckLicenseMetadataAsync(PackageArc
                     }
                     else
                     {
-                        warnings.Add(new MissingLicenseValidationMessage(Strings.UploadPackage_LicenseShouldBeSpecified));
+                        if (_featureFlagService.IsDisplayUploadWarningV2Enabled(user))
+                        {
+                            warnings.Add(new MissingLicenseValidationMessageV2(Strings.UploadPackage_LicenseShouldBeSpecifiedV2));
+                        }
+                        else
+                        {
+                            warnings.Add(new MissingLicenseValidationMessage(Strings.UploadPackage_LicenseShouldBeSpecified));
+                        }
                     }
                 }
 
@@ -415,13 +422,18 @@ private async Task<PackageValidationResult> CheckIconMetadataAsync(PackageArchiv
             return null;
         }
 
-        private async Task<PackageValidationResult> CheckReadmeMetadataAsync(PackageArchiveReader nuGetPackage, User user)
+        private async Task<PackageValidationResult> CheckReadmeMetadataAsync(PackageArchiveReader nuGetPackage, List<IValidationMessage> warnings, User user)
         {
             var nuspecReader = GetNuspecReader(nuGetPackage);
             var readmeElement = nuspecReader.ReadmeElement;
 
             if (readmeElement == null)
             {
+                if (_featureFlagService.IsDisplayUploadWarningV2Enabled(user))
+                {
+                    warnings.Add(new UploadPackageMissingReadme());
+                }
+
                 return null;
             }
 
diff --git a/src/NuGetGallery/Services/UploadPackageMissingReadme.cs b/src/NuGetGallery/Services/UploadPackageMissingReadme.cs
@@ -0,0 +1,12 @@
+﻿namespace NuGetGallery
+{
+    /// <summary>
+    /// Represents a package ID reservation conflict
+    /// </summary>
+    public class UploadPackageMissingReadme : IValidationMessage
+    {
+        public bool HasRawHtmlRepresentation => true;
+        public string PlainTextMessage => Strings.UploadPackage_MissingReadme;
+        public string RawHtmlMessage => Strings.UploadPackage_MissingReadmeHtml;
+    }
+}
diff --git a/src/NuGetGallery/Strings.Designer.cs b/src/NuGetGallery/Strings.Designer.cs
diff --git a/src/NuGetGallery/Strings.resx b/src/NuGetGallery/Strings.resx
@@ -538,6 +538,12 @@ For more information, please contact '{2}'.</value>
   <data name="ReadMeUrlHostInvalid" xml:space="preserve">
     <value>The Documentation URL must be a raw Markdown file hosted on GitHub.</value>
   </data>
+  <data name="UploadPackage_MissingReadmeHtml" xml:space="preserve">
+    <value>&lt;strong&gt;Readme&lt;/strong&gt; missing.&lt;a href="https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme"&gt; See how to include a readme file within the package&lt;/a&gt;, or add it as you upload.</value>
+  </data>
+  <data name="UploadPackage_MissingReadme" xml:space="preserve">
+    <value>Readme missing. Go to https://learn.microsoft.com/en-us/nuget/create-packages/package-authoring-best-practices#readme learn How to include a readme file within the package.</value>
+  </data>	
   <data name="ReservedNamespace_OwnerAdded" xml:space="preserve">
     <value>The user '{0}' is now an owner of the prefix '{1}'.</value>
   </data>
@@ -1070,11 +1076,17 @@ The {1} Team</value>
   </data>
   <data name="UploadPackage_LearnMore" xml:space="preserve">
     <value>Learn more</value>
+  </data>	
+  <data name="UploadPackage_LicenseShouldBeSpecifiedHtmlV2" xml:space="preserve">
+    <value>&lt;strong&gt;License&lt;/strong&gt; missing.</value>
+  </data>	
+  <data name="UploadPackage_LicenseShouldBeSpecifiedV2" xml:space="preserve">
+    <value>License missing.</value>
   </data>
   <data name="UploadPackage_LicenseShouldBeSpecified" xml:space="preserve">
     <value>All published packages should have license information specified.</value>
   </data>
-  <data name="UploadPackage_NonFsfOrOsiLicense" xml:space="preserve">
+ <data name="UploadPackage_NonFsfOrOsiLicense" xml:space="preserve">
     <value>License expression must only contain licenses that are approved by Open Source Initiative or Free Software Foundation. Unsupported licenses: {0}.</value>
     <comment>{0} is list of the package license ids that are neither OSI nor FSF approved</comment>
   </data>
@@ -1190,6 +1202,9 @@ The {1} Team</value>
   <data name="ReadmeNotEditableWithEmbeddedReadme" xml:space="preserve">
     <value>The readme is not editable with the package has the embedded readme</value>
   </data>
+  <data name="UploadPackage_LearnMore_PackagingLicenseV2" xml:space="preserve">
+    <value>See how to include a license within the package</value>
+  </data>
   <data name="UploadPackage_LearnMore_PackagingLicense" xml:space="preserve">
     <value>Learn more about including a license within the package</value>
   </data>
diff --git a/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml b/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml
diff --git a/src/NuGetGallery/Web.config b/src/NuGetGallery/Web.config
diff --git a/src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs b/src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs
diff --git a/tests/NuGetGallery.Facts/Services/PackageMetadataValidationServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageMetadataValidationServiceFacts.cs

Original file line number	Diff line number	Diff line change
`@@ -290,5 +290,10 @@ public bool ProxyGravatarEnSubdomain()`
`290`	`290`	`{`
`291`	`291`	`throw new NotImplementedException();`
`292`	`292`	`}`
	`293`	`+`
	`294`	`+ public bool IsDisplayUploadWarningV2Enabled(User user)`
	`295`	`+ {`
	`296`	`+ throw new NotImplementedException();`
	`297`	`+ }`
`293`	`298`	`}`
`294`	`299`	`}`
Original file line number	Diff line number	Diff line change
`@@ -130,6 +130,11 @@ public bool IsMarkdigMdSyntaxHighlightEnabled()`
`130`	`130`	`throw new NotImplementedException();`
`131`	`131`	`}`
`132`	`132`
	`133`	`+ public bool IsDisplayUploadWarningV2Enabled(User user)`
	`134`	`+ {`
	`135`	`+ throw new NotImplementedException();`
	`136`	`+ }`
	`137`	`+`
`133`	`138`	`public bool IsODataDatabaseReadOnlyEnabled()`
`134`	`139`	`{`
`135`	`140`	`throw new NotImplementedException();`
Original file line number	Diff line number	Diff line change
`@@ -108,6 +108,12 @@`
`108`	`108`	`"SiteAdmins": false,`
`109`	`109`	`"Accounts": [],`
`110`	`110`	`"Domains": []`
	`111`	`+ },`
	`112`	`+ "NuGetGallery.DisplayUploadWarningV2": {`
	`113`	`+ "All": true,`
	`114`	`+ "SiteAdmins": false,`
	`115`	`+ "Accounts": [],`
	`116`	`+ "Domains": []`
`111`	`117`	`}`
`112`	`118`	`}`
`113`	`119`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,5 +14,7 @@ public class SubmitPackageRequest`
`14`	`14`	`public bool IsUserLocked { get; set; }`
`15`	`15`
`16`	`16`	`public bool IsMarkdigMdSyntaxHighlightEnabled { get; set; }`
	`17`	`+`
	`18`	`+ public bool IsDisplayUploadWarningV2Enabled { get; set; }`
`17`	`19`	`}`
`18`	`20`	`}`