diff --git a/.env_sample b/.env_sample index 13e99fed6f89b..5dde17f05dca7 100644 --- a/.env_sample +++ b/.env_sample @@ -90,6 +90,12 @@ NODE_ENV="development" HONEYBADGER_API_KEY="Optional" HONEYBADGER_JS_API_KEY="Optional" +# Better Stack for production logs, in addition to stdout. Off unless the token is set. +# Both values are on the source's page in Better Stack; the host defaults to in.logs.betterstack.com. +# (https://betterstack.com/docs/logs/ruby-and-rails/) +# BETTERSTACK_SOURCE_TOKEN= +# BETTERSTACK_INGESTING_HOST= + # AWS for images storages AWS_ID= AWS_SECRET= diff --git a/Gemfile b/Gemfile index bc34e6b981b7d..95ec3ae9992b9 100644 --- a/Gemfile +++ b/Gemfile @@ -64,6 +64,7 @@ gem "jwt", "2.10.3" # Verify delegated access tokens gem "kaminari", "~> 1.2" # A Scope and Engine based, clean, powerful, customizable and sophisticated paginator gem "katex", "~> 0.9.0" # This rubygem enables you to render TeX math to HTML using KaTeX. It uses ExecJS under the hood gem "liquid", "~> 5.4" # A secure, non-evaling end user template engine with aesthetic markup +gem "logtail-rails", "~> 0.2.12", require: false # Ships Rails logs to Better Stack when BETTERSTACK_SOURCE_TOKEN is set (see lib/betterstack/log_device.rb and config/initializers/betterstack.rb before upgrading) gem "metainspector", "~> 5.12" # To get and parse website metadata for Open Graph rich objects gem "mini_magick", "~> 4.13" # Manipulate images with minimal use of memory via ImageMagick / GraphicsMagick gem "nokogiri", "~> 1.18" # HTML, XML, SAX, and Reader parser @@ -105,6 +106,9 @@ gem "rouge", "~> 4.2" # A pure-ruby code highlighter gem "rss", "~> 0.2.9" # Ruby's standard library for RSS gem "rubyzip", "~> 2.4" # Rubyzip is a ruby library for reading and writing zip files gem "s3_direct_upload", "~> 0.1" # Direct Upload to Amazon S3 +gem "sentry-rails", "~> 5.19", require: false # Pilot: ships errors to Better Stack (Sentry-protocol compatible); loaded in config/application.rb +gem "sentry-ruby", "~> 5.19", require: false # Pilot: see config/initializers/sentry.rb, runs alongside Honeybadger +gem "sentry-sidekiq", "~> 5.19", require: false # Pilot: reports Sidekiq worker errors and traces jobs; loaded in config/application.rb gem "sidekiq", "~> 6.5.3" # Sidekiq is used to process background jobs with the help of Redis gem "sidekiq-throttled", "~> 1.5" # Concurrency control for Sidekiq gem "sidekiq-cron", "~> 1.7" # Allows execution of scheduled cron jobs as specific times diff --git a/Gemfile.lock b/Gemfile.lock index 2642b58cce2b0..16f27f16be8fc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -554,6 +554,17 @@ GEM rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) logger (1.7.0) + logtail (0.1.17) + msgpack (~> 1.0) + logtail-rack (0.2.6) + logtail (~> 0.1) + rack (>= 1.2, < 4.0) + logtail-rails (0.2.12) + actionpack (>= 5.0.0) + activerecord (>= 5.0.0) + logtail (~> 0.1, >= 0.1.14) + logtail-rack (~> 0.1) + railties (>= 5.0.0) loofah (2.25.1) crass (~> 1.0.2) nokogiri (>= 1.12.0) @@ -952,6 +963,15 @@ GEM faraday (>= 0.17.3, < 3) sax-machine (1.3.2) securerandom (0.4.1) + sentry-rails (5.28.1) + railties (>= 5.0) + sentry-ruby (~> 5.28.1) + sentry-ruby (5.28.1) + bigdecimal + concurrent-ruby (~> 1.0, >= 1.0.2) + sentry-sidekiq (5.28.1) + sentry-ruby (~> 5.28.1) + sidekiq (>= 3.0) sexp_processor (4.17.5) shellany (0.0.1) shoulda-matchers (5.3.0) @@ -1195,6 +1215,7 @@ DEPENDENCIES launchy (~> 2.5) liquid (~> 5.4) listen (~> 3.7) + logtail-rails (~> 0.2.12) memory_profiler (~> 1.0) metainspector (~> 5.12) mini_magick (~> 4.13) @@ -1252,6 +1273,9 @@ DEPENDENCIES rubyzip (~> 2.4) s3_direct_upload (~> 0.1) sassc-rails (~> 2.1.2) + sentry-rails (~> 5.19) + sentry-ruby (~> 5.19) + sentry-sidekiq (~> 5.19) shoulda-matchers (~> 5.3) sidekiq (~> 6.5.3) sidekiq-cron (~> 1.7) diff --git a/app/decorators/notification_decorator.rb b/app/decorators/notification_decorator.rb index ea48925bd7acc..bbde9b4f2411f 100644 --- a/app/decorators/notification_decorator.rb +++ b/app/decorators/notification_decorator.rb @@ -131,7 +131,14 @@ def article_id end def article_url - @article_url ||= json_data.dig("article", "url") || json_data.dig("article", "path") + @article_url ||= begin + url = json_data.dig("article", "url") + if url.present? && url.match?(%r{:\d+:\d+}) + json_data.dig("article", "path").presence || url.sub(%r{(:\d+):\d+}, '\1') + else + url.presence || json_data.dig("article", "path") + end + end end def article_title diff --git a/app/lib/url.rb b/app/lib/url.rb index 90bb547ad54e5..f40b45d3eb991 100644 --- a/app/lib/url.rb +++ b/app/lib/url.rb @@ -45,12 +45,20 @@ def self.domain(domain_or_subforem = nil) def self.url(uri = nil, domain_or_subforem = nil) base_url = "#{protocol}#{domain(domain_or_subforem)}" port = dev_port - base_url += ":#{port}" if Rails.env.development? && port.present? && base_url.exclude?(":#{port}") + base_url += ":#{port}" if Rails.env.development? && port.present? && !port_present?(base_url) return base_url unless uri Addressable::URI.parse(base_url).join(uri).normalize.to_s end + def self.port_present?(url) + Addressable::URI.heuristic_parse(url).port.present? + rescue Addressable::URI::InvalidURIError + false + end + + private_class_method :port_present? + # Creates an article URL # # @param article [Article] the article to create the URL for diff --git a/app/models/article.rb b/app/models/article.rb index 550e7f1a2b063..bb0870f6a8b9d 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -353,6 +353,10 @@ def self.permitted_video_source_url?(url) after_save :generate_social_image after_save :generate_context_notes + after_update_commit :notify_co_author_changes, if: proc { |article| + article.published? && article.saved_change_to_co_author_ids? + } + after_update_commit :update_notifications, if: proc { |article| article.notifications.any? && !article.saved_changes.empty? } @@ -1475,7 +1479,7 @@ def fetch_video_duration end def update_notifications - Notification.update_notifications(self, I18n.t("models.article.published")) + Notification.update_notifications(self, %w[Published CoAuthor]) end def update_notification_subscriptions @@ -2037,6 +2041,13 @@ def update_dependent_embeds_if_key_info_changed end end + def notify_co_author_changes + before, after = saved_change_to_co_author_ids + removed_user_ids = Array.wrap(before).map(&:to_i) - Array.wrap(after).map(&:to_i) + + Notification.send_to_co_authors(self, removed_user_ids) + end + def cleanup_memberships_if_unpublished concept_memberships.destroy_all trend_memberships.destroy_all diff --git a/app/models/notification.rb b/app/models/notification.rb index 599d52b4a2bc0..7fa15d54f4818 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -13,7 +13,7 @@ class Notification < ApplicationRecord before_create :mark_notified_at_time after_commit :cleanup_old_notifications, on: :create - scope :for_published_articles, -> { where(notifiable_type: "Article", action: "Published") } + scope :for_published_articles, -> { where(notifiable_type: "Article", action: %w[Published CoAuthor]) } scope :for_comments, -> { where(notifiable_type: "Comment", action: nil) } # nil action means "not a reaction" scope :for_mentions, -> { where(notifiable_type: "Mention") } @@ -73,6 +73,16 @@ def send_to_mentioned_users_and_followers(notifiable, _action = nil) # Kicks off a worker to send any notifications about the post being published, if necessary. Notification.send_to_followers(notifiable, "Published") + + # Credited co-authors are notified separately from followers. + Notification.send_to_co_authors(notifiable) + end + + def send_to_co_authors(notifiable, removed_user_ids = []) + return unless notifiable.is_a?(Article) + return if notifiable.co_author_ids.blank? && removed_user_ids.blank? + + Notifications::CoAuthorWorker.perform_async(notifiable.id, removed_user_ids) end def send_to_followers(notifiable, action = nil) diff --git a/app/services/articles/updater.rb b/app/services/articles/updater.rb index f8825a214c3a4..f8148a9a2740b 100644 --- a/app/services/articles/updater.rb +++ b/app/services/articles/updater.rb @@ -83,6 +83,8 @@ def send_to_mentioned_users_and_followers def remove_all_notifications Notification.remove_all_by_action_without_delay(notifiable_ids: article.id, notifiable_type: "Article", action: "Published") + Notification.remove_all_by_action_without_delay(notifiable_ids: article.id, notifiable_type: "Article", + action: "CoAuthor") ContextNotification.delete_by(context_id: article.id, context_type: "Article", action: "Published") diff --git a/app/services/moderator/unpublish_all_articles.rb b/app/services/moderator/unpublish_all_articles.rb index 4836a187dc311..f6fc3759b089f 100644 --- a/app/services/moderator/unpublish_all_articles.rb +++ b/app/services/moderator/unpublish_all_articles.rb @@ -56,6 +56,9 @@ def clean_up_notifications(article) Notification.remove_all_by_action_without_delay(notifiable_ids: article.id, notifiable_type: "Article", action: "Published") + Notification.remove_all_by_action_without_delay(notifiable_ids: article.id, + notifiable_type: "Article", + action: "CoAuthor") ContextNotification.delete_by(context_id: article.id, context_type: "Article", action: "Published") diff --git a/app/services/notifications/co_author/remove.rb b/app/services/notifications/co_author/remove.rb new file mode 100644 index 0000000000000..db901201cb4f4 --- /dev/null +++ b/app/services/notifications/co_author/remove.rb @@ -0,0 +1,32 @@ +# Removes co-author notifications for users who are no longer credited. +module Notifications + module CoAuthor + class Remove + def self.call(...) + new(...).call + end + + # @param article_id [Integer] + # @param user_ids [Array] + def initialize(article_id, user_ids) + @article_id = article_id + @user_ids = Array.wrap(user_ids).map(&:to_i) + end + + def call + return if user_ids.empty? + + Notification.where( + user_id: user_ids, + notifiable_id: article_id, + notifiable_type: "Article", + action: Notifications::CoAuthor::Send::ACTION, + ).delete_all + end + + private + + attr_reader :article_id, :user_ids + end + end +end diff --git a/app/services/notifications/co_author/send.rb b/app/services/notifications/co_author/send.rb new file mode 100644 index 0000000000000..2129223b42b1e --- /dev/null +++ b/app/services/notifications/co_author/send.rb @@ -0,0 +1,76 @@ +# Notifies co-authors that a post they are credited on is live. +module Notifications + module CoAuthor + class Send + ACTION = "CoAuthor".freeze + + def self.call(...) + new(...).call + end + + # @param article [Article] + def initialize(article) + @article = article + end + + delegate :user_data, :article_data, :organization_data, to: Notifications + + def call + return unless article.is_a?(Article) + return unless article.published? && !article.scheduled? && article.type_of == "full_post" + + recipient_ids = Array.wrap(article.co_author_ids).map(&:to_i) - [article.user_id] + return if recipient_ids.empty? + + # Skip anyone already notified so re-publishing or an unrelated edit + # does not notify the same co-author twice. + already_notified = Notification.where( + user_id: recipient_ids, + notifiable_id: article.id, + notifiable_type: "Article", + action: ACTION, + ).pluck(:user_id) + + pending_ids = recipient_ids - already_notified + return if pending_ids.empty? + + now = Time.current + attributes = User.where(id: pending_ids).ids.map do |user_id| + { + user_id: user_id, + notifiable_id: article.id, + notifiable_type: "Article", + subforem_id: article.subforem_id, + action: ACTION, + json_data: json_data, + created_at: now, + notified_at: now, + updated_at: now + } + end + return if attributes.empty? + + # upsert rather than insert: a unique index covers + # (user_id, notifiable_id, notifiable_type, action), and two publishes + # racing would otherwise raise instead of no-opping. + Notification.upsert_all( + attributes, + unique_by: :index_notifications_on_user_notifiable_and_action_not_null, + ) + end + + private + + attr_reader :article + + def json_data + data = { + user: user_data(article.user), + article: article_data(article) + } + data[:organization] = organization_data(article.organization) if article.organization_id + data + end + end + end +end diff --git a/app/services/notifications/notifiable_action/send.rb b/app/services/notifications/notifiable_action/send.rb index ff01d30a6876b..0d895c7e5c29a 100644 --- a/app/services/notifications/notifiable_action/send.rb +++ b/app/services/notifications/notifiable_action/send.rb @@ -32,12 +32,16 @@ def call # have a mention in order to avoid sending a user multiple notifications for one article. user_ids_with_article_mentions = notifiable.mentions&.pluck(:user_id) + # Co-authors receive their own notification, so exclude them here for the + # same reason mentions are excluded: one notification per person per post. + co_author_ids = Array.wrap(notifiable.co_author_ids).map(&:to_i) + article_followers = User.joins("INNER JOIN follows ON follows.follower_id = users.id") .where("(follows.followable_id = ? AND follows.followable_type = ?) OR (follows.followable_id = ? AND follows.followable_type = ?)", notifiable&.user&.id, "User", notifiable&.organization&.id, "Organization") .where(follows: { subscription_status: "all_articles" }) - .where.not(id: (user_ids_with_article_mentions + [notifiable.user])) + .where.not(id: (user_ids_with_article_mentions + co_author_ids + [notifiable.user])) .recently_active(FOLLOWER_SEND_LIMIT).distinct article_followers.find_each do |follower| diff --git a/app/views/notifications/_article.html.erb b/app/views/notifications/_article.html.erb index e14436811aa33..afad863a4abd5 100644 --- a/app/views/notifications/_article.html.erb +++ b/app/views/notifications/_article.html.erb @@ -11,6 +11,15 @@

<%= time_ago_in_words json_data["article"]["published_at"], scope: :"datetime.distance_in_words_ago" %>

+ <%= render "notifications/shared/article_preview", notification: notification, context: "default" %> + <% elsif notification.action == "CoAuthor" %> +
+

+ <%= message_user_acted_maybe_org(json_data, "views.notifications.co_author.verb_html", if_org: "views.notifications.co_author.if_org_html") %> +

+

<%= time_ago_in_words json_data["article"]["published_at"], scope: :"datetime.distance_in_words_ago" %>

+
+ <%= render "notifications/shared/article_preview", notification: notification, context: "default" %> <% elsif notification.action == "Moderation" %> <% new_user = mod_article_user(json_data) %> diff --git a/app/workers/notifications/co_author_worker.rb b/app/workers/notifications/co_author_worker.rb new file mode 100644 index 0000000000000..55cb0fe8fa374 --- /dev/null +++ b/app/workers/notifications/co_author_worker.rb @@ -0,0 +1,16 @@ +module Notifications + class CoAuthorWorker + include Sidekiq::Job + + sidekiq_options queue: :low_priority, lock: :until_executing, on_conflict: :replace, retry: 10 + + def perform(article_id, removed_user_ids = []) + Notifications::CoAuthor::Remove.call(article_id, removed_user_ids) + + article = Article.find_by(id: article_id) + return unless article + + Notifications::CoAuthor::Send.call(article) + end + end +end diff --git a/config/application.rb b/config/application.rb index 3618d3e61f73a..814f32b9b818b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -19,6 +19,15 @@ # you've limited to :test, :development, or :production. Bundler.require(*Rails.groups) +# Better Stack's Rails integration has to load before the app boots; only load it when configured +# (see config/initializers/betterstack.rb). +require "logtail-rails" if Rails.env.production? && ENV["BETTERSTACK_SOURCE_TOKEN"].present? +# Same for the Better Stack errors pilot (see config/initializers/sentry.rb). +if Rails.env.production? && ENV["BETTER_STACK_ERRORS_DSN"].present? + require "sentry-rails" + require "sentry-sidekiq" +end + if defined?(Anyway) Anyway.loaders.delete(:secrets) end diff --git a/config/initializers/betterstack.rb b/config/initializers/betterstack.rb new file mode 100644 index 0000000000000..e6ebd15ae3a83 --- /dev/null +++ b/config/initializers/betterstack.rb @@ -0,0 +1,56 @@ +# Ship production logs to Better Stack as well as to the existing logger (stdout on Heroku). +# Off unless BETTERSTACK_SOURCE_TOKEN is set; logtail-rails is only loaded then (config/application.rb). +if Rails.env.production? && ENV["BETTERSTACK_SOURCE_TOKEN"].present? + require Rails.root.join("lib/betterstack/log_device") + require Rails.root.join("lib/betterstack/request_context") + + betterstack_logger = Logtail::Logger.new( + Betterstack::LogDevice.new( + ENV.fetch("BETTERSTACK_SOURCE_TOKEN"), + # Falls back to logtail's default host when unset. + ingesting_host: ENV["BETTERSTACK_INGESTING_HOST"].presence, + ), + ) + # Set BETTERSTACK_LOG_LEVEL=info to also get logtail-rails' per-request events; stdout keeps LOG_LEVEL. + betterstack_logger.level = ENV["BETTERSTACK_LOG_LEVEL"].presence&.to_sym || Rails.logger.level + + # logtail-rails' request events go only to Better Stack, so stdout keeps Rails' own lines. + # Rails' log subscribers and Rack logger stay in place for the same reason. ErrorEvent is off + # because Rails already logs every unhandled exception; it would ship each one twice. + Logtail.config.logger = betterstack_logger + [ + Logtail::Integrations::ActionController, + Logtail::Integrations::ActionView, + Logtail::Integrations::ActiveRecord, + Logtail::Integrations::Rails::RackLogger, + Logtail::Integrations::Rails::ErrorEvent, + Logtail::Integrations::Rack::HTTPContext, + ].each { |integration| integration.enabled = false } + # Above DebugExceptions, so unhandled exceptions carry the request too (see RequestContext). + Rails.application.config.middleware.insert_after ActionDispatch::RequestId, Betterstack::RequestContext + # Only the user's id: logtail-rack would otherwise send names and emails. + Logtail::Integrations::Rack::UserContext.custom_user_hash = lambda do |env| + (user = env["warden"]&.user) && { id: user.id } + end + # Request events (info level) record headers; never send credentials. One event per request. + Logtail::Integrations::Rack::HTTPEvents.http_header_filters = %w[ + Authorization Proxy-Authorization Cookie Set-Cookie X-CSRF-Token + api-key x-algolia-api-key health-check-token + ] + Logtail::Integrations::Rack::HTTPEvents.collapse_into_single_event = true + + # Exceptions Rails already turns into responses (404s, CSRF, routing, Pundit; see + # rescue_responses) are logged with full traces. Keep them on stdout, not in Better Stack. + rescued_classes = Regexp.union(ActionDispatch::ExceptionWrapper.rescue_responses.keys) + rescued_exception = /\A\s*(?:\[[^\]]*\]\s*)*(?:#{rescued_classes}) \(/ + # Rails' own request lines duplicate the request event above. + request_line = /\A(?:Started [A-Z]+ "| Parameters: |Completed \d{3} )/ + Logtail.config.filter_sent_to_better_stack do |entry| + message = entry.message.to_s + message.match?(rescued_exception) || (entry.event.nil? && message.match?(request_line)) + end + + # Logtail::Logger must not respond to #tagged: BroadcastLogger runs a tagged block once per + # logger that does, so ActiveJob#perform_now would run every job twice. + Rails.logger.broadcast_to(betterstack_logger) +end diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb new file mode 100644 index 0000000000000..5fcee905054bd --- /dev/null +++ b/config/initializers/sentry.rb @@ -0,0 +1,46 @@ +# PILOT: sends errors and sampled traces to Better Stack via its +# Sentry-compatible ingest, running alongside Honeybadger. Honeybadger remains the alerting +# source of truth. sentry-rails is only loaded when BETTER_STACK_ERRORS_DSN is +# set (config/application.rb, with sentry-sidekiq); unset it to disable entirely. +if Rails.env.production? && ENV["BETTER_STACK_ERRORS_DSN"].present? + # Mirrors config/initializers/honeybadger.rb so Better Stack's counts are + # comparable: the same ignored classes, and the same classes collapsed into + # one error each instead of being dropped. + ignored_exceptions = %w[ + ActiveRecord::QueryCanceled + ActiveRecord::RecordNotFound + Pundit::NotAuthorizedError + RateLimitChecker::LimitReached + ] + message_fingerprints = { + "Rack::Timeout::RequestTimeoutException" => "rack_timeout", + "Rack::Timeout::RequestTimeoutError" => "rack_timeout", + "PG::QueryCanceled" => "pg_query_canceled" + } + + Sentry.init do |config| + config.dsn = ENV.fetch("BETTER_STACK_ERRORS_DSN") + config.enabled_environments = %w[production] + config.environment = ENV.fetch("SENTRY_ENVIRONMENT", Rails.env) + config.release = ENV.fetch("HEROKU_SLUG_COMMIT", nil) + config.breadcrumbs_logger = [:active_support_logger] + # Sampled request and job traces; Better Stack stores them as spans. + config.traces_sample_rate = ENV.fetch("SENTRY_TRACES_SAMPLE_RATE", "0.05").to_f + # Like Honeybadger's attempt_threshold: skip job failures that will be retried. + config.sidekiq.report_after_job_retries = true + config.excluded_exceptions += ignored_exceptions + config.inspect_exception_causes_for_exclusion = true + + config.before_send = lambda do |event, hint| + exception = hint[:exception] + next if exception.is_a?(SignalException) && exception.message.include?("SIGHUP") + + # Honeybadger matches its error message ("Class: message"), which also + # catches these classes when they arrive wrapped in another exception. + error_message = "#{exception.class.name}: #{exception&.message}" + fingerprint = message_fingerprints.detect { |key, _| error_message.include?(key) }&.last + event.fingerprint = [fingerprint] if fingerprint + event + end + end +end diff --git a/config/initializers/zeitwerk.rb b/config/initializers/zeitwerk.rb index 4da82d4fd1d13..b81646759aaa0 100644 --- a/config/initializers/zeitwerk.rb +++ b/config/initializers/zeitwerk.rb @@ -16,3 +16,6 @@ Rails.autoloaders.main.ignore(Rails.root.join("lib/generators/service")) Rails.autoloaders.main.ignore(Rails.root.join("lib/generators/settings_model")) Rails.autoloaders.main.ignore(Rails.root.join("lib/cypress-rails")) + +# Required by config/initializers/betterstack.rb only when Better Stack is configured +Rails.autoloaders.main.ignore(Rails.root.join("lib/betterstack")) diff --git a/config/locales/views/notifications/en.yml b/config/locales/views/notifications/en.yml index 6d7b52cd5f62f..161a785c5ff81 100644 --- a/config/locales/views/notifications/en.yml +++ b/config/locales/views/notifications/en.yml @@ -27,6 +27,9 @@ en: settings: Settings event: reminder_html: "Reminder: %{event_link} starts in %{time}!" + co_author: + if_org_html: " under %{org}" + verb_html: "%{user} published a post you are credited on%{if_org}" comment: commented_html: "%{user} commented on %{title}" first_html: "%{user} wrote their first comment on %{title}" diff --git a/config/locales/views/notifications/fr.yml b/config/locales/views/notifications/fr.yml index ece0b299e39e0..1f300701d7905 100644 --- a/config/locales/views/notifications/fr.yml +++ b/config/locales/views/notifications/fr.yml @@ -27,6 +27,9 @@ fr: settings: Settings event: reminder_html: "Rappel : %{event_link} commence dans %{time} !" + co_author: + if_org_html: " sous %{org}" + verb_html: "%{user} a publié un article dont vous êtes co-auteur%{if_org}" comment: commented_html: "%{user} commented on %{title}" first_html: "%{user} wrote their first comment on %{title}" diff --git a/config/locales/views/notifications/pt.yml b/config/locales/views/notifications/pt.yml index 90e29a85a997d..3ae7ccdc8cc29 100644 --- a/config/locales/views/notifications/pt.yml +++ b/config/locales/views/notifications/pt.yml @@ -27,6 +27,9 @@ pt: settings: Configurações event: reminder_html: "Lembrete: %{event_link} começa em %{time}!" + co_author: + if_org_html: " em %{org}" + verb_html: "%{user} publicou um post em que você é coautor%{if_org}" comment: commented_html: "%{user} comentou em %{title}" first_html: "%{user} escreveu seu primeiro comentário em %{title}" diff --git a/lib/betterstack/log_device.rb b/lib/betterstack/log_device.rb new file mode 100644 index 0000000000000..3973dcb21bc89 --- /dev/null +++ b/lib/betterstack/log_device.rb @@ -0,0 +1,38 @@ +require "logtail" + +module Betterstack + # Logtail's HTTP log device, fixed for running in every Puma worker and Sidekiq process. + # Only loaded when BETTERSTACK_SOURCE_TOKEN is set (see config/initializers/betterstack.rb). + # + # The overridden methods are private in logtail; spec/lib/betterstack/log_device_spec.rb + # fails if an upgrade renames them. + class LogDevice < Logtail::LogDevices::HTTP + # Upstream reconnects immediately after a failed connection, so a fast failure (refused, + # TLS error, unknown host) keeps a CPU core busy until Better Stack is reachable again. + RECONNECT_INTERVAL = 1 # second + + # Upstream waits up to 20 seconds for undelivered logs when the process exits. Heroku + # sends SIGKILL 30 seconds after SIGTERM, so don't let an outage hold up shutdown. + FLUSH_TIMEOUT = 5 # seconds + + private + + def build_http + wait = RECONNECT_INTERVAL - (monotonic_now - @last_connect_at) if @last_connect_at + sleep(wait) if wait&.positive? + @last_connect_at = monotonic_now + + # Upstream turns off TLS certificate verification. + super.tap { |http| http.verify_mode = OpenSSL::SSL::VERIFY_PEER } + end + + def wait_on_request_queue + deadline = monotonic_now + FLUSH_TIMEOUT + sleep(0.1) while (@request_queue.size.positive? || @requests_in_flight.positive?) && monotonic_now < deadline + end + + def monotonic_now + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end + end +end diff --git a/lib/betterstack/request_context.rb b/lib/betterstack/request_context.rb new file mode 100644 index 0000000000000..5eed8dfaf09c4 --- /dev/null +++ b/lib/betterstack/request_context.rb @@ -0,0 +1,26 @@ +require "logtail" + +module Betterstack + # Adds the request (host, method, path, client IP, request id) to every Better Stack line logged + # during it. Replaces logtail-rack's HTTPContext, which config.app_middleware puts below + # ActionDispatch::DebugExceptions: its context is gone by the time Rails logs an unhandled exception. + # Only loaded when BETTERSTACK_SOURCE_TOKEN is set (see config/initializers/betterstack.rb). + class RequestContext + def initialize(app) + @app = app + end + + def call(env) + request = ActionDispatch::Request.new(env) + context = Logtail::Contexts::HTTP.new( + host: request.host, + method: request.request_method, + path: request.path, + # Same client IP as the rest of Forem: Fastly's header, else Rails' remote_ip. + remote_addr: (env["HTTP_FASTLY_CLIENT_IP"] || request.remote_ip).to_s, + request_id: request.request_id, + ) + Logtail::CurrentContext.with(context.to_hash) { @app.call(env) } + end + end +end diff --git a/spec/decorators/notification_decorator_spec.rb b/spec/decorators/notification_decorator_spec.rb index c43996ef5941e..9e79fe3e7866e 100644 --- a/spec/decorators/notification_decorator_spec.rb +++ b/spec/decorators/notification_decorator_spec.rb @@ -331,6 +331,23 @@ expect(decorated.article_updated_at).to eq("2023-06-02T06:55:53.406Z") end + it "falls back to path or cleans malformed double ports in article_url" do + double_port_notif = build(:notification, json_data: { + "article" => { + "url" => "http://localhost:3000:3100/a_user/article-here", + "path" => "/a_user/article-here", + }, + }).decorate + expect(double_port_notif.article_url).to eq("/a_user/article-here") + + no_path_notif = build(:notification, json_data: { + "article" => { + "url" => "http://localhost:3000:3100/a_user/article-here", + }, + }).decorate + expect(no_path_notif.article_url).to eq("http://localhost:3000/a_user/article-here") + end + it "responds to comment and commentable fields (even if blank)" do expect(decorated.comment_id).to be_blank expect(decorated.commentable_article_id).to be_blank diff --git a/spec/initializers/betterstack_spec.rb b/spec/initializers/betterstack_spec.rb new file mode 100644 index 0000000000000..67e77c0189f20 --- /dev/null +++ b/spec/initializers/betterstack_spec.rb @@ -0,0 +1,111 @@ +require "rails_helper" +require "logtail-rails" +require Rails.root.join("lib/betterstack/log_device") + +RSpec.describe "Better Stack initializer" do # rubocop:disable RSpec/DescribeClass + let(:initializer_path) { Rails.root.join("config/initializers/betterstack.rb") } + let(:stdout) { StringIO.new } + let(:stdout_logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(stdout)) } + let(:rails_logger) { ActiveSupport::BroadcastLogger.new(stdout_logger).tap { |logger| logger.level = :error } } + let(:device) { Betterstack::LogDevice.new("token", flush_continuously: false) } + + before do + allow(Rails).to receive(:logger).and_return(rails_logger) + allow(Rails.env).to receive(:production?).and_return(true) + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("BETTERSTACK_SOURCE_TOKEN").and_return("token") + allow(ENV).to receive(:[]).with("BETTERSTACK_INGESTING_HOST").and_return("in.logs.example.com") + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("BETTERSTACK_SOURCE_TOKEN").and_return("token") + allow(Betterstack::LogDevice).to receive(:new).and_return(device) + allow(device).to receive(:write).and_return(true) + # The app's middleware stack is frozen after boot. + allow(Rails.application.config.middleware).to receive(:insert_after) + end + + it "adds a Better Stack logger at the Rails log level" do + load initializer_path + + betterstack_logger = rails_logger.broadcasts.last + expect(rails_logger.broadcasts.first).to be(stdout_logger) + expect(betterstack_logger).to be_a(Logtail::Logger) + expect(betterstack_logger.level).to eq(Logger::ERROR) + expect(Betterstack::LogDevice).to have_received(:new).with("token", ingesting_host: "in.logs.example.com") + end + + it "sends lines at or above the Rails log level to Better Stack and stdout" do + load initializer_path + + rails_logger.warn("ignored") + rails_logger.error("boom") + + expect(device).to have_received(:write).once.with(having_attributes(level: :error, message: "boom")) + expect(stdout.string).to include("boom") + expect(stdout.string).not_to include("ignored") + end + + it "sends logtail-rails events only to Better Stack, with the user's id and no other user data" do + load initializer_path + + expect(Logtail.config.logger).to be(rails_logger.broadcasts.last) + expect(Logtail::Integrations::ActionController).not_to be_enabled + expect(Logtail::Integrations::Rails::ErrorEvent).not_to be_enabled + expect(Rails.application.config.middleware).to have_received(:insert_after) + .with(ActionDispatch::RequestId, Betterstack::RequestContext) + user = instance_double(User, id: 7, name: "Ada", email: "ada@example.com") + env = { "warden" => instance_double(Warden::Proxy, user: user) } + expect(Logtail::Integrations::Rack::UserContext.custom_user_hash.call(env)).to eq(id: 7) + end + + it "keeps exceptions Rails rescues into responses out of Better Stack, but on stdout" do + sent = [] + # Upstream's LogDevice#write applies the filter first; device.write is stubbed above. + allow(device).to receive(:write) { |entry| sent << entry.message if Logtail.config.send_to_better_stack?(entry) } + load initializer_path + + rails_logger.error(" \n[req-1] ActiveRecord::RecordNotFound (Not Found):\n app/x.rb:1") + rails_logger.error("[req-2] Pundit::NotAuthorizedError (not allowed)") + rails_logger.error("PG::NotNullViolation (null value)\nCaused by: ActiveRecord::RecordNotFound (x)") + + expect(sent).to eq(["PG::NotNullViolation (null value)\nCaused by: ActiveRecord::RecordNotFound (x)"]) + expect(stdout.string).to include("RecordNotFound", "Pundit::NotAuthorizedError") + end + + it "runs tagged blocks, like ActiveJob#perform_now, once" do + load initializer_path + runs = 0 + + rails_logger.tagged("ActiveJob") do + runs += 1 + rails_logger.error("inside a job") + end + + expect(runs).to eq(1) + expect(device).to have_received(:write).once + expect(stdout.string).to include("[ActiveJob] inside a job") + end + + it "falls back to logtail's default host when BETTERSTACK_INGESTING_HOST is unset" do + allow(ENV).to receive(:[]).with("BETTERSTACK_INGESTING_HOST").and_return(nil) + + load initializer_path + + expect(Betterstack::LogDevice).to have_received(:new).with("token", ingesting_host: nil) + end + + it "does nothing without BETTERSTACK_SOURCE_TOKEN" do + allow(ENV).to receive(:[]).with("BETTERSTACK_SOURCE_TOKEN").and_return(nil) + + load initializer_path + + expect(rails_logger.broadcasts).to eq([stdout_logger]) + end + + it "does nothing outside production" do + allow(Rails.env).to receive(:production?).and_return(false) + + load initializer_path + + expect(rails_logger.broadcasts).to eq([stdout_logger]) + end +end diff --git a/spec/lib/betterstack/log_device_spec.rb b/spec/lib/betterstack/log_device_spec.rb new file mode 100644 index 0000000000000..6a7eaede9a81c --- /dev/null +++ b/spec/lib/betterstack/log_device_spec.rb @@ -0,0 +1,49 @@ +require "rails_helper" +require Rails.root.join("lib/betterstack/log_device") + +RSpec.describe Betterstack::LogDevice do + # Nothing listens on this port, so nothing is delivered + let(:unreachable_options) do + { ingesting_host: "127.0.0.1", ingesting_port: 9, ingesting_scheme: "http", flush_continuously: false } + end + + it "overrides methods logtail still defines" do + expect(Logtail::LogDevices::HTTP.private_instance_methods(false)) + .to include(:build_http, :wait_on_request_queue) + end + + it "verifies Better Stack's TLS certificate" do + http = described_class.new("token", ingesting_host: "in.logs.example.com").__send__(:build_http) + + expect(http.use_ssl?).to be(true) + expect(http.verify_mode).to eq(OpenSSL::SSL::VERIFY_PEER) + end + + it "uses logtail's default host when none is given" do + http = described_class.new("token", ingesting_host: nil).__send__(:build_http) + + expect(http.address).to eq(Logtail::LogDevices::HTTP::DEFAULT_INGESTING_HOST) + end + + it "waits between connection attempts" do + device = described_class.new("token", unreachable_options) + allow(device).to receive(:sleep) + + device.__send__(:build_http) + expect(device).not_to have_received(:sleep) + + device.__send__(:build_http) + expect(device).to have_received(:sleep).with(a_value_within(0.1).of(described_class::RECONNECT_INTERVAL)) + end + + it "stops waiting for undelivered logs after FLUSH_TIMEOUT" do + stub_const("#{described_class}::FLUSH_TIMEOUT", 0.2) + device = described_class.new("token", unreachable_options) + device.write(Logtail::LogEntry.new(:error, Time.current, nil, "boom", {}, nil)) + + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + device.flush + + expect(Process.clock_gettime(Process::CLOCK_MONOTONIC) - started).to be < 1 + end +end diff --git a/spec/lib/betterstack/request_context_spec.rb b/spec/lib/betterstack/request_context_spec.rb new file mode 100644 index 0000000000000..be7110a9e3f27 --- /dev/null +++ b/spec/lib/betterstack/request_context_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" +require Rails.root.join("lib/betterstack/request_context") + +RSpec.describe Betterstack::RequestContext do + def context_during(env) + seen = nil + described_class.new(->(_) { seen = Logtail::CurrentContext.instance.snapshot }).call(env) + seen[:http] + end + + let(:env) do + Rack::MockRequest.env_for("https://dev.to/some/path", "REMOTE_ADDR" => "167.82.161.32", + "action_dispatch.request_id" => "req-1") + end + + it "adds the request to the log context while the request runs" do + expect(context_during(env)).to include(host: "dev.to", method: "GET", path: "/some/path", request_id: "req-1") + expect(Logtail::CurrentContext.instance.snapshot).not_to have_key(:http) + end + + it "uses Fastly's client IP, like the rest of Forem" do + env["HTTP_FASTLY_CLIENT_IP"] = "198.51.100.23" + + expect(context_during(env)).to include(remote_addr: "198.51.100.23") + end + + it "falls back to Rails' remote_ip" do + expect(context_during(env)).to include(remote_addr: "167.82.161.32") + end +end diff --git a/spec/lib/url_spec.rb b/spec/lib/url_spec.rb index 56227da5d3c0c..1dd06331f0717 100644 --- a/spec/lib/url_spec.rb +++ b/spec/lib/url_spec.rb @@ -130,6 +130,12 @@ expect(described_class.url).to eq("https://localhost:3005") end + it "does not append dev_port when the domain already contains a different port" do + allow(described_class).to receive(:dev_port).and_return("3100") + allow(Settings::General).to receive(:app_domain).and_return("localhost:3000") + expect(described_class.url).to eq("https://localhost:3000") + end + it "omits the port entirely when dev_port is blank, for a TLS proxy in front of the app" do allow(described_class).to receive(:dev_port).and_return("") expect(described_class.url).to eq("https://localhost") diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index 2b3a8a9d8a15e..8ee18f24b45f6 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -4279,4 +4279,35 @@ def foo(): expect(Organizations::RecompilePagesWorker).to have_received(:perform_async).with(organization.id) end end + + describe "#notify_co_author_changes" do + let(:author) { create(:user) } + let(:co_author1) { create(:user) } + let(:co_author2) { create(:user) } + + before do + allow(Notification).to receive(:send_to_co_authors) + end + + it "notifies co-authors when co_author_ids changes on a published article" do + article = create(:article, user: author, published: true, co_author_ids: [co_author1.id]) + article.update!(co_author_ids: [co_author1.id, co_author2.id]) + + expect(Notification).to have_received(:send_to_co_authors).with(article, []) + end + + it "passes removed user ids when a co-author is dropped from a published article" do + article = create(:article, user: author, published: true, co_author_ids: [co_author1.id, co_author2.id]) + article.update!(co_author_ids: [co_author1.id]) + + expect(Notification).to have_received(:send_to_co_authors).with(article, [co_author2.id]) + end + + it "does not notify when co_author_ids changes on an unpublished draft" do + article = create(:article, user: author, published: false, co_author_ids: [co_author1.id]) + article.update!(co_author_ids: [co_author1.id, co_author2.id]) + + expect(Notification).not_to have_received(:send_to_co_authors) + end + end end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index f4f53dc4d5515..20855a04e86f0 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -559,6 +559,21 @@ def skip_notifications_for_status_article?(article) expected_notification_organization_id = described_class.last.json_data["organization"]["id"] expect(expected_notification_organization_id).to eq(organization.id) end + + it "updates CoAuthor notifications with the new article title" do + co_author = create(:user) + article.update(co_author_ids: [co_author.id]) + sidekiq_perform_enqueued_jobs { described_class.send_to_co_authors(article) } + + new_title = "Brand New Co-Authored Title" + new_body_markdown = article.body_markdown.gsub(article.title, new_title) + article.update(title: new_title, body_markdown: new_body_markdown) + described_class.update_notifications(article, %w[Published CoAuthor]) + sidekiq_perform_enqueued_jobs + + co_author_notification = co_author.notifications.find_by(action: "CoAuthor") + expect(co_author_notification.json_data["article"]["title"]).to eq(new_title) + end end end @@ -695,4 +710,47 @@ def skip_notifications_for_status_article?(article) end end end + + describe ".send_to_co_authors" do + before do + allow(Notifications::CoAuthorWorker).to receive(:perform_async) + end + + it "enqueues CoAuthorWorker when co_author_ids are present" do + article = create(:article, co_author_ids: [user.id]) + described_class.send_to_co_authors(article) + + expect(Notifications::CoAuthorWorker).to have_received(:perform_async).with(article.id, []) + end + + it "enqueues CoAuthorWorker when removed_user_ids are present even if co_author_ids is empty" do + article = create(:article, co_author_ids: []) + described_class.send_to_co_authors(article, [user.id]) + + expect(Notifications::CoAuthorWorker).to have_received(:perform_async).with(article.id, [user.id]) + end + + it "skips enqueuing when both co_author_ids and removed_user_ids are blank" do + article = create(:article, co_author_ids: []) + described_class.send_to_co_authors(article, []) + + expect(Notifications::CoAuthorWorker).not_to have_received(:perform_async) + end + + it "skips enqueuing when notifiable is not an Article" do + described_class.send_to_co_authors(comment, [user.id]) + + expect(Notifications::CoAuthorWorker).not_to have_received(:perform_async) + end + end + + describe ".for_published_articles" do + it "includes both Published and CoAuthor notifications" do + published_notification = create(:notification, notifiable: article, action: "Published") + co_author_notification = create(:notification, notifiable: article, action: "CoAuthor") + _other_notification = create(:notification, notifiable: article, action: "Moderation") + + expect(described_class.for_published_articles).to contain_exactly(published_notification, co_author_notification) + end + end end diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb index 1e6e4ed3ef54f..e22bb88022336 100644 --- a/spec/requests/notifications_spec.rb +++ b/spec/requests/notifications_spec.rb @@ -786,6 +786,55 @@ def renders_visit_profile_button end end + context "when a user has a new co-author notification" do + let(:co_author) { create(:user) } + let(:article) { create(:article, user_id: user.id, co_author_ids: [co_author.id]) } + + before do + sidekiq_perform_enqueued_jobs do + Notification.send_to_co_authors(article) + end + sign_in co_author + end + + it "renders the proper message in GET /notifications", :aggregate_failures do + get "/notifications" + + expect(response.body).to include "published a post you are credited on" + renders_article_path(article) + renders_authors_name(article) + renders_article_published_at(article) + end + + it "renders cleanly in GET /notifications/posts filter", :aggregate_failures do + get "/notifications/posts" + + expect(response.body).to include "published a post you are credited on" + renders_article_path(article) + renders_authors_name(article) + end + end + + context "when a user has a new co-author notification with an organization" do + let(:co_author) { create(:user) } + let(:article) do + create(:organization_membership, user: co_author, organization: organization) + create(:article, user_id: user.id, organization_id: organization.id, co_author_ids: [co_author.id]) + end + + it "renders the organization credit", :aggregate_failures do + sidekiq_perform_enqueued_jobs { Notification.send_to_co_authors(article) } + sign_in co_author + + get "/notifications" + + expect(response.body).to include( + "under " \ + "#{CGI.escapeHTML(organization.name)}", + ) + end + end + context "when a user is an admin" do let(:admin) { create(:user, :super_admin) } let(:user2) { create(:user) } diff --git a/spec/services/articles/updater_spec.rb b/spec/services/articles/updater_spec.rb index 9dbfe250bf890..5ff1ac8839dc8 100644 --- a/spec/services/articles/updater_spec.rb +++ b/spec/services/articles/updater_spec.rb @@ -300,6 +300,13 @@ # expect(ContextNotification).to have_received(:delete_all) end + it "destroys preexisting co-author notifications" do + allow(Notification).to receive(:remove_all_by_action_without_delay).and_call_original + described_class.call(user, article, attributes) + attrs = { notifiable_ids: article.id, notifiable_type: "Article", action: "CoAuthor" } + expect(Notification).to have_received(:remove_all_by_action_without_delay).with(attrs) + end + it "destroys the preexisting context notifications" do create(:context_notification, context: article, action: "Published") expect do diff --git a/spec/services/notifications/co_author/remove_spec.rb b/spec/services/notifications/co_author/remove_spec.rb new file mode 100644 index 0000000000000..3228009d25476 --- /dev/null +++ b/spec/services/notifications/co_author/remove_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" + +RSpec.describe Notifications::CoAuthor::Remove, type: :service do + let(:author) { create(:user) } + let(:co_author) { create(:user) } + let(:article) { create(:article, user: author, published: true, co_author_ids: [co_author.id]) } + + def co_author_notifications + Notification.where(notifiable_id: article.id, notifiable_type: "Article", action: "CoAuthor") + end + + before { Notifications::CoAuthor::Send.call(article) } + + it "removes the notification for a dropped co-author" do + expect { described_class.call(article.id, [co_author.id]) } + .to change(co_author_notifications, :count).by(-1) + end + + it "leaves notifications for co-authors who remain" do + other = create(:user) + + expect { described_class.call(article.id, [other.id]) } + .not_to change(co_author_notifications, :count) + end + + it "does nothing when given no user ids" do + expect { described_class.call(article.id, []) } + .not_to change(co_author_notifications, :count) + end +end diff --git a/spec/services/notifications/co_author/send_spec.rb b/spec/services/notifications/co_author/send_spec.rb new file mode 100644 index 0000000000000..33d8f3a17d517 --- /dev/null +++ b/spec/services/notifications/co_author/send_spec.rb @@ -0,0 +1,94 @@ +require "rails_helper" + +RSpec.describe Notifications::CoAuthor::Send, type: :service do + let(:author) { create(:user) } + let(:co_author) { create(:user) } + + def co_authored_article(co_authors: [co_author], published: true) + create(:article, user: author, published: published, co_author_ids: co_authors.map(&:id)) + end + + def co_author_notifications(article) + Notification.where(notifiable_id: article.id, notifiable_type: "Article", action: "CoAuthor") + end + + it "notifies a credited co-author" do + article = co_authored_article + + expect { described_class.call(article) } + .to change { co_author_notifications(article).count }.by(1) + + expect(co_author_notifications(article).first.user_id).to eq(co_author.id) + end + + it "stores the article and author in the payload" do + article = co_authored_article + described_class.call(article) + + json_data = co_author_notifications(article).first.json_data + + expect(json_data["article"]["title"]).to eq(article.title) + expect(json_data["user"]["id"]).to eq(author.id) + end + + it "does not notify twice when called again" do + article = co_authored_article + described_class.call(article) + + expect { described_class.call(article) } + .not_to change { co_author_notifications(article).count } + end + + it "only notifies co-authors who do not have a notification yet" do + second_co_author = create(:user) + article = co_authored_article + described_class.call(article) + + article.update_columns(co_author_ids: [co_author.id, second_co_author.id]) + + expect { described_class.call(article.reload) } + .to change { co_author_notifications(article).count }.by(1) + + expect(co_author_notifications(article).pluck(:user_id)) + .to contain_exactly(co_author.id, second_co_author.id) + end + + it "does not notify the author even if listed as a co-author" do + article = co_authored_article + article.update_columns(co_author_ids: [author.id]) + + described_class.call(article.reload) + + expect(co_author_notifications(article).pluck(:user_id)).not_to include(author.id) + end + + it "does nothing for an unpublished article" do + article = co_authored_article(published: false) + + expect { described_class.call(article) } + .not_to change { co_author_notifications(article).count } + end + + it "sets notified_at on the created notification" do + article = co_authored_article + described_class.call(article) + + notification = co_author_notifications(article).first + expect(notification.notified_at).to be_present + expect(notification.notified_at).to be_within(5.seconds).of(Time.current) + end + + it "handles nil co_author_ids safely" do + article = create(:article, user: author, published: true) + article.update_columns(co_author_ids: nil) + + expect { described_class.call(article.reload) }.not_to raise_error + expect(co_author_notifications(article)).to be_empty + end + + it "does nothing when there are no co-authors" do + article = create(:article, user: author, published: true) + + expect { described_class.call(article) }.not_to change(Notification, :count) + end +end diff --git a/spec/services/notifications/notifiable_action/send_spec.rb b/spec/services/notifications/notifiable_action/send_spec.rb index 786019b3aacd5..616fb73a1a3b4 100644 --- a/spec/services/notifications/notifiable_action/send_spec.rb +++ b/spec/services/notifications/notifiable_action/send_spec.rb @@ -9,6 +9,27 @@ let(:user2) { create(:user) } let(:user3) { create(:user) } + context "when a follower is also a co-author" do + before do + user2.follow(user) + article.update_columns(co_author_ids: [user2.id]) + end + + it "does not send a follower notification, since the co-author gets their own" do + expect do + described_class.call(article.reload, "Published") + end.not_to change { + Notification.where(user_id: user2.id, notifiable_id: article.id, action: "Published").count + } + end + + it "handles nil co_author_ids safely" do + article.update_columns(co_author_ids: nil) + + expect { described_class.call(article.reload, "Published") }.not_to raise_error + end + end + context "when following a user or organization" do before do user2.follow(user) diff --git a/spec/workers/notifications/co_author_worker_spec.rb b/spec/workers/notifications/co_author_worker_spec.rb new file mode 100644 index 0000000000000..62701ae10102e --- /dev/null +++ b/spec/workers/notifications/co_author_worker_spec.rb @@ -0,0 +1,35 @@ +require "rails_helper" + +RSpec.describe Notifications::CoAuthorWorker, type: :worker do + subject(:worker) { described_class.new } + + let(:article) { create(:article) } + let(:removed_user_ids) { [1, 2] } + + before do + allow(Notifications::CoAuthor::Remove).to receive(:call) + allow(Notifications::CoAuthor::Send).to receive(:call) + end + + it "calls Remove and Send when the article exists" do + worker.perform(article.id, removed_user_ids) + + expect(Notifications::CoAuthor::Remove).to have_received(:call).with(article.id, removed_user_ids) + expect(Notifications::CoAuthor::Send).to have_received(:call).with(article) + end + + it "defaults removed_user_ids to an empty array" do + worker.perform(article.id) + + expect(Notifications::CoAuthor::Remove).to have_received(:call).with(article.id, []) + expect(Notifications::CoAuthor::Send).to have_received(:call).with(article) + end + + it "calls Remove but skips Send when the article does not exist" do + non_existent_id = -1 + worker.perform(non_existent_id, removed_user_ids) + + expect(Notifications::CoAuthor::Remove).to have_received(:call).with(non_existent_id, removed_user_ids) + expect(Notifications::CoAuthor::Send).not_to have_received(:call) + end +end diff --git a/vendor/cache/logtail-0.1.17.gem b/vendor/cache/logtail-0.1.17.gem new file mode 100644 index 0000000000000..0d04d8b405fdb Binary files /dev/null and b/vendor/cache/logtail-0.1.17.gem differ diff --git a/vendor/cache/logtail-rack-0.2.6.gem b/vendor/cache/logtail-rack-0.2.6.gem new file mode 100644 index 0000000000000..a6e27d1db0fe4 Binary files /dev/null and b/vendor/cache/logtail-rack-0.2.6.gem differ diff --git a/vendor/cache/logtail-rails-0.2.12.gem b/vendor/cache/logtail-rails-0.2.12.gem new file mode 100644 index 0000000000000..f8a696fea4224 Binary files /dev/null and b/vendor/cache/logtail-rails-0.2.12.gem differ diff --git a/vendor/cache/sentry-rails-5.28.1.gem b/vendor/cache/sentry-rails-5.28.1.gem new file mode 100644 index 0000000000000..6adcdb0be1212 Binary files /dev/null and b/vendor/cache/sentry-rails-5.28.1.gem differ diff --git a/vendor/cache/sentry-ruby-5.28.1.gem b/vendor/cache/sentry-ruby-5.28.1.gem new file mode 100644 index 0000000000000..3db3b0d6fdfa1 Binary files /dev/null and b/vendor/cache/sentry-ruby-5.28.1.gem differ diff --git a/vendor/cache/sentry-sidekiq-5.28.1.gem b/vendor/cache/sentry-sidekiq-5.28.1.gem new file mode 100644 index 0000000000000..d6ba282d33de3 Binary files /dev/null and b/vendor/cache/sentry-sidekiq-5.28.1.gem differ