[katello-devel] On the plane, I tried 2 controllers worth of API changes

[Tomas Strachota]

On 07/09/2012 10:01 AM, Bryan Kearney wrote:
> someone should really check the pull request i sent in, since I was not
> able to really test. Here were some notes I took while playing with the
> code:

I went through the code. Most of your notes are obvious bugs but in some 
cases I'm not sure about the best solution. Ideas from experienced 
RESTers are more than welcome!

>
> changesets_content_controller
> =============================
>
> * entire api controller returns text instead of json.
> * passing in a non existent product id still gets you a 200 return code.
> (look at render_after_removal)
> * Looks like the posts take in query parameters instead of JSON in the
> body. Any reason for that?
> * For remove_* is seems nasty to have to pass in the product id. Dont we
> just know this since it is in the changeset already?
I'm not sure if we can get rid of product ids. We add packages, errata, 
repos and distros from a scope of product. That means we can 
theoretically have two packages with the same id in a changeset twice, 
each from a different product.
Using id of ChangesetPackage record instead of ids of the packages 
itself could be a solution here. (the same applies to errata, distros 
and repos)

> * For URLS like this:
>
>    api :DELETE, "/changesets/:changeset_id/templates/:id", "
>
> why use :changeset_id and then :id (as opposed to template_id"
+1 it makes code more readable

>
> Changeset_controller
> ====================
> * Update and create seems to do json, which is good
> * is organization_id ever checked?
I don't think so. We should shorten the route from
POST /organizations/:organization_id/environments/:environment_id/changesets
to
POST /environments/:environment_id/changesets

> * Why is name used in #index
> * Promote seems icky. We are POSTING to /promote which is more of an RPC
> call. Can we do a POST to another environment instead?
Agree. I'm not sure how you meant post to another env though. I'd maybe 
suggest to create a promotion controller.

> * The promotion API automatically moves the changeset to REVIEW. Should
> we instead make that a unique call?
An unique call would be definitely better. It shouldn't be a big deal to 
change it.

T.