Skip to content

Conversation

@MeronNagy
Copy link
Contributor

@MeronNagy MeronNagy commented May 6, 2025

Q A
Branch? 4.1
Tickets #7126
License MIT
Doc PR

Currently BackedEnumFilter displays integer backed enums on the docs page but does not filter them.
This is because the $value received by filterProperty is a string and not an integer.
I solved this by adding this short code snippet at the start of the normalizeValue function in the BackedEnumFilterTrait to attempt to convert the value to an integer if the enum is a backed integer enum.

        $firstCase = $this->enumTypes[$property]::cases()[0] ?? null;
        if (
            is_int($firstCase?->value)
            && false !== filter_var($value, FILTER_VALIDATE_INT)
        ) {
            $value = (int) $value;
        }

Alternatively, we could use ReflectionEnum::getBackingType(), but this avoids the reflection overhead.

Example Repo: https://github.com/MeronNagy/api-platform-integer-backed-enum


/**
* @var array<string, string>
* @var array<string, class-string>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe class-string to be more appropriate here, I can revert this if needed.

@MeronNagy MeronNagy force-pushed the backed-enum-filter-support-integer-backed-enums-4.1 branch from d2d120f to 22920f8 Compare May 6, 2025 14:39
@soyuka
Copy link
Member

soyuka commented May 7, 2025

would it be possible to add a test? can be a functional test (there should be some example in tests/Functional) or unit (src/Doctrine/Common/Tests) check CONTRIBUTING.md if you need more informations

@MeronNagy
Copy link
Contributor Author

would it be possible to add a test? can be a functional test (there should be some example in tests/Functional) or unit (src/Doctrine/Common/Tests) check CONTRIBUTING.md if you need more informations

Yes sure I'll add one tomorrow!

@MeronNagy
Copy link
Contributor Author

@soyuka added 2 functional tests, lmk if they are sufficient or require further changes :)

@soyuka soyuka merged commit 4dd0cdf into api-platform:4.1 May 9, 2025
94 checks passed
@soyuka
Copy link
Member

soyuka commented May 9, 2025

many thanks @MeronNagy this will be tagged next week!

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