Page MenuHomePhabricator
Create Task
Maniphest T196984

Validate action parameters client-side
Open, MediumPublic
Actions

Description

Right now it's possible to select an action without specifying any parameter. Specifically, this is possible for "throttle" (just leave the fields blank), "warn" (select "other message" and leave the field blank) and "tag" (blank field) options. I think those fields should be required and validated server-side as well, if their checkbox is selected. My proposal is to wait for OOUI conversion (T132284), then add the needed validation. In fact, with OOUI some field types will be changed and would require a brand new validation system.

Another thing is to avoid validation for hidden fields, see T203336#4551702.

  • Throttle field
  • Warn field
  • Disallow field
  • Tag field
  • No validation for hidden fields

Details

SubjectRepoBranchLines +/-
Add client-side validation for action parametersmediawiki/extensions/AbuseFiltermaster+89 -10
Customize query in gerrit

Related Objects

Event Timeline

Daimona created this task.Jun 12 2018, 11:51 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 12 2018, 11:51 AM
Daimona triaged this task as Low priority.Jun 12 2018, 11:52 AM
Daimona moved this task from Backlog to Management features on the AbuseFilter board.
Daimona added a subscriber: Volker_E.Jun 12 2018, 12:23 PM

@Volker_E Are you aware of a way to dynamically toggle the required attribute for OOUI fields? I mean, I can't set them required from PHP, otherwise they'd be required even if they're hidden. I could write a JS function to toggle the "required" if the field is shown, but this way we need to infuse every field and keep track of it. Is there a quicker way? Something that you can set from PHP and automatically determines whether the requirement should be applied with JS. Specifically, I'm thinking to something similar to HTMLForm's "hide-if".

• Vvjjkkii renamed this task from Make action parameters required if the checkbox is selected to j7aaaaaaaa.Jul 1 2018, 1:04 AM
• Vvjjkkii raised the priority of this task from Low to High.
• Vvjjkkii edited projects, added CheckUser, Connected-Open-Heritage-Batch-uploads (RAÄ-KMB_1_2017-02), Tamil-Sites, Gamepress, Hashtags, Jade, KartoEditor, Language-2018-Apr-June, New-Editor-Experiences, Mail, TCB-Team (now WMDE-TechWish); removed AbuseFilter.
• Vvjjkkii updated the task description. (Show Details)
• Vvjjkkii removed a subscriber: Aklapper.
Daimona renamed this task from j7aaaaaaaa to Make action parameters required if the checkbox is selected.Jul 1 2018, 10:12 AM
Daimona lowered the priority of this task from High to Low.
Daimona edited projects, added AbuseFilter; removed TCB-Team (now WMDE-TechWish), Mail, New-Editor-Experiences, Language-2018-Apr-June, KartoEditor, Jade, Hashtags, Gamepress, Tamil-Sites, Connected-Open-Heritage-Batch-uploads (RAÄ-KMB_1_2017-02), CheckUser.
Daimona updated the task description. (Show Details)
Daimona added a subscriber: Aklapper.
Daimona mentioned this in T203336: Fatal exception when editing an abuse filter: Error: 1048 Column 'afa_parameters' cannot be null.Sep 2 2018, 8:04 AM
Daimona mentioned this in T203353: Correctly validate action parameters server-side when saving a filter.Sep 2 2018, 12:35 PM
Daimona renamed this task from Make action parameters required if the checkbox is selected to Validate action parameters client-side.Sep 2 2018, 12:37 PM
Daimona raised the priority of this task from Low to Medium.
Daimona updated the task description. (Show Details)
gerritbot subscribed.Sep 3 2018, 1:31 PM

Change 457445 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] [WIP] Add client-side validation for action parameters

https://gerrit.wikimedia.org/r/457445

gerritbot added a project: Patch-For-Review.Sep 3 2018, 1:31 PM
Daimona claimed this task.Sep 3 2018, 1:38 PM
Daimona added a comment.Sep 3 2018, 2:31 PM

@Volker_E As stated in the commit message, I can't seem to find a setRequired method for TagMultiselect and CheckboxMultiselect. Is this intended, or they still have to be added?

Daimona updated the task description. (Show Details)Sep 3 2018, 5:04 PM
Daimona updated the task description. (Show Details)Oct 12 2018, 2:27 PM
Aklapper edited projects, added Patch-Needs-Improvement; removed Patch-For-Review.Oct 15 2020, 1:07 PM
gerritbot added a comment.Nov 25 2020, 12:55 AM

Change 457445 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add client-side validation for action parameters

https://gerrit.wikimedia.org/r/457445

gerritbot added a project: Patch-For-Review.Nov 25 2020, 12:55 AM
Restricted Application removed a project: Patch-Needs-Improvement. · View Herald TranscriptNov 25 2020, 12:55 AM
Aklapper edited projects, added Patch-Needs-Improvement; removed Patch-For-Review.Aug 16 2022, 10:58 AM
Daimona removed Daimona as the assignee of this task.Aug 22 2022, 12:43 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL