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

Support request media type configuration for an API service. #102

Merged
merged 3 commits into from Jun 25, 2015

Conversation

nebolsin
Copy link
Contributor

Service can specify several media types which it can consume as an input.

 ---
  basePath: ''
  description: ''
  name: Lurker Demo Application
  domains: {}
  extensions: {}
  consumes:
  - application/x-www-form-urlencoded
  - application/json

Generated browsable HTML documentation allows user to select from the consumable media types and generate requests and cURL templates appropriately.

If no consumes key is present in the service config, then it's assumed to be application/x-www-form-urlencoded.

:label => "#{print_labels(parent_labels)}#{label}",
:label_text => "#{print_labels(parent_labels)}#{label}",
:accessor => "#{accessors.compact.join('.')}",
:label => "#{print_labels(accessors)}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary spacing detected.

@razum2um
Copy link
Owner

@nebolsin

  1. avoid commiting lib/lurker/templates/public/application.css and lib/lurker/templates/public/application.js - I precompile them by myself before release
  2. squash it into a single commit, please

},
handleSubmit: function() {
// FIXME
Lurker.onSubmit(jQuery('#payload'));
Lurker.disableSubmitButton();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason, (besides convenience aka no need to precompile lurker.js for developing that) for extracting Lurker.onSubmit & Lurker.onComplete when we can pass requestMediaType (or state) as parameter?

Let me clarify the things: I'd like to keep this js embedded into erb as small as possible and would be glad to move code backwards, from here to js as much as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to pass action, payload, requestMediaType and method. And it looks a little bit dirty to me. I don't like passing state either, since it's an internal implementation detail of the React component. Finally, since request and handler callback are only used from that React component it seemed logical to me to move the code to the component itself.

If you think that it's better to move this code out of the component, I can certainly do that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the code certainly belongs to the component, but nowadays Lurker component in js it all about this form, as well.
I think we cannot eliminate passing variables from ruby to js and these 2 parts will continue to live side-by-side.
we probably can inline all the detectContentType, fillInInfoTab into here, but I really dislike the idea about such amount of js embedded in erb. as such I suggest:

@nebolsin nebolsin force-pushed the feature/json-request-format branch 3 times, most recently from 44ebf73 to a45bd24 Compare June 24, 2015 16:44
return results.join(' ');
},

performRequest: function(host, method, template, values, payload, contentType) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long.

@nebolsin nebolsin force-pushed the feature/json-request-format branch from a45bd24 to 5240801 Compare June 24, 2015 17:04
switch (Lurker.detectContentType(xhr)) {
case "json":
var json = JSON.stringify(JSON.parse(xhr.responseText), null, 2);
var content = hljs.highlightAuto(json).value;
content = hljs.highlightAuto(json).value;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'hljs' is not defined.

@nebolsin nebolsin force-pushed the feature/json-request-format branch 3 times, most recently from 1ac5419 to fb1ab0a Compare June 24, 2015 21:36
@nebolsin nebolsin force-pushed the feature/json-request-format branch from fb1ab0a to b19d89c Compare June 25, 2015 00:56
@Strech Strech added this to the 0.6.8 milestone Jun 25, 2015
razum2um added a commit that referenced this pull request Jun 25, 2015
Support request media type configuration for an API service.
@razum2um razum2um merged commit f046743 into razum2um:master Jun 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants