Merge lp:~gary-lasker/software-center/previous-purchase-sorting-lp873104 into lp:software-center/5.2

Proposed by Gary Lasker on 2012-06-08
Status: Merged
Merged at revision: 3049
Proposed branch: lp:~gary-lasker/software-center/previous-purchase-sorting-lp873104
Merge into: lp:software-center/5.2
Diff against target: 83 lines (+11/-7)
3 files modified
softwarecenter/backend/channel.py (+2/-0)
softwarecenter/ui/gtk3/panes/availablepane.py (+9/-5)
softwarecenter/ui/gtk3/panes/softwarepane.py (+0/-2)
To merge this branch: bzr merge lp:~gary-lasker/software-center/previous-purchase-sorting-lp873104
Reviewer Review Type Date Requested Status
Michael Vogt 2012-06-08 Approve on 2012-06-11
Review via email: mp+109458@code.launchpad.net

Commit message

* lp:~gary-lasker/software-center/previous-purchase-sorting-lp873104:
   - return correct results when sorting the list of previous
     purchases (LP: #873104)
   - fix bug that caused a search of the list of previous purchases to
     return too many items (LP: #969273)

Description of the change

This branch fixes bug 873104 and bug 969273 by replacing the "special case" code for the previous purchases view with in an implementation using a SoftwareChannel. This allows us to use the view state in the way that we do for the other available pane views, and thereby fixes the two issues described by these bug reports.

Please also note that I removed the unused 'previous_purchases_query' from the DisplayState class.

All unit tests pass.

Many thanks for your review!

To post a comment you must log in.
Michael Vogt (mvo) wrote :

Thanks for your branch.

This is great work and a nice improvement, especially that the special case got removed is great.

The change:
29 + self.state.search_term = ""
make me a little bit nervous as the _clear_search() is called in various places, but I could not
think of (or find one by playing around) any regression this might cause so I think its fine.

Unfortunately bug #969273 is only fixed inside the "Previous purchases" subcategory (which is nice),
but its not fixed in the "All software" case which Michael Terry was referring to in the bugreport.
I updated the description there to make the testcase clearer. For me it still returns
three items with this branch. But that is not a regression or anything like this, so I'm happy to
merge the branch as #873104 is fixed.

review: Approve
Michael Vogt (mvo) wrote :

While testing I noticed bug #1011522, but its not a regression in this branch, so this is just a FYI :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'softwarecenter/backend/channel.py'
--- softwarecenter/backend/channel.py 2012-05-21 20:10:39 +0000
+++ softwarecenter/backend/channel.py 2012-06-08 23:52:19 +0000
@@ -237,6 +237,8 @@
237 channel_display_name = self.distro.get_distro_channel_description()237 channel_display_name = self.distro.get_distro_channel_description()
238 elif channel_name == "For Purchase":238 elif channel_name == "For Purchase":
239 channel_display_name = _("For Purchase")239 channel_display_name = _("For Purchase")
240 elif channel_name == "Previous Purchases":
241 channel_display_name = _("Previous Purchases")
240 elif channel_name == "Application Review Board PPA":242 elif channel_name == "Application Review Board PPA":
241 channel_display_name = _("Independent")243 channel_display_name = _("Independent")
242 elif channel_name == "notdownloadable":244 elif channel_name == "notdownloadable":
243245
=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
--- softwarecenter/ui/gtk3/panes/availablepane.py 2012-05-30 12:34:44 +0000
+++ softwarecenter/ui/gtk3/panes/availablepane.py 2012-06-08 23:52:19 +0000
@@ -48,6 +48,7 @@
48from softwarepane import SoftwarePane48from softwarepane import SoftwarePane
49from softwarecenter.ui.gtk3.session.viewmanager import get_viewmanager49from softwarecenter.ui.gtk3.session.viewmanager import get_viewmanager
50from softwarecenter.ui.gtk3.session.appmanager import get_appmanager50from softwarecenter.ui.gtk3.session.appmanager import get_appmanager
51from softwarecenter.backend.channel import SoftwareChannel
51from softwarecenter.backend.unitylauncher import (UnityLauncher,52from softwarecenter.backend.unitylauncher import (UnityLauncher,
52 UnityLauncherInfo,53 UnityLauncherInfo,
53 TransactionDetails)54 TransactionDetails)
@@ -537,6 +538,7 @@
537 self.searchentry.clear_with_no_signal()538 self.searchentry.clear_with_no_signal()
538 self.apps_limit = 0539 self.apps_limit = 0
539 self.apps_search_term = ""540 self.apps_search_term = ""
541 self.state.search_term = ""
540542
541 def _is_custom_list_search(self, search_term):543 def _is_custom_list_search(self, search_term):
542 return (search_term and544 return (search_term and
@@ -697,15 +699,15 @@
697699
698 def display_previous_purchases(self, page, view_state):700 def display_previous_purchases(self, page, view_state):
699 self.nonapps_visible = NonAppVisibility.ALWAYS_VISIBLE701 self.nonapps_visible = NonAppVisibility.ALWAYS_VISIBLE
700 self.app_view.set_header_labels(_("Previous Purchases"), None)702 header_strings = self._get_header_for_view_state(view_state)
703 self.app_view.set_header_labels(*header_strings)
701 self.notebook.set_current_page(AvailablePane.Pages.LIST)704 self.notebook.set_current_page(AvailablePane.Pages.LIST)
702 # clear any search terms705 # clear any search terms
703 self._clear_search()706 self._clear_search()
704 # do not emit app-list-changed here, this is done async when707 self.refresh_apps()
705 # the new model is ready
706 self.refresh_apps(query=self.previous_purchases_query)
707 self.action_bar.clear()708 self.action_bar.clear()
708 self.cat_view.stop_carousels()709 self.cat_view.stop_carousels()
710 return True
709711
710 def on_subcategory_activated(self, subcat_view, category):712 def on_subcategory_activated(self, subcat_view, category):
711 LOG.debug("on_subcategory_activated: %s %s" % (713 LOG.debug("on_subcategory_activated: %s %s" % (
@@ -761,7 +763,9 @@
761 """ called to activate the previous purchases view """763 """ called to activate the previous purchases view """
762 #print cat_view, name, query764 #print cat_view, name, query
763 LOG.debug("on_previous_purchases_activated with query: %s" % query)765 LOG.debug("on_previous_purchases_activated with query: %s" % query)
764 self.previous_purchases_query = query766 self.state.channel = SoftwareChannel("Previous Purchases",
767 "software-center-agent",
768 None, channel_query=query)
765 vm = get_viewmanager()769 vm = get_viewmanager()
766 vm.display_page(self, AvailablePane.Pages.LIST, self.state,770 vm.display_page(self, AvailablePane.Pages.LIST, self.state,
767 self.display_previous_purchases)771 self.display_previous_purchases)
768772
=== modified file 'softwarecenter/ui/gtk3/panes/softwarepane.py'
--- softwarecenter/ui/gtk3/panes/softwarepane.py 2012-05-17 13:57:45 +0000
+++ softwarecenter/ui/gtk3/panes/softwarepane.py 2012-06-08 23:52:19 +0000
@@ -66,7 +66,6 @@
66 'application': (type(None), Application),66 'application': (type(None), Application),
67 'limit': (int,),67 'limit': (int,),
68 'filter': (type(None), AppFilter),68 'filter': (type(None), AppFilter),
69 'previous_purchases_query': (type(None), xapian.Query),
70 'vadjustment': (float, ),69 'vadjustment': (float, ),
71 }70 }
7271
@@ -78,7 +77,6 @@
78 self.application = None77 self.application = None
79 self.limit = 078 self.limit = 0
80 self.filter = None79 self.filter = None
81 self.previous_purchases_query = None
82 self.vadjustment = 0.080 self.vadjustment = 0.0
8381
84 def __setattr__(self, name, val):82 def __setattr__(self, name, val):

Subscribers

People subscribed via source and target branches