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: custom JMS enum type handling #2372

Merged
merged 3 commits into from
Oct 31, 2024

Conversation

bobvandevijver
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #2327

JMS implemented the enum type annotations inconsistently compared to the other type annotations. I've created schmittjoh/serializer#1561 to fix that (which has been merged and released with 3.31.0), and this PR adds compatibility for both methods.

Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.36%. Comparing base (c0a4aee) to head (632d3be).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2372      +/-   ##
==========================================
+ Coverage   89.35%   89.36%   +0.01%     
==========================================
  Files          77       77              
  Lines        2846     2849       +3     
==========================================
+ Hits         2543     2546       +3     
  Misses        303      303              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DjordyKoert DjordyKoert merged commit 1e283ef into nelmio:master Oct 31, 2024
16 checks passed
@DjordyKoert
Copy link
Collaborator

Thank you! 😄

@bobvandevijver bobvandevijver deleted the enum-handling-fix branch October 31, 2024 11:04
DjordyKoert pushed a commit that referenced this pull request Oct 31, 2024
| Q | A |

|---------------|---------------------------------------------------------------------------------------------------------------------------|
| Bug fix? | no |
| New feature? | yes <!-- please update src/**/CHANGELOG.md files --> |
| Deprecations? | no <!-- please update UPGRADE-*.md and
src/**/CHANGELOG.md files --> |
| Issues | - <!-- prefix each issue number with "Fix #", no need to
create an issue if none exists, explain below instead --> |

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the
documentation.

Additionally:
 - Always add tests and ensure they pass.
- For new features, provide some code snippets to help understand usage.
-->

This PR add a method to detect when the developer has annotated their
property with a JMS type that specifies that the backed enum needs to be
serialized with its name instead of value. I needed to use the
serialization context as the model options are not included in the model
hash, while I do need to generate a new unique description. I've chosen
to append `Name` to the resulting Model name.

Depends on #2372
DjordyKoert pushed a commit that referenced this pull request Oct 31, 2024
DjordyKoert pushed a commit that referenced this pull request Oct 31, 2024
| Q | A |

|---------------|---------------------------------------------------------------------------------------------------------------------------|
| Bug fix? | no |
| New feature? | yes <!-- please update src/**/CHANGELOG.md files --> |
| Deprecations? | no <!-- please update UPGRADE-*.md and
src/**/CHANGELOG.md files --> |
| Issues | - <!-- prefix each issue number with "Fix #", no need to
create an issue if none exists, explain below instead --> |

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the
documentation.

Additionally:
 - Always add tests and ensure they pass.
- For new features, provide some code snippets to help understand usage.
-->

This PR add a method to detect when the developer has annotated their
property with a JMS type that specifies that the backed enum needs to be
serialized with its name instead of value. I needed to use the
serialization context as the model options are not included in the model
hash, while I do need to generate a new unique description. I've chosen
to append `Name` to the resulting Model name.

Depends on #2372

(cherry picked from commit 9ee5f58)
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.

[Bug]: enum_exists(): Argument #1 ($enum) must be of type string, array given
2 participants