Skip to content
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

[WIP] Fix api blueprint format #1

Merged
merged 23 commits into from
Apr 3, 2018
Merged

[WIP] Fix api blueprint format #1

merged 23 commits into from
Apr 3, 2018

Conversation

FabienChaynes
Copy link

@FabienChaynes FabienChaynes commented Feb 8, 2018

I'm keeping this PR inside the FinalCAD organization for now because it's not completely ready yet and I'll add comments related to Finalcloud.
Feel free to make a PR against https://github.com/zipmark/rspec_api_documentation when it'll be finished.

The goal of this PR is to fix the rspec_api_documentation gem regarding the generation of documentation targeting the API Blueprint format.
Apiary was used to spot the errors in the generated file.

The API Blueprint Language Specification is available here. I targeted format 1A.

Here's what was fixed so far:

Some errors remain when we generate documentation with these fixes:

  • duplicate definition of 'Content-Type' header (Fixed in b57259d)
  • parameter 'foo' is not found within the URI template '/bar/baz' for 'Route name'

Both are warnings. The second one is more important than the first one because the parameters are not correctly described.

  1. The "duplicate definition" one shouldn't be too complicated. It occurs because the media type is defined both in the Resource Model Section and in the Headers. For example: (Fixed in b57259d)
+ Request Get Work Element (application/json)

    + Headers

            Accept: application/x-finalcad-api-v3.6+json
            Content-Type: application/json
            Host: rollout.finalcad.fr
  1. The "parameter 'foo' is not found within the URI template" one is more complicated. In all the other formats of rspec_api_documentation, query parameters are described by using the parameter method, be it an url parameter (for example id in /users/:id) or a parameter sent in the request body.
    But for the API Blueprint format, there's a distinction. URL parameters are described with the parameter method but body parameters are described using the attribute method.
    If we use the parameter method to describe an attribute, we have a warning because we describe a parameter which is not present in the URL.
    This means that in our spec on finalcloud, we'll have to change all the description of attributes using the parameter method and replace it by the attribute one (there's no difference between both methods signatures so we'll just have to replace it where we need and keep parameter when it's an URL parameter).
    The problem in the gem is that it only sends parameters described with the parameter method and not the other ones.
    The following code will work and send {"team": {"id": "d2988ee2-23b1-4573-8724-e2e8b94b8c84"}} in the body.
route "api/projects/:project_id/teams.json", "Create Team" do
    post "Create Team" do
      parameter :id, "Uuid of the Team (uuid)", required: true, scope: :team

      let(:id) { 'd2988ee2-23b1-4573-8724-e2e8b94b8c84' }
# ...

But if we replace parameter by attribute, the body will be empty.
To fix this, we'll need to allow attributes to be added to the body when they're defined with the attribute method. We might also need to consider them as parameters in the other formats, otherwise they might not be displayed.

@FabienChaynes
Copy link
Author

I had to switch from path segment expansion to simple string expansion for the URLs because the former is not supported by Apiary.

FTR here's a file generated with the modified version of the gem.

@FabienChaynes
Copy link
Author

I couldn’t figure out how to fix the attribute thing. If I had to guess I’d say that it’s around RspecApiDocumentation::DSL::Resource.attribute and RspecApiDocumentation::DSL::Endpoint::Params#call but I’m not sure.

@yld yld merged commit 74e731b into master Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants