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

[Rename] Display rename information on package detail page #7964

Merged
merged 8 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Initial commit
  • Loading branch information
zhhyu authored and zhhyu committed Apr 24, 2020
commit 30d50764330730009446a9f724c6f861e06f24de
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class FeatureFlagService : IFeatureFlagService
private const string ShowEnable2FADialog = GalleryPrefix + "ShowEnable2FADialog";
private const string Get2FADismissFeedback = GalleryPrefix + "Get2FADismissFeedback";
private const string UsabillaOnEveryPageFeatureName = GalleryPrefix + "UsabillaEveryPage";
private const string PackageRenamesFeatureName = GalleryPrefix + "PackageRenames";

private readonly IFeatureFlagClient _client;

Expand Down Expand Up @@ -173,5 +174,10 @@ public bool IsUsabillaButtonEnabledOnEveryPage()
{
return _client.IsEnabled(UsabillaOnEveryPageFeatureName, defaultValue: false);
}

public bool IsPackageRenamesEnabled(User user)
{
return _client.IsEnabled(PackageRenamesFeatureName, user, defaultValue: false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,5 +125,10 @@ public interface IFeatureFlagService
/// Whether we should enable the Usabilla feedback button on every page.
/// </summary>
bool IsUsabillaButtonEnabledOnEveryPage();

/// <summary>
/// Whether the user is able to see or manage the package renames information.
/// </summary>
bool IsPackageRenamesEnabled(User user);
}
}
9 changes: 9 additions & 0 deletions src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,11 @@ protected override void Load(ContainerBuilder builder)
.As<IEntityRepository<PackageDeprecation>>()
.InstancePerLifetimeScope();

builder.RegisterType<EntityRepository<PackageRename>>()
.AsSelf()
.As<IEntityRepository<PackageRename>>()
.InstancePerLifetimeScope();

ConfigureGalleryReadOnlyReplicaEntitiesContext(builder, loggerFactory, configuration, secretInjector);

var supportDbConnectionFactory = CreateDbConnectionFactory(
Expand Down Expand Up @@ -406,6 +411,10 @@ protected override void Load(ContainerBuilder builder)
.As<IPackageDeprecationService>()
.InstancePerLifetimeScope();

builder.RegisterType<PackageRenameService>()
.As<IPackageRenameService>()
.InstancePerLifetimeScope();

builder.RegisterType<PackageUpdateService>()
.As<IPackageUpdateService>()
.InstancePerLifetimeScope();
Expand Down
9 changes: 8 additions & 1 deletion src/NuGetGallery/Controllers/PackagesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ public partial class PackagesController
private readonly ILicenseExpressionSplitter _licenseExpressionSplitter;
private readonly IFeatureFlagService _featureFlagService;
private readonly IPackageDeprecationService _deprecationService;
private readonly IPackageRenameService _renameService;
private readonly IABTestService _abTestService;
private readonly IIconUrlProvider _iconUrlProvider;
private readonly DisplayPackageViewModelFactory _displayPackageViewModelFactory;
Expand Down Expand Up @@ -154,6 +155,7 @@ public PackagesController(
ILicenseExpressionSplitter licenseExpressionSplitter,
IFeatureFlagService featureFlagService,
IPackageDeprecationService deprecationService,
IPackageRenameService renameService,
IABTestService abTestService,
IIconUrlProvider iconUrlProvider)
{
Expand Down Expand Up @@ -185,6 +187,7 @@ public PackagesController(
_licenseExpressionSplitter = licenseExpressionSplitter ?? throw new ArgumentNullException(nameof(licenseExpressionSplitter));
_featureFlagService = featureFlagService ?? throw new ArgumentNullException(nameof(featureFlagService));
_deprecationService = deprecationService ?? throw new ArgumentNullException(nameof(deprecationService));
_renameService = renameService ?? throw new ArgumentNullException(nameof(renameService));
_abTestService = abTestService ?? throw new ArgumentNullException(nameof(abTestService));
_iconUrlProvider = iconUrlProvider ?? throw new ArgumentNullException(nameof(iconUrlProvider));

Expand Down Expand Up @@ -837,11 +840,14 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
.GroupBy(d => d.PackageKey)
.ToDictionary(g => g.Key, g => g.First());

var packageRenames = _renameService.GetPackageRenames(package.PackageRegistration);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be behind the flag as well, so we can turn if off if perf is an issue.


var model = _displayPackageViewModelFactory.Create(
package,
allVersions,
currentUser,
packageKeyToDeprecation,
packageRenames,
readme);

model.ValidatingTooLong = _validationService.IsValidatingTooLong(package);
Expand All @@ -850,8 +856,9 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
model.IsCertificatesUIEnabled = _contentObjectService.CertificatesConfiguration?.IsUIEnabledForUser(currentUser) ?? false;
model.IsAtomFeedEnabled = _featureFlagService.IsPackagesAtomFeedEnabled();
model.IsPackageDeprecationEnabled = _featureFlagService.IsManageDeprecationEnabled(currentUser, allVersions);
model.IsPackageRenamesEnabled = _featureFlagService.IsPackageRenamesEnabled(currentUser);

if(model.IsGitHubUsageEnabled = _featureFlagService.IsGitHubUsageEnabled(currentUser))
if (model.IsGitHubUsageEnabled = _featureFlagService.IsGitHubUsageEnabled(currentUser))
{
model.GitHubDependenciesInformation = _contentObjectService.GitHubUsageConfiguration.GetPackageInformation(id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public DeletePackageViewModel Setup(
allVersions,
currentUser,
packageKeyToDeprecation: null,
packageRenames: null,
readmeResult: null);

return SetupInternal(viewModel, package, reasons);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public DisplayPackageViewModel Create(
IReadOnlyCollection<Package> allVersions,
User currentUser,
IReadOnlyDictionary<int, PackageDeprecation> packageKeyToDeprecation,
IReadOnlyList<PackageRename> packageRenames,
RenderedReadMeResult readmeResult)
{
var viewModel = new DisplayPackageViewModel();
Expand All @@ -32,6 +33,7 @@ public DisplayPackageViewModel Create(
allVersions,
currentUser,
packageKeyToDeprecation,
packageRenames,
readmeResult);
}

Expand All @@ -41,11 +43,12 @@ public DisplayPackageViewModel Setup(
IReadOnlyCollection<Package> allVersions,
User currentUser,
IReadOnlyDictionary<int, PackageDeprecation> packageKeyToDeprecation,
IReadOnlyList<PackageRename> packageRenames,
RenderedReadMeResult readmeResult)
{
_listPackageItemViewModelFactory.Setup(viewModel, package, currentUser);
SetupCommon(viewModel, package, pushedBy: null, packageKeyToDeprecation: packageKeyToDeprecation);
return SetupInternal(viewModel, package, allVersions, currentUser, packageKeyToDeprecation, readmeResult);
return SetupInternal(viewModel, package, allVersions, currentUser, packageKeyToDeprecation, packageRenames, readmeResult);
}

private DisplayPackageViewModel SetupInternal(
Expand All @@ -54,6 +57,7 @@ private DisplayPackageViewModel SetupInternal(
IReadOnlyCollection<Package> allVersions,
User currentUser,
IReadOnlyDictionary<int, PackageDeprecation> packageKeyToDeprecation,
IReadOnlyList<PackageRename> packageRenames,
RenderedReadMeResult readmeResult)
{
var dependencies = package.Dependencies.ToList();
Expand Down Expand Up @@ -112,6 +116,16 @@ private DisplayPackageViewModel SetupInternal(
viewModel.CustomMessage = deprecation.CustomMessage;
}

if (packageRenames != null)
{
viewModel.PackageRenames = packageRenames;
}

if (package.PackageRegistration?.RenamedMessage != null)
{
viewModel.RenamedMessage = package.PackageRegistration.RenamedMessage;
}

viewModel.ReadMeHtml = readmeResult?.Content;
viewModel.ReadMeImagesRewritten = readmeResult != null ? readmeResult.ImagesRewritten : false;
viewModel.HasEmbeddedIcon = package.HasEmbeddedIcon;
Expand Down
5 changes: 5 additions & 0 deletions src/NuGetGallery/NuGetGallery.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@
<Compile Include="Services\GalleryContentFileMetadataService.cs" />
<Compile Include="Services\InvalidLicenseUrlValidationMessage.cs" />
<Compile Include="Services\InvalidUrlEncodingForLicenseUrlValidationMessage.cs" />
<Compile Include="Services\IPackageRenameService.cs" />
<Compile Include="Services\ISearchSideBySideService.cs" />
<Compile Include="Services\ISymbolPackageUploadService.cs" />
<Compile Include="Services\ISymbolPackageFileService.cs" />
Expand All @@ -496,6 +497,7 @@
<Compile Include="Services\PackageDeleteService.cs" />
<Compile Include="Services\PackageDeprecationManagementService.cs" />
<Compile Include="Services\PackageDeprecationService.cs" />
<Compile Include="Services\PackageRenameService.cs" />
<Compile Include="Services\PackageShouldNotBeSignedUserFixableValidationMessage.cs" />
<Compile Include="Services\PlainTextOnlyValidationMessage.cs" />
<Compile Include="Services\RenderedReadMeResult.cs" />
Expand Down Expand Up @@ -2359,6 +2361,9 @@
<Version>9.3.3</Version>
</PackageReference>
</ItemGroup>
<ItemGroup>
<Content Include="Views\Packages\_DisplayPackageRenames.cshtml" />
</ItemGroup>
<PropertyGroup>
<VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">10.0</VisualStudioVersion>
<VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>
Expand Down
9 changes: 9 additions & 0 deletions src/NuGetGallery/Scripts/gallery/page-display-package.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
$(function () {
'use strict';

// Configure the rename information container
var container = $('#show-rename-content-container');
window.nuget.configureExpander("rename-content-container", "ChevronDown", null, "ChevronUp");
container.keydown(function (event) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be DRY with the deprecation below. Shared function, loop, whatever.

if (event.which === 13) { // Enter
$(event.target).click();
}
});

// Configure the deprecation information container
var container = $('#show-deprecation-content-container');
if ($('#deprecation-content-container').children().length) {
Expand Down
13 changes: 13 additions & 0 deletions src/NuGetGallery/Services/IPackageRenameService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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 NuGet.Services.Entities;

namespace NuGetGallery
{
public interface IPackageRenameService
{
IReadOnlyList<PackageRename> GetPackageRenames(PackageRegistration package);
}
}
30 changes: 30 additions & 0 deletions src/NuGetGallery/Services/PackageRenameService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// 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.Linq;
using System.Data.Entity;
using System.Collections.Generic;
using NuGet.Services.Entities;

namespace NuGetGallery
{
public class PackageRenameService : IPackageRenameService
{
private readonly IEntityRepository<PackageRename> _packageRenameRepository;

public PackageRenameService (
IEntityRepository<PackageRename> packageRenameRepository)
{
_packageRenameRepository = packageRenameRepository ?? throw new ArgumentNullException(nameof(packageRenameRepository));
}

public IReadOnlyList<PackageRename> GetPackageRenames(PackageRegistration packageRegistration)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this new query significantly impact the display package page performance?

return _packageRenameRepository.GetAll()
.Where(pr => pr.FromPackageRegistrationKey == packageRegistration.Key)
.Include(pr => pr.ToPackageRegistration)
.ToList();
}
}
}
4 changes: 4 additions & 0 deletions src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class DisplayPackageViewModel : ListPackageItemViewModel
public bool IsDotnetNewTemplatePackageType { get; set; }
public bool IsAtomFeedEnabled { get; set; }
public bool IsPackageDeprecationEnabled { get; set; }
public bool IsPackageRenamesEnabled { get; set; }
public bool IsGitHubUsageEnabled { get; set; }
public NuGetPackageGitHubInformation GitHubDependenciesInformation { get; set; }
public bool HasEmbeddedIcon { get; set; }
Expand Down Expand Up @@ -82,6 +83,9 @@ public bool HasNewerRelease
public string AlternatePackageVersion { get; set; }
public string CustomMessage { get; set; }

public IReadOnlyCollection<PackageRename> PackageRenames { get; set; }
public string RenamedMessage { get; set; }

public void InitializeRepositoryMetadata(string repositoryUrl, string repositoryType)
{
RepositoryType = RepositoryKind.Unknown;
Expand Down
5 changes: 5 additions & 0 deletions src/NuGetGallery/Views/Packages/DisplayPackage.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,11 @@
@Html.Partial("_DisplayPackageDeprecation")
}

@if (Model.IsPackageRenamesEnabled && Model.PackageRenames != null && Model.PackageRenames.Count != 0)
{
@Html.Partial("_DisplayPackageRenames")
}

@if (Model.Prerelease)
{
@ViewHelpers.AlertInfo(
Expand Down
48 changes: 48 additions & 0 deletions src/NuGetGallery/Views/Packages/_DisplayPackageRenames.cshtml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
@model DisplayPackageViewModel

<div class="deprecation-container">
<div class="icon-text alert alert-info">
<div id="show-rename-content-container" class="deprecation-expander" tabindex="0" data-toggle="collapse" data-target="#rename-content-container" aria-expanded="false" aria-controls="rename-content-container" aria-labelledby="rename-container-label" role="button">
<div class="deprecation-expander" role="button">
<div class="deprecation-expander-container">
<i class="deprecation-expander-icon ms-Icon ms-Icon--Info" aria-hidden="true"></i>

<div id="rename-container-label" class="deprecation-expander-info-right">
@{
if (Model.PackageRenames.Count == 1)
{
@:This package has been renamed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Period at the end of sentence.

}
else
{
@:This package has been renamed or split into new packages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Period at the end of sentence.

}
}
</div>
</div>

<div class="deprecation-expander-container">
<i class="deprecation-expander-icon deprecation-expander-info-right ms-Icon ms-Icon--ChevronDown" aria-hidden="true"></i>
</div>
</div>
</div>

<div class="deprecation-content-container collapse" id="rename-content-container">
<ul class="list-unstyled">
@foreach (var packageRename in Model.PackageRenames)
{
var renamePackageName = packageRename.ToPackageRegistration.Id;
var renamePackageUrl = Url.Package(renamePackageName);
<li><a href="@renamePackageUrl">@renamePackageName</a></li>
}
</ul>
<p></p>

@if (!string.IsNullOrEmpty(Model.RenamedMessage))
{
<b>Additional Details</b>
<p>@Html.PreFormattedText(Model.RenamedMessage, Config)</p>
}
</div>
</div>
</div>