DeleteAction-related cleanup, part 3

Move all output-related code from Article to DeleteAction.
Article::doDelete is now deprecated, because there are some callers in
the wild, although I don't think any caller should need it.

Kill some ancient-PHP-style pass-by-refs that are useless and only make
the code more error-prone for both the caller and the callee.

Bug: T288282
Change-Id: Ic1de0ed8ebba15da5ed9f5cd11625017360a7672
This commit is contained in:
Daimona Eaytoy 2021-08-12 15:43:04 +02:00
parent 85b6398f2b
commit c0c213ba46
5 changed files with 50 additions and 25 deletions

View file

@ -105,6 +105,7 @@ because of Phabricator reports.
* The following methods from FileDeleteForm were removed:
- ::__construct (the class is no longer newable)
- ::execute()
- ::haveDeletableFile()
Use FileDeleteAction instead.
* MessageCache::singleton(), deprecated since 1.34, has been removed.
* LockManagerGroup::singleton() and ::destroySingletons(), both deprecated since
@ -176,6 +177,9 @@ because of Phabricator reports.
* Content::getParserOutput() was deprecated.
Use ContentRenderer::getParserOutput and override
ContentHandler::fillParserOutput instead.
* Article::doDelete() was deprecated. Use WikiPage::doDeleteArticleReal if
you only need to delete the article. If you also need things to happen
with OutputPage, you may want to check the hooks in DeleteAction instead.
* MessageContent class was hard-deprecated.
* Message::content() was hard-deprecated.
* The following methods from the ParserOutput class were hard deprecated:

View file

@ -34,9 +34,9 @@ class FileDeleteForm {
/**
* Really delete the file
*
* @param Title &$title
* @param LocalFile &$file
* @param ?string &$oldimage Archive name
* @param Title $title
* @param LocalFile $file
* @param string|null $oldimage Archive name
* @param string $reason Reason of the deletion
* @param bool $suppress Whether to mark all deleted versions as restricted
* @param UserIdentity $user
@ -44,7 +44,7 @@ class FileDeleteForm {
* @throws MWException
* @return Status
*/
public static function doDelete( &$title, &$file, &$oldimage, $reason,
public static function doDelete( Title $title, LocalFile $file, ?string $oldimage, $reason,
$suppress, UserIdentity $user, $tags = []
): Status {
if ( $oldimage ) {
@ -101,7 +101,7 @@ class FileDeleteForm {
$logtype = $suppress ? 'suppress' : 'delete';
$logEntry = new ManualLogEntry( $logtype, 'delete' );
$logEntry->setPerformer( $user );
$logEntry->setTarget( clone $title );
$logEntry->setTarget( $title );
$logEntry->setComment( $reason );
$logEntry->addTags( $tags );
$logid = $logEntry->insert();
@ -147,20 +147,4 @@ class FileDeleteForm {
&& strpos( $oldimage, '/' ) === false
&& strpos( $oldimage, '\\' ) === false;
}
/**
* Could we delete the file specified? If an `oldimage`
* value was provided, does it correspond to an
* existing, local, old version of this file?
*
* @param LocalFile &$file
* @param LocalFile &$oldfile
* @param string $oldimage
* @return bool
*/
public static function haveDeletableFile( &$file, &$oldfile, $oldimage ) {
return $oldimage
? $oldfile && $oldfile->exists() && $oldfile->isLocal()
: $file && $file->exists() && $file->isLocal();
}
}

View file

@ -82,6 +82,7 @@ class DeleteAction extends FormlessAction {
$context = $this->getContext();
$user = $context->getUser();
$request = $context->getRequest();
$outputPage = $context->getOutput();
$this->runExecuteChecks( $title );
$this->prepareOutput( $context->msg( 'delete-confirm', $title->getPrefixedText() ), $title );
@ -91,7 +92,6 @@ class DeleteAction extends FormlessAction {
$request->wasPosted() ? WikiPage::READ_LATEST : WikiPage::READ_NORMAL
);
if ( !$article->getPage()->exists() ) {
$outputPage = $context->getOutput();
$outputPage->setPageTitle( $context->msg( 'cannotdelete-title', $title->getPrefixedText() ) );
$outputPage->wrapWikiMsg( "<div class=\"error mw-error-cannotdelete\">\n$1\n</div>",
[ 'cannotdelete', wfEscapeWikiText( $title->getPrefixedText() ) ]
@ -120,7 +120,39 @@ class DeleteAction extends FormlessAction {
$suppress = $request->getCheck( 'wpSuppress' ) &&
$context->getAuthority()->isAllowed( 'suppressrevision' );
$article->doDelete( $reason, $suppress );
$error = '';
$context = $this->getContext();
$user = $context->getUser();
$status = $this->getWikiPage()->doDeleteArticleReal( $reason, $user, $suppress, null, $error );
if ( $status->isOK() ) {
$deleted = $this->getTitle()->getPrefixedText();
$outputPage->setPageTitle( $this->msg( 'actioncomplete' ) );
$outputPage->setRobotPolicy( 'noindex,nofollow' );
if ( $status->isGood() ) {
$loglink = '[[Special:Log/delete|' . $this->msg( 'deletionlog' )->text() . ']]';
$outputPage->addWikiMsg( 'deletedtext', wfEscapeWikiText( $deleted ), $loglink );
$this->getHookRunner()->onArticleDeleteAfterSuccess( $this->getTitle(), $outputPage );
} else {
$outputPage->addWikiMsg( 'delete-scheduled', wfEscapeWikiText( $deleted ) );
}
$outputPage->returnToMain();
} else {
$outputPage->setPageTitle( $this->msg( 'cannotdelete-title', $this->getTitle()->getPrefixedText() ) );
if ( $error === '' ) {
$outputPage->wrapWikiTextAsInterface(
'error mw-error-cannotdelete',
$status->getWikiText( false, false, $context->getLanguage() )
);
$this->showLogEntries( $this->getTitle() );
} else {
$outputPage->addHTML( $error );
}
}
$this->watchlistManager->setWatch( $request->getCheck( 'wpWatch' ), $context->getAuthority(), $title );
}

View file

@ -91,7 +91,8 @@ class FileDeleteAction extends DeleteAction {
$request = $context->getRequest();
if ( !FileDeleteForm::haveDeletableFile( $file, $this->oldFile, $this->oldImage ) ) {
$checkFile = $this->oldFile ?: $file;
if ( !$checkFile->exists() || !$checkFile->isLocal() ) {
$outputPage->addHTML( $this->prepareMessage( 'filedelete-nofile' ) );
$outputPage->addReturnTo( $this->title );
return;

View file

@ -1795,7 +1795,11 @@ class Article implements Page {
}
/**
* Perform a deletion and output success or failure messages
* Perform a deletion and output success or failure messages.
*
* @deprecated since 1.37 Use WikiPage::doDeleteArticleReal if you only need to delete the article. If you also need
* things to happen with OutputPage, you may want to check the hooks in DeleteAction instead.
*
* @param string $reason
* @param bool $suppress
* @param bool $immediate false allows deleting over time via the job queue