Skip to content

Conversation

Timu57
Copy link
Contributor

@Timu57 Timu57 commented Feb 23, 2024

I think it is an unexprected behaviour that resource description and summary take the shortName parameter into consideration when generating the OpenApi documentation but the identifier parameter description doesn't.

Q A
Branch? current stable version
Tickets -
License MIT
Doc PR -

@Timu57 Timu57 changed the title Fixed description of identifier parameter Fix description of identifier parameter Feb 23, 2024
@Timu57 Timu57 changed the title Fix description of identifier parameter fix: description of identifier parameter Feb 23, 2024
@priyadi
Copy link
Contributor

priyadi commented Feb 24, 2024

This PR has the same goal as mine in #6155 . But I think @soyuka wants to address this in a more fundamental or comprehensive way.

@soyuka
Copy link
Member

soyuka commented Feb 29, 2024

@priyadi as this doesn't execute the $this->resourceMetadataFactory->create($uriVariable->getFromClass()); I'm more willing into merging this for now then yours I hope that's okay. This is definitely better then the actual code and a good temporary fix.

@soyuka soyuka merged commit 4138cb7 into api-platform:3.2 Feb 29, 2024
@soyuka
Copy link
Member

soyuka commented Feb 29, 2024

thanks!

@priyadi
Copy link
Contributor

priyadi commented Feb 29, 2024

@priyadi as this doesn't execute the $this->resourceMetadataFactory->create($uriVariable->getFromClass()); I'm more willing into merging this for now then yours I hope that's okay. This is definitely better then the actual code and a good temporary fix.

This one probably won't fix cases like this:

#[ApiResource(
    shortName: 'User/Review',
    routePrefix: '/user',
    operations: [
        // ...
        new Get(
            uriTemplate: '/books/{bookId}/reviews/{id}',
            uriVariables: [
                'bookId' => new Link(
                    fromClass: BookDto::class,
                )
            ],
        ),
        // ...
    ]
)]

But I have no problem as this one should fix the majority of the cases. Should probably test that case first, just to make sure it doesn't generate a wrong documentation.

@soyuka
Copy link
Member

soyuka commented Feb 29, 2024

indeed I definitely need to add this to the #5995 scope

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