.to_prepare to extend Noticed::Notification class doesn't work in test env

[phil-6]

Hey @excid3,

Using the suggested .to_prepare method to extend the Noticed::Notification class results in an error in the test environment.

In application.rb we have

config.to_prepare do
  Noticed::Event.include Noticed::EventExtensions
  Noticed::Notification.include Noticed::NotificationExtensions
end

and the error is

bin/rails aborted!
ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR:  relation "noticed_notifications" does not exist (ActiveRecord::StatementInvalid)
LINE 10:  WHERE a.attrelid = '"noticed_notifications"'::regclass
                             ^
/home/phil/dev/ryalto/ryalto-v4/app/models/noticed/notification_extensions.rb:16:in `block (2 levels) in <module:NotificationExtensions>'
/home/phil/dev/ryalto/ryalto-v4/app/models/noticed/notification_extensions.rb:25:in `block in <module:NotificationExtensions>'
/home/phil/dev/ryalto/ryalto-v4/config/application.rb:44:in `include'
/home/phil/dev/ryalto/ryalto-v4/config/application.rb:44:in `block in <class:Application>'
/home/phil/dev/ryalto/ryalto-v4/config/environment.rb:5:in `<main>'

Caused by:
PG::UndefinedTable: ERROR:  relation "noticed_notifications" does not exist (PG::UndefinedTable)
LINE 10:  WHERE a.attrelid = '"noticed_notifications"'::regclass
                             ^
/home/phil/dev/ryalto/ryalto-v4/app/models/noticed/notification_extensions.rb:16:in `block (2 levels) in <module:NotificationExtensions>'
/home/phil/dev/ryalto/ryalto-v4/app/models/noticed/notification_extensions.rb:25:in `block in <module:NotificationExtensions>'
/home/phil/dev/ryalto/ryalto-v4/config/application.rb:44:in `include'
/home/phil/dev/ryalto/ryalto-v4/config/application.rb:44:in `block in <class:Application>'
/home/phil/dev/ryalto/ryalto-v4/config/environment.rb:5:in `<main>'
Tasks: TOP => test:prepare => dartsass:build => environment
(See full trace by running task with --trace)

Is there something that I should be doing differently?

app/models/noticed/notification_extensions.rb

module Noticed::NotificationExtensions
  extend ActiveSupport::Concern
  EXCLUDED_TYPES = %w[Call::EndedNotifier::Notification Call::StartedNotifier::Notification
                      NewMessageNotifier::Notification SwitchedOrganisationNotifier::Notification].freeze

  included do
    delegate :organisation, :action_type, :title, :body, :url, to: :event, allow_nil: true
    delegate :id, :name, to: :organisation, prefix: true, allow_nil: true

    scope :for_index_and_counts, -> { where.not(type: EXCLUDED_TYPES) }
    scope :filter_by_org, ->(organisation_id) { joins(:event).where(event: { organisation_id: }) }
    scope :filter_by_type, ->(type) { where(type:) }
    scope :exclude_type, ->(type) { where.not(type:) }

    counter_culture :recipient, column_name: proc { |n| n.include_in_counts? ? "notifications_count" : nil },
                                column_names: { Noticed::Notification.for_index_and_counts => :notifications_count }
    counter_culture :recipient, column_name: proc { |n| 'unread_notifications_count' if n.read_at.nil? },
                                column_names: { Noticed::Notification.for_index_and_counts.unread => :unread_notifications_count } 
  end
end

[excid3]

That error says you don't have a noticed_notifications table in your test database.

[excid3]

Also rails db:test:prepare is what you want to run for testing.

Not entirely sure why that task is hidden from rails -T

[phil-6]

rails db:test:prepare was giving the same error so I was trying to break it down into smaller steps.

[phil-6]

did you have anything else in config/initializers/noticed.rb ?

[excid3]

Strange. Maybe to_prepare runs earlier there because it's happening on application load. The reason we use to_prepare is classes can be reloaded in development so we want to make sure those extensions are always applied in case Noticed models get reloaded.

# config/initializers/noticed.rb
module EventExtension
  extend ActiveSupport::Concern

  included do
  end
end

module NotificationExtension
  extend ActiveSupport::Concern

  included do
  end
end

Rails.configuration.to_prepare do
  Noticed::Event.include EventExtension
  Noticed::Notification.include NotificationExtension
end

[phil-6]

Works like a charm. Thank you

Doesn't feel as elegant though, I might have a further play tomorrow to see if I can get the extraction to separate files to work with test.

[excid3]

Should be able to move the extensions to app/models/concerns like normal. I just put them all in the same place for simplicity.

[phil-6]

Also, I've just realised I misunderstood your message here

I'm now more interested that they ran as extracted files!

[phil-6]

Hmm, okay, I think it's just too late for me!

Final change is just

renamed:    app/models/noticed/notification_extensions.rb -> app/models/concerns/noticed/notification_extensions.rb
renamed:    app/models/noticed/event_extensions.rb -> app/models/concerns/noticed/event_extensions.rb

Thanks Chris!

[stephaneliu]

Since Noticed::Notification records are created using insert_all it bypasses Counter Culture's callback hooks. @phil-6, I'm curious what you ended up doing to cache notification counts the recipient.

[phil-6]

We created a custom delivery method to update the counters.

# app/notifiers/delivery_methods/counter_culture_updates.rb
class DeliveryMethods::CounterCultureUpdates < ApplicationDeliveryMethod
  def deliver
    return unless recipient.is_a?(User) && notification.include_in_counts?

    # rubocop:disable Rails/SkipsModelValidations
    # update_columns is used for speed
    recipient.update_columns(notifications_count: recipient.notifications.for_index_and_counts.size,
                             unread_notifications_count: recipient.notifications.for_index_and_counts.unread.size)
    # rubocop:enable Rails/SkipsModelValidations
  end
end

[stephaneliu]

Nice. I was thinking of using an after_deliver hook in a custom, delivery method but using deliver is more direct. Thanks