New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document the OFN API #3001

Open
myriamboure opened this Issue Nov 8, 2018 · 8 comments

Comments

4 participants
@myriamboure
Copy link
Contributor

myriamboure commented Nov 8, 2018

Feature candidate description

https://community.openfoodnetwork.org/t/document-our-api/1498

T-shirt size estimate

@luisramos0 said he would need 5 days, like 30 hours. @Matt-Yorkley @sauloperez @HugsDaniel and Luis agreed on T-shirt sizing "L".

Definition of done / Acceptance criteria

Product team / support people are able to answer requests from users who want to have access to the OFN API and the concerned users are able to understand what is the current API.

Description of the feature including mockups

The feature is basically a wiki page that will document the OFN API. No mockups needed.

Story map

No sub stories, only one issue.

Lead team

Product owner:
Tech owner:

@myriamboure myriamboure added the size M label Nov 8, 2018

@myriamboure myriamboure added this to Ready (stories in delivery b'log) in Product Feature Backlog Nov 8, 2018

@myriamboure myriamboure moved this from Ready (stories in delivery b'log) to New features backlog in Product Feature Backlog Nov 8, 2018

@myriamboure myriamboure moved this from New features backlog to Ready (stories in delivery b'log) in Product Feature Backlog Nov 8, 2018

@myriamboure myriamboure moved this from Ready (stories in delivery b'log) to New features backlog in Product Feature Backlog Nov 8, 2018

@myriamboure myriamboure added size L and removed size M labels Nov 8, 2018

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Jan 11, 2019

there are quite a lot of good links on the thread Myriam linked above, the two first ones are the must reads before you start:

This issue is about having some support docs that help making sense of the output of the command "rake routes".

As I revisit this topic, if I was a student :-) I'd learn Swagger https://swagger.io/tools/swagger-editor/
Actually we may have a case for https://swagger.io/tools/swagger-inspector/
https://swagger.io/docs/swagger-inspector/how-to-create-an-openapi-definition-using-swagger/

So, in terms of output of this story we have:

option 1 is awesomeness, option 2 is easier as you don't have to learn what is swagger and the openAPI.

@kevinchristianson

This comment has been minimized.

Copy link
Contributor

kevinchristianson commented Jan 12, 2019

Thanks for the info, @luisramos0 I think we'll try to learn Swagger and make it there. What's the best way to set it up? Since Swagger hosts the API docs we're wondering if we should create an account on Swagger and then give it to OFN when we're done or if you want to create your own and give us access to it.

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Jan 13, 2019

I think you can use "login with github" and then share access to the project with other accounts.

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Jan 13, 2019

Even if the API gets based on your account it should be easy later to move it around, it's supposed to be easy to export/import.

@sigmundpetersen sigmundpetersen moved this from New features backlog to Inception (scoping) in Product Feature Backlog Jan 16, 2019

@kevinchristianson

This comment has been minimized.

Copy link
Contributor

kevinchristianson commented Jan 19, 2019

@luisramos0 we found a ruby gem called swagger-blocks that uses Swagger UI to create Swagger documentation connected to ruby code. This would allow us to write documentation within classes themselves and then generate Swagger UI webpages from that. We set it up on our fork with some demo classes and it seems to work well. Would you be okay with this approach to the documentation? We see many positives in this approach both for us and you, but it would mean adding another gem to OFN. Let us know what you think and thank again for working with us on this!

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Jan 19, 2019

ah, brilliant!!! you guys rock!
Your suggestion is a great idea!!! API docs in the code help keep the docs up to date but it's a lot of noise in the controllers...

You will not be able to cover the full API unless you automate the process, right? So, automating the docs to get a good coverage is the most important thing here. My main question is thus: would you be able to use Swagger Inspector and generate these for every API controller in both OFN and Spree?

Also, this tool does not cover openAPI v3 which I think is a big drawback, if we use openAPI we should start we the latest version. v3 was released in 2017 (we could use https://www.apimatic.io/transformer to convert to v3).

But I think the main drawback of this approach is OFN specific: there are many API endpoints in Spree that we want to use and document but we do not want to change the controllers code... it's Spree code.

So, what's the advantages you see in using this approach? Why not just use swagger inspector and generate the json outside the code?

@haseleyi

This comment has been minimized.

Copy link
Contributor

haseleyi commented Jan 23, 2019

RE automation: it’s a strong word; these gems operate by generating json from special code within the code. Swagger-blocks does so via added controller code. Another gem that we’ve succeeded setting up is rswag. It’s similar, but has the advantage of keeping documentation in specs. This could help reduce clutter in the controllers. Swagger blocks has an open PR for covering openAPI V3: fotinakis/swagger-blocks#120. That updated two days ago and tests are passing, so between these two gems, rswag has less (zero) controller clutter while swagger-blocks may soon be V3-compatible.

The advantage to this approach is twofold:

  1. Interactivity: people looking to integrate with our API could make actual endpoint calls on an authorized “test” account and see real data come back in the form of json.
  2. Maintainability: Whereas having externally-hosted documentation could complicate changes to code, with an integrated gem, each PR that changes the API would have documentation changes encapsulated within.

That said, there are other gem integrations available: a quick search yields rabble, rails-restapi, rspec_api_documentation, rapi_doc. But you raise a good point: with both ‘APIs’ at play here, can one gem coalesce all their endpoints?

Our knowledge of OFN and Spree’s marriage is failing us a little here. Is the same data behind both? Are both APIs accessible from the OFN repo?

@luisramos0

This comment has been minimized.

Copy link
Contributor

luisramos0 commented Jan 23, 2019

nice, thanks for your input! your list of advantages is good.

By automation I mean to automate the generation of the docs from the existing API without having to review any endpoint. From what I understand with swagger blocks you would have to generate the "special code" in the controllers. if there are 200 endpoints in our controllers (I think they are more), that means you would have to write and review that "special code" for all 200 endpoints.
By automation I mean swagger inspector type of thing where you just run a script and the existing API has some docs ready to be read and improved manually. These tools just call the existing API, document the endpoints, the methods and the results (by parsing real results of calls to the endpoints).

Thanks for raising this topic, after this conversation I am more convinced that this type of tool like swagger blocks is not a good approach for us, because we would end up with a good approach but with a very incomplete description of our API because we would never have the time to document all the endpoints with that "special code". I had a look at this "special code" (it's a DSL) and it's actually a bit complex with many details to be filled in for each endpoint, isn't it?

In terms of spree vs OFN APIs, yes both APIs are available in a OFN instance, including your local instance. If you run "rake routes" locally you will see the list of endpoints and if you follow some of the endpoints to their controllers you will see that some are in OFN, others are in Spree.
Let me give more details about this:

likewise, the default spree api controllers are under app/controllers/spree/api/ (these are overriding these) and the ofn ones are under app/controllers/api/.

I hope these links help!

Have you tried swagger inspector?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment