Add error handling for the undocumented parameter input
Closed, ResolvedPublic5 Estimated Story Points
Actions

Assigned To

Authored By

	ECohen_WMDE
	Jun 30 2021, 2:45 PM

Description

Background

There are certain values which are not accepted and currently it simply seems like the input is broken. The same values are not allowed, but we will display error messages so that the user understands what is happening.

Requirements

Display errors when:
- the user enters a parameter name already existing in the template (either documented or not)
- the user enters a parameter name already existing in the template (either documented or not) that is unselected
- the user enters the alias of a parameter already existing in the template (can only be a documented param)
- the user enters the name/alias of a deprecated parameter
- the user enters a forbidden character {, =, |, and }.
Error displayed using OOUI component "FieldLayout with error message"
Button to add the parameter is disabled

Mocks

Error message text

Param exists: "Cannot add two parameters of the same name."
Alias: "“$input” is an alias of the parameter “$conflicting parameter name” and cannot be added because it would be a duplicate."
Deprecated: "“$input” cannot be added because the parameter has been marked as deprecated."
Forbidden character: ""$inputsymbol" is a forbidden character. Please remove it to add the parameter."

Details

Subject	Repo	Branch	Lines +/-
Improve messages about duplicate template parameters	mediawiki/extensions/VisualEditor	master	+6 -6
Add test for MWTemplateModel.getOriginalParameterName()	mediawiki/extensions/VisualEditor	master	+27 -0
Improve input validation for the add parameter page	mediawiki/extensions/VisualEditor	master	+28 -14
Don't allow users to add parameters that contain forbidden chars	mediawiki/extensions/VisualEditor	master	+12 -2
Add input validation to the add parameter page	mediawiki/extensions/VisualEditor	master	+97 -10
Add test for ve.ui.MWAddParameterPage.getValidationErrors()	mediawiki/extensions/VisualEditor	master	+44 -0
Simplify check for forbidden characters	mediawiki/extensions/VisualEditor	master	+3 -7
Don't allow parameter names that break the wikitext syntax	mediawiki/extensions/VisualEditor	master	+23 -5

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Resolved	WMDE-Fisch	T306967 Deploy VE template dialog improvements to enwiki
Resolved	Lena_WMDE	T296759 Deploy VE template dialog improvements to more wikis
Resolved	Lena_WMDE	T286992 Deploy VE template dialog improvements to small set of wikis
Resolved	WMDE-Fisch	T272487 New input for undocumented parameters
Resolved	thiemowmde	T290977 Hide "duplicate parameter" error message when it doesn't apply any more
Resolved	ECohen_WMDE	T285869 Add error handling for the undocumented parameter input
Invalid	None	T291059 [DRAFT] Adjust messages for special alias edge cases

Event Timeline

ECohen_WMDE created this task.Jun 30 2021, 2:45 PM

ECohen_WMDE mentioned this in T274551: "deprecated" parameter handling.Jun 30 2021, 2:50 PM

ECohen_WMDE updated the task description. (Show Details)Jun 30 2021, 3:03 PM

Lena_WMDE moved this task from Backlog to Ready for story time on the WMDE-Templates-FocusArea board.Jun 30 2021, 3:11 PM

ECohen_WMDE updated the task description. (Show Details)Jun 30 2021, 3:11 PM

ECohen_WMDE mentioned this in T272487: New input for undocumented parameters.Jun 30 2021, 3:21 PM

@Sarai-WMDE to update description and add mock up representing 4th error state: when the users search for a documented parameter or an undocumented parameter that has been added but is not selected (not present in the main form).

Lena_WMDE set the point value for this task to 5.Jul 21 2021, 1:48 PM

Sarai-WMDE updated the task description. (Show Details)Jul 22 2021, 1:12 PM

Ticket updated

Lena_WMDE moved this task from Ready for story time to Ready for pickup on the WMDE-Templates-FocusArea board.Aug 4 2021, 8:30 AM

Lena_WMDE added a project: WMDE-TechWish-Sprint-2021-08-04.Aug 4 2021, 8:36 AM

ECohen_WMDE moved this task from Ready for pickup to In sprint on the WMDE-Templates-FocusArea board.Aug 4 2021, 8:43 AM

Minor question, maybe this should be a "parent" task of the input, ie. we first create the input and then implement error handling?

awight added a project: WMDE-TechWish-Sprint-2021-08-18.Aug 18 2021, 8:07 AM

Andrew-WMDE claimed this task.Aug 31 2021, 10:20 AM

Andrew-WMDE moved this task from Sprint Backlog to Doing on the WMDE-TechWish-Sprint-2021-08-18 board.

Change 715755 had a related patch set uploaded (by Andrew-WMDE; author: Andrew-WMDE):

[mediawiki/extensions/VisualEditor@master] Add input validation to the add parameter page

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

gerritbot added a project: Patch-For-Review.Aug 31 2021, 4:04 PM

WMDE-Fisch added a project: WMDE-TechWish-Sprint-2021-09-01.Sep 1 2021, 8:14 AM

thiemowmde moved this task from Sprint Backlog to Doing on the WMDE-TechWish-Sprint-2021-09-01 board.Sep 1 2021, 8:14 AM

Sarai-WMDE removed a subscriber: Sarai-WMDE.Sep 1 2021, 9:38 AM

Change 717385 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Don't allow parameter names that break the wikitext syntax

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

thiemowmde updated the task description. (Show Details)Sep 3 2021, 1:28 PM

@thiemowmde

Just disable the button when the input is not even a valid parameter name. Forbidden characters are {, =, |, and }.

If this is another case where the user is blocked, I think it makes sense to include an error message here as well while we are adding the rest. Such as "($symbol) is a forbidden character. Please remove it to add the parameter." Otherwise I'm not sure it would be totally clear as to why the button is disabled.

Change 717439 had a related patch set uploaded (by Andrew-WMDE; author: Andrew-WMDE):

[mediawiki/extensions/VisualEditor@master] Improve input validation for the add parameter page

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

Sure, we can show a message. However, there is still the edge-case when the input field is empty. What to do then?

Change 717466 had a related patch set uploaded (by Andrew-WMDE; author: Andrew-WMDE):

[mediawiki/extensions/VisualEditor@master] Don't allow users to add parameters that contain forbidden chars

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

Change 717385 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't allow parameter names that break the wikitext syntax

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

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.23; 2021-09-13).Sep 6 2021, 9:00 AM

the user enters the alias of a parameter already existing in the template (can only be a documented param)

This one is slightly annoying when a required parameter is aliased. Since the required parameter cannot be unchecked, the alias can never be used.

thiemowmde claimed this task.Sep 7 2021, 10:16 AM

thiemowmde moved this task from Doing to Review on the WMDE-TechWish-Sprint-2021-09-01 board.

thiemowmde added a subscriber: Andrew-WMDE.

Change 719250 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Add test for MWTemplateModel.getOriginalParameterName()

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

ECohen_WMDE updated the task description. (Show Details)Sep 7 2021, 1:44 PM

ECohen_WMDE updated the task description. (Show Details)

Change 719483 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Simplify check for forbidden characters

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

Change 719507 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] Add test for ve.ui.MWAddParameterPage.getValidationErrors()

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

Change 719483 abandoned by Thiemo Kreuz (WMDE):

[mediawiki/extensions/VisualEditor@master] Simplify check for forbidden characters

Reason:

Squashed. Now part if I1011949.

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

Change 719507 abandoned by Thiemo Kreuz (WMDE):

[mediawiki/extensions/VisualEditor@master] Add test for ve.ui.MWAddParameterPage.getValidationErrors()

Reason:

Squashed. Now part of Iebb982e.

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

While testing the current implementation I came across an edge case:

a parameter (e.g. Foo) has an alias (e.g. fooalias)
and the alias is being used as the name of the parameter
if you enter the original name (Foo)
the error message says there is a duplicate name (although Foo is not displayed)

Screenshot from 2021-09-09 11-51-22.png (719×1 px, 139 KB)

Possible solution: display a different error message for aliases in usage

Picking up what @lilients_WMDE wrote above. For demo time or @ECohen_WMDE to look at.

Cannot add two parameters of the same name.
There is a special case when a parameter is already used, but via an alias, and the alias is shown in the sidebar. Suggestion: Cannot add a duplicate of the parameter "x". Show this messages in all cases, no matter if the sidebar shows the primary name or an alias. The "x" will repeat the user's input most of the time, but this doesn't hurt, I believe.

"a" is an alias of the parameter "y" and cannot be added because it would be a duplicate.
This is currently shown in two situations. When the parameter is already used, and when it's not. It makes a lot of sense to talk about a duplicate when the parameter is already used. But what's the reason we don't want users to add a known parameter via an alias? Isn't it more like this: "x" is an alias of the parameter "y", which is already available for use. Please check the options in the sidebar." Is this worth a separate message, or is the existing one good enough?

Change 715755 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Add input validation to the add parameter page

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

Change 717466 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't allow users to add parameters that contain forbidden chars

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

@ECohen_WMDE It turns out we need to specify the priority of these different cases. The issue that's been a sticking point in code review is that we can get alias errors when the alias or primary parameter is already present in the template invocation. In this case, should we show the alias error (third attached mock), or the duplicate parameter error (first attached mock)?

Change 717439 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Improve input validation for the add parameter page

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

Change 719250 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Add test for MWTemplateModel.getOriginalParameterName()

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

thiemowmde moved this task from Review to Demo on the WMDE-TechWish-Sprint-2021-09-01 board.Sep 14 2021, 7:55 AM

In this case, should we show the alias error (third attached mock), or the duplicate parameter error (first attached mock)?

After reading through, I think for this case we can simply show the alias error (third mock F34533178). From my understanding, the text still makes sense. I think this resolves all the above issues but if there is still something outstanding, let me know

thiemowmde updated the task description. (Show Details)Sep 14 2021, 8:24 AM

Lena_WMDE moved this task from Demo to Done on the WMDE-TechWish-Sprint-2021-09-01 board.Sep 14 2021, 8:28 AM

thiemowmde moved this task from Done to Doing on the WMDE-TechWish-Sprint-2021-09-01 board.Sep 14 2021, 9:06 AM

Change 720991 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/VisualEditor@master] [WIP] Improve messages about duplicate template parameters

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

[…] simply show the alias error (third mock F34533178).

This would make the message claim that the actual parameter name is an "alias". Technically this is not wrong. It might still be confusing.

thiemowmde mentioned this in T290977: Hide "duplicate parameter" error message when it doesn't apply any more.Sep 14 2021, 3:03 PM

thiemowmde added a parent task: T290977: Hide "duplicate parameter" error message when it doesn't apply any more.

thiemowmde reassigned this task from thiemowmde to ECohen_WMDE.Sep 15 2021, 7:38 AM

ECohen_WMDE mentioned this in T291059: [DRAFT] Adjust messages for special alias edge cases.Sep 15 2021, 7:55 AM

We reviewed this ticket during demo time. It was implemented according the requirements and can be closed. A few behaviors were noted as confusing and will be dealt with in a follow-up ticket T291059: [DRAFT] Adjust messages for special alias edge cases (currently a draft, ideal behavior TBD).

ECohen_WMDE moved this task from Doing to Done on the WMDE-TechWish-Sprint-2021-09-01 board.Sep 15 2021, 7:58 AM

Change 720991 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Improve messages about duplicate template parameters