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

Highlight the role of the :id binding in cowboy route #75

Merged
merged 2 commits into from
Mar 8, 2017
Merged

Highlight the role of the :id binding in cowboy route #75

merged 2 commits into from
Mar 8, 2017

Conversation

lucafavatella
Copy link
Contributor

@lucafavatella lucafavatella commented Mar 7, 2017

Does the name of the parameter in the Swagger schema e.g. this need to be called id e.g. in case cowboy_swagger enforces it?

  • If not, examples here and here would benefit from clarification on that;
  • I did not test that, and I do not plan to test the Swagger handler for the moment.

Please refer to the commit message for details.

See also inaka/sumo_db#294

* https://github.com/inaka/sumo_rest/blob/0.3.3/src/sr_state.erl#L42

  * Function spec may be further detailed, but that would require type
    spec and new internal type (for mandatory key);

  * Having a field and exported function for id is enough.

* https://github.com/inaka/sumo_rest/blob/0.3.3/src/sr_single_entity_handler.erl#L43

  * It already comments on the :id element of cowboy route clearly.

* https://github.com/inaka/sumo_rest/blob/0.3.3/src/sr_request.erl#L18

  * Added internal type for documentation purpose.

* https://github.com/inaka/sumo_db/blob/0.7.1/src/sumo_internal.erl#L88

  * See inaka/sumo_db#294

* https://github.com/inaka/sumo_rest/blob/0.3.3/test/sr_test/sr_single_session_handler.erl#L60
  and
  https://github.com/inaka/sumo_rest/blob/0.3.3/test/sr_test/sr_single_element_handler.erl#L66

  * They already mention `:id` clearly;

  * The only source of confusion may be whether the name of the
    parameter in the Swagger schema
    e.g. [this](https://github.com/inaka/sumo_rest/blob/0.3.3/test/sr_test/sr_single_element_handler.erl#L33)
    needed to be called `id` e.g. in case cowboy_swagger enforces it.

* https://github.com/inaka/sumo_rest/blob/0.3.3/README.md

  * It already mentions `:id` clearly.
@lucafavatella lucafavatella changed the title [WIP] Highlight the role of the :id binding in cowboy route Highlight the role of the :id binding in cowboy route Mar 8, 2017
@elbrujohalcon
Copy link
Member

@lucafavatella you are correct. The entity id must be called :id if present in the path.
I opened issue #78 to improve that in future versions.

@elbrujohalcon
Copy link
Member

I'll merge this PR and we can deal with having different binding fields in #78

@elbrujohalcon elbrujohalcon merged commit 2623318 into inaka:master Mar 8, 2017
@lucafavatella lucafavatella deleted the id branch March 9, 2017 11:19
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.

None yet

2 participants