Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Serializer] Fix deserializing XML Attributes into string properties#58488

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

Hanmac
Copy link
Contributor

@HanmacHanmac commentedOct 8, 2024
edited by nicolas-grekas
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
IssuesFix#58479
LicenseMIT

For XML Attributes that are turned into numerics by the XMLEncoder,
but the Property wanted different type (like String), convert them back.

I also added a TestCase

@Hanmac
Copy link
ContributorAuthor

@dunglas can someone take a look at this?

@Hanmac
Copy link
ContributorAuthor

@dunglas what should be done about that AppVeyor?

@OskarStark
Copy link
Contributor

You can rebase, AppVeyor is gone, we moved to GithubActions even for windows tests

@Hanmac
Copy link
ContributorAuthor

Ugh wrong button 😑

Need to rebase manually tomorrow

@HanmacHanmacforce-pushed theserializerXMLAttributeFloatStringFix branch from40bf017 to7825527CompareDecember 10, 2024 08:43
@Hanmac
Copy link
ContributorAuthor

Some HttpClient problems, but that shouldn't be related to my PR?

@Hanmac
Copy link
ContributorAuthor

@OskarStark what should I do about the failed Unit Tests? These look unrelated?

@HanmacHanmacforce-pushed theserializerXMLAttributeFloatStringFix branch from7825527 toa5ad19eCompareDecember 12, 2024 12:44
@HanmacHanmacforce-pushed theserializerXMLAttributeFloatStringFix branch froma5ad19e to67e2997CompareFebruary 13, 2025 09:07
@Hanmac
Copy link
ContributorAuthor

Hanmac commentedFeb 13, 2025
edited
Loading

Can someone check the failed tests?

XliffFileDumperTest::testEmptyMetadataNotes should be unrelated to this MR
it was#59747 that was faulty

tried to rebase on a previous commit
Rebase worked locally, but i can't trick the Unit Tests there

@HanmacHanmacforce-pushed theserializerXMLAttributeFloatStringFix branch 2 times, most recently from9e9007b to9a7183eCompareFebruary 13, 2025 10:22
@Hanmac
Copy link
ContributorAuthor

@nicolas-grekas there all tests are green too (except the Semaphore)

@nicolas-grekas
Copy link
Member

Thanks for the ping. Looking at the code, I'm wondering if what you want isn't mean to be achieve by using the TYPE_CAST_ATTRIBUTES context option instead of changing anything in the code.

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedFeb 13, 2025
edited
Loading

Given the discussion in#58479, I think this should be considered as an improvement, so for 7.3
About the implementation, the logic should be moved to the main foreach loop just above, where all similar concerns are located.

@nicolas-grekasnicolas-grekas modified the milestones:6.4,7.3Feb 13, 2025
@HanmacHanmacforce-pushed theserializerXMLAttributeFloatStringFix branch from9a7183e to8032ef8CompareFebruary 13, 2025 11:48
@HanmacHanmac changed the base branch from6.4 to7.3February 13, 2025 11:48
@Hanmac
Copy link
ContributorAuthor

@nicolas-grekas there are some unrelated problems withtestCreateTypeContextOrUseProvided

but only for PHP 8.5, i try to pinpoint when it broke

@fabpotfabpotforce-pushed theserializerXMLAttributeFloatStringFix branch from5933418 to52a3832CompareFebruary 14, 2025 11:28
@fabpot
Copy link
Member

Thank you@Hanmac.

@fabpotfabpot merged commit7cf8df1 intosymfony:7.3Feb 14, 2025
3 of 10 checks passed
@HanmacHanmac deleted the serializerXMLAttributeFloatStringFix branchFebruary 14, 2025 11:28
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

@lyrixxlyrixxAwaiting requested review from lyrixx

@ycerutoycerutoAwaiting requested review from yceruto

@chalasrchalasrAwaiting requested review from chalasr

@OskarStarkOskarStarkAwaiting requested review from OskarStark

@jderussejderusseAwaiting requested review from jderusse

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

[Serializer][XMLEncoder] Fail to Deserialize string attributes when attributes are int/float
5 participants
@Hanmac@OskarStark@nicolas-grekas@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp