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

fix: description of identifier parameter #6178

Merged
merged 1 commit into from Feb 29, 2024
Merged

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
53 of 55 checks passed
@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.

None yet

3 participants