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

[Serializer] Fixed BackedEnumNormalizer priority for translatable enum #54315

Merged

Conversation

IndraGunawan
Copy link
Contributor

@IndraGunawan IndraGunawan commented Mar 17, 2024

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT
enum MyEnum: string implements TranslatableInterface
{
    case Get = 'GET';
    case Post = 'POST';
    case Update = 'UPDATE';

    public function trans(TranslatorInterface $translator, ?string $locale = null): string
    {
        return match ($this) {
            self::Get  => 'custom_get',
            self::Post  => 'custom_post',
            self::Update => 'custom_update'
        };
    }
}
class MyController {
    public function __invoke(SerializerInterface $serializer) {
        dd($serializer->serialize(MyEnum::Get)); // ""custom_get"" , Expected result: ""GET""
    }
}

serialize a BackedEnum that implements TranslatableInterface will return the translation value instead of the enum item value. this is because TranslatableNormalizer (ref) has higher priority than BackedEnumNormalizer

this PR changes the BackedEnumNormalizer priority higher than TranslatableNormalizer priority

@carsonbot carsonbot added this to the 7.1 milestone Mar 17, 2024
@IndraGunawan IndraGunawan changed the base branch from 7.1 to 6.4 March 17, 2024 17:05
@IndraGunawan IndraGunawan changed the base branch from 6.4 to 7.1 March 17, 2024 17:05
@IndraGunawan IndraGunawan force-pushed the fixed-backedenum-normalizer-priority branch from fc827ce to b7fccc1 Compare March 17, 2024 17:11
@IndraGunawan IndraGunawan changed the base branch from 7.1 to 6.4 March 17, 2024 17:11
@IndraGunawan IndraGunawan changed the title [Serializer] Fixed BackedEnumNormalizer priority for translatable enum [FrameworkBundle] Fixed BackedEnumNormalizer priority for translatable enum Mar 17, 2024
@IndraGunawan IndraGunawan force-pushed the fixed-backedenum-normalizer-priority branch from b7fccc1 to 56118df Compare March 17, 2024 17:22
@symfony symfony deleted a comment from carsonbot Mar 21, 2024
@carsonbot carsonbot changed the title [FrameworkBundle] Fixed BackedEnumNormalizer priority for translatable enum [Serializer] Fixed BackedEnumNormalizer priority for translatable enum Mar 21, 2024
@chalasr chalasr force-pushed the fixed-backedenum-normalizer-priority branch from b2ae966 to 42fde94 Compare March 23, 2024 16:08
@chalasr
Copy link
Member

chalasr commented Mar 23, 2024

Good catch, thanks @IndraGunawan.

@chalasr chalasr merged commit c87e4a1 into symfony:6.4 Mar 23, 2024
8 of 10 checks passed
@xabbuh
Copy link
Member

xabbuh commented Mar 24, 2024

What if someone relies on the current order? Do we consider this an unsupported use case?

@chalasr
Copy link
Member

chalasr commented Mar 24, 2024

Good question. I would say yes as there are similar precedents changing e.g. compiler passes order or event listeners priority as part of a bugfix. It's probably worth being decided on a case-by-case basis and mentioned in the BC promise.

@ambroisemaupate
Copy link

This change breaks the way ApiPlatform exposes BackEnum as ApiResource

api-platform/core/issues/6279

@Valentin-CosaVostra
Copy link

Valentin-CosaVostra commented Apr 4, 2024

As a BC break workaround, the easiest and maintainable way I found is to create a custom normalizer.

For example, the project I'm working on needed the translated enumerations on serialization:

use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Contracts\Translation\TranslatableInterface;
use Symfony\Contracts\Translation\TranslatorInterface;

final readonly class AcmeNormalizer implements NormalizerInterface
{
    public function __construct(private TranslatorInterface $translator)
    {
    }

    /**
     * @param TranslatableInterface $object
     */
    public function normalize($object, ?string $format = null, array $context = []): string
    {
        return $object->trans($this->translator);
    }

    public function supportsNormalization($data, ?string $format = null, array $context = []): bool
    {
        return $data instanceof TranslatableInterface;
    }

    public function getSupportedTypes(?string $format): array
    {
        return [
            TranslatableInterface::class => true,
        ];
    }
}

I don't know if this is the best way to do it or not but it seems to me to be quite maintainable.

@chalasr
Copy link
Member

chalasr commented Apr 4, 2024

Implementation changed in #54484

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.

None yet

9 participants