Merge lp:~mvo/software-center/appview-tweaks into lp:software-center

[Michael Vogt]

This branch is based on the very nice work in lp:~gary-lasker/software-center/sorting-fix-lp969215

The main reason for it is combine user_defined_search_sort_method and user_defined_sort_method
into the single single variable. Test test cases from #969215 and #966878 are working nicely for me
and the tests work as well.

But given how close we are to the end of the cycle and how delicate this code is, I would appreciate
a extra critical review of this to ensure we do not add regressions.

[Gary Lasker (gary-lasker)]

Hi Michael. Thanks for this, I like it very much! I just have one small detail that I am not sure that we really want to do. It's the part of softwarecenter/ui/gtk3/panes/availablepane.py where we reset to the default search mode every time we change the category (the change at line 693). I think that it's preferable to just remove this piece, so that even when navigating between categories we retain the search criteria that the user has set.

I personally prefer this and I also think that it is in the spirit of the fix for bug 966878. Note the comment in the bug description "This does not occur when browsing catagories and it seems to remember the setting fine". We will be breaking that expectation if we do this reset.

Please let me know if you agree or not. Other than this one detail to be decided, I am all for merging this branch! I'll set it to approved in any case.

Thanks again, Michael!

review: Approve

[Michael Vogt (mvo)]

Thanks for your review. I agree, its indeed better to leave that out and not reset the sorting on category changes. I will revert that change.

[lp:~mvo/software-center/appview-tweaks]

2969. By Michael Vogt on 2012-04-13

softwarecenter/ui/gtk3/panes/availablepane.py: do not reseting the sort mode on category change, that is a bit too much, thanks to Gary Lasker for spotting this

=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
--- softwarecenter/ui/gtk3/panes/availablepane.py	2012-04-11 18:47:43 +0000
+++ softwarecenter/ui/gtk3/panes/availablepane.py	2012-04-13 11:33:23 +0000
@@ -533,7 +533,7 @@
 
         # reset the flag in the app_view because each new search should
         # reset the sort criteria
-        self.app_view.user_defined_search_sort_method = False
+        self.app_view.reset_default_sort_mode()
 
         self.state.search_term = new_text
 

=== modified file 'softwarecenter/ui/gtk3/panes/softwarepane.py'
--- softwarecenter/ui/gtk3/panes/softwarepane.py	2012-04-11 14:38:36 +0000
+++ softwarecenter/ui/gtk3/panes/softwarepane.py	2012-04-13 11:33:23 +0000
@@ -486,11 +486,7 @@
         if (self.state.category and
             self.state.category.sortmode != SortMethods.BY_ALPHABET):
             return self.state.category.sortmode
-        # searches are always by ranking unless the user decided differently
-        if (self._is_in_search_mode() and
-            not self.app_view.user_defined_sort_method):
-            return SortMethods.BY_SEARCH_RANKING
-        # use the appview combo
+        # ask the app_view for the sort-mode
         return self.app_view.get_sort_mode()
 
     def on_search_entry_key_press_event(self, event):

=== modified file 'softwarecenter/ui/gtk3/views/appview.py'
--- softwarecenter/ui/gtk3/views/appview.py	2012-04-11 18:50:42 +0000
+++ softwarecenter/ui/gtk3/views/appview.py	2012-04-13 11:33:23 +0000
@@ -95,8 +95,7 @@
         self.vadj = 0.0
 
         # list view sorting stuff
-        self.user_defined_sort_method = False
-        self.user_defined_search_sort_method = False
+        self._force_default_sort_method = True
         self._handler = self.sort_methods_combobox.connect(
                                     "changed",
                                     self.on_sort_method_changed)
@@ -132,7 +131,6 @@
         pass
 
     def on_sort_method_changed(self, *args):
-        self.user_defined_sort_method = True
         self.vadj = 0.0
         self.emit("sort-method-changed", self.sort_methods_combobox)
 
@@ -205,23 +203,37 @@
         self.tree_view_scroll.get_vadjustment().set_lower(self.vadj)
         self.tree_view_scroll.get_vadjustment().set_value(self.vadj)
 
+    def reset_default_sort_mode(self):
+        """ force the appview to reset to the default sort method without
+            doing a refresh or sending any signals
+        """
+        self._force_default_sort_method = True
+
     def configure_sort_method(self, is_search=False):
         """ configures the sort method UI appropriately based on current
-            conditions, including whether a search is in progress
+            conditions, including whether a search is in progress.
+
+            Note that this will not change the users current sort method,
+            if that is the intention, call reset_default_sort_mode()
         """
-        if is_search and not self.user_defined_search_sort_method:
-            # the first results for any new search should always be returned
-            # sorted by relevance
+        # figure out what combobox we need
+        if is_search:
             self._use_combobox_with_sort_by_search_ranking()
-            self.set_sort_method_with_no_signal(
-                                        self._SORT_BY_SEARCH_RANKING)
-            self.user_defined_search_sort_method = True
         else:
-            if not is_search:
-                self._use_combobox_without_sort_by_search_ranking()
-            if ((self.get_sort_mode() == SortMethods.BY_SEARCH_RANKING) and
-                not self.user_defined_sort_method):
-                self.set_sort_method_with_no_signal(self._SORT_BY_TOP_RATED)
+            self._use_combobox_without_sort_by_search_ranking()
+
+        # and what sorting
+        if self._force_default_sort_method:
+            # always reset this, its the job of the user of the appview
+            # to call reset_default_sort_mode() to reset this
+            self._force_default_sort_method = False
+            # and now set the default sort depending on if its a view or not
+            if is_search:
+                self.set_sort_method_with_no_signal(
+                    self._SORT_BY_SEARCH_RANKING)
+            else:
+                self.set_sort_method_with_no_signal(
+                    self._SORT_BY_TOP_RATED)
 
     def clear_model(self):
         return self.tree_view.clear_model()

=== modified file 'test/gtk3/test_app_view.py'
--- test/gtk3/test_app_view.py	2012-04-11 19:23:32 +0000
+++ test/gtk3/test_app_view.py	2012-04-13 11:33:23 +0000
@@ -80,7 +80,6 @@
 
         # now repeat and simulate a search
         matches = get_test_enquirer_matches(appview.helper.db)
-        appview.user_defined_search_sort_method = False
         appview.display_matches(matches, is_search=True)
         appview.configure_sort_method(is_search=True)
         do_events()
@@ -91,7 +90,6 @@
 
         # and back again to no search
         matches = get_test_enquirer_matches(appview.helper.db)
-        appview.user_defined_search_sort_method = False
         appview.display_matches(matches, is_search=False)
         appview.configure_sort_method(is_search=False)
         do_events()