Fix template calcChain regeneration and $= formula cells (MiniExcel.OpenXml) - #993
Conversation
…cel.OpenXml) Porting the v1.x template-render fixes to the v2 OpenXml line, where the engine moved to src/MiniExcel.OpenXml/ and a LINQ-to-XML rewrite. Two defects made Excel reject an otherwise-valid rendered package: - $= formula cells kept their source t="inlineStr" attribute after the inline string was replaced with an <f> child — a cell typed inlineStr with no <is> is schema-invalid. The attribute is now removed. - The regenerated calcChain entry took the cell address from the running child-index, which is wrong for sparse rows (cells with no content are not emitted, so position != column). The address is now read from the cell's own r attribute. - The template's own calcChain was reused/copied even though row insertion shifts every formula cell, leaving stale entries; and an empty calcChain (no rendered formulas) was written, which is schema-invalid. The chain is now regenerated only when formulas were rendered and dropped entirely otherwise — Excel rebuilds it on open. The <f> namespace fix from the v1 change is not needed here: v2's XElement-based serialization already emits it in the spreadsheetml namespace.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTemplate formula conversion now produces valid formula cells and derives calculation-chain references from cell addresses. Template saving conditionally regenerates calculation-chain metadata and rebuilds worksheet relationships. Synchronous and asynchronous tests cover stale chains and sparse-row formulas. ChangesCalcChain Template Rendering
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant OpenXmlTemplate
participant OpenXmlTemplateImpl
participant ValueExtractorHook
participant OutputArchive
OpenXmlTemplate->>OpenXmlTemplateImpl: Convert formula cells and generate calcChain content
OpenXmlTemplate->>ValueExtractorHook: Rebuild worksheet relationships
ValueExtractorHook->>ValueExtractorHook: Remove calcChain relationship when requested
OpenXmlTemplate->>OutputArchive: Write calcChain.xml when content is valid
OpenXmlTemplate->>OutputArchive: Remove calcChain metadata when content is invalid
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs`:
- Around line 943-949: Update the cell reference assignment near cellRef so
calcChain entries use the complete c@r value directly whenever it is present,
rather than reconstructing it with rowIndex or the running index. Preserve the
existing coordinate fallback only when c@r is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e4ade420-4e26-4481-b2b7-bd2137761a9f
📒 Files selected for processing (2)
src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cssrc/MiniExcel.OpenXml/Templates/OpenXmlTemplate.cs
… on the full "r" attribute when available
Added logic to remove calcChain references from [Content_Types].xml and workbook.xml.rels when the calcChain is not to be copied from the template's archive.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.cs`:
- Around line 180-182: Normalize the Content Types calcChain PartName to the
archive-path form before matching in OpenXmlTemplate, so `/xl/calcChain.xml`
matches `xl/calcChain.xml`; in OpenXmlTemplate.ValueExtractorHook, remove the
calcChain relationship by relationship type or compare its target using the
relative `calcChain.xml` form. Extend the stale-chain tests to verify both the
content-type entry and workbook relationship are absent when `xl/calcChain.xml`
is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 11825d7a-4bfa-4b07-8d71-6183788e4cc2
📒 Files selected for processing (6)
src/MiniExcel.OpenXml/MiniExcel.OpenXml.csprojsrc/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cssrc/MiniExcel.OpenXml/Templates/OpenXmlTemplate.ValueExtractorHook.cssrc/MiniExcel.OpenXml/Templates/OpenXmlTemplate.cstests/MiniExcel.OpenXml.Tests/Templates/CalcChainAsyncTests.cstests/MiniExcel.OpenXml.Tests/Templates/CalcChainTests.cs
💤 Files with no reviewable changes (1)
- src/MiniExcel.OpenXml/MiniExcel.OpenXml.csproj
Ports the v1.x template-render fixes from #990 to the v2
MiniExcel.OpenXmlline (the engine moved tosrc/MiniExcel.OpenXml/with a LINQ-to-XML rewrite).Two defects made Excel reject an otherwise-valid rendered package:
$=formula cells kept their sourcet="inlineStr"attribute after the inline string was replaced with an<f>child. A cell typedinlineStrwith no<is>is schema-invalid. The attribute is now removed.rattribute.The
<f>namespace fix from the v1 change is not needed here: v2'sXElement-based serialization already emits it in the spreadsheetml namespace.Existing
MiniExcelTemplateTests/MiniExcelTemplateAsyncTestsstay green on net8/9/10.Summary by CodeRabbit
Bug Fixes