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

Open any API URL with your browser and a nice UI shows up #726

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Sep 5, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

This PR enhances the whole Developper eXperience of API Platform by integrating Swagger UI more deeply:

capture d ecran 2016-09-05 a 17 31 19

- Any request having an `Accept` header with HTML set as the preferred format (usually a browser) or to an URL ending with `.html` now displays Swagger UI - The result of the request in the default format of the API (usually JSON-LD) is shown in Swagger UI - The doc of the requested endpoint is opened (docs for all other endpoints are collapsed) and the browser scrolls automatically to it

Alternate formats available for the current document are also directly listed from this page and can be downloaded from the browser:
capture d ecran 2016-09-05 a 17 33 02

Last but not least, if the request format is set to HTML (usually, a browser) and an exception occurs, the default Symfony's exception page is displayed. In all other cases, our JSON error mechanism is used to display the error serialized in the API Problem format or the Hydra format depending of the requested MIME type.
capture d ecran 2016-09-05 a 17 34 23

@dunglas dunglas changed the title Enhance the doc system Open any API URL with your browser and a nice UI shows up Sep 5, 2016
@@ -26,7 +26,10 @@ public function onKernelException(GetResponseForExceptionEvent $event)
{
$request = $event->getRequest();
// Normalize exceptions only for routes managed by API Platform
if (!$request->attributes->has('_api_resource_class') && !$request->attributes->has('_api_respond')) {
if (
(!$request->attributes->has('_api_resource_class') && !$request->attributes->has('_api_respond')) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

null === ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The has method returns a boolean.

Copy link
Contributor

@Simperfit Simperfit Sep 5, 2016

Choose a reason for hiding this comment

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

Oh, I didn't see that you used has here, why are you using get up here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, has should be used in the other class.

@dunglas dunglas merged commit 16c7a7e into api-platform:master Sep 6, 2016
@dunglas dunglas deleted the doc branch September 6, 2016 12:34
@teohhanhui
Copy link
Contributor

I have really mixed feelings about this one.

@dunglas
Copy link
Member Author

dunglas commented Sep 6, 2016

@teohhanhui why? I was just thinking about adding a config flag for this feature.

@teohhanhui
Copy link
Contributor

Nobody really expects HTML from an API. Yes, the browser asking for it, but
who actually wants that from an API? For example, it makes browser
extensions for JSON viewing useless.

But more importantly, it's wrong on principle. Showing the documentation
instead of a resource (or collection) - does that really count as just a
different representation? I think it has crossed the line.

On Wed, 7 Sep 2016, 04:11 Kévin Dunglas, notifications@github.com wrote:

@teohhanhui https://github.com/teohhanhui why? I was just thinking
about adding a config flag for this feature.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#726 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhf61A2o1NwO0JEjYRUuG_vYzMCn_tpks5qncj7gaJpZM4J1K9H
.

@dunglas
Copy link
Member Author

dunglas commented Sep 6, 2016

It's more a developper tool (and especially a client developper tool) than a real representation of the resource. As you pointed out, it allows to interact with the API even if you don't have Postman or a similar tool installed. It also allows to discover the API (and API Platform) with only a browser (and even a smartphone), be aware quickly of available endpoints, formats and features...

Django REST as a somewhat similar feature and it's pleasant to use: http://restframework.herokuapp.com/

But I got your point, it's borderline regarding the REST philosophy (some may argue that this is a real HTML representation of the resource, the data is technically displayed as HTML). WDYT about keeping this feature on by default but having a config option to easily disable it?

@soyuka
Copy link
Member

soyuka commented Sep 6, 2016

WDYT about keeping this feature on by default but having a config option to easily disable it?

It's necessary to be able to disable it, from my point of view ofc. I'd have it disabled by default but I also understand that it's an attractive feature which is great to have with zero configuration.

@teohhanhui
Copy link
Contributor

I can't pass judgement on the DX until I've actually used it (probably soon).

@teohhanhui
Copy link
Contributor

Just tried it. The DX leaves much to be desired. There is an additional delay which is really annoying, also Swagger UI is not very helpful for discovery, but most importantly it prevents tools from working.

@teohhanhui
Copy link
Contributor

teohhanhui commented Sep 7, 2016

But on the other hand, disabling it is already possible by setting the api_platform.formats config to not include html. (not so easy)

magarzon pushed a commit to magarzon/core that referenced this pull request Feb 12, 2017
Open any API URL with your browser and a nice UI shows up
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