Merge pull request #1754 from jgmize/release-notes-improvements

Improve release_notes view to fix multiple bugs

bedrock/firefox/tests/test_views.py

@@ -1,7 +1,9 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
+from django.http import Http404
 from django.test.client import RequestFactory
+from django.test.utils import override_settings
 
 from mock import patch
 from nose.tools import eq_
@@ -44,7 +46,7 @@ def test_release_notes(self, get_object_or_404):
         views.release_notes(self.request, '27.0')
         # Should use fixed version for query
         get_object_or_404.assert_called_with(
-            Release, version='27.0.0', channel='Release', product='Firefox')
+            Release, version='27.0', product='Firefox')
         # Should use original version for context variable
         eq_(self.last_ctx['version'], '27.0')
         eq_(self.last_ctx['major_version'], '27')
@@ -63,13 +65,17 @@ def test_system_requirements(self, get_object_or_404):
         """
         views.system_requirements(self.request, '27.0.1')
         get_object_or_404.assert_called_with(
-            Release, version='27.0.1', channel='Release', product='Firefox')
+            Release, version='27.0.1', product='Firefox')
         eq_(self.last_ctx['release'], get_object_or_404.return_value)
         eq_(self.last_ctx['version'], '27.0.1')
         eq_(self.mock_render.call_args[0][1],
             'firefox/releases/system_requirements.html')
 
     def test_release_notes_template(self):
+        """
+        Should return correct template name based on channel
+        and product
+        """
         eq_(views.release_notes_template('', 'Firefox OS'),
             'firefox/releases/os-notes.html')
         eq_(views.release_notes_template('Nightly', 'Firefox'),
@@ -84,3 +90,23 @@ def test_release_notes_template(self):
             'firefox/releases/esr-notes.html')
         eq_(views.release_notes_template('', ''),
             'firefox/releases/release-notes.html')
+
+    @patch('bedrock.firefox.views.get_object_or_404')
+    def test_firefox_os_manual_template(self, get_object_or_404):
+        """
+        Should render from pre-RNA template without querying DB
+        """
+        views.release_notes(self.request, '1.0.1', product='Firefox OS')
+        get_object_or_404.assert_never_called()
+        eq_(self.mock_render.call_args[0][1],
+            'firefox/os/notes-1.0.1.html')
+
+    @override_settings(DEV=False)
+    @patch('bedrock.firefox.views.get_object_or_404')
+    def test_non_public_release(self, get_object_or_404):
+        """
+        Should raise 404 if not release.is_public and not settings.DEV
+        """
+        get_object_or_404.return_value = Release(is_public=False)
+        with self.assertRaises(Http404):
+            views.release_notes(self.request, '42')

bedrock/firefox/urls.py

@@ -16,7 +16,7 @@
 tour_re = latest_re % (version_re, 'tour')
 product_re = '(?P<product>firefox|mobile)'
 channel_re = '(?P<channel>beta|aurora|organizations)'
-releasenotes_re = latest_re % (version_re, 'releasenotes')
+releasenotes_re = latest_re % (version_re, r'(aurora|release)notes')
 sysreq_re = latest_re % (version_re, 'releasenotes/system-requirements')
 
 
@@ -76,9 +76,9 @@
     page('firefox/os/releases', 'firefox/os/releases.html'),
 
     # firefox/os/notes/ should redirect to the latest version; update this in /redirects/urls.py
-    page('firefox/os/notes/1.0.1', 'firefox/os/notes-1.0.1.html'),
-    page('firefox/os/notes/1.1', 'firefox/os/notes-1.1.html'),
-    page('firefox/os/notes/1.2', 'firefox/os/notes-1.2.html'),
+    url('^firefox/os/notes/(?P<fx_version>%s)/$' % version_re,
+        views.release_notes, {'product': 'Firefox OS'},
+        name='firefox.os.releasenotes'),
 
     page('mwc', 'firefox/os/mwc-2014-preview.html'),
     page('firefox/os/devices', 'firefox/os/devices.html'),

bedrock/firefox/views.py

@@ -8,7 +8,8 @@
 import re
 
 from django.conf import settings
-from django.http import HttpResponsePermanentRedirect, HttpResponseRedirect
+from django.http import (
+    Http404, HttpResponsePermanentRedirect, HttpResponseRedirect)
 from django.shortcuts import get_object_or_404
 from django.views.decorators.csrf import csrf_exempt, csrf_protect
 from django.views.decorators.vary import vary_on_headers
@@ -411,37 +412,34 @@ def get(self, request, *args, **kwargs):
         return super(TourView, self).get(request, *args, **kwargs)
 
 
-def fix_fx_version(fx_version):
-    if len(fx_version.split('.')) == 2:
-        return fx_version + '.0'
-    else:
-        return fx_version
-
-
 def release_notes_template(channel, product):
     if product == 'Firefox OS':
         return 'firefox/releases/os-notes.html'
     prefix = dict((c, c.lower()) for c in Release.CHANNELS)
     return 'firefox/releases/%s-notes.html' % prefix.get(channel, 'release')
 
 
-def release_notes(request, fx_version, channel='Release', product='Firefox'):
-    release = get_object_or_404(Release, version=fix_fx_version(fx_version),
-                                channel=channel, product=product)
+def release_notes(request, fx_version, product='Firefox'):
+    if product == 'Firefox OS' and fx_version in (
+            '1.0.1', '1.1', '1.2', '1.3'):
+        return l10n_utils.render(
+            request, 'firefox/os/notes-%s.html' % fx_version)
+
+    release = get_object_or_404(Release, version=fx_version, product=product)
+    if not release.is_public and not settings.DEV:
+        raise Http404
     new_features, known_issues = release.notes()
     return l10n_utils.render(
-        request, release_notes_template(channel, product), {
+        request, release_notes_template(release.channel, product), {
             'version': fx_version,
             'major_version': fx_version.split('.', 1)[0],
             'release': release,
             'new_features': new_features,
             'known_issues': known_issues})
 
 
-def system_requirements(request, fx_version, channel='Release',
-                        product='Firefox'):
-    release = get_object_or_404(Release, version=fix_fx_version(fx_version),
-                                channel=channel, product=product)
+def system_requirements(request, fx_version, product='Firefox'):
+    release = get_object_or_404(Release, version=fx_version, product=product)
     return l10n_utils.render(
         request, 'firefox/releases/system_requirements.html',
         {'release': release, 'version': fx_version})