-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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] Add headers label support for CsvEncoder
using csv_headers
#55334
Conversation
@@ -11,6 +11,7 @@ CHANGELOG | |||
* Add `CamelCaseToSnakeCaseNameConverter::REQUIRE_SNAKE_CASE_PROPERTIES` context option | |||
* Deprecate `AbstractNormalizerContextBuilder::withDefaultContructorArguments(?array $defaultContructorArguments)`, use `withDefaultConstructorArguments(?array $defaultConstructorArguments)` instead (note the missing `s` character in Contructor word in deprecated method) | |||
* Add `XmlEncoder::CDATA_WRAPPING_PATTERN` context option | |||
* Add headers label support for `CsvEncoder` using `csv_headers` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this to a new 7.2 section, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping I still have a chance to see it merged in 7.1 branch since the final version is not released yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I just added a test |
@@ -242,14 +242,38 @@ public function testEncodeCustomHeaders() | |||
CsvEncoder::HEADERS_KEY => [ | |||
'b', | |||
'c', | |||
'n.0', | |||
'n.k', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Headers for nested data was already supported but not tested.
Shouldn't the mapping be used in both, encoding & decoding? Or is that already the case 🤔 Also I'm wondering, would it make more sense to implement this mapping in |
@valtzu not sure to understand your question. This PR is not about mapping but adding labels to generated CSV, which is relevant only when exporting. |
@Seb33300 I assume by adding labels you mean csv column names / header – and those may exist when exporting or importing csv data. To me it feels quite unintuitive that you can not import the exported file using $data = [['firstName' => 'foo', 'lastName' => 'bar']];
$context = [
CsvEncoder::HEADERS_KEY => [
'Prénom' => 'firstName',
'Nom' => 'lastName',
],
];
$encoded = $this->serializer->encode($data, 'csv', $context);
/* =
Prénom,Nom
foo,bar
*/
$decoded = $this->serializer->decode($encoded, 'csv', $context);
/* =
Array
(
[0] => Array
(
[Prénom] => foo
[Nom] => bar
)
)
*/ I would expect If this was implemented in a name converter, it would work both ways. Basically same as It would be something like 👇 (such thing is not implemented) $csv = $serializer->serialize($data, 'csv', [
AbstractNormalizer::ATTRIBUTES => ['firstName', 'lastName'],
MetadataAwareNameConverter::MAPPING => [
YourEntity::class => [
'Prénom' => 'firstName',
'Nom' => 'lastName',
],
],
]); |
You are right. |
Improve the existing
csv_headers
option ofCsvEncoder
to pass CSV headers labels.