848568 - non-firefox browsers shouldn't be allowed to give feedback

Status: RESOLVED FIXED

(Input Graveyard :: Submission, defect)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

The current Input site doesn't let non-Firefox browsers see the feedback page. If you're using (say) Chromium, then you get a page that says something like:

"""
Oh! So, you want to offer us feedback on Firefox. Thank you, but you'll need to be on the latest version.
To do that, simply go to our download page and install/update to the latest version of Firefox!
"""

Fjord lets non-Firefox browsers see the feedback page, but kicks up an HTTP 500 when they try to submit feedback because it parses the user agent and returns None in some critical fields and then that gets rejected by our data model in saving.

Traceback (most recent call last):

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/core/handlers/base.py", line 111, in get_response
    response = callback(request, *callback_args, **callback_kwargs)

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/views/decorators/csrf.py", line 77, in wrapped_view
    return view_func(*args, **kwargs)

  File "/data/www/input-dev.allizom.org/input/fjord/feedback/views.py", line 175, in feedback_router
    return csrf_checked_feedback_router(request, *args, **kwargs)

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/utils/decorators.py", line 91, in _wrapped_view
    response = view_func(request, *args, **kwargs)

  File "/data/www/input-dev.allizom.org/input/fjord/feedback/views.py", line 138, in csrf_checked_feedback_router
    return feedback_router_actual(request, *args, **kwargs)

  File "/data/www/input-dev.allizom.org/input/fjord/feedback/views.py", line 157, in feedback_router_actual
    return view(request, *args, **kwargs)

  File "/data/www/input-dev.allizom.org/input/fjord/feedback/views.py", line 98, in desktop_stable_feedback
    response, form = _handle_feedback_post(request)

  File "/data/www/input-dev.allizom.org/input/fjord/feedback/views.py", line 53, in _handle_feedback_post
    opinion.save()

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/db/models/base.py", line 463, in save
    self.save_base(using=using, force_insert=force_insert, force_update=force_update)

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/db/models/base.py", line 551, in save_base
    result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw)

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/db/models/manager.py", line 203, in _insert
    return insert_query(self.model, objs, fields, **kwargs)

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/db/models/query.py", line 1593, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/db/models/sql/compiler.py", line 912, in execute_sql
    cursor.execute(sql, params)

  File "/data/www/input-dev.allizom.org/input/vendor/lib/python/django/db/backends/mysql/base.py", line 114, in execute
    return self.cursor.execute(query, args)

  File "/usr/lib64/python2.6/site-packages/MySQLdb/cursors.py", line 173, in execute
    self.errorhandler(self, exc, value)

  File "/usr/lib64/python2.6/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue

IntegrityError: (1048, "Column 'browser' cannot be null")


This needs to get fixed before we go to production.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

Grabbing this one. I'll work on it tomorrow.

Comment 2

[James Socol [:jsocol, :james]]

This behavior may be a very old remnant of a requirement.Before reimplementing it, I'd just like to double-check:

Cww:
1) Is the intent still that non-Firefox browsers are not allowed to give feedback?
1.1) If not, is sufficient for Fjord to only accept Firefox for now?
2) Should we change the message? The "Latest" language is from when you could only use the latest released version of Firefox with Input.

Comment 3

[[:Cww]]

Wait, I thought we removed the requirement AGES ago... bah.

I'd like us never to refuse feedback. Ideal case: if they're on Chrome, they get an option to say what version of Firefox they'd like to give feedback for. Next idealest case: let's say that non-Firefox feedback applies to Firefox-latest-release and we store the user agent so I can asterisk it in data processing.

For now, do what's easiest.

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

The easiest code change to make is to prevent non-firefox browsers from leaving feedback and instead they get a page that says "please use firefox to leave firefox feedback." Kind of like what the current old input does, but with better text.

Both of the options in comment #3 will require non-trivial work. I think I'm inclined to do what the current input does and make a bug for doing something better after we go to prod.

I know that's not ideal, but is it ok for now?

Comment 5

[Will Kahn-Greene [:willkg] ET needinfo? me]

I did what I suggested in comment #4 and landed it in https://github.com/mozilla/fjord/commit/7a3a97a9fa55f74eb4f43e69baf535f106383e41

If you go to the feedback page with a non-Firefox browser, then you'll get a "hey--download firefox!" text. I used the same text.

If we can figure out a different low-risk solution, I'm game, but we should do it under a different bug. This is a good stop-gap solution for now that solves the original problem (HTTP 500 errors).

Marking as FIXED.