don't enforce_ua and updated tests to not check for redirect #42

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

No description provided.

Working on merge conflicts.

@davedash davedash commented on the diff Feb 22, 2012

apps/feedback/tests/test_feedback_views.py
@@ -9,7 +9,7 @@
import input
from input import FIREFOX, OPINION_PRAISE, OPINION_ISSUE
-from input.tests import ViewTestCase, enforce_ua
@davedash

davedash Feb 22, 2012

Member

Can you remove enforce_ua entirely?

@readevalprint

readevalprint Feb 22, 2012

sure thing

On Wed, Feb 22, 2012 at 1:52 PM, Dave Dash
reply@reply.github.com
wrote:

@@ -9,7 +9,7 @@

 import input
 from input import FIREFOX, OPINION_PRAISE, OPINION_ISSUE
-from input.tests import ViewTestCase, enforce_ua

Can you remove enforce_ua entirely?


Reply to this email directly or view it on GitHub:
https://github.com/mozilla/input.mozilla.org/pull/42/files#r477873

@davedash davedash commented on the diff Feb 22, 2012

apps/feedback/views.py
@@ -25,6 +25,8 @@ def enforce_ua(f):
Can be disabled with settings.ENFORCE_USER_AGENT = False.
"""
+ raise DeprecationWarning('This function is no longer in use.')
@davedash

davedash Feb 22, 2012

Member

can't we just remove it.

Can we also remove the download page altogether?

@davedash davedash commented on the diff Feb 22, 2012

apps/feedback/views.py
@@ -72,6 +73,7 @@ def feedback(request, ua):
form = IdeaForm(request.POST, auto_id='idea-%s')
if form.is_valid():
+ ua = request.META.get('HTTP_USER_AGENT', None)
@davedash

davedash Feb 22, 2012

Member

.get's default argument is already None.

@readevalprint

readevalprint Feb 22, 2012

is it? i thought it raised an exception

@davedash

davedash Feb 22, 2012

Member
d = dict(foo='bar')
d['hi'] # exception
d.get('hi') # None

@davedash davedash commented on the diff Feb 22, 2012

apps/feedback/tests/test_release_views.py
@@ -15,31 +15,9 @@ def _get_page(self, ver=None):
return self.client.get(reverse('feedback'), **extra)
@enforce_ua
- def test_no_ua(self):
+ def test_feedback(self):
"""No UA: redirect."""
@davedash

davedash Feb 22, 2012

Member

this docstring seems wrong now.

Member

davedash commented Feb 22, 2012

There should be one test that has a FX user agent and we should query using the ORM for the opinion and making sure everything is logged (correct Fx version, etc).

Then we should have one where we go in with a Chrome string and make sure no FX metadata is collected.

Member

mythmon commented Nov 21, 2012

Since this is 9 months old, I'm going to close it to work on cleaning up pull requests. If you still want to merge this, please reopen.

mythmon closed this Nov 21, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment