Skip to content

Commit 165984b

Browse files
rocklanrocklan
authored
Package list can be sorted by ID, owners, downloads and version (#8158)
Addresses #7806
1 parent 65185e6 commit 165984b

7 files changed

Lines changed: 195 additions & 37 deletions

File tree

‎src/Bootstrap/dist/css/bootstrap-theme.css‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Bootstrap/less/theme/base.less‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,3 +425,7 @@ img.reserved-indicator-icon {
425425
z-index: 99999;
426426
right: 0;
427427
}
428+
429+
.sortable {
430+
cursor: pointer;
431+
}

‎src/NuGetGallery/Controllers/UsersController.cs‎

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using System.Web.Mvc;
1313
using NuGet.Services.Entities;
1414
using NuGet.Services.Messaging.Email;
15+
using NuGet.Versioning;
1516
using NuGetGallery.Areas.Admin;
1617
using NuGetGallery.Areas.Admin.ViewModels;
1718
using NuGetGallery.Authentication;
@@ -521,16 +522,12 @@ public virtual ActionResult Packages()
521522
var wasAADLoginOrMultiFactorAuthenticated = User.WasMultiFactorAuthenticated() || User.WasAzureActiveDirectoryAccountUsedForSignin();
522523

523524
var packages = PackageService.FindPackagesByAnyMatchingOwner(currentUser, includeUnlisted: true);
524-
var listedPackages = packages
525-
.Where(p => p.Listed && p.PackageStatusKey == PackageStatus.Available)
526-
.Select(p => _listPackageItemRequiredSignerViewModelFactory.Create(p, currentUser, wasAADLoginOrMultiFactorAuthenticated))
527-
.OrderBy(p => p.Id)
528-
.ToList();
529-
var unlistedPackages = packages
530-
.Where(p => !p.Listed || p.PackageStatusKey != PackageStatus.Available)
531-
.Select(p => _listPackageItemRequiredSignerViewModelFactory.Create(p, currentUser, wasAADLoginOrMultiFactorAuthenticated))
532-
.OrderBy(p => p.Id)
533-
.ToList();
525+
526+
var listedPackages = GetPackages(packages, currentUser, wasAADLoginOrMultiFactorAuthenticated,
527+
p => p.Listed && p.PackageStatusKey == PackageStatus.Available);
528+
529+
var unlistedPackages = GetPackages(packages, currentUser, wasAADLoginOrMultiFactorAuthenticated,
530+
p => !p.Listed || p.PackageStatusKey != PackageStatus.Available);
534531

535532
// find all received ownership requests
536533
var userReceived = _packageOwnerRequestService.GetPackageOwnershipRequests(newOwner: currentUser);
@@ -567,6 +564,36 @@ public virtual ActionResult Packages()
567564
return View(model);
568565
}
569566

567+
/// <summary>
568+
/// Returns all packages based on the predicate, with the VersionSortOrder populated
569+
/// </summary>
570+
/// <param name="packages"></param>
571+
/// <param name="currentUser"></param>
572+
/// <param name="wasAADLoginOrMultiFactorAuthenticated"></param>
573+
/// <param name="predicate"></param>
574+
/// <returns></returns>
575+
private List<ListPackageItemRequiredSignerViewModel> GetPackages(
576+
IEnumerable<Package> packages,
577+
User currentUser,
578+
bool wasAADLoginOrMultiFactorAuthenticated,
579+
Func<Package, bool> predicate)
580+
{
581+
var listedPackages = packages
582+
.Where(p => predicate(p))
583+
.Select(p => _listPackageItemRequiredSignerViewModelFactory.Create(p, currentUser, wasAADLoginOrMultiFactorAuthenticated))
584+
.OrderBy(p => NuGetVersion.Parse(p.FullVersion))
585+
.ToList();
586+
587+
for (int i = 0; i < listedPackages.Count; i++)
588+
{
589+
listedPackages[i].VersionSortOrder = i;
590+
}
591+
592+
listedPackages.Sort((x, y) => string.Compare(x.Id, y.Id, StringComparison.OrdinalIgnoreCase));
593+
594+
return listedPackages;
595+
}
596+
570597
[HttpGet]
571598
[UIAuthorize]
572599
public virtual ActionResult Organizations()

‎src/NuGetGallery/Scripts/gallery/page-manage-packages.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
this.CanEdit = packageItem.CanEdit;
3939
this.CanManageOwners = packageItem.CanManageOwners;
4040
this.CanDelete = packageItem.CanDelete;
41+
this.VersionSortOrder = packageItem.VersionSortOrder;
4142

4243
this.FormattedDownloadCount = ko.pureComputed(function () {
4344
return ko.unwrap(this.DownloadCount).toLocaleString();
@@ -355,6 +356,49 @@
355356
this.RequestsSent = new OwnerRequestsListViewModel(this, initialData.RequestsSent, false, true);
356357
}
357358

359+
function setupColumnSorting() {
360+
$('.sortable').click(function () {
361+
362+
var table = $(this).parents('table').eq(0);
363+
var rows = table.find('tr:gt(0)').toArray().sort(comparer($(this).index()));
364+
this.asc = !this.asc;
365+
if (!this.asc) {
366+
rows = rows.reverse();
367+
}
368+
for (var i = 0; i < rows.length; i++) {
369+
table.append(rows[i]);
370+
}
371+
372+
table.find('.sortable').each(function () {
373+
var currentText = $(this).text();
374+
var newText = currentText.replace(' ▲', '').replace(' ▼', '');
375+
$(this).text(newText);
376+
});
377+
378+
var columnText = $(this).text();
379+
$(this).text(columnText + " " + (this.asc ? "▼" : "▲"));
380+
381+
})
382+
function comparer(index) {
383+
return function (a, b) {
384+
var valA = getCellValue(a, index), valB = getCellValue(b, index);
385+
return $.isNumeric(valA) && $.isNumeric(valB) ? valB - valA : valA.toString().localeCompare(valB);
386+
}
387+
}
388+
function getCellValue(row, index) {
389+
var td = $(row).children('td').eq(index);
390+
391+
// check for the data-sortby attribute, if found, use that data to sort by
392+
var sortby = td.data('sortby');
393+
394+
if (typeof sortby !== 'undefined') {
395+
return sortby;
396+
}
397+
398+
return td.text();
399+
}
400+
}
401+
358402
// Immediately load initial expander data
359403
showInitialPackagesData("#listed-data", initialData.ListedPackages);
360404
showInitialPackagesData("#unlisted-data", initialData.UnlistedPackages);
@@ -365,6 +409,9 @@
365409
// Set up the data binding.
366410
var managePackagesViewModel = new ManagePackagesViewModel(initialData);
367411
ko.applyBindings(managePackagesViewModel, document.body);
412+
413+
setupColumnSorting();
414+
368415
});
369416

370417
})();

‎src/NuGetGallery/ViewModels/ListPackageItemViewModel.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public class ListPackageItemViewModel : PackageViewModel
2525
public string ShortDescription { get; private set; }
2626
public bool IsDescriptionTruncated { get; set; }
2727
public bool? IsVerified { get; set; }
28+
public int VersionSortOrder { get; set; }
2829
public string SignatureInformation
2930
{
3031
get

‎src/NuGetGallery/Views/Users/Packages.cshtml‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
ViewBag.Title = "Manage My Package";
55
ViewBag.Tab = "Packages";
66
}
7-
87
<section role="main" class="container main-container page-manage-packages">
98
@ViewHelpers.AjaxAntiForgeryToken(Html)
109

@@ -113,14 +112,14 @@
113112
<thead>
114113
<tr class="manage-package-headings">
115114
<th class="hidden-xs"><span class="hidden">Package Icon</span></th>
116-
<th>Package ID</th>
117-
<th>Owners</th>
115+
<th class="sortable">Package ID</th>
116+
<th class="sortable">Owners</th>
118117
@if (Model.IsCertificatesUIEnabled)
119118
{
120-
<th>Signing Owner</th>
119+
<th class="sortable">Signing Owner</th>
121120
}
122-
<th>Downloads</th>
123-
<th>Latest Version</th>
121+
<th class="sortable">Downloads</th>
122+
<th class="sortable">Latest Version</th>
124123
<th><span class="hidden">Icon</span></th>
125124
</tr>
126125
</thead>
@@ -166,10 +165,10 @@
166165
<!-- /ko -->
167166
</td>
168167
}
169-
<td class="align-middle text-nowrap">
168+
<td class="align-middle text-nowrap" data-bind="attr: { 'data-sortby': DownloadCount }">
170169
<span data-bind="text: FormattedDownloadCount"></span>
171170
</td>
172-
<td class="align-middle text-nowrap">
171+
<td class="align-middle text-nowrap" data-bind="attr: { 'data-sortby': VersionSortOrder }">
173172
<span data-bind="text: LatestVersion"></span>
174173
</td>
175174
<td class="text-right align-middle package-controls">
@@ -353,7 +352,8 @@
353352
CanDelete = p.CanUnlistOrRelist,
354353
p.CanEditRequiredSigner,
355354
p.ShowRequiredSigner,
356-
p.ShowTextBox
355+
p.ShowTextBox,
356+
p.VersionSortOrder
357357
};
358358
}
359359

‎tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs‎

Lines changed: 94 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@
1212
using System.Threading.Tasks;
1313
using System.Web;
1414
using System.Web.Mvc;
15+
1516
using Moq;
17+
1618
using NuGet.Services.Entities;
1719
using NuGet.Services.Messaging.Email;
20+
1821
using NuGetGallery.Areas.Admin;
1922
using NuGetGallery.Areas.Admin.Models;
2023
using NuGetGallery.Areas.Admin.ViewModels;
@@ -24,6 +27,7 @@
2427
using NuGetGallery.Infrastructure.Authentication;
2528
using NuGetGallery.Infrastructure.Mail.Messages;
2629
using NuGetGallery.Security;
30+
2731
using Xunit;
2832

2933
namespace NuGetGallery
@@ -3704,43 +3708,115 @@ public void DeleteHappyAccount(bool withPendingIssues)
37043708

37053709
public class ThePackagesAction : TestContainer
37063710
{
3707-
[Fact]
3708-
public void UsesProperIconUrl()
3711+
private UsersController _testController;
3712+
private User _testUser;
3713+
private string userName = "RegularUser";
3714+
private Fakes _fakes;
3715+
3716+
public ThePackagesAction()
37093717
{
3710-
string userName = "RegularUser";
3711-
var controller = GetController<UsersController>();
3712-
var fakes = Get<Fakes>();
3713-
var testUser = fakes.CreateUser(userName);
3714-
testUser.IsDeleted = false;
3715-
testUser.Key = 1;
3716-
controller.SetCurrentUser(testUser);
3718+
_testController = GetController<UsersController>();
3719+
_fakes = Get<Fakes>();
3720+
_testUser = _fakes.CreateUser(userName);
3721+
_testUser.IsDeleted = false;
3722+
_testUser.Key = 1;
3723+
_testController.SetCurrentUser(_testUser);
3724+
}
37173725

3726+
private PackageRegistration CreatePackageRegistration(string Id, int Key, string Version, string Description)
3727+
{
37183728
var packageRegistration = new PackageRegistration();
3719-
packageRegistration.Owners.Add(testUser);
3729+
packageRegistration.Id = Id;
3730+
packageRegistration.Owners.Add(_testUser);
37203731

3721-
var userPackage = new Package()
3732+
var userPackage1 = new Package()
37223733
{
3723-
Description = "TestPackage",
3724-
Key = 1,
3725-
Version = "1.0.0",
3734+
Key = Key,
3735+
Version = Version,
37263736
PackageRegistration = packageRegistration,
3737+
Description = Description
37273738
};
3728-
packageRegistration.Packages.Add(userPackage);
3739+
packageRegistration.Packages.Add(userPackage1);
3740+
return packageRegistration;
3741+
}
3742+
3743+
[Fact]
3744+
public void PackagesAreSortedById()
3745+
{
3746+
PackageRegistration packageRegistration1 = CreatePackageRegistration("Company.ZebraPackage", 1, "1.0.0", "last");
3747+
PackageRegistration packageRegistration2 = CreatePackageRegistration("Company.AlphaPackage", 1, "1.0.0", "first");
3748+
PackageRegistration packageRegistration3 = CreatePackageRegistration("Company.NormalPackage", 1, "1.0.0", "middle");
37293749

3750+
var userPackages = new List<Package>() {
3751+
packageRegistration1.Packages.First() ,
3752+
packageRegistration2.Packages.First(),
3753+
packageRegistration3.Packages.First()
3754+
};
3755+
3756+
GetMock<IUserService>()
3757+
.Setup(stub => stub.FindByUsername(userName, false))
3758+
.Returns(_testUser);
3759+
3760+
GetMock<IPackageService>()
3761+
.Setup(stub => stub.FindPackagesByAnyMatchingOwner(_testUser, It.IsAny<bool>(), false))
3762+
.Returns(userPackages);
3763+
3764+
var model = ResultAssert.IsView<ManagePackagesViewModel>(_testController.Packages());
3765+
3766+
Assert.Equal("Company.AlphaPackage", model.ListedPackages.ToArray()[0].Id);
3767+
Assert.Equal("Company.NormalPackage", model.ListedPackages.ToArray()[1].Id);
3768+
Assert.Equal("Company.ZebraPackage", model.ListedPackages.ToArray()[2].Id);
3769+
}
3770+
3771+
[Fact]
3772+
public void PackagesVersionSortOrderIsSetBySemVer()
3773+
{
3774+
PackageRegistration packageRegistration1 = CreatePackageRegistration("Company.ZebraPackage", 1, "1.0.0", "middle");
3775+
PackageRegistration packageRegistration2 = CreatePackageRegistration("Company.NormalPackage", 1, "0.0.1", "first");
3776+
PackageRegistration packageRegistration3 = CreatePackageRegistration("Company.AlphaPackage", 1, "1.1.0", "last");
3777+
3778+
var userPackages = new List<Package>() {
3779+
packageRegistration1.Packages.First() ,
3780+
packageRegistration2.Packages.First(),
3781+
packageRegistration3.Packages.First()
3782+
};
3783+
3784+
GetMock<IUserService>()
3785+
.Setup(stub => stub.FindByUsername(userName, false))
3786+
.Returns(_testUser);
3787+
3788+
GetMock<IPackageService>()
3789+
.Setup(stub => stub.FindPackagesByAnyMatchingOwner(_testUser, It.IsAny<bool>(), false))
3790+
.Returns(userPackages);
3791+
3792+
var model = ResultAssert.IsView<ManagePackagesViewModel>(_testController.Packages());
3793+
3794+
// The "VersionSortOrder" should be set according to Semantic Version sort order, not package Id
3795+
Assert.Equal(0, model.ListedPackages.First(x => x.Version == "0.0.1").VersionSortOrder);
3796+
Assert.Equal(1, model.ListedPackages.First(x => x.Version == "1.0.0").VersionSortOrder);
3797+
Assert.Equal(2, model.ListedPackages.First(x => x.Version == "1.1.0").VersionSortOrder);
3798+
}
3799+
3800+
[Fact]
3801+
public void UsesProperIconUrl()
3802+
{
3803+
PackageRegistration packageRegistration1 = CreatePackageRegistration("TestPackage", 1, "1.0.0", "TestPackage");
3804+
Package userPackage = packageRegistration1.Packages.First();
37303805
var userPackages = new List<Package>() { userPackage };
37313806

37323807
const string iconUrl = "https://some.test/icon";
3808+
37333809
GetMock<IIconUrlProvider>()
37343810
.Setup(iup => iup.GetIconUrlString(It.IsAny<Package>()))
37353811
.Returns(iconUrl);
37363812
GetMock<IUserService>()
37373813
.Setup(stub => stub.FindByUsername(userName, false))
3738-
.Returns(testUser);
3814+
.Returns(_testUser);
37393815
GetMock<IPackageService>()
3740-
.Setup(stub => stub.FindPackagesByAnyMatchingOwner(testUser, It.IsAny<bool>(), false))
3816+
.Setup(stub => stub.FindPackagesByAnyMatchingOwner(_testUser, It.IsAny<bool>(), false))
37413817
.Returns(userPackages);
37423818

3743-
var model = ResultAssert.IsView<ManagePackagesViewModel>(controller.Packages());
3819+
var model = ResultAssert.IsView<ManagePackagesViewModel>(_testController.Packages());
37443820

37453821
GetMock<IIconUrlProvider>()
37463822
.Verify(iup => iup.GetIconUrlString(userPackage), Times.AtLeastOnce);

0 commit comments

Comments
 (0)