From 8a6266660a2c331160cc882eb0726fa56f4fd638 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 18 Nov 2025 14:51:08 +1000
Subject: [PATCH] FIX: Various bugs from 2024 rewind

* Use post excerpt for best posts to avoid giant posts
  ruining it
* Make sure short emojis (e.g. :joy:) render in best posts
  and best topics
* Make images show in best post excerpts (requires core change)
* Fix invite action error, using column that doesn't exist
* Add missing # for top category slugs
* Refactor report service + controller a little to make it easier
  to figure out what went wrong if it fails, and to use the guardian
  user

Copies a couple of fixes from
https://github.com/discourse/discourse-rewind/pull/28 to make
things work locally, the conflicts will be easy to resolve.
---
 .../discourse_rewind/rewinds_controller.rb       | 11 ++++++-----
 .../discourse_rewind/action/best_posts.rb        | 14 ++++++++++++--
 app/services/discourse_rewind/action/invites.rb  |  8 ++------
 app/services/discourse_rewind/fetch_reports.rb   | 10 +++-------
 .../discourse/components/reports/best-posts.gjs  | 11 ++++++-----
 .../discourse/components/reports/best-topics.gjs |  5 +++--
 .../reports/most-viewed-categories.gjs           |  2 +-
 config/locales/server.en.yml                     |  3 +++
 plugin.rb                                        |  2 +-
 spec/requests/rewinds_controller_spec.rb         | 16 ++++++++++++++++
 spec/services/fetch_reports_spec.rb              |  7 ++++---
 11 files changed, 57 insertions(+), 32 deletions(-)

diff --git a/app/controllers/discourse_rewind/rewinds_controller.rb b/app/controllers/discourse_rewind/rewinds_controller.rb
index 5d3d7aa..b71d2ff 100644
--- a/app/controllers/discourse_rewind/rewinds_controller.rb
+++ b/app/controllers/discourse_rewind/rewinds_controller.rb
@@ -8,12 +8,13 @@ class RewindsController < ::ApplicationController
 
     def show
       DiscourseRewind::FetchReports.call(service_params) do
-        on_model_not_found(:year) { raise Discourse::NotFound }
-        on_model_not_found(:user) { raise Discourse::NotFound }
-        on_success do |reports:|
-          @reports = reports
-          render json: MultiJson.dump(reports), status: :ok
+        on_model_not_found(:year) do
+          raise Discourse::NotFound.new(nil, custom_message: "discourse_rewind.invalid_year")
         end
+        on_model_not_found(:reports) do
+          raise Discourse::NotFound.new(nil, custom_message: "discourse_rewind.report_failed")
+        end
+        on_success { |reports:| render json: MultiJson.dump(reports), status: :ok }
       end
     end
   end
diff --git a/app/services/discourse_rewind/action/best_posts.rb b/app/services/discourse_rewind/action/best_posts.rb
index b3c3783..547d9db 100644
--- a/app/services/discourse_rewind/action/best_posts.rb
+++ b/app/services/discourse_rewind/action/best_posts.rb
@@ -10,9 +10,19 @@ def call
             .where(created_at: date)
             .where(deleted_at: nil)
             .where("post_number > 1")
-            .order("like_count DESC NULLS LAST")
+            .order("like_count DESC NULLS LAST, created_at ASC")
             .limit(3)
-            .pluck(:post_number, :topic_id, :like_count, :reply_count, :raw, :cooked)
+            .select(:post_number, :topic_id, :like_count, :reply_count, :raw, :cooked)
+            .map do |post|
+              {
+                post_number: post.post_number,
+                topic_id: post.topic_id,
+                like_count: post.like_count,
+                reply_count: post.reply_count,
+                excerpt:
+                  post.excerpt(200, { strip_links: true, remap_emoji: true, keep_images: true }),
+              }
+            end
 
         { data: best_posts, identifier: "best-posts" }
       end
diff --git a/app/services/discourse_rewind/action/invites.rb b/app/services/discourse_rewind/action/invites.rb
index dc2feae..8c720e9 100644
--- a/app/services/discourse_rewind/action/invites.rb
+++ b/app/services/discourse_rewind/action/invites.rb
@@ -13,7 +13,7 @@ def call
         return if total_invites == 0
 
         # Redeemed invites (users who actually joined)
-        redeemed_count = invites.where.not(redeemed_at: nil).count
+        redeemed_count = invites.where("redemption_count > 0").count
 
         # Get the users who were invited (via InvitedUser or redeemed invites)
         invited_user_ids = InvitedUser.where(invite: invites).pluck(:user_id).compact
@@ -22,11 +22,7 @@ def call
 
         # Calculate impact of invitees
         invitee_post_count =
-          Post
-            .where(user_id: invited_user_ids)
-            .where(created_at: date)
-            .where(deleted_at: nil)
-            .count
+          Post.where(user_id: invited_user_ids).where(created_at: date).where(deleted_at: nil).count
 
         invitee_topic_count =
           Topic
diff --git a/app/services/discourse_rewind/fetch_reports.rb b/app/services/discourse_rewind/fetch_reports.rb
index c959a24..679b916 100644
--- a/app/services/discourse_rewind/fetch_reports.rb
+++ b/app/services/discourse_rewind/fetch_reports.rb
@@ -41,16 +41,11 @@ class FetchReports
     ]
 
     model :year
-    model :user
     model :date
     model :reports
 
     private
 
-    def fetch_user(guardian:)
-      User.find_by_username(guardian.user.username)
-    end
-
     def fetch_year
       current_date = Time.zone.now
       current_month = current_date.month
@@ -76,12 +71,13 @@ def fetch_date(params:, year:)
       Date.new(year).all_year
     end
 
-    def fetch_reports(date:, user:, guardian:, year:)
+    def fetch_reports(date:, guardian:, year:)
       key = "rewind:#{guardian.user.username}:#{year}"
       reports = Discourse.redis.get(key)
 
       if !reports
-        reports = REPORTS.map { |report| report.call(date:, user:, guardian:) }
+        reports =
+          REPORTS.map { |report| report.call(date:, user: guardian.user, guardian:) }.compact
         Discourse.redis.setex(key, CACHE_DURATION, MultiJson.dump(reports))
       else
         reports = MultiJson.load(reports, symbolize_keys: true)
diff --git a/assets/javascripts/discourse/components/reports/best-posts.gjs b/assets/javascripts/discourse/components/reports/best-posts.gjs
index 2ba8c8b..a72b5ca 100644
--- a/assets/javascripts/discourse/components/reports/best-posts.gjs
+++ b/assets/javascripts/discourse/components/reports/best-posts.gjs
@@ -1,6 +1,5 @@
 import Component from "@glimmer/component";
 import { concat } from "@ember/helper";
-import { get } from "@ember/object";
 import { htmlSafe } from "@ember/template";
 import icon from "discourse/helpers/d-icon";
 import { i18n } from "discourse-i18n";
@@ -22,13 +21,15 @@ export default class BestPosts extends Component {
             <div class={{concat "rewind-card" " rank-" (this.rank idx)}}>
               <span class="best-posts -rank"></span>
               <span class="best-posts -rank"></span>
-              <div class="best-posts__post">{{htmlSafe (get post "5")}}</div>
+              <div class="best-posts__post"><p>{{htmlSafe
+                    post.excerpt
+                  }}</p></div>
               <div class="best-posts__metadata">
                 <span class="best-posts__likes">
-                  {{icon "heart"}}{{htmlSafe (get post "2")}}</span>
+                  {{icon "heart"}}{{post.like_count}}</span>
                 <span class="best-posts__replies">
-                  {{icon "comment"}}{{htmlSafe (get post "3")}}</span>
-                <a href="/t/{{get post '1'}}/{{get post '0'}}">{{i18n
+                  {{icon "comment"}}{{post.reply_count}}</span>
+                <a href="/t/{{post.topic_id}}/{{post.post_number}}">{{i18n
                     "discourse_rewind.reports.best_posts.view_post"
                   }}</a>
               </div>
diff --git a/assets/javascripts/discourse/components/reports/best-topics.gjs b/assets/javascripts/discourse/components/reports/best-topics.gjs
index a0ab586..19c8de1 100644
--- a/assets/javascripts/discourse/components/reports/best-topics.gjs
+++ b/assets/javascripts/discourse/components/reports/best-topics.gjs
@@ -1,6 +1,7 @@
 import Component from "@glimmer/component";
 import { concat } from "@ember/helper";
 import { htmlSafe } from "@ember/template";
+import replaceEmoji from "discourse/helpers/replace-emoji";
 import { i18n } from "discourse-i18n";
 
 export default class BestTopics extends Component {
@@ -25,8 +26,8 @@ export default class BestTopics extends Component {
                 <span class="best-topics -rank"></span>
                 <span class="best-topics -rank"></span>
                 <h2 class="best-topics__header">{{topic.title}}</h2>
-                <span class="best-topics__excerpt">{{htmlSafe
-                    topic.excerpt
+                <span class="best-topics__excerpt">{{replaceEmoji
+                    (htmlSafe topic.excerpt)
                   }}</span>
               </a>
             {{/each}}
diff --git a/assets/javascripts/discourse/components/reports/most-viewed-categories.gjs b/assets/javascripts/discourse/components/reports/most-viewed-categories.gjs
index 5d72a85..26d2ff4 100644
--- a/assets/javascripts/discourse/components/reports/most-viewed-categories.gjs
+++ b/assets/javascripts/discourse/components/reports/most-viewed-categories.gjs
@@ -14,7 +14,7 @@ const MostViewedCategories = <template>
             <p
               class="most-viewed-categories__category"
               href={{concat "/c/-/" data.category_id}}
-            >{{data.name}}</p>
+            >#{{data.name}}</p>
           </a>
         {{/each}}
       </div>
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index c2073db..64c4787 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1,3 +1,6 @@
 en:
   site_settings:
     discourse_rewind_enabled: "Enable Discourse Rewind for a fun end-of-year summary for members' activity in the community."
+  discourse_rewind:
+    report_failed: "Failed to generate Discourse Rewind report."
+    invalid_year: "Rewind can only be generated in January or December."
diff --git a/plugin.rb b/plugin.rb
index 1143126..25c5f18 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -27,6 +27,6 @@ def self.public_asset_path(name)
 
 after_initialize do
   add_to_serializer(:current_user, :is_rewind_active) do
-    Date.today.month == 1 || Date.today.month == 12
+    Rails.env.development? || Date.today.month == 1 || Date.today.month == 12
   end
 end
diff --git a/spec/requests/rewinds_controller_spec.rb b/spec/requests/rewinds_controller_spec.rb
index f99ace6..cac8caf 100644
--- a/spec/requests/rewinds_controller_spec.rb
+++ b/spec/requests/rewinds_controller_spec.rb
@@ -15,6 +15,7 @@
         get "/rewinds.json"
 
         expect(response.status).to eq(404)
+        expect(response.parsed_body["errors"].first).to eq(I18n.t("discourse_rewind.invalid_year"))
       end
     end
 
@@ -26,6 +27,21 @@
 
         expect(response.status).to eq(200)
       end
+
+      context "when reports are not found or error" do
+        before do
+          DiscourseRewind::Action::TopWords.stubs(:call).raises(StandardError.new("Some error"))
+        end
+
+        it "returns 404 with message" do
+          get "/rewinds.json"
+
+          expect(response.status).to eq(404)
+          expect(response.parsed_body["errors"].first).to eq(
+            I18n.t("discourse_rewind.report_failed"),
+          )
+        end
+      end
     end
   end
 end
diff --git a/spec/services/fetch_reports_spec.rb b/spec/services/fetch_reports_spec.rb
index 10f4ef1..edaeba0 100644
--- a/spec/services/fetch_reports_spec.rb
+++ b/spec/services/fetch_reports_spec.rb
@@ -35,10 +35,11 @@
       before { freeze_time DateTime.parse("2021-12-22") }
 
       it "returns the cached reports" do
-        expect(result.reports.length).to eq(16)
+        initial_count = result.reports.length
+        expect(initial_count).to be > 0
 
         allow(DiscourseRewind::Action::TopWords).to receive(:call)
-        expect(result.reports.length).to eq(16)
+        expect(result.reports.length).to eq(initial_count)
         expect(DiscourseRewind::Action::TopWords).to_not have_received(:call)
       end
     end
@@ -51,7 +52,7 @@
 
       it "returns the reports" do
         allow(DiscourseRewind::Action::TopWords).to receive(:call)
-        expect(result.reports.length).to eq(16)
+        expect(result.reports.length).to be > 0
         expect(DiscourseRewind::Action::TopWords).to have_received(:call)
       end
     end