Cleanup CommentController and document

* Document all methods
* Rename comment to threadItem
* Use this.threadItem instead of passing in identical
  threadItem in various methods.
* Don't pass threadItem to ReplyWidget as we already
  pass the whole CommentController.

Change-Id: If9aad0bcf9f0e4ebf3342b75631ddac8b57f7d87
This commit is contained in:
Ed Sanders 2021-12-20 17:06:24 +00:00 committed by Bartosz Dziewoński
parent dd9b3d517e
commit 7381d9d963
6 changed files with 108 additions and 66 deletions

View file

@ -33,12 +33,19 @@ if ( defaultVisual || enable2017Wikitext ) {
mw.loader.using( plainModules );
}
function CommentController( $pageContainer, comment, parser ) {
/**
* Handles setup, save and teardown of commenting widgets
*
* @param {jQuery} $pageContainer Page container
* @param {ThreadItem} threadItem Thread item to attach new comment to
* @param {mw.dt.Parser} parser Comment parser
*/
function CommentController( $pageContainer, threadItem, parser ) {
// Mixin constructors
OO.EventEmitter.call( this );
this.$pageContainer = $pageContainer;
this.comment = comment;
this.threadItem = threadItem;
this.parser = parser;
this.newListItem = null;
this.replyWidgetPromise = null;
@ -68,15 +75,15 @@ function getLatestRevId( pageName ) {
}
/**
* Like #checkCommentOnPage, but assumes the comment was found on the current page,
* Like #checkThreadItemOnPage, but assumes the comment was found on the current page,
* and then follows transclusions to determine the source page where it is written.
*
* @param {CommentItem} comment Comment
* @return {jQuery.Promise} Promise which resolves with a CommentDetails object, or rejects with an error
*/
CommentController.prototype.getTranscludedFromSource = function ( comment ) {
CommentController.prototype.getTranscludedFromSource = function () {
var pageName = mw.config.get( 'wgRelevantPageName' ),
oldId = mw.config.get( 'wgCurRevisionId' );
oldId = mw.config.get( 'wgCurRevisionId' ),
threadItem = this.getThreadItem();
function followTransclusion( recursionLimit, code, data ) {
var errorData;
@ -85,7 +92,7 @@ CommentController.prototype.getTranscludedFromSource = function ( comment ) {
if ( errorData.follow && typeof errorData.transcludedFrom === 'string' ) {
return getLatestRevId( errorData.transcludedFrom ).then( function ( latestRevId ) {
// Fetch the transcluded page, until we cross the recursion limit
return controller.checkCommentOnPage( errorData.transcludedFrom, latestRevId, comment )
return controller.checkThreadItemOnPage( errorData.transcludedFrom, latestRevId, threadItem )
.catch( followTransclusion.bind( null, recursionLimit - 1 ) );
} );
}
@ -95,7 +102,7 @@ CommentController.prototype.getTranscludedFromSource = function ( comment ) {
// Arbitrary limit of 10 steps, which should be more than anyone could ever need
// (there are reasonable use cases for at least 2)
var promise = controller.checkCommentOnPage( pageName, oldId, comment )
var promise = controller.checkThreadItemOnPage( pageName, oldId, threadItem )
.catch( followTransclusion.bind( null, 10 ) );
return promise;
@ -114,7 +121,7 @@ CommentController.static.initType = 'page';
* @param {boolean} [hideErrors] Suppress errors, e.g. when restoring auto-save
*/
CommentController.prototype.setup = function ( mode, hideErrors ) {
var comment = this.comment,
var threadItem = this.getThreadItem(),
commentController = this;
if ( mode === undefined ) {
@ -132,8 +139,8 @@ CommentController.prototype.setup = function ( mode, hideErrors ) {
} );
if ( !this.replyWidgetPromise ) {
this.replyWidgetPromise = this.getTranscludedFromSource( comment ).then( function ( commentDetails ) {
return commentController.createReplyWidget( comment, commentDetails, { mode: mode } );
this.replyWidgetPromise = this.getTranscludedFromSource().then( function ( commentDetails ) {
return commentController.createReplyWidget( commentDetails, { mode: mode } );
}, function ( code, data ) {
commentController.teardown();
@ -155,7 +162,7 @@ CommentController.prototype.setup = function ( mode, hideErrors ) {
} );
// On first load, add a placeholder list item
commentController.newListItem = modifier.addListItem( comment, dtConf.replyIndentation );
commentController.newListItem = modifier.addListItem( threadItem, dtConf.replyIndentation );
if ( commentController.newListItem.tagName.toLowerCase() === 'li' ) {
// When using bullet syntax, hide the marker. (T259864#7634107)
$( commentController.newListItem ).addClass( 'ext-discussiontools-init-noMarker' );
@ -171,7 +178,7 @@ CommentController.prototype.setup = function ( mode, hideErrors ) {
commentController.replyWidgetPromise.then( function ( replyWidget ) {
if ( !commentController.newListItem ) {
// On subsequent loads, there's no list item yet, so create one now
commentController.newListItem = modifier.addListItem( comment, dtConf.replyIndentation );
commentController.newListItem = modifier.addListItem( threadItem, dtConf.replyIndentation );
if ( commentController.newListItem.tagName.toLowerCase() === 'li' ) {
// When using bullet syntax, hide the marker. (T259864#7634107)
$( commentController.newListItem ).addClass( 'ext-discussiontools-init-noMarker' );
@ -188,6 +195,21 @@ CommentController.prototype.setup = function ( mode, hideErrors ) {
} );
};
/**
* Get thread item this controller is attached to
*
* @return {ThreadItem} Thread item
*/
CommentController.prototype.getThreadItem = function () {
return this.threadItem;
};
/**
* Get the reply widget class to use in this controller
*
* @param {boolean} visual Prefer the VE-based class
* @return {jQuery.Promise} Promise which resolves with a Function: the reply widget class
*/
CommentController.prototype.getReplyWidgetClass = function ( visual ) {
// If 2017WTE mode is enabled, always use ReplyWidgetVisual.
visual = visual || enable2017Wikitext;
@ -198,15 +220,17 @@ CommentController.prototype.getReplyWidgetClass = function ( visual ) {
};
/**
* @param {CommentItem} comment
* Create a reply widget
*
* @param {CommentDetails} commentDetails
* @param {Object} config
* @return {jQuery.Promise} Promise resolved with a ReplyWidget
*/
CommentController.prototype.createReplyWidget = function ( comment, commentDetails, config ) {
CommentController.prototype.createReplyWidget = function ( commentDetails, config ) {
var commentController = this;
return this.getReplyWidgetClass( config.mode === 'visual' ).then( function ( ReplyWidget ) {
return new ReplyWidget( commentController, comment, commentDetails, config );
return new ReplyWidget( commentController, commentDetails, config );
} );
};
@ -244,14 +268,14 @@ CommentController.prototype.teardown = function ( abandoned ) {
/**
* Get the parameters of the API query that can be used to post this comment.
*
* @param {ThreadItem} comment Parent comment
* @param {string} pageName Title of the page to post on
* @param {Object} checkboxes Value of the promise returned by controller#getCheckboxesPromise
* @return {Object.<string,string>} API query data
*/
CommentController.prototype.getApiQuery = function ( comment, pageName, checkboxes ) {
CommentController.prototype.getApiQuery = function ( pageName, checkboxes ) {
var threadItem = this.getThreadItem();
var replyWidget = this.replyWidget;
var sameNameComments = this.parser.findCommentsByName( comment.name );
var sameNameComments = this.parser.findCommentsByName( threadItem.name );
var mode = replyWidget.getMode();
var tags = [
@ -268,9 +292,9 @@ CommentController.prototype.getApiQuery = function ( comment, pageName, checkbox
action: 'discussiontoolsedit',
paction: 'addcomment',
page: pageName,
commentname: comment.name,
commentname: threadItem.name,
// Only specify this if necessary to disambiguate, to avoid errors if the parent changes
commentid: sameNameComments.length > 1 ? comment.id : undefined,
commentid: sameNameComments.length > 1 ? threadItem.id : undefined,
summary: replyWidget.getEditSummary(),
formtoken: replyWidget.getFormToken(),
assert: mw.user.isAnon() ? 'anon' : 'user',
@ -304,12 +328,19 @@ CommentController.prototype.getApiQuery = function ( comment, pageName, checkbox
return data;
};
CommentController.prototype.save = function ( comment, pageName ) {
/**
* Save the comment in the comment controller
*
* @param {string} pageName Page title
* @return {jQuery.Promise} Promise which resolves when the save is complete
*/
CommentController.prototype.save = function ( pageName ) {
var replyWidget = this.replyWidget,
commentController = this;
commentController = this,
threadItem = this.getThreadItem();
return this.replyWidget.checkboxesPromise.then( function ( checkboxes ) {
var data = commentController.getApiQuery( comment, pageName, checkboxes );
var data = commentController.getApiQuery( pageName, checkboxes );
// No timeout. Huge talk pages can take a long time to save, and falsely reporting an error
// could result in duplicate messages if the user retries. (T249071)
@ -339,11 +370,16 @@ CommentController.prototype.save = function ( comment, pageName ) {
}
return $.Deferred().reject( code, responseData ).promise();
} ).then( function ( responseData ) {
controller.update( responseData, comment, pageName, replyWidget );
controller.update( responseData, threadItem, pageName, replyWidget );
} );
} );
};
/**
* Switch reply widget to wikitext input
*
* @return {jQuery.Promise} Promise which resolves when switch is complete
*/
CommentController.prototype.switchToWikitext = function () {
var oldWidget = this.replyWidget,
target = oldWidget.replyBodyWidget.target,
@ -355,7 +391,6 @@ CommentController.prototype.switchToWikitext = function () {
// TODO: We may need to pass oldid/etag when editing is supported
var wikitextPromise = target.getWikitextFragment( target.getSurface().getModel().getDocument() );
this.replyWidgetPromise = this.createReplyWidget(
oldWidget.comment,
oldWidget.commentDetails,
{ mode: 'source' }
);
@ -443,6 +478,11 @@ CommentController.prototype.getUnsupportedNodeSelectors = function () {
};
};
/**
* Switch reply widget to visual input
*
* @return {jQuery.Promise} Promise which resolves when switch is complete
*/
CommentController.prototype.switchToVisual = function () {
var oldWidget = this.replyWidget,
oldShowAdvanced = oldWidget.showAdvanced,
@ -476,7 +516,6 @@ CommentController.prototype.switchToVisual = function () {
parsePromise = $.Deferred().resolve( '' ).promise();
}
this.replyWidgetPromise = this.createReplyWidget(
oldWidget.comment,
oldWidget.commentDetails,
{ mode: 'visual' }
);

View file

@ -5,6 +5,12 @@ var
CommentController = require( './CommentController.js' ),
HeadingItem = require( './HeadingItem.js' );
/**
* Handles setup, save and teardown of new topic widget
*
* @param {jQuery} $pageContainer Page container
* @param {mw.dt.Parser} parser Comment parser
*/
function NewTopicController( $pageContainer, parser ) {
this.container = new OO.ui.PanelLayout( {
classes: [ 'ext-discussiontools-ui-newTopic' ],
@ -30,16 +36,16 @@ function NewTopicController( $pageContainer, parser ) {
this.container.$element.append( this.$notices, this.sectionTitleField.$element );
// HeadingItem representing the heading being added, so that we can pretend we're replying to it
var comment = new HeadingItem( {
var threadItem = new HeadingItem( {
startContainer: this.sectionTitleField.$element[ 0 ],
startOffset: 0,
endContainer: this.sectionTitleField.$element[ 0 ],
endOffset: this.sectionTitleField.$element[ 0 ].childNodes.length
} );
comment.id = utils.NEW_TOPIC_COMMENT_ID;
comment.isNewTopic = true;
threadItem.id = utils.NEW_TOPIC_COMMENT_ID;
threadItem.isNewTopic = true;
NewTopicController.super.call( this, $pageContainer, comment, parser );
NewTopicController.super.call( this, $pageContainer, threadItem, parser );
}
OO.inheritClass( NewTopicController, CommentController );
@ -267,8 +273,8 @@ NewTopicController.prototype.getUnsupportedNodeSelectors = function () {
/**
* @inheritdoc
*/
NewTopicController.prototype.getApiQuery = function ( comment, pageName, checkboxes ) {
var data = NewTopicController.super.prototype.getApiQuery.call( this, comment, pageName, checkboxes );
NewTopicController.prototype.getApiQuery = function ( pageName, checkboxes ) {
var data = NewTopicController.super.prototype.getApiQuery.call( this, pageName, checkboxes );
// Rebuild the tags array and remove the reply tag
var tags = ( data.dttags || '' ).split( ',' );

View file

@ -137,16 +137,16 @@ function getPageData( pageName, oldId, isNewTopic ) {
}
/**
* Check if a given comment on a page can be replied to
* Check if a given thread item on a page can be replied to
*
* @param {string} pageName Page title
* @param {number} oldId Revision ID
* @param {CommentItem} comment Comment
* @param {ThreadItem} threadItem Thread item
* @return {jQuery.Promise} Resolved with a CommentDetails object if the comment appears on the page.
* Rejects with error data if the comment is transcluded, or there are lint errors on the page.
*/
function checkCommentOnPage( pageName, oldId, comment ) {
var isNewTopic = comment.id === utils.NEW_TOPIC_COMMENT_ID;
function checkThreadItemOnPage( pageName, oldId, threadItem ) {
var isNewTopic = threadItem.id === utils.NEW_TOPIC_COMMENT_ID;
return getPageData( pageName, oldId, isNewTopic )
.then( function ( response ) {
@ -155,17 +155,17 @@ function checkCommentOnPage( pageName, oldId, comment ) {
transcludedFrom = response.transcludedfrom;
if ( !isNewTopic ) {
// First look for data by the comment's ID. If not found, also look by name.
// First look for data by the thread item's ID. If not found, also look by name.
// Data by ID may not be found due to differences in headings (e.g. T273413, T275821),
// or if a comment's parent changes.
// Data by name might be combined from two or more comments, which would only allow us to
// or if a thread item's parent changes.
// Data by name might be combined from two or more thread items, which would only allow us to
// treat them both as transcluded from unknown source, unless we check ID first.
var isTranscludedFrom = transcludedFrom[ comment.id ];
var isTranscludedFrom = transcludedFrom[ threadItem.id ];
if ( isTranscludedFrom === undefined ) {
isTranscludedFrom = transcludedFrom[ comment.name ];
isTranscludedFrom = transcludedFrom[ threadItem.name ];
}
if ( isTranscludedFrom === undefined ) {
// The comment wasn't found when generating the "transcludedfrom" data,
// The thread item wasn't found when generating the "transcludedfrom" data,
// so we don't know where the reply should be posted. Just give up.
return $.Deferred().reject( 'discussiontools-commentid-notfound-transcludedfrom', { errors: [ {
code: 'discussiontools-commentid-notfound-transcludedfrom',
@ -898,11 +898,11 @@ function init( $container, state ) {
* Update the page after a comment is published/saved
*
* @param {Object} data Edit API response data
* @param {ThreadItem} comment Parent thread item
* @param {ThreadItem} threadItem Parent thread item
* @param {string} pageName Page title
* @param {mw.dt.ReplyWidget} replyWidget ReplyWidget
*/
function update( data, comment, pageName, replyWidget ) {
function update( data, threadItem, pageName, replyWidget ) {
var api = getApi(),
pageUpdated = $.Deferred(),
$content;
@ -920,7 +920,7 @@ function update( data, comment, pageName, replyWidget ) {
replyWidget.unbindBeforeUnloadHandler();
replyWidget.clearStorage();
replyWidget.setPending( true );
window.location = mw.util.getUrl( pageName, { dtrepliedto: comment.id } );
window.location = mw.util.getUrl( pageName, { dtrepliedto: threadItem.id } );
return;
}
@ -932,7 +932,7 @@ function update( data, comment, pageName, replyWidget ) {
if ( OO.ui.isMobile() ) {
// MobileFrontend does not use the 'wikipage.content' hook, and its interface will not
// re-initialize properly (e.g. page sections won't be collapsible). Reload the whole page.
window.location = mw.util.getUrl( pageName, { dtrepliedto: comment.id } );
window.location = mw.util.getUrl( pageName, { dtrepliedto: threadItem.id } );
return;
}
@ -986,7 +986,7 @@ function update( data, comment, pageName, replyWidget ) {
} ).catch( function () {
// We saved the reply, but couldn't purge or fetch the updated page. Seems difficult to
// explain this problem. Redirect to the page where the user can at least see their reply…
window.location = mw.util.getUrl( pageName, { dtrepliedto: comment.id } );
window.location = mw.util.getUrl( pageName, { dtrepliedto: threadItem.id } );
} );
}
@ -1004,7 +1004,7 @@ function update( data, comment, pageName, replyWidget ) {
pageUpdated.then( function () {
// Re-initialize and highlight the new reply.
mw.dt.initState.repliedTo = comment.id;
mw.dt.initState.repliedTo = threadItem.id;
// We need our init code to run after everyone else's handlers for this hook,
// so that all changes to the page layout have been completed (e.g. collapsible elements),
@ -1028,7 +1028,7 @@ function update( data, comment, pageName, replyWidget ) {
module.exports = {
init: init,
update: update,
checkCommentOnPage: checkCommentOnPage,
checkThreadItemOnPage: checkThreadItemOnPage,
getCheckboxesPromise: getCheckboxesPromise,
getApi: getApi
};

View file

@ -19,12 +19,11 @@ require( './AbandonTopicDialog.js' );
* @extends OO.ui.Widget
* @constructor
* @param {CommentController} commentController Comment controller
* @param {CommentItem} comment Comment item
* @param {CommentDetails} commentDetails
* @param {Object} [config] Configuration options
* @param {Object} [config.input] Configuration options for the comment input widget
*/
function ReplyWidget( commentController, comment, commentDetails, config ) {
function ReplyWidget( commentController, commentDetails, config ) {
var widget = this;
config = config || {};
@ -34,18 +33,18 @@ function ReplyWidget( commentController, comment, commentDetails, config ) {
this.pending = false;
this.commentController = commentController;
this.comment = comment;
var threadItem = commentController.getThreadItem();
this.commentDetails = commentDetails;
this.isNewTopic = !!comment.isNewTopic;
this.isNewTopic = !!threadItem.isNewTopic;
this.pageName = commentDetails.pageName;
this.oldId = commentDetails.oldId;
// pageExists can only be false for the new comment tool, so we
// don't need to worry about transcluded replies.
this.pageExists = mw.config.get( 'wgRelevantArticleId', 0 ) !== 0;
var contextNode = utils.closestElement( comment.range.endContainer, [ 'dl', 'ul', 'ol' ] );
var contextNode = utils.closestElement( threadItem.range.endContainer, [ 'dl', 'ul', 'ol' ] );
this.context = contextNode ? contextNode.nodeName.toLowerCase() : 'dl';
// TODO: Should storagePrefix include pageName?
this.storagePrefix = 'reply/' + comment.id;
this.storagePrefix = 'reply/' + threadItem.id;
this.storage = mw.storage.session;
// eslint-disable-next-line no-jquery/no-global-selector
this.contentDir = $( '#mw-content-text' ).css( 'direction' );
@ -54,12 +53,12 @@ function ReplyWidget( commentController, comment, commentDetails, config ) {
{
placeholder: this.isNewTopic ?
mw.msg( 'discussiontools-replywidget-placeholder-newtopic' ) :
mw.msg( 'discussiontools-replywidget-placeholder-reply', comment.author ),
mw.msg( 'discussiontools-replywidget-placeholder-reply', threadItem.author ),
authors: this.isNewTopic ?
// No suggestions in new topic tool yet (T277357)
[] :
// comment is a CommentItem when replying
comment.getHeading().getAuthorsBelow()
// threadItem is a CommentItem when replying
threadItem.getHeading().getAuthorsBelow()
},
config.input
);
@ -520,7 +519,7 @@ ReplyWidget.prototype.setup = function ( data ) {
// in NewTopicController#onSectionTitleChange
summary = '';
} else {
var title = this.comment.getHeading().getLinkableTitle();
var title = this.commentController.getThreadItem().getHeading().getLinkableTitle();
summary = ( title ? '/* ' + title + ' */ ' : '' ) +
mw.msg( 'discussiontools-defaultsummary-reply' );
}
@ -843,10 +842,10 @@ ReplyWidget.prototype.onReplyClick = function () {
logger( { action: 'saveIntent' } );
// TODO: When editing a transcluded page, VE API returning the page HTML is a waste, since we won't use it
var pageName = this.pageName,
comment = this.comment;
var pageName = this.pageName;
logger( { action: 'saveAttempt' } );
widget.commentController.save( comment, pageName ).fail( function ( code, data ) {
widget.commentController.save( pageName ).fail( function ( code, data ) {
// Compare to ve.init.mw.ArticleTargetEvents.js in VisualEditor.
var typeMap = {
badtoken: 'userBadToken',

View file

@ -7,7 +7,6 @@ var utils = require( 'ext.discussionTools.init' ).utils;
* @extends mw.dt.ReplyWidget
* @constructor
* @param {CommentController} commentController
* @param {CommentItem} comment
* @param {CommentDetails} commentDetails
* @param {Object} [config]
*/

View file

@ -15,12 +15,11 @@ require( './dt-ve/dt.ce.PingNode.js' );
* @extends mw.dt.ReplyWidget
* @constructor
* @param {CommentController} commentController
* @param {CommentItem} comment
* @param {CommentDetails} commentDetails
* @param {Object} [config]
* @param {string} [config.mode] Default edit mode, 'source' or 'visual'
*/
function ReplyWidgetVisual( commentController, comment, commentDetails, config ) {
function ReplyWidgetVisual( commentController, commentDetails, config ) {
this.defaultMode = config.mode;
// Parent constructor