From ed5e9dd1d5f65d9874a4ef6872c7327911a0ab61 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Wed, 22 Nov 2017 10:55:24 -0800 Subject: [PATCH] Hygiene: Refactor Minerva history link generation * Create several sub functions to manage complexity of the history link * Clean up MinervaTemplate to use data directly and remove isMainPage template variable which is no longer being used. Change-Id: I124aec9637f3635a335c58e559e578b2a56eb4c5 --- includes/skins/MinervaTemplate.php | 9 +- includes/skins/SkinMinerva.php | 129 ++++++++++++++++------------- includes/skins/history.mustache | 8 +- includes/skins/minerva.mustache | 2 +- 4 files changed, 78 insertions(+), 70 deletions(-) diff --git a/includes/skins/MinervaTemplate.php b/includes/skins/MinervaTemplate.php index d747916..7726d05 100644 --- a/includes/skins/MinervaTemplate.php +++ b/includes/skins/MinervaTemplate.php @@ -107,18 +107,11 @@ class MinervaTemplate extends BaseTemplate { protected function getHistoryLinkHtml( $data ) { $action = Action::getActionName( RequestContext::getMain() ); if ( isset( $data['historyLink'] ) && $action === 'view' ) { - $historyLink = $data['historyLink']; $args = [ 'clockIconClass' => MinervaUI::iconClass( 'clock-gray', 'before' ), 'arrowIconClass' => MinervaUI::iconClass( 'arrow-gray', 'element', 'mw-ui-icon-small mf-mw-ui-icon-rotate-anti-clockwise indicator' ), - 'isMainPage' => $this->getSkin()->getTitle()->isMainPage(), - 'link' => $historyLink['href'], - 'text' => $historyLink['text'], - 'username' => $historyLink['data-user-name'], - 'userGender' => $historyLink['data-user-gender'], - 'timestamp' => $historyLink['data-timestamp'] - ]; + ] + $data['historyLink']; $templateParser = new TemplateParser( __DIR__ ); return $templateParser->processTemplate( 'history', $args ); } else { diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 4b7ccaa..d5e3d2e 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -717,17 +717,61 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin { } } + /** + * Get a history link which describes author and relative time of last edit + * @param Title $title The Title object of the page being viewed + * @param int $timestamp + * @return array + */ + protected function getRelativeHistoryLink( Title $title, $timestamp ) { + $user = $this->getUser(); + $text = $this->msg( + 'minerva-last-modified-date', + $this->getLanguage()->userDate( $timestamp, $user ), + $this->getLanguage()->userTime( $timestamp, $user ) + )->parse(); + return [ + // Use $edit['timestamp'] (Unix format) instead of $timestamp (MW format) + 'data-timestamp' => wfTimestamp( TS_UNIX, $timestamp ), + 'href' => $this->getHistoryUrl( $title ), + 'text' => $text, + ] + $this->getRevisionEditorData( $title ); + } + + /** + * Get a history link which makes no reference to user or last edited time + * @param Title $title The Title object of the page being viewed + * @return array + */ + protected function getGenericHistoryLink( Title $title ) { + $text = $this->msg( 'mobile-frontend-history' )->plain(); + return [ + 'href' => $this->getHistoryUrl( $title ), + 'text' => $text, + ]; + } + + /** + * Get the URL for the history page for the given title using Special:History + * when available. + * @param Title $title The Title object of the page being viewed + * @return string + */ + protected function getHistoryUrl( Title $title ) { + return SpecialPageFactory::exists( 'History' ) ? + SpecialPage::getTitleFor( 'History', $title )->getLocalURL() : + $title->getLocalURL( [ 'action' => 'history' ] ); + } + /** * Prepare the content for the 'last edited' message, e.g. 'Last edited on 30 August * 2013, at 23:31'. This message is different for the main page since main page * content is typically transcuded rather than edited directly. * @param Title $title The Title object of the page being viewed - * @param Boolean $doNotShowRelativeTime (optional) when passed will use a generic - * label which makes no reference to the time the page was edited. * @return array */ - protected function getHistoryLink( Title $title, $doNotShowRelativeTime ) { - $user = $this->getUser(); + protected function getHistoryLink( Title $title ) { + $isLatestRevision = $this->getRevisionId() === $title->getLatestRevID(); // Get rev_timestamp of current revision (preloaded by MediaWiki core) $timestamp = $this->getOutput()->getRevisionTimestamp(); # No cached timestamp, load it from the database @@ -735,61 +779,34 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin { $timestamp = Revision::getTimestampFromId( $this->getTitle(), $this->getRevisionId() ); } - // Main pages tend to include transclusions (see bug 51924) - if ( $doNotShowRelativeTime ) { - $lastModified = $this->msg( 'mobile-frontend-history' )->plain(); - } else { - $lastModified = $this->msg( - 'minerva-last-modified-date', - $this->getLanguage()->userDate( $timestamp, $user ), - $this->getLanguage()->userTime( $timestamp, $user ) - )->parse(); - } - - if ( SpecialPageFactory::exists( 'History' ) ) { - $historyUrl = SpecialPage::getTitleFor( 'History', $title )->getLocalURL(); - } else { - $historyUrl = $title->getLocalURL( [ 'action' => 'history' ] ); - } - - $rev = Revision::newFromTitle( $title ); - if ( $rev ) { - $editor = $this->getRevisionEditor( $rev ); - } else { - $editor = false; - } - return [ - // Use $edit['timestamp'] (Unix format) instead of $timestamp (MW format) - 'data-timestamp' => $doNotShowRelativeTime ? '' : wfTimestamp( TS_UNIX, $timestamp ), - 'href' => $historyUrl, - 'text' => $lastModified, - 'data-user-name' => $editor ? $editor['name'] : '', - 'data-user-gender' => $editor ? $editor['gender'] : '' - ]; + return !$isLatestRevision || $title->isMainPage() ? + $this->getGenericHistoryLink( $title ) : + $this->getRelativeHistoryLink( $title, $timestamp ); } /** - * Returns the editor of a current revision. - * There appears to be no shorthand method for this in core. - * @return array|false representing user with name and gender fields. False if the editor no longer - * exists in the database or is hidden from public view + * Returns data attributes representing the editor for the current revision. + * @param Title $title The Title object of the page being viewed + * @return array representing user with name and gender fields. Empty if the editor no longer + * exists in the database or is hidden from public view. */ - private function getRevisionEditor( Revision $rev ) { - $editorName = ''; - $editorGender = ''; - $revUserId = $rev->getUser(); - // Note the user will only be returned if that information is public - if ( $revUserId ) { - $revUser = User::newFromId( $revUserId ); - $editorName = $revUser->getName(); - $editorGender = $revUser->getOption( 'gender' ); - } else { - return false; + private function getRevisionEditorData( Title $title ) { + $rev = Revision::newFromTitle( $title ); + $result = []; + if ( $rev ) { + $revUserId = $rev->getUser(); + // Note the user will only be returned if that information is public + if ( $revUserId ) { + $revUser = User::newFromId( $revUserId ); + $editorName = $revUser->getName(); + $editorGender = $revUser->getOption( 'gender' ); + $result += [ + 'data-user-name' => $editorName, + 'data-user-gender' => $editorGender, + ]; + } } - return [ - 'name' => $editorName, - 'gender' => $editorGender, - ]; + return $result; } /** @@ -888,15 +905,13 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin { $tpl->set( 'postheadinghtml', $postHeadingHtml ); if ( $this->canUseWikiPage() ) { - $isLatestRevision = $this->getRevisionId() === $title->getLatestRevID(); // If it's a page that exists, add last edited timestamp // The relative time is only rendered on the latest revision. // For older revisions the last modified // information will not render with a relative time // nor will it show the name of the editor. if ( $this->getWikiPage()->exists() ) { - $tpl->set( 'historyLink', $this->getHistoryLink( $title, - !$isLatestRevision || $title->isMainPage() ) ); + $tpl->set( 'historyLink', $this->getHistoryLink( $title ) ); } } $tpl->set( 'headinghtml', $this->getHeadingHtml() ); diff --git a/includes/skins/history.mustache b/includes/skins/history.mustache index c2e1728..84a24fb 100644 --- a/includes/skins/history.mustache +++ b/includes/skins/history.mustache @@ -1,10 +1,10 @@
-
+
{{text}}
diff --git a/includes/skins/minerva.mustache b/includes/skins/minerva.mustache index 4d0b1c1..9daca50 100644 --- a/includes/skins/minerva.mustache +++ b/includes/skins/minerva.mustache @@ -32,4 +32,4 @@ {{>footer}}
- +