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

[ApiLoader] Support _format resolving #4292

Merged
merged 4 commits into from
May 26, 2021
Merged

[ApiLoader] Support _format resolving #4292

merged 4 commits into from
May 26, 2021

Conversation

ahmed-bhs
Copy link
Contributor

@ahmed-bhs ahmed-bhs commented May 25, 2021

Q A
Branch? main
Bug fix? no, yes
New feature? no, yes
BC breaks? no
Deprecations? no
Tickets #There's no ticket
License MIT

Hello there,

Currently, it is not possible to resolve the default value for the _format parameter under a custom operation definition.
This is due to the ApiLoader which forces this value to null, no matter what value we set in the configuration.
This fix, makes it possible to take into account the definition of the defaults._format parameter and have the right value under the request route params.

Use case:
Imagine that we have a resource which is used to export both CSV and XLSX data.

resources.yaml:

resources:
    App\Entity\Activity\Contract:
        collectionOperations:
            export:
                method: 'GET'
                path: /contracts/export.{_format}
                defaults:
                    _format: "xlsx"
               requirements:
                   _format: "xlsx|csv"
                controller: App\Controller\Api\ExportContractController

ExportContractController:

final class ExportContractController extends AbstractController
{
    public function __invoke($_format, $data)
    {
    // before the fix, $_format return null
    dd($_format);
    // after the fix, $_format return 'xlsx'
    
    }
}

@ahmed-bhs ahmed-bhs changed the title [ApiLoader] Support _format reading [ApiLoader] Support _format resolving May 25, 2021
@alanpoulain
Copy link
Member

Seems alright to me. Could you add a CHANGELOG entry and if possible a PR for the documentation?

@soyuka soyuka merged commit 287b79c into api-platform:main May 26, 2021
@soyuka
Copy link
Member

soyuka commented May 26, 2021

thanks @ahmed-bhs !

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.

4 participants