Phuedx has uploaded a new change for review.
https://gerrit.wikimedia.org/r/325287
Change subject: Increment preview count
......................................................................
Increment preview count
Action changes:
* Add the PREVIEW_SHOW action.
Reducer changes:
* Increment the user's preview count in the eventLogging reducer.
Changes:
* Add the previewCount change listener, which is responsible for
persisting the user's preview count to storage.
* Call the change listener factories individually in
#registerChangeListeners as their signatures aren't consistent.
Bug: T152223
Change-Id: Ifb493c5bff66712a25614ebb905251e43375420a
---
M extension.json
M resources/ext.popups/actions.js
M resources/ext.popups/boot.js
A resources/ext.popups/previewCountChangeListener.js
M resources/ext.popups/reducers.js
M resources/ext.popups/renderChangeListener.js
A tests/qunit/ext.popups/previewCountChangeListener.test.js
M tests/qunit/ext.popups/reducers.eventLogging.test.js
A tests/qunit/ext.popups/renderChangeListener.test.js
9 files changed, 219 insertions(+), 12 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups
refs/changes/87/325287/1
diff --git a/extension.json b/extension.json
index 264ae1b..9408bd2 100644
--- a/extension.json
+++ b/extension.json
@@ -71,6 +71,7 @@
"resources/ext.popups/linkTitleChangeListener.js",
"resources/ext.popups/renderer.js",
"resources/ext.popups/renderChangeListener.js",
+
"resources/ext.popups/previewCountChangeListener.js",
"resources/ext.popups/boot.js"
],
"templates": {
diff --git a/resources/ext.popups/actions.js b/resources/ext.popups/actions.js
index 838f901..bea886a 100644
--- a/resources/ext.popups/actions.js
+++ b/resources/ext.popups/actions.js
@@ -15,6 +15,7 @@
PREVIEW_ABANDON_END: 'PREVIEW_ABANDON_END',
PREVIEW_ANIMATING: 'PREVIEW_ANIMATING',
PREVIEW_INTERACTIVE: 'PREVIEW_INTERACTIVE',
+ PREVIEW_SHOW: 'PREVIEW_SHOW',
PREVIEW_CLICK: 'PREVIEW_CLICK',
COG_CLICK: 'COG_CLICK',
SETTINGS_DIALOG_RENDERED: 'SETTINGS_DIALOG_RENDERED',
@@ -174,7 +175,7 @@
};
/**
- * Represents the user dwelling on a preivew with their mouse.
+ * Represents the user dwelling on a preview with their mouse.
*
* @return {Object}
*/
@@ -206,6 +207,20 @@
};
/**
+ * Represents a preview being shown to the user.
+ *
+ * This action is dispatched by the `mw.popups.changeListeners.render`
change
+ * listener.
+ *
+ * @return {Object}
+ */
+ actions.previewShow = function () {
+ return {
+ type: 'PREVIEW_SHOW'
+ };
+ };
+
+ /**
* Represents the user clicking either the "Enable previews" footer
menu link,
* or the "cog" icon that's present on each preview.
*
diff --git a/resources/ext.popups/boot.js b/resources/ext.popups/boot.js
index 05c077e..6729599 100644
--- a/resources/ext.popups/boot.js
+++ b/resources/ext.popups/boot.js
@@ -1,4 +1,4 @@
-( function ( mw, Redux, ReduxThunk, $ ) {
+( function ( mw, Redux, ReduxThunk ) {
var BLACKLISTED_LINKS = [
'.extiw',
'.image',
@@ -28,13 +28,19 @@
* `mw.popups.changeListeners`.
*
* @param {Redux.Store} store
+ * @param {Object} actions
+ * @param {ext.popups.UserSettings} userSettings
*/
- function registerChangeListeners( store, actions ) {
- $.each( mw.popups.changeListeners, function ( _,
changeListenerFactory ) {
- var changeListener = changeListenerFactory( actions );
+ function registerChangeListeners( store, actions, userSettings ) {
- mw.popups.registerChangeListener( store, changeListener
);
- } );
+ // Sugar.
+ var changeListeners = mw.popups.changeListeners,
+ registerChangeListener =
mw.popups.registerChangeListener;
+
+ registerChangeListener( store, changeListeners.footerLink(
actions ) );
+ registerChangeListener( store, changeListeners.linkTitle() );
+ registerChangeListener( store, changeListeners.render( actions
) );
+ registerChangeListener( store, changeListeners.previewCount(
userSettings ) );
}
/**
@@ -84,7 +90,7 @@
) )
);
actions = createBoundActions( store );
- registerChangeListeners( store, actions );
+ registerChangeListeners( store, actions, userSettings );
actions.boot(
isUserInCondition,
@@ -118,4 +124,4 @@
} );
} );
-}( mediaWiki, Redux, ReduxThunk, jQuery ) );
+}( mediaWiki, Redux, ReduxThunk ) );
diff --git a/resources/ext.popups/previewCountChangeListener.js
b/resources/ext.popups/previewCountChangeListener.js
new file mode 100644
index 0000000..40d7b37
--- /dev/null
+++ b/resources/ext.popups/previewCountChangeListener.js
@@ -0,0 +1,38 @@
+( function ( mw ) {
+
+ /**
+ * Creates an instance of the preview count change listener.
+ *
+ * When the user dwells on a link for long enough that a preview is
shown,
+ * then their preview count will be incremented (see
+ * `mw.popups.reducers.eventLogging`, and is persisted to local storage.
+ *
+ * @return {ext.popups.ChangeListener}
+ */
+ mw.popups.changeListeners.previewCount = function ( userSettings ) {
+
+ /**
+ * FIXME: This kind of function, one that selects the specific
part(s) of
+ * the state tree that a change listener will use, is a common
pattern and
+ * should be extracted.
+ *
+ * @param {Object} state
+ * @return {Number}
+ */
+ function getPreviewCount( state ) {
+ return state.eventLogging.baseData.previewCount;
+ }
+
+ return function ( prevState, state ) {
+ var previewCount = getPreviewCount( state );
+
+ if (
+ prevState &&
+ previewCount > getPreviewCount( prevState )
+ ) {
+ userSettings.setPreviewCount( previewCount );
+ }
+ };
+ };
+
+}( mediaWiki ) );
diff --git a/resources/ext.popups/reducers.js b/resources/ext.popups/reducers.js
index 9a80a77..89ac525 100644
--- a/resources/ext.popups/reducers.js
+++ b/resources/ext.popups/reducers.js
@@ -127,7 +127,8 @@
* @return {Object} The state as a result of processing the action
*/
mw.popups.reducers.eventLogging = function ( state, action ) {
- var nextQueue;
+ var nextQueue,
+ nextCount;
if ( state === undefined ) {
state = {
@@ -153,11 +154,23 @@
popupEnabled:
action.isUserInCondition,
pageToken: action.pageToken,
sessionToken:
action.sessionToken,
- editCountBucket:
counts.getEditCountBucket( action.user.editCount )
+ editCountBucket:
counts.getEditCountBucket( action.user.editCount ),
+ previewCount:
action.user.previewCount,
+ previewCountBucket:
counts.getPreviewCountBucket( action.user.previewCount )
},
queue: nextQueue
} );
+ case mw.popups.actionTypes.PREVIEW_SHOW:
+ nextCount = state.baseData.previewCount + 1;
+
+ return nextState( state, {
+ baseData: nextState( state.baseData, {
+ previewCount: nextCount,
+ previewCountBucket:
counts.getPreviewCountBucket( nextCount )
+ } )
+ } );
+
default:
return state;
}
diff --git a/resources/ext.popups/renderChangeListener.js
b/resources/ext.popups/renderChangeListener.js
index 99fd821..99c9ccb 100644
--- a/resources/ext.popups/renderChangeListener.js
+++ b/resources/ext.popups/renderChangeListener.js
@@ -12,7 +12,8 @@
return function ( prevState, state ) {
if ( state.preview.shouldShow && !preview ) {
preview = mw.popups.renderer.render(
state.preview.fetchResponse );
- preview.show( state.preview.activeEvent,
boundActions );
+ preview.show( state.preview.activeEvent,
boundActions )
+ .done( boundActions.previewShow );
} else if ( !state.preview.shouldShow && preview ) {
preview.hide()
.done( function () {
diff --git a/tests/qunit/ext.popups/previewCountChangeListener.test.js
b/tests/qunit/ext.popups/previewCountChangeListener.test.js
new file mode 100644
index 0000000..7c5ce09
--- /dev/null
+++ b/tests/qunit/ext.popups/previewCountChangeListener.test.js
@@ -0,0 +1,63 @@
+( function ( mw, $ ) {
+
+ QUnit.module( 'ext.popups/previewCountChangeListener', {
+ setup: function () {
+ this.userSettings = {
+ setPreviewCount: this.sandbox.spy()
+ };
+
+ this.changeListener =
mw.popups.changeListeners.previewCount( this.userSettings );
+ }
+ } );
+
+ QUnit.test(
+ 'it shouldn\'t update the storage if the preview count hasn\'t
changed',
+ function ( assert ) {
+ var state,
+ prevState;
+
+ assert.expect( 1 );
+
+ state = {
+ eventLogging: {
+ baseData: {
+ previewCount: 222
+ }
+ }
+ };
+
+ this.changeListener( undefined, state );
+
+ // ---
+
+ prevState = $.extend( true, {}, state );
+
+ this.changeListener( prevState, state );
+
+ assert.notOk( this.userSettings.setPreviewCount.called
);
+ }
+ );
+
+ QUnit.test( 'it should update the storage', function ( assert ) {
+ var prevState,
+ state;
+
+ assert.expect( 1 );
+
+ prevState = {
+ eventLogging: {
+ baseData: {
+ previewCount: 222
+ }
+ }
+ };
+
+ state = $.extend( true, {}, prevState );
+ ++state.eventLogging.baseData.previewCount;
+
+ this.changeListener( prevState, state );
+
+ assert.ok( this.userSettings.setPreviewCount.calledWith( 223 )
);
+ } );
+
+}( mediaWiki, jQuery ) );
diff --git a/tests/qunit/ext.popups/reducers.eventLogging.test.js
b/tests/qunit/ext.popups/reducers.eventLogging.test.js
index 28b7047..f758c18 100644
--- a/tests/qunit/ext.popups/reducers.eventLogging.test.js
+++ b/tests/qunit/ext.popups/reducers.eventLogging.test.js
@@ -57,4 +57,35 @@
);
} );
+ QUnit.test( 'PREVIEW_SHOW', function ( assert ) {
+ var state,
+ count = 22,
+ action,
+ expectedCount = count + 1;
+
+ state = {
+ baseData: {
+ previewCount: count,
+ previewCountBucket:
counts.getPreviewCountBucket( count )
+ },
+ queue: []
+ };
+
+ action = {
+ type: 'PREVIEW_SHOW'
+ };
+
+ assert.deepEqual(
+ mw.popups.reducers.eventLogging( state, action ),
+ {
+ baseData: {
+ previewCount: expectedCount,
+ previewCountBucket:
counts.getPreviewCountBucket( expectedCount )
+ },
+ queue: []
+ },
+ 'It increments the user\'s preview count and re-buckets
that count.'
+ );
+ } );
+
}( mediaWiki ) );
diff --git a/tests/qunit/ext.popups/renderChangeListener.test.js
b/tests/qunit/ext.popups/renderChangeListener.test.js
new file mode 100644
index 0000000..7b65dc7
--- /dev/null
+++ b/tests/qunit/ext.popups/renderChangeListener.test.js
@@ -0,0 +1,39 @@
+( function ( mw, $ ) {
+
+ // Since mw.popups.changeListeners.render manipulates the DOM, this
test is,
+ // by necessity, an integration test.
+ QUnit.module( 'ext.popups/renderChangeListener @integration' );
+
+ QUnit.test(
+ 'it should call the showPreview action creator when the preview
is shown',
+ function ( assert ) {
+ var boundActions,
+ changeListener,
+ state;
+
+ boundActions = {
+ previewShow: this.sandbox.spy()
+ };
+
+ this.sandbox.stub( mw.popups.renderer, 'render',
function () {
+ return {
+ show: function () {
+ return $.Deferred().resolve();
+ }
+ };
+ } );
+
+ state = {
+ preview: {
+ shouldShow: true
+ }
+ };
+
+ changeListener = mw.popups.changeListeners.render(
boundActions );
+ changeListener( undefined, state );
+
+ assert.ok( boundActions.previewShow.called );
+ }
+ );
+
+}( mediaWiki, jQuery ) );
--
To view, visit https://gerrit.wikimedia.org/r/325287
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb493c5bff66712a25614ebb905251e43375420a
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Phuedx <samsm...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
