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

Manage packages page / user profile broken #1624

Merged
merged 2 commits into from
Oct 7, 2013
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
30 changes: 15 additions & 15 deletions src/NuGetGallery/Controllers/UsersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,17 @@ public virtual ActionResult Thanks()
public virtual ActionResult Packages()
{
var user = UserService.FindByUsername(Identity.Name);
var packages = PackageService.FindPackagesByOwner(user);
var packages = PackageService.FindPackagesByOwner(user, includeUnlisted: true)
.Select(p => new PackageViewModel(p)
{
DownloadCount = p.PackageRegistration.DownloadCount,
Version = null
}).ToList();

var model = new ManagePackagesViewModel
{
Packages = from p in packages
select new PackageViewModel(p)
{
DownloadCount = p.PackageRegistration.DownloadCount,
Version = null
},
};
{
Packages = packages
};
return View(model);
}

Expand Down Expand Up @@ -276,12 +276,12 @@ public virtual ActionResult Profiles(string username)
return HttpNotFound();
}

var packages = (from p in PackageService.FindPackagesByOwner(user)
where p.Listed
orderby p.Version descending
group p by p.PackageRegistration.Id)
.Select(c => new PackageViewModel(c.First()))
.ToList();
var packages = PackageService.FindPackagesByOwner(user, includeUnlisted: false)
.Select(p => new PackageViewModel(p)
{
DownloadCount = p.PackageRegistration.DownloadCount,
Version = null
}).ToList();

var model = new UserProfileModel(user)
{
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/Services/IPackageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public interface IPackageService
PackageRegistration FindPackageRegistrationById(string id);
Package FindPackageByIdAndVersion(string id, string version, bool allowPrerelease = true);
IQueryable<Package> GetPackagesForListing(bool includePrerelease);
IEnumerable<Package> FindPackagesByOwner(User user);
IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted);
IEnumerable<Package> FindDependentPackages(Package package);

/// <summary>
Expand Down
21 changes: 10 additions & 11 deletions src/NuGetGallery/Services/PackageService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,20 +155,22 @@ public IQueryable<Package> GetPackagesForListing(bool includePrerelease)
: packages.Where(p => p.IsLatestStable);
}

public IEnumerable<Package> FindPackagesByOwner(User user)
public IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted)
{
// Like DisplayPackage we should prefer to show you information from the latest stable version,
// but show you the latest version otherwise.

var latestStablePackageVersions = _packageRegistrationRepository.GetAll()
.Where(pr => pr.Owners.Where(owner => owner.Username == user.Username).Any())
.Select(pr => pr.Packages.Where(p => p.IsLatestStable).FirstOrDefault())
var latestStablePackageVersions = _packageRepository.GetAll()
.Where(p =>
p.PackageRegistration.Owners.Any(owner => owner.Key == user.Key)
&& p.IsLatestStable)
.Include(p => p.PackageRegistration)
.Include(p => p.PackageRegistration.Owners);

var latestPackageVersions = _packageRegistrationRepository.GetAll()
.Where(pr => pr.Owners.Where(owner => owner.Username == user.Username).Any())
.Select(pr => pr.Packages.OrderByDescending(p => p.Version).FirstOrDefault())
var latestPackageVersions = _packageRepository.GetAll()
.Where(p =>
p.PackageRegistration.Owners.Any(owner => owner.Key == user.Key)
&& p.IsLatest)
.Include(p => p.PackageRegistration)
.Include(p => p.PackageRegistration.Owners);

Expand All @@ -179,10 +181,7 @@ public IEnumerable<Package> FindPackagesByOwner(User user)
}
foreach (var package in latestStablePackageVersions)
{
if (package != null)
{
mergedResults[package.PackageRegistration.Id] = package;
}
mergedResults[package.PackageRegistration.Id] = package;
Copy link
Contributor

Choose a reason for hiding this comment

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

There's gotta be a LINQier way to do this, but whatever :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Enumerable.Union maybe? :)

}

return mergedResults.Values;
Expand Down
18 changes: 18 additions & 0 deletions tests/NuGetGallery.Facts/Framework/TestContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ protected TController GetController<TController>() where TController : Controlle
return c;
}

protected TService GetService<TService>()
{
var serviceInterfaces = typeof(TService).GetInterfaces();
Kernel.Bind(serviceInterfaces).To(typeof(TService));
return Get<TService>();
}

protected FakeEntitiesContext GetFakeContext()
{
var fakeContext = new FakeEntitiesContext();
Kernel.Bind<IEntitiesContext>().ToConstant(fakeContext);
Kernel.Bind<IEntityRepository<Package>>().ToConstant(new EntityRepository<Package>(fakeContext));
Kernel.Bind<IEntityRepository<PackageOwnerRequest>>().ToConstant(new EntityRepository<PackageOwnerRequest>(fakeContext));
Kernel.Bind<IEntityRepository<PackageStatistics>>().ToConstant(new EntityRepository<PackageStatistics>(fakeContext));
Kernel.Bind<IEntityRepository<PackageRegistration>>().ToConstant(new EntityRepository<PackageRegistration>(fakeContext));
return fakeContext;
}

protected T Get<T>()
{
if(typeof(Controller).IsAssignableFrom(typeof(T))) {
Expand Down
105 changes: 105 additions & 0 deletions tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Threading.Tasks;
using Moq;
using NuGet;
using NuGetGallery.Framework;
using NuGetGallery.Packaging;
using Xunit;

Expand Down Expand Up @@ -1151,6 +1152,110 @@ public void FindPackageReturnsTheLatestVersionIfNoLatestVersionIsAvailable()
}
}

public class TheFindPackagesByOwnerMethod : TestContainer
{
[Fact]
public void ReturnsAListedPackage()
{
var owner = new User { Username = "someone" };
var packageRegistration = new PackageRegistration { Id = "theId", Owners = { owner }};
var package = new Package { Version = "1.0", PackageRegistration = packageRegistration, Listed = true, IsLatest = true, IsLatestStable = true };
packageRegistration.Packages.Add(package);

var context = GetFakeContext();
context.Users.Add(owner);
context.PackageRegistrations.Add(packageRegistration);
context.Packages.Add(package);
var service = Get<PackageService>();

var packages = service.FindPackagesByOwner(owner, includeUnlisted: false);
Assert.Equal(1, packages.Count());
}

[Fact]
public void ReturnsNoUnlistedPackagesWhenIncludeUnlistedIsFalse()
{
var owner = new User { Username = "someone" };
var packageRegistration = new PackageRegistration { Id = "theId", Owners = { owner } };
var package = new Package { Version = "1.0", PackageRegistration = packageRegistration, Listed = false, IsLatest = false, IsLatestStable = false };
packageRegistration.Packages.Add(package);

var context = GetFakeContext();
context.Users.Add(owner);
context.PackageRegistrations.Add(packageRegistration);
context.Packages.Add(package);
var service = Get<PackageService>();

var packages = service.FindPackagesByOwner(owner, includeUnlisted: false);
Assert.Equal(0, packages.Count());
}

[Fact]
public void ReturnsAnUnlistedPackageWhenIncludeUnlistedIsTrue()
{
var owner = new User { Username = "someone" };
var packageRegistration = new PackageRegistration { Id = "theId", Owners = { owner } };
var package = new Package { Version = "1.0", PackageRegistration = packageRegistration, Listed = true, IsLatest = true, IsLatestStable = true };
packageRegistration.Packages.Add(package);

var context = GetFakeContext();
context.Users.Add(owner);
context.PackageRegistrations.Add(packageRegistration);
context.Packages.Add(package);
var service = Get<PackageService>();

var packages = service.FindPackagesByOwner(owner, includeUnlisted: true);
Assert.Equal(1, packages.Count());
}

[Fact]
public void ReturnsAPackageForEachPackageRegistration()
{
var owner = new User { Username = "someone" };
var packageRegistrationA = new PackageRegistration { Id = "idA", Owners = { owner } };
var packageRegistrationB = new PackageRegistration { Id = "idB", Owners = { owner } };
var packageA = new Package { Version = "1.0", PackageRegistration = packageRegistrationA, Listed = true, IsLatest = true, IsLatestStable = true };
var packageB = new Package { Version = "1.0", PackageRegistration = packageRegistrationB, Listed = true, IsLatest = true, IsLatestStable = true };
packageRegistrationA.Packages.Add(packageA);
packageRegistrationB.Packages.Add(packageB);

var context = GetFakeContext();
context.Users.Add(owner);
context.PackageRegistrations.Add(packageRegistrationA);
context.PackageRegistrations.Add(packageRegistrationB);
context.Packages.Add(packageA);
context.Packages.Add(packageB);
var service = Get<PackageService>();

var packages = service.FindPackagesByOwner(owner, includeUnlisted: false).ToList();
Assert.Equal(2, packages.Count);
Assert.Contains(packageA, packages);
Assert.Contains(packageB, packages);
}

[Fact]
public void ReturnsOnlyLatestStablePackageIfBothExist()
{
var owner = new User { Username = "someone" };
var packageRegistration = new PackageRegistration { Id = "theId", Owners = { owner } };
var latestPackage = new Package { Version = "2.0.0-alpha", PackageRegistration = packageRegistration, Listed = true, IsLatest = true };
var latestStablePackage = new Package { Version = "1.0", PackageRegistration = packageRegistration, Listed = true, IsLatestStable = true };
packageRegistration.Packages.Add(latestPackage);
packageRegistration.Packages.Add(latestStablePackage);

var context = GetFakeContext();
context.Users.Add(owner);
context.PackageRegistrations.Add(packageRegistration);
context.Packages.Add(latestPackage);
context.Packages.Add(latestStablePackage);
var service = Get<PackageService>();

var packages = service.FindPackagesByOwner(owner, includeUnlisted: false).ToList();
Assert.Equal(1, packages.Count);
Assert.Contains(latestStablePackage, packages);
}
}

public class TheMarkPackageListedMethod
{
[Fact]
Expand Down
12 changes: 12 additions & 0 deletions tests/NuGetGallery.Facts/TestUtils/FakeEntitiesContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ public IDbSet<PackageRegistration> PackageRegistrations
}
}

public IDbSet<Package> Packages
{
get
{
return Set<Package>();
}
set
{
throw new NotSupportedException();
}
}

public IDbSet<User> Users
{
get
Expand Down