Skip to content

Commit

Permalink
deletesymbols should use selectlist
Browse files Browse the repository at this point in the history
  • Loading branch information
Scott Bommarito committed Feb 26, 2019
1 parent 41100b7 commit f916241
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 53 deletions.
44 changes: 26 additions & 18 deletions src/NuGetGallery/Controllers/PackagesController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1435,9 +1435,25 @@ public virtual async Task<ActionResult> Manage(string id, string version = null)
[RequiresAccountConfirmation("delete a symbols package")]
public virtual ActionResult DeleteSymbols(string id, string version)
{
var package = _packageService.FindPackageByIdAndVersion(id, version, SemVerLevelKey.SemVer2);
Package package = null;

// Load all versions of the package.
var packages = _packageService.FindPackagesById(id);
if (version != null)
{
// Try to find the exact version if it was specified.
package = packages.SingleOrDefault(p => p.NormalizedVersion == NuGetVersionFormatter.Normalize(version));
}

if (package == null)
{
// If the exact version was not found, fall back to the latest version.
package = _packageService.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, allowPrerelease: true);
}

if (package == null)
{
// If the package has no versions, return not found.
return HttpNotFound();
}

Expand All @@ -1449,26 +1465,18 @@ public virtual ActionResult DeleteSymbols(string id, string version)

var model = new DeletePackageViewModel(package, currentUser, DeleteReasons);

// Fetch all the available symbols package for all the versions from the
// database since the DisplayPackageViewModel(base class for DeletePackageViewModel) does not
// set the `LatestSymbolsPackage` data on the model(since it is an unnecessary and expensive db
// query). It is fine to do this here when invoking delete page. Note: this could also potentially
// cause unbounded(high number) db calls based on the number of versions associated with a package.
var packageViewModelsForAllAvailableSymbolsPackage = package
.PackageRegistration
.Packages
// Fetch all versions of the package with symbols.
var versionsWithSymbols = packages
.Where(p => p.PackageStatusKey != PackageStatus.Deleted)
.Select(p => p.LatestSymbolPackage())
.Where(sp => sp != null && sp.StatusKey == PackageStatus.Available)
.Select(sp => new PackageViewModel(sp.Package));
.Where(p => (p.LatestSymbolPackage()?.StatusKey ?? PackageStatus.Deleted) == PackageStatus.Available);

model.VersionSelectList = new SelectList(
packageViewModelsForAllAvailableSymbolsPackage
.Select(pvm => new
model.VersionSelectList = versionsWithSymbols
.Select(versionWithSymbols => new SelectListItem
{
text = pvm.NuGetVersion.ToFullString() + (pvm.LatestVersionSemVer2 ? " (Latest)" : string.Empty),
url = Url.DeleteSymbolsPackage(pvm)
}), "url", "text", Url.DeleteSymbolsPackage(model));
Text = PackageHelper.GetSelectListText(versionWithSymbols),
Value = Url.DeleteSymbolsPackage(new TrivialPackageVersionModel(versionWithSymbols)),
Selected = package == versionWithSymbols
});

return View(model);
}
Expand Down
117 changes: 82 additions & 35 deletions tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2484,11 +2484,11 @@ public class TheManageMethodWithMissingVersion : TheManageMethodWithLatestVersio
}
}

public class TheDeleteSymbolsMethod : TestContainer
public abstract class TheDeleteSymbolsMethod : TestContainer
{
private string _packageId = "CrestedGecko";
private PackageRegistration _packageRegistration;
private Package _package;
protected string _packageId = "CrestedGecko";
protected PackageRegistration _packageRegistration;
protected Package _package;

public TheDeleteSymbolsMethod()
{
Expand All @@ -2501,6 +2501,7 @@ public TheDeleteSymbolsMethod()
Key = 2,
PackageRegistration = _packageRegistration,
Version = "1.0.0+metadata",
NormalizedVersion = "1.0.0",
Listed = true,
IsLatestSemVer2 = true,
HasReadMe = false,
Expand All @@ -2511,6 +2512,7 @@ public TheDeleteSymbolsMethod()
Key = 1,
PackageRegistration = _packageRegistration,
Version = "1.0.0-alpha",
NormalizedVersion = "1.0.0-alpha",
IsLatest = true,
IsLatestSemVer2 = true,
Listed = true,
Expand All @@ -2527,7 +2529,14 @@ public TheDeleteSymbolsMethod()
[Fact]
public void Returns404IfPackageNotFound()
{
var controller = CreateController(GetConfigurationService());
var packageService = new Mock<IPackageService>();
packageService
.Setup(x => x.FindPackagesById(_packageRegistration.Id, false))
.Returns(new Package[0]);

var controller = CreateController(
GetConfigurationService(),
packageService: packageService);

var result = controller.DeleteSymbols(_packageRegistration.Id, _package.Version);

Expand Down Expand Up @@ -2609,8 +2618,8 @@ public void DisplaysFullVersionStringAndUsesNormalizedVersionsInUrlsInSelectList

foreach (var pkg in _packageRegistration.Packages)
{
var valueField = controller.Url.DeleteSymbolsPackage(model);
var textField = model.NuGetVersion.ToFullString() + (pkg.IsLatestSemVer2 ? " (Latest)" : string.Empty);
var valueField = controller.Url.DeleteSymbolsPackage(new TrivialPackageVersionModel(pkg));
var textField = PackageHelper.GetSelectListText(pkg);

var selectListItem = model.VersionSelectList
.SingleOrDefault(i => string.Equals(i.Text, textField) && string.Equals(i.Value, valueField));
Expand All @@ -2621,30 +2630,13 @@ public void DisplaysFullVersionStringAndUsesNormalizedVersionsInUrlsInSelectList
}
}

[Fact]
public void WhenPackageRegistrationIsLockedReturnsLockedState()
[Theory]
[MemberData(nameof(Owner_Data))]
public void WhenPackageRegistrationIsLockedReturnsLockedState(User currentUser, User owner)
{
// Arrange
var user = new User("Frodo") { Key = 1 };
var packageRegistration = new PackageRegistration { Id = "Foo", IsLocked = true };
packageRegistration.Owners.Add(user);

var package = new Package
{
Key = 2,
PackageRegistration = packageRegistration,
Version = "1.0.0+metadata",
};

var packageService = new Mock<IPackageService>(MockBehavior.Strict);
packageService.Setup(svc => svc.FindPackageByIdAndVersion("Foo", "1.0.0", SemVerLevelKey.SemVer2, true))
.Returns(package);

var controller = CreateController(GetConfigurationService(), packageService: packageService);
controller.SetCurrentUser(user);
_packageRegistration.IsLocked = true;

// Act
var result = controller.DeleteSymbols("Foo", "1.0.0");
var result = GetDeleteSymbolsResult(currentUser, owner, out var controller);

// Assert
var model = ResultAssert.IsView<DeletePackageViewModel>(result);
Expand All @@ -2655,11 +2647,7 @@ private ActionResult GetDeleteSymbolsResult(User currentUser, User owner, out Pa
{
_packageRegistration.Owners.Add(owner);

var packageService = new Mock<IPackageService>(MockBehavior.Strict);
packageService
.Setup(svc => svc.FindPackageByIdAndVersion(_packageId, _package.Version, SemVerLevelKey.SemVer2, true))
.Returns(_package).Verifiable();

var packageService = CreatePackageService();
controller = CreateController(
GetConfigurationService(),
packageService: packageService);
Expand All @@ -2669,11 +2657,70 @@ private ActionResult GetDeleteSymbolsResult(User currentUser, User owner, out Pa
Routes.RegisterRoutes(routeCollection);
controller.Url = new UrlHelper(controller.ControllerContext.RequestContext, routeCollection);

var result = controller.DeleteSymbols(_packageId, _package.Version);
var result = InvokeDeleteSymbols(controller);

packageService.Verify();
return result;
}

protected abstract Mock<IPackageService> CreatePackageService();

protected abstract ActionResult InvokeDeleteSymbols(PackagesController controller);
}

public class TheDeleteSymbolsMethodWithExactVersion : TheDeleteSymbolsMethod
{
protected override Mock<IPackageService> CreatePackageService()
{
var packageService = new Mock<IPackageService>(MockBehavior.Strict);
packageService
.Setup(svc => svc.FindPackagesById(_packageId, false))
.Returns(_packageRegistration.Packages.ToList())
.Verifiable();

return packageService;
}

protected override ActionResult InvokeDeleteSymbols(PackagesController controller)
{
return controller.DeleteSymbols(_packageId, _package.Version);
}
}

public abstract class TheDeleteSymbolsMethodThatFilters : TheDeleteSymbolsMethod
{
protected override Mock<IPackageService> CreatePackageService()
{
var packages = _packageRegistration.Packages.ToList();
var packageService = new Mock<IPackageService>(MockBehavior.Strict);
packageService
.Setup(svc => svc.FindPackagesById(_packageId, false))
.Returns(packages)
.Verifiable();

packageService
.Setup(svc => svc.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, true))
.Returns(_package)
.Verifiable();

return packageService;
}
}

public class TheDeleteSymbolsMethodWithMissingVersion : TheDeleteSymbolsMethodThatFilters
{
protected override ActionResult InvokeDeleteSymbols(PackagesController controller)
{
return controller.DeleteSymbols(_packageId, "missing");
}
}

public class TheDeleteSymbolsMethodWithNullVersion : TheDeleteSymbolsMethodThatFilters
{
protected override ActionResult InvokeDeleteSymbols(PackagesController controller)
{
return controller.DeleteSymbols(_packageId, null);
}
}

public class TheDeleteSymbolsPackageMethod : TestContainer
Expand Down

0 comments on commit f916241

Please sign in to comment.