Skip to content

Fix system notifications showing Oopsie error#1488

Merged
maebeale merged 2 commits into
mainfrom
maebeale/fix-system-notifications
Apr 17, 2026
Merged

Fix system notifications showing Oopsie error#1488
maebeale merged 2 commits into
mainfrom
maebeale/fix-system-notifications

Conversation

@maebeale
Copy link
Copy Markdown
Collaborator

@maebeale maebeale commented Apr 17, 2026

What is the goal of this PR and why is this important?

  • Fix the system notifications page showing "Oopsie!" instead of the notifications table
  • Two issues: (1) the turbo frame rendered data inline AND fired a redundant lazy-load request via src — if that request failed, the content was replaced with the error, and (2) links inside the turbo frame (View, Record) were intercepted by Turbo, which tried to load the target page inside the frame — those pages don't have a matching frame, triggering the error

How did you approach the change?

  • Adopted the same lazy-load pattern used by other controllers (stories, people, etc.) — skeleton on initial load, data fetched via turbo frame request
  • Added data: { turbo_frame: "_top" } to View and Record links so they navigate the full page instead of targeting the frame
  • Removed orphaned _notifications_results.html.erb partial (referenced a non-existent _notifications_count partial and was never used)
  • Updated request specs to use turbo frame headers and test actual filtering behavior

UI Testing Checklist

  • Visit /notifications as admin — should show skeleton briefly then load notifications table
  • Click "View" on a notification — should navigate to the show page (not show Oopsie)
  • Click a Record link — should navigate to the record page (not show Oopsie)
  • Use search filters (email, record type, email topic, subject) — results should update in the turbo frame
  • Clear filters — should reset to all notifications

Anything else to add?

  • The root cause was that "View" and "Record" links inside the notifications_results turbo frame were intercepted by Turbo. Turbo tried to load the linked pages inside the frame, but those pages don't contain a matching <turbo-frame id="notifications_results">, so turbo:frame-missing fired and the handler replaced the frame content with "Oopsie!"

🤖 Generated with Claude Code

The notifications index rendered data inline inside a turbo frame while
also setting a src attribute to re-fetch the same data. If the redundant
request failed, the turbo:frame-missing handler replaced the content
with an error message. Aligns with the lazy-load pattern used by other
controllers (stories, people, etc.) — skeleton on initial load, data
fetched via turbo frame request.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
authorize!
per_page = params[:number_of_items_per_page].presence || 25
base_scope = authorized_scope(Notification.includes(:noticeable))
filtered = base_scope.search_by_params(params.to_unsafe_h)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopted the same lazy-load pattern as StoriesController, PeopleController, etc. — only load data for turbo frame requests, render a skeleton for the initial page load.

Links inside the notifications_results turbo frame were being
intercepted by Turbo, which tried to load the target page inside
the frame. The show page and polymorphic record pages don't contain
a matching turbo frame, triggering the turbo:frame-missing handler
and showing the "Oopsie!" error.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@maebeale maebeale marked this pull request as ready for review April 17, 2026 01:12
@maebeale maebeale requested a review from jmilljr24 April 17, 2026 01:12
Copy link
Copy Markdown
Collaborator

@jmilljr24 jmilljr24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maebeale Thanks!!

@maebeale maebeale merged commit 64782c6 into main Apr 17, 2026
3 checks passed
@maebeale maebeale deleted the maebeale/fix-system-notifications branch April 17, 2026 02:11
jmilljr24 pushed a commit that referenced this pull request May 20, 2026
* Fix system notifications showing "Oopsie!" by adopting lazy-load pattern

The notifications index rendered data inline inside a turbo frame while
also setting a src attribute to re-fetch the same data. If the redundant
request failed, the turbo:frame-missing handler replaced the content
with an error message. Aligns with the lazy-load pattern used by other
controllers (stories, people, etc.) — skeleton on initial load, data
fetched via turbo frame request.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add turbo_frame: _top to links inside notifications turbo frame

Links inside the notifications_results turbo frame were being
intercepted by Turbo, which tried to load the target page inside
the frame. The show page and polymorphic record pages don't contain
a matching turbo frame, triggering the turbo:frame-missing handler
and showing the "Oopsie!" error.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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