Commit graph

757 commits

Author SHA1 Message Date
Andrew Kostka a38338259d Improve input validation for the add parameter page
This patch improves the error handling for when a user tries to add
a parameter which is either an alias of a existing parameter, the
primary name of a existing aliased parameter, or a name/alias of an
existing parameter which is shown with an override label.

The error message was modified to always refer to the conflicting
parameter using the same name that is has in the sidebar.

Example: A parameter named "Parameter B" is already present in the
sidebar under its alias "B". When a user tries to add "Parameter B",
the new error message will inform the user that the parameter they
are trying to add already exists as "B".

Bug: T285869
Change-Id: I762b72b6cf14eb8ff5fcef63b4dcb70e297050de
2021-09-13 16:58:11 +02:00
Adam Wight ea8b90b015 Fix term in valuable data test
We want to assert that value is true-ish, and that it doesn't equal a
default or auto string.

Bug: T290554
Change-Id: I454dda8d0085a8d3898a0d5b1a3ecc6dd7c2c9e4
2021-09-10 09:25:40 +00:00
Thiemo Kreuz 9dbbc06273 Fix …TemplateSpecModel reporting missing pages as documented
The code in .cacheTemplateDataApiResponse() where the `specCache`
is filled skips missing pages. .setTemplateData() is never called.
While we could – in theory – check the `missing` flag (as done in
patchset 1), this flag never makes it to the spec.

Rather simple solution: Mark everything as undocumented, as long
as .setTemplateData() is not called.

This affects only missing pages. .setTemplateData() is called in
all other situations.

Bug: T272487
Bug: T276574
Bug: T290136
Change-Id: I7045e84f2f2ba5aa4591c94ea495b0249e6c40d6
2021-09-01 14:59:03 +02:00
jenkins-bot a86ef8db42 Merge "Add temporary compatibility to ve.dm.MWTransclusionModel" 2021-09-01 07:33:41 +00:00
jenkins-bot 37f78f9afa Merge "Harden title parsing in MWTransclusionModel a bit" 2021-08-31 17:09:37 +00:00
Thiemo Kreuz d8718fde95 Add temporary compatibility to ve.dm.MWTransclusionModel
Method names have been changed in I8fa47ed, assuming these are
private. It looks like some hacks exist out there. Let's make
these peoples life easier.

Change-Id: I63c80761fe06e2f3a4bb104fe3e8c17d1c7faa02
2021-08-31 07:00:32 +00:00
jenkins-bot bdb3a783c9 Merge "Add basic QUnit tests for all transclusion outline classes" 2021-08-27 09:59:16 +00:00
Thiemo Kreuz c8536f1a71 Remove unnecessary title parsing from template related code
There are 2 situations:

1. Either the template name is used in a [[…]] link. In this case
we must provide the namespace. MWTemplateModel.getTitle() does
this. However, it's not a mw.Title object and therefor not really
guaranteed to be a valid title. This is fine. The worst thing
that can happen is that the link points to an error message.
But this should be entirely unreachable anyway.

2. Some messages want to display the name of the template.
Ideally without the namespace. That's what
MWTemplateSpecModel.getLabel() is for. Again this is not
guaranteed to be a valid mw.Title. But it doesn't need to. It's
only used as a label.

Change-Id: I03d0481201620a2f5c444ee32b656bcaade98aac
2021-08-26 15:58:15 +02:00
Thiemo Kreuz df713170e0 Harden title parsing in MWTransclusionModel a bit
It appears like it's currently not possible to reach this code
with an invalid template name like `{{foo}}`. But this is not
guaranteed.

The purpose of this code is to call the TemplateData API. This is
pointless when a title is invalid. We know a page with this name
can't exist. So we skip it.

But that's all this code cares about. It should not crash. Nor
does it need to report this situation.

This is related to the discussion in Ic364e75.

Change-Id: If9bacc91b1c7bb110b33bfd395e1cbdf538e6c22
2021-08-26 14:46:01 +02:00
Thiemo Kreuz f6953d4096 Add basic QUnit tests for all transclusion outline classes
This is just the smallest possible boilerplate to get the first
trivial test running. More test cases will be added in the
following patches.

Bug: T289560
Change-Id: I3a4e49a7b9761db00b211e933386bad71d4f0942
2021-08-24 11:31:14 +02:00
WMDE-Fisch fc8ce9d7e3 Fix typos in doc
Change-Id: I5895207ca2d9de91bb92e6ab3566fbf2880bf393
2021-08-20 14:56:32 +02:00
Thiemo Kreuz 7045371a4d Better method names in MWTransclusionModel class
Names like "fetch" or "resolve" are heavily ambiguous and
continue to confuse me. I hope these new names reflect better
what's going on.

Bug: T288827
Depends-On: I587a203a9370e4742f87586b4f1867b37459c375
Change-Id: I8fa47ed313e7d7b2c114a5638a67c4f3c8b830f1
2021-08-20 10:00:10 +02:00
jenkins-bot b7f572bd06 Merge "Reduce deep nesting in ve.dm.MWTransclusionModel" 2021-08-19 13:40:55 +00:00
WMDE-Fisch 9aaeac3318 Add getters for un-/documented parameters
Will be needed to figure out when the new UI to add parameters should
be visible.

Bug: T272487
Change-Id: I97d2429b7845905784ef18e9d41cd5e6f85bbd01
2021-08-19 07:00:30 +00:00
Thiemo Kreuz 7503b323b1 Stop re-creating template parameter pages over and over again
This is what actually happens:
* We call `addParameter()`.
* This triggers an `add` event.
* This calls an `MWTemplateDialog.onAddParameter` event handler.
* This code doesn't check if a parameter already exists (because
  it shouldn't). It detroys the page in the content pane on the
  right and recreates it from scratch.

The only reason we do this is to focus the input field on the
right. This patch introduces a dedicated event to do this.

Bug: T288827
Change-Id: I47effe05427cfabfcf534920edee79521eaa033f
2021-08-17 13:34:41 +02:00
Thiemo Kreuz b264a9c381 Add isDocumented() feature to ve.dm.MWTemplateSpecModel
Bug: T272487
Change-Id: I39c612358cc0238c515c8eb28aa1b40418a10cef
2021-08-13 12:47:28 +02:00
WMDE-Fisch 8f39c56ded Fixing minor jsdoc typo
Change-Id: Id543c939836d1c15f5cfdcfc7450efc574e85cb6
2021-08-10 16:15:30 +02:00
Thiemo Kreuz 9e74e2f352 Ignore default values as not being valuable as well
This check makes sure the user doesn't loose work when clicking
the back button. I would like to argue that neither of these
values is valuable enough to block the user with a confirmation
dialog:
* Literally nothing is lost when the input is empty.
* The auto-value is only temporarily lost, but will
  automatically be restored when the user decides to add the
  template back. The input field is pre-filled with this value.
* The default value doesn't need to be manually entered. It will
  show up anyway when the parameter is missing.

There is a rare edge-case, but it is not really relevant in this
situation. Some templates allow to override a default value with
the empty string. This will be considered irrelevant by this
code. However, this was already happening before and doesn't
change with this patch.

The only edge-case where this patch makes a difference is if a
parameter is marked as required or suggested, is documented to
have a default value, _and_ the template allows to override it
with an empty string. But this combination is rather crazy, if
not bogus, and not worth considering here, I believe.

Bug: T274551
Change-Id: Ib176a82844335c3d4dd5b720d335ec28245e1637
2021-08-09 10:28:09 +00:00
jenkins-bot aa868059ee Merge "Better name for ambiguous "empty" concept in the model" 2021-08-09 09:40:40 +00:00
Thiemo Kreuz 35080a8646 Better name for ambiguous "empty" concept in the model
This is really only about the methods name, but doesn't change
any behavior.

I realized we work with several different definitions of what
"empty" means. There are at least two significant definitions:

1. When a parameter's value is the empty string or identical
to the default value, the behavior of the template is the same.
It will use the default value just as if the user entered it.

The auto-value is a meaningful value in this scenario and can't
be considered equal to the empty string.

2. The context here is when the user presses the back button.
This will destroy all user input. But an auto-value is not user
input. It will appear again when the user realizes they made a
mistake. Nothing is lost.

Personally, I would not use the word "empty" to describe this
concept. Things like "containsUserProvidedValue",
"isCustomValue", "isMeaningfulValue", … come to mind. These are
all still a big vague. A "user provided" value can be identical
to the default or auto-value. "Custom" how? I went for
"containsValuableData" for now.

Bug: T274551
Change-Id: I2912a35556795c867a6b2396cbad291e947f0ed6
2021-08-06 15:11:44 +02:00
jenkins-bot 1cb4a5a471 Merge "Remove redundant ve.dm.MWTemplatePlaceholderModel.isEmpty" 2021-08-06 10:56:00 +00:00
jenkins-bot fce48b0958 Merge "Split MWTransclusionModel methods to do one thing only" 2021-08-05 10:32:18 +00:00
Thiemo Kreuz 87410aaa7d Reduce deep nesting in ve.dm.MWTransclusionModel
This is split from I8fa47ed. I believe these changes are
non-controversial.

Change-Id: I3c5d4e1867ee0f53736f55fb7c6f27ad8d304596
2021-08-05 10:59:22 +02:00
Thiemo Kreuz 920550560d Remove redundant ve.dm.MWTemplatePlaceholderModel.isEmpty
This method already exists in the ve.dm.MWTransclusionPartModel
base class where it does the exact same.

Bug: T274551
Change-Id: I19d5914ed9b4b435c83ea4d64019bc46ce1ce8fd
2021-08-03 14:40:50 +02:00
Thiemo Kreuz (WMDE) 8d9b4526c6 Revert "Hide deprecated parameters if they don't have a value"
This reverts commit 0d4dee341b.

Reason for revert: This made it entirely impossible to add a
deprecated parameter, even if done intentionally. Needs more tests.

Bug: T274551
Change-Id: I7389bad0845cd1ce78f9d7ef71592cb1ce2a063e
2021-08-03 08:24:24 +00:00
Andrew Kostka 0d4dee341b Hide deprecated parameters if they don't have a value
Bug: T274551
Change-Id: I6bfceae2579a5bcedae9da7eb5326fe8f60ea7c0
2021-07-22 12:24:57 +02:00
jenkins-bot 18a18bc825 Merge "Fix incomplete template dialog event handling in new sidebar" 2021-07-16 13:07:03 +00:00
jenkins-bot 4ab5fd5c6b Merge "Add a message next to undocumented parameters" 2021-07-16 12:05:29 +00:00
Thiemo Kreuz 434c11f6de Fix incomplete template dialog event handling in new sidebar
Notably:
* Don't require the model in the new sidebar via dependency
  injection, but connect the event handlers later. This is
  relevant because we currently create the new sidebar in the
  wrong spot. Removing the hard dependency allows us to split
  the code and utilize initialize() and getSetupProcess()
  correctly. This will be done in a following patch.
* The change event now includes the new position. This makes
  it very easy to add this missing feature to the new sidebar.

Also:
* Stop triggering change events when nothing changed. These
  events are expensive. They bubble all the way up to the
  TransclusionModel, and to all linked
  onTransclusionModelChange() handlers.
* Update event documentation to make this more visible.

Bug: T274544
Change-Id: Iafe29f18a6fed14d9c3124c9756aa840886afbbc
2021-07-16 11:29:26 +00:00
Thiemo Kreuz 1164f67f40 Tweaks and cleanups to template parameter search
Notably:
* Include parameter aliases, labels and descriptions in the
  search.
* Don't use a possibly outdated search index, but live data.
* Clear filter when a new checkbox is added.

Bug: T272481
Change-Id: Ie90a803af6178a8bb6de370a0f8e079800d9f8a2
2021-07-16 12:11:13 +02:00
Andrew Kostka f02c48ea52 Add a message next to undocumented parameters
Bug: T274550
Change-Id: I1af71150239ebc9966cc22e7d28883fbac99fdf1
2021-07-15 14:47:21 +02:00
Thiemo Kreuz 766b220f5a Finishing touches to new template editor sidebar
In detail:
* Allow clicks on all elements in the new sidebar. This should
  focus the corresponding element on the right.
* Make all elements in the new sidebar tabbable.
* Fix MWTransclusionOutlineTemplateWidget.createCheckbox() to
  not need a temporary param object any more.
* Rewrite more code in MWTransclusionOutlineTemplateWidget to
  be shorter and easier to read.
* Fix MWTemplateModel.addParameter() to not do way to much
  stuff when a parameter already exists.
* Update code documentation.
* Use more specific, less ambiguous variable and method names.

Bug: T274544
Change-Id: Iaf6f7d1b0f7bf0e9b03eb86d01f3eceadece6fe4
2021-07-15 08:40:35 +00:00
jenkins-bot 240a405dfa Merge "Remove unused parameters from MWTransclusionModel methods" 2021-07-12 09:05:51 +00:00
Thiemo Kreuz fae6118071 Document and use mw.Api parameter defaults
Reasoning:
* format=json must be the default. Nothing else makes sense in
  the context of this code. This should not be a surprise.
* formatversion=2 is only a default when the custom
  getContentApi() is used, but not when mw.Api is used. One
  might argue that it's safer to always specify formatversion=2.
  However, this is not done in other places in this codebase.
  It should never be done or always.
* I find it confusing when the action=… is missing. Let's not
  rely on this default.

Change-Id: I6ca29f76bffc0849103c5bcff4aaf28fcaaa4c52
2021-07-12 09:13:59 +02:00
jenkins-bot c2bc8c7e26 Merge "Move code that belongs to the template specification" 2021-07-08 08:46:40 +00:00
Thiemo Kreuz 3cd91100a9 Move code that belongs to the template specification
Separation of concerns:
* The template model knows which parameters are currently used,
  but doesn't know what's documented.
* The spec knows what's documented, but doesn't know what's
  currently used.

Change-Id: I97cac00d6775a17a07059d0e8a7a116adc6080b3
2021-07-07 16:45:35 +02:00
Thiemo Kreuz 1c98f4cce0 Remove unnecessary code from template related classes
For example, checking if a parameter is required works just fine
for unknown parameters. They are never required. Since I16708b0
we don't need to guard the spec related methods any more.

Change-Id: Id90e4cb810dc9faca3b26f122a534f276ee31709
2021-07-07 10:18:41 +02:00
Thiemo Kreuz 06bc660b35 Split MWTransclusionModel methods to do one thing only
Before the method fetchRequestAlways() was doing two entirely
different things. Note how the two function arguments are
split now. Each method uses only one of them.

Change-Id: I592a1f29fd9c677a0ff18115cccda36950172001
2021-07-06 15:50:09 +02:00
Thiemo Kreuz e3d3fd9eaf Remove unused parameters from MWTransclusionModel methods
Change-Id: Icdeddb92a498e0dd1182b19de1c4effdb1741fef
2021-07-06 15:47:32 +02:00
Thiemo Kreuz 137be4438f Remove unused .getWikitext() methods from transclusion classes
These methods are special in so far that they create *minimal*
wikitext where optional whitespace is not preserved. I tried
to rename the methods to reflect this, but could not find a
caller. What's used instead are the .serialize() methods.

Bug: T284895
Change-Id: Iedaa5b7efa9675151cc0553854d8aef3f9a46cbb
2021-07-06 10:58:11 +02:00
jenkins-bot 9e6387176b Merge "Add @private/@protected documentation to template dialog code" 2021-07-06 08:41:55 +00:00
jenkins-bot de71ebc9de Merge "Simplify ve.dm.MWTemplateModel.hasParameter() a lot" 2021-07-06 08:23:15 +00:00
jenkins-bot 4a5f811c82 Merge "Keep template parameter position when resolving aliases" 2021-07-05 11:44:19 +00:00
jenkins-bot 8504fd9545 Merge "Minor code cleanups to the MWTemplateSpecModel class" 2021-07-05 10:01:46 +00:00
Thiemo Kreuz fbb7d211d0 Simplify ve.dm.MWTemplateModel.hasParameter() a lot
A lot of the checks are redundant. The first check still is
redundant because the later two cover everything as well. But
I left it for performance reasons.

Additionally:
* There was no test for the method.
* This patch also updates a few pieces of documentation in the
  same class.

Change-Id: I10f2944a844cc070bdc08dec6719929b383e34fa
2021-07-05 11:58:42 +02:00
Thiemo Kreuz 16ca60009b Minor code cleanups to the MWTemplateSpecModel class
Bug: T285483
Change-Id: I22005907effe855ab5e830c94d2bc32c640b5aa5
2021-07-05 09:16:23 +00:00
jenkins-bot 6acdb3bc8e Merge "Add missing JSDoc documentation to template related classes" 2021-07-05 09:09:40 +00:00
jenkins-bot 7cd766becf Merge "Avoid calling own getters in template model class" 2021-07-05 09:09:38 +00:00
Andrew Kostka 6af13f0d42 Fix parameter ordering when using aliases
If a known parameter is present using one of it's aliases, then
only the aliased name should be shown to the user. This patch,
therefore, resolves the issue of the same parameter being added
to the sidebar twice.

When adding a parameter that is aliased, it will receive the same
position as the non-aliased parameter it is replacing.

Bug: T274545
Change-Id: If4e58c941fd0f0e690d3603935f5a5d3f9938163
2021-07-05 08:14:42 +00:00
Thiemo Kreuz b849131e74 Add missing JSDoc documentation to template related classes
This also removes a few lines of text that don't explain
anything that would not be obvious from the code or @return
tag anyway.

Change-Id: I2f8f02dd61c50d9990d72c0e8ea79d679c9b11f2
2021-07-05 09:32:22 +02:00