DEV: extract anniversary badge query (#19716)
So it can easily be overwritten in a plugin for example.
### Added more tests to provide better coverage
We previously only had `u.silenced_till IS NULL` but I made it consistent with pretty much every other places where we check for "active" users.
These two new lines do change the query a tiny bit though.
**Before**
- You could not get the badge if you were currently silenced (no matter what period is being checked)
- You could get the badge if you were suspended 😬
**After**
- You can't get the badge if you were silenced during the past year
- You can't get the badge if you were suspended during the past year
### Improved the performance of the query by using `NOT EXISTS` instead of `LEFT JOIN / COUNT() = 0`
There is no difference in behaviour between
```sql
LEFT JOIN user_badges AS ub ON ub.user_id = u.id AND ...
[...]
HAVING COUNT(ub.*) = 0
```
and
```sql
NOT EXISTS (SELECT 1 FROM user_badges AS ub WHERE ub.user_id = u.id AND ...)
```
The only difference is performance-wise. The `NOT EXISTS` is 10-30% faster on very large databases (aka. posts and users in X millions). I checked on 3 of the largest datasets I could find.
This commit is contained in:
parent
553b4888ba
commit
b704e338ef
|
@ -6,34 +6,13 @@ module Jobs
|
||||||
|
|
||||||
def execute(args)
|
def execute(args)
|
||||||
return unless SiteSetting.enable_badges?
|
return unless SiteSetting.enable_badges?
|
||||||
badge = Badge.find_by(id: Badge::Anniversary, enabled: true)
|
return unless badge = Badge.find_by(id: Badge::Anniversary, enabled: true)
|
||||||
return unless badge
|
|
||||||
|
|
||||||
start_date = args[:start_date] || 1.year.ago
|
start_date = args[:start_date] || 1.year.ago
|
||||||
end_date = start_date + 1.year
|
end_date = start_date + 1.year
|
||||||
|
|
||||||
fmt_end_date = end_date.iso8601(6)
|
sql = BadgeQueries.anniversaries(start_date, end_date)
|
||||||
fmt_start_date = start_date.iso8601(6)
|
user_ids = DB.query_single(sql)
|
||||||
|
|
||||||
user_ids = DB.query_single <<~SQL
|
|
||||||
SELECT u.id AS user_id
|
|
||||||
FROM users AS u
|
|
||||||
INNER JOIN posts AS p ON p.user_id = u.id
|
|
||||||
INNER JOIN topics AS t ON p.topic_id = t.id
|
|
||||||
LEFT OUTER JOIN user_badges AS ub ON ub.user_id = u.id AND
|
|
||||||
ub.badge_id = #{Badge::Anniversary} AND
|
|
||||||
ub.granted_at BETWEEN '#{fmt_start_date}' AND '#{fmt_end_date}'
|
|
||||||
WHERE u.active AND
|
|
||||||
u.silenced_till IS NULL AND
|
|
||||||
NOT p.hidden AND
|
|
||||||
p.deleted_at IS NULL AND
|
|
||||||
t.visible AND
|
|
||||||
t.archetype <> 'private_message' AND
|
|
||||||
p.created_at BETWEEN '#{fmt_start_date}' AND '#{fmt_end_date}' AND
|
|
||||||
u.created_at <= '#{fmt_start_date}'
|
|
||||||
GROUP BY u.id
|
|
||||||
HAVING COUNT(p.id) > 0 AND COUNT(ub.id) = 0
|
|
||||||
SQL
|
|
||||||
|
|
||||||
User
|
User
|
||||||
.where(id: user_ids)
|
.where(id: user_ids)
|
||||||
|
|
|
@ -271,4 +271,32 @@ module BadgeQueries
|
||||||
WHERE "rank" = 1
|
WHERE "rank" = 1
|
||||||
SQL
|
SQL
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def self.anniversaries(start_date, end_date)
|
||||||
|
start_date = start_date.iso8601(6)
|
||||||
|
end_date = end_date.iso8601(6)
|
||||||
|
|
||||||
|
<<~SQL
|
||||||
|
SELECT u.id
|
||||||
|
FROM users AS u
|
||||||
|
JOIN posts AS p ON p.user_id = u.id
|
||||||
|
JOIN topics AS t ON p.topic_id = t.id
|
||||||
|
WHERE u.id > 0
|
||||||
|
AND u.active
|
||||||
|
AND NOT u.staged
|
||||||
|
AND (u.silenced_till IS NULL OR u.silenced_till < '#{start_date}')
|
||||||
|
AND (u.suspended_till IS NULL OR u.suspended_till < '#{start_date}')
|
||||||
|
AND u.created_at <= '#{start_date}'
|
||||||
|
AND NOT p.hidden
|
||||||
|
AND p.deleted_at IS NULL
|
||||||
|
AND p.created_at BETWEEN '#{start_date}' AND '#{end_date}'
|
||||||
|
AND t.visible
|
||||||
|
AND t.archetype <> 'private_message'
|
||||||
|
AND t.deleted_at IS NULL
|
||||||
|
AND NOT EXISTS (SELECT 1 FROM user_badges AS ub WHERE ub.user_id = u.id AND ub.badge_id = #{Badge::Anniversary} AND ub.granted_at BETWEEN '#{start_date}' AND '#{end_date}')
|
||||||
|
AND NOT EXISTS (SELECT 1 FROM anonymous_users AS au WHERE au.user_id = u.id)
|
||||||
|
GROUP BY u.id
|
||||||
|
HAVING COUNT(p.id) > 0
|
||||||
|
SQL
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -12,6 +12,15 @@ RSpec.describe Jobs::GrantAnniversaryBadges do
|
||||||
expect(badge).to be_blank
|
expect(badge).to be_blank
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "doesn't award to a bot" do
|
||||||
|
user = Fabricate(:bot, created_at: 400.days.ago)
|
||||||
|
Fabricate(:post, user: user, created_at: 1.week.ago)
|
||||||
|
granter.execute({})
|
||||||
|
|
||||||
|
badge = user.user_badges.where(badge_id: Badge::Anniversary)
|
||||||
|
expect(badge).to be_blank
|
||||||
|
end
|
||||||
|
|
||||||
it "doesn't award to an inactive user" do
|
it "doesn't award to an inactive user" do
|
||||||
user = Fabricate(:user, created_at: 400.days.ago, active: false)
|
user = Fabricate(:user, created_at: 400.days.ago, active: false)
|
||||||
Fabricate(:post, user: user, created_at: 1.week.ago)
|
Fabricate(:post, user: user, created_at: 1.week.ago)
|
||||||
|
@ -30,6 +39,33 @@ RSpec.describe Jobs::GrantAnniversaryBadges do
|
||||||
expect(badge).to be_blank
|
expect(badge).to be_blank
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "doesn't award to a suspended user" do
|
||||||
|
user = Fabricate(:user, created_at: 400.days.ago, suspended_till: 1.year.from_now)
|
||||||
|
Fabricate(:post, user: user, created_at: 1.week.ago)
|
||||||
|
granter.execute({})
|
||||||
|
|
||||||
|
badge = user.user_badges.where(badge_id: Badge::Anniversary)
|
||||||
|
expect(badge).to be_blank
|
||||||
|
end
|
||||||
|
|
||||||
|
it "doesn't award to a staged user" do
|
||||||
|
user = Fabricate(:user, created_at: 400.days.ago, staged: true)
|
||||||
|
Fabricate(:post, user: user, created_at: 1.week.ago)
|
||||||
|
granter.execute({})
|
||||||
|
|
||||||
|
badge = user.user_badges.where(badge_id: Badge::Anniversary)
|
||||||
|
expect(badge).to be_blank
|
||||||
|
end
|
||||||
|
|
||||||
|
it "doesn't award to an anonymous user" do
|
||||||
|
user = Fabricate(:anonymous, created_at: 400.days.ago)
|
||||||
|
Fabricate(:post, user: user, created_at: 1.week.ago)
|
||||||
|
granter.execute({})
|
||||||
|
|
||||||
|
badge = user.user_badges.where(badge_id: Badge::Anniversary)
|
||||||
|
expect(badge).to be_blank
|
||||||
|
end
|
||||||
|
|
||||||
it "doesn't award when a post is deleted" do
|
it "doesn't award when a post is deleted" do
|
||||||
user = Fabricate(:user, created_at: 400.days.ago)
|
user = Fabricate(:user, created_at: 400.days.ago)
|
||||||
Fabricate(:post, user: user, created_at: 1.week.ago, deleted_at: 1.day.ago)
|
Fabricate(:post, user: user, created_at: 1.week.ago, deleted_at: 1.day.ago)
|
||||||
|
|
Loading…
Reference in New Issue