From 34973acc557eb340e44a08de8bfdd9ee48715f7c Mon Sep 17 00:00:00 2001
From: Rafael Silva <xfalcox@gmail.com>
Date: Wed, 26 Nov 2025 15:12:59 -0300
Subject: [PATCH] PERF: Load reports async

---
 .../discourse_rewind/rewinds_controller.rb    | 24 ++++-
 app/services/discourse_rewind/fetch_report.rb | 92 +++++++++++++++++++
 .../discourse_rewind/fetch_reports.rb         | 43 +++++++--
 .../discourse/components/rewind.gjs           | 70 +++++++++++++-
 config/locales/server.en.yml                  |  1 +
 config/routes.rb                              |  5 +-
 spec/requests/rewinds_controller_spec.rb      | 92 +++++++++++++++++--
 7 files changed, 306 insertions(+), 21 deletions(-)
 create mode 100644 app/services/discourse_rewind/fetch_report.rb

diff --git a/app/controllers/discourse_rewind/rewinds_controller.rb b/app/controllers/discourse_rewind/rewinds_controller.rb
index b71d2ff..391d996 100644
--- a/app/controllers/discourse_rewind/rewinds_controller.rb
+++ b/app/controllers/discourse_rewind/rewinds_controller.rb
@@ -6,7 +6,7 @@ class RewindsController < ::ApplicationController
 
     requires_login
 
-    def show
+    def index
       DiscourseRewind::FetchReports.call(service_params) do
         on_model_not_found(:year) do
           raise Discourse::NotFound.new(nil, custom_message: "discourse_rewind.invalid_year")
@@ -14,7 +14,27 @@ def show
         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 }
+        on_success do |reports:, total_available:|
+          render json: { reports: reports, total_available: total_available }, status: :ok
+        end
+      end
+    end
+
+    def show
+      DiscourseRewind::FetchReport.call(service_params) do
+        on_model_not_found(:year) do
+          raise Discourse::NotFound.new(nil, custom_message: "discourse_rewind.invalid_year")
+        end
+        on_model_not_found(:report_class) do
+          raise Discourse::NotFound.new(
+                  nil,
+                  custom_message: "discourse_rewind.invalid_report_index",
+                )
+        end
+        on_model_not_found(:report) do
+          raise Discourse::NotFound.new(nil, custom_message: "discourse_rewind.report_failed")
+        end
+        on_success { |report:| render json: { report: report }, status: :ok }
       end
     end
   end
diff --git a/app/services/discourse_rewind/fetch_report.rb b/app/services/discourse_rewind/fetch_report.rb
new file mode 100644
index 0000000..2e8c74a
--- /dev/null
+++ b/app/services/discourse_rewind/fetch_report.rb
@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+module DiscourseRewind
+  # Service responsible to fetch a single report by index.
+  #
+  # @example
+  #  ::DiscourseRewind::FetchReport.call(
+  #    guardian: guardian,
+  #    index: 3
+  #  )
+  #
+  class FetchReport
+    include Service::Base
+
+    # @!method self.call(guardian:, params:)
+    #   @param [Guardian] guardian
+    #   @param [Hash] params
+    #   @option params [Integer] :index the report index
+    #   @return [Service::Base::Context]
+
+    CACHE_DURATION = Rails.env.development? ? 10.seconds : 5.minutes
+
+    model :year
+    model :date
+    model :report_class
+    model :report
+
+    private
+
+    def fetch_year
+      current_date = Time.zone.now
+      current_month = current_date.month
+      current_year = current_date.year
+
+      case current_month
+      when 1
+        current_year - 1
+      when 12
+        current_year
+      else
+        # Otherwise it's impossible to test in browser unless you're
+        # in December or January
+        if Rails.env.development?
+          current_year
+        else
+          false
+        end
+      end
+    end
+
+    def fetch_date(params:, year:)
+      Date.new(year).all_year
+    end
+
+    def fetch_report_class(date:, guardian:, year:, params:)
+      # Use the same cached all_reports list as FetchReports
+      # If not cached, generate it now
+      key = "rewind:#{guardian.user.username}:#{year}:all_reports"
+      cached_list = Discourse.redis.get(key)
+
+      all_reports =
+        if cached_list
+          MultiJson.load(cached_list, symbolize_keys: true)
+        else
+          # Generate all reports and cache them
+          reports =
+            FetchReports::REPORTS.filter_map do |report_class|
+              begin
+                report_class.call(date:, user: guardian.user, guardian:)
+              rescue => e
+                Rails.logger.error("Failed to generate report #{report_class.name}: #{e.message}")
+                nil
+              end
+            end
+          Discourse.redis.setex(key, CACHE_DURATION, MultiJson.dump(reports))
+          reports
+        end
+
+      # Access index from params data object (params.index) or hash (params[:index])
+      index = (params[:index] || params.index).to_i
+
+      return false if index < 0 || index >= all_reports.length
+
+      all_reports[index]
+    end
+
+    def fetch_report(report_class:)
+      # Report is already generated and cached in the all_reports list
+      report_class
+    end
+  end
+end
diff --git a/app/services/discourse_rewind/fetch_reports.rb b/app/services/discourse_rewind/fetch_reports.rb
index 679b916..598f8c2 100644
--- a/app/services/discourse_rewind/fetch_reports.rb
+++ b/app/services/discourse_rewind/fetch_reports.rb
@@ -16,9 +16,11 @@ class FetchReports
     #   @param [Hash] params
     #   @option params [Integer] :year of the rewind
     #   @option params [Integer] :username of the rewind
+    #   @option params [Integer] :count number of reports to fetch (optional, defaults to 3)
     #   @return [Service::Base::Context]
 
     CACHE_DURATION = Rails.env.development? ? 10.seconds : 5.minutes
+    INITIAL_REPORT_COUNT = 3
 
     # order matters
     REPORTS = [
@@ -42,7 +44,9 @@ class FetchReports
 
     model :year
     model :date
+    model :enabled_reports
     model :reports
+    model :total_available
 
     private
 
@@ -71,19 +75,38 @@ def fetch_date(params:, year:)
       Date.new(year).all_year
     end
 
-    def fetch_reports(date:, guardian:, year:)
-      key = "rewind:#{guardian.user.username}:#{year}"
-      reports = Discourse.redis.get(key)
+    def fetch_enabled_reports(date:, guardian:, year:)
+      # Generate all reports and filter out nils (disabled/empty reports)
+      # Cache the full list to maintain consistent indices across requests
+      key = "rewind:#{guardian.user.username}:#{year}:all_reports"
+      cached_list = Discourse.redis.get(key)
 
-      if !reports
-        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)
-      end
+      return MultiJson.load(cached_list, symbolize_keys: true) if cached_list
 
+      reports =
+        REPORTS.filter_map do |report_class|
+          begin
+            report_class.call(date:, user: guardian.user, guardian:)
+          rescue => e
+            Rails.logger.error("Failed to generate report #{report_class.name}: #{e.message}")
+            nil
+          end
+        end
+
+      # Cache the complete enabled reports list
+      Discourse.redis.setex(key, CACHE_DURATION, MultiJson.dump(reports))
       reports
     end
+
+    def fetch_total_available(enabled_reports:)
+      enabled_reports.length
+    end
+
+    def fetch_reports(enabled_reports:, params:)
+      count = params[:count]&.to_i || INITIAL_REPORT_COUNT
+      count = [[count, 1].max, enabled_reports.length].min
+
+      enabled_reports.first(count)
+    end
   end
 end
diff --git a/assets/javascripts/discourse/components/rewind.gjs b/assets/javascripts/discourse/components/rewind.gjs
index 750b5b9..c52641a 100644
--- a/assets/javascripts/discourse/components/rewind.gjs
+++ b/assets/javascripts/discourse/components/rewind.gjs
@@ -3,6 +3,7 @@ import { tracked } from "@glimmer/tracking";
 import { on } from "@ember/modifier";
 import { action } from "@ember/object";
 import didInsert from "@ember/render-modifiers/modifiers/did-insert";
+import willDestroy from "@ember/render-modifiers/modifiers/will-destroy";
 import DButton from "discourse/components/d-button";
 import concatClass from "discourse/helpers/concat-class";
 import { ajax } from "discourse/lib/ajax";
@@ -24,17 +25,44 @@ export default class Rewind extends Component {
   @tracked rewind = [];
   @tracked fullScreen = true;
   @tracked loadingRewind = false;
+  @tracked totalAvailable = 0;
+  @tracked loadingNextReport = false;
+
+  BUFFER_SIZE = 3;
+
+  // Arrow function for event listener - maintains 'this' binding
+  handleScroll = () => {
+    if (!this.scrollWrapper || this.loadingRewind) {
+      return;
+    }
+
+    const scrollTop = this.scrollWrapper.scrollTop;
+    const scrollHeight = this.scrollWrapper.scrollHeight;
+    const clientHeight = this.scrollWrapper.clientHeight;
+
+    // Trigger preload when scrolled 70% through content
+    const scrollPercentage = (scrollTop + clientHeight) / scrollHeight;
+    if (scrollPercentage > 0.7) {
+      this.preloadNextReports();
+    }
+  };
 
   @action
   registerScrollWrapper(element) {
     this.scrollWrapper = element;
+    this.scrollWrapper.addEventListener("scroll", this.handleScroll);
   }
 
   @action
   async loadRewind() {
     try {
       this.loadingRewind = true;
-      this.rewind = await ajax("/rewinds");
+      const response = await ajax("/rewinds");
+      this.rewind = response.reports;
+      this.totalAvailable = response.total_available;
+
+      // Preload next report to maintain buffer
+      this.preloadNextReports();
     } catch (e) {
       popupAjaxError(e);
     } finally {
@@ -42,6 +70,38 @@ export default class Rewind extends Component {
     }
   }
 
+  @action
+  async preloadNextReports() {
+    // Load reports to maintain BUFFER_SIZE ahead of current position
+    const currentIndex = this.rewind.length;
+    const targetIndex = currentIndex + this.BUFFER_SIZE;
+
+    if (
+      this.loadingNextReport ||
+      currentIndex >= this.totalAvailable ||
+      targetIndex > this.totalAvailable
+    ) {
+      return;
+    }
+
+    try {
+      this.loadingNextReport = true;
+      const response = await ajax(`/rewinds/${currentIndex}`);
+      this.rewind = [...this.rewind, response.report];
+
+      // Continue preloading if we haven't reached buffer size yet
+      if (this.rewind.length < targetIndex) {
+        this.preloadNextReports();
+      }
+    } catch (e) {
+      // Silently fail for preloading - user already has content to view
+      // eslint-disable-next-line no-console
+      console.error("Failed to preload report:", e);
+    } finally {
+      this.loadingNextReport = false;
+    }
+  }
+
   @action
   toggleFullScreen() {
     this.fullScreen = !this.fullScreen;
@@ -66,6 +126,13 @@ export default class Rewind extends Component {
     this.rewindContainer = element;
   }
 
+  @action
+  cleanup() {
+    if (this.scrollWrapper) {
+      this.scrollWrapper.removeEventListener("scroll", this.handleScroll);
+    }
+  }
+
   <template>
     <div
       class={{concatClass
@@ -73,6 +140,7 @@ export default class Rewind extends Component {
         (if this.fullScreen "-fullscreen")
       }}
       {{didInsert this.loadRewind}}
+      {{willDestroy this.cleanup}}
       {{on "keydown" this.handleEscape}}
       {{on "click" this.handleBackdropClick}}
       {{didInsert this.registerRewindContainer}}
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 64c4787..be7e789 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -4,3 +4,4 @@ en:
   discourse_rewind:
     report_failed: "Failed to generate Discourse Rewind report."
     invalid_year: "Rewind can only be generated in January or December."
+    invalid_report_index: "Invalid report index."
diff --git a/config/routes.rb b/config/routes.rb
index 5ee0cbe..5d99a5f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
-DiscourseRewind::Engine.routes.draw { get "/rewinds" => "rewinds#show" }
+DiscourseRewind::Engine.routes.draw do
+  get "/rewinds" => "rewinds#index", :constraints => { format: :json }
+  get "/rewinds/:index" => "rewinds#show", :constraints => { index: /\d+/ }
+end
 
 Discourse::Application.routes.draw { mount ::DiscourseRewind::Engine, at: "/" }
diff --git a/spec/requests/rewinds_controller_spec.rb b/spec/requests/rewinds_controller_spec.rb
index cac8caf..e5c91b2 100644
--- a/spec/requests/rewinds_controller_spec.rb
+++ b/spec/requests/rewinds_controller_spec.rb
@@ -3,11 +3,11 @@
 RSpec.describe DiscourseRewind::RewindsController do
   before { SiteSetting.discourse_rewind_enabled = true }
 
-  describe "#show" do
-    fab!(:current_user, :user)
+  fab!(:current_user, :user)
 
-    before { sign_in(current_user) }
+  before { sign_in(current_user) }
 
+  describe "#index" do
     context "when out of valid month" do
       before { freeze_time DateTime.parse("2022-11-24") }
 
@@ -22,26 +22,104 @@
     context "when in valid month" do
       before { freeze_time DateTime.parse("2022-12-24") }
 
-      it "returns 200" do
+      it "returns 200 with reports and total_available" do
         get "/rewinds.json"
 
         expect(response.status).to eq(200)
+        expect(response.parsed_body["reports"]).to be_present
+        expect(response.parsed_body["total_available"]).to be_present
+        expect(response.parsed_body["reports"].length).to eq(3)
+      end
+
+      it "limits initial reports to 3 by default" do
+        get "/rewinds.json"
+
+        expect(response.parsed_body["reports"].length).to eq(3)
+        expect(response.parsed_body["total_available"]).to be > 3
       end
 
-      context "when reports are not found or error" do
+      context "when a report errors" do
         before do
           DiscourseRewind::Action::TopWords.stubs(:call).raises(StandardError.new("Some error"))
         end
 
-        it "returns 404 with message" do
+        it "filters out failed reports and returns remaining reports" do
           get "/rewinds.json"
 
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["reports"]).to be_present
+          expect(response.parsed_body["reports"].map { |r| r["identifier"] }).not_to include(
+            "top-words",
+          )
+        end
+      end
+    end
+  end
+
+  describe "#show" do
+    context "when out of valid month" do
+      before { freeze_time DateTime.parse("2022-11-24") }
+
+      it "returns 404" do
+        get "/rewinds/0.json"
+
+        expect(response.status).to eq(404)
+        expect(response.parsed_body["errors"].first).to eq(I18n.t("discourse_rewind.invalid_year"))
+      end
+    end
+
+    context "when in valid month" do
+      before { freeze_time DateTime.parse("2022-12-24") }
+
+      it "returns 200 with a single report" do
+        get "/rewinds/0.json"
+
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["report"]).to be_present
+        expect(response.parsed_body["report"]["identifier"]).to be_present
+      end
+
+      it "returns different reports for different indices" do
+        # Get all reports first to know what to expect
+        get "/rewinds.json"
+        total = response.parsed_body["total_available"]
+
+        # Request second report if there is one
+        if total > 1
+          get "/rewinds/1.json"
+
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["report"]["identifier"]).to be_present
+        end
+      end
+
+      context "when index is out of bounds" do
+        it "returns 404" do
+          # Request index that exceeds the REPORTS array length
+          # There are 16 items in REPORTS array, so 999 should definitely be out of bounds
+          get "/rewinds/999", params: {}, as: :json
+
           expect(response.status).to eq(404)
           expect(response.parsed_body["errors"].first).to eq(
-            I18n.t("discourse_rewind.report_failed"),
+            I18n.t("discourse_rewind.invalid_report_index"),
           )
         end
       end
+
+      context "when report fails to generate" do
+        before do
+          DiscourseRewind::Action::TopWords.stubs(:call).raises(StandardError.new("Some error"))
+        end
+
+        it "filters out failed report and adjusts indices" do
+          # TopWords is filtered out, index 0 points to first successful report
+          get "/rewinds/0.json"
+
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["report"]["identifier"]).to be_present
+          expect(response.parsed_body["report"]["identifier"]).not_to eq("top-words")
+        end
+      end
     end
   end
 end