add missing double-quotes to extra_fields output message #28840
Conversation
danielkay
changed the title
|
I think the intention was to have something like |
|
Congrats on your first Symfony pull request! 🎉 For the merger team: an alternative could be add the extra quotes before and after the imploded fields, but I agree with the style proposed (it's unlikely that form field names will contain commas, and the extra quotes get escaped in APIs using JSON format making it look ugly). Status: Reviewed |
|
@fabpot Unfortunately it isn't doing so... I feel we should take the code one way or the other - would with quotes or without quotes be better? Without the first and last quote feels weird... |
|
I would add the missing quotes instead of removing them to be consistent with the way we display other error messages. |
danielkay
force-pushed
the
fix/erroneous-quotes-in-extra_fields-output
branch
from
ade8f68
to
2379b40
Compare
danielkay
changed the title
|
added missing quotes as per suggestion from @fabpot |
| ->setInvalidValue($form->getExtraData()) | ||
| ->setCode(Form::NO_SUCH_FIELD_ERROR) | ||
| ->addViolation(); | ||
| } else { | ||
| $this->buildViolation($config->getOption('extra_fields_message')) | ||
| ->setParameter('{{ extra_fields }}', implode('", "', array_keys($form->getExtraData()))) | ||
| ->setParameter('{{ extra_fields }}', '"'.implode(', ', array_keys($form->getExtraData())).'"') |
There was a problem hiding this comment.
here '", "' should remain as glue also right
danielkay
force-pushed
the
fix/erroneous-quotes-in-extra_fields-output
branch
3 times, most recently
from
3ef528c
to
d1d2c48
Compare
fabpot
force-pushed
the
fix/erroneous-quotes-in-extra_fields-output
branch
from
d1d2c48
to
9e74141
Compare
|
Thank you @danielkay. |
…danielkay) This PR was squashed before being merged into the 2.8 branch (closes #28840). Discussion ---------- add missing double-quotes to extra_fields output message | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A When using the extra_fields_message option on FormTypes to include the extra_fields items in the message, the output included some superfluous (or, perhaps, missing?) quotes. This resulted in some strange output: `This form should not contain extra fields: notes1\", \"notes2\", \"notes3`. This PR removes the quotes and instead implodes the extra_fields array using merely ', ' as the glue, resulting in the output: `This form should not contain extra fields: notes1, notes2, notes3`. Commits ------- 9e74141 add missing double-quotes to extra_fields output message
When using the extra_fields_message option on FormTypes to include the extra_fields items in the message, the output included some superfluous (or, perhaps, missing?) quotes. This resulted in some strange output:
This form should not contain extra fields: notes1\", \"notes2\", \"notes3.This PR removes the quotes and instead implodes the extra_fields array using merely ', ' as the glue, resulting in the output:
This form should not contain extra fields: notes1, notes2, notes3.