Polish upload UI for license html file (#8735)

lyndaidaii · web-flow · commit 9dd5b06a539b · 2021-08-31T15:13:48.000-07:00
* polish upload for license html file
diff --git a/src/Bootstrap/dist/css/bootstrap-theme.css b/src/Bootstrap/dist/css/bootstrap-theme.css
diff --git a/src/Bootstrap/less/theme/common-licenses.less b/src/Bootstrap/less/theme/common-licenses.less
@@ -4,7 +4,14 @@
         word-break: normal;
     }
 
-    .custom-license-container {
+    .custom-license-container, .license-file-html-container{
+        display: block;
+        padding: 10.5px;
+        background-color: @pre-bg;
+        border: 1px solid #ccc;
+        word-break: normal;
         margin-bottom: @default-margin-bottom;
+        overflow: auto;
+        max-height: 450px;
     }
 }
diff --git a/src/NuGetGallery/App_Code/ViewHelpers.cshtml b/src/NuGetGallery/App_Code/ViewHelpers.cshtml
@@ -81,6 +81,23 @@
     )
 }
 
+@helper AlertImageSourceDisallowed()
+{
+    @AlertWarning(
+    @<text>
+        Some images are not displayed as they are not from <a href='https://aka.ms/nuget-org-readme#allowed-domains-for-images-and-badges'>trusted domains</a>.
+    </text>
+    )
+}
+
+@helper AlertImagesRewritten()
+{
+    @AlertWarning(
+    @<text>
+        This documentation had some images automatically rewritten to use secure links and may be broken.
+    </text>)
+}
+
 @helper ErrorPage(UrlHelper url, System.Web.Mvc.HtmlHelper html, string errorNumber, string errorName, Func<MvcHtmlString, HelperResult> errorTextMain, Func<MvcHtmlString, HelperResult> errorTextSub = null)
 {
     <section role="main" class="container main-container">
diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs
@@ -658,7 +658,7 @@ private async Task<JsonResult> GetVerifyPackageView(User currentUser,
                 packageMetadata.LicenseMetadata?.Type == LicenseType.File &&
                 Path.GetExtension(license).Equals(ServicesConstants.MarkdownFileExtension, StringComparison.InvariantCulture))
             {
-                model.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(packageContentData.LicenseFileContents, incrementHeadersBy: 2)?.Content;
+                model.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(packageContentData.LicenseFileContents, incrementHeadersBy: 2);
             }
 
             model.LicenseExpressionSegments = packageContentData.LicenseExpressionSegments;
diff --git a/src/NuGetGallery/Helpers/ViewModelExtensions/DisplayLicenseViewModelFactory.cs b/src/NuGetGallery/Helpers/ViewModelExtensions/DisplayLicenseViewModelFactory.cs
@@ -49,6 +49,7 @@ private DisplayLicenseViewModel SetupInternal(
             string licenseFileContents,
             User currentUser)
         {
+            viewModel.CanDisplayPrivateMetadata = CanPerformAction(currentUser, package, ActionsRequiringPermissions.DisplayPrivatePackageMetadata);
             viewModel.EmbeddedLicenseType = package.EmbeddedLicenseType;
             viewModel.LicenseExpression = package.LicenseExpression;
             if (PackageHelper.TryPrepareUrlForRendering(package.LicenseUrl, out string licenseUrl))
@@ -68,10 +69,15 @@ private DisplayLicenseViewModel SetupInternal(
                 package.EmbeddedLicenseType == EmbeddedLicenseFileType.Markdown && 
                 licenseFileContents != null)
             {
-                viewModel.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(licenseFileContents)?.Content;
+                viewModel.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(licenseFileContents);
             }
 
             return viewModel;
         }
+
+        private static bool CanPerformAction(User currentUser, Package package, ActionRequiringPackagePermissions action)
+        {
+            return action.CheckPermissionsOnBehalfOfAnyAccount(currentUser, package) == PermissionsCheckResult.Allowed;
+        }
     }
 }
diff --git a/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs b/src/NuGetGallery/RequestModels/VerifyPackageRequest.cs
@@ -105,7 +105,7 @@ public VerifyPackageRequest(PackageMetadata packageMetadata, IEnumerable<User> p
         public string LicenseExpression { get; set; }
         public IReadOnlyCollection<CompositeLicenseExpressionSegmentViewModel> LicenseExpressionSegments { get; set; }
         public string LicenseFileContents { get; set; }
-        public string LicenseFileContentsHtml { get; set; }
+        public RenderedMarkdownResult LicenseFileContentsHtml { get; set; }
         public string MinClientVersionDisplay { get; set; }
         public string ProjectUrl { get; set; }
         public string RepositoryUrl { get; set; }
diff --git a/src/NuGetGallery/ViewModels/DisplayLicenseViewModel.cs b/src/NuGetGallery/ViewModels/DisplayLicenseViewModel.cs
@@ -15,6 +15,7 @@ public class DisplayLicenseViewModel : PackageViewModel
         public IReadOnlyCollection<string> LicenseNames { get; set; }
         public IReadOnlyCollection<CompositeLicenseExpressionSegment> LicenseExpressionSegments { get; set; }
         public string LicenseFileContents { get; set; }
-        public string LicenseFileContentsHtml { get; set; }
+        public RenderedMarkdownResult LicenseFileContentsHtml { get; set; }
+        public bool CanDisplayPrivateMetadata { get; set; }
     }
 }
diff --git a/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml b/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml
@@ -522,12 +522,12 @@
                     <div id="readme-container" class="collapse in">
                         @if (Model.ReadMeImagesRewritten && Model.CanDisplayPrivateMetadata)
                         {
-                            @ViewHelpers.AlertWarning(@<text>This documentation had some images automatically rewritten to use secure links and may be broken.</text>);
+                            @ViewHelpers.AlertImagesRewritten();
                         }
 
                         @if (Model.ReadmeImageSourceDisallowed && Model.CanDisplayPrivateMetadata)
                         {
-                            @ViewHelpers.AlertWarning(@<text>Some images are not displayed as they are not from <a href='https://aka.ms/nuget-org-readme#allowed-domains-for-images-and-badges'>trusted domains</a>.</text>);
+                            @ViewHelpers.AlertImageSourceDisallowed();
                         }
                         <div id="readme-less" class="collapse in">
                             @Html.Raw(Model.ReadMeHtml)
diff --git a/src/NuGetGallery/Views/Packages/DisplayPackageV2.cshtml b/src/NuGetGallery/Views/Packages/DisplayPackageV2.cshtml
@@ -561,12 +561,12 @@
                             <div id="readme-container">
                                 @if (Model.ReadMeImagesRewritten && Model.CanDisplayPrivateMetadata)
                                 {
-                                    @ViewHelpers.AlertWarning(@<text>This documentation had some images automatically rewritten to use secure links and may be broken.</text>);
+                                    @ViewHelpers.AlertImagesRewritten();
                                 }
 
                                 @if (Model.ReadmeImageSourceDisallowed && Model.CanDisplayPrivateMetadata)
                                 {
-                                    @ViewHelpers.AlertWarning(@<text>Some images are not displayed as they are not from <a href='https://aka.ms/nuget-org-readme#allowed-domains-for-images-and-badges'>trusted domains</a>.</text>);
+                                    @ViewHelpers.AlertImageSourceDisallowed();
                                 }
 
                                 @Html.Raw(Model.ReadMeHtml)
diff --git a/src/NuGetGallery/Views/Packages/License.cshtml b/src/NuGetGallery/Views/Packages/License.cshtml
@@ -64,13 +64,22 @@
                 }
                 else
                 {
-                    <label>License text</label>
+                    <label>License file</label>
                     <div class="common-licenses">
                         @if (Model.LicenseFileContentsHtml != null)
                         {
-                            @Html.Raw(Model.LicenseFileContentsHtml)
-                        } 
-                        else 
+                            if (Model.LicenseFileContentsHtml.ImageSourceDisallowed && Model.CanDisplayPrivateMetadata)
+                            {
+                                @ViewHelpers.AlertImageSourceDisallowed();
+                            }
+
+                            if (Model.LicenseFileContentsHtml.ImagesRewritten && Model.CanDisplayPrivateMetadata)
+                            {
+                                @ViewHelpers.AlertImagesRewritten();
+                            }
+                            @Html.Raw(Model.LicenseFileContentsHtml.Content)
+                        }
+                        else
                         {
                             <pre class="license-file-contents custom-license-container">@Model.LicenseFileContents</pre>
                         }
diff --git a/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml b/src/NuGetGallery/Views/Packages/_VerifyMetadata.cshtml
@@ -135,7 +135,13 @@
             <div class="verify-package-field common-licenses">
                 <label class="verify-package-field-heading">License file</label>
                 <!-- ko if: $data.LicenseFileContentsHtml -->
-                <span data-bind="html: $data.LicenseFileContentsHtml"></span>
+                <!-- ko if: $data.LicenseFileContentsHtml.ImageSourceDisallowed-->
+                @ViewHelpers.AlertImageSourceDisallowed();
+                <!-- /ko -->
+                <!-- ko if: $data.LicenseFileContentsHtml.ImagesRewritten-->
+                @ViewHelpers.AlertImagesRewritten();
+                <!-- /ko -->
+                <span class="license-file-html-container" data-bind="html: $data.LicenseFileContentsHtml.Content"></span>
                 <!-- /ko -->
                 <!-- ko ifnot: $data.LicenseFileContentsHtml -->
                 <pre class="license-file-contents custom-license-container" data-bind="text: $data.LicenseFileContents"></pre>
diff --git a/tests/NuGetGallery.Facts/ViewModels/DisplayLicenseViewModelFacts.cs b/tests/NuGetGallery.Facts/ViewModels/DisplayLicenseViewModelFacts.cs
@@ -119,6 +119,52 @@ public void ItInitializesLicenseFileContents()
             Assert.Equal(licenseFileContents, model.LicenseFileContents);
         }
 
+        [Fact]
+        public void ItInitializesLicenseFileContentsWithEmbeddedMarkdownLicense()
+        {
+            // arrange
+            Mock<IFeatureFlagService> _featureFlagService = new Mock<IFeatureFlagService>();
+            Mock<IIconUrlProvider> _iconUrlProvider = new Mock<IIconUrlProvider>();
+            Mock<IMarkdownService> _markdownService = new Mock<IMarkdownService>();
+            User user = new User();
+
+            DisplayLicenseViewModelFactory displayLicenseViewModelFactory = new DisplayLicenseViewModelFactory(_iconUrlProvider.Object,
+                _markdownService.Object,
+                _featureFlagService.Object);
+
+            var licenseFileContents = "It's a license";
+            var package = new Package
+            {
+                Version = "1.0.0",
+                PackageRegistration = new PackageRegistration { Id = "SomeId" },
+                EmbeddedLicenseType = EmbeddedLicenseFileType.Markdown,
+            };
+
+            RenderedMarkdownResult expectedlicenseContentResult = new RenderedMarkdownResult
+            {
+                Content = licenseFileContents,
+                ImagesRewritten = false,
+                ImageSourceDisallowed = false,
+            };
+
+            _featureFlagService.Setup(x => x.IsLicenseMdRenderingEnabled(user))
+                .Returns(true);
+            _markdownService.Setup(x => x.GetHtmlFromMarkdown(licenseFileContents))
+                .Returns(expectedlicenseContentResult);
+            
+            // act
+            var model = displayLicenseViewModelFactory.Create(package, null, licenseFileContents, user);
+
+            // assert
+            Assert.NotNull(model.LicenseFileContentsHtml);
+            Assert.Equal(expectedlicenseContentResult.Content,
+                model.LicenseFileContentsHtml.Content);
+            Assert.Equal(expectedlicenseContentResult.ImageSourceDisallowed,
+                model.LicenseFileContentsHtml.ImageSourceDisallowed);
+            Assert.Equal(expectedlicenseContentResult.ImagesRewritten,
+                model.LicenseFileContentsHtml.ImagesRewritten);
+        }
+
         private static DisplayLicenseViewModel CreateDisplayLicenseViewModel(
             Package package,
             IReadOnlyCollection<CompositeLicenseExpressionSegment> licenseExpressionSegments = null,

Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,14 @@`
`4`	`4`	`word-break: normal;`
`5`	`5`	`}`
`6`	`6`
`7`		`- .custom-license-container {`
	`7`	`+ .custom-license-container, .license-file-html-container{`
	`8`	`+ display: block;`
	`9`	`+ padding: 10.5px;`
	`10`	`+ background-color: @pre-bg;`
	`11`	`+ border: 1px solid #ccc;`
	`12`	`+ word-break: normal;`
`8`	`13`	`margin-bottom: @default-margin-bottom;`
	`14`	`+ overflow: auto;`
	`15`	`+ max-height: 450px;`
`9`	`16`	`}`
`10`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -658,7 +658,7 @@ private async Task<JsonResult> GetVerifyPackageView(User currentUser,`
`658`	`658`	`packageMetadata.LicenseMetadata?.Type == LicenseType.File &&`
`659`	`659`	`Path.GetExtension(license).Equals(ServicesConstants.MarkdownFileExtension, StringComparison.InvariantCulture))`
`660`	`660`	`{`
`661`		`- model.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(packageContentData.LicenseFileContents, incrementHeadersBy: 2)?.Content;`
	`661`	`+ model.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(packageContentData.LicenseFileContents, incrementHeadersBy: 2);`
`662`	`662`	`}`
`663`	`663`
`664`	`664`	`model.LicenseExpressionSegments = packageContentData.LicenseExpressionSegments;`
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,7 @@ private DisplayLicenseViewModel SetupInternal(`
`49`	`49`	`string licenseFileContents,`
`50`	`50`	`User currentUser)`
`51`	`51`	`{`
	`52`	`+ viewModel.CanDisplayPrivateMetadata = CanPerformAction(currentUser, package, ActionsRequiringPermissions.DisplayPrivatePackageMetadata);`
`52`	`53`	`viewModel.EmbeddedLicenseType = package.EmbeddedLicenseType;`
`53`	`54`	`viewModel.LicenseExpression = package.LicenseExpression;`
`54`	`55`	`if (PackageHelper.TryPrepareUrlForRendering(package.LicenseUrl, out string licenseUrl))`
`@@ -68,10 +69,15 @@ private DisplayLicenseViewModel SetupInternal(`
`68`	`69`	`package.EmbeddedLicenseType == EmbeddedLicenseFileType.Markdown &&`
`69`	`70`	`licenseFileContents != null)`
`70`	`71`	`{`
`71`		`- viewModel.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(licenseFileContents)?.Content;`
	`72`	`+ viewModel.LicenseFileContentsHtml = _markdownService.GetHtmlFromMarkdown(licenseFileContents);`
`72`	`73`	`}`
`73`	`74`
`74`	`75`	`return viewModel;`
`75`	`76`	`}`
	`77`	`+`
	`78`	`+ private static bool CanPerformAction(User currentUser, Package package, ActionRequiringPackagePermissions action)`
	`79`	`+ {`
	`80`	`+ return action.CheckPermissionsOnBehalfOfAnyAccount(currentUser, package) == PermissionsCheckResult.Allowed;`
	`81`	`+ }`
`76`	`82`	`}`
`77`	`83`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@ public class DisplayLicenseViewModel : PackageViewModel`
`15`	`15`	`public IReadOnlyCollection<string> LicenseNames { get; set; }`
`16`	`16`	`public IReadOnlyCollection<CompositeLicenseExpressionSegment> LicenseExpressionSegments { get; set; }`
`17`	`17`	`public string LicenseFileContents { get; set; }`
`18`		`- public string LicenseFileContentsHtml { get; set; }`
	`18`	`+ public RenderedMarkdownResult LicenseFileContentsHtml { get; set; }`
	`19`	`+ public bool CanDisplayPrivateMetadata { get; set; }`
`19`	`20`	`}`
`20`	`21`	`}`
Original file line number	Diff line number	Diff line change
`@@ -522,12 +522,12 @@`
`522`	`522`	`<div id="readme-container" class="collapse in">`
`523`	`523`	`@if (Model.ReadMeImagesRewritten && Model.CanDisplayPrivateMetadata)`
`524`	`524`	`{`
`525`		`- @ViewHelpers.AlertWarning(@<text>This documentation had some images automatically rewritten to use secure links and may be broken.</text>);`
	`525`	`+ @ViewHelpers.AlertImagesRewritten();`
`526`	`526`	`}`
`527`	`527`
`528`	`528`	`@if (Model.ReadmeImageSourceDisallowed && Model.CanDisplayPrivateMetadata)`
`529`	`529`	`{`
`530`		`- @ViewHelpers.AlertWarning(@<text>Some images are not displayed as they are not from <a href='https://aka.ms/nuget-org-readme#allowed-domains-for-images-and-badges'>trusted domains</a>.</text>);`
	`530`	`+ @ViewHelpers.AlertImageSourceDisallowed();`
`531`	`531`	`}`
`532`	`532`	`<div id="readme-less" class="collapse in">`
`533`	`533`	`@Html.Raw(Model.ReadMeHtml)`
Original file line number	Diff line number	Diff line change
`@@ -561,12 +561,12 @@`
`561`	`561`	`<div id="readme-container">`
`562`	`562`	`@if (Model.ReadMeImagesRewritten && Model.CanDisplayPrivateMetadata)`
`563`	`563`	`{`
`564`		`- @ViewHelpers.AlertWarning(@<text>This documentation had some images automatically rewritten to use secure links and may be broken.</text>);`
	`564`	`+ @ViewHelpers.AlertImagesRewritten();`
`565`	`565`	`}`
`566`	`566`
`567`	`567`	`@if (Model.ReadmeImageSourceDisallowed && Model.CanDisplayPrivateMetadata)`
`568`	`568`	`{`
`569`		`- @ViewHelpers.AlertWarning(@<text>Some images are not displayed as they are not from <a href='https://aka.ms/nuget-org-readme#allowed-domains-for-images-and-badges'>trusted domains</a>.</text>);`
	`569`	`+ @ViewHelpers.AlertImageSourceDisallowed();`
`570`	`570`	`}`
`571`	`571`
`572`	`572`	`@Html.Raw(Model.ReadMeHtml)`
Original file line number	Diff line number	Diff line change
`@@ -64,13 +64,22 @@`
`64`	`64`	`}`
`65`	`65`	`else`
`66`	`66`	`{`
`67`		`- <label>License text</label>`
	`67`	`+ <label>License file</label>`
`68`	`68`	`<div class="common-licenses">`
`69`	`69`	`@if (Model.LicenseFileContentsHtml != null)`
`70`	`70`	`{`
`71`		`- @Html.Raw(Model.LicenseFileContentsHtml)`
`72`		`- }`
`73`		`- else`
	`71`	`+ if (Model.LicenseFileContentsHtml.ImageSourceDisallowed && Model.CanDisplayPrivateMetadata)`
	`72`	`+ {`
	`73`	`+ @ViewHelpers.AlertImageSourceDisallowed();`
	`74`	`+ }`
	`75`	`+`
	`76`	`+ if (Model.LicenseFileContentsHtml.ImagesRewritten && Model.CanDisplayPrivateMetadata)`
	`77`	`+ {`
	`78`	`+ @ViewHelpers.AlertImagesRewritten();`
	`79`	`+ }`
	`80`	`+ @Html.Raw(Model.LicenseFileContentsHtml.Content)`
	`81`	`+ }`
	`82`	`+ else`
`74`	`83`	`{`
`75`	`84`	`<pre class="license-file-contents custom-license-container">@Model.LicenseFileContents</pre>`
`76`	`85`	`}`