Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify warning message on display package page when missing readme #9365

Merged
merged 5 commits into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/AccountDeleter/EmptyFeatureFlagService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,11 @@ public bool IsDisplayUploadWarningV2Enabled(User user)
throw new NotImplementedException();
}

public bool IsDisplayPackageReadmeWarningEnabled(User user)
{
throw new NotImplementedException();
}

public bool IsFrameworkFilteringEnabled(User user) {
throw new NotImplementedException();
}
Expand Down
3 changes: 3 additions & 0 deletions src/Bootstrap/dist/css/bootstrap-theme.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/Bootstrap/less/theme/page-display-package.less
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,10 @@
margin-bottom: -1px;
}

.nav-tabs > li.active > a.body-warning-tab > {
background-color: #fff4ce;
}

.nav-tabs > li > a {
border-left: 0px;
border-right: 0px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ public bool IsDisplayUploadWarningV2Enabled(User user)
throw new NotImplementedException();
}

public bool IsDisplayPackageReadmeWarningEnabled(User user)
{
throw new NotImplementedException();
}

public bool IsODataDatabaseReadOnlyEnabled()
{
throw new NotImplementedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public class FeatureFlagService : IFeatureFlagService
private const string MarkdigMdRenderingFlightName = GalleryPrefix + "MarkdigMdRendering";
private const string MarkdigMdSyntaxHighlightFlightName = GalleryPrefix + "MarkdigMdSyntaxHighlight";
private const string DisplayUploadWarningV2FlightName = GalleryPrefix + "DisplayUploadWarningV2";
private const string DisplayPackageReadmeWarningFlightName = GalleryPrefix + "DisplayPackageReadmeWarning";
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
private const string DeletePackageApiFlightName = GalleryPrefix + "DeletePackageApi";
private const string ImageAllowlistFlightName = GalleryPrefix + "ImageAllowlist";
private const string DisplayBannerFlightName = GalleryPrefix + "Banner";
Expand Down Expand Up @@ -348,6 +349,11 @@ public bool IsDisplayUploadWarningV2Enabled(User user)
return _client.IsEnabled(DisplayUploadWarningV2FlightName, user, defaultValue: false);
}

public bool IsDisplayPackageReadmeWarningEnabled(User user)
{
return _client.IsEnabled(DisplayPackageReadmeWarningFlightName, user, defaultValue: false);
}

public bool IsDeletePackageApiEnabled(User user)
{
return _client.IsEnabled(DeletePackageApiFlightName, user, defaultValue: false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,11 @@ public interface IFeatureFlagService
/// </summary>
bool IsDisplayUploadWarningV2Enabled(User user);

/// <summary>
/// Whether the new warning of the missing readme is displayed to package authors
/// </summary>
bool IsDisplayPackageReadmeWarningEnabled(User user);
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Whether or not the user can delete a package through the API.
/// </summary>
Expand Down
6 changes: 6 additions & 0 deletions src/NuGetGallery/App_Data/Files/Content/flags.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@
"SiteAdmins": false,
"Accounts": [],
"Domains": []
},
"NuGetGallery.DisplayPackageReadmeWarning": {
"All": true,
"SiteAdmins": false,
"Accounts": [],
"Domains": []
}
}
}
3 changes: 3 additions & 0 deletions src/NuGetGallery/Controllers/PackagesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,8 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
packageRenames,
readme);

var canDisplayReadmeWarning = _featureFlagService.IsDisplayPackageReadmeWarningEnabled(currentUser) && !model.HasEmbeddedReadmeFile && model.ReadMeHtml == null;

model.ValidatingTooLong = _validationService.IsValidatingTooLong(package);
model.PackageValidationIssues = _validationService.GetLatestPackageValidationIssues(package);
model.SymbolsPackageValidationIssues = _validationService.GetLatestPackageValidationIssues(model.LatestSymbolsPackage);
Expand All @@ -956,6 +958,7 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
model.IsDisplayTargetFrameworkEnabled = _featureFlagService.IsDisplayTargetFrameworkEnabled(currentUser);
model.IsComputeTargetFrameworkEnabled = _featureFlagService.IsComputeTargetFrameworkEnabled();
model.IsMarkdigMdSyntaxHighlightEnabled = _featureFlagService.IsMarkdigMdSyntaxHighlightEnabled();
model.CanDisplayReadmeWarning = canDisplayReadmeWarning;

if (model.IsComputeTargetFrameworkEnabled || model.IsDisplayTargetFrameworkEnabled)
{
Expand Down
1 change: 1 addition & 0 deletions src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public class DisplayPackageViewModel : ListPackageItemViewModel
public bool IsPackageDependentsEnabled { get; set; }
public bool IsRecentPackagesNoIndexEnabled { get; set; }
public bool IsMarkdigMdSyntaxHighlightEnabled { get; set; }
public bool CanDisplayReadmeWarning { get; set; }
public NuGetPackageGitHubInformation GitHubDependenciesInformation { get; set; }
public bool HasEmbeddedIcon { get; set; }
public bool HasEmbeddedReadmeFile { get; set; }
Expand Down
17 changes: 14 additions & 3 deletions src/NuGetGallery/Views/Packages/DisplayPackage.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,19 @@
role="tab"
data-toggle="tab"
id="readme-body-tab"
class="body-tab"
class="body-@(Model.CanDisplayReadmeWarning && Model.CanDisplayPrivateMetadata? "warning-" : "")tab"
aria-controls="readme-tab"
aria-expanded="@(activeBodyTab == "readme" ? "true" : "false")"
aria-selected="@(activeBodyTab == "readme" ? "true" : "false")"
tabindex="@(activeBodyTab == "readme" ? "0" : "-1")">
<i class="ms-Icon ms-Icon--Dictionary" aria-hidden="true"></i>
@if (Model.CanDisplayReadmeWarning && Model.CanDisplayPrivateMetadata)
{
<i class="ms-Icon ms-Icon--Warning" aria-hidden="true"></i>
}
else
{
<i class="ms-Icon ms-Icon--Dictionary" aria-hidden="true"></i>
}
README
</a>
</li>
Expand Down Expand Up @@ -670,7 +677,11 @@
}
else
{
if (Model.CanDisplayPrivateMetadata)
if (Model.CanDisplayPrivateMetadata && Model.CanDisplayReadmeWarning)
{
@ViewHelpers.AlertWarning(@<text>Your package is missing a README. Please update your package to <a href='https://aka.ms/nuget-include-readme'>include a README</a> or <a href=@Url.ManagePackage(Model)>add a README here</a>.</text>);
lyndaidaii marked this conversation as resolved.
Show resolved Hide resolved
}
else if (Model.CanDisplayPrivateMetadata)
{
@ViewHelpers.AlertWarning(@<text>The package description is shown below. Please update your package to <a href='https://docs.microsoft.com/nuget/nuget-org/package-readme-on-nuget-org'>include a README</a>.</text>);
}
Expand Down
2 changes: 2 additions & 0 deletions src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ public class FakeFeatureFlagService : IFeatureFlagService

public bool IsDisplayUploadWarningV2Enabled(User user) => throw new NotImplementedException();

public bool IsDisplayPackageReadmeWarningEnabled(User user) => throw new NotImplementedException();

public bool IsFrameworkFilteringEnabled(User user) => throw new NotImplementedException();
}
}