Skip to content

Commit

Permalink
Add preview banner
Browse files Browse the repository at this point in the history
  • Loading branch information
loic-sharma committed Jul 26, 2021
1 parent 0fde8b0 commit 322ffb3
Show file tree
Hide file tree
Showing 11 changed files with 147 additions and 24 deletions.
5 changes: 5 additions & 0 deletions src/AccountDeleter/EmptyFeatureFlagService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ public bool IsDisplayPackagePageV2Enabled(User user)
throw new NotImplementedException();
}

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

public bool IsDisplayVulnerabilitiesEnabled()
{
throw new NotImplementedException();
Expand Down
5 changes: 5 additions & 0 deletions src/GitHubVulnerabilities2Db/Fakes/FakeFeatureFlagService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -264,5 +264,10 @@ public bool IsDisplayNuGetPackageExplorerLinkEnabled()
{
throw new NotImplementedException();
}

public bool IsDisplayPackagePageV2PreviewEnabled(User user)
{
throw new NotImplementedException();
}
}
}
6 changes: 6 additions & 0 deletions src/NuGetGallery.Services/Configuration/FeatureFlagService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class FeatureFlagService : IFeatureFlagService
private const string DeletePackageApiFlightName = GalleryPrefix + "DeletePackageApi";
private const string ImageAllowlistFlightName = GalleryPrefix + "ImageAllowlist";
private const string DisplayBannerFlightName = GalleryPrefix + "Banner";
private const string DisplayPackagePageV2PreviewFeatureName = GalleryPrefix + "DisplayPackagePageV2Preview";
private const string DisplayPackagePageV2FeatureName = GalleryPrefix + "DisplayPackagePageV2";
private const string ShowReportAbuseSafetyChanges = GalleryPrefix + "ShowReportAbuseSafetyChanges";

Expand Down Expand Up @@ -246,6 +247,11 @@ public bool AreEmbeddedReadmesEnabled(User user)
return _client.IsEnabled(EmbeddedReadmeFlightName, user, defaultValue: false);
}

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

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

/// <summary>
/// Whether the preview of the new design of the display package page is enabled.
/// </summary>
bool IsDisplayPackagePageV2PreviewEnabled(User user);

/// <summary>
/// Whether the new design of the display package page is enabled.
/// </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 @@ -103,6 +103,12 @@
"SiteAdmins": false,
"Accounts": [],
"Domains": []
},
"NuGetGallery.DisplayPackagePageV2Preview": {
"All": true,
"SiteAdmins": false,
"Accounts": [],
"Domains": []
}
}
}
31 changes: 28 additions & 3 deletions src/NuGetGallery/Controllers/PackagesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ public HttpStatusCodeResult DisplayPackage()
=> new HttpStatusCodeWithHeadersResult(HttpStatusCode.MethodNotAllowed, new NameValueCollection() { { "allow", "GET" } });

[HttpGet]
public virtual async Task<ActionResult> DisplayPackage(string id, string version)
public virtual async Task<ActionResult> DisplayPackage(string id, string version, string preview = null)
{
// Attempt to normalize the version but allow version strings that are not actually SemVer strings. For
// example, "absoluteLatest" is allowed as the version string as a reference to the absolute latest version
Expand Down Expand Up @@ -1034,7 +1034,28 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
}
ViewBag.FacebookAppID = _config.FacebookAppId;

if (_featureFlagService.IsDisplayPackagePageV2Enabled(GetCurrentUser()))
var enableRedesign = _featureFlagService.IsDisplayPackagePageV2Enabled(currentUser);

if (_featureFlagService.IsDisplayPackagePageV2PreviewEnabled(currentUser))
{
if (!string.IsNullOrEmpty(preview))
{
enableRedesign = true;
}

// If the user is on the current design, show the banner to preview the redesign.
// If the user is on the preview design, show the banner that links to the feedback survey.
ViewBag.ShowRedesignPreviewBanner = !enableRedesign;
ViewBag.ShowRedesignSurveyBanner = enableRedesign;
ViewBag.ShowRedesignUrl = Url.Package(package, preview: true);
}
else
{
ViewBag.ShowRedesignPreviewBanner = false;
ViewBag.ShowRedesignSurveyBanner = false;
}

if (enableRedesign)
{
return View("DisplayPackageV2", model);
}
Expand Down Expand Up @@ -2986,7 +3007,11 @@ public virtual async Task<JsonResult> GetReadMeMd(string id, string version)
[ValidateAntiForgeryToken]
public virtual async Task<ActionResult> SetLicenseReportVisibility(string id, string version, bool visible)
{
return await SetLicenseReportVisibility(id, version, visible, Url.Package);
return await SetLicenseReportVisibility(
id,
version,
visible,
(package, relativeUrl) => Url.Package(package, relativeUrl));
}

internal virtual async Task<ActionResult> SetLicenseReportVisibility(string id, string version, bool visible, Func<Package, bool, string> urlFactory)
Expand Down
18 changes: 12 additions & 6 deletions src/NuGetGallery/UrlHelperExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,27 +225,33 @@ public static string Package(
this UrlHelper url,
string id,
string version,
bool relativeUrl = true)
bool relativeUrl = true,
bool preview = false)
{
string normalized = (version != null) ? NuGetVersionFormatter.Normalize(version) : version;
var normalized = (version != null) ? NuGetVersionFormatter.Normalize(version) : version;

string result = GetRouteLink(
var result = GetRouteLink(
url,
RouteName.DisplayPackage,
relativeUrl,
routeValues: new RouteValueDictionary
{
{ "id", id },
{ "version", normalized }
{ "version", normalized },
{ "preview", preview ? "1" : null }
});

// Ensure trailing slashes for versionless package URLs, as a fix for package filenames that look like known file extensions
return version == null ? EnsureTrailingSlash(result) : result;
}

public static string Package(this UrlHelper url, Package package, bool relativeUrl = true)
public static string Package(
this UrlHelper url,
Package package,
bool relativeUrl = true,
bool preview = false)
{
return url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl);
return url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl, preview);
}

public static string Package(this UrlHelper url, IPackageVersionModel package, bool relativeUrl = true)
Expand Down
31 changes: 30 additions & 1 deletion src/NuGetGallery/Views/Shared/Gallery/Header.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
</div>
}

@if (ViewBag.DisplayBanner == true)
@if (ViewBag.DisplayBanner == true)
{
<div class="container-fluid banner banner-info text-center">
<div class="row">
Expand All @@ -35,6 +35,35 @@
</div>
}

@if (ViewBag.ShowRedesignPreviewBanner == true)
{
<div class="container-fluid banner banner-info text-center">
<div class="row">
<div class="col-sm-12">
<span>
&#128075; Changes are coming to this page!
Test out the <a href="@ViewBag.ShowRedesignUrl">new design</a> and
<a href="https://aka.ms/NuGetRedesignSurvey" target="_blank">tell us what you think</a>.
</span>
</div>
</div>
</div>
}

@if (ViewBag.ShowRedesignSurveyBanner == true)
{
<div class="container-fluid banner banner-info text-center">
<div class="row">
<div class="col-sm-12">
<span>
&#128075; What do you think about the new design?
<a href="https://aka.ms/NuGetRedesignSurvey" target="_blank">Take the survey (3 min.)</a>
</span>
</div>
</div>
</div>
}

@helper DisplayNavigationItem(string tab, string url, bool bold = false, string title = null)
{
<li class="@(ViewBag.Tab == tab ? "active" : string.Empty)" role="presentation">
Expand Down
5 changes: 5 additions & 0 deletions src/VerifyMicrosoftPackage/Fakes/FakeFeatureFlagService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,10 @@ public class FakeFeatureFlagService : IFeatureFlagService
public bool IsDisplayBannerEnabled() => throw new NotImplementedException();

public bool IsDisplayNuGetPackageExplorerLinkEnabled() => throw new NotImplementedException();

public bool IsDisplayPackagePageV2PreviewEnabled(User user)
{
throw new NotImplementedException();
}
}
}
43 changes: 35 additions & 8 deletions tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ private static PackagesController CreateController(
featureFlagService
.Setup(x => x.IsDisplayPackagePageV2Enabled(It.IsAny<User>()))
.Returns(false);
featureFlagService
.Setup(x => x.IsDisplayPackagePageV2PreviewEnabled(It.IsAny<User>()))
.Returns(false);
}

renameService = renameService ?? new Mock<IPackageRenameService>();
Expand Down Expand Up @@ -1167,9 +1170,21 @@ public async Task GetsValidationIssues()
}

[Theory]
[InlineData(true, "DisplayPackageV2")]
[InlineData(false, "DisplayPackage")]
public async Task DisplayPackageV2(bool isDisplayV2Enabled, String expectedViewName)
[InlineData(false, false, null, "DisplayPackage", false, false)]
[InlineData(false, false, "1", "DisplayPackage", false, false)]
[InlineData(false, true, null, "DisplayPackage", true, false)]
[InlineData(false, true, "1", "DisplayPackageV2", false, true)]
[InlineData(true, false, null, "DisplayPackageV2", false, false)]
[InlineData(true, false, "1", "DisplayPackageV2", false, false)]
[InlineData(true, true, null, "DisplayPackageV2", false, true)]
[InlineData(true, true, "1", "DisplayPackageV2", false, true)]
public async Task DisplayPackageV2(
bool enableV2,
bool enablePreview,
string previewParam,
string expectedViewName,
bool expectPreviewBanner,
bool expectSurveyBanner)
{
// Arrange
var packageService = new Mock<IPackageService>();
Expand Down Expand Up @@ -1198,23 +1213,35 @@ public async Task DisplayPackageV2(bool isDisplayV2Enabled, String expectedViewN
};

var packages = new[] { package };
packageService.Setup(p => p.FindPackagesById("Foo", /*includePackageRegistration:*/ true))
packageService
.Setup(p => p.FindPackagesById("Foo", /*includePackageRegistration:*/ true))
.Returns(packages);

packageService.Setup(p => p.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, true))
packageService
.Setup(p => p.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, true))
.Returns(package);

indexingService.Setup(i => i.GetLastWriteTime()).Returns(Task.FromResult((DateTime?)DateTime.UtcNow));
indexingService
.Setup(i => i.GetLastWriteTime())
.Returns(Task.FromResult((DateTime?)DateTime.UtcNow));

featureFlag
.Setup(x => x.IsDisplayPackagePageV2Enabled(It.IsAny<User>()))
.Returns(isDisplayV2Enabled);
.Returns(enableV2);
featureFlag
.Setup(x => x.IsDisplayPackagePageV2PreviewEnabled(It.IsAny<User>()))
.Returns(enablePreview);

// Act
var result = await controller.DisplayPackage("Foo", version: null);
var result = await controller.DisplayPackage("Foo", version: null, preview: previewParam);

// Assert
ResultAssert.IsView<DisplayPackageViewModel>(result, expectedViewName);

var view = (ViewResult)result;

Assert.Equal(expectPreviewBanner, view.ViewBag.ShowRedesignPreviewBanner);
Assert.Equal(expectSurveyBanner, view.ViewBag.ShowRedesignSurveyBanner);
}

[Theory]
Expand Down
16 changes: 10 additions & 6 deletions tests/NuGetGallery.Facts/UrlHelperExtensionsFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,15 @@ public void UsesNormalizedVersionInUrls()
}

[Theory]
[InlineData("https://nuget.org", true, "/packages/id/1.0.0")]
[InlineData("https://nuget.org", false, "https://nuget.org/packages/id/1.0.0")]
[InlineData("https://localhost:66", true, "/packages/id/1.0.0")]
[InlineData("https://localhost:66", false, "https://localhost:66/packages/id/1.0.0")]
public void ReturnsCorrectRouteLink(string siteRoot, bool relativeUrl, string expectedUrl)
[InlineData("https://nuget.org", true, false, "/packages/id/1.0.0")]
[InlineData("https://nuget.org", false, false, "https://nuget.org/packages/id/1.0.0")]
[InlineData("https://localhost:66", true, false, "/packages/id/1.0.0")]
[InlineData("https://localhost:66", false, false, "https://localhost:66/packages/id/1.0.0")]
[InlineData("https://nuget.org", true, true, "/packages/id/1.0.0?preview=1")]
[InlineData("https://nuget.org", false, true, "https://nuget.org/packages/id/1.0.0?preview=1")]
[InlineData("https://localhost:66", true, true, "/packages/id/1.0.0?preview=1")]
[InlineData("https://localhost:66", false, true, "https://localhost:66/packages/id/1.0.0?preview=1")]
public void ReturnsCorrectRouteLink(string siteRoot, bool relativeUrl, bool preview, string expectedUrl)
{
// Arrange
var configurationService = GetConfigurationService();
Expand All @@ -90,7 +94,7 @@ public void ReturnsCorrectRouteLink(string siteRoot, bool relativeUrl, string ex
var urlHelper = TestUtility.MockUrlHelper(siteRoot);

// Act
var result = urlHelper.Package("id", "1.0.0", relativeUrl);
var result = urlHelper.Package("id", "1.0.0", relativeUrl, preview);

// Assert
Assert.Equal(expectedUrl, result);
Expand Down

0 comments on commit 322ffb3

Please sign in to comment.