Commit graph

85 commits

Author SHA1 Message Date
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
Baha 4afa1958a0 Add reading depth
Use schema revision 16163887.

Add the 'checkin' action, which is accompanied by the 'checkin'
property. The action is logged at the following seconds
(Fibonacci numbers) after the page loads: 1, 2, 3, 5, 8, 13, 21,
34, 55, 89, 144, 233, 377, 610, 987, 1597, 2584, 4181, 6765.
The `checkin` property contains the values listed above.

Bug: T147314
Change-Id: Ib9ec7bd0e60aa34a04e32222b025347f6ee31794
2016-12-21 15:08:20 -05:00
jenkins-bot 397d3e8bc2 Merge "Hygiene: Make mw.popups#isEnabled return boolean" into mpga 2016-12-16 19:48:10 +00:00
jenkins-bot 7e64a87d7f Merge "Hygiene: Remove experiment-related code" into mpga 2016-12-16 19:48:09 +00:00
jenkins-bot 4fe1cd9f69 Merge "Redefine when Page Previews are enabled" into mpga 2016-12-16 19:48:08 +00:00
Sam Smith b2bd1ebdf7 Hygiene: Make mw.popups#isEnabled return boolean
Action changes:
* Update the BOOT action to include isEnabled and update the associated
  tests.

Reducer changes:
* Make the preview and eventLogging reducers handle the BOOT action's
  new shape.

Bug: T152687
Change-Id: I3fa2098269a32912eda99ceb8f13887688a14c15
2016-12-16 10:59:47 +00:00
Sam Smith 6bd082920f Hygiene: Remove experiment-related code
Changes:
* Remove $wgPopupsExperiment and $wgPopupsExperimentConfig config
  variables.
* Remove $wgPopupsExperimentIsBetaFeatureEnabled client-side config
  variable.
* Remove MakeGlobalVariablesScript hook handler as it's redundant.

Bug: T152687
Change-Id: Id25cd7c16da14b7ce9bad502565de9ff6d29434e
2016-12-16 10:59:47 +00:00
Sam Smith a6423fbc75 Redefine when Page Previews are enabled
The Page Previews A/B test was disabled in T144490. After the changes
for that task had been deployed we discovered that the old definition
for when previews are enabled was invalid.

Previews are enabled:
* If the beta feature is enabled and the user has enabled the beta
  feature.
* The user hasn't disabled previews via the settings modal.
* And soon, if the beta feature is disabled and the user hasn't disabled
  previews via their user preferences (see T146889).

Changes:
* mw.popups#createExperiment -> mw.popups#isEnabled
* Make mw.popups#isEnabled act like a factory to maintain backwards
  compatibility.
* Update the associated tests.

Bug: T152687
Change-Id: I713a90dfb29866b27738e7d19e8a22518a12d417
2016-12-16 10:35:55 +00:00
joakin 212dcf1d55 Update the settings enabled radio with the enabled state
When opening the settings dialog, the form didn't represent the current
enabled status of the application. This change makes sure that every
time the form is shown, the form state represents the current
application state.

If the application is enabled, then the 'simple' (Enabled) option is
selected on the form. If it is disabled then if there's Navigation
popups, it selects 'advanced' (Advanced popups), if there are not, it
selects 'off' (Disabled).

Changes:
* Change listeners
  * Set the form state every time the form is going to be shown
    * Update tests
* Supporting changes
  * Add settingsDialog#setEnabled which updates the DOM form based on the
    enabled flag and the navpops state
  * Extract isNavPopupsEnabled function in settingsDialog.js to be used in the
    form creation and also when setting the form enabled state
  * Add test verifying changeListeners#settings shows and sets enabled state
    properly when showing the dialog more than one time

Change-Id: Ic660f48d9a78e47c09a192ab86681196d2e01d61
2016-12-16 10:29:34 +00:00
joakin 8631a183de Sync enabled preview state to storage on change
Changes:
* Generalized previewCount change listener to syncUserSettings
* Added state.preview.enabled to be saved on change

Change-Id: I403a490fee9c8e125175996ba30c63c232b5598b
2016-12-16 10:27:24 +00:00
joakin efb7b3d872 Wire up saving enabled/disabled in settings dialog
The save action has been implemented, and it is listened to by the
canonical enabled state in the previews reducer, and by the settings
reducer to perform UI changes.

The enabled state of the application has been kept in the preview
reducer as the canonical source of truth. See supporting changes for
documentation about the decision.

Actions:
* Introduce new action SETTINGS_CHANGE with the enabled status
* Trigger that action when clicking Save in the settings dialog

Reducers:
* Listen to SETTINGS_CHANGE in the preview reducer to update enabled
  status
* On the settings reducer
  * Handle the SETTINGS_CHANGE action
  * Add showHelp flag to determine if the help should be showing

Change listeners:
* Switch to compare past vs present changes in the implementation
* Handle showing and hiding the help

Supporting changes:
* On the rendered settings dialog:
  * Change #hide to actually just hide and remove legacy if statement
  * Add #toggleHelp method to show or hide the help dialog
* Add doc/adr/0003-keep-enabled-state-only-in-preview-reducer.md to
  support the decision of making the saveSettings action creator return
  a Redux.Thunk and keeping the enabled state just in the preview
  reducer.
  * Add NB to actions#saveSettings explaining and linking to the
    document

Follow commits soon:
* Persist the settings change to local storage when it changes, and
  unify with the preview change listener

Change-Id: I80dc5f29fbe6286f2e3e3b50d909894bc5041ccd
2016-12-14 14:35:59 +00:00
Sam Smith fcfe079d79 Hygiene: Organize reducers
Reducer changes:
* Break the mw.popups.reducers.rootReducer test into @@INIT tests
  for each reducer as there's shouldn't be any need to test that a
  framework works as documented.

Changes:
* Move the mw.popups.reducers#preview, #eventLogging, and #settings
  reducers into their own files in resources/ext.popups/reducers/.
* Make the associated tests mirror the new layout.
* Move the initialization of the mw.popups.reducers namespace into its
  own file.
* Remove the mw.popups.rootReducer property in favour of a simple
  private factory function per the reducer change above.

Change-Id: I94229d9ef1985f3806eec44c2e8234a5bbddc94f
2016-12-13 18:45:10 +00:00
jenkins-bot 492c170aae Merge "Introduce the settings dialog UI" into mpga 2016-12-13 12:54:37 +00:00
jenkins-bot f7746bb860 Merge "Introduce the settings change listener" into mpga 2016-12-13 11:26:48 +00:00
jenkins-bot cacd6ee04c Merge "Create the settings reducer" into mpga 2016-12-13 11:03:47 +00:00
joakin 4d5269320f Introduce the settings dialog UI
* Port ext.popups.desktop/ext.popups.settings.js to settingsDialog.js
  * Blank tests for now. Needs Qunit integration tests.
    * Transform into a factory function for future testing.
  * Saving functionality is commented out, will be removed when immplemented in
    the actions
* Add new incomplete action saveSettings
  * Will perform the saveSettings async tasks and then trigger enabling or
    disabling popups.
* Rename action settingsDialogClosed to hideSettings for consistency

Change-Id: I3936d3a4bc476de16d76025139be09f1798796c4
2016-12-13 11:42:21 +01:00
joakin 034f1840ab Introduce the settings change listener
Which for the moment is only in charge of showing and hiding the dialog

Change-Id: Ib57250236ff424abddb0fa627d2a48167a5d8d74
2016-12-13 11:32:53 +01:00
joakin 8415bd1e9b Create the settings reducer
Right now only with visibility state on it.

Change-Id: Idbe99aca652eb04357ba85f22ba413dcd38cd54b
2016-12-13 10:46:13 +01:00
Sam Smith df441982fa {LINK,PREVIEW}_ABANDON_END logs an event
Since the user can dwell on a link, abandon it while moving to dwell on
the preview and vice versa:

* The time at which the link/preview was abandoned is recorded and can
  be updated.
* The event can only be queued when the link/preview has definitely been
  abandoned.

Reducer changes:
* Make the eventLogging reducer:
  * Calculate the time it took to show the preview.
  * Mark the interaction as finished when the user abandons the link or
    the preview.
  * Queue a "dismissed" event if the preview hasn't been shown or a
    "dwelledButAbandoned" event if it has when the abandon has
    finalized.

Bug: T152225
Change-Id: I6a254136f615484fc26e440fe5125289e74688a6
2016-12-12 19:00:33 +00:00
Sam Smith 60b65ad6c4 LINK_CLICK logs an "opened" event
Reducer changes:
* Make the eventLogging reducer queue an "opened" event with the
  requisite supporting data.

Changes:
* Update the Popups EventLogging schema to the latest revision, which
  defines the "opened" action.

Bug: T152225
Change-Id: I9d67daf83815f1c08b6479497c566dfa337de4bc
2016-12-12 18:59:57 +00:00
Sam Smith 5b76fb0276 LINK_DWELL starts an interaction
Action changes:
* Mix in timing information into actions that are likely to need it:
  LINK_DWELL, {LINK,PREVIEW}_ABANDON_START, LINK_CLICK, and
  PREVIEW_CLICK.
* Generate and include an unique token in the LINK_CLICK action.

Reducer changes:
* Make the eventLogging reducer:
  * Handle the LINK_CLICK action's new shape.
  * Start (create) and maintain a model of the user interacting with a
    link.

Bug: T152225
Change-Id: I671b12432ba2f7a93bf81043adb57ac30a4c38c3
2016-12-12 18:59:40 +00:00
Sam Smith 71b97cd089 Hygiene: Organise change listeners
Move all of the change listeners into
resources/ext.popups/changeListeners and remove the ChangeListener
suffix. Also, do the above for the associated tests.

Bug: T152225
Change-Id: I90ada465ea291d601f8f1c5c6e775148a2100319
2016-12-12 13:06:07 +00:00
Sam Smith bbeb618e1d Store/update user's preview count
Action changes:
* Add the PREVIEW_SHOW action.

Reducer changes:
* Increment the user's preview count in the eventLogging reducer.

Changes:
* Dispatch the PREVIEW_SHOW action when the preview has been shown.
* Add the previewCount change listener, which is responsible for
  persisting the user's preview count to storage when it changes.
* Call the change listener factories individually in
  #registerChangeListeners as their signatures aren't consistent.

Bug: T152225
Change-Id: Ifb493c5bff66712a25614ebb905251e43375420a
2016-12-12 13:05:50 +00:00
Sam Smith b7effdc000 Include user's preview count in BOOT action
Action changes:
* Include the user's preview count in the user property of the action.

Reducer changes:
* Make the eventLogging reducer add the bucketed user's preview count to
  the state tree.

Changes:
* Extract mw.popups.UserSettings#getPreviewCount and #setPreviewCount
  and associated tests from the ext.popups.core module.

Bug: T152225
Change-Id: I6b7afef31311be8fede685deb536f577845cb9cf
2016-12-12 13:01:44 +00:00