Page MenuHomePhabricator
Create Task
Maniphest T274551

"deprecated" parameter handling
Closed, ResolvedPublic3 Estimated Story Points
Actions

Description

  • If a deprecate parameter is in use in the wikitext (no matter if it does have a value or not), show it in the sidebar.
  • For deprecated parameters not in use, hide them in the sidebar so that they cannot be added.
  • Removing a deprecated parameter does not immediately remove it's entry in the sidebar. The behavior should be similar to an undocumented parameter.
  • Make sure we never create a dirty diff on save.

Note: Deprecated parameters will be prevented from being added through the undocumented parameter input, added in T272487: New input for undocumented parameters. This is covered in the related ticket about error handling, see T285869: Add error handling for the undocumented parameter input.

Details

Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

awight created this task.Feb 11 2021, 3:22 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 11 2021, 3:22 PM
Maintenance_bot added a project: VisualEditor.Feb 11 2021, 3:45 PM
matmarex moved this task from To Triage to Triaged on the VisualEditor board.Feb 11 2021, 4:08 PM
Lena_WMDE moved this task from Backlog to In preparation on the WMDE-Templates-FocusArea board.Mar 10 2021, 1:54 PM
awight added a subtask: T274545: Wire template parameters into the checkbox list view.Mar 11 2021, 2:44 PM
Lena_WMDE updated the task description. (Show Details)Apr 14 2021, 1:19 PM
Lena_WMDE set the point value for this task to 3.
Andrew-WMDE added a parent task: T274550: Handle undocumented template parameters.Apr 14 2021, 1:22 PM
awight removed a parent task: T274550: Handle undocumented template parameters.May 14 2021, 7:58 AM
Lena_WMDE moved this task from In preparation to Ready for pickup on the WMDE-Templates-FocusArea board.May 14 2021, 8:10 AM
ECohen_WMDE merged a task: T130676: Visual editor Template tool should not show deprecated parameter(s), unless already has value.Jun 30 2021, 10:23 AM
ECohen_WMDE added a subscriber: Kipod.
ECohen_WMDE updated the task description. (Show Details)Jun 30 2021, 12:16 PM
ECohen_WMDE mentioned this in T272368: Add checkboxes to param list sidebar in VE TemplateDialog.Jun 30 2021, 12:21 PM
ECohen_WMDE updated the task description. (Show Details)Jun 30 2021, 2:50 PM
WMDE-Fisch closed subtask T274545: Wire template parameters into the checkbox list view as Resolved.Jul 6 2021, 11:30 AM
Andrew-WMDE claimed this task.Jul 16 2021, 8:28 AM
Andrew-WMDE added projects: WMDE-TechWish-Sprint-2021-07-07, Unplanned-Sprint-Work.
Andrew-WMDE moved this task from Sprint Backlog to Doing on the WMDE-TechWish-Sprint-2021-07-07 board.
thiemowmde moved this task from Ready for pickup to In sprint on the WMDE-Templates-FocusArea board.Jul 16 2021, 9:28 AM
gerritbot added a comment.Jul 16 2021, 3:17 PM

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

[mediawiki/extensions/VisualEditor@master] [WIP] Hide deprecated parameters if they don't have an existing value

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

gerritbot added a project: Patch-For-Review.Jul 16 2021, 3:17 PM
WMDE-Fisch moved this task from Proposed to Currently in sprint on the WMDE-TechWish board.Jul 20 2021, 9:46 AM
Lena_WMDE edited projects, added WMDE-TechWish-Sprint-2021-07-21; removed Unplanned-Sprint-Work.Jul 21 2021, 8:22 AM
WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-TechWish-Sprint-2021-07-21 board.Jul 21 2021, 8:22 AM
Andrew-WMDE moved this task from Doing to Review on the WMDE-TechWish-Sprint-2021-07-21 board.Jul 21 2021, 2:10 PM
gerritbot added a comment.Jul 23 2021, 11:04 AM

Change 704982 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Hide deprecated parameters if they don't have a value

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

Maintenance_bot removed a project: Patch-For-Review.Jul 23 2021, 11:10 AM
ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.16; 2021-07-26).Jul 23 2021, 12:00 PM
WMDE-Fisch moved this task from Review to Demo on the WMDE-TechWish-Sprint-2021-07-21 board.Jul 26 2021, 7:19 AM
thiemowmde moved this task from Demo to Doing on the WMDE-TechWish-Sprint-2021-07-21 board.Aug 3 2021, 8:20 AM
gerritbot added a comment.Aug 3 2021, 8:24 AM

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

[mediawiki/extensions/VisualEditor@master] Revert \"Hide deprecated parameters if they don't have a value\"

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

gerritbot added a project: Patch-For-Review.Aug 3 2021, 8:24 AM
thiemowmde subscribed.EditedAug 3 2021, 8:25 AM

From today's demo time:

  • The basic idea to not offer a checkbox for empty deprecated parameters works fine.
  • However, it should be possible to still add such a deprecated parameter the same way as an undocumented parameter. It appears like this is broken.
  • We worried this might have broken the old workflow that is currently deployed on all wikis. Luckily this is not the case as the problematic code is wrapped in a if ( …NewSidebar ) conditional. Still we reverted the patch for the moment just to be sure. We will redo it.
gerritbot added a comment.Aug 3 2021, 8:42 AM

Change 709647 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Revert \"Hide deprecated parameters if they don't have a value\"

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

Maintenance_bot removed a project: Patch-For-Review.Aug 3 2021, 9:10 AM
gerritbot added a comment.Aug 3 2021, 12:39 PM

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

[mediawiki/extensions/VisualEditor@master] [WIP] Initially hide empty deprecated parameters

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

gerritbot added a project: Patch-For-Review.Aug 3 2021, 12:39 PM
gerritbot added a comment.Aug 3 2021, 12:42 PM

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

[mediawiki/extensions/VisualEditor@master] Remove redundant ve.dm.MWTemplatePlaceholderModel.isEmpty

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

gerritbot added a comment.Aug 3 2021, 1:15 PM

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

[mediawiki/extensions/VisualEditor@master] Document definition of \"empty\" in ve.ui.MWParameterPage

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

gerritbot added a comment.Aug 3 2021, 1:36 PM

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

[mediawiki/extensions/VisualEditor@master] Update ambiguous definition of isEmpty in the model

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

gerritbot added a comment.Aug 3 2021, 1:49 PM

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

[mediawiki/extensions/VisualEditor@master] Update and encode definition of empty for template parameters

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

ReleaseTaggerBot edited projects, added MW-1.37-notes (1.37.0-wmf.18; 2021-08-09); removed MW-1.37-notes (1.37.0-wmf.16; 2021-07-26).Aug 3 2021, 3:00 PM
gerritbot added a comment.Aug 3 2021, 3:50 PM

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

[mediawiki/extensions/VisualEditor@master] Drop deprecated parameters when they are empty

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

thiemowmde added a subscriber: Lena_WMDE.Aug 3 2021, 4:13 PM

I found more issues with the original patch https://gerrit.wikimedia.org/r/704982.

Before, the new sidebar assumed there is a 1:1 relationship between the list of checkboxes and the list of parameters that are tracked in the spec (these include documented and undocumented, used and unused parameters, just as the list of checkboxes). The only exception is the placeholder, but this is always at the end and doesn't matter because of this. The patch broke this assumption, resulting in weird behavior when adding undocumented parameters. Their order got messed up.

Another issue I run into was the realization that the task, as it is described, essentially forces us to create a new type of parameter: A parameter that doesn't have a checkbox but still exists in the template (this must be done to not cause dirty diffs). This confuses all code that checks if a parameter is already in the template – and there is a lot of such code. I tried to introduce some kind of "this parameter exists but is hidden" flag, but gave up. The code got messy and unmaintainable very fast.

Another idea is to create all checkboxes (this avoids the mismatches described above) and just hide the one that should not be there. Unfortunately this requires the same amount of messy code to deal with checkboxes that can be hidden – a concept that didn't exist before.

I figured that the benefits of such a rather complicated approach are not worth it. Instead I suggest to actually remove empty deprecated parameters the moment the template is edited. This is rare anyway. This brings everything back into a stable, predictable state: There is still no checkbox for the deprecated parameter, but this is fine, because the parameter actually doesn't exist in the template. It behaves basically the same as an undocumented parameter. You can search for it's name and add it, which creates the checkbox, and puts it at the correct position.

TL;DR: The 3rd acceptance criteria doesn't work without either making the code a mess or significant investments (for a rather niche edge-case). 👈 @Lena_WMDE

thiemowmde claimed this task.Aug 4 2021, 8:07 AM
thiemowmde added a subscriber: Andrew-WMDE.
thiemowmde added a project: WMDE-TechWish-Sprint-2021-08-04.Aug 4 2021, 8:10 AM
thiemowmde moved this task from Sprint Backlog to Doing on the WMDE-TechWish-Sprint-2021-08-04 board.Aug 4 2021, 8:13 AM
gerritbot added a comment.Aug 4 2021, 2:36 PM

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

[mediawiki/extensions/VisualEditor@master] Don't offer deprecated parameters when they aren't used

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

awight added a comment.Aug 5 2021, 7:17 AM

Minor updates from offline discussion:

  • Deprecated parameters will not be available like undocumented parameters, instead they will be blocked unless already present in the template invocation.
  • An existing deprecated parameter in the invocation is not treated specially just because its value is empty. We decided that an empty value is valid, may have an effect in template code, and should be rendered the same as if there were a non-empty value in the field.
thiemowmde updated the task description. (Show Details)Aug 5 2021, 7:29 AM
thiemowmde moved this task from Doing to Review on the WMDE-TechWish-Sprint-2021-08-04 board.
Andrew-WMDE updated the task description. (Show Details)Aug 5 2021, 9:08 AM
Andrew-WMDE moved this task from Review to Demo on the WMDE-TechWish-Sprint-2021-08-04 board.
gerritbot added a comment.Aug 5 2021, 9:21 AM

Change 710051 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Don't offer deprecated parameters when they aren't used

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

gerritbot added a comment.Aug 5 2021, 10:23 AM

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

[mediawiki/extensions/VisualEditor@master] Fix off-by-1 error when deprecated parameters are hidden

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

thiemowmde moved this task from Demo to Review on the WMDE-TechWish-Sprint-2021-08-04 board.Aug 5 2021, 10:25 AM
gerritbot added a comment.Aug 6 2021, 10:56 AM

Change 709686 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Remove redundant ve.dm.MWTemplatePlaceholderModel.isEmpty

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

gerritbot added a comment.Aug 6 2021, 1:07 PM

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

[mediawiki/extensions/VisualEditor@master] Split long ve.ui.MWTransclusionOutlineTemplateWidget methods

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

gerritbot added a comment.Aug 6 2021, 1:28 PM

Change 710232 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Fix off-by-1 error when deprecated parameters are hidden

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

gerritbot added a comment.Aug 9 2021, 9:40 AM

Change 709700 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Better name for ambiguous \"empty\" concept in the model

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

gerritbot added a comment.Aug 9 2021, 10:41 AM

Change 709742 abandoned by Thiemo Kreuz (WMDE):

[mediawiki/extensions/VisualEditor@master] Drop deprecated parameters when they are empty

Reason:

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

gerritbot added a comment.Aug 9 2021, 10:42 AM

Change 709685 abandoned by Thiemo Kreuz (WMDE):

[mediawiki/extensions/VisualEditor@master] [WIP] Initially hide empty deprecated parameters

Reason:

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

gerritbot added a comment.Aug 9 2021, 10:51 AM

Change 709707 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Ignore default values as not being valuable as well

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

gerritbot added a comment.Aug 9 2021, 12:50 PM

Change 710537 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Split long ve.ui.MWTransclusionOutlineTemplateWidget methods

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

gerritbot added a comment.Aug 11 2021, 7:44 AM

Change 709694 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Rename and document definition of \"empty\" in ve.ui.MWParameterPage

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

ReleaseTaggerBot edited projects, added MW-1.37-notes (1.37.0-wmf.19; 2021-08-16); removed MW-1.37-notes (1.37.0-wmf.18; 2021-08-09).Aug 11 2021, 8:00 AM
awight moved this task from Review to Demo on the WMDE-TechWish-Sprint-2021-08-04 board.Aug 16 2021, 1:20 PM
WMDE-Fisch closed this task as Resolved.Aug 17 2021, 8:43 AM
WMDE-Fisch moved this task from Demo to Done on the WMDE-TechWish-Sprint-2021-08-04 board.
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 · Credits