Remove email invariant and add adapter to toggle email_report#133
Conversation
xispa
left a comment
There was a problem hiding this comment.
This invariant checks whether the value provided in the email field is valid (and non-empty) when the boolean field validate_email_report is set to True (i.e., when the "Email results report" checkbox is ticked).
However, this validation is tightly coupled to the form submission workflow. Instead of performing this check at the schema level, I suggest removing the invariant and implementing an IAjaxEditFormAdapter to hide and uncheck the email_report (“Email results report”) field dynamically. This approach keeps the schema cleaner and moves the UI-dependent logic to the appropriate layer.
xispa
left a comment
There was a problem hiding this comment.
Could you move the "EmailReport" field to appear after the "Primary Email Address" field? Currently, the two fields are on different tabs, which can be a bit confusing
Description of the issue/feature this PR addresses
This PR removes the
validate_email_reportinvariant and replaces it with an EditFormAdapter that handles the visibility of theemail_reportfield based on whether the entered email is valid. Email validation should not be part of the invariant’s responsibility.Current behavior before PR
The
validate_email_reportinvariant attempted to validate the email by checkingrequest.form, which does not work for JSON API requests and mixes responsibilities. As a result, email validation failed in API scenarios and the logic for showing/hidingemail_reportwas not handled cleanly.Desired behavior after PR is merged
EditFormAdapternow automatically shows theemail_reportfield when theemailfield contains a valid email, and hides it otherwise.--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.