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
