Commit graph

390 commits

Author SHA1 Message Date
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 422a02d61a Hygiene: remove unneeded renderer test setup
Change-Id: Id3e0a4af1f88c57c3ef141dac67a8b13643a2dcb
2018-03-20 09:27:08 -05:00
Stephen Niedzielski ece4670710 Hygiene: use arrow for anonymous functions
In some places, the arrow function seems more natural. This patch
approximates the following with manual amendments:

  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%function\s*(\([^)]*\))%\1 =>%g'

Files to focus on were identified with:

  rg -slg\!/resources/dist/ -g\!/i18n/ -g\!/doc/ 'this|self|arguments|bind|call|apply|new'|
  xargs -rd\\n git difftool -y

Bug: T165036
Change-Id: Ic66b6000b8fc000f9bfde39749f9cfa69924a13c
2018-03-20 09:27:08 -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 42816702eb Hygiene: replace Mustache templates w/ ES6 strings
Replace Mustache.js templates with template literals. An effort was made
to minimize additional refactoring, so feel free to ask for more but it
ain't coming in this PS.

Bug: T165036
Change-Id: I4a6a1d93a2922c3a9ef3ae93c47da17a35c644f0
2018-03-20 08:06:02 -05:00
jdlrobson 695a7b6b58 QA: Cleanup unnecessary test changes
Various tweaks were made in Ibe212721807d3698dc45ef46b2dbde15ca9d2f70
to get the browser tests to pass. Many of them were unnecessary.

Change-Id: I47ac37512da38a33f4b3b919cc412b79231b0324
2018-03-16 09:53:04 -07:00
Stephen Niedzielski 674ade83e7 Hygiene: move settings dialog to separate file
Separate the settings dialog and settings dialog renderer into two
files.

Bug: T165036
Change-Id: I500add4b89ec8c5d1a76d7c5572f62204d61f62d
2018-03-14 21:39:39 +00:00
Stephen Niedzielski e26c12db52 Hygiene: move thumbnail code to separate file
Move thumbnail code from renderer to a separate file, thumbnail.

Bug: T165036
Change-Id: I6c55750ec302de6341e8e91dee34581ee66499d7
2018-03-14 14:19:41 -05:00
Stephen Niedzielski 4eb9c0efa8 Hygiene: move SVG string to file
- Add SVG Inline Loader for Webpack. This allows SVG files to be
  imported.

- Update the Webpack and test configurations to use the new loader.

- Scope the ESLint rules down to just JavaScript files so that linting
  isn't attempted on the SVG.

Bug: T165036
Change-Id: I00bccff4c3167975c19d577be6343dcaca7ddb2d
2018-03-14 12:04:28 -07:00
Jan Drewniak 1e946a379d Custom page preview for disambiguation pages
Creating a different page preview for disambiguation pages.

This patch:
- modifies the Preview model to accept a new 'type' property
- modifies the Restbase Gateway to pass the 'type' prop to the Preview model
- creates a new template to accept both generic/disambig previews
- modifies the renderer to render the new template
- generates icons for new template through resource loader
- adds new i18n strings
- modifies event-logging "preview seen" event to send new "disambiguation" previewType
- updates event logging schema version
- adds tests for Preview model and renderer for new preview type
- does way too much? yes, yes it does.

Bug: T168392
Change-Id: Idc936cc3eabbdd99a3d98f43c66b4cdbb7d24917
2018-03-14 11:24:26 -07:00
jdlrobson deaaf0961b Remove popups from critical rendering path
Instead load it via mw.loader.using

We retain the module name ext.popups as this will be present
in cached HTML, however now it will load the bulk of the code
inside ext.popups.main

Bug: T176211
Change-Id: Ibe212721807d3698dc45ef46b2dbde15ca9d2f70
2018-03-13 08:44:31 -07:00
jdlrobson f5f21a8d09 RESTBase url is configurable
Allow developers to use different endpoints for summaries
= developer happiness

This is useful for the following use cases:
* A developer wants to test against a production endpoint via
CORS
* A developer has setup an API where REST is hosted elsewhere
e.g. http://localhost:6927/en.wikipedia.org/v1/
* A user wants to create their own REST summary compatible
endpoint
* A wiki e.g. wikidata wants to use a different endpoint which is compatible
with the summary endpoint.

We are unlikely to use it ourselves on Wikimedia wikis (the
default should suffice) but this will be a powerful tool for

When not configured this will continue to work as per normal

Change-Id: I8a7e12fbc43cddbac678e0d7b81d1e877b747b22
2018-03-09 16:15:19 +00:00
jdlrobson ca440dfbed Remove client side formatters in Popups code base for MW API.
Stripping parentheticals were designed specifically for working around
issues with content inside wikimedia wikis and error prone.

This problem for wikimedia wikis is solved by the mobile content
service.

Given we have no intentions to use the MediaWiki API for summaries.
They are not necessarily useful to third parties and it makes little
sense to maintain them (a third party can configure their own API or
use their own REST endpoint if they really do need them).

Bug: T189042
Change-Id: I2729dc9f172af0afee1c6f0cd563c556b4ae0aeb
2018-03-08 21:05:49 +00:00
Željko Filipin 541a42dd17 Update README file for Selenium tests
Bug: T187862
Change-Id: Ib62a6059e8048712617870d07746bee1ab92e371
2018-02-27 14:04:39 +01:00
jdlrobson 535149c16c Upgrade schema and log the required fields
This change updates the schema and begins to log
additional information such as source_namespace, id
and title. This information is provided inside the
boot action.

Additional changes:
* Allow camel case in bundle artifact

Bug: T184793
Bug: T186728
Change-Id: I425ffecc018bef2958d0dfe957a40a065e3e6c56
2018-02-26 10:25:30 -08:00
jdlrobson fe654d7f5e Hygiene: namespaceID => namespaceId
For consistency with pageId and sessionId let's rename
namespaceID to namespaceId

Change-Id: Id0f6f6434691b62d3c5c7ae941970b79e0a99366
2018-02-26 10:23:38 -08:00
Stephen Niedzielski e406d4556f Fix: don't assume thumbnail URLs contain pixel size
Don't assume that thumbnail URLs contain a dimension delimiter of "px-".
Previously, thumbnail URLs always contained the width. e.g.:

  https://upload.wikimedia.org/wikipedia/commons/a/aa/100px-Red_Giant_Earth_warm.jpg

However, thumbnail URLs that actually point to the original are not
sizable:

  https://upload.wikimedia.org/wikipedia/commons/a/aa/Red_Giant_Earth_warm.jpg

These are provided, for example, when the thumbnail size requested is
larger than the original. There was code designed to handle this
scenario but it only applies when RESTBase and page preview thumbnail
sizes happen to be in sync. In other words, if RESTBase requests a large
thumbnail on behalf of page previews, and page previews only requested a
small thumbnail, the original may be unexpectedly provided. A
conditional is introduced in this patch to verify that "px-" is actually
detected. If it is not present, the original is used.

Bug: T187955
Change-Id: If4e29dd870aecd6d461cc8203f6576d1bb8844f2
2018-02-22 12:36:30 -06:00
Sam Smith 35daa2a689 Hygiene: Page view -> Pageview
Pageview is consistent with verbiage used by Research and Analytics
Engineering in their reports and documentation, e.g.
https://wikitech.wikimedia.org/wiki/Analytics/Pageviews.

Bug: T184793
Change-Id: I8ae085b4af85aa72f234f3db27f0cac2c4d014e5
2018-02-21 18:51:49 +00:00
jdlrobson 21f2d4ab0f Model should capture page id
We're going to need this for logging page views.

Change-Id: I1fc46a9fab54f512edaf5feb5abbaa4f025dcb4a
2018-02-19 16:46:13 +00:00
jdlrobson a702c0f499 Capture page view-like interactions
* New action added PREVIEW_SEEN
* The action will be used to signal that a page view needs
to be recorded.
* PREVIEW_SEEN is a delayed action which is triggered
as a side-effect of the previewShow action. It is only dispatched
if the user is still previewing the same card and the page
related to the card has preview type `page`
* The pageview changelistener is added when
$wgPopupsVirtualPageViews is set to true.
* The page view changelistener listens for page views and logs
them using EventLogging when needed using
https://meta.wikimedia.org/wiki/Schema:VirtualPageView

Note:
* Currently if a user has enabled the DNT header, the
event will not be logged. There is ongoing discussion on the
ticket and fixing this will be addressed separately.
* Only title and referrer are logged in the initial version.
The task demands that "namespace" is logged but this information
is not provided by the summary endpoints we use so will need
to be added later (if indeed needed) either via a change to that
endpoint of by using JavaScript to parse the URL.

Bug: T184793
Change-Id: Id1fe34e4bdada3a41f0d888a753af366d4756590
2018-02-16 23:03:33 +00:00
joakin 1fff0d2ea7 Improve & fix action and integration tests
This change fixes some issues with assertions not running, removes
unnecessary promise dances, and improves legibility and some code
patterns in the action and integration node tests mainly.

Detailed changes:

* actions.js
  * Fully migrate out of jQuery 1 promises (no done/fail)
  * Fix linkDwell action not returning the fetch action promise

* actions.test.js
  * Simplify setupWait for the tests
    * It always autoresolves immediately the wait call to ensure speedy tests
    * No waitDeferreds or waitPromises array coordination, rely on action
      returned promises instead
  * Get rid of that = this in favor of arrow functions
  * Rename generic "p" promises to meaningful names
  * Add assert.expect for more solid tests (so that we don't skip assertions in
    the future if we change them)
  * Fix some assertions that weren't being run because of the incorrect promise
    being returned (p.then, and then just returning p)
  * Get rid of $.when stubbing in favor of waiting for the promise returned from
    the action
    * Get rid of hacky setTimeout(..., 0) to run assertions after the promises

* integration.test.js
  * Get rid of wait(0) calls to hack around asynchronous actions
    * Use the action returned promises instead of the waitPromises/Deferreds
  * Remove unused "el" parameter being passed to this.abandon in several tests
  * Remove unnecessary test helper this.abandonPreview (it was the same as
    this.abandon)
  * Clarify a bit the last and more complex test with some comments and variable
    name changes
  * Get rid of that=this in favor of arrow functions

* container.test.js
  * Get rid of that=this in favor of arrow functions

* previewBehavior.test.js
  * Get rid of that=this in favor of arrow functions
  * Get rid of $.each in favor of .forEach

Bug: T170807
Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 13:52:56 +01:00
Jan Drewniak 95b880aa29 Return promises from action thunks
Returning promises from the `linkDwell` and `abandon` thunks and
removing some of the `wait` stubs in the unit test for these actions.

Also converting a fetch callback from a `.fail` to a Promise A+
compatible `.catch`.

Bug: T170807
Change-Id: I4bbf2863db090e222ba926d3bc36a99da4bdb601
2018-02-14 20:08:05 +00:00
Jan Drewniak f4d04b95b6 Updating mw-node-qunit to v3
Fixes tests for sinon v4, which is a dependency of mw-node-qunit v3.

Bug: T180255
Change-Id: I8c7b703f81140e06546aa954f98b8766f5225ff5
2018-02-08 12:20:52 +01:00
joakin 807100bcca Limit line length to 80 characters
Enforce it with eslint.

Ignore:
* Comment lines with eslint disable directives
* QUnit test lines as they contain long subjects (QUnit.* (only, test,
  module, skip, etc)
* Strings, since long strings are used extensively in tests
  * Ignore template literals for similar reasons
* Regex literals as they may be too long, but can't be easily
  split in several lines
* Long urls

See bug for more general proposal for eslint-wikimedia-config.

Bug: T185295
Change-Id: I3aacaf46e61a4d96547c513073e179ef997deb09
2018-01-19 14:20:39 +01:00
joakin c6efe18f6a Restore test subject
Test subject was changed and stopped matching the implementation. In
this particular change the test (a bit convoluted but) tests that wait
is called appropriately, which is why the subject read "should delay
dispatching ..."

Change-Id: I3c8d9d8769f3d1c2869a267af105b9489df86cf5
2018-01-18 18:33:58 +01:00
WMDE-Fisch 4adf63b024 When running Selenium tests, use wd/hub
Now it works on Mac, Windows and Linux.

Change-Id: I6ac70e02bc634bb559426baa29dd22f710e554a1
2018-01-18 15:01:01 +01:00
Stephen Niedzielski 9114981380 Update: show placeholder preview for more failures
Functional changes

- Show the default / error preview for all extract request failures
  except those due to network circumstances (such as CORS) or no
  connectivity (offline). Previously, the error preview was displayed
  only for missing pages.

- FETCH_COMPLETE was previously only dispatched after FETCH_END. Now
  it's also dispatched after FETCH_FAILED. The additional "fetch
  complete" is not expected to impact logging. The states of success
  are: START, END, COMPLETE. The new failure states are consistent with
  success: START, FAILED, COMPLETE.

Testing

Errors may be stimulated in a number of ways including:

- Timeout: add a timeout field to RESTBaseGateway /
  MediaWikiGateway.fetch().

  http://api.jquery.com/jquery.ajax/

- Bad request: change MediaWikiGateway.fetch's action field to
  `Math.random() > 0.5 ? 'query' : 'fail'` and RESTBaseGateway.fetch's
  url field to
  `RESTBASE_ENDPOINT + ( Math.random() > 0.5 ? encodeURIComponent( title ) : '%%%' )`.

- Desired Gateway can be configured in Gateway#createGateway().

- Note: T184534 describes a circumstance where cached previews may not
  appear when offline. Disable browser caching to avoid confusion.

Bug: T183151
Bug: T184534
Change-Id: I7332284da0e0fb1ecd234a6f1e146ebd9ad8d81f
2018-01-16 18:44:00 -06:00
Stephen Niedzielski d9d85ab971 Hygiene: add more fetch failure test cases
Add fetch failure tests for integration and preview state.

Bug: T183151
Change-Id: I1a541d96f3a83b99c26a5180ad5ee8626cda97d2
2018-01-16 18:37:03 -06:00
Stephen Niedzielski 6ba57786fb Update: missing preview copy
Update the placeholder extract and button text shown when a page preview
is unavailable from:

  "popups-preview-no-preview": "Looks like there isn't a preview for this page"
  "popups-preview-footer-read": "Read"

To:

  "popups-preview-no-preview": "There was an issue displaying this preview"
  "popups-preview-footer-read": "Go to this page"

Bug: T183151
Change-Id: I0600dbc2e4d51a13675041d3c0741a793f4eae37
2018-01-16 18:36:54 -06: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 ff49e7627b Better error handling for unexpected responses
Bug: T182639
Change-Id: Iea04fe41b4be8e15927e93f16cbb4bb44328374f
2018-01-02 10:42:12 -08:00
jenkins-bot 76bfe72d97 Merge "Clean up ESLint configuration" 2017-12-28 09:52:02 +00:00
Umherirrender 3515b32cfe Use absolute class name in @covers
Namespaced classes need to be absolutely named.

Fixed one @covers of testEventHandling
Change-Id: I92ae621ac35992c3dd6070528e255fafdaa1af76
2017-12-27 20:53:07 +01:00
Željko Filipin b174a6f298 Clean up ESLint configuration
parserOptions is redundant sice env is already set to es6.

Bug: T177262
Change-Id: If478a0aaeb7fef184be965645fbd2cb276b630c8
2017-12-22 16:12:54 +01:00
Željko Filipin c175cffe9b Fix location of LocalSettings.php
It used to be in tests/browser, now it's in tests/selenium.

Bug: T177262
Change-Id: I102c77834f4c63da06fcc6140a28de9dba231ae6
2017-12-19 13:35:46 +01:00
Baha 58cdac8882 Schema:Popups - use revision 17430287
Event timestamp (as reported by `window.performance.now()`) is also sent
along with every action.

Bug: T180036
Change-Id: Ie3a648298005d51b8b4fbaa6a53e78caf50fa2d3
2017-11-17 12:49:22 -05:00
jdlrobson cf774be88b Port Popups browser tests to selenium
Bug: T177262
Change-Id: Ie77cf7bc79f454904983ed5cf65dc9fe2b1ececc
2017-11-01 13:31:36 -07:00
jdlrobson 730c216534 Drop beta feature step testing in browser tests
Page previews is enabled by default on the beta cluster and
in master.
In production config we enable it for all wikis but enwiki
and dewiki.

We should thus stop testing this to bring our testing in line with
reality.

This allows our browser tests to run against the beta cluster as
well as per commit as well as reduces the amount of code we need to
port to Node.js

Additional changes:
* Remove redundant support pages and properties

Bug: T177262
Change-Id: Ia85e7b60774232ced35641f2671462ca0ef64f18
2017-10-13 17:18:50 +00:00
jenkins-bot b8f61fb1bf Merge "Use https for browser test environment" 2017-10-12 09:27:38 +00:00
jenkins-bot 347852632c Merge "Delete unimplemented browser tests" 2017-10-11 23:22:52 +00:00
jdlrobson 8494ada735 Use https for browser test environment
Change-Id: Ia469152ee4aaeaff491c32d7f8b98a8e6d9dbe02
2017-10-11 15:10:20 -07:00
Ed Sanders 7f4dd1159b build: Update eslint and other linters
Change-Id: I0a20ad5d16d46afbc2a328f8e254295972f58ab0
2017-10-09 15:56:15 +01:00
Ed Sanders 867e75f9a0 Remove old jscs file
Change-Id: I16a4cbffc8dc7232f4bf620f44ff2b7597860517
2017-10-09 15:42:11 +01:00
jdlrobson 11d631a975 Delete unimplemented browser tests
These browser tests were added in I55f311b6b8845e6ebf4cc5698758afd1f9042a45
but broken in Ibc0d07e8394aeb640f9efaabfe10be5317bf4b8c

Let's remove these while porting over to the Node.js version
of these browser tests since they are not run anywhere.

(Try bundle exec cucumber tests/browser/features/ --dry-run)

Bug: T177262
Change-Id: Ica745d084acbb56ec66d88a1785e7557cb12ad6e
2017-10-03 18:17:04 +00:00
smarita 3ec185cb01 Consider using more common image sizes for Page previews
Modified necessary files to increase size of popup from 300px to 320px

Bug: T173434
Change-Id: I47bbe9defe961008163551d5be4fc7b1ca08d0d1
2017-10-01 22:02:29 +05:30
Bartosz Dziewoński 2b637e42ef Improve how we render help messages in preferences
* The 'help' parameter expects a string, not a Message object. It
  seemed to be working, but caused fatals when switching the
  preferences form to the OOUI implementation (T117781).
* Use 'help-message' instead, which also avoids using the global
  request context for Message objects.

Bug: T117781
Change-Id: I5b2e44df35a2696da0e7ba8394eccf41914f6dda
2017-09-12 17:07:37 +00:00
jdlrobson f18c8e4eaf Run settings browser tests and make sure jQuery 3 not enabled
Change-Id: I599c8c002c570d4911b7c86294f714279833e80b
2017-09-01 10:16:17 -04:00