-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix: Handle extLst in data validation for deletion (#2133) #2137
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
Conversation
This commit resolves issue #2133 by ensuring that data validations defined within an <extLst> (Extension List) are correctly parsed and accessible. This prevents DeleteDataValidation from prematurely returning nil when ws.DataValidations was previously unpopulated due to unhandled extLst elements. Key changes: - Modified internal 'xlsxDataValidation' struct to include an 'ExtLst' field, enabling the parsing of <extLst> from worksheet XML. - Added 'ExtLstXML' string field to the public 'DataValidation' struct to expose the raw XML content of the extension list to users. - Updated the 'getDataValidations' helper function to correctly transfer the extLst XML content from the internal structure to the public DataValidation object. These changes ensure that data validations, including those defined via extensions, are fully loaded, allowing DeleteDataValidation to operate correctly on them.
extLstValidationTest_after_delete_attempt.xlsx The original file and the file deleted extLst validation |
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.
Thanks for your issue. Unit test is required for coverage this changes.
- Support delete data validation by given with multiple cell ranges with reference sequence slice or blank separated reference sequence string - Update unit tests
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've fix code review issues based on your branch. Thanks for your contribution.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2137 +/- ##
=======================================
Coverage 99.23% 99.23%
=======================================
Files 32 32
Lines 30243 30303 +60
=======================================
+ Hits 30012 30072 +60
Misses 153 153
Partials 78 78
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
This pull request addresses an issue where data validations defined within an
<extLst>
(Extension List) in the worksheet XML were not being fully parsed. This incomplete parsing led tows.DataValidations
beingnil
in certain cases, causing theDeleteDataValidation
function to return prematurely without attempting to delete the relevant data validation rules.The key changes implemented in this PR are:
xlsxDataValidation
struct by adding anExtLst
field. This allows theexcelize
library to correctly parse and capture the content of<extLst>
elements from the worksheet XML.ExtLstXML
string field to the publicDataValidation
struct. This field exposes the raw XML content of the extension list associated with a data validation rule, making this information accessible to users of the library.getDataValidations
helper function to ensure that the raw XML content from the parseddv.ExtLst.Ext
(internal struct) is correctly transferred to thedataValidation.ExtLstXML
field of the publicDataValidation
object.These modifications ensure that all data validation rules, including those defined via XML extensions, are fully loaded and represented. Consequently,
DeleteDataValidation
can now correctly identify and operate on these data validations.Related Issue
Fixes #2133
Motivation and Context
This change is required to fix a bug where
DeleteDataValidation
would not work for Excel files if their data validations were defined using extension list (extLst
) elements. The original issue (#2133) describes how the function would return early because theDataValidations
field on the worksheet object wasnil
, due toextLst
not being parsed. This PR ensures that such data validations are properly read, allowing them to be managed (specifically, deleted) correctly by the library, thus improving compatibility and correctness when handling modern Excel files.How Has This Been Tested
The changes have been tested as follows:
excelize
project were run usinggo test -v ./...
after applying the changes, and all tests passed successfully. This ensures no existing functionality was broken.extLstDebugFile.xlsx
) was manually created, containing a data validation rule on sheetSheetWithExtLstDV
cellB1
that was intentionally defined using an<extLst>
element (specifically,<ext uri="{CCE6A557-97BC-4b89-ADB6-D9C93CAAB3DF}...">
and<ext uri="{FAC63952-8661-4B54-9155-E8378167997E}...">
).main.go
in a separate tester project موادreplace
directive) was used to:excelize
library.GetDataValidations("SheetWithExtLstDV")
and verify (via logging) that theExtLstXML
field of the returnedDataValidation
object was correctly populated with the XML content from theextLst
.DeleteDataValidation("SheetWithExtLstDV", "B1")
.GetDataValidations("SheetWithExtLstDV")
again and verify that no data validations were returned, confirming successful deletion in memory.f.SaveAs()
.excelize
library.Testing Environment:
Types of changes
Checklist