Commit graph

25 commits

Author SHA1 Message Date
Ed Sanders 9b3029e0ee Build: Update linters
Change-Id: Ia2a833a01e1bb05d6be3923dd452b1851afd7655
2019-09-17 12:47:25 +01:00
Ed Sanders 4d885286a3 eslint: Enforce template-curly-spacing
Change-Id: I5640e86cba25f6100c7814c2ef8a845941f73497
2019-08-15 10:24:43 +02:00
WMDE-Fisch a498423f90 Avoid popups on self links with file extension
This fixes page previews appearing when there is self link to a page
that has a file extension in it's name.

Bug: T222869
Change-Id: I5caf610fa76986026948a5b7b55537723752b755
2019-05-10 18:02:50 +02:00
Thiemo Kreuz 8305eb8634 Minimize createStubTitle() helper method
Pretty much all usages of this function do *not* use the second
parameter to pass a page name prefixed with a namespace, but pass the
page name only.

This patch here removes the redundancy. The namespace prefix is now a
generated one, saying "Namespace <number>:…". It turns out no test
relies on this so far.

Bug: T220097
Change-Id: Ibd45d49c91e86a2647afe676a5e3bb07dfeab6ed
2019-04-26 07:45:13 -06:00
WMDE-Fisch 6c535739bf Use title.getNameText() to compare selflinks
config.get( 'wgTitle' ) returns the unnormalized title of the current
page while title.getName() gets the normalized title (e.g. with underscores ).

On pages with spaces in the check failed before this patch.

Bug: T220097
Change-Id: I58a532627bb27be030cbc553f1181a89109edd80
2019-04-26 13:58:08 +02:00
WMDE-Fisch 35aa05afee Use title and namespace id to check if link is current page
Bug: T220097
Change-Id: Ieffd6a02b4126f6713610e968d662516499d4998
2019-04-23 15:17:06 +02:00
Thiemo Kreuz 97a5d335d7 Replace rare {!…} and {?…} JSDoc syntax
This is documented at http://usejsdoc.org/tags-type.html, but not in many
other places, especially not in the JSDuck documentation.

The {!…} syntax means "can not be null". This is the default anyway.

The {?…} syntax means nullable. In a few situation is was used when a
parameter can be undefined. I decided to remove it everywhere and replace
it with {…|null} when appropriate, because this is much more explicit. Less
syntax to remember.

Note I'm intentionally not using the […] syntax when a parameter is followed
by non-optional parameters. Actually skipping a parameter in such a situation
would mess the parameter order up. Having optional parameters not at the end
is sometimes used as a feature in JavaScript code, but not in this codebase,
as far as I can see.

Change-Id: Ie370cfe08c32d1af5b0341951bed044fc3511c57
2019-01-24 21:00:45 +01:00
Thiemo Kreuz 553e76e2bc Add QUnit tests for most new reference preview code
Excluding tests for the renderer which keeps failing. This will be
readded in a later patch.

Bug: T213415
Bug: T213908
Change-Id: If79fa3d0a7a20f121b1ceda6e0e33ad691b1ad30
2019-01-24 19:35:38 +01:00
jdlrobson 53d1a2c329 Use getPageviewToken api
Currently the Popups schema is disabled but if we were to re-enable
it, the page tokens would now be consistent with the ReadingDepth
schema

The getToken function is kept to allow generation of the
pageInteractionToken's relating to a link interaction, for which
there is no centralised API.

Additional changes:
* I've updated various tests to use the central mw.user stub to make
clearer where tokens come from.

Bug: T203013
Change-Id: If746bea5aeed2b4c192a9b8a02feb1fe06480633
2018-09-05 20:02:17 +00:00
Stephen Niedzielski ab7a5808ef Hygiene: update JSDoc boxed and JQuery types
Although Popups only uses JSDocs at this time which seemingly doesn't
care about casing[1], we should endeavor to use the proper return types.

This patch lowercases typing to indicate primitive / boxed type as
appropriate.[2] As a special case, function types are uppercased for
compatibility with TypeScript type checking.

Lastly, JQuery types are of type "JQuery". The global JQuery object's
identifier is "jQuery". This patch uppercases J's where appropriate.

[0] https://github.com/jsdoc3/jsdoc/issues/1046#issuecomment-126477791

[1] find src tests -iname \*.js|
    xargs -rd\\n sed -ri '
      s%\{\s*([?!])?(number|string|boolean|null|undefined)%{\1\L\2%gi;
      s%\{\s*([?!])?(function|object)%{\1\u\2%gi;
      s%\{\s*([?!])?jquery%{\1JQuery%gi
    '

Change-Id: I771bdbb69dc978796a331998c0657622ac39c449
2018-07-17 08:20:08 -05:00
Stephen Niedzielski 007cc5ab7c Hygiene: favor string templates over concatenation
Update the ESLint config to favor ES6 strings where variables or string
concatenation is used and fix *all* offenders automatically via:

  eslint \
    --cache \
    --max-warnings 0 \
    --report-unused-disable-directives \
    --fix \
    src tests

Change-Id: I1a9345162348e5ded21d541e7a2ce251ea72ab5a
2018-03-21 08:05:55 -05:00
Stephen Niedzielski 57762e0417 Hygiene: favor const
Bug: T165036
Change-Id: I17d374eaac6627ca6a8ba178862b2a9cff2538c0
2018-03-21 10:44:24 +00:00
Stephen Niedzielski 0bee0906d4 Hygiene: replace var with let and const
eslint \
    --cache \
    --max-warnings 0 \
    --report-unused-disable-directives \
    --fix \
    src tests

Change-Id: I051275126ae7fa9affd16c2504017c0584f2d9c7
2018-03-20 14:14:02 -05:00
Stephen Niedzielski a67466acc0 Hygiene: replace obvious function methods
Replace easily identifiable object functions with method syntax:

  find \
    -not \( \( -name node_modules -o -name .git -o -name vendor -o -name doc -o -name resources \) -prune \) \
    -iname \*.js|
  xargs -rd\\n sed -ri 's%:\s*function\s*(\([^)]*\))%\1%g'

Bug: T165036
Change-Id: I90693ee99a6a36dff820dd5ae6f6000429763058
2018-03-20 09:27:07 -05:00
Stephen Niedzielski a2a743d775 Hygiene: use object shorthand where obvious
Approximately:

  find \
    -not \( \( -name node_modules -o -name .git -o -name vendor -o -name doc -o -name resources \) -prune \) \
    -iname \*.js|
  xargs -rd\\n sed -ri 's%(\b\S+\b)\s*:\s*\b\1\b%\1%g'

Bug: T165036
Change-Id: I48869dc93b66f908e070288eb2f035bb064993e3
2018-03-20 09:26:20 -05:00
Stephen Niedzielski 8ca0cf2088 Fix: preview page URL for 404 RESTBase responses
Functional changes:

- Require page URL when constructing a PreviewModel null object. These
  models have valid titles and are used to display a preview when an
  extract is unobtainable. When presented with an empty URL, their
  linkage incorrectly pointed to the browser's current URL. Additional
  tests were added to verify the fix.

- Check missing title in addition to falsy response in RESTBase gateway
  and update the test assertion to check title. It isn't clear if this
  can happen in the wild.

- Forbid state mutation in the conclusion of
  MediaWikiGateway.getPageSummary() with a call to Deferred.promise().
  This is consistent with the rest of repo including RESTBaseGateway.

  http://api.jquery.com/deferred.promise/

Nonfunctional changes:

- Collapse two RESTBase gateway 404 tests into one as the scenarios and
  expectations were very similar.

- Add failure HTTP status to 'MediaWiki API gateway handles API failure'
  test stub HTTP response for consistency with other cases.

- Add nullity expectations to JSDocs touched and fix a couple typos
  throughout.

- Make the gateway tests a little more consistent by collapsing Deferred
  variable usage where appropriate.

This change is necessary to the completion of T183151 which uses the
PreviewModel null objects for additional error cases.

Bug: T183151
Change-Id: Ib77627fb9c80d8e806208bbafcfc615b130e3278
2018-01-16 18:36:48 -06:00
jdlrobson e4e9bb3bd6 Popups A/B test infrastructure
Introduce PopupsAnonsExperimentalGroupSize config variable. This defines
a population size who will be subject to experimentation. If the group
size is undefined or 0 (default) and PopupsBetaFeature is false
(default value) Popups will be enabled for everyone. If it is any other
value, half that group will see page previews.

Drop the config variable PopupsSchemaSamplingRate - we will now only
EventLog when an experiment is occuring. This means we can simplify the
MWEventLogger class as shouldLog will always be truthy. Given server
side eventlogging is only used for preference changes
traffic should be low and not need sampling.

Introduce getUserBucket which determines whether a user is in a bucket
on, off or control based on the value of
PopupsAnonsExperimentalGroupSize. Add tests showing how these
buckets are calculated.

Caution:
A kill switch wgPopupsEventLogging is provided for safety.
It defaults to false. Before merging, please check if any config changes
are necessary.

Bug: T171853
Change-Id: If2a0c5fceae78262c44cb522af38a925cc5919d3
2017-08-17 21:07:07 +00:00
joakin e6081106f1 Use EcmaScript modules instead of common.js modules
Why: Because they are the approved standard by TC39 and Ecma for
JavaScript modules.

Changes:
  * Wrap mw-node-qunit in run.js to register babel to transpile modules
    for node v6
  * Change all sources in src/ to use ES modules
    * Change constants.js to be able to run without
      jQuery.bracketedDevicePixelRatio given ES modules are hoisted to
      the top by spec so we can't patch globals before importing it
  * Change all tests in tests/node-qunit/ to use ES modules
  * Drop usage of mock-require given ES modules are easy to stub with
    sinon

Additional changes:
  * Rename tests/node-qunit/renderer.js to renderer.test.js to follow
    the convention of all the other files
  * Make npm run test:node run only .test.js test files so that it
    doesn't run the stubs.js or run.js file.

Bug: T171951
Change-Id: I17a0b76041d5e2fd18e2d54950d9d7c0db99a941
2017-07-31 23:05:44 +00:00
Sam Smith fb8d54c7ce actions/rest: Use DB-key version of title
This reduces the number of 301 Redirect responses when fetching previews
from RESTBase.

Bug: T167633
Change-Id: I830947ab79e72dcc023193412c8d5bcee986e23f
2017-06-12 11:22:55 +01:00
Sam Smith 35bf613964 eventLogging: Add missing *Hover properties
Action changes:
* Include the namespace ID in LINK_DWELL.

Reducer changes:
* Add the createEvent helper, which adds the pageTitleHover and
  namespaceIdHover properties in the event.
* Create "dismissed", "dwelledButAbandoned", and "opened" events using
  the createEvent helper.

Additional changes:
* Store the target page's associated mw.Title using jQuery.
* Update the eventLogging reducer's test cases so that all LINK_DWELL
  representations include title and namespaceID attributes.
* Create the createStubTitle factory function, which returns a minimal
  usable mw.Title stub, and use it in the actions and integration test
  cases.

Bug: T164256
Change-Id: I8a63d82a65324680dff9176020a8ea97695428c4
2017-05-13 08:27:58 +01:00
joakin 406d41d9ae Hygiene: Tests: Remove unused stubs
Bug: T160406
Change-Id: Ifa58ebfd53c037ac483f4b480efc581f18ecb936
2017-04-28 22:39:49 +01:00
joakin c9d325d01e Tests: Migrate processLinks.test.js to node-qunit
Tests are basically unchanged, except for some stubs on beforeEach.

Supporting changes:
* Bring stubs from the mediawiki library for mw.Uri,
  mw.Title.newFromText and mw.RegExp into stubs.js
* Remove hook onResourceLoaderTestModules given there are no resource
  loader test modules after migrating processLinks.test.js

Why bring stubs from real source? This is not optimal. It could be the
case that the stubs would need to be updated at some point in the
future. That's why in the comment of each stub, it is specified where it
came from, and what was changed to make it work. It is not optimal but
it should help with a future update if necessary.

Also checked the history of the stubs and these three stubs are very
stable with a small commits per year, usually adding some extra
functionality (not breaking changes) (the rest of the commits are
docs/format stuff), so the core behavior that we rely on here shouldn't
change in a fundamental way. See the github links:

* https://github.com/wikimedia/mediawiki/commits/master/resources/src/mediawiki/mediawiki.Uri.js
* https://github.com/wikimedia/mediawiki/commits/master/resources/src/mediawiki/mediawiki.Title.js
* https://github.com/wikimedia/mediawiki/commits/master/resources/src/mediawiki/mediawiki.RegExp.js

Right now this stubs allow us to bring the test to run in isolation in
node.

The initial plan was to do change the test to be less test-case oriented
with dependencies on mediawiki.*.js and not to bring fake "real" stubs,
but after looking into it, given that:
1. the test cases in the test seem pretty informative showing the kind
   of links that popups accepts
2. the stubs are acceptably easy to bring in, and are pretty stable
I decided to go with this approach initially to finish the migration
without changing the meaning of the tests.

If we want to remove the stubs and morph the test to verify stub calls
and move the test cases to documentation on the source, I'll tackle that
on a future commit.

Bug: T160406
Change-Id: Ieea378c9b7fec9116222b4a099c226d1f1131f65
2017-04-26 12:26:43 +02:00
Baha 9a94300858 Log events to statsv for monitoring PagePreviews performance
For logging to work:
1. $wgWMEStatsdBaseUri needs to point to a valid statsv endpoint,
   e.g. 'https://en.wikipedia.org/beacon/statsv'.
2. $wgPopupsStatsvSamplingRate needs to be set. Note that the codebase
   already contains the EventLogging functionality, which is configured
   separately. Separately configuring different logging mechanisms
   allows us to avoid sampling mistakes that may arise while choosing
   one or the other. For example, let's say we want to use EventLogging for
   10% of users and statsv for 5%. We'd sample all users into two
   buckets: 50/50. And then we'd have to set the sampling rates as
   20% and 10% respectively, only because of the bucketing above. To avoid
   this kind of complications, separate sampling rates are used for each
   logging mechanism. This, of course, may result in situations where a
   session is logged via both EventLogging and statsv.
3. The WikimediaEvents extension needs to be installed. The extension
   adds the `ext.wikimediaEvents` module to the output page. The
   logging functionality is delegated to this module.

Notable changes:
* The FETCH_START and FETCH_END actions are converted to a timed action.
* The experiments stub used in tests has been extracted to the stubs
  file.

Logged data is visualized at
https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews

Bug: T157111
Change-Id: If3f1a06f1f623e8e625b6c30a48b7f5aa9de24db
2017-03-14 08:51:10 +00:00
joakin 35c7068fbe Tests: Extract createStubMap
Minimal mw.Map stub that covers get with key and a default value

Change-Id: I15d60d78ed86747a94f371fd3df400906f0c6dab
2017-03-01 12:40:42 +01:00
joakin f3839189dc Test: Migrate isEnabled.test.js to node-qunit
Change-Id: I85a08725c9138f47a51cdc34c20bc3a60b61af34
2017-02-20 20:01:01 +01:00