silverstripe / addons.silverstripe.org

Website hosting Silverstripe Framework extensions
BSD 3-Clause "New" or "Revised" License
13 stars 16 forks source link

Update Supported Addons API to include expanded support data #221

Open Cheddam opened 5 years ago

Cheddam commented 5 years ago

Following on from #220, we should expose the additional support data in the output of the Supported Addons API. This would be a breaking change in schema, so may require versioning the API endpoint.

robbieaverill commented 5 years ago

This would be a breaking change in schema, so may require versioning the API endpoint.

Are you referring to https://addons.silverstripe.org/api/supported-addons here? If so, I see what you mean - changing from an array to an object would be breaking, and adding a new key to the schema may not be the cleanest solution.

Not trying to say we shouldn't follow a good API design practice by versioning it, but maybe we could just add a new API endpoint instead?

We could redirect /api/rating(s) to /api/v1/rating(s) to prevent it from breaking, OR we could just add the new endpoints to /api and continue without versioning it (feel free to kick me when I get home for suggesting this if you want to).

robbieaverill commented 5 years ago

Also FYI we use justinrainbow/json-schema in cow for JSON schema definition - seems to work well

Cheddam commented 5 years ago

"Versioning the endpoint" and "adding a /v2/ endpoint" are the same thing in my mind - I should have been more explicit about what I was suggesting, sorry!

I like your suggested endpoints - some of them aren't necessary to cover the scope of this issue, but representing supported addons as a subset of all addons certainly makes sense to me.

I think most HTTP clients will gracefully handle a 301 for the existing endpoints if we shift them to use a /v1/ prefix, and from a cleanliness angle it would be ideal, but it might not be necessary as long as the new endpoints are under /v2/.

ScopeyNZ commented 5 years ago

We could do a GraphQL endpoint 🤔

Cheddam commented 5 years ago

Why not both?

robbieaverill commented 5 years ago

We can use GraphQL now!

chillu commented 5 years ago

Thinking of working on this during Hackday. It's somewhat on the path to identifying issues mentioned in our Helpdesk, for the upcoming Experience Debt work.

robbieaverill commented 5 years ago

@chillu FYI if you do you will need to add in most of the supported modules again (it's a checkbox on the Addon record in the CMS) since we lost the DB a couple of months ago. We haven't gotten around to re-adding them all yet.