From 29d50cac7d02b1adcf1be844c63849926ab62328 Mon Sep 17 00:00:00 2001 From: maebeale Date: Thu, 16 Apr 2026 20:58:31 -0400 Subject: [PATCH 1/2] Fix system notifications showing "Oopsie!" by adopting lazy-load pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- app/controllers/notifications_controller.rb | 17 ++++++++----- .../_notifications_results.html.erb | 21 ---------------- .../notifications/_results_skeleton.html.erb | 24 +++++++++++++++++++ app/views/notifications/index.html.erb | 9 +------ spec/requests/notifications_spec.rb | 19 ++++++++------- 5 files changed, 47 insertions(+), 43 deletions(-) delete mode 100644 app/views/notifications/_notifications_results.html.erb create mode 100644 app/views/notifications/_results_skeleton.html.erb diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 0b76fd2a0..6c948f914 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -3,13 +3,18 @@ class NotificationsController < ApplicationController def index 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) - @notifications = filtered.order(created_at: :desc) - .paginate(page: params[:page], per_page: per_page) - render turbo_frame_request? ? :notifications_results : :index + if turbo_frame_request? + 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) + @notifications = filtered.order(created_at: :desc) + .paginate(page: params[:page], per_page: per_page) + + render :notifications_results + else + render :index + end end def show diff --git a/app/views/notifications/_notifications_results.html.erb b/app/views/notifications/_notifications_results.html.erb deleted file mode 100644 index 5638b31c2..000000000 --- a/app/views/notifications/_notifications_results.html.erb +++ /dev/null @@ -1,21 +0,0 @@ -<%= turbo_stream.replace("notifications_count", partial: "notifications_count") %> -<% if @notifications.any? %> -
- <%= render "index" %> -
- -
- -
-<% else %> -
-

No notifications yet

- <% if allowed_to?(:new?, Notification) %> -
- <%= link_to "Create a notification", - new_notification_path, - class: "btn btn-primary" %> -
- <% end %> -
-<% end %> diff --git a/app/views/notifications/_results_skeleton.html.erb b/app/views/notifications/_results_skeleton.html.erb new file mode 100644 index 000000000..3f95119aa --- /dev/null +++ b/app/views/notifications/_results_skeleton.html.erb @@ -0,0 +1,24 @@ +
+ + + + + + + + + + + + <% 5.times do %> + + + + + + + + <% end %> + +
+
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 9bde34ce3..25317c709 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -11,14 +11,7 @@ <% result_src = notifications_path + "?" + request.query_string %> <%= turbo_frame_tag "notifications_results", src: result_src do %> -
- <%= render "index" %> -
- - - + <%= render "results_skeleton" %> <% end %> diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb index 7cfdc557f..89629af49 100644 --- a/spec/requests/notifications_spec.rb +++ b/spec/requests/notifications_spec.rb @@ -8,22 +8,25 @@ describe "GET /notifications" do before { sign_in admin } + let(:turbo_headers) { { "Turbo-Frame" => "notifications_results" } } let!(:story_notification) { create(:notification, noticeable: create(:story_idea), email_subject: "New story idea") } let!(:user_notification) { create(:notification, noticeable: create(:user), email_subject: "Welcome") } - it "preserves the email_topic filter selection" do - get notifications_path, params: { email_topic: "User: confirm new email" } - expect(response.body).to include('selected="selected"') - expect(response.body).to include("User: confirm new email") + it "filters by email_topic" do + matching = create(:notification, email_subject: "Confirm your new email address") + get notifications_path, params: { email_topic: "User: confirm new email" }, headers: turbo_headers + expect(response.body).to include(matching.recipient_email) + expect(response.body).not_to include(story_notification.recipient_email) end - it "preserves the record_type filter selection" do - get notifications_path, params: { record_type: "StoryIdea" } - expect(response.body).to include('selected="selected"') + it "filters by record_type" do + get notifications_path, params: { record_type: "StoryIdea" }, headers: turbo_headers + expect(response.body).to include(story_notification.recipient_email) + expect(response.body).not_to include(user_notification.recipient_email) end it "wraps results in a turbo frame" do - get notifications_path + get notifications_path, headers: turbo_headers expect(response.body).to include('id="notifications_results"') end end From f18a07499ec57aaa67ffbbf4c7814f4dbc2b9fa0 Mon Sep 17 00:00:00 2001 From: maebeale Date: Thu, 16 Apr 2026 21:09:01 -0400 Subject: [PATCH 2/2] 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 --- app/views/notifications/_index.html.erb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/notifications/_index.html.erb b/app/views/notifications/_index.html.erb index ad0e3e9bd..22898cca7 100644 --- a/app/views/notifications/_index.html.erb +++ b/app/views/notifications/_index.html.erb @@ -44,6 +44,7 @@ <% if notification.noticeable.present? %> <%= link_to notification.noticeable.class.name, polymorphic_path(notification.noticeable), + data: { turbo_frame: "_top" }, class: "btn btn-secondary-outline" %> <% else %> — @@ -54,6 +55,7 @@ <%= link_to "View", notification_path(notification), + data: { turbo_frame: "_top" }, class: "btn btn-secondary-outline" %>