[pull] main from forem:main

app/lib/url.rb

@@ -8,7 +8,14 @@ def self.protocol
   # to preserve historical behavior, but can be overridden by setting the
   # PORT environment variable to match the actual server port (useful when
   # Forem runs alongside other Rails apps that also default to 3000).
+  #
+  # URL_PORT, when set, wins over PORT: it names the port the public URL
+  # should carry rather than the one Puma listens on. Set it to an empty value
+  # when a TLS proxy such as Caddy fronts the app on the standard port, so
+  # OAuth callbacks and other absolute URLs carry no port at all.
   def self.dev_port
+    return ENV["URL_PORT"] if ENV.key?("URL_PORT")
+
     ENV.fetch("PORT", "3000")
   end
 
@@ -37,8 +44,10 @@ def self.domain(domain_or_subforem = nil)
 
   def self.url(uri = nil, domain_or_subforem = nil)
     base_url = "#{protocol}#{domain(domain_or_subforem)}"
-    base_url += ":#{dev_port}" if Rails.env.development? && !base_url.include?(":#{dev_port}")
+    port = dev_port
+    base_url += ":#{port}" if Rails.env.development? && port.present? && base_url.exclude?(":#{port}")
     return base_url unless uri
+
     Addressable::URI.parse(base_url).join(uri).normalize.to_s
   end
 

app/models/article_activity.rb

@@ -13,6 +13,8 @@
 #   daily_comments[date]   = N            (count of scored comments)
 #   daily_referrers[date]  = { domain => N }
 class ArticleActivity < ApplicationRecord
+  include ArticleActivityBulkBackfillable
+
   belongs_to :article
 
   REACTION_CATEGORIES = %w[like readinglist unicorn exploding_head raised_hands fire].freeze

app/models/concerns/article_activity_bulk_backfillable.rb

@@ -0,0 +1,82 @@
+module ArticleActivityBulkBackfillable
+  extend ActiveSupport::Concern
+
+  module ClassMethods
+    def bulk_backfill!(article_ids)
+      ids = Article.where(id: article_ids).ids
+      return if ids.empty?
+
+      aggregates = ids.index_with do
+        {
+          daily_page_views: {}, daily_reactions: {}, daily_comments: {}, daily_referrers: {},
+          total_page_views: 0, total_reactions: 0, total_comments: 0
+        }
+      end
+
+      aggregate_page_views(ids, aggregates)
+      aggregate_reactions(ids, aggregates)
+      aggregate_comments(ids, aggregates)
+
+      now = Time.current
+      rows = aggregates.map do |article_id, values|
+        values.merge(article_id: article_id, last_aggregated_at: now, created_at: now, updated_at: now)
+      end
+      insert_all(rows, unique_by: :index_article_activities_on_article_id)
+    end
+
+    private
+
+    def aggregate_page_views(ids, aggregates)
+      PageView.where(article_id: ids)
+        .group(:article_id, "DATE(created_at)", :domain)
+        .pluck(
+          :article_id,
+          Arel.sql("DATE(created_at)"),
+          :domain,
+          Arel.sql("COALESCE(SUM(counts_for_number_of_views), 0)"),
+          Arel.sql("COALESCE(SUM(time_tracked_in_seconds) FILTER (WHERE user_id IS NOT NULL), 0)"),
+          Arel.sql("COUNT(*) FILTER (WHERE user_id IS NOT NULL)"),
+        ).each do |article_id, date, domain, total, sum_read, logged|
+          iso = date.iso8601
+          day = aggregates.fetch(article_id)[:daily_page_views]
+            .fetch(iso, { "total" => 0, "sum_read_seconds" => 0, "logged_in_count" => 0 })
+          day["total"] += total.to_i
+          day["sum_read_seconds"] += sum_read.to_i
+          day["logged_in_count"] += logged.to_i
+          aggregates.fetch(article_id)[:daily_page_views][iso] = day
+          aggregates.fetch(article_id)[:daily_referrers][iso] ||= {}
+          aggregates.fetch(article_id)[:daily_referrers][iso][domain.to_s] = total.to_i
+          aggregates.fetch(article_id)[:total_page_views] += total.to_i
+        end
+    end
+
+    def aggregate_reactions(ids, aggregates)
+      Reaction.for_analytics
+        .where(reactable_id: ids, reactable_type: "Article")
+        .group(:reactable_id, "DATE(created_at)")
+        .pluck(
+          :reactable_id,
+          Arel.sql("DATE(created_at)"),
+          Arel.sql("COUNT(*)"),
+          *self::REACTION_CATEGORIES.map { |category| Arel.sql("COUNT(*) FILTER (WHERE category = '#{category}')") },
+          Arel.sql("COALESCE(array_agg(DISTINCT user_id) FILTER (WHERE user_id IS NOT NULL), '{}')"),
+        ).each do |row|
+          article_id, date, total, *counts, reactor_ids = row
+          values = { "total" => total.to_i, "reactor_ids" => Array(reactor_ids).map(&:to_i) }
+          self::REACTION_CATEGORIES.zip(counts) { |category, count| values[category] = count.to_i }
+          aggregates.fetch(article_id)[:daily_reactions][date.iso8601] = values
+          aggregates.fetch(article_id)[:total_reactions] += total.to_i
+        end
+    end
+
+    def aggregate_comments(ids, aggregates)
+      Comment.where(commentable_id: ids, commentable_type: "Article")
+        .where("score > 0")
+        .group(:commentable_id, "DATE(created_at)")
+        .count.each do |(article_id, date), total|
+          aggregates.fetch(article_id)[:daily_comments][date.iso8601] = total
+          aggregates.fetch(article_id)[:total_comments] += total
+        end
+    end
+  end
+end

app/services/analytics_service.rb

@@ -191,7 +191,7 @@ def follower_engagement
 
   attr_reader(
     :user_or_org, :article_id, :start_date, :end_date,
-    :article_data, :reaction_data, :comment_data, :follow_data, :page_view_data
+    :article_data, :article_ids, :reaction_data, :comment_data, :follow_data, :page_view_data
   )
 
   def load_data
@@ -201,12 +201,10 @@ def load_data
 
       # check article_id is published and belongs to the user/org
       raise ArgumentError, I18n.t("services.analytics_service.no_stats") unless @article_data.exists?
-
-      article_ids = [@article_id]
-    else
-      article_ids = @article_data.ids
     end
 
+    @article_ids = @article_data.ids
+
     # prepare relations for metrics
     @comment_data = Comment
       .where(commentable_id: article_ids, commentable_type: "Article")
@@ -439,7 +437,6 @@ def referrers_from_activities(top: 20)
   def scoped_activities
     return @scoped_activities if defined?(@scoped_activities)
 
-    article_ids = article_data.ids
     if article_ids.empty?
       @scoped_activities = nil
       return nil
@@ -452,7 +449,7 @@ def scoped_activities
       # that owners with thousands of articles don't spike the queue on a
       # cold dashboard load. The current request still falls back to the
       # raw-table path; the cache is warm on the next visit.
-      Articles::BackfillActivitiesWorker.perform_async(missing)
+      Articles::BackfillActivitiesWorker.perform_async(missing.sort)
       @scoped_activities = nil
       return nil
     end

app/workers/articles/backfill_activities_worker.rb

@@ -16,14 +16,8 @@ class BackfillActivitiesWorker
 
     def perform(article_ids)
       Array(article_ids).each_slice(100) do |chunk|
-        existing = ArticleActivity.where(article_id: chunk).pluck(:article_id).to_set
-        missing = chunk.reject { |id| existing.include?(id) }
-        next if missing.empty?
-
-        Article.where(id: missing).find_each do |article|
-          activity = ArticleActivity.find_or_create_by!(article_id: article.id)
-          activity.recompute_all!
-        end
+        missing = chunk - ArticleActivity.where(article_id: chunk).pluck(:article_id)
+        ArticleActivity.bulk_backfill!(missing) if missing.any?
       end
     end
   end

spec/lib/url_spec.rb

@@ -16,16 +16,36 @@
 
   describe ".dev_port" do
     it "defaults to 3000 when the PORT env var is not set" do
+      allow(ENV).to receive(:key?).and_call_original
+      allow(ENV).to receive(:key?).with("URL_PORT").and_return(false)
       allow(ENV).to receive(:fetch).and_call_original
       allow(ENV).to receive(:fetch).with("PORT", "3000").and_return("3000")
       expect(described_class.dev_port).to eq("3000")
     end
 
     it "returns the value of the PORT env var when set" do
+      allow(ENV).to receive(:key?).and_call_original
+      allow(ENV).to receive(:key?).with("URL_PORT").and_return(false)
       allow(ENV).to receive(:fetch).and_call_original
       allow(ENV).to receive(:fetch).with("PORT", "3000").and_return("3005")
       expect(described_class.dev_port).to eq("3005")
     end
+
+    it "prefers URL_PORT over PORT when URL_PORT is set" do
+      allow(ENV).to receive(:key?).and_call_original
+      allow(ENV).to receive(:key?).with("URL_PORT").and_return(true)
+      allow(ENV).to receive(:[]).and_call_original
+      allow(ENV).to receive(:[]).with("URL_PORT").and_return("8443")
+      expect(described_class.dev_port).to eq("8443")
+    end
+
+    it "returns an empty string when URL_PORT is set but blank" do
+      allow(ENV).to receive(:key?).and_call_original
+      allow(ENV).to receive(:key?).with("URL_PORT").and_return(true)
+      allow(ENV).to receive(:[]).and_call_original
+      allow(ENV).to receive(:[]).with("URL_PORT").and_return("")
+      expect(described_class.dev_port).to eq("")
+    end
   end
 
   describe ".domain" do
@@ -109,6 +129,11 @@
         allow(Settings::General).to receive(:app_domain).and_return("localhost:3005")
         expect(described_class.url).to eq("https://localhost:3005")
       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")
+      end
     end
   end
 

spec/models/article_activity_spec.rb

@@ -107,6 +107,78 @@
     end
   end
 
+  describe ".bulk_backfill!" do
+    it "rebuilds multiple article rows from batch aggregates" do
+      other_article = create(:article)
+      user = create(:user)
+      ts = Time.utc(day.year, day.month, day.day, 12, 0, 0)
+      page_view = create(
+        :page_view,
+        article: article,
+        user: user,
+        created_at: ts,
+        counts_for_number_of_views: 4,
+        time_tracked_in_seconds: 30,
+      )
+      page_view.update_column(:domain, "example.com")
+      create(:page_view, article: other_article, created_at: ts,
+                         counts_for_number_of_views: 2, domain: "forem.com")
+      create(:reaction, reactable: article, user: user, category: "fire", created_at: ts)
+      create(:comment, commentable: other_article, user: user, score: 5, created_at: ts)
+
+      sql_queries = []
+      callback = lambda do |*, payload|
+        sql_queries << payload[:sql] unless %w[SCHEMA TRANSACTION].include?(payload[:name])
+      end
+      ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do
+        described_class.bulk_backfill!([article.id, other_article.id])
+      end
+
+      first = described_class.find_by!(article: article)
+      second = described_class.find_by!(article: other_article)
+      expected_page_views = {
+        "total" => 4,
+        "sum_read_seconds" => 30,
+        "logged_in_count" => 1
+      }
+      expect(first).to have_attributes(
+        daily_page_views: hash_including(iso => expected_page_views),
+        daily_referrers: hash_including(iso => { "example.com" => 4 }),
+        total_page_views: 4,
+        total_reactions: 1,
+        last_aggregated_at: be_present,
+      )
+      expect(first.daily_reactions[iso]["fire"]).to eq(1)
+      expect(sql_queries.size).to eq(5)
+      expect(second).to have_attributes(
+        daily_page_views: hash_including(iso => hash_including("total" => 2)),
+        daily_comments: hash_including(iso => 1),
+        total_comments: 1,
+        last_aggregated_at: be_present,
+      )
+
+      columns = %w[daily_page_views daily_reactions daily_comments daily_referrers
+                   total_page_views total_reactions total_comments]
+      [first, second].each do |record|
+        bulk_values = record.attributes.slice(*columns)
+        record.recompute_all!
+        expect(record.reload.attributes.slice(*columns)).to eq(bulk_values)
+      end
+    end
+
+    it "preserves a row created after aggregation but before insertion" do
+      article_id = article.id
+      allow(described_class).to receive(:insert_all).and_wrap_original do |original, *args, **kwargs|
+        described_class.create!(article_id: article_id, total_page_views: 99)
+        original.call(*args, **kwargs)
+      end
+
+      described_class.bulk_backfill!([article_id])
+
+      expect(described_class.find_by!(article_id: article_id).total_page_views).to eq(99)
+    end
+  end
+
   describe "#referrer_totals" do
     it "sorts by count desc and limits" do
       activity.apply_page_view_delta!("iso" => iso, "total" => 1, "sum_read_seconds" => 0,

spec/services/analytics_service_spec.rb

@@ -287,6 +287,28 @@ def format_date(datetime)
     end
   end
 
+  describe "activity cache lookup" do
+    it "uses an existing activity for a string article ID without enqueueing a backfill" do
+      ArticleActivity.create!(article: article, total_page_views: 42,
+                              daily_page_views: { "2019-04-01" => { "total" => 42 } })
+      allow(Articles::BackfillActivitiesWorker).to receive(:perform_async)
+
+      service = described_class.new(user, article_id: article.id.to_s)
+
+      expect(service.totals[:page_views][:total]).to eq(42)
+      expect(Articles::BackfillActivitiesWorker).not_to have_received(:perform_async)
+    end
+
+    it "enqueues missing article IDs in canonical order" do
+      articles = create_list(:article, 2, user: user, published: true)
+      allow(Articles::BackfillActivitiesWorker).to receive(:perform_async)
+
+      described_class.new(user).totals
+
+      expect(Articles::BackfillActivitiesWorker).to have_received(:perform_async).with(articles.map(&:id).sort)
+    end
+  end
+
   describe "#grouped_by_day" do
     it "returns stats grouped by day" do
       stats = described_class.new(

spec/workers/articles/backfill_activities_worker_spec.rb

@@ -0,0 +1,26 @@
+require "rails_helper"
+
+RSpec.describe Articles::BackfillActivitiesWorker do
+  describe "#perform" do
+    it "backfills missing rows as one batch" do
+      articles = create_list(:article, 2)
+      ids = articles.map(&:id)
+      allow(ArticleActivity).to receive(:bulk_backfill!)
+
+      described_class.new.perform(ids)
+
+      expect(ArticleActivity).to have_received(:bulk_backfill!).with(ids)
+    end
+
+    it "skips existing rows" do
+      article = create(:article)
+      activity = ArticleActivity.create!(article: article, total_page_views: 12)
+      allow(ArticleActivity).to receive(:bulk_backfill!)
+
+      described_class.new.perform([article.id])
+
+      expect(ArticleActivity).not_to have_received(:bulk_backfill!)
+      expect(activity.reload.total_page_views).to eq(12)
+    end
+  end
+end