Opened on 04/02/2015 at 02:21:37 PM
Closed on 02/08/2019 at 09:02:13 PM
#2267 closed change (fixed)
Unify form handling by reusing the form_handler decorator and encode_email_address()
| Reported by: | sebastian | Assignee: | rhowell |
|---|---|---|---|
| Priority: | P4 | Milestone: | |
| Module: | Sitescripts | Keywords: | goodfirstbug |
| Cc: | Blocked By: | #2234 | |
| Blocking: | Platform: | Unknown | |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): |
http://codereview.adblockplus.org/5717434384252928 |
||
Description (last modified by sebastian)
Background
With #2234 the form_handler decorator is introduced to encapsulate the logic of validating headers and parsing the request body, when processing form data. This logic is currently duplicated in sitescripts.formmail.web.formmail and sitescripts.reports.web.updateReport. So those controllers need to be updated to use the form_handlerdecorator as well.
Also with #2234, a reusable function to validate email addresses were introduced (i.e. sitescripts.utils.encode_email_address). However, sitescripts.formmail.web.formmail still validates email addresses differently.
What to change
Remove logic parsing form data from sitescripts.formmail.web.formmail and sitescripts.reports.web.updateReport and adapt those controllers to use the form_handler decorator.
Remove logic for validating email addresses from sitescripts.formmail.web.formmail and use sitescripty.utils.encode_email_address() instead.
While on it, also remove the call of the deprecated setupStderr() function from the scripts changed here.
Attachments (0)
Change History (8)
comment:1 Changed on 04/02/2015 at 02:24:13 PM by sebastian
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 04/24/2015 at 12:49:22 PM by sebastian
- Description modified (diff)
- Summary changed from Unify form handling by reusing the form_handler decorator to Unify form handling by reusing the form_handler decorator and encode_email_address()
comment:3 Changed on 09/21/2017 at 01:56:41 AM by sebastian
- Keywords goodfirstbug added
- Owner sebastian deleted
- Tester set to Unknown
comment:4 Changed on 01/19/2019 at 12:32:34 AM by rhowell
- Owner set to rhowell
comment:5 Changed on 01/19/2019 at 12:35:35 AM by rhowell
In #4413, the previous version of formmail.py was replaced with formmail2.py. The latter already uses the form_handler decorator and encode_email_address(), so it looks like no change is necessary there, but these changes can be applied in /sitescripts/reports/web/updateReport.py.
comment:7 Changed on 02/08/2019 at 08:58:34 PM by abpbot
A commit referencing this issue has landed:
Issue 2267 - Unify form handling by reusing form_handler()
comment:8 Changed on 02/08/2019 at 09:02:13 PM by rhowell
- Resolution set to fixed
- Status changed from reviewing to closed
I never got around to properly test my patch, for two years now, hence it is unlikely to happen any time soon. It might need rebasing too. Therefore I'm unassigning myself. If anybody else wants to rebase, test and land the patch, go ahead. ;)