Commit graph

158 commits

Author SHA1 Message Date
joakin 600b21182d Test: Migrate eventLoggingChangeListener.test.js to node-qunit
And on the way there actually put it in a changeListeners/ folder and
rename it to eventLogging.test.js

Change-Id: I60685021841b44f606f39b07bf7f5262344262f4
2017-02-20 20:01:01 +01:00
joakin d2d7ab10fa Test: Migrate changeListener.test.js to node-qunit
Change-Id: Ib6d5156d2f1bc56c113866ed0510966586d9ca07
2017-02-20 20:01:01 +01:00
joakin f3839189dc Test: Migrate isEnabled.test.js to node-qunit
Change-Id: I85a08725c9138f47a51cdc34c20bc3a60b61af34
2017-02-20 20:01:01 +01:00
joakin c233ffb4a5 Test: Migrate preview/model.test.js to node-qunit
Change-Id: Ie5a895a1760dbfc58e11369fdb8b979021b8ae74
2017-02-20 20:01:01 +01:00
joakin b7a4029adb Test: Migrate reducers/eventLogging.test.js to node-qunit
Change-Id: I4bc6d77a496a8c4bf1ecafbf3a6a71986c77e423
2017-02-20 20:01:01 +01:00
joakin d06bbe5871 Test: Migrate reducers/preview.test.js to node-qunit
Change-Id: I700bc43dace64503058337a6b458673070bc5db0
2017-02-20 20:01:01 +01:00
joakin 33c05394f4 Set up qunit running in node to migrate tests to commonjs
In order to run qunit tests on sources that use common.js modules, set
up infra to run qunit tests in the node cli when running:

    npm run test:node

Changes:
* Add npm script test:node that runs the tests
* Run node tests on CI (npm test)
* Add a qunit node test runner: mw-node-qunit
* Migrate a test from the root hierarchy and another one from the nested
  one to prove it works (globs fail otherwise)
  * reducers/settings.test.js to node qunit to prove it works
  * counts.test.js to node qunit to prove it works

Change-Id: I55d76b7db168f3745e0ac69852c152322ab385c3
2017-02-20 20:01:01 +01:00
jdlrobson 6d859a73a3 Resize thumbnails images returned by REST endpoint
This change resizes thumbnails to the appropriate width
based on the value of mw.popups.gateway.THUMBNAIL_SIZE

Tests cover
* When requested thumbnail is < than original size
* When requested thumbnail is > than original size
* When requested thumbnail is an svg and originalimage
smaller than requested thumb size

Bug: T156800
Change-Id: Ib375b97e2bc959e91de5177efc3df1f2ded54a5b
2017-02-16 16:19:14 -08:00
jenkins-bot fa0426e008 Merge "Hygiene: Rename isEnabledByUser to shouldSendModuleToUser" into mpga 2017-02-10 18:57:27 +00:00
jdlrobson 64e7bfd620 Hygiene: Rename isEnabledByUser to shouldSendModuleToUser
Sometimes we make choices on the users behalf if we don't have
sufficient information, so this name is more applicable.

If beta features is disabled and the user is anon they have not
explicitly opted in.

Change-Id: I5d816f569fc54f8bf74d6e5a06246b7fa7036e06
2017-02-10 10:07:28 -08:00
jenkins-bot cd9451e538 Merge "Hygiene: Split and organize the gateways" into mpga 2017-02-08 21:20:18 +00:00
jenkins-bot d7b5f56665 Merge "Hygiene: Use factory functions instead of classes" into mpga 2017-02-08 19:38:53 +00:00
joakin 955189230a Hygiene: Split and organize the gateways
* Create new namespace mw.popups.gateway to contain them
* Create folder resources/ext.popups/gateway/
* Split gateway{,.test}.js into gateway/{mediawiki,rest}{.test,}.js
* Extract stateless functions out of factory scope now that there are no
  name collisions between the factories.

Bug: T123445
Change-Id: Ib256871c3e4cfe3f13361cb66d4e9a67e9823c7b
2017-02-08 18:58:46 +01:00
joakin 557fd113c8 Hygiene: Use factory functions instead of classes
Instead of using constructor functions, use factory functions to
generate the gateway objects, because of:

* Consistency on the approach in the repository, no constructor
  functions are used, the factory function pattern prevales.
* Real private data with closures
* No use of `this`, which is dynamic and unbound in JS and a source of
  errors. In contrast, by using a factory function, the
  function/methods-of-the-object are tightly bound to the internal data.

Additional changes:
* Specify more the type of createRESTBaseGateway's parameter to improve
clarity on the intent of the parameter
  * From: @param {jQuery} api
  * To:   @param {Function} ajax function from jQuery for example

Supporting documentation:
* https://medium.com/javascript-scene/javascript-factory-functions-vs-constructor-functions-vs-classes-2f22ceddf33e

Change-Id: Iacbb098b646843a01f459b15343057e2e4851d35
2017-02-08 18:28:21 +01:00
jenkins-bot bc7ae795c3 Merge "Add support for RESTBase endpoint consumption" into mpga 2017-02-07 18:48:18 +00:00
Baha 9764f7fe64 Add support for RESTBase endpoint consumption
* Introduce the new config variable `PopupsAPIUseRESTBase`.
* Create a gateway interface that exposes the getPageSummary
  method. Both MediaWiki API gateway and RESTBase gateway
  implement this interface.

Bug: T123445
Change-Id: I71a8a848f3143fa4a0dfd4ca182ee086903110bc
2017-02-07 12:36:11 -05:00
Sam Smith 75aaf7a23e Don't log editCountBucket when user is logged out
Bug: T156893
Change-Id: I3cdfbe0c9872407b900f0676396b0bc3102a43fd
2017-02-06 11:58:18 +00:00
jdlrobson eea509637a Remove unused code in repo
The existence of these files was causing me much confusion as I thought
they were still being used in the mpga branch.

If this is note the case they should be removed to make the repo easier
to understand.

Changes:
* Move the ext.popups.animation.less into ext.popups since that's where
its shipped.
* Remove files which are not being used.
* Since csslint now runs on ext.popups.animation.less fix the linting
issues (previously linting was not occuring)

Change-Id: Iae0a78d0b8a13353de70794b67387f2c3bab44c6
2017-02-02 16:18:41 -08:00
Sam Smith ab75283392 model: extract -> page
In I6bbff6a5 a preview was assigned the "extract" type if it had an
extract whereas it should've been assigned the "page" type.

Bug: T152004
Change-Id: I5e0cdc8f528647f1e20873753be29c6b85b3a384
2017-02-02 17:06:46 +00:00
jdlrobson 09ccfa9a13 Remove last modified line
Note this is a breaking change in the sense that it breaks the function
signature for createModel which no longer needs to be passed the last
modified time. Given this change is in the mpga branch at current time
and hasnt been exposed for public use I have deemed this okay.

The change removes tests relating to the last modified bar, removes
lines in the templates and updates existing tests and code to reflect
the new function signature for create model.

Settings icon is floated to right which will be flipped for RTL
users.

Bug: T137775
Change-Id: I7737e37d956c62f1f1c0694d7a25a58d91651f4d
2017-02-02 11:28:11 +00:00
Sam Smith 2d42471ccd reducers: Add preview type to interaction state
The preview model is included in the FETCH_END action so, when reducing
that action, pluck the preview type from the model and add it to the
interaction state.

Change-Id: I0e51a85b4dada8f9a17ec4882e0c13b2c4382d24
2017-01-30 10:05:17 +00:00
Sam Smith 44c6d3d9b9 Add type to preview model
Per T152004, if the preview has an empty extract, then it's a "generic"
preview; it's an "extract" preview otherwise.

Bug: T152004
Change-Id: I6bbff6a507fe3e4f03b3a44f64d875e7dbb512df
2017-01-30 08:29:29 +00:00
Sam Smith 7ce7423639 Make gateway use preview model
Following on from I20f29657, simplify the gateway by making it return
instances ofext.popups.PreviewModel.

Supporting changes:
* data -> model in mw.popups.renderer#render and the
  create{,Empty}Preview helper functions.

Bug: T152004
Change-Id: I531f609056ae813ae5e6b1d53010d77be0799ec1
2017-01-30 08:29:24 +00:00
Sam Smith 622f2af318 gateway: Don't create new object for thumbnail
As noted in the review of I71a8a84, creating a new object to represent
the thumbnail is confusing. It's also somewhat wasteful.

Since both the MediaWiki API and RESTBase both agree on using "source"
as the key for the thumbnail's URL, it's reasonable to use it in the
gateway and renderer.

Bug: T152004
Change-Id: Ie1346d13b6e88004b817f5968448188ed83cfda4
2017-01-27 14:22:54 +00:00
Sam Smith e3ae38f966 Introduce preview model
After the gateway has made a request, it processes the page. This
processing both mangles and processes the MediaWiki API response. The
latter step contains two pieces of related business logic:

1. Deciding if a page has been modified recently.
2. Processing the extract/deciding if it's viable (see processExtract).

In order to support switching between the MediaWiki API and RESTBase and
to simplify the event logging reducer when it comes to instrumenting the
"type" of preview, extract this business logic into a model.

Bug: T152004
Change-Id: I20f29657fcf94101a71ed13c0920508db71292ce
2017-01-27 14:15:37 +00:00
Sam Smith 6cc6252704 Add hovercardsSuppressedByGadget to logging
I4c15296e introduced the wgPopupsConflictsWithNavPopupGadget client-side
config variable, an always-available flag that represents whether the
user has enabled the NavPopups gadget. This property has been missing
from the Popups instrumentation since I8a3f5835.

Changes:
* Add the value of wgPopupsConflictsWithNavPopupGadget to BOOT action
  and reduce it into the base instrumentation data.

Bug: T151058
Change-Id: I09924d0cb10e96b0a846940506ebfeaa7086cd17
2017-01-25 14:21:34 +00:00
Sam Smith 51c6f2c8e5 Hygiene: Make settings reducer control footer link
Since the footer link part of the settings system - clicking the link
opens the settings modal - it seems appropriate that it should be
controlled by state managed by the settings reducer.

Bug: T146889
Change-Id: I3a8549dbf1952cd0556f663496c55de91acaf2c0
2017-01-17 06:40:36 -08:00
Sam Smith c51b33c8b7 Send blob to everyone
As before, we need to send the Page Previews blob that logged out users
can enable/disable previews.

Changes:
* Unconditionally add the ext.popups RL module to the output.
* Send the value of Popups\PopupsContext#isEnabledByUser to the client
  as $wgPopupsIsEnabledByUser.
* Make mw.popups#isEnabled return $wgPopupsIsEnabledByUser if the user
  is logged in.

Bug: T146889
Change-Id: Id2ccfaa81a327e222fff400f4a5f22f96c99ea31
2017-01-17 06:40:24 -08:00
Sam Smith c6a502954d Vary cog click behavior
The behavior of the cog varies when:

* The user is logged in/out.
* Page Previews is enabled as a beta feature.

Since the behavior of the cog doesn't vary per-preview, it can be
determined at boot time and passed to the renderer. However, in order to
keep the renderer stateless, we pass the behavior to it when a preview
is added to the DOM.

Changes:
* Add the mw.popups.createPreviewBehavior factory function, which
  returns an object that encapsulates how a preview responds to the user
  dwelling on it, abandoning it, and clicking on the cog.
* Invoke mw.popups.createPreviewBehavior at boot time and pass it to the
  mw.popups.changeListeners.render change listener, which then passes it
  to mw.popups.Preview#show.
* Make mw.popups.Preview#show responsible for binding event handlers
  and configuring the cog based on the behavior.

Bug: T146889
Change-Id: I39d7d0afd7b1fe896019a1b3a82ee907bfb20edd
2017-01-15 16:20:22 -08:00
jenkins-bot 1b7283a301 Merge "Hygiene: Rename SchemaPopupsSamplingRate" into mpga 2017-01-15 23:38:53 +00:00
Sam Smith 98b2dcf631 Hygiene: Rename SchemaPopupsSamplingRate
The SchemaPopupsSamplingRate config variable is inconsistently named and
even forwarded to the client as wgPopupsSchemaPopupsSamplingRate.

Changes:
* SchemaPopupsSamplingRate -> PopupsSchemaSamplingRate
* Forward PopupsSchemaSamplingRate to the client as
  $wgPopupsSchemaSamplingRate.

Bug: T146889
Bug: T146434
Change-Id: I80d6a0abccf462e2eb0fd96af6849b5e82b49c26
2017-01-15 11:01:00 -08:00
Sam Smith 787660f956 actions: Merge LINK_ABANDON_* + PREVIEW_ABANDON_*
Per Id1339dc2, the LINK_ABANDON_* and PREVIEW_ABANDON_* actions can (and
should) be merged, as they are always reduced in the same way.

Change-Id: I71b30d4d2774deb4efea9e565f2ccd7383bf08c1
2017-01-15 09:55:36 -08:00
Sam Smith 49ce7643f0 Hygiene: Remove link element from LINK_ABANDON_*
Initially, the link element was added to LINK_ABANDON_* to determine
when an interaction should be reset. However, this wasn't enough to
protect against timing-related bugs, e.g. T154923. Now that tokens are
used to determine when interactions should be reset, the link element
can safely be removed from the LINK_ABANON_* actions.

Supporting changes:
* Update the return type of mw.popups.actions.linkAbandon in its
  DocBlock.

Change-Id: Iaaed7a4846af75f9c4051d7bc761022ea5b3f6cf
2017-01-11 14:33:46 -08:00
jenkins-bot a57a0a321d Merge "Fix linkTitleChangeListener" into mpga 2017-01-11 04:27:30 +00:00
Sam Smith e17e209003 Fix linkTitleChangeListener
I2ecf575b introduced a regression where the linkTitleChangeListener
wouldn't destroy the active link's title if it had been dwelled on
within 300 ms of the previous active link being abandoned.

The LINK_ABANDON_END action is dispatched after 300 ms and ignored if
the active link has changed.

Change-Id: If0c16afd6ca1c44f0e7eed497f62f0190005a619
2017-01-10 18:03:47 -08:00
joakin 21230742c0 Change preview to check *_ABANDON_END with tokens
... instead of using the active elements, and check also the user
dwelling flag to only act if the user is not dwelling.

Preview and link abandon end and start share now test cases, meaning it
can be collapsed into one action creator in a followup commit.

Change-Id: Id1339dc23e8f1b1b7514c769cd09f8bdadc759b4
2017-01-10 17:52:53 -08:00
joakin e49103061e Don't reset interaction info when dwelling back to a link
When dwelling back to a link, previously a new LINK_DWELL action would
be sent and the reducer would reset the interaction as if it was a new
one.

This shouldn't happen, since dwelling back to the active link (from
the preview, for example) is not a new interaction, and should not
create a new interaction or hide and show the preview.

With this change, the preview reducer has the notion of new or repeat
interactions, and only resets state on new ones, and only sets
isUserDwelling when dwelling on a the link of the current interaction.

Also when the interaction is repeat, we guard on the action creator and
don't trigger the wait or the fetch request.

Change-Id: I71cde81cbfe50b6f955e562e7e5b57d0f920fdb9
2017-01-10 17:44:37 -08:00
joakin 36518b1215 Don't hide preview when user dwells on link
... after dwelling on preview.

Currently, the asynchronous PREVIEW_ABANDON_END action wouldn't be
ignored if the user were to have dwelled on the link that the preview is
about. The action is only ignored if the user is dwelling on the
preview, i.e. if preview.isUserDwelling is truthy.

Set preview.isUserDwelling to truthy if the user is dwelling either on
the link or on the preview.

Bug: T154923
Change-Id: I8e6989790da5884992ee9dda9e4895e2e58e6694
2017-01-10 17:43:55 -08:00
Sam Smith af05386ea4 actions: Add token to PREVIEW_ABANDON_**
Change-Id: Ic84aa0c3144761b93beff9b7b82932647e504a5b
2017-01-10 16:47:19 -08:00
Sam Smith 489f1e69f2 Run previewAbandon action creator tests
Unfortunately, QUnit doesn't complain that QUnit#module was being called
with a function and not an object.

Change-Id: I6d3b4941566fa4e4ecad2dbe6f7b5fdd62cdaf72
2017-01-10 16:47:11 -08:00
Sam Smith df0f175d72 actions: Conditionally dispatch LINK_DWELL
Change-Id: I9046296f061677674f9ad991ec6dacd1dd9db65c
2017-01-10 10:12:03 -08:00
Sam Smith 4d736b9a78 actions: Add token to LINK_ABANDON_*
Change-Id: I7f6187f6c0249c112a08163a343d35ee75cf85e8
2017-01-10 09:50:58 -08:00
Sam Smith 46cd38849e reducers: Store interaction tokens
Consider the user repeatedly dwelling on and abandoning a link. Testing
whether or not the element has changed before modifying state is not
sufficient. Fortunately, we already generate a unique token and include
it in the LINK_DWELL action for use in the eventLogging reducer - the
so-called "interaction token".

Change-Id: Iae9544055f7eccb4f7f071797eb360bc3456861a
2017-01-10 09:50:58 -08:00
Sam Smith f26768afe8 Hygiene: interactionToken -> token
... for brevity.

Change-Id: I03626728fae36176cec0eb844efb92303377bfbf
2017-01-10 09:50:57 -08:00
jenkins-bot b081b0aa0a Merge "Consistently delay showing previews" into mpga 2017-01-10 17:42:47 +00:00
jenkins-bot b7348f4036 Merge "Fix checkin tests to use fake timers" into mpga 2017-01-10 17:34:35 +00:00
joakin 84043e86f9 Fix checkin tests to use fake timers
* To speed the time the tests spend running
* To make them reliably execute and not depend on magic numbers

Additional changes:
* Use mw.now instead of Date on checkin.js
* Split big setVisibleTimeout test into several smaller ones
  * Extract helpers for the tests

Change-Id: I9d3233fccf6de0997f968d096e375df996e87786
2017-01-10 09:31:32 -08:00
Sam Smith cbdcf7d353 Consistently delay showing previews
While I1bf953b2 fixed the typo in FETCH_START_DELAY, it didn't do
anything to address the inconsistency in the time it takes to render a
preview (preview delay).

If the gateway result resolves before the target preview delay (500 ms)
is met, then delay dispatching the FETCH_END action until it is. If,
however, the gateway result resolves after the target preview delay,
then dispatch the FETCH_END action immediately.

Supporting changes:
* Expose the fetch action to ease testing it and to help tidy up the
  mw.popups.actions.linkDwell tests.

Change-Id: Idf02343a0417cfbb33093a81d97ecebc5c1c7379
2017-01-09 15:00:02 -08:00
joakin 014d442974 Preview actions+reducer qunit integration tests
Initial approach booting reducers and using actions to manipulate the
state.

Change-Id: I0dba4b2186dd2e092df80fe1806dd320bb829914
2017-01-09 11:53:07 -08:00
Sam Smith 60adbd0831 Hygiene: Avoid touching internal state in tests
Test what mw.popups.haveCheckinActionsBeenSetup guards against so that
it doesn't have to be touched during the associated test.

Bug: T147314
Change-Id: I315e0782a0e3b1d04878f1ee6f538e54531f7df5
2017-01-04 11:13:34 +00:00