Skip to content

Conversation

@OstafinL
Copy link
Contributor

@OstafinL OstafinL commented Dec 3, 2025

🎫 Issue IBX-11029

Description:

For QA:

Documentation:

Copy link
Contributor

@ViniTou ViniTou left a comment

Choose a reason for hiding this comment

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

guess we can use numeric-string instead of string in those cases

@tbialcz
Copy link
Contributor

tbialcz commented Dec 4, 2025

guess we can use numeric-string instead of string in those cases

Yes, I can switch to numeric-string, but PHPStan will still warn because numeric-string is always a string — so is_string() is seen as redundant.

Comment on lines +57 to +61
new FormErrorDataTestWrapper(
'The selected choice is invalid.',
['{{ value }}' => 'foo'],
'children[limitation_type]'
),
Copy link
Contributor

Choose a reason for hiding this comment

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

That codifies duplicate validation instead of addressing it. If the form only needs one message, this change will hide a regression and could show users two identical errors. But maybe it is not the case at all. What about changing the message in the second form error?

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked it — error_bubbling => false doesn’t change anything. Symfony 7.3 now produces two errors for expanded choice fields, so the duplication is expected.

… expectations in all relevant data transformers and tests for improved clarity.
@tbialcz tbialcz requested a review from mikadamczyk December 4, 2025 08:42
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 4, 2025

@mikadamczyk mikadamczyk merged commit abb1d86 into main Dec 4, 2025
28 checks passed
@mikadamczyk mikadamczyk deleted the IBX-11029-Fixed-tests branch December 4, 2025 09:38
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.

10 participants