jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/395666 )
Change subject: Discard unused metadata from schema ...................................................................... Discard unused metadata from schema Drop columns for color, image, icon. Bug: T180092 Change-Id: Icc712cb8abc025b992ed25afbf4f67d4e67386f9 --- M i18n/en.json M i18n/qqq.json A sql/patches/04-drop-metadata-columns.sql M sql/readinglists.sql M src/Api/ApiQueryReadingLists.php M src/Api/ApiReadingListsCreate.php M src/Api/ApiReadingListsUpdate.php M src/Doc/ReadingListRow.php M src/HookHandler.php M src/ReadingListRepository.php M tests/src/ReadingListRepositoryTest.php 11 files changed, 13 insertions(+), 129 deletions(-) Approvals: jenkins-bot: Verified Mholloway: Looks good to me, approved diff --git a/i18n/en.json b/i18n/en.json index f3acfe8..ce2fbaa 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -62,18 +62,11 @@ "apihelp-readinglists+create-extended-description": "The user must have fewer than $1 (non-deleted) {{PLURAL:$1|list|lists}}.", "apihelp-readinglists+create-param-name": "List name.", "apihelp-readinglists+create-param-description": "List description.", - "apihelp-readinglists+create-param-color": "List color. (The meaning and range of values is left to the clients.)", - "apihelp-readinglists+create-param-image": "List image. (The meaning and format of values is left to the clients.)", - "apihelp-readinglists+create-param-icon": "List icon. (The meaning and format of values is left to the clients.)", "apihelp-readinglists+create-example-1": "Create a new reading list.", - "apihelp-readinglists+create-example-2": "Create a new reading list with custom appearance.", "apihelp-readinglists+update-summary": "Update a list belonging to the current user.", "apihelp-readinglists+update-param-list": "List ID.", "apihelp-readinglists+update-param-name": "New list name.", "apihelp-readinglists+update-param-description": "New list description.", - "apihelp-readinglists+update-param-color": "New list color.", - "apihelp-readinglists+update-param-image": "New list image.", - "apihelp-readinglists+update-param-icon": "New list icon.", "apihelp-readinglists+update-example-1": "Change the name of the reading list with ID <kbd>42</kbd>.", "apihelp-readinglists+delete-summary": "Delete a list belonging to the current user.", "apihelp-readinglists+delete-extended-description": "Deleted lists remain available for some amount of time through the <kbd>[[Special:ApiHelp/query+readinglists|readinglists]]</kbd> and <kbd>[[Special:ApiHelp/query+readinglistentries|readinglistentries]]</kbd> modules (via the <var>$1changedsince</var> parameter). There is no way to undelete.", diff --git a/i18n/qqq.json b/i18n/qqq.json index b79480e..878682b 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -63,18 +63,11 @@ "apihelp-readinglists+create-extended-description": "{{doc-apihelp-extended-description|readinglists+create|params=* $1 - Maximum number of lists per user.|paramstart=2}}", "apihelp-readinglists+create-param-name": "{{doc-apihelp-param|readinglists+create|name}}", "apihelp-readinglists+create-param-description": "{{doc-apihelp-param|readinglists+create|description}}", - "apihelp-readinglists+create-param-color": "{{doc-apihelp-param|readinglists+create|color}}", - "apihelp-readinglists+create-param-image": "{{doc-apihelp-param|readinglists+create|image}}", - "apihelp-readinglists+create-param-icon": "{{doc-apihelp-param|readinglists+create|icon}}", "apihelp-readinglists+create-example-1": "{{doc-apihelp-example|readinglists+create}}", - "apihelp-readinglists+create-example-2": "{{doc-apihelp-example|readinglists+create}}", "apihelp-readinglists+update-summary": "{{doc-apihelp-summary|readinglists+update}}", "apihelp-readinglists+update-param-list": "{{doc-apihelp-param|readinglists+update|list}}", "apihelp-readinglists+update-param-name": "{{doc-apihelp-param|readinglists+update|name}}", "apihelp-readinglists+update-param-description": "{{doc-apihelp-param|readinglists+update|description}}", - "apihelp-readinglists+update-param-color": "{{doc-apihelp-param|readinglists+update|color}}", - "apihelp-readinglists+update-param-image": "{{doc-apihelp-param|readinglists+update|image}}", - "apihelp-readinglists+update-param-icon": "{{doc-apihelp-param|readinglists+update|icon}}", "apihelp-readinglists+update-example-1": "{{doc-apihelp-example|readinglists+update}}", "apihelp-readinglists+delete-summary": "{{doc-apihelp-summary|readinglists+delete}}", "apihelp-readinglists+delete-extended-description": "{{doc-apihelp-extended-description|readinglists+delete}}", diff --git a/sql/patches/04-drop-metadata-columns.sql b/sql/patches/04-drop-metadata-columns.sql new file mode 100644 index 0000000..e3a29eb --- /dev/null +++ b/sql/patches/04-drop-metadata-columns.sql @@ -0,0 +1,4 @@ +ALTER TABLE reading_list + DROP COLUMN rl_color, + DROP COLUMN rl_image, + DROP COLUMN rl_icon; diff --git a/sql/readinglists.sql b/sql/readinglists.sql index 11ced2e..bd61d74 100644 --- a/sql/readinglists.sql +++ b/sql/readinglists.sql @@ -13,12 +13,6 @@ rl_name VARCHAR(255) BINARY NOT NULL, -- Description of the list. rl_description VARBINARY(767) NOT NULL DEFAULT '', - -- List color as 3x2 hex digits. - rl_color VARBINARY(6) DEFAULT NULL, - -- List image as file name to pass to wfFindFile() or the like. - rl_image VARBINARY(255) DEFAULT NULL, - -- List icon. - rl_icon VARBINARY(32) DEFAULT NULL, -- Creation timestamp. rl_date_created BINARY(14) NOT NULL default '19700101000000', -- Last modification timestamp. diff --git a/src/Api/ApiQueryReadingLists.php b/src/Api/ApiQueryReadingLists.php index cd024c8..2e3fd1d 100644 --- a/src/Api/ApiQueryReadingLists.php +++ b/src/Api/ApiQueryReadingLists.php @@ -163,9 +163,6 @@ 'name' => $row->rl_name, 'default' => (bool)$row->rl_is_default, 'description' => $row->rl_description, - 'color' => $row->rl_color, - 'image' => $row->rl_image, - 'icon' => $row->rl_icon, 'created' => wfTimestamp( TS_ISO_8601, $row->rl_date_created ), 'updated' => wfTimestamp( TS_ISO_8601, $row->rl_date_updated ), ]; diff --git a/src/Api/ApiReadingListsCreate.php b/src/Api/ApiReadingListsCreate.php index f4f9925..7d4c9af 100644 --- a/src/Api/ApiReadingListsCreate.php +++ b/src/Api/ApiReadingListsCreate.php @@ -26,7 +26,7 @@ $params = $this->extractRequestParams(); $listId = $this->getReadingListRepository( $this->getUser() )->addList( $params['name'], - $params['description'], $params['color'], $params['image'], $params['icon'] ); + $params['description'] ); $this->getResult()->addValue( null, $this->getModuleName(), [ 'id' => $listId ] ); } @@ -45,21 +45,6 @@ self::PARAM_TYPE => 'string', self::PARAM_DFLT => '', self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_description'], - ], - 'color' => [ - self::PARAM_TYPE => 'string', - self::PARAM_DFLT => '', - self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_color'], - ], - 'image' => [ - self::PARAM_TYPE => 'string', - self::PARAM_DFLT => '', - self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_image'], - ], - 'icon' => [ - self::PARAM_TYPE => 'string', - self::PARAM_DFLT => '', - self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_icon'], ], ]; } @@ -91,9 +76,6 @@ return [ 'action=readinglists&command=create&name=dogs&description=Woof!&token=123ABC' => 'apihelp-readinglists+create-example-1', - 'action=readinglists&command=create&name=dogs&description=Woof!' - . '&color=brown&image=File:Australien_Kelpie.jpg&icon=File:Icon dog.gif&token=123ABC' - => 'apihelp-readinglists+create-example-2', ]; } diff --git a/src/Api/ApiReadingListsUpdate.php b/src/Api/ApiReadingListsUpdate.php index 8c81f3d..1e2508a 100644 --- a/src/Api/ApiReadingListsUpdate.php +++ b/src/Api/ApiReadingListsUpdate.php @@ -23,10 +23,10 @@ */ public function execute() { $params = $this->extractRequestParams(); - $this->requireAtLeastOneParameter( $params, 'name', 'description', 'color', 'image', 'icon' ); + $this->requireAtLeastOneParameter( $params, 'name', 'description' ); $this->getReadingListRepository( $this->getUser() )->updateList( $params['list'], - $params['name'], $params['description'], $params['color'], $params['image'], $params['icon'] ); + $params['name'], $params['description'] ); } /** @@ -46,18 +46,6 @@ 'description' => [ self::PARAM_TYPE => 'string', self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_description'], - ], - 'color' => [ - self::PARAM_TYPE => 'string', - self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_color'], - ], - 'image' => [ - self::PARAM_TYPE => 'string', - self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_image'], - ], - 'icon' => [ - self::PARAM_TYPE => 'string', - self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rl_icon'], ], ]; } diff --git a/src/Doc/ReadingListRow.php b/src/Doc/ReadingListRow.php index 43b1915..4c19ae3 100644 --- a/src/Doc/ReadingListRow.php +++ b/src/Doc/ReadingListRow.php @@ -32,15 +32,6 @@ /** @var string Description of the list. */ public $rl_description; - /** @var string List color as 3x2 hex digits. */ - public $rl_color; - - /** @var string List image as file name to pass to wfFindFile() or the like. */ - public $rl_image; - - /** @var string List icon. */ - public $rl_icon; - /** @var string Creation timestamp. */ public $rl_date_created; diff --git a/src/HookHandler.php b/src/HookHandler.php index c73d078..5ef6357 100644 --- a/src/HookHandler.php +++ b/src/HookHandler.php @@ -49,6 +49,8 @@ "$patchDir/02-add-reading_list_project.sql" ); $updater->addExtensionIndex( 'reading_list', 'rl_user_deleted_name_id', "$patchDir/03-add-sort-indexes.sql" ); + $updater->dropExtensionField( 'reading_list', 'rl_color', + "$patchDir/04-drop-metadata-columns.sql" ); } return true; } diff --git a/src/ReadingListRepository.php b/src/ReadingListRepository.php index dda6e72..f541074 100644 --- a/src/ReadingListRepository.php +++ b/src/ReadingListRepository.php @@ -46,9 +46,6 @@ public static $fieldLength = [ 'rl_name' => 255, 'rl_description' => 767, - 'rl_color' => 6, - 'rl_image' => 255, - 'rl_icon' => 32, 'rlp_project' => 255, 'rle_title' => 255, ]; @@ -125,9 +122,6 @@ 'rl_is_default' => 1, 'rl_name' => 'default', 'rl_description' => '', - 'rl_color' => '', - 'rl_image' => '', - 'rl_icon' => '', 'rl_date_created' => $this->dbw->timestamp(), 'rl_date_updated' => $this->dbw->timestamp(), 'rl_deleted' => 0, @@ -197,19 +191,13 @@ * Create a new list. * @param string $name * @param string $description - * @param string $color - * @param string $image - * @param string $icon * @return int The ID of the new list * @throws ReadingListRepositoryException */ - public function addList( $name, $description = '', $color = '', $image = '', $icon = '' ) { + public function addList( $name, $description = '' ) { $this->assertUser(); $this->assertFieldLength( 'rl_name', $name ); $this->assertFieldLength( 'rl_description', $description ); - $this->assertFieldLength( 'rl_color', $color ); - $this->assertFieldLength( 'rl_image', $image ); - $this->assertFieldLength( 'rl_icon', $icon ); if ( !$this->isSetupForUser( self::READ_LOCKING ) ) { throw new ReadingListRepositoryException( 'readinglists-db-error-not-set-up' ); } @@ -225,9 +213,6 @@ 'rl_is_default' => 0, 'rl_name' => $name, 'rl_description' => $description, - 'rl_color' => $color, - 'rl_image' => $image, - 'rl_icon' => $icon, 'rl_date_created' => $this->dbw->timestamp(), 'rl_date_updated' => $this->dbw->timestamp(), 'rl_deleted' => 0, @@ -283,22 +268,14 @@ * @param int $id * @param string|null $name * @param string|null $description - * @param string|null $color - * @param string|null $image - * @param string|null $icon * @return void * @throws ReadingListRepositoryException * @throws LogicException */ - public function updateList( - $id, $name = null, $description = null, $color = null, $image = null, $icon = null - ) { + public function updateList( $id, $name = null, $description = null ) { $this->assertUser(); $this->assertFieldLength( 'rl_name', $name ); $this->assertFieldLength( 'rl_description', $description ); - $this->assertFieldLength( 'rl_color', $color ); - $this->assertFieldLength( 'rl_image', $image ); - $this->assertFieldLength( 'rl_icon', $icon ); $row = $this->selectValidList( $id, self::READ_LOCKING ); if ( $row->rl_is_default ) { throw new ReadingListRepositoryException( 'readinglists-db-error-cannot-update-default-list' ); @@ -307,9 +284,6 @@ $data = array_filter( [ 'rl_name' => $name, 'rl_description' => $description, - 'rl_color' => $color, - 'rl_image' => $image, - 'rl_icon' => $icon, 'rl_date_updated' => $this->dbw->timestamp(), ], function ( $field ) { return $field !== null; @@ -749,9 +723,6 @@ 'rl_is_default', 'rl_name', 'rl_description', - 'rl_color', - 'rl_image', - 'rl_icon', 'rl_date_created', 'rl_date_updated', 'rl_deleted', diff --git a/tests/src/ReadingListRepositoryTest.php b/tests/src/ReadingListRepositoryTest.php index fcd1c5c..2d71fae 100644 --- a/tests/src/ReadingListRepositoryTest.php +++ b/tests/src/ReadingListRepositoryTest.php @@ -142,15 +142,11 @@ 'rl_user_id' => '1', 'rl_name' => 'foo', 'rl_description' => '', - 'rl_color' => '', - 'rl_image' => '', - 'rl_icon' => '', 'rl_is_default' => '0', 'rl_deleted' => '0', ], $data ); - $listId = $repository->addList( 'bar', 'here is some bar', 'blue', - 'fake://example.png', 'fake://example_icon.png' ); + $listId = $repository->addList( 'bar', 'here is some bar' ); /** @var ReadingListRow $row */ $row = $this->db->selectRow( 'reading_list', '*', [ 'rl_id' => $listId ] ); $this->assertTimestampEquals( wfTimestampNow(), $row->rl_date_created ); @@ -161,9 +157,6 @@ 'rl_user_id' => '1', 'rl_name' => 'bar', 'rl_description' => 'here is some bar', - 'rl_color' => 'blue', - 'rl_image' => 'fake://example.png', - 'rl_icon' => 'fake://example_icon.png', 'rl_is_default' => '0', 'rl_deleted' => '0', ], $data ); @@ -247,9 +240,6 @@ 'default' => [ 'rl_name' => 'default', 'rl_description' => '', - 'rl_color' => '', - 'rl_image' => '', - 'rl_icon' => '', 'rl_is_default' => '1', 'rl_date_created' => wfTimestampNow(), 'rl_date_updated' => wfTimestampNow(), @@ -258,9 +248,6 @@ 'foo' => [ 'rl_name' => 'foo', 'rl_description' => '', - 'rl_color' => '', - 'rl_image' => '', - 'rl_icon' => '', 'rl_is_default' => '0', 'rl_date_created' => '20100101000000', 'rl_date_updated' => '20120101000000', @@ -269,9 +256,6 @@ 'foo_2' => [ 'rl_name' => 'foo', 'rl_description' => 'this is the second foo', - 'rl_color' => '', - 'rl_image' => '', - 'rl_icon' => '', 'rl_is_default' => '0', 'rl_date_created' => '20170101000000', 'rl_date_updated' => '20170101000000', @@ -280,9 +264,6 @@ 'bar' => [ 'rl_name' => 'bar', 'rl_description' => '', - 'rl_color' => '', - 'rl_image' => '', - 'rl_icon' => '', 'rl_is_default' => '0', 'rl_date_created' => '20010101000000', 'rl_date_updated' => '20020101000000', @@ -336,9 +317,6 @@ 'rl_description' => 'xxx', 'rl_date_created' => '20100101000000', 'rl_date_updated' => '20120101000000', - 'rl_color' => 'blue', - 'rl_image' => 'image', - 'rl_icon' => 'ICON', 'rl_deleted' => '0', ], [ @@ -346,9 +324,6 @@ 'rl_description' => 'yyy', 'rl_date_created' => '20100101000000', 'rl_date_updated' => '20120101000000', - 'rl_color' => 'blue', - 'rl_image' => 'image', - 'rl_icon' => 'ICON', 'rl_deleted' => '1', ], ] ); @@ -358,20 +333,14 @@ $row = $this->db->selectRow( 'reading_list', '*', [ 'rl_id' => $listId ] ); $this->assertEquals( 'bar', $row->rl_name ); $this->assertEquals( 'xxx', $row->rl_description ); - $this->assertEquals( 'blue', $row->rl_color ); - $this->assertEquals( 'image', $row->rl_image ); - $this->assertEquals( 'ICON', $row->rl_icon ); $this->assertTimestampEquals( '20100101000000', $row->rl_date_created ); $this->assertTimestampEquals( wfTimestampNow(), $row->rl_date_updated ); - $repository->updateList( $listId, 'bar', 'yyy', 'red', 'img', 'ico' ); + $repository->updateList( $listId, 'bar', 'yyy' ); /** @var ReadingListRow $row */ $row = $this->db->selectRow( 'reading_list', '*', [ 'rl_id' => $listId ] ); $this->assertEquals( 'bar', $row->rl_name ); $this->assertEquals( 'yyy', $row->rl_description ); - $this->assertEquals( 'red', $row->rl_color ); - $this->assertEquals( 'img', $row->rl_image ); - $this->assertEquals( 'ico', $row->rl_icon ); $this->assertFailsWith( 'readinglists-db-error-no-such-list', function () use ( $repository ) { -- To view, visit https://gerrit.wikimedia.org/r/395666 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Icc712cb8abc025b992ed25afbf4f67d4e67386f9 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/ReadingLists Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org> Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org> Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org> Gerrit-Reviewer: Siebrand <siebr...@kitano.nl> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits