Fixing popularity transfer download count bug, surfacing additional validation info #9710
Conversation
|
Not blocking, 1 improvement idea. Could we have |
@erdembayar Thanks for pointing this out, I've fixed it now |
advay26
dismissed stale reviews from mariaghiondea and erdembayar
via
b8aa851
| @@ -23,6 +23,17 @@ public IReadOnlyList<PackageRename> GetPackageRenames(PackageRegistration packag | |||
| { | |||
| return _packageRenameRepository.GetAll() | |||
| .Where(pr => pr.FromPackageRegistrationKey == packageRegistration.Key) | |||
| .Include(pr => pr.FromPackageRegistration) | |||
| .Include(pr => pr.ToPackageRegistration) | |||
| .ToList(); | |||
There was a problem hiding this comment.
@advay26
Are you going to address this? After this I can approve again.
There was a problem hiding this comment.
.GetAll() doesn't currently support ToListAsync(). EF Core would allow us to use this, but regular Entity Framework doesn't. I spoke to @agr offline, and we won't look to fix this right now with this change.
For context, this is the error it throws when I tried to make the call async:
The source IQueryable doesn't implement IDbAsyncEnumerable<NuGet.Services.Entities.PackageRename>. Only sources that implement IDbAsyncEnumerable can be used for Entity Framework asynchronous operations.
The Popularity Transfer Admin Panel has a bug where it shows version-specific download counts for packages, rather than the package registration's download count, which is what we want (we transfer popularity from one package registration to another). I had to refactor some code to get that data correctly. This now works:
(DEV packages: https://dev.nugettest.org/packages/Newtonsoft.Json/)
I also added some more validation info to the Admin Panel page, which should save us from having to go look in the SQL DB manually every time every time:
