Page MenuHomePhabricator

Implement rules for form validation
Closed, ResolvedPublic

Description

As a user, I should not be able to submit a harassment that does not contain required information.

Required fields:

  • Behaviors checkbox (at least one selected) T340197#9129944
  • Link to evidence
  • User being reported

Acceptance criteria:

  • On clicking submit, check that all required fields contain data
  • If they do, continue
  • If they do not, display codex's error state for any component that is missing data
  • Once data is entered, reset the error state

Event Timeline

Assuming this task is about the Incident-Reporting-System code project, hence adding that project tag so other people who don't know or don't care about team tags can also find this task when searching per codebase. Please set appropriate project tags when possible. Thanks! :)

A few related questions from Derrick that we should think through as part of this work (and potentially spin up further tickets):

  1. What’s the minimum amount of text that’s required for “Who is violating behavioral guidelines” text-field, and the “Links to evidence of harassment” text-field, and the “Your email” text-field?

If there will be a required amount of text, then will there be an error message displaying when a user enters less than the Minimum amount of text?
For example: A user is editing a text field and only inserts 2 characters into the text-field, and then the user clicks the Submit button, but then an error message is displayed that says “you must enter at least 5 characters”.

  1. What’s the maximum amount of text that’s allowed for “Who is violating behavioral guidelines” text-field, and the “Links to evidence of harassment” text-field, and the “Your email” text-field?

If there will be a limit to the amount of text allowed, then will there be an error message displaying when a user enters more than the Maximum amount of text allowed?
For example: A user is editing a text-field and inserts 500 characters into the text-field, and then the user clicks the Submit button, but then an error message is displayed that says “you cannot enter more than 100 characters”.

  1. Lets say a user edits the “Who is violating behavioral guidelines” text-field with random characters...

Will there be validation logic to verify if the user inserted a valid username?
If there will be validation logic then what error message will be displayed if an invalid username is used?

  1. Lets say a user edits the “Links to evidence of harassment” text-field with an invalid url string (example: www.fakeurl.com.com)...

Will there be validation logic to verify that a valid url string is being entered by the user?
If there will be validation logic then what error message will be displayed if an invalid url string is used?

  1. Lets say a user edits the “Your email” text-field with random characters...

Will there be validation logic to verify if the user inserted a valid email address?
If there will be validation logic then what error message will be displayed if an invalid email address is used?

Q: do we validate only on submission, or do we check fields as they navigate the form?

@Madalina can you take a look at these questions and update the task description, please?

kostajh renamed this task from Do not let a user submit an invalid form to Implement rules for form validation.Aug 30 2023, 1:30 PM
kostajh updated the task description. (Show Details)
kostajh removed a subscriber: JKieserman.
  1. For who is violating... - Can we autofill the username of the person being reported when someone starts the journey from a message? Alternately, can the input field have suggestion matching the username strings? something like this example
    Screenshot 2023-09-05 at 16.40.15.png (336×882 px, 47 KB)
    For email address - the check could be for valid email address format XX@XX.XX/X like many web forms do to validate email ids.
  1. Usernames should be picked from a list if possible, see example above. For email id it's hard to say. World's longest email id is apparently 345 characters long :-) But afaik there is a limit of 64 character before @ and 255 character for the domain so max 320 character.
  1. Yes there should be a validation logic to see if it's a valid username. It could be a type ahead as explained on 1. Error message - "Please enter a valid user name" in case the field is empty
  1. Yes ideally there should be a check to see if it's a valid url format. However, this check seems more complicated. Error message - "Please enter a valid website"
  1. Yes as explained in point 1. Error message - "Please enter a valid email address"
  1. For who is violating... - Can we autofill the username of the person being reported when someone starts the journey from a message? Alternately, can the input field have suggestion matching the username strings? something like this example
    Screenshot 2023-09-05 at 16.40.15.png (336×882 px, 47 KB)

The form has two entrypoints:

  1. next to a topic heading, which can a single comment, or multiple comments, within it.
    • If there's a single comment, we should be able to pre-populate the username associated with the author of the comment in the topic.
    • if there are multiple comments, we can't prefill the username field. We would allow the user to auto-complete usernames, based on any registered user on the wiki. (This is how DiscussionTools implementation currently works.)
  2. Next to a comment: here we can pre-populate the username associated with the author of the comment

For email address - the check could be for valid email address format XX@XX.XX/X like many web forms do to validate email ids.

We have a validateEmail function in MediaWiki, for JS and PHP. (But noting that per discussion in T339276: Remove email field, and rely on user having a verified email address, we might not show an email field in the form.)

For email address - the check could be for valid email address format XX@XX.XX/X like many web forms do to validate email ids.
  1. Usernames should be picked from a list if possible, see example above. For email id it's hard to say. World's longest email id is apparently 345 characters long :-) But afaik there is a limit of 64 character before @ and 255 character for the domain so max 320 character.
  1. Yes there should be a validation logic to see if it's a valid username. It could be a type ahead as explained on 1. Error message - "Please enter a valid user name" in case the field is empty

Per comment above: we will allow autocompleting usernames based on the query allusers list API endpoint, e.g. /w/api.php?action=query&format=json&formatversion=2&uselang=en&list=allusers&auprefix=Ad&auprop=blockinfo&auwitheditsonly=1&aulimit=16

  1. Yes ideally there should be a check to see if it's a valid url format. However, this check seems more complicated. Error message - "Please enter a valid website"

Given that the report button entrypoint is tied to a comment or topic header, perhaps we don't need this field, and can include the link to the comment or topic header in the generated report.

Change 959761 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/ReportIncident@master] Set input-type as email for the reporter email field

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

Change 959761 abandoned by Dreamy Jazz:

[mediawiki/extensions/ReportIncident@master] Set input-type as email for the reporter email field

Reason:

Per Kosta's reasoning

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

Change 960142 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/ReportIncident@master] [Very WIP] Perform form validation on step 2

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

Change 960142 merged by jenkins-bot:

[mediawiki/extensions/ReportIncident@master] Perform form validation on step 2 and reset form on successful submit

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

Change 961380 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/ReportIncident@master] Follow-up: Reset required field disable booleans on succesful submit

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

Change 961380 merged by jenkins-bot:

[mediawiki/extensions/ReportIncident@master] Follow-up: Disable required field checks again after call to $reset.

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

Suggested QA steps:

  1. Install the ReportIncident extension
  2. Open a user talk page
  3. Click on the "Report" button
  4. Navigate to step 2, leave the fields empty, and press submit
  5. Verification item 1: Check that all fields except additional details show a error that asks for the user to fill in the field
  6. Click on one of the checkboxes
  7. Verification item 2: Verify that the required field error only disappears for the checkboxes
  8. Fill in the rest of the form and press submit (the dialog should close when you do this)
  9. Click on the "Report" button again
  10. Navigate to step 2 but don't interact with the form yet
  11. Verification item 3: Verify that no form errors are displayed
  12. Click on one of the required fields (e.g. username)
  13. Click somewhere else
  14. Verification item 4: Verify that the the field you clicked in and then out of now shows the required field error message.

Test wiki created on Patch demo by DJacksonA using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/6a32cd58e1/w

Just to add a little more context to why this is blocked is because this ticket relies upon an email being sent after the user clicks Submit... See ticket https://phabricator.wikimedia.org/T339275 for further details.

Test wiki created on Patch demo by DJacksonA using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/fdeb74d8d9/w

Test wiki on Patch demo by DJacksonA using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/fdeb74d8d9/w/

Test wiki created on Patch demo by DJacksonA using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/127fb9e231/w

Test wiki created on Patch demo by DJacksonA using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/5210b254d7/w

I have verified the Acceptance Criteria (listed in the ticket Description) has been met and is functioning as expected... Great job @Dreamy_Jazz and @kostajh!!!

image.png (861×683 px, 176 KB)
image.png (870×693 px, 182 KB)

Test wiki on Patch demo by DJacksonA using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/6a32cd58e1/w/

Test wiki on Patch demo by DJacksonA using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/127fb9e231/w/

Test wiki on Patch demo by DJacksonA using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/5210b254d7/w/