FEATURE: makes reports loadable in bulk (#6309)

app/assets/javascripts/admin/components/admin-report.js.es6

@@ -1,7 +1,7 @@
+import ReportLoader from "discourse/lib/reports-loader";
 import Category from "discourse/models/category";
 import { exportEntity } from "discourse/lib/export-csv";
 import { outputExportResult } from "discourse/lib/export-result";
-import { ajax } from "discourse/lib/ajax";
 import { SCHEMA_VERSION, default as Report } from "admin/models/report";
 import computed from "ember-addons/ember-computed-decorators";
 import {
@@ -95,7 +95,7 @@ export default Ember.Component.extend({
         this.get("currentMode")
       );
     } else if (this.get("dataSourceName")) {
-      this._fetchReport().finally(() => this._computeReport());
+      this._fetchReport();
     }
   },
 
@@ -306,29 +306,31 @@ export default Ember.Component.extend({
 
     this.setProperties({ isLoading: true, rateLimitationString: null });
 
-    let payload = this._buildPayload(["prev_period"]);
+    Ember.run.next(() => {
+      let payload = this._buildPayload(["prev_period"]);
 
-    return ajax(this.get("dataSource"), payload)
-      .then(response => {
-        if (response && response.report) {
-          this._reports.push(this._loadReport(response.report));
-        } else {
-          console.log("failed loading", this.get("dataSource"));
+      const callback = response => {
+        if (!this.element || this.isDestroying || this.isDestroyed) {
+          return;
         }
-      })
-      .catch(data => {
-        if (data.jqXHR && data.jqXHR.status === 429) {
+
+        this.set("isLoading", false);
+
+        if (response === 429) {
           this.set(
             "rateLimitationString",
             I18n.t("admin.dashboard.too_many_requests")
           );
+        } else if (response === 500) {
+          this.set("model.error", "exception");
+        } else if (response) {
+          this._reports.push(this._loadReport(response));
+          this._computeReport();
         }
-      })
-      .finally(() => {
-        if (this.element && !this.isDestroying && !this.isDestroyed) {
-          this.set("isLoading", false);
-        }
-      });
+      };
+
+      ReportLoader.enqueue(this.get("dataSourceName"), payload.data, callback);
+    });
   },
 
   _buildPayload(facets) {

app/assets/javascripts/discourse/lib/reports-loader.js.es6

@@ -0,0 +1,87 @@
+import { ajax } from "discourse/lib/ajax";
+const { debounce } = Ember.run;
+
+let _queue = [];
+let _processing = 0;
+
+// max number of reports which will be requested in one bulk request
+const MAX_JOB_SIZE = 5;
+
+// max number of concurrent bulk requests
+const MAX_CONCURRENCY = 3;
+
+// max number of jobs stored, first entered jobs will be evicted first
+const MAX_QUEUE_SIZE = 20;
+
+const BULK_REPORTS_ENDPOINT = "/admin/reports/bulk";
+
+const DEBOUNCING_DELAY = 50;
+
+export default {
+  enqueue(type, params, callback) {
+    // makes sures the queue is not filling indefinitely
+    if (_queue.length >= MAX_QUEUE_SIZE) {
+      const removedJobs = _queue.splice(0, 1)[0];
+      removedJobs.forEach(job => {
+        // this is technically not a 429, but it's the result
+        // of client doing too many requests so we want the same
+        // behavior
+        job.runnable()(429);
+      });
+    }
+
+    _queue.push({ runnable: () => callback, type, params });
+
+    debounce(this, this._processQueue, DEBOUNCING_DELAY);
+  },
+
+  _processQueue() {
+    if (_queue.length === 0) return;
+    if (_processing >= MAX_CONCURRENCY) return;
+
+    _processing++;
+
+    const jobs = _queue.splice(0, MAX_JOB_SIZE);
+
+    // if queue has still jobs after splice, we request a future processing
+    if (_queue.length > 0) {
+      debounce(this, this._processQueue, DEBOUNCING_DELAY);
+    }
+
+    let reports = {};
+    jobs.forEach(job => {
+      reports[job.type] = job.params;
+    });
+
+    ajax(BULK_REPORTS_ENDPOINT, { data: { reports } })
+      .then(response => {
+        jobs.forEach(job => {
+          const report = response.reports.findBy("type", job.type);
+          job.runnable()(report);
+        });
+      })
+      .catch(data => {
+        jobs.forEach(job => {
+          if (data.jqXHR && data.jqXHR.status === 429) {
+            job.runnable()(429);
+          } else if (data.jqXHR && data.jqXHR.status === 500) {
+            job.runnable()(500);
+          } else {
+            job.runnable()();
+          }
+        });
+      })
+      .finally(() => {
+        _processing--;
+
+        // when a request is done we want to start processing queue
+        // without waiting for debouncing
+        debounce(this, this._processQueue, DEBOUNCING_DELAY, true);
+      });
+  },
+
+  _reset() {
+    _queue = [];
+    _processing = 0;
+  }
+};

app/controllers/admin/reports_controller.rb

@@ -18,44 +18,41 @@ def index
     render_json_dump(reports: reports.sort_by { |report| report[:title] })
   end
 
-  def show
-    report_type = params[:type]
+  def bulk
+    reports = []
 
-    raise Discourse::NotFound unless report_type =~ /^[a-z0-9\_]+$/
+    hijack do
+      params[:reports].each do |report_type, report_params|
+        args = parse_params(report_params)
 
-    start_date = (params[:start_date].present? ? Time.parse(params[:start_date]).to_date : 1.days.ago).beginning_of_day
-    end_date = (params[:end_date].present? ? Time.parse(params[:end_date]).to_date : start_date + 30.days).end_of_day
+        report = nil
+        if (report_params[:cache])
+          report = Report.find_cached(report_type, args)
+        end
 
-    if params.has_key?(:category_id) && params[:category_id].to_i > 0
-      category_id = params[:category_id].to_i
-    else
-      category_id = nil
-    end
+        if report
+          reports << report
+        else
+          report = Report.find(report_type, args)
 
-    if params.has_key?(:group_id) && params[:group_id].to_i > 0
-      group_id = params[:group_id].to_i
-    else
-      group_id = nil
-    end
+          if (report_params[:cache]) && report
+            Report.cache(report, 35.minutes)
+          end
 
-    facets = nil
-    if Array === params[:facets]
-      facets = params[:facets].map { |s| s.to_s.to_sym }
-    end
+          reports << report if report
+        end
+      end
 
-    limit = nil
-    if params.has_key?(:limit) && params[:limit].to_i > 0
-      limit = params[:limit].to_i
+      render_json_dump(reports: reports)
     end
+  end
 
-    args = {
-      start_date: start_date,
-      end_date: end_date,
-      category_id: category_id,
-      group_id: group_id,
-      facets: facets,
-      limit: limit
-    }
+  def show
+    report_type = params[:type]
+
+    raise Discourse::NotFound unless report_type =~ /^[a-z0-9\_]+$/
+
+    args = parse_params(params)
 
     report = nil
     if (params[:cache])
@@ -77,7 +74,43 @@ def show
 
       render_json_dump(report: report)
     end
-
   end
 
+  private
+
+  def parse_params(report_params)
+    start_date = (report_params[:start_date].present? ? Time.parse(report_params[:start_date]).to_date : 1.days.ago).beginning_of_day
+    end_date = (report_params[:end_date].present? ? Time.parse(report_params[:end_date]).to_date : start_date + 30.days).end_of_day
+
+    if report_params.has_key?(:category_id) && report_params[:category_id].to_i > 0
+      category_id = report_params[:category_id].to_i
+    else
+      category_id = nil
+    end
+
+    if report_params.has_key?(:group_id) && report_params[:group_id].to_i > 0
+      group_id = report_params[:group_id].to_i
+    else
+      group_id = nil
+    end
+
+    facets = nil
+    if Array === report_params[:facets]
+      facets = report_params[:facets].map { |s| s.to_s.to_sym }
+    end
+
+    limit = nil
+    if report_params.has_key?(:limit) && report_params[:limit].to_i > 0
+      limit = report_params[:limit].to_i
+    end
+
+    {
+      start_date: start_date,
+      end_date: end_date,
+      category_id: category_id,
+      group_id: group_id,
+      facets: facets,
+      limit: limit
+    }
+  end
 end

config/routes.rb

@@ -76,6 +76,7 @@
     end
 
     get "reports" => "reports#index"
+    get "reports/bulk" => "reports#bulk"
     get "reports/:type" => "reports#show"
 
     resources :groups, constraints: AdminConstraint.new do

spec/requests/admin/reports_controller_spec.rb

@@ -13,6 +13,40 @@
       sign_in(admin)
     end
 
+    describe '#bulk' do
+      context "valid params" do
+        it "renders the reports as JSON" do
+          Fabricate(:topic)
+          get "/admin/reports/bulk.json", params: {
+            reports: {
+              topics: { limit: 10 },
+              likes: { limit: 10 }
+            }
+          }
+
+          expect(response.status).to eq(200)
+          expect(JSON.parse(response.body)["reports"].count).to eq(2)
+        end
+      end
+
+      context "invalid params" do
+        context "inexisting report" do
+          it "returns only existing reports" do
+            get "/admin/reports/bulk.json", params: {
+              reports: {
+                topics: { limit: 10 },
+                xxx: { limit: 10 }
+              }
+            }
+
+            expect(response.status).to eq(200)
+            expect(JSON.parse(response.body)["reports"].count).to eq(1)
+            expect(JSON.parse(response.body)["reports"][0]["type"]).to eq("topics")
+          end
+        end
+      end
+    end
+
     describe '#show' do
       context "invalid id form" do
         let(:invalid_id) { "!!&asdfasdf" }

test/javascripts/components/admin-report-test.js.es6

@@ -138,7 +138,7 @@ componentTest("rate limited", {
     };
 
     // prettier-ignore
-    server.get("/admin/reports/signups_rate_limited", () => { //eslint-disable-line
+    server.get("/admin/reports/bulk", () => { //eslint-disable-line
       return response({"errors":["You’ve performed this action too many times. Please wait 10 seconds before trying again."],"error_type":"rate_limit","extras":{"wait_seconds":10}});
     });
   },