Merge pull request #424 from michael-lazar/issue_401

Updating sorting options for search results

rtv/content.py

@@ -478,7 +478,7 @@ class SubredditContent(Content):
             query (text): Content to search for on the given subreddit or
                 user's page.
         """
-        # TODO: refactor this into smaller methods
+        # TODO: This desperately needs to be refactored
 
         # Strip leading, trailing, and redundant backslashes
         parts = [seg for seg in name.strip(' /').split('/') if seg]
@@ -521,13 +521,21 @@ class SubredditContent(Content):
         else:
             period = None
 
-        if order not in ['hot', 'top', 'rising', 'new', 'controversial', None]:
+        if query:
+            # The allowed orders for sorting search results are different
+            orders = ['relevance', 'top', 'comments', 'new', None]
+            period_allowed = ['top', 'comments']
+        else:
+            orders = ['hot', 'top', 'rising', 'new', 'controversial', None]
+            period_allowed = ['top', 'controversial']
+
+        if order not in orders:
             raise InvalidSubreddit('Invalid order `%s`' % order)
         if period not in ['all', 'day', 'hour', 'month', 'week', 'year', None]:
             raise InvalidSubreddit('Invalid period `%s`' % period)
-        if period and order not in ['top', 'controversial']:
+        if period and order not in period_allowed:
-            raise InvalidSubreddit('`%s` order does not allow sorting by'
+            raise InvalidSubreddit(
-                                   ' period' % order)
+                '`%s` order does not allow sorting by period' % order)
 
         # On some objects, praw doesn't allow you to pass arguments for the
         # order and period. Instead you need to call special helper functions

rtv/docs.py

@@ -83,6 +83,11 @@ BANNER = """
 [1]hot [2]top [3]rising [4]new [5]controversial
 """
 
+BANNER_SEARCH = """
+[1]relevance [2]top [3]comments [4]new
+"""
+
+
 FOOTER_SUBREDDIT = """
 [?]Help [q]Quit [l]Comments [/]Prompt [u]Login [o]Open [c]Post [a/z]Vote
 """

rtv/page.py

@@ -97,32 +97,29 @@ class Page(object):
 
     @PageController.register(Command('SORT_HOT'))
     def sort_content_hot(self):
-        self.refresh_content(order='hot')
+        if self.content.query:
+            self.refresh_content(order='relevance')
+        else:
+            self.refresh_content(order='hot')
 
     @PageController.register(Command('SORT_TOP'))
     def sort_content_top(self):
-
+        order = self._prompt_period('top')
-        choices = {
+        if order is None:
-            '\n': 'top',
-            '1': 'top-hour',
-            '2': 'top-day',
-            '3': 'top-week',
-            '4': 'top-month',
-            '5': 'top-year',
-            '6': 'top-all'}
-
-        message = docs.TIME_ORDER_MENU.strip().splitlines()
-        ch = self.term.show_notification(message)
-        ch = six.unichr(ch)
-        if ch not in choices:
             self.term.show_notification('Invalid option')
-            return
+        else:
-
+            self.refresh_content(order=order)
-        self.refresh_content(order=choices[ch])
 
     @PageController.register(Command('SORT_RISING'))
     def sort_content_rising(self):
-        self.refresh_content(order='rising')
+        if self.content.query:
+            order = self._prompt_period('comments')
+            if order is None:
+                self.term.show_notification('Invalid option')
+            else:
+                self.refresh_content(order=order)
+        else:
+            self.refresh_content(order='rising')
 
     @PageController.register(Command('SORT_NEW'))
     def sort_content_new(self):
@@ -130,24 +127,14 @@ class Page(object):
 
     @PageController.register(Command('SORT_CONTROVERSIAL'))
     def sort_content_controversial(self):
-
+        if self.content.query:
-        choices = {
+            self.term.flash()
-            '\n': 'controversial',
+        else:
-            '1': 'controversial-hour',
+            order = self._prompt_period('controversial')
-            '2': 'controversial-day',
+            if order is None:
-            '3': 'controversial-week',
+                self.term.show_notification('Invalid option')
-            '4': 'controversial-month',
+            else:
-            '5': 'controversial-year',
+                self.refresh_content(order=order)
-            '6': 'controversial-all'}
-
-        message = docs.TIME_ORDER_MENU.strip().splitlines()
-        ch = self.term.show_notification(message)
-        ch = six.unichr(ch)
-        if ch not in choices:
-            self.term.show_notification('Invalid option')
-            return
-
-        self.refresh_content(order=choices[ch])
 
     @PageController.register(Command('MOVE_UP'))
     def move_cursor_up(self):
@@ -458,7 +445,9 @@ class Page(object):
         ch, attr = str(' '), curses.A_BOLD | Color.YELLOW
         window.bkgd(ch, attr)
 
-        items = docs.BANNER.strip().split(' ')
+        banner = docs.BANNER_SEARCH if self.content.query else docs.BANNER
+        items = banner.strip().split(' ')
+
         distance = (n_cols - sum(len(t) for t in items) - 1) / (len(items) - 1)
         spacing = max(1, int(distance)) * ' '
         text = spacing.join(items)
@@ -584,3 +573,20 @@ class Page(object):
         n_rows, _ = window.getmaxyx()
         for row in range(n_rows):
             window.chgat(row, 0, 1, attribute)
+
+    def _prompt_period(self, order):
+
+        choices = {
+            '\n': order,
+            '1': '{0}-hour'.format(order),
+            '2': '{0}-day'.format(order),
+            '3': '{0}-week'.format(order),
+            '4': '{0}-month'.format(order),
+            '5': '{0}-year'.format(order),
+            '6': '{0}-all'.format(order)}
+
+        message = docs.TIME_ORDER_MENU.strip().splitlines()
+        ch = self.term.show_notification(message)
+        ch = six.unichr(ch)
+        return choices.get(ch)
+

tests/test_page.py

@@ -75,7 +75,8 @@ def test_page_unauthenticated(reddit, terminal, config, oauth):
             page.controller.trigger('?')
         assert Popen.called
 
-        # Sort content
+        # Sort content - normal page
+        page.content.query = ''
         page.controller.trigger('1')
         page.refresh_content.assert_called_with(order='hot')
         page.controller.trigger('3')
@@ -83,6 +84,13 @@ def test_page_unauthenticated(reddit, terminal, config, oauth):
         page.controller.trigger('4')
         page.refresh_content.assert_called_with(order='new')
 
+        # Sort content - search results
+        page.content.query = 'search text'
+        page.controller.trigger('1')
+        page.refresh_content.assert_called_with(order='relevance')
+        page.controller.trigger('4')
+        page.refresh_content.assert_called_with(order='new')
+
         logged_in_methods = [
             'a',  # Upvote
             'z',  # Downvote

tests/test_submission.py

@@ -560,7 +560,7 @@ def test_copy_to_clipboard_linux(submission_page, terminal, refresh_token):
         assert data.get('permalink') == content
         window.addstr.assert_called_with(1, 1, b'Copied permalink to clipboard')
     else:
-        # Nither xclip or xsel installed, this is what happens on Travis CI
+        # Neither xclip or xsel installed, this is what happens on Travis CI
         text = b'Failed to copy permalink: External copy application not found'
         window.addstr.assert_called_with(1, 1, text)
 
@@ -571,6 +571,6 @@ def test_copy_to_clipboard_linux(submission_page, terminal, refresh_token):
         assert data.get('url_full') == content
         window.addstr.assert_called_with(1, 1, b'Copied url to clipboard')
     else:
-        # Nither xclip or xsel installed, this is what happens on Travis CI
+        # Neither xclip or xsel installed, this is what happens on Travis CI
         text = b'Failed to copy url: External copy application not found'
         window.addstr.assert_called_with(1, 1, text)

tests/test_subreddit.py

@@ -196,6 +196,39 @@ def test_subreddit_order_controversial(subreddit_page, terminal):
         assert subreddit_page.content.order == 'controversial'
 
 
+def test_subreddit_order_search(subreddit_page, terminal):
+
+    # Search the current subreddit
+    with mock.patch.object(terminal, 'prompt_input'):
+        terminal.prompt_input.return_value = 'search term'
+        subreddit_page.controller.trigger('f')
+        assert subreddit_page.content.name == '/r/python'
+        assert terminal.prompt_input.called
+        assert not terminal.loader.exception
+
+    # Sort by relevance
+    subreddit_page.controller.trigger('1')
+    assert subreddit_page.content.order == 'relevance'
+
+    # Sort by top
+    with mock.patch.object(terminal, 'show_notification'):
+        terminal.show_notification.reset_mock()
+        terminal.show_notification.return_value = ord('6')
+        subreddit_page.controller.trigger('2')
+        assert subreddit_page.content.order == 'top-all'
+
+    # Sort by comments
+    with mock.patch.object(terminal, 'show_notification'):
+        terminal.show_notification.reset_mock()
+        terminal.show_notification.return_value = ord('6')
+        subreddit_page.controller.trigger('3')
+        assert subreddit_page.content.order == 'comments-all'
+
+    # Sort by new
+    subreddit_page.controller.trigger('4')
+    assert subreddit_page.content.order == 'new'
+
+
 def test_subreddit_open(subreddit_page, terminal, config):
 
     # Open the selected submission