Fix incorrect CSV headers when exporting multiple record types #842
Conversation
This file contains 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
mstenta
added a commit
to mstenta/farmOS
that referenced
this pull request
May 7, 2024
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
from
4da5b2e
to
c925d76
Compare
mstenta
added a commit
to mstenta/farmOS
that referenced
this pull request
May 8, 2024
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
from
c925d76
to
b2e1222
Compare
|
I added two more commits to this. The first checks to see if multiple bundles are being included, and if so it adds to the warning message to highlight the fact that all columns will be included and their order may vary. Screenshot:
The second commit adds the ability to select which columns are included in the export. Screenshots:
|
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
from
934fb5f
to
d236c11
Compare
mstenta
added a commit
to mstenta/farmOS
that referenced
this pull request
May 9, 2024
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
from
d236c11
to
8f3f004
Compare
paul121
reviewed
May 9, 2024
modules/core/export/modules/csv/src/Form/EntityCsvActionForm.php
Outdated
Show resolved
Hide resolved
mstenta
added a commit
to mstenta/farmOS
that referenced
this pull request
May 9, 2024
mstenta
added a commit
to mstenta/farmOS
that referenced
this pull request
May 9, 2024
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
from
8f3f004
to
8880e06
Compare
symbioquine
reviewed
May 10, 2024
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
from
e38f7d0
to
12c3f2b
Compare
mstenta
added a commit
to mstenta/farmOS
that referenced
this pull request
May 10, 2024
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
2 times, most recently
from
c64c54e
to
f59c7fd
Compare
|
Unfortunately I realized that this PR only partially fixes #805. See this comment for more detail: #805 (comment) So I pushed a commit to this PR that omits bundle-specific columns when multiple bundles are included in an export. This is a sacrifice of (currently broken) functionality, but it doesn't seem like we have a choice. I will update the description of this PR accordingly, for future reference. |
paul121
approved these changes
May 16, 2024
…l columns are included and their order may vary.
mstenta
force-pushed
the
3.x-fix-csv-export-columns
branch
from
a6c4872
to
ecda35f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #805
This omits bundle-specific columns from CSV exports that includes entities of multiple bundles. It adds a warning message to the user to let them know this is happening.
It also adds the ability to select which bundles are included in the export via checkboxes.
Original description for posterity:
This is a quick and dirty fix. It builds the list of CSV column headers by iterating over every row to build a combined list. So the more rows you are exporting the less performant it may be.So it fixes a bug at the expense of a new performance consideration, which is the lesser of two evils IMO. And it buys us some time to work on a better overall fix (along with some of the other feature ideas we talked about in #805).Worth noting, one of the effects of this is that the order of columns will depend on the data, so if you export two sets of data, or if you apply different sorts to your data, you may get different column orders. But all the data will be there in the proper place.