477715 - [W-1.4.*] Collection Management Scaffolding

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Collections, defect)

Description

[Justin Scott [:fligtar]]

Scaffolding: Each collection can have its name, description, and directory listing checkbox modified. Additional users can also be given Owner or Publisher roles to each collection. Collections can be deleted.

Comment 1

[Ryan Doherty (:rdoherty)]

Attachment: sanity patch

Comment 2

[Les Orchard [:lorchard]]

Comment on attachment 362803 [details] [diff] [review]
sanity patch

Patch applied fine, and the new user roles stuff answers some questions I was about to ask about the API.

But, I had some problems:

* In site/app/config/sql/remora.sql, a constraint is removed from the end of the CREATE TABLE for collections.  There's a trailing comma after INDEX (`listed`) that needs to be removed, causing a syntax error otherwise.

* In the User model, the getCollections() method throws an undefined variable warning for $collections when $collectionIds is empty, because it only gets defined inside that conditional.  

* Probably related, there's also an invalid argument warning (not an array) in a foreach in site/app/views/collections/index.thtml in the "Your Collections" section.

* When a user creates a collection, they should probably be added as admin (at least) and as a subscriber (maybe) automatically.

* Adding a non-existent user to a collection throws a SQL error.  Error checking needed eventually.

* Subscribing with an already-subscribed user throws a SQL error.  Error checking needed eventually.

* I can't unsubscribe from a collection.

* I can't delete a collection.

* As with other patches, I had to dig around in the controller to discover the methods - eg. no links to add / edit collections.  It's just scaffolding, but it's hard to get to the important parts from the index.  Might be a hurdle for Briks as they develop the extension using the scaffold and API.

Comment 3

[Ryan Doherty (:rdoherty)]

(In reply to comment #2)
> (From update of attachment 362803 [details] [diff] [review])
> Patch applied fine, and the new user roles stuff answers some questions I was
> about to ask about the API.
> 
> But, I had some problems:
 
Will fix them and have a new patch later today.

Comment 4

[Les Orchard [:lorchard]]

Also, looking at the collections table, you removed the user_id column in favor of roles but I still need it to supply a "creator" field for the collection in the API.

Comment 5

[Les Orchard [:lorchard]]

Hmm, though I suppose I could just do a join and look up the user with a role of COLLECTION_ROLE_OWNER.

Comment 6

[Ryan Doherty (:rdoherty)]

(In reply to comment #5)
> Hmm, though I suppose I could just do a join and look up the user with a role
> of COLLECTION_ROLE_OWNER.

Yeah, I was about to say that :)

Comment 7

[Les Orchard [:lorchard]]

Also, you probably want to escape / validate the variables included in SQL in the collection.php model - they look SQL-injection ready.  I haven't tried it yet, but I suspect I could pull a Bobby Tables with the collection ID passed in via the URL.

Comment 8

[Ryan Doherty (:rdoherty)]

Attachment: Sanity patch

New patch with fixes and functionality requested.

SQL injection isn't possible as input is sanitized in controller. The SQL errors for subscriptions and adding users I'm not going to bother with as this is just scaffolding.

Comment 9

[Les Orchard [:lorchard]]

Comment on attachment 362971 [details] [diff] [review]
Sanity patch

Hmm...  Applied the patch fine, and everything works well except one last thing:

How do I add/remove addons in a collection?  Or are we just doing that in the API for now?  (I could be confused)

Comment 10

[Ryan Doherty (:rdoherty)]

(In reply to comment #9)
> (From update of attachment 362971 [details] [diff] [review])
> Hmm...  Applied the patch fine, and everything works well except one last
> thing:
> 
> How do I add/remove addons in a collection?  Or are we just doing that in the
> API for now?  (I could be confused)

On the add-on view page, in the right column there is a section 'Add to a collection:' that allows you to do this.

Comment 11

[Ryan Doherty (:rdoherty)]

r22382, will file IT bug for SQL updates

Comment 12

[Les Orchard [:lorchard]]

Cool - I can add addons! r++

Comment 13

[Justin Scott [:fligtar]]

Thanks Ryan and Les! Looking forward to playing with this.

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; scaffolding's in, and we've been filing bugs as we test.