Opened 4 years ago

Closed 6 months ago

#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
https://codereview.adblockplus.org/29993614/

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.

Change History (8)

comment:1 Changed 4 years ago by sebastian

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 4 years ago 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 2 years ago by sebastian

  • Keywords goodfirstbug added
  • Owner sebastian deleted
  • Tester set to Unknown

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. ;)

comment:4 Changed 7 months ago by rhowell

  • Owner set to rhowell

comment:5 Changed 7 months ago 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:6 Changed 7 months ago by rhowell

  • Review URL(s) modified (diff)

comment:7 Changed 6 months ago by abpbot

A commit referencing this issue has landed:
Issue 2267 - Unify form handling by reusing form_handler()

comment:8 Changed 6 months ago by rhowell

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.