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

[Swagger] Do not tag a sub-resource operation with child resource #1612

Closed
wants to merge 1 commit into from

Conversation

lyrixx
Copy link
Contributor

@lyrixx lyrixx commented Dec 27, 2017

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

Before:

image

After:

image

@dunglas
Copy link
Member

dunglas commented Dec 27, 2017

Can you give more insight? Wouldn't it be more logicial to only tag the example with Answer (as it's what it returns?

@lyrixx
Copy link
Contributor Author

lyrixx commented Dec 27, 2017

Actually, I opened this PR to get some feedback. I'm really not sure about this one ;)

But for me it seems more logical to gather all root "resources" together as displayed on the screenshot.

What do you think?

@dunglas
Copy link
Member

dunglas commented Jan 10, 2018

I don't know what to think about this one. Both behaviors looks "logical" to me... Any feedback @api-platform/core-team?

@soyuka
Copy link
Member

soyuka commented Jan 10, 2018

It all comes down to #1617 (comment) (same problematic). To me both are valid:

  1. On the subresource "initiator" (ex Question because route starts with Question)
  2. On the class we retrieve (ex Answer, because we receive an Answer)

I'd even search for it in Answer in the first place because it returns a collection of Answer not a Question.

@Toflar
Copy link
Contributor

Toflar commented Feb 14, 2018

This looks way more logical to me as well 👍 Listing the same thing twice only confuses first-time API users who will go like "wait, I read this already".

@meyerbaptiste
Copy link
Member

IMO, /api/questions/{id}/answer should only be tagged with the Answer tag. Indeed, I want to deal with an Answer resource so I go to the Answer section.

@soyuka
Copy link
Member

soyuka commented Mar 12, 2018

IMO, /api/questions/{id}/answer should only be tagged with the Answer tag. Indeed, I want to deal with an Answer resource so I go to the Answer section.

Okay for swagger. Though, were should be the subresource configuration (see api-platform/api-platform#529). To me it'd be easier to have it on the "Initiator" class.

@Simperfit
Copy link
Contributor

@lyrixx could you please update this according to the discussion ?

@lyrixx
Copy link
Contributor Author

lyrixx commented Apr 6, 2018

Sure, But I don't know when. I should dedicate some time to work on API-Platform ;)

@Simperfit
Copy link
Contributor

@lyrixx when you can :p

@soyuka
Copy link
Member

soyuka commented Apr 7, 2019

Closing in favor of #2706

@soyuka soyuka closed this Apr 7, 2019
@lyrixx lyrixx deleted the subresource branch April 7, 2019 09:17
Copy link

@Wyze5280 Wyze5280 left a comment

Choose a reason for hiding this comment

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

...

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.

7 participants