Merge lp:~fabricematrat/charmworld/redirect-left into lp:charmworld

[Fabrice Matrat]

Added redirect for search, home, charmers

Added redirection for home, search, charmers

[Francesco Banconi (frankban)]

Looks good, thank you.

review: Approve

[Brad Crittenden (bac)]

Nice, no QA yet.

review: Approve (code)

[Brad Crittenden (bac)]

QA-not-OK. The /charmers redirect appears to be broken, unless I am exercising it incorrectly.

review: Needs Fixing (qa)

[Brad Crittenden (bac)]

QA-OK. Thanks for pointing me to the new endpoint.

review: Approve (code, qa)

[Juju Gui Bot (juju-gui-bot)]

FAILED: Autolanding.
No commit message was specified in the merge proposal. Hit 'Add commit message' on the merge proposal web page or follow the link below. You can approve the merge proposal yourself to rerun.
https://code.launchpad.net/~fabricematrat/charmworld/redirect-left/+merge/249169/+edit-commit-message

review: Needs Fixing (continuous-integration)

[Juju Gui Bot (juju-gui-bot)]

review: Approve (continuous-integration)

=== modified file 'charmworld/views/misc.py'
--- charmworld/views/misc.py	2013-11-13 12:48:24 +0000
+++ charmworld/views/misc.py	2015-02-10 11:13:22 +0000
@@ -1,8 +1,6 @@
 # Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
-
-from collections import defaultdict
-
+from pyramid.httpexceptions import HTTPMovedPermanently
 from pyramid.view import view_config
 
 from charmworld import cached_view_config
@@ -27,26 +25,17 @@
     route_name="home",
     renderer="charmworld:templates/index.pt")
 def home(request):
-    return {}
+    redirect_url = request.registry.settings.get('redirect_jujucharms')
+    raise HTTPMovedPermanently(location=redirect_url)
 
 
 @view_config(
     route_name="charmers",
     renderer="charmworld:templates/charmers.pt")
 def charmers(request):
-    # We want to collect which users have contributes on which charms.
-    # This however reports charm owners ordered decending by ownership.
-    # The largest owner is not included because we think this is ~charmers.
-    results = request.db.charms.find(
-        {}, {"name": 1, 'owner': 1, 'short_url': 1})
-
-    owner_map = defaultdict(int)
-    for i in results:
-        owner_map[i['owner']] += 1
-    owners = sorted(
-        owner_map.items(), key=lambda i: (i[1], i[0]), reverse=True)
-    owners.pop(0)
-    return {"owners": owners}
+    redirect_url = request.registry.settings.get('redirect_jujucharms')
+    location = '{url}/community/charmers'.format(url=redirect_url)
+    raise HTTPMovedPermanently(location=location)
 
 
 @view_config(

=== modified file 'charmworld/views/search.py'
--- charmworld/views/search.py	2014-03-18 15:20:47 +0000
+++ charmworld/views/search.py	2015-02-10 11:13:22 +0000
@@ -1,46 +1,19 @@
 # Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
-from pyelasticsearch.exceptions import ElasticHttpError
-from pyramid.httpexceptions import HTTPFound
-from pyramid.renderers import render_to_response
+from pyramid.httpexceptions import HTTPMovedPermanently
 from pyramid.view import view_config
 
-from charmworld.search import (
-    IndexNotReady,
-    SearchServiceNotAvailable,
-)
-from charmworld.views import log
-
-
-def do_search(request):
-    text = request.params["search_text"]
-    return request.index_client.search(text)
-
 
 @view_config(
     route_name="search",
     renderer="charmworld:templates/search.pt")
 def search(request):
+    redirect_url = request.registry.settings.get('redirect_jujucharms')
+    location = '{url}/q'.format(url=redirect_url)
     if not "search_text" in request.params:
-        url = request.route_url("home")
-        return HTTPFound(location=url)
-    try:
-        search = do_search(request)
-    except (IndexNotReady, SearchServiceNotAvailable):
-        response = render_to_response('../templates/search-not-ready.pt', {},
-                                      request=request)
-        response.status_code = 503
-        return response
-    except ElasticHttpError as exc:
-        text = request.params["search_text"]
-        log.warning(
-            "User search error with search text='{}'".format(text))
-        log.exception(exc)
-        response = render_to_response(
-            '../templates/search-terms-bad.pt',
-            {'search_text': text},
-            request=request)
-        response.status_code = 400
-        return response
-    return dict((key, value) for key, value in search.items()
-                if key != 'matches')
+        raise HTTPMovedPermanently(location=location)
+    text = request.params["search_text"].strip()
+    text = '/'.join(text.split())
+    location = '{location}/{search}'.format(location=location,
+                                            search=text)
+    raise HTTPMovedPermanently(location=location)

=== modified file 'charmworld/views/tests/test_auth.py'
--- charmworld/views/tests/test_auth.py	2013-09-04 14:48:30 +0000
+++ charmworld/views/tests/test_auth.py	2015-02-10 11:13:22 +0000
@@ -40,19 +40,12 @@
 
     def test_starts_with_login(self):
         """Without being logged in you get the login nav link."""
-        resp = self.app.get('/', status=200)
-
-        body = resp.body
-        self.assertIn("/login/openid", body)
-        self.assertNotIn("/logout", body)
+        self.app.get('/', status=301)
 
     def test_after_login_see_logout(self):
         """Once logged in you get the logout nav link."""
         with login_request(self.app) as resp:
-            resp.goto('/', status=200)
-            body = resp.body
-            self.assertNotIn("/login/openid", body)
-            self.assertIn("/logout", body)
+            resp.goto('/', status=301)
 
     def test_login_route(self):
         path = '/login/openid'

=== modified file 'charmworld/views/tests/test_misc.py'
--- charmworld/views/tests/test_misc.py	2013-11-14 13:38:24 +0000
+++ charmworld/views/tests/test_misc.py	2015-02-10 11:13:22 +0000
@@ -2,7 +2,6 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import os
-import re
 import unittest
 
 from mock import patch
@@ -74,32 +73,6 @@
         else:
             self.fail('Cache-Control header not found.')
 
-    def test_main_template_scheme(self):
-        """Image and stylesheet URLs respect the WSGI scheme."""
-        src_pattern = re.compile('src="([a-z]+)://')
-        css_pattern = re.compile('href="([a-z]+)://.*\.css.*"')
-        # Https enabled.
-        response = self.app.get(
-            '/', extra_environ={'wsgi.url_scheme': 'https'})
-        https_count = 0
-        for match in src_pattern.finditer(response.ubody):
-            https_count += 1
-            self.assertEqual('https', match.group(1))
-        for match in css_pattern.finditer(response.ubody):
-            https_count += 1
-            self.assertEqual('https', match.group(1))
-        self.assertTrue(https_count >= 2)
-        # Http default.
-        response = self.app.get('/')
-        http_count = 0
-        for match in src_pattern.finditer(response.ubody):
-            http_count += 1
-            self.assertEqual('http', match.group(1))
-        for match in css_pattern.finditer(response.ubody):
-            http_count += 1
-            self.assertEqual('http', match.group(1))
-        self.assertTrue(https_count == http_count)
-
 
 class HeartbeatWebTestCase(WebTestBase):
 

=== modified file 'charmworld/views/tests/test_search.py'
--- charmworld/views/tests/test_search.py	2013-11-15 20:00:35 +0000
+++ charmworld/views/tests/test_search.py	2015-02-10 11:13:22 +0000
@@ -1,14 +1,8 @@
 # Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-from mock import patch
-from pyelasticsearch import ElasticSearch
-from pyelasticsearch.exceptions import ElasticHttpError
+from pyramid.httpexceptions import HTTPMovedPermanently
 
-from charmworld.search import (
-    ElasticSearchClient,
-    IndexNotReady,
-)
 from charmworld.testing import (
     ViewTestBase,
     WebTestBase,
@@ -26,72 +20,26 @@
 
 class SearchTests(ViewTestBase):
 
-    @patch('charmworld.views.search.do_search',
-           autospec=True, return_value=RETURN_VAL)
-    def test_search(self, mock):
-        terms = {"search_text": 'foo'}
-        keys = ['result_total', 'results', 'search_time']
-        request = self.getRequest(params=terms)
-        response = search.search(request)
-
-        mock.assert_called_with(request)
-        self.assertEqual(keys, sorted(response.keys()))
-        self.assertEqual(0, response['result_total'])
-        self.assertEqual([], response['results'])
-        self.assertEqual(0.0, response['search_time'])
-
-    @patch('charmworld.views.search.do_search',
-           autospec=True, side_effect=IndexNotReady)
-    def test_search_503(self, mock):
-        terms = {"search_text": 'foo'}
-        request = self.getRequest(params=terms)
-        # render_to_response requires the request to be much more like a real
-        # request, with scheme and static stuff handling, so mock it out.
-        with patch('charmworld.views.search.render_to_response') as render:
-            response = search.search(request)
-        render.assert_called_with('../templates/search-not-ready.pt', {},
-                                  request=request)
-        self.assertEqual(503, response.status_code)
-
-    def test_search_503_on_connection_error(self):
-        terms = {"search_text": 'foo'}
-        request = self.getRequest(params=terms)
-        request.index_client = ElasticSearchClient(
-            ElasticSearch(['http://localhost:70']), 'foo')
-        # render_to_response requires the request to be much more like a real
-        # request, with scheme and static stuff handling, so mock it out.
-        with patch('charmworld.views.search.render_to_response') as render:
-            response = search.search(request)
-        render.assert_called_with('../templates/search-not-ready.pt', {},
-                                  request=request)
-        self.assertEqual(503, response.status_code)
-
-    def test_tildes_not_in_start_are_unchanged(self):
-        terms = {"search_text": 'fo~o'}
-        request = self.getRequest(params=terms)
-        request.index_client = ElasticSearchClient(
-            ElasticSearch(['http://localhost:70']), '')
-        with patch.object(request.index_client, 'search') as req_search:
-            search.search(request)
-            req_search.assert_called_with('fo~o')
-
-    def test_otherwise_tilde_causes_400(self):
-        terms = {"search_text": 'fo~o'}
-        request = self.getRequest(params=terms)
-        request.index_client = ElasticSearchClient(
-            ElasticSearch(['http://localhost:70']), '')
-
-        def mock_search(request):
-            raise ElasticHttpError('oops')
-
-        with patch.object(request.index_client, 'search', mock_search):
-            with patch('charmworld.views.search.render_to_response') as render:
-                response = search.search(request)
-        render.assert_called_with(
-            '../templates/search-terms-bad.pt',
-            {'search_text': 'fo~o'},
-            request=request)
-        self.assertEqual(400, response.status_code)
+    def test_search(self):
+        terms = {"search_text": 'foo'}
+        request = self.getRequest(params=terms)
+        with self.assertRaises(HTTPMovedPermanently) as e:
+            search.search(request)
+        self.assertIn('/q/foo', e.exception.location)
+
+    def test_search_multiple(self):
+        terms = {"search_text": 'foo bar'}
+        request = self.getRequest(params=terms)
+        with self.assertRaises(HTTPMovedPermanently) as e:
+            search.search(request)
+        self.assertIn('/q/foo/bar', e.exception.location)
+
+    def test_search_with_space(self):
+        terms = {"search_text": ' foo   bar '}
+        request = self.getRequest(params=terms)
+        with self.assertRaises(HTTPMovedPermanently) as e:
+            search.search(request)
+        self.assertIn('/q/foo/bar', e.exception.location)
 
 
 class SearchJSONTests(WebTestBase):