- Issue created by @ultrabob
- Assigned to shubham_jain
- last update
about 1 year ago 3 pass - 🇮🇳India shubham_jain
Hi, fixed the issue was causing because the parameters inside the setCellValueByColumnAndRow is now changed and they have to be defined in a new way. So, raised MR for that which fixes the issue. Please review and verify.
- Status changed to Needs review
about 1 year ago 5:41am 21 September 2023 - First commit to issue fork.
- last update
11 months ago 3 pass This actually caused an error on our project when used with Data export phpspreadsheet → :
Error: Call to undefined method PhpOffice\PhpSpreadsheet\Worksheet\Worksheet::setCellValueByColumnAndRow() in Drupal\views_data_export\Plugin\views\display\DataExport::processBatch() (line 838 of {...}/modules/contrib/views_data_export/src/Plugin/views/display/DataExport.php).
That module has a composer dependency on
"phpoffice/phpspreadsheet": "^1 || ^2"
. View Data Export does not have a dependency on that library. It does have a dev-dependency on Excel Serialization → , which has a dependency on"phpoffice/phpspreadsheet": "^1.26"
, but dev-dependencies of a dependency are not loaded by composer for the root project. In any case, there no constraints against loading 2.x of phpoffice/phpspreadsheet, whereWorksheet::setCellValueByColumnAndRow()
has been removed.Added a suggestion to the MR for a more minimal diff.
- 🇳🇿New Zealand ericgsmith
Just flagging that https://git.drupalcode.org/project/xls_serialization/-/commit/aace003203... was committed to xls_serialization so this looks like it will be needed for the next release.
Suggestion from @godotislate on the MR looks good to me
- First commit to issue fork.
- Status changed to RTBC
3 months ago 3:02am 2 September 2024 - 🇳🇿New Zealand ericgsmith
Tested MR27
Implementation of the change suggested by godotislate looks good and export using a batch now works as expected.
PHPStan error in pipeline as is per the pipeline on 8.x-1.x-dev so looks good to me.
- 🇳🇿New Zealand ericgsmith
As mentioned in #8 as there is no hard dependency on the lib version in this module there is a chance users could be running a very old version. (And as mentioned in 8, users could also already be on v2 where this fix is needed for the batch exports to work).
For reference the new method was introduced in version 1.23.0 https://github.com/PHPOffice/PhpSpreadsheet/blob/master/CHANGELOG.md#123...
It might be worth calling out in the release notes that users should check to see if they need to update PHPOffice/PhpSpreadsheet
It might make sense to add this to composer.json:
"conflict": { "phpoffice/phpspreadsheet": "<1.23.0" }
See https://getcomposer.org/doc/04-schema.md#conflict.
Because of the CVEs, it might actually be be better to make that conflict constraint "<2.2.1", but that's possibly out of scope for this issue.
- 🇮🇳India nitesh624 Ranchi, India
Hoping This should be committed soon as there is security issue mentioned in https://www.drupal.org/project/views_data_export/issues/3388703#comment-... 🐛 Deprecated call to setCellValueByColumnAndRow RTBC
- 🇳🇿New Zealand ericgsmith
Bumping to critical since latest version of xls_serialization only supports v2 and we can't do batch exports without this patch now
- 🇳🇿New Zealand ericgsmith
Updating issue summary to reflect that now this is an error rather than a deprecation report
- 🇺🇦Ukraine andriy khomych
I faced the same issue and the above patch works fine.
I guess, such cases should be covered with tests. - 🇨🇦Canada leducdubleuet Chicoutimi QC
This patch is working well and is now mandatory. Would be great to have a new version including it!
Thx
- Issue was unassigned.
- 🇬🇧United Kingdom david.qdoscc
Looks like this change has been implemented in the latest push to 8.x-1.x-dev so this patch is no longer required.
- 🇬🇧United Kingdom steven jones
Yeah, sorry about that, I didn't appreciate that this issue was here. Thanks for all the effort though.
- 🇬🇧United Kingdom steven jones
I think @godotislate makes a great point in #14 and we should probably add that conflict section to our composer.json.
- Merge request !55Issue #3388703 by shubham_jain, amanp, ericgsmith, dydave, godotislate,... → (Merged) created by steven jones
-
steven jones →
committed fec5eaba on 8.x-1.x
Issue #3388703 by shubham_jain, amanp, steven jones, ericgsmith, dydave...
-
steven jones →
committed fec5eaba on 8.x-1.x
- 🇬🇧United Kingdom steven jones
Added that section to 8.x-1.x. Thanks again everyone.
-
steven jones →
committed fec5eaba on 8.x-2.x
Issue #3388703 by shubham_jain, amanp, steven jones, ericgsmith, dydave...
-
steven jones →
committed fec5eaba on 8.x-2.x