From 3a7ca97c36536ae4e49ce5c479e9fb50dc389fcc Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 10 Jun 2020 18:57:39 +0300 Subject: [PATCH] FIX: Use include-subcategories filter in report export (#10007) Some filters were renamed and the conversion of the filter names and arguments was removed. --- .../admin/components/admin-report.js | 16 +++++--- app/controllers/export_csv_controller.rb | 2 +- app/jobs/regular/export_csv_file.rb | 9 ++--- app/models/report.rb | 6 ++- app/models/reports/top_uploads.rb | 4 +- config/locales/client.en.yml | 4 +- spec/jobs/export_csv_file_spec.rb | 21 +++++++++- spec/models/report_spec.rb | 39 ++++++++++++++++--- 8 files changed, 76 insertions(+), 25 deletions(-) diff --git a/app/assets/javascripts/admin/components/admin-report.js b/app/assets/javascripts/admin/components/admin-report.js index bacb6177e1d..e64e02f3ebb 100644 --- a/app/assets/javascripts/admin/components/admin-report.js +++ b/app/assets/javascripts/admin/components/admin-report.js @@ -226,14 +226,18 @@ export default Component.extend({ @action exportCsv() { - const customFilters = this.get("filters.customFilters") || {}; - exportEntity("report", { + const args = { name: this.get("model.type"), start_date: this.startDate.toISOString(true).split("T")[0], - end_date: this.endDate.toISOString(true).split("T")[0], - category_id: customFilters.category, - group_id: customFilters.group - }).then(outputExportResult); + end_date: this.endDate.toISOString(true).split("T")[0] + }; + + const customFilters = this.get("filters.customFilters"); + if (customFilters) { + Object.assign(args, customFilters); + } + + exportEntity("report", args).then(outputExportResult); }, @action diff --git a/app/controllers/export_csv_controller.rb b/app/controllers/export_csv_controller.rb index 6ac04125512..19257357db4 100644 --- a/app/controllers/export_csv_controller.rb +++ b/app/controllers/export_csv_controller.rb @@ -18,7 +18,7 @@ class ExportCsvController < ApplicationController def export_params @_export_params ||= begin params.require(:entity) - params.permit(:entity, args: [:name, :start_date, :end_date, :category_id, :group_id, :trust_level]).to_h + params.permit(:entity, args: Report::FILTERS).to_h end end end diff --git a/app/jobs/regular/export_csv_file.rb b/app/jobs/regular/export_csv_file.rb index 7d9b00d1661..3a63c88ea16 100644 --- a/app/jobs/regular/export_csv_file.rb +++ b/app/jobs/regular/export_csv_file.rb @@ -215,12 +215,9 @@ module Jobs end @extra[:filters] = {} - if @extra[:category_id].present? - @extra[:filters][:category] = @extra[:category_id].to_i - end - if @extra[:group_id].present? - @extra[:filters][:group] = @extra[:group_id].to_i - end + @extra[:filters][:category] = @extra[:category].to_i if @extra[:category].present? + @extra[:filters][:group] = @extra[:group].to_i if @extra[:group].present? + @extra[:filters][:include_subcategories] = !!ActiveRecord::Type::Boolean.new.cast(@extra[:include_subcategories]) if @extra[:include_subcategories].present? report = Report.find(@extra[:name], @extra) diff --git a/app/models/report.rb b/app/models/report.rb index b66e36c7721..bf8d1c33135 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -5,6 +5,8 @@ class Report # and you want to ensure cache is reset SCHEMA_VERSION = 4 + FILTERS = [:name, :start_date, :end_date, :category, :group, :trust_level, :file_extension, :include_subcategories] + attr_accessor :type, :data, :total, :prev30Days, :start_date, :end_date, :labels, :prev_period, :facets, :limit, :average, :percent, :higher_is_better, :icon, :modes, :prev_data, @@ -80,9 +82,9 @@ class Report add_filter('category', type: 'category', default: category_id) return if category_id.blank? - include_subcategories = filters[:'include-subcategories'] + include_subcategories = filters[:include_subcategories] include_subcategories = !!ActiveRecord::Type::Boolean.new.cast(include_subcategories) - add_filter('include-subcategories', type: 'bool', default: include_subcategories) + add_filter('include_subcategories', type: 'bool', default: include_subcategories) [category_id, include_subcategories] end diff --git a/app/models/reports/top_uploads.rb b/app/models/reports/top_uploads.rb index e5a4f542cfa..f410fd4c6d4 100644 --- a/app/models/reports/top_uploads.rb +++ b/app/models/reports/top_uploads.rb @@ -3,8 +3,8 @@ Report.add_report('top_uploads') do |report| report.modes = [:table] - extension_filter = report.filters.dig(:"file-extension") - report.add_filter('file-extension', + extension_filter = report.filters.dig(:file_extension) + report.add_filter('file_extension', type: 'list', default: extension_filter || 'any', choices: (SiteSetting.authorized_extensions.split('|') + Array(extension_filter)).uniq diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1dc4865929e..2de77fce976 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3458,13 +3458,13 @@ en: more: 'Search logs' disabled: 'Trending search report is disabled. Enable log search queries to collect data.' filters: - file-extension: + file_extension: label: File extension group: label: Group category: label: Category - include-subcategories: + include_subcategories: label: "Include Subcategories" commits: diff --git a/spec/jobs/export_csv_file_spec.rb b/spec/jobs/export_csv_file_spec.rb index a29bae2549f..45ab2520cb0 100644 --- a/spec/jobs/export_csv_file_spec.rb +++ b/spec/jobs/export_csv_file_spec.rb @@ -121,7 +121,7 @@ describe Jobs::ExportCsvFile do user1.user_visits.create!(visited_at: '2010-01-03', posts_read: 420) exporter.extra['name'] = 'visits' - exporter.extra['group_id'] = group.id + exporter.extra['group'] = group.id report = exporter.report_export.to_a expect(report.length).to eq(2) @@ -188,6 +188,25 @@ describe Jobs::ExportCsvFile do expect(report[3]).to contain_exactly("2010-01-03", "3", "6", "9") end + it 'works with posts reports and filters' do + category = Fabricate(:category) + subcategory = Fabricate(:category, parent_category: category) + + Fabricate(:post, topic: Fabricate(:topic, category: category), created_at: '2010-01-01 12:00:00 UTC') + Fabricate(:post, topic: Fabricate(:topic, category: subcategory), created_at: '2010-01-01 12:00:00 UTC') + + exporter.extra['name'] = 'posts' + + exporter.extra['category'] = category.id + report = exporter.report_export.to_a + expect(report[0]).to contain_exactly("Count", "Day") + expect(report[1]).to contain_exactly("1", "2010-01-01") + + exporter.extra['include_subcategories'] = true + report = exporter.report_export.to_a + expect(report[0]).to contain_exactly("Count", "Day") + expect(report[1]).to contain_exactly("2", "2010-01-01") + end end let(:user_list_header) { diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 7ccaf609411..576c713125d 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -771,7 +771,7 @@ describe Report do include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('flags', filters: { category: c0.id, 'include-subcategories': true }) } + let(:report) { Report.find('flags', filters: { category: c0.id, 'include_subcategories': true }) } include_examples 'category filtering on subcategories' end @@ -801,7 +801,7 @@ describe Report do include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('topics', filters: { category: c0.id, 'include-subcategories': true }) } + let(:report) { Report.find('topics', filters: { category: c0.id, 'include_subcategories': true }) } include_examples 'category filtering on subcategories' end @@ -892,7 +892,7 @@ describe Report do include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('posts', filters: { category: c0.id, 'include-subcategories': true }) } + let(:report) { Report.find('posts', filters: { category: c0.id, 'include_subcategories': true }) } include_examples 'category filtering on subcategories' end @@ -924,7 +924,7 @@ describe Report do include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('topics_with_no_response', filters: { category: c0.id, 'include-subcategories': true }) } + let(:report) { Report.find('topics_with_no_response', filters: { category: c0.id, 'include_subcategories': true }) } include_examples 'category filtering on subcategories' end @@ -960,7 +960,7 @@ describe Report do include_examples 'category filtering' context "on subcategories" do - let(:report) { Report.find('likes', filters: { category: c0.id, 'include-subcategories': true }) } + let(:report) { Report.find('likes', filters: { category: c0.id, 'include_subcategories': true }) } include_examples 'category filtering on subcategories' end @@ -1236,4 +1236,33 @@ describe Report do end end end + + describe "top_uploads" do + context "with no data" do + it "works" do + report = Report.find("top_uploads") + + expect(report.data).to be_empty + end + end + + context "with data" do + fab!(:jpg_upload) { Fabricate(:upload, extension: :jpg) } + fab!(:png_upload) { Fabricate(:upload, extension: :png) } + + it "works" do + report = Report.find("top_uploads") + + expect(report.data.length).to eq(2) + expect(report.data.map { |row| row[:extension] }).to contain_exactly("jpg", "png") + end + + it "works with filters" do + report = Report.find("top_uploads", filters: { file_extension: "jpg" }) + + expect(report.data.length).to eq(1) + expect(report.data[0][:extension]).to eq("jpg") + end + end + end end