Glaisher has uploaded a new change for review.
https://gerrit.wikimedia.org/r/235437
Change subject: Update Newsletter DB schema
......................................................................
Update Newsletter DB schema
* Add table prefixes to fields to avoid ambiguity when doing joins
and also per MediaWiki conventions
* Remove foreign key constraints because it's not supported by MyISAM
* Add nls_subscriber_id index to allow for doing queries to specific users
* Add unique nl_name index for order/where queries
* Add nlp_nl_id index to query all publishers for a newsletter
Change-Id: I4b6512285f30759c2ede9c26314071303e4499c6
---
M includes/NewsletterDb.php
M includes/specials/pagers/NewsletterManageTablePager.php
M includes/specials/pagers/NewsletterTablePager.php
M sql/nl_issues.sql
M sql/nl_newsletters.sql
M sql/nl_publishers.sql
M sql/nl_subscriptions.sql
7 files changed, 61 insertions(+), 54 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Newsletter
refs/changes/37/235437/1
diff --git a/includes/NewsletterDb.php b/includes/NewsletterDb.php
index d71e44b..d2116b7 100644
--- a/includes/NewsletterDb.php
+++ b/includes/NewsletterDb.php
@@ -26,8 +26,8 @@
*/
public function addSubscription( $userId, $newsletterId ) {
$rowData = array(
- 'newsletter_id' => $newsletterId,
- 'subscriber_id' =>$userId,
+ 'nls_newsletter_id' => $newsletterId,
+ 'nls_subscriber_id' =>$userId,
);
try{
return $this->writeDb->insert( 'nl_subscriptions',
$rowData, __METHOD__ );
@@ -44,8 +44,8 @@
*/
public function removeSubscription( $userId, $newsletterId ) {
$rowData = array(
- 'newsletter_id' => $newsletterId,
- 'subscriber_id' => $userId,
+ 'nls_newsletter_id' => $newsletterId,
+ 'nls_subscriber_id' => $userId,
);
return $this->writeDb->delete( 'nl_subscriptions', $rowData,
__METHOD__ );
}
@@ -56,17 +56,18 @@
* @return int[]
*/
public function getUserIdsSubscribedToNewsletter( $newsletterId ) {
+ // @todo use selectFieldValues() here
$res = $this->readDb->select(
'nl_subscriptions',
- array( 'subscriber_id' ),
- array( 'newsletter_id' => $newsletterId ),
+ array( 'nls_subscriber_id' ),
+ array( 'nls_newsletter_id' => $newsletterId ),
__METHOD__,
array()
);
$subscriberIds = array();
foreach ( $res as $row ) {
- $subscriberIds[] = $row->subscriber_id;
+ $subscriberIds[] = $row->nls_subscriber_id;
}
return $subscriberIds;
}
@@ -79,8 +80,8 @@
*/
public function addPublisher( $userId, $newsletterId ) {
$rowData = array(
- 'newsletter_id' => $newsletterId,
- 'publisher_id' =>$userId,
+ 'nlp_newsletter_id' => $newsletterId,
+ 'nlp_publisher_id' =>$userId,
);
try{
return $this->writeDb->insert( 'nl_publishers',
$rowData, __METHOD__ );
@@ -97,8 +98,8 @@
*/
public function removePublisher( $userId, $newsletterId ) {
$rowData = array(
- 'newsletter_id' => $newsletterId,
- 'publisher_id' => $userId,
+ 'nlp_newsletter_id' => $newsletterId,
+ 'nlp_publisher_id' => $userId,
);
return $this->writeDb->delete( 'nl_publishers', $rowData,
__METHOD__ );
}
@@ -184,10 +185,10 @@
$res = $this->readDb->select(
array( 'nl_publishers', 'nl_newsletters' ),
array( 'nl_id', 'nl_name', 'nl_desc',
'nl_main_page_id', 'nl_frequency', 'nl_owner_id' ),
- array( 'publisher_id' => $user->getId() ),
+ array( 'nlp_publisher_id' => $user->getId() ),
__METHOD__,
array(),
- array( 'nl_newsletters' => array( 'LEFT JOIN',
'nl_id=newsletter_id' ) )
+ array( 'nl_newsletters' => array( 'LEFT JOIN',
'nl_id=nlp_publisher_id' ) )
);
return $this->getNewslettersFromResult( $res );
@@ -236,16 +237,16 @@
//Note: the writeDb is used as this is used in the next insert
$lastIssueId = $this->writeDb->selectRowCount(
'nl_issues',
- array( 'issue_id' ),
- array( 'issue_newsletter_id' => $newsletterId ),
+ array( 'nli_issue_id' ),
+ array( 'nli_newsletter_id' => $newsletterId ),
__METHOD__
);
-
+ // @todo should probably AUTO INCREMENT here
$rowData = array(
- 'issue_id' => $lastIssueId + 1,
- 'issue_page_id' => $pageId,
- 'issue_newsletter_id' => $newsletterId,
- 'issue_publisher_id' => $publisherId,
+ 'nli_issue_id' => $lastIssueId + 1,
+ 'nli_page_id' => $pageId,
+ 'nli_newsletter_id' => $newsletterId,
+ 'nli_publisher_id' => $publisherId,
);
try{
return $this->writeDb->insert( 'nl_issues', $rowData,
__METHOD__ );
diff --git a/includes/specials/pagers/NewsletterManageTablePager.php
b/includes/specials/pagers/NewsletterManageTablePager.php
index 7a8c4b1..0a8a1ac 100644
--- a/includes/specials/pagers/NewsletterManageTablePager.php
+++ b/includes/specials/pagers/NewsletterManageTablePager.php
@@ -14,8 +14,8 @@
public function getFieldNames() {
if ( $this->fieldNames === null ) {
$this->fieldNames = array(
- 'newsletter_id' => $this->msg(
'newsletter-manage-header-name' )->text(),
- 'publisher_id' => $this->msg(
'newsletter-manage-header-publisher' )->text(),
+ 'nl_id' => $this->msg(
'newsletter-manage-header-name' )->text(),
+ 'nlp_publisher_id' => $this->msg(
'newsletter-manage-header-publisher' )->text(),
'permissions' => $this->msg(
'newsletter-manage-header-permissions' )->text(),
'action' => $this->msg(
'newsletter-manage-header-action' )->text(),
);
@@ -27,12 +27,12 @@
return array(
'tables' => array( 'nl_publishers', 'nl_newsletters' ),
'fields' => array(
- 'newsletter_id',
- 'publisher_id',
- 'is_owner' => 'publisher_id = nl_owner_id',
+ 'nl_id',
+ 'nlp_publisher_id',
+ 'is_owner' => 'nlp_publisher_id = nl_owner_id',
),
'join_conds' => array(
- 'nl_newsletters' => array( 'LEFT JOIN',
'newsletter_id = nl_id' ),
+ 'nl_newsletters' => array( 'LEFT JOIN',
'nlp_newsletter_id = nl_id' ),
),
);
}
@@ -41,11 +41,12 @@
static $previous;
switch ( $field ) {
- case 'newsletter_id':
+ case 'nl_id':
if ( $previous === $value ) {
return null;
} else {
+ // @todo should be retrieved as a batch
$dbr = wfGetDB( DB_SLAVE );
$res = $dbr->select(
'nl_newsletters',
@@ -62,7 +63,7 @@
return $newsletterName;
}
- case 'publisher_id' :
+ case 'nlp_publisher_id' :
$user = User::newFromId( $value );
return $user->getName();
@@ -89,16 +90,16 @@
return $radioOwner . $radioPublisher;
case 'action' :
- $isCurrentUser =
$this->mCurrentRow->publisher_id == $this->getUser()->getId();
+ $isCurrentUser =
$this->mCurrentRow->nlp_publisher_id == $this->getUser()->getId();
if ( !$this->mCurrentRow->is_owner &&
!$isCurrentUser ) {
return HTML::element(
'input',
array(
'type' => 'button',
- 'value' => 'Remove',
+ 'value' => 'Remove', //
@todo needs i18n
'name' => $previous,
- 'id' =>
$this->mCurrentRow->publisher_id,
+ 'id' =>
$this->mCurrentRow->nlp_publisher_id,
)
);
}
@@ -113,7 +114,7 @@
}
public function getDefaultSort() {
- return 'newsletter_id';
+ return 'nl_id';
}
public function isFieldSortable( $field ) {
diff --git a/includes/specials/pagers/NewsletterTablePager.php
b/includes/specials/pagers/NewsletterTablePager.php
index fab47ff..b77cfc0 100644
--- a/includes/specials/pagers/NewsletterTablePager.php
+++ b/includes/specials/pagers/NewsletterTablePager.php
@@ -3,6 +3,7 @@
/**
* @license GNU GPL v2+
* @author Tina Johnson
+ * @todo Optimize queries here
*/
class NewsletterTablePager extends TablePager {
@@ -32,8 +33,8 @@
'nl_name',
'nl_desc',
'nl_id',
- 'subscribers' => ( '( SELECT COUNT(*) FROM
nl_subscriptions WHERE newsletter_id = nl_id )' ),
- 'current_user_subscribed' => "$userId IN
(SELECT subscriber_id FROM nl_subscriptions WHERE newsletter_id = nl_id )" ,
+ 'subscribers' => ( '( SELECT COUNT(*) FROM
nl_subscriptions WHERE nls_newsletter_id = nl_id )' ),
+ 'current_user_subscribed' => "$userId IN
(SELECT nls_subscriber_id FROM nl_subscriptions WHERE nls_newsletter_id = nl_id
)" ,
),
'options' => array( 'DISTINCT nl_id' ),
);
@@ -44,6 +45,7 @@
public function formatValue( $field, $value ) {
switch ( $field ) {
case 'nl_name':
+ // @todo do batch queries instead of separate
queries for each row
$dbr = wfGetDB( DB_SLAVE );
$res = $dbr->select(
'nl_newsletters',
diff --git a/sql/nl_issues.sql b/sql/nl_issues.sql
index b425b6f..8b73978 100644
--- a/sql/nl_issues.sql
+++ b/sql/nl_issues.sql
@@ -1,11 +1,11 @@
-- Database schema for creating nl_issues table.
CREATE TABLE /*_*/nl_issues(
- issue_id int unsigned NOT NULL,
- issue_page_id int NOT NULL,
+ nli_issue_id int unsigned NOT NULL,
+ nli_page_id int NOT NULL,
-- Foreign key referenced from nl_newsletters
- issue_newsletter_id int REFERENCES nl_newsletters(nl_id),
- issue_publisher_id int NOT NULL,
+ nli_newsletter_id int,
+ nli_publisher_id int NOT NULL,
-- Composite primary key
- PRIMARY KEY (issue_id, issue_newsletter_id)
-)/*$wgDBTableOptions*/;
\ No newline at end of file
+ PRIMARY KEY (nli_issue_id, nli_newsletter_id)
+)/*$wgDBTableOptions*/;
diff --git a/sql/nl_newsletters.sql b/sql/nl_newsletters.sql
index d38f7b2..566b777 100644
--- a/sql/nl_newsletters.sql
+++ b/sql/nl_newsletters.sql
@@ -1,13 +1,14 @@
-- Database schema for creating nl_newsletter table.
CREATE TABLE /*_*/nl_newsletters(
- --Primary key
+ -- Primary key
nl_id int unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT,
- nl_name varchar(50) NOT NULL UNIQUE,
- nl_desc varbinary(767),
- nl_main_page_id int NOT NULL UNIQUE,
+ nl_name varchar(64) NOT NULL,
+ nl_desc varbinary(1024),
+ nl_main_page_id int unsigned NOT NULL UNIQUE,
nl_frequency varchar(50) NOT NULL,
nl_owner_id int NOT NULL
)/*$wgDBTableOptions*/;
-CREATE INDEX /*i*/nl_owner_id ON /*_*/nl_newsletters(nl_owner_id);
\ No newline at end of file
+CREATE INDEX /*i*/nl_owner_id ON /*_*/nl_newsletters(nl_owner_id);
+CREATE UNIQUE INDEX /*i*/nl_name ON /*_*/nl_newsletters (nl_name);
diff --git a/sql/nl_publishers.sql b/sql/nl_publishers.sql
index d6cef4b..93e74d5 100644
--- a/sql/nl_publishers.sql
+++ b/sql/nl_publishers.sql
@@ -1,8 +1,9 @@
-- Database schema for creating nl_publishers table.
CREATE TABLE /*_*/nl_publishers(
- --Primary key
- newsletter_id int REFERENCES nl_newsletter(nl_id),
- publisher_id int NOT NULL,
- PRIMARY KEY (publisher_id, newsletter_id)
+ nlp_newsletter_id int unsigned NOT NULL,
+ nlp_publisher_id int NOT NULL,
+ PRIMARY KEY (nlp_publisher_id, nlp_newsletter_id)
)/*$wgDBTableOptions*/;
+
+CREATE INDEX /*i*/nlp_nl_id ON /*_*/nl_publishers(nlp_newsletter_id);
diff --git a/sql/nl_subscriptions.sql b/sql/nl_subscriptions.sql
index 6b982af..4f857ef 100644
--- a/sql/nl_subscriptions.sql
+++ b/sql/nl_subscriptions.sql
@@ -1,9 +1,10 @@
-- Database schema for creating nl_subscriptions table.
CREATE TABLE /*_*/nl_subscriptions(
- -- Foreign key referenced from nl_newsletters
- newsletter_id int REFERENCES nl_newsletters(nl_id),
- subscriber_id int NOT NULL,
+ nls_newsletter_id int NOT NULL,
+ nls_subscriber_id int NOT NULL,
-- Composite primary key
- PRIMARY KEY (newsletter_id, subscriber_id)
-)/*$wgDBTableOptions*/;
\ No newline at end of file
+ PRIMARY KEY (nls_newsletter_id, nls_subscriber_id)
+)/*$wgDBTableOptions*/;
+
+CREATE INDEX /*i*/nls_subscriber_id ON
/*_*/nl_subscriptions(nls_subscriber_id);
--
To view, visit https://gerrit.wikimedia.org/r/235437
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4b6512285f30759c2ede9c26314071303e4499c6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Newsletter
Gerrit-Branch: master
Gerrit-Owner: Glaisher <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
