Commit graph

57 commits

Author SHA1 Message Date
Thiemo Kreuz 8831887e3c Update template search fallback to max out at 10
Before, the fallback algorithm was somewhat adaptive, trying harder and
harder the fewer CirrusSearch results have been found. This updated
algorithm guarantees that 10 results are shown.

Warning: You might see only 9 results. The reason is a bogus, unrelated
behavior in the mw.widgets.TitleWidget in core that's used as a base
class here. There is a "showMissing" option that's apparently enabled
by default, and prepends a non-existing title from the main namespace,
ignoring the "namespace" option. This extra result from the wrong
namespace is later dropped by the very same widget.

This code here sees 10 results before the bogus one is dropped.
Disabling "showMissing" causes other issues. We would need a series (?)
of custom hacks to work around all this, but this seems inappropriate.
Let's live with 9 for the moment.

Bug: T303524
Change-Id: I2c577c9ef2752b6c6cd360f4023e151e9272fcd5
2022-04-21 09:25:00 +00:00
Thiemo Kreuz 50497ee7f3 Drop assumption that all template search index start at 1
The main advantage of this change is that it drops the assumption
that the index starts with 1. This is not necessarily the case when
we prepend extra search results. Dropping this assumption here allows
to simplify such code.

* The incoming list of pages is guaranteed to be an array.
* There is no point (any more) that could cause the array to become
  sparse.
* Note we still make a copy of the `origPages` array at some point,
  e.g. on `.filter()`.

Bug: T303524
Change-Id: Ifbd92bb052155c613d2ca21ab6d54a0b3ef28c0c
2022-04-21 11:23:48 +02:00
Thiemo Kreuz 89e5c2fb92 Disable useless "showMissing" option in TemplateTitleInputWidget
This option is not only buggy (it just doesn't work when the namespace
option is set the same time), it is not useful in this context even
if it would work. It doesn't make much sense to suggest non-existing
templates in the context of the template dialog. If adding a
non-existing template really is what the user wants, they can still do
this by simply typing the name of the template and submitting the form.
We never need this to show up in the suggester below the input field.

The main advantage of this change is that is saves 1 useless API
request that's potentially done every time a key is pressed.

Bug: T303524
Change-Id: I903340a06d6e6490bb58f628f41903aa044ccb21
2022-04-20 14:43:29 +00:00
Thiemo Kreuz d3d3e1cf75 Move "index from redirect" logic up in template search widget
This separates the two steps:
1. See if items in the list of `origPages` miss their `.index`
   property, and add it if possible.
2. Later code doesn't need to care about redirects any more.

Note that `origPages` is not used for anything else. And even if,
it's not wrong to have the index for each search result on both the
redirect and the redirect target.

Change-Id: I12135f0430c944b4e33c49ece7779d7c3bb6c211
2022-04-20 14:50:36 +02:00
Thiemo Kreuz e24c8ecd2d Fix misleading variable name in TemplateTitleInputWidget
This is not a pageid, but a simple numeric index in an array. Luckily
it doesn't make a practical difference for this particluar way of
iterating something.

Change-Id: I7ec9ace00d4fba7adde17670058a0365b30f5617
2022-04-19 14:40:00 +00:00
Thiemo Kreuz af2779fc06 Fix API response default in TemplateTitleInputWidget
The result is guaranteed to be in formatversion=2, where the list of
pages is an array, not an object.

Change-Id: Ic73a68c3e249a70108a6a19a89f4ff6c475794ed
2022-04-19 13:35:27 +02:00
Thiemo Kreuz 912bc34f62 Fill template search results with prefix matches when Cirrus fails
This code is optimized for the 2 most relevant use cases:

1. When Cirrus finds 10 results, we still want to search for the top 1
prefix match. This is critical for templates like !!. This will appear
at the top. unshiftPages() makes sure the limit of 10 is enforced.

2. When Cirrus fails to find anything, we search for 10 prefix matches
and use these instead.

The code can also handle everything in between. For example, when
Cirrus finds 5 results, we search for 5 more prefix matches and add
them when Cirrus missed them. The total number in the end might be 5 to
10 depending on the number of duplicates. This is intentional. Why?
Let's say we always search for 10 prefix matches and add them to the
top when Cirrus missed them. This might remove _all_ Cirrus results.
This shouldn't happen. This extra code is only to fill in glaring gaps,
not to replace Cirrus. 5 results are fine.

Bug: T303524
Change-Id: Ib0471795124c0c7001b6901edaf8e7b380e426b1
2022-03-29 13:18:12 +02:00
Thiemo Kreuz 6bbf7d1907 Full fallback to prefixsearch when Cirrus returns nothing
There is already some kind of "fallback" to prefixsearch. We always
check if the top-1 prefixsearch result is part of the result set.
Because of this the current worst-case scenario is that only this
1 result is shown.

This patch implements a full fallback to prefixsearch. But only when
there are 0 CirrusSearch results. Further tuning might be done in
later patches.

Bug: T304925
Change-Id: I1927eedad60c9b9ac2021481a85376c08ccf6fdb
2022-03-29 13:17:05 +02:00
Thiemo Kreuz 579cdaa145 Enforce formatversion=2 in template search
This is guaranteed via ve.init.mw.Target.getContentApi(). But the
ContentTranslation extension replaces this, and does not set a
formatversion. See e.g. SectionTranslationTarget.getContentApi().

Bug: T298599
Change-Id: I8768cae3153e9cbc29a8796ec21ef249f80471ed
2022-01-05 12:49:44 +01:00
WMDE-Fisch f65eac3d66 Add placeholder for finding a template
Bug: T296465
Change-Id: I07c6e601111073b1269a3ab6c552f83e12196156
2021-11-26 16:30:19 +01:00
WMDE-Fisch bc89f1d8fd Set search icon on template placeholder search
Bug: T296465
Change-Id: I02932821b26c21eb559fdb391054b83d1da41d2a
2021-11-26 14:34:44 +00:00
Ed Sanders 33840e88ee build: Update eslint-config-wikimedia to 0.21.0
Change-Id: I19465a5ab3bf71cd97967fd1fac41c645f05a419
2021-11-10 14:52:56 -05:00
Thiemo Kreuz 866547d081 Harden …TemplateTitleInputWidget to avoid JS console spam
I run into this in some local test. There are two reasons this code
can be reached:
* When a wiki doesn't have the TemplateData extension, the
  additional API call from line #154 will fail. But the original
  search query succeeded. We have the `originalResponse` and can
  return it. This makes the code behave as if the additional
  TemplateData API call was never done.
* But what if the original search query failed? We still end in
  line #183 – as we should. But this time it can't return anything
  but undefined. This will be considered a valid, successful API
  response. But it isn't.

There might be a better way to clean up this chain of promises.
This is the smallest fix I found.

Change-Id: I02d3d053156da222ee424382007621f314777015
2021-10-06 20:14:55 +00:00
Thiemo Kreuz aa556e3ef8 Update and fix all @param config and @cfg documentation
I tried to review all of them. Some of the changes I did:
* Make sure the `config` parameter is not marked as optional
  when it is not.
* Make sure default values are mentioned.
* List individual `@cfg` options when it makes sense.

Note I don't list all options a class could accept (e.g. via all
its parent classes and mixins). That's too much. Instead I checked
how a class is actually used and list only these options.

Even then I don't list everything, e.g. unspecific options
like "classes" that can be used pretty much everywhere.

Change-Id: Idf4fbe1dc3608ace277df9e385f2f140df3a2f50
2021-09-12 12:35:27 +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
Thiemo Kreuz 12dca65912 Delete last template search result, not a random one
Because the API uses a generator, the search results are ordered
alphabetically. The actual search result ranking is in a .index
field. This code accidentially deleted the alphabetically lowest
template instead of the least relevant one.

Change-Id: I79de024feb569e9f06bedab908a6509a4d4fa99b
2021-07-08 15:33:42 +02:00
Thiemo Kreuz ba1718ea11 Always add 1 prefixsearch match when searching for templates
We do this additional prefixsearch anyway. What we did before
was ignoring the result when it was not a 100% exact match.
Instead we can always add this 1 prefixsearch result when it
was not already part of the CirrusSearch result set.

This won't happen often. Usually the 1st prefixsearch result
was already part of the CirrusSearch result set anyway. But
if it wasn't, that's a serious issue for expert users that
expect the search to behave similar to the suggester at the
top of the MediaWiki interface (which is also a prefixsearch).

Change-Id: I959d2b058a3d64596a8cfbe5476ab351e40f8760
2021-07-08 11:15:34 +02:00
Thiemo Kreuz db531ddba5 Add missing search result limitation to template search
Bug: T274903
Change-Id: If57d28f7796d69825a25fa9b67bcd5c127c9e3b7
2021-07-01 09:33:52 +02:00
Thiemo Kreuz b3bede3ba0 Extract MWTemplateTitleInputWidget.addExactMatch into a method
Same as in TemplateWizard, see I128bc00.

Bug: T274903
Change-Id: I8693b85625a8640e44c213300d7c6862430bec47
2021-06-25 17:07:32 +02:00
jenkins-bot 2cbb32302f Merge "Inline many var declarations in the code below" 2021-06-23 09:24:05 +00:00
Thiemo Kreuz 4367235dcc Inline many var declarations in the code below
This makes the code more readable and easier to reason about.
The ESLint rule responsible for this code style was removed
just recently.

Notes:
* I focus on classes that are relevant for what the WMDE team
  does right now.
* I merge multiple `var` keywords only when the variables are
  strongly connected.
* Caching the length in a for loop makes the code hard to
  read, but not really faster when it's a trivial property
  access anyway.

Bug: T284895
Change-Id: I621fed61d894a83dc95f58129bbe679d82b0f5f5
2021-06-23 09:02:24 +00:00
WMDE-Fisch c4f59132ae Only add asterisk after word characters in improved template search
This will avoid that the search breaks in edge cases where symbols
are used.

Including a fallback for ES5 browsers. The fallback should cover
almost all cases. Worst case would be not adding the asterisk even
though it might be valid.

Bug: T284554
Change-Id: Ie4aee0b77492b7a73bc251a8723a206dbd641600
2021-06-21 21:07:28 +02:00
jenkins-bot c29dc420e0 Merge "Fix object vs. array initialization in template search" 2021-06-14 13:25:30 +00:00
jenkins-bot df5238cd11 Merge "Fix duplication bug in MWTemplateTitleInputWidget" 2021-06-04 10:50:39 +00:00
Thiemo Kreuz e97e325ac6 Guarantee exact match when searching for a template
When the existing search results don't contain an exact match
(see previous patch), perform an additional search for the
title. This uses OpenSearch. This is recommended in multiple
places and also used in the quick search field at the top of
MediaWiki.

Again, I came to the conclusion that an isolated unit test
would be complicated and not test much anyway. Better test
on-wiki.

Bug: T274903
Change-Id: Ib575248e089ff66814400202d224deff6369c772
2021-05-31 08:31:49 +00:00
Thiemo Kreuz 1a37edc53f Move exact matches to the top in template search
This code detects a few edge-cases:

1. When some search results are exact matches, make sure they
are always at the very top.

2. When the prefixsearch API is used, e.g. as a fallback,
redirects show up as a separate metadata structure outside of
the pages array. Consider these and stop if there is already
an exact match.

3. CirrusSearch returns redirects as part of the pages array.
When there is an exact match, make these redirects separate
options and add them to the top.

All of this is case-insensitive, on purpose. In case two
templates with different capitalization exist, we rely on
the backend to return both. The code introduced here is fine
with this.

Notes:
* This doesn't guarantee an exact match is always there. This
  requires an additional HTTP request and is done in the next
  patch.
* I tried to write unit tests for this, but gave up. The setup
  is complicated. An isolated unit test would not test much
  anyway. Better test this on-wiki.

Bug: T274903
Change-Id: I64e1b5633e7b878a4d0d23d66229ca87e69d0045
2021-05-31 09:54:39 +02:00
Thiemo Kreuz 621bca1df4 Show redirects as part of description in template search
These are the most minimal (and therefor most stable,
hopefully) hacks I could come up with so far.

Bug: T274903
Change-Id: I28ba414dd34aad756e29400eb656f0942291a923
2021-05-27 12:40:51 +02:00
jenkins-bot 028658a4e1 Merge "Remove/fix a few small pieces of unused code" 2021-05-24 13:23:53 +00:00
Thiemo Kreuz 8f045a7155 Remove/fix a few small pieces of unused code
I found these thanks to PHPStorm.

Change-Id: Ieb30a95debb58d3a454ac3c6f0546e5dbbe77ed4
2021-05-21 11:43:45 +00:00
Thiemo Kreuz b15455ec3d Fix duplication bug in MWTemplateTitleInputWidget
Template names sometimes show up twice when searching for a
template in the "Add a template" dialog.

This is a bit hard to test. The code responsible for this
is not in a single place. The feature is in the upstream
TitleWidget class. It's not broken. It makes sense to
provide e.g. "foo" and "Foo" as two separate options when
the user typed "foo", but the page is named "Foo". Both are
valid, and the feature allows the user to pick either.

But the VE widget does it's own normalization. Both entries
are normalized to "Foo". Both do the same. The additional
one is pointless.

You can try this on the actual enwiki: Open VE, insert a
template, search for "Template:nHLE".

Change-Id: I65e706c4d131a2f8c605d7979a02ea56f831bf03
2021-05-21 11:43:31 +00:00
Thiemo Kreuz b7043d6f24 Fix object vs. array initialization in template search
The "redirects" part in a prefixsearch query is always an
array, no matter if formatversion 1 or 2 is used.

The "pages" part is an object with formatversion 1, and an
array with formatversion 2.

As of now this always uses formatversion 1. This is
hard-coded in the upstream TitleWidget class.

Change-Id: I8cde8e104f8a288015da745db41016f6639b453b
2021-05-21 11:24:48 +02:00
Thiemo Kreuz 333cadd5d4 Add star to template search term only when it's possible
Discussed in T274903#7077957. Note this might not be the
"perfect" solution. We are still experimenting, and this is
all hidden behind a feature flag. This is the change with the
most minimal impact. Actively trimming the input is another
solution, but with a bigger impact we might want to discuss
first.

Bug: T274903
Change-Id: I2ed06c04bb96c7b61bd7e87ad001e639ea6d06a2
2021-05-11 16:31:42 +00:00
Thiemo Kreuz 6e08a27fc7 Use standard search API when searching for templates
Bug: T274903
Change-Id: I7de8f6cc55ab678ed741ae5ebbaad608b9a9b0db
2021-04-30 12:52:45 +00:00
Thiemo Kreuz d43f13f4f8 Reduce deep indention in MWTemplateTitleInputWidget
Change-Id: Ie1bd279753dba3b96b694660977b9f44f6f0834e
2021-03-02 14:34:41 +00:00
WMDE-Fisch 094a9aa044 Replace deprecated doNotIgnoreMissingTitles
This parameter name was deprecated and replaced in 1.31. See also
Ie5fe2097cda45968bb080643d3afcac0b2868a6c

Change-Id: Ie9d6c70d3dfe3954504d3d698c122dceede7603d
2020-12-15 12:52:55 +01:00
James D. Forrester 2c77e88d2c doc: Bump copyright year for 2020
Change-Id: I30539877543dc2a57bd1428a00d10ac46d8fc294
2020-01-08 09:13:24 -08:00
Ed Sanders 934572cdf7 API: Use formatversion=2 by default
V2 has better handling for booleans.

Change-Id: I5388bd2cc64ae0c9ed4a185adce1e3bb4cdd92fc
2019-04-30 19:31:28 +01:00
Ed Sanders 0db4ae6e00 eslint: Enable valid-jsdoc
Change-Id: Ia0d1e57246a1c567d73022ceca9b8c02850f9bc8
2019-04-17 17:13:39 +01:00
James D. Forrester c1d690e53d MWTemplateTitleInputWidget: Filter out templates named '…/doc'
As configured on-wiki via MediaWiki:templatedata-doc-subpage; this will
probably have a few false positives, but that's worth it.

Bug: T54448
Change-Id: Id91f95b5865e151f8007a2421428aeb82b11b3fd
2019-01-26 13:23:38 -08:00
James D. Forrester 3c293ea00c doc: Bump copyright year for 2019
Change-Id: I8991b97c980d4149f53eb5601036220ef3c0c440
2019-01-01 13:24:23 +00:00
David Lynch bd64b58071 MWTemplateTitleInputWidget: recover gracefully if TemplateData not present
Bug: T187917
Change-Id: Ica0c853e1a7bc908651260df115d59d1939a7939
2018-02-21 11:53:56 -06:00
James D. Forrester 0a7a845a42 doc: Bump copyright year
Change-Id: I0b299c840ede1a1b8552cecfc70c5760ab036181
2018-01-03 17:45:07 +00:00
James D. Forrester 8fd621ad74 MWTransclusionModel: Update for change in TemplateData
Depends-On: I7905502d0c419a04e4487095214634f1513b461c
Change-Id: I97a1bfc9f9ead082a673a91b9d2053630a90309c
2017-04-24 19:13:59 +00:00
James D. Forrester 122f49b2dd build: Bump file copyright notices for 2017
Change-Id: I3c20809e71cc0da58123e1b5f29c4f3aac945496
2017-01-03 08:58:33 -08:00
Alex Monk c843824c97 Batch gallery imageinfo requests via ImageInfoCache subclass
Also get rid of .join( '|' ) for API request parameters per Bartosz

Bug: T147067
Change-Id: If4444cca300d65e28d6fb9003fcac5e076a5129a
2016-12-06 22:10:19 +00:00
Alex Monk 5bd95db994 Use widget.getApi, not this.getApi
Bug: T152154
Change-Id: Id7e77323ffabf4eeee3e73481cf6bcb9a1e649dc
2016-12-01 22:48:45 +00:00
Ed Sanders 37910b78ec MWTemplateTitleInputWidget: Use new #getApi method from upstream
Depends-On: I81811cdd1a0750a8335432eee8f971ab9e0b8ee7
Change-Id: Ic73213df5ede6f82de85df8b69c60dd31f033792
2016-11-30 19:51:45 +00:00
Ed Sanders e655880d14 eslint: Remove unused exception and fix documentation errors
Don't enable valid-jsdoc yet though, due to @chainable bug.

Change-Id: I4d2a6de19c72c6e4c20733446616d8046419d431
2016-10-28 12:02:36 -07:00
James D. Forrester 36befda61c build: Replace jscs and jshint with eslint
It's new, it's fresh, it's amazing, it's here.

Change-Id: I5dc784411f704685ed5cc763a2b2b1c5d3e5a610
2016-10-28 18:33:15 +00:00
James D. Forrester ada58df361 build: Bump file copyright notices for 2016
Change-Id: I3c618c196e504a80ca297a4132a17f1977a24fb7
2016-01-03 14:57:25 -08:00