Jhernandez has uploaded a new change for review.
https://gerrit.wikimedia.org/r/190260
Change subject: Hygiene: models\Collection shouldn't use stores\Collection
......................................................................
Hygiene: models\Collection shouldn't use stores\Collection
Just as stores\CollectionsList (and subclasses) create and manages
models\CollectionsList, stores\Collection now creates and manages
a models\Collection.
That way:
* Models are data objects only using other models.
* Stores are concerned with side effects, and loading data from places into
models or plain data structures.
Change-Id: Idf5ebc2da568e270bb89f8ba52fa951b7bc1c630
---
M includes/models/Collection.php
M includes/specials/SpecialGather.php
M includes/stores/Collection.php
M includes/stores/DumbWatchlistOnlyCollectionsList.php
M includes/stores/WatchlistCollection.php
5 files changed, 44 insertions(+), 48 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Gather
refs/changes/60/190260/1
diff --git a/includes/models/Collection.php b/includes/models/Collection.php
index 5cb14f3..37e3d9c 100644
--- a/includes/models/Collection.php
+++ b/includes/models/Collection.php
@@ -50,11 +50,12 @@
protected $public;
/**
+ * @param int $id id of the collection. Null if not persisted yet
* @param User $user User that owns the collection
* @param string $title Title of the collection
* @param string $description Description of the collection
*/
- public function __construct( User $user, $title = '', $description =
'', $public = true ) {
+ public function __construct( $id = null, User $user, $title = '',
$description = '', $public = true ) {
$this->owner = $user;
$this->title = $title;
$this->description = $description;
@@ -75,6 +76,17 @@
*/
public function add( CollectionItem $item ) {
$this->items[] = $item;
+ }
+
+ /**
+ * Adds an array of items to the collection
+ *
+ * @param CollectionItem[] $items list of items to add
+ */
+ public function batch( $items ) {
+ foreach ( $items as $item ) {
+ $this->add( $item );
+ }
}
/**
@@ -159,19 +171,6 @@
*/
public function getItems() {
return $this->items;
- }
-
- /**
- * Adds an array of titles to the collection
- *
- * @param stores\Collection $store
- */
- public function load( stores\Collection $store ) {
- $this->id = $store->getId();
- $items = $store->getItems();
- foreach ( $items as $item ) {
- $this->add( $item );
- }
}
}
diff --git a/includes/specials/SpecialGather.php
b/includes/specials/SpecialGather.php
index b97de75..3d92e84 100644
--- a/includes/specials/SpecialGather.php
+++ b/includes/specials/SpecialGather.php
@@ -70,22 +70,19 @@
* @param int $id collection id
*/
public function renderUserCollection( User $user, $id ) {
- $collection = new models\Collection(
- $user,
- $this->msg( 'gather-watchlist-title' ),
- $this->msg( 'gather-watchlist-description' )
- );
// Watchlist lives at id 0
if ( (int)$id === 0 ) {
- // Watchlist is private
- $collection->setPublic( false );
if ( $this->isOwner( $user ) ) {
- $collection->load( new
stores\WatchlistCollection( $user ) );
+ $watchlistStore = new
stores\WatchlistCollection( $user );
+ $this->render( new views\Collection(
$watchlistStore->getCollection() ) );
+ } else {
+ // FIXME: No permissions to visit this. Showing
not found ATM.
+ $this->renderNotFoundError();
}
+ } else {
+ // FIXME: Not rendering other collections yet. Render
not found ATM.
+ $this->renderNotFoundError();
}
- // FIXME: For empty-collection and not-allowed-to-see-this we
are doing the
- // same thing right now.
- $this->render( new views\Collection( $collection ) );
}
/**
diff --git a/includes/stores/Collection.php b/includes/stores/Collection.php
index a3f46c8..2d03183 100644
--- a/includes/stores/Collection.php
+++ b/includes/stores/Collection.php
@@ -7,16 +7,9 @@
*/
interface Collection {
/**
- * Get CollectionItem of all pages in the current collection.
+ * Get Collection model of the current collection.
*
- * @return CollectionItem[] titles
+ * @return Collection collection
*/
- public function getItems();
-
- /**
- * Get current collection identifier
- *
- * @return int id
- */
- public function getId();
+ public function getCollection();
}
diff --git a/includes/stores/DumbWatchlistOnlyCollectionsList.php
b/includes/stores/DumbWatchlistOnlyCollectionsList.php
index d67f661..e540253 100644
--- a/includes/stores/DumbWatchlistOnlyCollectionsList.php
+++ b/includes/stores/DumbWatchlistOnlyCollectionsList.php
@@ -24,14 +24,8 @@
// Get watchlist collection (private)
// Directly avoid adding if no privates
if ( $this->includePrivate ) {
- $watchlist = new models\Collection(
- $this->user,
- wfMessage( 'gather-watchlist-title' ),
- wfMessage( 'gather-watchlist-description' ),
- false
- );
- $watchlist->load( new WatchlistCollection( $this->user
) );
- $collections[] = $watchlist;
+ $watchlistStore = new WatchlistCollection( $this->user
);
+ $collections[] = $watchlistStore->getCollection();
}
return $collections;
}
diff --git a/includes/stores/WatchlistCollection.php
b/includes/stores/WatchlistCollection.php
index 35602a4..ac40dfe 100644
--- a/includes/stores/WatchlistCollection.php
+++ b/includes/stores/WatchlistCollection.php
@@ -14,15 +14,15 @@
*/
class WatchlistCollection implements Collection {
/**
- * @var models\CollectionItem[]
+ * @var models\Collection
*/
- protected $items = array();
+ protected $collection;
/**
* @inheritdoc
*/
- public function getItems() {
- return $this->items;
+ public function getCollection() {
+ return $this->collection;
}
/**
@@ -39,13 +39,26 @@
* @param User $user to lookup watchlist members for
*/
public function __construct( User $user ) {
+ // Load the different data we need
$titles = $this->loadTitles( $user );
$extracts = ItemExtracts::loadExtracts( $titles );
$images = ItemImages::loadImages( $titles );
+ // Merge the data into models\CollectionItem
+ $items = array();
foreach ( $titles as $key=>$title ) {
- $this->items[] = new models\CollectionItem( $title,
$images[$key], $extracts[$key] );
+ $items[] = new models\CollectionItem( $title,
$images[$key], $extracts[$key] );
}
+
+ // Construct the internal models\Collection
+ $this->collection = new models\Collection(
+ $this->getId(),
+ $user,
+ wfMessage( 'gather-watchlist-title' ),
+ wfMessage( 'gather-watchlist-description' ),
+ false
+ );
+ $this->collection->batch( $items );
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/190260
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idf5ebc2da568e270bb89f8ba52fa951b7bc1c630
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Gather
Gerrit-Branch: master
Gerrit-Owner: Jhernandez <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits