Skip to content

Skip the fullscreen alert for a meeting joined early - #1037

Open
bluejay-creators wants to merge 2 commits into
leits:masterfrom
bluejay-creators:fix/skip-fullscreen-after-early-join
Open

bluejay-creators wants to merge 2 commits into
leits:masterfrom
bluejay-creators:fix/skip-fullscreen-after-early-join

Conversation

@bluejay-creators

@bluejay-creators bluejay-creators commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Joining a meeting early through MeetingBar does not stop its fullscreen alert. When the alert time arrives, the fullscreen window still covers the screen, interrupting the meeting you are already in. The "already handled" record for the fullscreen alert was written only when the alert itself fired, never on join.

Joining now writes that same record for the event (NotificationRecordStore.recordJoined), through a new AppEnvironment.recordMeetingJoined hook called from AppModel for .joinMeeting and .joinNearestMeeting, so it covers the menu, the status-item shortcut and notification Join actions. It is skipped when the event has no meeting link, since opening then falls back to the event page rather than joining.

User-visible behavior

A meeting joined through MeetingBar before its alert time no longer shows the fullscreen alert. Meetings you have not joined still get it. If the meeting is changed afterwards (new last-modified date), the alert applies again, as before. Joins made outside MeetingBar are unchanged.

Regression risks

Notifications: only the fullscreen processed-event record is written, using the existing format and cleanup; auto-join and the event-start script are unaffected. Meeting opening: openMeeting is called exactly as before.

Verification

  • New end-to-end test testFullscreenNotificationSkipsEventJoinedEarly: two due events, one joined first. It fails without the fix (["E1", "E2"] alerted) and passes with it (["E2"] only).
  • New unit test testJoiningRecordsOnlyEventsWithAMeetingLink.
  • CI and Lint workflows on my fork: on the test-only commit, 568 app-hosted tests with 1 failure (the new end-to-end test); with the fix, 569 (adding the unit test) with 0 failures. SwiftLint passes.

Release-sensitive changes (if any)

None. Suggested release note: "Joining a meeting early no longer brings up its fullscreen alert at start time."

Summary by CodeRabbit

  • Bug Fixes
    • Events opened through a meeting link are now recorded as joined, so fullscreen notifications skip meetings you’ve already joined.
    • Events without a meeting link continue to open without being marked as joined.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fc8e125c-d13a-45a0-8093-ba550eb3b398
📥 Commits

Reviewing files that changed from the base of the PR and between 30b3820 and f45eed6.

📒 Files selected for processing: 5
  • MeetingBar/App/AppModel.swift
  • MeetingBar/Notifications/NotificationActionRunner.swift
  • MeetingBarTests/AppModelTests.swift
  • MeetingBarTests/EndToEndFlowTests.swift
  • MeetingBarTests/Helpers/AppModelTestHarness.swift

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

Meeting joins now use a shared helper that opens the event and records it as joined only when it has a meeting link. The record store updates fullscreen notification records. Tests cover join recording and subsequent notification handling.

Changes

Joined Meeting Notifications

Layer / File(s) Summary
Persist joined meeting records
MeetingBar/Notifications/NotificationActionRunner.swift
NotificationRecordStore.recordJoined replaces any matching fullscreen processed-event record with the event’s current ID, last-modified date, and end date, then saves the records.
Wire join actions to record linked meetings
MeetingBar/App/AppModel.swift, MeetingBarTests/AppModelTests.swift, MeetingBarTests/EndToEndFlowTests.swift, MeetingBarTests/Helpers/AppModelTestHarness.swift
Both join actions use a shared helper. It records an event as joined only when the event has a meeting link. Unit and end-to-end tests check recording behavior and fullscreen notification handling.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant AppModel
  participant NotificationRecordStore
  participant FullscreenNotificationHandling
  AppModel->>AppModel: Open event
  AppModel->>NotificationRecordStore: Record event when meetingLink is non-nil
  NotificationRecordStore->>FullscreenNotificationHandling: Save updated fullscreen records
Loading

Suggested reviewers: leits

Merge Risk: ⚪ Minimal · up to f45ee

Meetings joined early through MeetingBar can be excluded from their later fullscreen alert. No issue requiring a fix before merge is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: suppressing the fullscreen alert for meetings joined before the alert time.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A rabbit taps “join” with a hop,
Linked meetings get logged on the spot.
The store saves the day,
While joined events stay away,
From fullscreen alerts that might pop.

Comment @coderabbitai help to get the list of available commands.

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