redirect /<carrier>/(.*) to /$1?carrier=<carrier> (bug 900614)

mkt/carriers/middleware.py

@@ -1,5 +1,7 @@
+from django.shortcuts import redirect
 from django.utils.cache import patch_vary_headers
 
+from amo.helpers import urlparams
 from amo.urlresolvers import set_url_prefix
 from mkt.constants.carriers import CARRIER_MAP
 
@@ -51,7 +53,7 @@ class CarrierURLMiddleware(object):
         if carrier and is_legacy:
             orig_path = request.path_info
             new_path = orig_path[len(carrier) + 1:] or '/'
-            request.path_info = new_path
+            return redirect(urlparams(new_path, carrier=carrier))
 
         set_carrier(carrier)
 

mkt/carriers/tests.py

@@ -1,5 +1,3 @@
-from django.conf import settings
-
 import mock
 from nose.tools import eq_
 from test_utils import RequestFactory
@@ -12,7 +10,6 @@ from .middleware import CarrierURLMiddleware
 
 
 class TestCarrierURLs(TestCase):
-    fixtures = ['base/users']
 
     def setUp(self):
         set_carrier(None)
@@ -30,11 +27,6 @@ class TestCarrierURLs(TestCase):
         CarrierURLMiddleware().process_request(request)
         return request
 
-    def test_strip_carrier(self):
-        request = self.get('/telefonica/foo')
-        eq_(request.path_info, '/foo')
-        assert request.set_cookie.called
-
     def test_ignore_non_carriers(self):
         request = self.get('/not-a-store')
         eq_(request.path_info, '/not-a-store')
@@ -45,11 +37,6 @@ class TestCarrierURLs(TestCase):
         eq_(get_carrier(), 'telefonica')
         assert request.set_cookie.called
 
-    def test_set_carrier_url(self):
-        request = self.get('/telefonica/')
-        eq_(get_carrier(), 'telefonica')
-        assert request.set_cookie.called
-
     def test_set_carrier_none(self):
         request = self.request('/?carrier=')
         request.COOKIES = {'carrier': 'telefonica'}
@@ -57,20 +44,31 @@ class TestCarrierURLs(TestCase):
         eq_(get_carrier(), None)
         assert request.set_cookie.called
 
-    def test_set_carrer_to_none_url(self):
+    def test_set_carrier_to_none_url(self):
         self.get('/telefonica/')
         self.get('/not-a-store')
         eq_(get_carrier(), None)
 
+        self.get('/?carrier=telefonica')
+        self.get('/?carrier=not-a-store')
+        eq_(get_carrier(), None)
+
     def test_reverse(self):
         self.get('/telefonica/')
         eq_(reverse('manifest.webapp'), '/manifest.webapp')
 
-    def test_context(self):
-        request = self.get('/telefonica/')
-        ctx = context_processors.carrier_data(request)
-        eq_(ctx['CARRIER'], 'telefonica')
+        self.get('/?carrier=telefonica')
+        eq_(reverse('manifest.webapp'), '/manifest.webapp')
 
-    def test_root_url(self):
+    def test_legacy_redirect(self):
+        res = self.client.get('/telefonica/')
+        self.assert3xx(res, '/?carrier=telefonica')
+
+        res = self.client.get('/telefonica/foo')
+        self.assert3xx(res, '/foo?carrier=telefonica')
+
+    def test_context(self):
         request = self.get('/?carrier=telefonica')
         eq_(request.path_info, '/')
+        ctx = context_processors.carrier_data(request)
+        eq_(ctx['CARRIER'], 'telefonica')

mkt/detail/urls.py

@@ -8,10 +8,13 @@ from mkt.stats.urls import app_stats_patterns
 from . import views
 
 
+DummyResponse = lambda *args, **kw: HttpResponse()
+
+
 urlpatterns = patterns('',
-    url('^$', HttpResponse, name='detail'),
-    url('^abuse$', HttpResponse, name='detail.abuse'),
-    url('^privacy$', HttpResponse, name='detail.privacy'),
+    url('^$', DummyResponse, name='detail'),
+    url('^abuse$', DummyResponse, name='detail.abuse'),
+    url('^privacy$', DummyResponse, name='detail.privacy'),
 
     # Merge app purchase / receipt patterns.
     ('^purchase/', include(app_purchase_patterns)),

mkt/ratings/urls.py

@@ -5,11 +5,14 @@ from mkt.ratings.feeds import RatingsRss
 from reviews.views import delete as amo_delete, flag as amo_flag
 
 
+DummyResponse = lambda *args, **kw: HttpResponse()
+
+
 # These all start with /apps/<app_slug>/reviews/<review_id>/.
 detail_patterns = patterns('',
     # Used by Fireplace.
-    url('^$', HttpResponse, name='ratings.detail'),
-    url('^edit$', HttpResponse, name='ratings.edit'),
+    url('^$', DummyResponse, name='ratings.detail'),
+    url('^edit$', DummyResponse, name='ratings.edit'),
 
     # Used by Reviewer Tools.
     url('^flag$', amo_flag, name='ratings.flag'),
@@ -20,10 +23,10 @@ detail_patterns = patterns('',
 # These all start with /apps/<app_slug>/reviews/.
 review_patterns = patterns('',
     # Used by Fireplace.
-    url('^$', HttpResponse, name='ratings.list'),
-    url('^add$', HttpResponse, name='ratings.add'),
+    url('^$', DummyResponse, name='ratings.list'),
+    url('^add$', DummyResponse, name='ratings.add'),
     url('^(?P<review_id>\d+)/', include(detail_patterns)),
-    url('^user:(?P<user_id>\d+)$', HttpResponse, name='ratings.user'),
+    url('^user:(?P<user_id>\d+)$', DummyResponse, name='ratings.user'),
 
     # TODO: The API should expose this.
     url('^format:rss$', RatingsRss(), name='ratings.list.rss'),