Skip to content

fix(drive): skip property-less sheets in has_sheet_titled - #1704

Merged
newhoggy merged 1 commit into
mainfrom
issue-1702-sheet-title-property-less
Sep 17, 2026
Merged

newhoggy merged 1 commit into
mainfrom
issue-1702-sheet-title-property-less

Conversation

@newhoggy

Copy link
Copy Markdown
Contributor

Summary

  • Spreadsheet::has_sheet_titled compared against Sheet::title(), which coerces a sheet with absent properties to "", causing a false collision with an explicit empty-title request.
  • Restore the pre-refactor behavior (filter_map(properties.as_ref()), matching structure.rs::resolve_sheet's hand-written check) so property-less sheets are skipped, not coerced.
  • Add a regression test covering a property-less sheet alongside a normal one.

Fixes #1702

Test plan

  • cargo test --lib drive::sheets::types — 13 passed, including the new has_sheet_titled_skips_a_property_less_sheet
  • cargo clippy -- -D warnings
  • cargo fmt --check

@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown

Coverage

Total: 97.28% ⚪ 0 pp vs main

Comparing fe55042..848d954 (merge-base → PR head)

File Before After Δ
src/drive/sheets/types.rs 98% 98.15% 🟢 0.15 pp
🔇 5 ignored region(s), 0 tolerated region(s)

ignore removes the lines from both reports; tolerate keeps them in the reported percentage but scores them against the baseline, so a cross-run flip cannot move a delta. Regions are read from each revision's own source.

File Kind Lines Rev Reason
src/cli/drive/lease.rs ignore 408-412 both a sheet restore's fresh backup is always a DriveCopy — restore.rs's own recheck refuses unless the target is still a spreadsheet, and acquire() only ever takes a Bytes backup of a non-native target; this arm exists solely for exhaustiveness over the shared LeaseBackup enum
src/cli/drive/lease.rs ignore 882-885 both guards this test helper against misuse; every call site below passes an acquire subcommand
src/cli/drive/lease.rs ignore 896-899 both guards this test helper against misuse; every call site below passes a prune subcommand
src/drive/lease/ledger.rs ignore 666-670 both mutate_locked refuses before ever calling the closure, so its body never runs — a hit here is a regression, not a coverage gap
src/drive/lease/restore.rs ignore 977-981 both every test using this double refuses before authenticating; a hit here is a regression, not a coverage gap

Patch coverage

Patch: 100% (13/13 new lines covered)

File Patch Uncovered new lines
src/drive/sheets/types.rs 100% (13/13)

📦 Full per-file coverage summary · run summary

@newhoggy
newhoggy force-pushed the issue-1702-sheet-title-property-less branch from 1aa6392 to 848d954 Compare September 17, 2026 15:01
Spreadsheet::has_sheet_titled compared against Sheet::title(), which
maps a sheet with absent properties to "" — coercing it into a false
collision with an explicit empty-title request. The hand-written loops
this helper replaced in structure.rs::resolve_sheet instead skip
property-less sheets entirely via filter_map(properties.as_ref()).
Restore that behavior and add a regression test.

Fixes #1702
@newhoggy
newhoggy force-pushed the issue-1702-sheet-title-property-less branch from 848d954 to e809a28 Compare September 17, 2026 15:50
@newhoggy
newhoggy merged commit c666c00 into main Sep 17, 2026
18 checks passed
@newhoggy
newhoggy deleted the issue-1702-sheet-title-property-less branch September 17, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(drive): has_sheet_titled coerces a property-less sheet to the empty title

1 participant