Closed Bug 997329 Opened 10 years ago Closed 10 years ago

Create a "Make List" API

Categories

(Webmaker Graveyard :: MakeAPI, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: cade, Assigned: cade)

References

Details

(Whiteboard: [tues])

Attachments

(2 files)

This API will enable the creation/curation of ordered lists of Makes.

API:

POST /list - Create a List
{
  name: "List Name",
  makes: [ "ID 1", "ID 2" ],
  private: true|false
}

PUT /list - Update a list
{
  id: <id of make list>,
  data: {
    name: "new name",
    makes: <new array of makes>
  }
}

POST and PUT requests will be aborted if any of the makes specified cannot be found.

GET /list/id/:id - return the list's make data 
GET /list/name/:name - return the list's make data
GET /lists/user/:username - return an array of list ids for a given user (excluding private lists, unless logged in user is the owner or admin)

The get routes will hydrate data by default, and if `?hydrate=false` is set, it will return the raw list description (i.e. Make ID's only). If a make in a list is deleted or becomes unreachable for any reason, it will not be hydrated in GET requests. A message should be provided in the response indicating that some makes could not be found. Any edits to the list will fail until the broken make references are fixed. 

DELETE /list/name/:name - Delete a list
DELETE /list/id/:id - Delete a list

Deleting will be restricted to the list owner and admins.

We'll create a new Model in mongoose. It won't be connected to Elasticsearch. GET requests will hydrate the list with ordered make data by default.
Blocks: 1005905
* Possible to expedite this one? Brett and I would like to use the MakeLister for Toronto work sprint running May 6 to 8

* Chris: what still has to happen to close this?
Flags: needinfo?(cade)
Whiteboard: [tues]
To Do's:

* Enforce list ownership
* clean up cruftyness in my commits
* document the API

Then code review.

I can have this in review before EOD today
Flags: needinfo?(cade)
Blocks: 1006660
Blocks: 1006662
Attachment #8412795 - Flags: review?(jon)
Attachment #8412794 - Flags: review?(jon)
Comment on attachment 8412794 [details] [review]
https://github.com/mozilla/makeapi-client/pull/36

This is pretty well done I think. What do you think of the naming changes?
Attachment #8412794 - Flags: review?(jon) → review-
Comment on attachment 8412795 [details] [review]
https://github.com/mozilla/MakeAPI/pull/213

I don't know if I have a basis in reality for this, but I can't seem to test the MakeList with this patch because it's not setting the webmaker login session. Some incompatibility with Express 4.0 and our webmaker auth stuff?
(In reply to Jon Buckley [:jbuck] from comment #6)
> Comment on attachment 8412795 [details] [review]
> https://github.com/mozilla/MakeAPI/pull/213
> 
> I don't know if I have a basis in reality for this, but I can't seem to test
> the MakeList with this patch because it's not setting the webmaker login
> session. Some incompatibility with Express 4.0 and our webmaker auth stuff?

Make sure the session secret is set properly on MakeLister, and make sure that you whitelist it on the login server. Also, set the LOGIN_URL on your env (with basic auth)

(In reply to Jon Buckley [:jbuck] from comment #5)
> Comment on attachment 8412794 [details] [review]
> https://github.com/mozilla/makeapi-client/pull/36
> 
> This is pretty well done I think. What do you think of the naming changes?

I can do those changes tomorrow.
Attachment #8412794 - Flags: review- → review?(jon)
Both patches updated.

jbuck: I've made significant modifications to the Make Lister (most notable it's now called the Galler Maker) https://github.com/cadecairos/Gallery-Maker - so update that before you test.
And an update on my authentication problems: I cannot login, and Chris cannot logout. Thus, Express is ignoring cookies entirely. I can workaround this problem by logging into another app though.
Comment on attachment 8412794 [details] [review]
https://github.com/mozilla/makeapi-client/pull/36

r+ with one nit noted in the PR
Attachment #8412794 - Flags: review?(jon) → review+
Comment on attachment 8412795 [details] [review]
https://github.com/mozilla/MakeAPI/pull/213

r-, just needs two more things:

* Remove username field from List object
* Add appOwner to List object, and enforce ownership in new or existing middleware
Attachment #8412795 - Flags: review?(jon) → review-
Attachment #8412795 - Flags: review- → review?(jon)
Attachment #8412795 - Flags: review?(jon) → review+
Landed! Shipping to Staging/prod soon.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: