Remove page issues instrumentation

Note, since the page issues code is not feature flagged,
the new treatment will only be accessible via query
string until T206179 is taken care of.

Bug: T206178
Change-Id: I5ab2f3396e642f7b973263e2bb3963e0e82721b3
This commit is contained in:
jdlrobson 2018-10-10 14:49:25 -07:00
parent cb2ad9374e
commit 72cd31f221
8 changed files with 10 additions and 468 deletions

View File

@ -91,7 +91,6 @@ class MinervaHooks {
// additional scaffolding (minus initialisation scripts)
'tests/qunit/skins.minerva.scripts/stubs.js',
'resources/skins.minerva.scripts/pageIssuesLogger.js',
'resources/skins.minerva.scripts/pageIssuesParser.js',
'resources/skins.minerva.scripts/DownloadIcon.js',
'resources/skins.minerva.scripts/AB.js',
@ -101,7 +100,6 @@ class MinervaHooks {
'tests/qunit/skins.minerva.scripts/DownloadIcon.test.js',
'tests/qunit/skins.minerva.scripts/pageIssuesParser.test.js',
'tests/qunit/skins.minerva.scripts/AB.test.js',
'tests/qunit/skins.minerva.scripts/PageIssuesOverlay.test.js',
'tests/qunit/skins.minerva.scripts/pageIssues.test.js',
'tests/qunit/skins.minerva.notifications.badge/NotificationBadge.test.js'
],

View File

@ -2,7 +2,6 @@
var
router = require( 'mediawiki.router' ),
issues = M.require( 'skins.minerva.scripts/pageIssues' ),
overlayManager = M.require( 'skins.minerva.scripts/overlayManager' ),
loader = M.require( 'mobile.startup/rlModuleLoader' ),
skin = M.require( 'skins.minerva.scripts/skin' ),
@ -32,7 +31,6 @@
*/
function onEditLinkClick() {
var section = ( new mw.Uri( this.href ) ).query.section || 'all';
issues.log( { action: 'editClicked' } );
router.navigate( '#/editor/' + section );
// prevent folding section when clicking Edit by stopping propagation
return false;

View File

@ -13,13 +13,11 @@
* @extends Overlay
*
* @param {IssueSummary[]} issues list of page issue summaries for display.
* @param {PageIssuesLogger} logger E.g., { log: console.log }.
* @param {string} section
* @param {number} namespaceID
*/
function PageIssuesOverlay( issues, logger, section, namespaceID ) {
function PageIssuesOverlay( issues, section, namespaceID ) {
var
options,
// Note only the main namespace is expected to make use of section issues, so the
// heading will always be minerva-meta-data-issues-section-header regardless of
// namespace.
@ -27,24 +25,10 @@
getNamespaceHeadingText( namespaceID ) :
mwMsg( 'minerva-meta-data-issues-section-header' );
this.issues = issues;
this.logger = logger;
this.section = section;
options = {};
options.issues = issues;
// Set default logging data
this.defaultLoggerData = {};
// In the case of KEYWORD_ALL_SECTIONS all issues are in the overlay and the sectionNumbers
// field should be no different from the default behaviour.
if ( this.section !== KEYWORD_ALL_SECTIONS ) {
this.defaultLoggerData.sectionNumbers = [ this.section ];
}
options.heading = '<strong>' + headingText + '</strong>';
Overlay.call( this, options );
this.on( Overlay.EVENT_EXIT, this.onExit.bind( this ) );
Overlay.call( this, {
issues: issues,
heading: '<strong>' + headingText + '</strong>'
} );
}
OO.mfExtend( PageIssuesOverlay, Overlay, {
@ -54,128 +38,15 @@
*/
className: 'overlay overlay-issues',
/**
* @memberof PageIssuesOverlay
* @instance
*/
events: util.extend( {}, Overlay.prototype.events, {
'click a[href*=redlink]': 'onRedLinkClick',
'click a:not(.external):not([href*=edit])': 'onInternalClick',
// Only register attempts to edit an existing page (should be the one we are on),
// not internal clicks on redlinks to nonexistent pages:
'click a[href*="edit"]:not([href*=redlink])': 'onEditClick'
} ),
/**
* @memberof PageIssuesOverlay
* @instance
*/
templatePartials: util.extend( {}, Overlay.prototype.templatePartials, {
content: mw.template.get( 'skins.minerva.scripts', 'PageIssuesOverlayContent.hogan' )
} ),
/**
* Log data via the associated logger, adding sectionNumbers to override the event default
* if applicable.
* @param {Object} data
* @instance
*/
log: function ( data ) {
this.logger.log( util.extend( {}, this.defaultLoggerData, data ) );
},
/**
* Note: an "on enter" state is tracked by the issueClicked log event.
* @return {void}
*/
onExit: function () {
var logData = {
action: 'modalClose',
issuesSeverity: this.issues.map( issueSummaryToSeverity )
},
currentSection = this.section;
// When users close the modal, `sectionNumbers` should correlate to each visible issue
// in the modal, provided that this.section is a valid number and not
// `KEYWORD_ALL_SECTIONS`.
if ( this.section !== KEYWORD_ALL_SECTIONS ) {
logData.sectionNumbers = this.issues.map( function () {
return currentSection;
} );
}
this.log( logData );
},
/**
* Event that is triggered when an internal link inside the overlay is clicked.
* This event will not be triggered if the link contains the edit keyword,
* in which case onEditClick will be
* fired. This is primarily used for instrumenting page issues (see
* https://meta.wikimedia.org/wiki/Schema:PageIssues).
* @param {JQuery.Event} ev
* @memberof PageIssuesOverlay
* @instance
*/
onInternalClick: function ( ev ) {
var severity = parseSeverity( this.$( ev.target ) );
this.log( {
action: 'modalInternalClicked',
issuesSeverity: [ severity ]
} );
},
/**
* Event that is triggered when a red link (e.g. a link to a page which doesn't exist)
* inside the overlay is clicked.
* @param {JQuery.Event} ev
* @memberof PageIssuesOverlay
* @instance
*/
onRedLinkClick: function ( ev ) {
var severity = parseSeverity( this.$( ev.target ) );
this.log( {
action: 'modalRedLinkClicked',
issuesSeverity: [ severity ]
} );
},
/**
* Event that is triggered when an edit link inside the overlay is clicked.
* This is primarily
* used for instrumenting page issues (see https://meta.wikimedia.org/wiki/Schema:PageIssues).
* The event will not be triggered in the case of red links.
* See onRedLinkClick for red links.
* @param {JQuery.Event} ev
* @memberof PageIssuesOverlay
* @instance
*/
onEditClick: function ( ev ) {
var severity = parseSeverity( this.$( ev.target ) );
this.log( {
action: 'modalEditClicked',
issuesSeverity: [ severity ]
} );
}
} )
} );
/**
* Obtain severity associated with a given $target node by looking at associated parent node
* (defined by templatePartials, PageIssuesOverlayContent.hogan).
*
* @param {JQuery.Object} $target
* @return {string[]} severity as defined in associated PageIssue
*/
function parseSeverity( $target ) {
return $target.parents( '.issue-notice' ).data( 'severity' );
}
/**
* @param {IssueSummary} issue
* @return {string} A PageIssue.severity.
*/
function issueSummaryToSeverity( issue ) {
return issue.severity;
}
/**
* Obtain a suitable heading for the issues overlay based on the namespace
* @param {number} namespaceID is the namespace to generate heading for

View File

@ -1,63 +1,17 @@
( function ( M, $ ) {
var AB = M.require( 'skins.minerva.scripts/AB' ),
Page = M.require( 'mobile.startup/Page' ),
var Page = M.require( 'mobile.startup/Page' ),
allIssues = {},
KEYWORD_ALL_SECTIONS = 'all',
config = mw.config,
user = mw.user,
NS_MAIN = 0,
NS_TALK = 1,
NS_CATEGORY = 14,
CURRENT_NS = config.get( 'wgNamespaceNumber' ),
pageIssuesLogger = M.require( 'skins.minerva.scripts/pageIssuesLogger' ),
pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ),
PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' ),
// setup ab test
abTest = new AB( {
testName: 'WME.PageIssuesAB',
// Run AB only on article namespace, otherwise set samplingRate to 0,
// forcing user into control (i.e. ignored/not logged) group.
samplingRate: ( CURRENT_NS === NS_MAIN ) ? config.get( 'wgMinervaABSamplingRate', 0 ) : 0,
sessionId: user.sessionId()
} ),
QUERY_STRING_FLAG = mw.util.getParamValue( 'minerva-issues' ),
// Per T204746 a user can request the new treatment regardless of test group
isUserRequestingNewTreatment = QUERY_STRING_FLAG === 'b',
newTreatmentEnabled = abTest.isB() || isUserRequestingNewTreatment;
function isLoggingRequired( pageIssues ) {
// No logging necessary when the A/B test is disabled (control group).
return !isUserRequestingNewTreatment && abTest.isEnabled() && pageIssues.length;
}
/**
* Array.reduce callback that returns the severity of page issues.
* In the case that a page-issue is part of a "multiple issues" template,
* returns the maximum severity for that group of issues.
*
* @param {array} formattedArr - the return array containing severities
* @param {IssueSummary} currentItem current IssueSummary object
* @param {number} currentIndex current index of pageIssues
* @param {array} pageIssues array of pageIssues
*
* @return {array} acc
*/
function formatPageIssuesSeverity( formattedArr, currentItem, currentIndex, pageIssues ) {
var lastItem = pageIssues[ currentIndex - 1 ],
issue = currentItem.issue,
lastFormattedIndex = formattedArr.length - 1,
lastFormattedValue = formattedArr[ lastFormattedIndex ];
// If the last and current item `grouped`, fold the maxSeverity
// of the two items into a single value.
if ( lastItem && lastItem.issue && lastItem.issue.grouped && issue.grouped ) {
formattedArr[ lastFormattedIndex ] = pageIssuesParser.maxSeverity(
[ lastFormattedValue, issue.severity ]
);
} else {
formattedArr.push( issue.severity );
}
return formattedArr;
}
// T206179 should update this value to enable it
newTreatmentEnabled = QUERY_STRING_FLAG === 'b';
/**
* Create a link element that opens the issues overlay.
@ -138,34 +92,12 @@
$learnMore.appendTo( $metadata.find( '.mbox-text' ) );
}
$metadata.click( function () {
var pageIssue = pageIssuesParser.parse( this );
pageIssuesLogger.log( {
action: 'issueClicked',
issuesSeverity: [ pageIssue.severity ],
sectionNumbers: [ section ]
} );
overlayManager.router.navigate( issueUrl );
return false;
} );
} else {
$link = createLinkElement( labelText );
$link.attr( 'href', '#/issues/' + section );
$link.click( function () {
pageIssuesLogger.log( {
action: 'issueClicked',
issuesSeverity: [
pageIssuesParser.maxSeverity(
getIssues( '0' )
.map( function ( issue ) { return issue.severity; } )
)
],
// In the old treatment, an issuesClicked event will always be '0'
// as the old treatment is always associated with the lead section and we
// are only sending one maximum severity for all of them.
// An issuesClicked event should only ever be associated with one issue box.
sectionNumbers: [ '0' ]
} );
} );
if ( $metadata.length ) {
$link.insertAfter( $( 'h1#section_0' ) );
$metadata.remove();
@ -269,39 +201,16 @@
}
}
if ( isLoggingRequired( getIssues( KEYWORD_ALL_SECTIONS ) ) ) {
// Enable logging of the PageIssues schema, setting up defaults.
pageIssuesLogger.subscribe(
newTreatmentEnabled,
pageIssuesLogger.newPageIssueSchemaData(
newTreatmentEnabled,
CURRENT_NS,
getIssues( KEYWORD_ALL_SECTIONS ).reduce( formatPageIssuesSeverity, [] ),
getAllIssuesSections( allIssues )
)
);
// Report that the page has been loaded.
pageIssuesLogger.log( {
action: 'pageLoaded'
} );
}
// Setup the overlay route.
overlayManager.add( new RegExp( '^/issues/(\\d+|' + KEYWORD_ALL_SECTIONS + ')$' ), function ( section ) {
return new PageIssuesOverlay(
getIssues( section ), pageIssuesLogger, section, CURRENT_NS );
getIssues( section ), section, CURRENT_NS );
} );
}
M.define( 'skins.minerva.scripts/pageIssues', {
init: initPageIssues,
// The logger requires initialization (subscription). Ideally, the logger would be
// initialized and passed to initPageIssues() by the client. Since it's not, expose a log
// method and hide the subscription call in cleanuptemplates.
log: pageIssuesLogger.log,
test: {
formatPageIssuesSeverity: formatPageIssuesSeverity,
getAllIssuesSections: getAllIssuesSections,
createBanner: createBanner
}

View File

@ -1,114 +0,0 @@
( function ( M, mwConfig, mwTrack, mwTrackSubscribe, mwUser ) {
var
util = M.require( 'mobile.startup/util' ),
EVENT_PAGE_ISSUE_LOG = 'minerva.PageIssuesAB';
/**
* Defines default data for Schema:PageIssues that will be recorded with every event.
* @param {boolean} newTreatmentEnabled
* @param {number} namespaceId The namespace for the page that has issues.
* @param {string[]} pageIssueSeverities An array of PageIssue severities.
* @param {array} pageIssuesSections
*
* @return {Object} A Partial<Schema:PageIssues> Object meant to be mixed with track data.
*/
function newPageIssueSchemaData(
newTreatmentEnabled, namespaceId, pageIssueSeverities, pageIssuesSections
) {
return {
pageTitle: mwConfig.get( 'wgTitle' ),
namespaceId: namespaceId,
pageIdSource: mwConfig.get( 'wgArticleId' ),
issuesVersion: bucketToVersion( newTreatmentEnabled ),
issuesSeverity: pageIssueSeverities,
sectionNumbers: pageIssuesSections,
isAnon: mwUser.isAnon(),
editCountBucket: getUserEditBuckets(),
sessionToken: mwUser.sessionId()
};
}
/**
* Enable tracking and add page token to every logged event.
* @param {boolean} newTreatmentEnabled
* @param {Object} pageIssueSchemaData A Partial<Schema:PageIssues> Object that will be mixed
* with track data.
* @return {void}
*/
function subscribe( newTreatmentEnabled, pageIssueSchemaData ) {
// set the page token on the request.
pageIssueSchemaData.pageToken = mw.user.getPageviewToken();
// intermediary event bus that extends the event data before being passed to event-logging.
mwTrackSubscribe( EVENT_PAGE_ISSUE_LOG, function ( topic, data ) {
var mixedData = util.extend( {}, pageIssueSchemaData, data );
// Although we use strings inside the issues code (due to the usage of the key word
// `all`) - these need to be numbers to be validated by the schema
mixedData.sectionNumbers = ( mixedData.sectionNumbers || [] ).map(
function ( sectionStr ) { return parseInt( sectionStr, 10 ); }
);
// For backwards compatibility with cached WikimediaEvents code
// (remove me in a weeks time!! T207423)
mwTrack( 'wikimedia.event.ReadingDepthSchema.enable', bucketToGroup( newTreatmentEnabled ) );
// Log readingDepth schema.(ReadingDepth is guarded against multiple enables).
// See https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/WikimediaEvents/+/437686/
mwTrack( 'wikimedia.ReadingDepthSchema.enable', bucketToGroup( newTreatmentEnabled ) );
// Log PageIssues schema.
mwTrack( 'wikimedia.event.PageIssues', mixedData );
} );
}
/**
* @param {boolean} newTreatmentEnabled
* @return {string} The page issues group associated with the treatment bucket.
*/
function bucketToGroup( newTreatmentEnabled ) {
return newTreatmentEnabled ? 'page-issues-b_sample' : 'page-issues-a_sample';
}
/**
* @param {boolean} newTreatmentEnabled
* @return {string} The page issues version associated with the treatment bucket.
*/
function bucketToVersion( newTreatmentEnabled ) {
return newTreatmentEnabled ? 'new2018' : 'old';
}
/**
* Converts user edit count into a predefined string. Note: these buckets have *nothing* to do
* with A/B bucketing.
* @return {string}
*/
function getUserEditBuckets() {
var editCount = mwConfig.get( 'wgUserEditCount', 0 );
if ( editCount === 0 ) { return '0 edits'; }
if ( editCount < 5 ) { return '1-4 edits'; }
if ( editCount < 100 ) { return '5-99 edits'; }
if ( editCount < 1000 ) { return '100-999 edits'; }
if ( editCount >= 1000 ) { return '1000+ edits'; }
// This is unlikely to ever happen. If so, we'll want to cast to a string
// that is not accepted and allow EventLogging to complain
// about invalid events so we can investigate.
return 'error (' + editCount + ')';
}
/**
* Log data to the PageIssuesAB test schema. It's safe to call this function prior to
* subscription.
* @param {Object} data to log
* @return {void}
*/
function log( data ) {
mwTrack( EVENT_PAGE_ISSUE_LOG, data );
}
M.define( 'skins.minerva.scripts/pageIssuesLogger', {
newPageIssueSchemaData: newPageIssueSchemaData,
subscribe: subscribe,
log: log
} );
}( mw.mobileFrontend, mw.config, mw.track, mw.trackSubscribe, mw.user ) );

View File

@ -158,7 +158,6 @@
}
},
"EventLoggingSchemas": {
"PageIssues": 18392542,
"WebClientError": 18340282
},
"ResourceModules": {
@ -436,7 +435,6 @@
"resources/skins.minerva.scripts/errorLogging.js",
"resources/skins.minerva.scripts/preInit.js",
"resources/skins.minerva.scripts/DownloadIcon.js",
"resources/skins.minerva.scripts/pageIssuesLogger.js",
"resources/skins.minerva.scripts/pageIssuesParser.js",
"resources/skins.minerva.scripts/AB.js",
"resources/skins.minerva.scripts/PageIssuesOverlay.js",

View File

@ -1,57 +0,0 @@
( function ( M ) {
var PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' );
QUnit.module( 'Minerva PageIssuesOverlay', {
beforeEach: function () {
this.logger = {
log: this.sandbox.spy()
};
}
} );
QUnit.test( '#log (section=all)', function ( assert ) {
var overlay = new PageIssuesOverlay( [], this.logger, 'all', 0 );
overlay.onExit();
assert.strictEqual( this.logger.log.calledOnce, true, 'Logger called once' );
assert.strictEqual(
this.logger.log.calledWith( {
action: 'modalClose',
issuesSeverity: []
} ), true, 'sectionNumbers is not set (T202940)'
);
} );
QUnit.test( '#log (section=1)', function ( assert ) {
var overlay = new PageIssuesOverlay( [
{
severity: 'MEDIUM'
}
], this.logger, '1', 0 );
overlay.onExit();
assert.strictEqual(
this.logger.log.calledWith( {
action: 'modalClose',
issuesSeverity: [ 'MEDIUM' ],
sectionNumbers: [ '1' ]
} ), true, 'sectionNumbers is set'
);
} );
QUnit.test( '#log (section=2) multiple issues', function ( assert ) {
var overlay = new PageIssuesOverlay(
[ {
severity: 'MEDIUM'
},
{
severity: 'LOW'
} ], this.logger, '2', 0 );
overlay.onExit();
assert.strictEqual(
this.logger.log.calledWith( {
action: 'modalClose',
issuesSeverity: [ 'MEDIUM', 'LOW' ],
sectionNumbers: [ '2', '2' ]
} ), true, 'sectionNumbers is set for each issue'
);
} );
}( mw.mobileFrontend ) );

View File

@ -3,7 +3,6 @@
util = M.require( 'mobile.startup/util' ),
createBanner = pageIssues.test.createBanner,
icon = {},
formatPageIssuesSeverity = pageIssues.test.formatPageIssuesSeverity,
MEDIUM_ISSUE = {
issue: {
severity: 'MEDIUM',
@ -12,24 +11,6 @@
iconString: 'i',
text: 't'
},
MEDIUM_MULTIPLE_ISSUE = {
issue: {
severity: 'MEDIUM',
grouped: true,
icon: icon
},
iconString: 'i',
text: 't'
},
LOW_MULTIPLE_ISSUE = {
issue: {
severity: 'LOW',
grouped: true,
icon: icon
},
iconString: 'i',
text: 't'
},
LOW_ISSUE = {
issue: {
severity: 'LOW',
@ -81,48 +62,6 @@
assert.strictEqual( window.location.hash, '#/issues/' + SECTION );
} );
// NOTE: Only for PageIssues AB
QUnit.test( 'clicking on the product of createBanner() should trigger a custom event', function ( assert ) {
var mockAction = {
action: 'issueClicked',
issuesSeverity: [ 'MEDIUM' ],
sectionNumbers: [ SECTION ]
};
mw.trackSubscribe( 'minerva.PageIssuesAB', function ( topic, data ) {
assert.deepEqual( mockAction, data );
} );
processedAmbox.click();
} );
QUnit.test( 'formatPageIssuesSeverity', function ( assert ) {
var multipleIssues = [
MEDIUM_MULTIPLE_ISSUE,
LOW_MULTIPLE_ISSUE,
LOW_MULTIPLE_ISSUE
],
multipleSingleIssues = [
LOW_ISSUE,
HIGH_ISSUE,
MEDIUM_ISSUE
],
mixedMultipleSingle = [
HIGH_ISSUE,
LOW_MULTIPLE_ISSUE,
MEDIUM_MULTIPLE_ISSUE,
LOW_ISSUE,
MEDIUM_ISSUE,
HIGH_ISSUE
],
testMultiple = multipleIssues.reduce( formatPageIssuesSeverity, [] ),
testSingle = multipleSingleIssues.reduce( formatPageIssuesSeverity, [] ),
testMixed = mixedMultipleSingle.reduce( formatPageIssuesSeverity, [] );
assert.deepEqual( testMultiple, [ 'MEDIUM' ], 'Multiple issues return one maxSeverity value' );
assert.deepEqual( testSingle, [ 'LOW', 'HIGH', 'MEDIUM' ], 'Single issues return each corresponding severity' );
assert.deepEqual( testMixed, [ 'HIGH', 'MEDIUM', 'LOW', 'MEDIUM', 'HIGH' ], 'Mixed single/multiple return one value for multiples' );
} );
QUnit.test( 'getAllIssuesSections', function ( assert ) {
var multipleIssuesWithDeletion,
multipleIssues, allIssuesOldTreatment, allIssuesNewTreatment;