diff --git a/app/controllers/better_together/notifications_controller.rb b/app/controllers/better_together/notifications_controller.rb index cf7bf7e45..09860b56a 100644 --- a/app/controllers/better_together/notifications_controller.rb +++ b/app/controllers/better_together/notifications_controller.rb @@ -13,8 +13,43 @@ def index @unread_count = helpers.current_person.notifications.unread.size end + def dropdown # rubocop:todo Metrics/AbcSize, Metrics/MethodLength + # Get basic info needed for cache key (minimal queries) + max_updated_at = helpers.current_person.notifications.maximum(:updated_at) + unread_count = helpers.current_person.notifications.unread.size + + # Create cache key based on max updated_at of user's notifications + cache_key = "notifications_dropdown/#{helpers.current_person.id}/#{max_updated_at&.to_i}/#{unread_count}" + + cached_content = Rails.cache.fetch(cache_key, expires_in: 1.hour) do + # Only fetch detailed data when cache misses + notifications = helpers.recent_notifications + + # Warm up fragment caches for individual notifications in background + warm_notification_fragment_caches(notifications) if Rails.env.production? + + render_to_string( + partial: 'better_together/notifications/dropdown_content', + locals: { notifications: notifications, unread_count: unread_count } + ) + end + + render html: cached_content.html_safe + end + # TODO: Make a Stimulus controller to dispatch this action async when messages are viewed - def mark_as_read # rubocop:todo Metrics/AbcSize, Metrics/MethodLength + def mark_as_read + process_mark_as_read_request + + respond_to do |format| + format.html { redirect_to notifications_path } + format.turbo_stream { render_turbo_stream_response } + end + end + + private + + def process_mark_as_read_request if params[:id] mark_notification_as_read(params[:id]) elsif params[:record_id] @@ -22,23 +57,40 @@ def mark_as_read # rubocop:todo Metrics/AbcSize, Metrics/MethodLength else helpers.current_person.notifications.unread.update_all(read_at: Time.current) end + end - respond_to do |format| - format.html { redirect_to notifications_path } - format.turbo_stream do - if @notification - render turbo_stream: turbo_stream.replace(helpers.dom_id(@notification), @notification) - else - render turbo_stream: turbo_stream.replace( - 'notifications', - partial: 'better_together/notifications/notifications', - locals: { notifications: helpers.current_person.notifications, unread_count: 0 } - ) - end - end + def render_turbo_stream_response + if @notification + render_notification_turbo_stream + else + render_notifications_list_turbo_stream end end + def render_notification_turbo_stream + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@notification), + partial: 'better_together/notifications/notification', + locals: notification_locals + ) + end + + def render_notifications_list_turbo_stream + render turbo_stream: turbo_stream.replace( + 'notifications', + partial: 'better_together/notifications/notifications', + locals: { notifications: helpers.current_person.notifications, unread_count: 0 } + ) + end + + def notification_locals + { + notification: @notification, + notification_title: @notification.event.record&.try(:title) || 'Notification', + notification_url: @notification.try(:url) || '#' + } + end + def mark_notification_as_read(id) @notification = helpers.current_person.notifications.find(id) @notification.update(read_at: Time.current) @@ -47,5 +99,22 @@ def mark_notification_as_read(id) def mark_record_notification_as_read(id) mark_notifications_read_for_record_id(id) end + + # Warm fragment caches for notifications to improve subsequent renders + def warm_notification_fragment_caches(notifications) + notifications.each do |notification| + next unless helpers.should_cache_notification?(notification) + + # Pre-warm the fragment cache keys + fragment_key = helpers.notification_fragment_cache_key(notification) + type_key = helpers.notification_type_fragment_cache_key(notification) + + # Check if fragments are already cached to avoid unnecessary work + unless Rails.cache.exist?(fragment_key) && Rails.cache.exist?(type_key) + # This could be moved to a background job for better performance + Rails.logger.debug "Warming fragment cache for notification #{notification.id}" + end + end + end end end diff --git a/app/helpers/better_together/notifications_helper.rb b/app/helpers/better_together/notifications_helper.rb index 7f9baee7b..57d90f3a5 100644 --- a/app/helpers/better_together/notifications_helper.rb +++ b/app/helpers/better_together/notifications_helper.rb @@ -24,5 +24,67 @@ def unread_notification_count def recent_notifications current_person.notifications.joins(:event).order(created_at: :desc).limit(5) end + + # Fragment cache key for notification types + def notification_fragment_cache_key(notification) + # Base cache key with notification's cache_key_with_version + base_key = [notification.cache_key_with_version] + + # Add related record version if it exists and has cache_key_with_version + if notification.event&.record.respond_to?(:cache_key_with_version) + base_key << notification.event.record.cache_key_with_version + end + + # Add event cache key for consistency + base_key << notification.event.cache_key_with_version if notification.event.respond_to?(:cache_key_with_version) + + # Add locale for i18n support + base_key << I18n.locale + + base_key + end + + # Type-specific fragment cache key for notification patterns + def notification_type_fragment_cache_key(notification) + type_key = [ + notification.type || notification.class.name, + notification.cache_key_with_version, + I18n.locale + ] + + # Add related record cache key if available + if notification.event&.record.respond_to?(:cache_key_with_version) + type_key << notification.event.record.cache_key_with_version + end + + type_key + end + + # Cache expiration utilities for notifications + def expire_notification_fragments(notification) + # Expire all fragments related to this notification + expire_fragment(notification_fragment_cache_key(notification)) + expire_fragment(notification_type_fragment_cache_key(notification)) + + # Expire header/content/footer fragments + expire_fragment(['notification_header', notification.cache_key_with_version]) + expire_fragment(['notification_content', notification.cache_key_with_version]) + expire_fragment(['notification_footer', notification.cache_key_with_version]) + end + + # Expire fragments for a specific notification type pattern + def expire_notification_type_fragments(notification_type) + # This is useful when you want to expire all cached fragments for a specific notification type + # Note: This requires more specific cache management based on your needs + Rails.cache.delete_matched("*#{notification_type}*") + end + + # Check if a notification should use fragment caching + def should_cache_notification?(notification) + # Only cache if the notification and its record are present and have cache keys + notification.present? && + notification.event&.record.present? && + notification.respond_to?(:cache_key_with_version) + end end end diff --git a/app/javascript/controllers/better_together/lazy_notifications_controller.js b/app/javascript/controllers/better_together/lazy_notifications_controller.js new file mode 100644 index 000000000..eb1b18a1e --- /dev/null +++ b/app/javascript/controllers/better_together/lazy_notifications_controller.js @@ -0,0 +1,79 @@ +// app/assets/javascripts/controllers/better_together/lazy_notifications_controller.js +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = [ "dropdown", "content" ] + static values = { + loaded: Boolean, + url: String + } + + connect() { + this.loadedValue = false + console.log('Lazy notifications controller connected with URL:', this.urlValue) + + // Find the dropdown toggle element (the tag with data-bs-toggle="dropdown") + const dropdownToggle = this.element.querySelector('[data-bs-toggle="dropdown"]') + + if (dropdownToggle) { + console.log('Found dropdown toggle, adding event listener') + // Listen for Bootstrap dropdown show event on the dropdown toggle + dropdownToggle.addEventListener('show.bs.dropdown', (event) => { + console.log('Bootstrap dropdown show event triggered') + if (!this.loadedValue) { + this.loadContent() + } + }) + } else { + console.log('No dropdown toggle found') + } + } + + toggle(event) { + // This method can be removed or kept for debugging + console.log('Dropdown toggle clicked') + } + + async loadContent() { + if (this.loadedValue) return + + console.log('Loading notifications from:', this.urlValue) + + try { + // Show loading state + this.contentTarget.innerHTML = ` +
+
+ Loading... +
+
Loading notifications...
+
+ ` + + const response = await fetch(this.urlValue, { + headers: { + 'Accept': 'text/html' + } + }) + + if (response.ok) { + const html = await response.text() + this.contentTarget.innerHTML = html + this.loadedValue = true + } else { + this.contentTarget.innerHTML = ` +
+ Failed to load notifications +
+ ` + } + } catch (error) { + console.error('Error loading notifications:', error) + this.contentTarget.innerHTML = ` +
+ Error loading notifications +
+ ` + } + } +} diff --git a/app/jobs/better_together/notification_cache_warming_job.rb b/app/jobs/better_together/notification_cache_warming_job.rb new file mode 100644 index 000000000..38442d2a0 --- /dev/null +++ b/app/jobs/better_together/notification_cache_warming_job.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module BetterTogether + # Background job for warming notification fragment caches + class NotificationCacheWarmingJob < ApplicationJob + queue_as :low_priority + + def perform(notification_ids) + notifications = Noticed::Notification.where(id: notification_ids).includes(event: :record) + + notifications.find_each do |notification| + warm_notification_fragments(notification) if should_warm_cache?(notification) + end + end + + private + + def warm_notification_fragments(notification) + # Generate cache keys without actually rendering to check existence + fragment_key = notification_fragment_cache_key(notification) + type_key = notification_type_fragment_cache_key(notification) + + # Only warm if not already cached + return if Rails.cache.exist?(fragment_key) && Rails.cache.exist?(type_key) + + # Render the notification to warm the cache + ApplicationController.renderer.render( + partial: notification, + locals: {}, + formats: [:html] + ) + rescue StandardError => e + Rails.logger.warn "Failed to warm cache for notification #{notification.id}: #{e.message}" + end + + def should_warm_cache?(notification) + notification.event&.record.present? && + notification.respond_to?(:cache_key_with_version) && + notification.created_at > 1.week.ago # Only warm recent notifications + end + + def notification_fragment_cache_key(notification) + base_key = [notification.cache_key_with_version] + if notification.event&.record.respond_to?(:cache_key_with_version) + base_key << notification.event.record.cache_key_with_version + end + base_key << notification.event.cache_key_with_version if notification.event.respond_to?(:cache_key_with_version) + base_key << I18n.locale + base_key + end + + def notification_type_fragment_cache_key(notification) + type_key = [ + notification.type || notification.class.name, + notification.cache_key_with_version, + I18n.locale + ] + if notification.event&.record.respond_to?(:cache_key_with_version) + type_key << notification.event.record.cache_key_with_version + end + type_key + end + end +end diff --git a/app/models/concerns/better_together/notification_cache_management.rb b/app/models/concerns/better_together/notification_cache_management.rb new file mode 100644 index 000000000..b92709102 --- /dev/null +++ b/app/models/concerns/better_together/notification_cache_management.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module BetterTogether + # Concern for managing notification fragment caches + module NotificationCacheManagement + extend ActiveSupport::Concern + + included do + # Automatically expire fragment caches when notification changes + after_update :expire_notification_caches, if: :should_expire_caches? + after_destroy :expire_notification_caches + end + + private + + def expire_notification_caches + # Use the helper methods to expire relevant fragments + ApplicationController.helpers.expire_notification_fragments(self) if respond_to_cache_methods? + end + + def should_expire_caches? + # Expire caches on status changes (read/unread) or content changes + saved_change_to_read_at? || saved_change_to_created_at? || saved_change_to_updated_at? + end + + def respond_to_cache_methods? + ApplicationController.helpers.respond_to?(:expire_notification_fragments) + rescue StandardError + false + end + end +end diff --git a/app/views/better_together/event_invitation_notifier/notifications/_notification.html.erb b/app/views/better_together/event_invitation_notifier/notifications/_notification.html.erb index c41ce72a2..f3a4f2e94 100644 --- a/app/views/better_together/event_invitation_notifier/notifications/_notification.html.erb +++ b/app/views/better_together/event_invitation_notifier/notifications/_notification.html.erb @@ -1,10 +1,12 @@ - <%= render layout: 'better_together/notifications/notification', locals: { notification: notification, notification_title: notification.title, notification_url: notification.invitable } do %> -

- <%= notification.body %> -

+ <% # Fragment cache for event invitation-specific content %> + <% cache ["event_invitation_content", notification.cache_key_with_version, notification.invitable.cache_key_with_version, I18n.locale] do %> +

+ <%= notification.body %> +

+ <% end %> <% end %> \ No newline at end of file diff --git a/app/views/better_together/joatu/agreement_notifier/notifications/_notification.html.erb b/app/views/better_together/joatu/agreement_notifier/notifications/_notification.html.erb index 7692dcfe3..77197be14 100644 --- a/app/views/better_together/joatu/agreement_notifier/notifications/_notification.html.erb +++ b/app/views/better_together/joatu/agreement_notifier/notifications/_notification.html.erb @@ -3,7 +3,10 @@ locals: { notification: notification, notification_title: notification.title, notification_url: notification.agreement } do %> -

- <%= notification.body %> -

+ <% # Fragment cache for agreement-specific content %> + <% cache ["agreement_notification_content", notification.cache_key_with_version, notification.agreement.cache_key_with_version, I18n.locale] do %> +

+ <%= notification.body %> +

+ <% end %> <% end %> diff --git a/app/views/better_together/new_message_notifier/notifications/_notification.html.erb b/app/views/better_together/new_message_notifier/notifications/_notification.html.erb index 17f04a9de..790b4b9cf 100644 --- a/app/views/better_together/new_message_notifier/notifications/_notification.html.erb +++ b/app/views/better_together/new_message_notifier/notifications/_notification.html.erb @@ -1,7 +1,16 @@ <%= render layout: 'better_together/notifications/notification', locals: { notification: notification, notification_title: t('better_together.new_message_notifier.new_message'), notification_url: notification.url } do %> -

- <%= policy(notification.sender).show? ? link_to(notification.sender, notification.sender, class: 'text-decoration-none') : notification.sender %>
- <%= notification.message.content.to_plain_text.truncate(50) %> -

+ <% # Fragment cache for message-specific content %> + <% cache ["message_notification_content", notification.cache_key_with_version, notification.message.cache_key_with_version, I18n.locale] do %> +

+ <% # Cache sender information separately %> + <% cache ["message_sender", notification.sender.cache_key_with_version, I18n.locale] do %> + <%= policy(notification.sender).show? ? link_to(notification.sender, notification.sender, class: 'text-decoration-none') : notification.sender %>
+ <% end %> + <% # Cache message content separately %> + <% cache ["message_content_truncated", notification.message.cache_key_with_version, I18n.locale] do %> + <%= notification.message.content.to_plain_text.truncate(50) %> + <% end %> +

+ <% end %> <% end %> \ No newline at end of file diff --git a/app/views/better_together/noticed/notifications/_notification.html.erb b/app/views/better_together/noticed/notifications/_notification.html.erb new file mode 100644 index 000000000..b73a5da97 --- /dev/null +++ b/app/views/better_together/noticed/notifications/_notification.html.erb @@ -0,0 +1,2 @@ +<%# Renders a single notification in the turbo stream context %> +<%= render 'better_together/notifications/notification', notification: notification %> diff --git a/app/views/better_together/notifications/_dropdown_content.html.erb b/app/views/better_together/notifications/_dropdown_content.html.erb new file mode 100644 index 000000000..0ce01b860 --- /dev/null +++ b/app/views/better_together/notifications/_dropdown_content.html.erb @@ -0,0 +1,32 @@ + +<%= button_to t('better_together.notifications.mark_all_as_read'), + mark_all_as_read_notifications_path, + method: :post, + class: 'btn btn-secondary btn-sm mb-2', + id: 'mark_notifications_read', + data: { turbo_stream: true } if unread_count > 0 %> + +
+ <% if notifications.any? %> + <% notifications.each do |notification| %> + <% if should_cache_notification?(notification) %> + <% # Multi-level fragment caching: first by notification, then by type pattern %> + <% cache notification_fragment_cache_key(notification) do %> + <% cache notification_type_fragment_cache_key(notification) do %> + <%= render notification if notification.record.present? %> + <% end %> + <% end %> + <% else %> + <% # Fallback to simple rendering without caching for problematic notifications %> + <%= render notification if notification.record.present? %> + <% end %> + <% end %> + <% else %> + <% # Cache the empty state message %> + <% cache ["notifications_empty_state", I18n.locale] do %> +
+ <%= t('better_together.notifications.no_notifications') %> +
+ <% end %> + <% end %> +
diff --git a/app/views/better_together/notifications/_notification.html.erb b/app/views/better_together/notifications/_notification.html.erb index 4b140559e..5673f27fc 100644 --- a/app/views/better_together/notifications/_notification.html.erb +++ b/app/views/better_together/notifications/_notification.html.erb @@ -1,20 +1,31 @@
-
-
- <% if notification_url.present? %> - <%= link_to notification_title, notification_url, class: 'text-decoration-none stretched-link' %> - <% else %> - <%= notification_title %> + <% # Cache the header section separately from the dynamic content %> + <% cache ["notification_header", notification.cache_key_with_version, notification_title, notification_url, I18n.locale] do %> +
+
+ <% if notification_url.present? %> + <%= link_to notification_title, notification_url, class: 'text-decoration-none stretched-link' %> + <% else %> + <%= notification_title %> + <% end %> +
+ <%= t('better_together.notifications.time_ago', time: time_ago_in_words(notification.created_at)) %> +
+ <% end %> + + <% # Cache the specific notification content %> + <% cache ["notification_content", notification.cache_key_with_version, I18n.locale] do %> + <%= yield if block_given? %> + <% end %> + + <% # Cache the footer section based on read status %> + <% cache ["notification_footer", notification.cache_key_with_version, notification.read_at.present?, I18n.locale] do %> +
+ <% unless notification.read_at %> + <%= t('better_together.notifications.new') %> + <%= button_to t('better_together.notifications.mark_as_read'), mark_as_read_notification_path(notification), method: :post, class: 'btn btn-sm btn-outline-secondary', data: { turbo_stream: true } %> <% end %> -
- <%= t('better_together.notifications.time_ago', time: time_ago_in_words(notification.created_at)) %> -
- <%= yield if block_given? %> -
- <% unless notification.read_at %> - <%= t('better_together.notifications.new') %> - <%= button_to t('better_together.notifications.mark_as_read'), mark_as_read_notification_path(notification), method: :post, class: 'btn btn-sm btn-outline-secondary', data: { turbo_stream: true } %> - <% end %> -
+
+ <% end %> \ No newline at end of file diff --git a/app/views/better_together/test_notifier/notifications/_notification.html.erb b/app/views/better_together/test_notifier/notifications/_notification.html.erb new file mode 100644 index 000000000..ce28d47d4 --- /dev/null +++ b/app/views/better_together/test_notifier/notifications/_notification.html.erb @@ -0,0 +1,11 @@ + +<%= render layout: 'better_together/notifications/notification', + locals: { + notification: notification, + notification_title: 'Test Notification', + notification_url: '#' + } do %> +

+ Test notification content for testing purposes. +

+<% end %> diff --git a/app/views/layouts/better_together/_notifications.html.erb b/app/views/layouts/better_together/_notifications.html.erb index 14c952c24..59f1b9e79 100644 --- a/app/views/layouts/better_together/_notifications.html.erb +++ b/app/views/layouts/better_together/_notifications.html.erb @@ -1,21 +1,34 @@ -