Skip to content

feat(calendar): add CalDAV calendar visibility toggles - #3395

Merged
ItsLemmy merged 1 commit into
noctalia-dev:mainfrom
floydya:feat/caldav-calendar-selection
Jul 18, 2026
Merged

feat(calendar): add CalDAV calendar visibility toggles#3395
ItsLemmy merged 1 commit into
noctalia-dev:mainfrom
floydya:feat/caldav-calendar-selection

Conversation

@floydya

@floydya floydya commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds CalDAV calendar visibility toggles to the calendar account editor.

  • Stores discovered CalDAV calendar metadata in app state under calendar_discovery
  • Shows discovered calendars when editing an existing CalDAV account
  • Persists selected calendar IDs to calendar.account.<id>.calendars
  • Keeps calendars = [] as the existing “all calendars” behavior
  • Leaves Google calendar selection unchanged (why - in additional notes)

Worth mentioning, the list of calendars is available only after first sync.

Motivation

CalDAV accounts can discover multiple calendars, but settings did not expose a way to choose which discovered calendars should sync. This adds UI for that existing config field without changing the public config contract.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring
  • Build / packaging

Related Issue

This PR probably handles #3158 (if I understood it correctly)

Testing

  • nix develop --command meson compile -C build-debug noctalia
  • nix develop --command meson test -C build-debug calendar_discovery_state config_schema_roundtrip --print-errorlogs
  • nix develop --command meson test -C build-debug calendar_discovery_state --print-errorlogs
  • Manual testing via Nix flake local path + rebuild

Manual Coverage

  • Tested on Niri
  • Tested on Hyprland
  • Tested on Sway
  • Tested on another compositor:
  • Tested with different bar positions and density settings
  • Tested at different interface scaling values
  • Tested with multiple monitors

Screenshots / Videos

When creating a new calendar (unchanged):

image

When editing already synced calendar(blurred = calendar names):

image

Checklist

  • This PR is ready for review, or it is marked as Draft.
  • I read and followed the relevant guidance in CONTRIBUTING.md.
  • I ran just format with clang-format v22+ installed, or this PR has no code changes.
  • I ran the relevant build or test commands, or explained why they were not run.
  • I self-reviewed the changes.
  • I checked for new warnings or errors.
  • I will update end-user documentation after merge, or this PR does not change user-facing configuration or behavior.
  • I added or updated assets/translations/en.json, or this PR adds no new user-facing strings.
  • I did not edit non-English translation files unless this PR is explicitly for translation tooling, an import/export sync, or a maintainer-requested locale change.
  • I used the existing canonical names for config keys, IPC names, paths, and identifiers.

Additional Notes

Google calendar selection is intentionally unchanged, cause it is handled by Google API's selected field. This PR only exposes calendar selection for CalDAV accounts.

@floydya
floydya force-pushed the feat/caldav-calendar-selection branch from f42a418 to 34ed9cb Compare July 12, 2026 18:30
@ItsLemmy

Copy link
Copy Markdown
Collaborator

Issues / Risks

  • Dead code: selectedCalendarSources is never used in production. It's exported in the header and covered by the test, but the real filtering path in calendar_service.cpp:234-239 operates inline on CalDavCollection (a different type). Either wire this helper into a real call site or drop it — as-is it's an untested-in-anger export that reads as if it's load-bearing. (calendar_discovery_state.h:14)

  • Orphaned state on account deletion. removeAccount (settings_window_popups.cpp:1056-1059) clears all four calendar_credentials._* keys but not the new calendar_discovery._calendars entry this PR introduces. Deleting a CalDAV account leaves a stale discovery blob in state.toml. Add the matching cleanup line for symmetry.

  • Test coverage is inverted. The trickiest new logic — setCalendarSourceChecked's materialize/collapse/can't-empty behavior — lives in an anonymous namespace in settings_window_popups.cpp and is untested, while the unused selectedCalendarSources is the one with unit tests. The serialize/parse round-trip and malformed-JSON cases are good; consider extracting the toggle logic somewhere testable (it's pure and doesn't depend on UI).

Minor

  • The calendars override is now pushed unconditionally (settings_window_popups.cpp:1398), including for Google accounts where the UI never edits it. It's a harmless no-op round-trip of account->calendars, but worth a one-line comment noting the field is CalDAV-driven so a future reader doesn't think Google selection is wired here.

@floydya
floydya force-pushed the feat/caldav-calendar-selection branch from 34ed9cb to 6c59f0a Compare July 17, 2026 20:02
@floydya

floydya commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Issues / Risks

  • Dead code: selectedCalendarSources is never used in production. It's exported in the header and covered by the test, but the real filtering path in calendar_service.cpp:234-239 operates inline on CalDavCollection (a different type). Either wire this helper into a real call site or drop it — as-is it's an untested-in-anger export that reads as if it's load-bearing. (calendar_discovery_state.h:14)
  • Orphaned state on account deletion. removeAccount (settings_window_popups.cpp:1056-1059) clears all four calendar_credentials._* keys but not the new calendar_discovery._calendars entry this PR introduces. Deleting a CalDAV account leaves a stale discovery blob in state.toml. Add the matching cleanup line for symmetry.
  • Test coverage is inverted. The trickiest new logic — setCalendarSourceChecked's materialize/collapse/can't-empty behavior — lives in an anonymous namespace in settings_window_popups.cpp and is untested, while the unused selectedCalendarSources is the one with unit tests. The serialize/parse round-trip and malformed-JSON cases are good; consider extracting the toggle logic somewhere testable (it's pure and doesn't depend on UI).

Minor

  • The calendars override is now pushed unconditionally (settings_window_popups.cpp:1398), including for Google accounts where the UI never edits it. It's a harmless no-op round-trip of account->calendars, but worth a one-line comment noting the field is CalDAV-driven so a future reader doesn't think Google selection is wired here.

Hey, thanks for the review.

  1. Dead code is resolved: I've removed the unused export and replaced it with selectedCalendarSourceIds. The new helper is now used by the real CalDAV filtering path in CalendarService::fetchCalDav, so the tested logic matches production behavior.
  2. Orphaned state on acc deletion: Updated removeAccount to also clear calendar_discovery.<accountId>_calendars, alongside the four credential entries. Deleted CalDAV accounts no longer leave stale discovery data in state.toml.
  3. Test coverage is inverted: Extracted setCalendarSourceChecked from the settings UI’s anonymous namespace into calendar_discovery_state. Also added more tests to cover the whole logic.
  4. Preserved the existing behavior because it is a harmless round-trip, but added a comment explaining that manual calendar selection is currently CalDAV-driven and Google selection comes from the CalendarList selected field.

@floydya
floydya force-pushed the feat/caldav-calendar-selection branch from 6c59f0a to f2fe3e8 Compare July 18, 2026 07:59
@ItsLemmy

Copy link
Copy Markdown
Collaborator

Thanks!

@ItsLemmy
ItsLemmy merged commit 3e973a9 into noctalia-dev:main Jul 18, 2026
2 checks passed
@floydya
floydya deleted the feat/caldav-calendar-selection branch July 19, 2026 10:28
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.

2 participants