Skip to content

fix(upsert): handle missing version.message in only-changed mode#55

Open
geopanther wants to merge 1 commit into
mainfrom
fix/only-changed-no-message
Open

fix(upsert): handle missing version.message in only-changed mode#55
geopanther wants to merge 1 commit into
mainfrom
fix/only-changed-no-message

Conversation

@geopanther

Copy link
Copy Markdown
Owner

Summary

Guard against AttributeError when existing_page.version.message is absent on older self-hosted Confluence servers that don't populate version.message on page creation.

Adapted from iamjackg/md2cf#135 by @briandcho.

Changes

  • Use hasattr() check before accessing version.message for both pages and attachments
  • Treat missing message as 'needs update' (safe default)
  • Add test for the no-message scenario

Tests

  • New test: test_page_needs_updating_when_no_message
  • All 17 upsert tests pass

Older self-hosted Confluence servers don't populate version.message on
page create. Accessing it directly causes an AttributeError when using
--only-changed. Guard with hasattr() and treat missing message as
'needs update'. Also guards the same pattern for attachments.

Adapted from iamjackg/md2cf#135 by @briandcho.
@geopanther

Copy link
Copy Markdown
Owner Author

Maintainer Review

Verdict: ✅ Approve

Real-world bug fix. Older self-hosted Confluence (and potentially Cloud edge cases) may not populate version.message on initial page creation. The hasattr() guard is correct — when no message exists, there's no hash to compare, so treating it as 'needs update' is the safe default.

Good: Extended the same fix to attachment hash matching (line 192 in original), which the upstream PR didn't cover. This prevents the same crash when using --only-changed with attachments on these servers.

Suggestion for future: Consider wrapping this pattern in a small helper like get_version_hash(obj) to avoid repeating the hasattr + regex logic, but not blocking on that.

No concerns. Ship it.

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.

1 participant