Ubuntu Apps Directory

Merge lp://staging/~zematynnad/ubuntu-webcatalog/duplicates_987826 into lp://staging/ubuntu-webcatalog

duplicates_987826
Merge into trunk

Proposed by Danny Tamez on 2012-05-23

Status:

Merged

Approved by:

Anthony Lenton on 2012-05-23

Approved revision:

127

Merged at revision:

122

Proposed branch:

lp://staging/~zematynnad/ubuntu-webcatalog/duplicates_987826

Merge into:

lp://staging/ubuntu-webcatalog

Diff against target:

93 lines (+41/-10)

2 files modified

src/webcatalog/tests/test_views.py (+26/-5)
src/webcatalog/views.py (+15/-5)

To merge this branch:

bzr merge lp://staging/~zematynnad/ubuntu-webcatalog/duplicates_987826

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Anthony Lenton (community)		2012-05-23	Approve on 2012-05-23
Review via email: mp+107091@code.staging.launchpad.net

Commit message

Removes duplicate apps from top rated apps widget.

Description of the change

Overview
=========
This branch is a fix for http://pad.lv/987826 where duplicate apps were showing up in the top rated apps carousel.

Details
========
The problem was that apps for the same package but for different distroseries were being returned in the query. Since django doesn't do distinct on individual fields (at least not without a patch) the duplicates have to be filtered out a different way. I just used a dictionary with the packagename as the key to avoid the duplicates.

To Test
========
$fab bootstrap test

Revision history for this message

Anthony Lenton (elachuni) wrote on 2012-05-23:

Hi Danny,

I really don't think the code is doing what we want there. Comments about the changes in src/webcatalog/views.py:
- You iterate over all of top_rated_apps (tens of thousands of apps) to keep just the first settings.NUMBER_TOP_RATED_APPS off the filtered list (tens of apps). See if there's a way to only filter enough apps as we need.
- For each package name you currently keep the last item on the list that matches the name (as the dict is overridden each time you find a new Application instance with the same package name), so I you're effectively keeping the *lowest* rated app instance for each package name.
- Finally, you're storing the apps in a dict and then asking for values(), so you can't expect any order in top_rated_apps.
- And, something really minor, Count is being imported unnecessarily.

Kind regards,

achuni.

review: Needs Fixing

lp://staging/~zematynnad/ubuntu-webcatalog/duplicates_987826 updated on 2012-05-23

126. By Danny Tamez on 2012-05-23: Remove unused import.
Ensure that as few rows as necessary are iterated on.
Ensure that order is preserved in final result.
127. By Danny Tamez on 2012-05-23: Refactor to filter as few as needed.