Issue 2267 - Unify form handling by reusing form_handler()

sitescripts/reports/web/updateReport.py

Index: sitescripts/reports/web/updateReport.py
===================================================================
--- a/sitescripts/reports/web/updateReport.py
+++ b/sitescripts/reports/web/updateReport.py
@@ -14,19 +14,29 @@
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import re
-import os
-import sys
 import random
 from urlparse import parse_qsl
-from sitescripts.utils import get_config, get_template, setupStderr
+from sitescripts.utils import get_config, get_template
 from sitescripts.web import url_handler
-from sitescripts.reports.utils import calculateReportSecret, calculateReportSecret_compat, getReport, saveReport, sendUpdateNotification, getUserId, updateUserUsefulness
+from sitescripts.reports.utils import (calculateReportSecret,
+                                       calculateReportSecret_compat, getReport,
+                                       saveReport, sendUpdateNotification,
+                                       getUserId, updateUserUsefulness)
+
+
+def _getReport(guid, test_mode):
+    if not test_mode:
+        return getReport(guid)
+    return {'usefulness': 1}

[rhowell, 2019/02/02 05:45:22]

If I also return an email address here, we can test more of the code in
handleRequest(), but then we'd also have to intercept updateUserUsefulness() and
sendUpdateNotification(). Wanted to check with you about how it looks so far
before potentially going too far in the wrong direction.

[Vasily Kuznetsov, 2019/02/04 17:48:50]

I think it would make more sense to mock getReport and saveReport (in the
testing code) instead of introducing all this testing-related logic here in
production code. Something like this:

def test_xxx(mocker):
    mocker.patch('sitescripts.reports.web.updateReport.getReport',
                 new=lambda *args: {'usefulness': 1})
    mocker.patch('sitescripts.reports.web.updateReport.saveReport',
                 new=test_saveReport)
    # do our thing and the patching will be undone at the end of the test.

See the documentation of pytest-mock (https://pypi.org/project/pytest-mock/) for
more info on how it works.

[rhowell, 2019/02/07 03:54:32]

Done.

+
+
+def _saveReport(guid, report_data, test_mode):
+    if not test_mode:
+        return saveReport(guid, report_data)
 
 
 @url_handler('/updateReport')
-def handleRequest(environ, start_response):
-    setupStderr(environ['wsgi.errors'])
-
+def handleRequest(environ, start_response, test_mode=False):
     if environ['REQUEST_METHOD'].upper() != 'POST' or not environ.get('CONTENT_TYPE', '').startswith('application/x-www-form-urlencoded'):
         return showError('Unsupported request method', start_response)
 
@@ -44,7 +54,7 @@
     if not re.match(r'^[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}$', guid):
         return showError('Invalid or missing report GUID', start_response)
 
-    reportData = getReport(guid)
+    reportData = _getReport(guid, params['test_mode'])
 
     if reportData == None:
         return showError('Report does not exist', start_response)
@@ -59,10 +69,11 @@
 
     oldusefulness = reportData.get('usefulness', '0')
     reportData['usefulness'] = params.get('usefulness', '0')
+
     if 'email' in reportData:
         updateUserUsefulness(getUserId(reportData['email']), reportData['usefulness'], oldusefulness)
 
-    saveReport(guid, reportData)
+    _saveReport(guid, reportData, params['test_mode'])
 
     if params.get('notify', '') and 'email' in reportData:
         email = reportData['email']
@@ -78,6 +89,7 @@
     newURL = get_config().get('reports', 'urlRoot') + guid
     newURL += '?updated=' + str(int(random.uniform(0, 10000)))
     newURL += '#secret=' + secret
+
     start_response('302 Found', [('Location', newURL.encode('utf-8'))])
     return []