jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/325287 )
Change subject: Store/update user's preview count
......................................................................
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
---
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
M tests/qunit/ext.popups/reducers.test.js
A tests/qunit/ext.popups/renderChangeListener.test.js
10 files changed, 204 insertions(+), 4 deletions(-)
Approvals:
Jhernandez: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 602d487..3dfaf95 100644
--- a/extension.json
+++ b/extension.json
@@ -73,6 +73,7 @@
"resources/ext.popups/renderChangeListener.js",
"resources/ext.popups/schema.js",
"resources/ext.popups/eventLoggingChangeListener.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 0cf7d7b..2afa447 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',
@@ -175,7 +176,7 @@
};
/**
- * Represents the user dwelling on a preivew with their mouse.
+ * Represents the user dwelling on a preview with their mouse.
*
* @return {Object}
*/
@@ -207,6 +208,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: types.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 0cf655f..895b463 100644
--- a/resources/ext.popups/boot.js
+++ b/resources/ext.popups/boot.js
@@ -30,8 +30,9 @@
* @param {Redux.Store} store
* @param {Object} actions
* @param {mw.eventLog.Schema} schema
+ * @param {ext.popups.UserSettings} userSettings
*/
- function registerChangeListeners( store, actions, schema ) {
+ function registerChangeListeners( store, actions, schema, userSettings
) {
// Sugar.
var changeListeners = mw.popups.changeListeners,
@@ -41,6 +42,7 @@
registerChangeListener( store, changeListeners.linkTitle() );
registerChangeListener( store, changeListeners.render( actions
) );
registerChangeListener( store, changeListeners.eventLogging(
actions, schema ) );
+ registerChangeListener( store, changeListeners.previewCount(
userSettings ) );
}
/**
@@ -102,7 +104,7 @@
) )
);
actions = createBoundActions( store );
- registerChangeListeners( store, actions, schema );
+ registerChangeListeners( store, actions, schema, userSettings );
actions.boot(
isUserInCondition,
diff --git a/resources/ext.popups/previewCountChangeListener.js
b/resources/ext.popups/previewCountChangeListener.js
new file mode 100644
index 0000000..56849d9
--- /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.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 6fe340d..7fae9c1 100644
--- a/resources/ext.popups/reducers.js
+++ b/resources/ext.popups/reducers.js
@@ -145,8 +145,11 @@
* @return {Object} The state as a result of processing the action
*/
mw.popups.reducers.eventLogging = function ( state, action ) {
+ var nextCount;
+
if ( state === undefined ) {
state = {
+ previewCount: undefined,
baseData: {},
event: undefined
};
@@ -177,6 +180,16 @@
event: undefined
} );
+ case mw.popups.actionTypes.PREVIEW_SHOW:
+ nextCount = state.previewCount + 1;
+
+ return nextState( state, {
+ previewCount: nextCount,
+ baseData: nextState( state.baseData, {
+ 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..4972327
--- /dev/null
+++ b/tests/qunit/ext.popups/previewCountChangeListener.test.js
@@ -0,0 +1,59 @@
+( 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: {
+ 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: {
+ previewCount: 222
+ }
+ };
+
+ state = $.extend( true, {}, prevState );
+ ++state.eventLogging.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 52f37bb..6e2c176 100644
--- a/tests/qunit/ext.popups/reducers.eventLogging.test.js
+++ b/tests/qunit/ext.popups/reducers.eventLogging.test.js
@@ -76,4 +76,35 @@
);
} );
+ QUnit.test( 'PREVIEW_SHOW', function ( assert ) {
+ var state,
+ count = 22,
+ action,
+ expectedCount = count + 1;
+
+ state = {
+ previewCount: count,
+ baseData: {
+ previewCountBucket:
counts.getPreviewCountBucket( count )
+ },
+ event: undefined
+ };
+
+ action = {
+ type: 'PREVIEW_SHOW'
+ };
+
+ assert.deepEqual(
+ mw.popups.reducers.eventLogging( state, action ),
+ {
+ previewCount: expectedCount,
+ baseData: {
+ previewCountBucket:
counts.getPreviewCountBucket( expectedCount )
+ },
+ event: undefined
+ },
+ 'It increments the user\'s preview count and re-buckets
that count.'
+ );
+ } );
+
}( mediaWiki ) );
diff --git a/tests/qunit/ext.popups/reducers.test.js
b/tests/qunit/ext.popups/reducers.test.js
index 45ecd45..9751c2a 100644
--- a/tests/qunit/ext.popups/reducers.test.js
+++ b/tests/qunit/ext.popups/reducers.test.js
@@ -22,6 +22,7 @@
isUserDwelling: false
},
eventLogging: {
+ previewCount: undefined,
baseData: {},
event: undefined
},
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: merged
Gerrit-Change-Id: Ifb493c5bff66712a25614ebb905251e43375420a
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Phuedx <samsm...@wikimedia.org>
Gerrit-Reviewer: Jhernandez <jhernan...@wikimedia.org>
Gerrit-Reviewer: Phuedx <samsm...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
