Conversation
We will use PhpSpreadsheet as a replacement for SpoonFileCSV. As a benefit PhpSpreadsheet has more features, which may come in handy later on.
This service will enable you to get a preconfigured CSV writer.
…with the usage of the ForkCMS\Utility\Csv\Writer service
carakas
requested changes
Aug 5, 2020
| * This is our extended version of SpoonFileCSV | ||
| * @deprecated remove this in Fork 6, just use ForkCMS\Utility\Csv\Writer | ||
| */ | ||
| class Csv extends \SpoonFileCSV |
Member
There was a problem hiding this comment.
Why did you remove the parent class?
As far as I know this is not backwards compatible
src/ForkCMS/Utility/Csv/Writer.php
Outdated
|
|
||
| public function forBackendUser(User $user): self | ||
| { | ||
| $this->options['Delimiter'] = $user->getSetting('csv_split_character'); |
Member
There was a problem hiding this comment.
you need to clone the instance here and return the clone since using it after calling the forBackendUser will still run it with those changes otherwise
Jelmer made a good point the service can be reused and setting the options for a user would affect other usages. I rewrote the whole class to be be more usefull and have no sideeffects.
This method is heavily inspired on importCsv. In essence it is a working version. As at the moment I think importCsv will fail because of fields that can't be NULL, but the code does not handle it. I didn't fix importCsv because it would require database-structure changes and won't be backwards compatible.
It introduces 2 methods: * findColumnIndexes, which will return the column names for the PhpSpreadsheet for the given named columns. This will be used to check if the columns are present in a CSV file * convertRowIntoMappedArray, which will convert a PhpSpreadsheet Row into a named array. At this moment we use a lot of array instead of objects so this will come in handy
carakas
approved these changes
Aug 10, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type
Resolves the following issues
Pull request description
SpoonFileCSV is deprecated so it should be replaced with something that is more maintained.
With this PR we introduce the usage of PhpSpreadsheet. I choose this one over
league/csv, as the later one does not really have a lot of features, and does not really integrate in the Symfony responses, per example: outputting a CSV file won't return a string, but just outputs the CSV and stop the code.Writing CSV files
So instead of working with arrays you should create a SpreadSheet instance first,
check the official documentation.
The easiest way to convert existing code is:
A service called
ForkCMS\Utility\Csv\Writeris added, which allows you to force the download of a spreadsheet in CSV-format thru a Symfony StreamedResponse. If you want to force a download you can use the code below in the backend: