mirror of
https://github.com/discourse/discourse.git
synced 2024-11-28 19:53:53 -06:00
d91456fd53
This adds the ability to collect stats without exposing them
among other stats via API.
The most important thing I wanted to achieve is to provide
an API where stats are not exposed by default, and a developer
has to explicitly specify that they should be
exposed (`expose_via_api: true`). Implementing an opposite
solution would be simpler, but that's less safe in terms of
potential security issues.
When working on this, I had to refactor the current solution.
I would go even further with the refactoring, but the next steps
seem to be going too far in changing the solution we have,
and that would also take more time. Two things that can be
improved in the future:
1. Data structures for holding stats can be further improved
2. Core stats are hard-coded in the About template (it's hard
to fix it without correcting data structures first, see point 1):
63a0700d45/app/views/about/index.html.erb (L61-L101)
The most significant refactorings are:
1. Introducing the `Stat` model
2. Aligning the way the core and the plugin stats' are registered
116 lines
4.0 KiB
Ruby
116 lines
4.0 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
RSpec.describe About do
|
|
describe "stats cache" do
|
|
include_examples "stats cacheable"
|
|
end
|
|
|
|
def register_stat(name, stats_block)
|
|
DiscoursePluginRegistry.register_stat(
|
|
Stat.new(name, show_in_ui: true, expose_via_api: true, &stats_block),
|
|
stub(enabled?: true),
|
|
)
|
|
end
|
|
|
|
after { DiscoursePluginRegistry.reset! }
|
|
|
|
describe "#stats" do
|
|
it "adds plugin stats to the output" do
|
|
stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 }
|
|
register_stat("some_group", Proc.new { stats })
|
|
expect(described_class.new.stats.with_indifferent_access).to match(
|
|
hash_including(
|
|
some_group_last_day: 1,
|
|
some_group_7_days: 10,
|
|
some_group_30_days: 100,
|
|
some_group_count: 1000,
|
|
),
|
|
)
|
|
end
|
|
|
|
it "does not add plugin stats to the output if they are missing one of the required keys" do
|
|
stats = { "7_days" => 10, "30_days" => 100, :count => 1000 }
|
|
register_stat("some_group", Proc.new { stats })
|
|
expect(described_class.new.stats).not_to match(
|
|
hash_including(
|
|
some_group_last_day: 1,
|
|
some_group_7_days: 10,
|
|
some_group_30_days: 100,
|
|
some_group_count: 1000,
|
|
),
|
|
)
|
|
end
|
|
|
|
it "does not error if any of the plugin stat blocks throw an error and still adds the non-errored stats to output" do
|
|
stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 }
|
|
register_stat("some_group", Proc.new { stats })
|
|
register_stat("other_group", Proc.new { raise StandardError })
|
|
expect(described_class.new.stats.with_indifferent_access).to match(
|
|
hash_including(
|
|
some_group_last_day: 1,
|
|
some_group_7_days: 10,
|
|
some_group_30_days: 100,
|
|
some_group_count: 1000,
|
|
),
|
|
)
|
|
expect { described_class.new.stats.with_indifferent_access }.not_to raise_error
|
|
end
|
|
end
|
|
|
|
describe "#category_moderators" do
|
|
let(:user) { Fabricate(:user) }
|
|
let(:public_cat_moderator) { Fabricate(:user, last_seen_at: 1.month.ago) }
|
|
let(:private_cat_moderator) { Fabricate(:user, last_seen_at: 2.month.ago) }
|
|
let(:common_moderator) { Fabricate(:user, last_seen_at: 3.month.ago) }
|
|
let(:common_moderator_2) { Fabricate(:user, last_seen_at: 4.month.ago) }
|
|
|
|
let(:public_group) do
|
|
group = Fabricate(:public_group)
|
|
group.add(public_cat_moderator)
|
|
group.add(common_moderator)
|
|
group.add(common_moderator_2)
|
|
group
|
|
end
|
|
|
|
let(:private_group) do
|
|
group = Fabricate(:group)
|
|
group.add(private_cat_moderator)
|
|
group.add(common_moderator)
|
|
group.add(common_moderator_2)
|
|
group
|
|
end
|
|
|
|
let!(:public_cat) { Fabricate(:category, reviewable_by_group: public_group) }
|
|
let!(:private_cat) do
|
|
Fabricate(:private_category, group: private_group, reviewable_by_group: private_group)
|
|
end
|
|
|
|
it "lists moderators of the category that the current user can see" do
|
|
results = About.new(private_cat_moderator).category_moderators
|
|
expect(results.map(&:category_id)).to contain_exactly(public_cat.id, private_cat.id)
|
|
expect(results.map(&:moderators).flatten.map(&:id).uniq).to contain_exactly(
|
|
public_cat_moderator.id,
|
|
common_moderator.id,
|
|
common_moderator_2.id,
|
|
private_cat_moderator.id,
|
|
)
|
|
|
|
[public_cat_moderator, user, nil].each do |u|
|
|
results = About.new(u).category_moderators
|
|
expect(results.map(&:category_id)).to contain_exactly(public_cat.id)
|
|
expect(results.map(&:moderators).flatten.map(&:id)).to eq(
|
|
[public_cat_moderator.id, common_moderator.id, common_moderator_2.id],
|
|
)
|
|
end
|
|
end
|
|
|
|
it "limit category moderators when there are too many for perf reasons" do
|
|
about = About.new(private_cat_moderator)
|
|
about.category_mods_limit = 4
|
|
results = about.category_moderators
|
|
expect(results.size).to eq(2)
|
|
results.each { |res| expect(res.moderators.size).to eq(2) }
|
|
end
|
|
end
|
|
end
|