01tonythomas has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/385796 )
Change subject: Refactor newsletter create-udpate codebase ...................................................................... Refactor newsletter create-udpate codebase * Avoid code repition * Cut down huge ugly functions Bug: T178743 Change-Id: I067c52036cb20b7a35f7530b1aac0807b2e6ecdb --- M includes/Newsletter.php M includes/NewsletterEditPage.php M includes/content/NewsletterDataUpdate.php 3 files changed, 88 insertions(+), 65 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Newsletter refs/changes/96/385796/1 diff --git a/includes/Newsletter.php b/includes/Newsletter.php index 7c19753..c9bebcb 100644 --- a/includes/Newsletter.php +++ b/includes/Newsletter.php @@ -248,4 +248,25 @@ public function canRestore( User $user ) { return $this->isPublisher( $user ) || $user->isAllowed( 'newsletter-restore' ); } + + /** + * Notify new publishers + * + * @param array $added + * @param User $addedBy + * + */ + public function notifyNewPublishers( array $added, User $addedBy ) { + EchoEvent::create( + [ + 'type' => 'newsletter-newpublisher', + 'extra' => [ + 'newsletter-name' => $this->getName(), + 'new-publishers-id' => $added, + 'newsletter-id' => $this->getId() + ], + 'agent' => $addedBy + ] + ); + } } diff --git a/includes/NewsletterEditPage.php b/includes/NewsletterEditPage.php index 8cb8793..d39a25c 100644 --- a/includes/NewsletterEditPage.php +++ b/includes/NewsletterEditPage.php @@ -96,6 +96,7 @@ * @param int $undoId * @param int $oldId * @return HTMLForm + * @throws BadRequestError */ protected function getManageForm( $revId, $undoId, $oldId ) { $publishers = UserArray::newFromIDs( $this->newsletter->getPublishers() ); @@ -416,17 +417,7 @@ } if ( $added ) { - EchoEvent::create( - [ - 'type' => 'newsletter-newpublisher', - 'extra' => [ - 'newsletter-name' => $this->newsletter->getName(), - 'new-publishers-id' => $added, - 'newsletter-id' => $newsletterId - ], - 'agent' => $user - ] - ); + $this->newsletter->notifyNewPublishers( $added, $user ); } foreach ( $removed as $ruId ) { diff --git a/includes/content/NewsletterDataUpdate.php b/includes/content/NewsletterDataUpdate.php index 945f00f..61c84d8 100644 --- a/includes/content/NewsletterDataUpdate.php +++ b/includes/content/NewsletterDataUpdate.php @@ -26,64 +26,85 @@ $this->title = $title; } - function doUpdate() { - $logger = LoggerFactory::getInstance( 'newsletter' ); + private function getNewsletterLogger() { + return LoggerFactory::getInstance( 'newsletter' ); + } + protected function getNewslettersWithNewsletterMainPage( $newNewsletterName ) { + $dbr = wfGetDB( DB_REPLICA ); + return $dbr->selectRowCount( + 'nl_newsletters', + [ 'nl_name', 'nl_main_page_id', 'nl_active' ], + $dbr->makeList( [ + 'nl_name' => $newNewsletterName, + $dbr->makeList( + [ + 'nl_main_page_id' => $this->content->getMainPage()->getArticleID(), + 'nl_active' => 1 + ], LIST_AND ) + ], LIST_OR ) + ); + } + + protected function createANewNewsletterWithData( NewsletterStore $store, $formData ) { + $newNewsletterName = $formData['Name']; + if ( $this->getNewslettersWithNewsletterMainPage( $newNewsletterName ) ) { + return false; + } + + $validator = new NewsletterValidator( $formData ); + $validation = $validator->validate( true ); + + if ( !$validation->isGood() ) { + // Invalid input was entered + return $validation; + } + + $title = Title::makeTitleSafe( NS_NEWSLETTER, $newNewsletterName ); + $newsletter = new Newsletter( 0, + $title->getText(), + $formData['Description'], + $formData['MainPage']->getArticleID() + ); + + $newsletterCreated = $store->addNewsletter( $newsletter ); + if ( !$newsletterCreated ) { + return false; + } + + $newsletter->subscribe( $this->user ); + $store->addPublisher( $newsletter, $this->user ); + + return $newsletter; + } + + + function doUpdate() { + $logger = $this->getNewsletterLogger(); $store = NewsletterStore::getDefaultInstance(); // We might have a situation when the newsletter is not created yet. Hence, we should add // that to the database, and exit. $newsletter = Newsletter::newFromName( $this->title->getText() ); + $formData = [ + 'Name' => $this->title->getText(), + 'Description' => $this->content->getDescription(), + 'MainPage' => $this->content->getMainPage() + ]; + if ( !$newsletter ) { // Possible API edit to create a new newsletter, and the newsletter is not in the // database yet. - $this->name = $this->title->getText(); - $dbr = wfGetDB( DB_REPLICA ); - $rows = $dbr->select( - 'nl_newsletters', - [ 'nl_name', 'nl_main_page_id', 'nl_active' ], - $dbr->makeList( [ - 'nl_name' => $this->name, - $dbr->makeList( - [ - 'nl_main_page_id' => $this->content->getMainPage()->getArticleID(), - 'nl_active' => 1 - ], LIST_AND ) - ], LIST_OR ) - ); - - // Check whether another existing newsletter has the same name or main page - foreach ( $rows as $row ) { - if ( $row->nl_name === $this->name ) { - $logger->warning( 'newsletter-exist-error', $this->name ); - return; - } elseif ( (int)$row->nl_main_page_id === $this->content->getMainPage()->getArticleID() - && (int)$row->nl_active === 1 - ) { - $logger->warning( 'newsletter-mainpage-in-use' ); - return; - } - } - $title = Title::makeTitleSafe( NS_NEWSLETTER, $this->name ); - $newsletter = new Newsletter( 0, - $title->getText(), - $this->content->getDescription(), - $this->content->getMainPage()->getArticleID() - ); - $newsletterCreated = $store->addNewsletter( $newsletter ); - if ( $newsletterCreated ) { - $newsletter->subscribe( $this->user ); - $store->addPublisher( $newsletter, $this->user ); - return; - } else { + $newsletter = $this->createANewNewsletterWithData( $store, $formData ); + if ( !$newsletter ) { // Couldn't insert to the DB.. $logger->warning( 'newsletter-create-error' ); return; } } - // This was a possible edit to an existing newsletter. $newsletterId = $newsletter->getId(); + if ( $this->content->getDescription() != $newsletter->getDescription() ) { $store->updateDescription( $newsletterId, $this->content->getDescription() ); } @@ -102,7 +123,6 @@ $updatedPublishersIds[] = $user->getId(); } } - // Do the actual modifications now $added = array_diff( $updatedPublishersIds, $oldPublishersIds ); $removed = array_diff( $oldPublishersIds, $updatedPublishersIds ); @@ -113,18 +133,9 @@ } if ( $added ) { - EchoEvent::create( - [ - 'type' => 'newsletter-newpublisher', - 'extra' => [ - 'newsletter-name' => $newsletter->getName(), - 'new-publishers-id' => $added, - 'newsletter-id' => $newsletterId - ], - 'agent' => $this->user - ] - ); + $newsletter->notifyNewPublishers( $added, $this->user ); } + foreach ( $removed as $ruId ) { $store->removePublisher( $newsletter, User::newFromId( $ruId ) ); } -- To view, visit https://gerrit.wikimedia.org/r/385796 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I067c52036cb20b7a35f7530b1aac0807b2e6ecdb Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Newsletter Gerrit-Branch: master Gerrit-Owner: 01tonythomas <01tonytho...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits