FIX: Award "User of the month" badge at the beginning of month

Previously the badge was granted one month after the last time the badge was granted. The exact date shifted by one day each month. The new logic tries to grant the badge always at the beginning of a new month by looking at new users of the previous month. The "granted at" date is set to the end of the previous month.

app/jobs/scheduled/grant_new_user_of_the_month_badges.rb

@@ -10,21 +10,22 @@ module Jobs
       badge = Badge.find(Badge::NewUserOfTheMonth)
       return unless SiteSetting.enable_badges? && badge.enabled?
 
-      # Don't award it if a month hasn't gone by
+      previous_month_beginning = 1.month.ago.beginning_of_month
-      return if UserBadge.where("badge_id = ? AND granted_at >= ?",
+      previous_month_end = 1.month.ago.end_of_month
-        badge.id,
+
-        (1.month.ago + 1.day)  # give it a day slack just in case
+      return if UserBadge.where("badge_id = ? AND granted_at BETWEEN ? AND ?",
+        badge.id, previous_month_beginning, Time.zone.now
       ).exists?
 
-      scores.each do |user_id, score|
+      scores(previous_month_beginning).each do |user_id, score|
         # Don't bother awarding to users who haven't received any likes
         if score > 0.0
           user = User.find(user_id)
           if user.badges.where(id: Badge::NewUserOfTheMonth).blank?
-            BadgeGranter.grant(badge, user)
+            BadgeGranter.grant(badge, user, created_at: previous_month_end)
 
             SystemMessage.new(user).create('new_user_of_the_month',
-              month_year: I18n.l(Time.now, format: :no_day),
+              month_year: I18n.l(previous_month_beginning, format: :no_day),
               url: "#{Discourse.base_url}/badges"
             )
           end
@@ -32,7 +33,7 @@ module Jobs
       end
     end
 
-    def scores
+    def scores(user_created_after_date)
       current_owners = UserBadge.where(badge_id: Badge::NewUserOfTheMonth).pluck(:user_id)
       current_owners = [-1] if current_owners.blank?
 
@@ -70,7 +71,7 @@ module Jobs
           AND NOT u.moderator
           AND u.suspended_at IS NULL
           AND u.suspended_till IS NULL
-          AND u.created_at >= CURRENT_TIMESTAMP - '1 month'::INTERVAL
+          AND u.created_at >= :min_user_created_at
           AND t.archetype <> '#{Archetype.private_message}'
           AND t.deleted_at IS NULL
           AND p.deleted_at IS NULL
@@ -82,7 +83,7 @@ module Jobs
         LIMIT #{MAX_AWARDED}
       SQL
 
-      Hash[*DB.query_single(sql)]
+      Hash[*DB.query_single(sql, min_user_created_at: user_created_after_date)]
     end
 
   end

spec/jobs/grant_new_user_of_the_month_badges_spec.rb

@@ -7,8 +7,12 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
   let(:granter) { described_class.new }
 
   it "runs correctly" do
+    freeze_time(DateTime.parse('2019-11-30 23:59 UTC'))
+
     u0 = Fabricate(:user, created_at: 2.weeks.ago)
-    BadgeGranter.grant(Badge.find(Badge::NewUserOfTheMonth), u0, created_at: 1.month.ago)
+    BadgeGranter.grant(Badge.find(Badge::NewUserOfTheMonth), u0, created_at: Time.now)
+
+    freeze_time(DateTime.parse('2020-01-01 00:00 UTC'))
 
     user = Fabricate(:user, created_at: 1.week.ago)
     p = Fabricate(:post, user: user)
@@ -21,8 +25,9 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
 
     granter.execute({})
 
-    badge = user.user_badges.where(badge_id: Badge::NewUserOfTheMonth)
+    badges = user.user_badges.where(badge_id: Badge::NewUserOfTheMonth)
-    expect(badge).to be_present
+    expect(badges).to be_present
+    expect(badges.first.granted_at.to_s).to eq('2019-12-31 23:59:59 UTC')
   end
 
   it "does nothing if badges are disabled" do
@@ -63,9 +68,13 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
     expect(badge).to be_blank
   end
 
-  it "does nothing if it's been awarded recently" do
+  it "does nothing if it's already been awarded in previous month" do
+    freeze_time(DateTime.parse('2019-11-30 23:59 UTC'))
+
     u0 = Fabricate(:user, created_at: 2.weeks.ago)
-    BadgeGranter.grant(Badge.find(Badge::NewUserOfTheMonth), u0)
+    BadgeGranter.grant(Badge.find(Badge::NewUserOfTheMonth), u0, created_at: Time.now)
+
+    freeze_time(DateTime.parse('2019-12-01 00:00 UTC'))
 
     user = Fabricate(:user, created_at: 1.week.ago)
     p = Fabricate(:post, user: user)
@@ -83,6 +92,9 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
   end
 
   describe '.scores' do
+    def scores
+      granter.scores(1.month.ago)
+    end
 
     it "doesn't award it to accounts over a month old" do
       user = Fabricate(:user, created_at: 2.months.ago)
@@ -93,7 +105,7 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
       old_user = Fabricate(:user, created_at: 6.months.ago)
       PostActionCreator.like(old_user, p)
 
-      expect(granter.scores.keys).not_to include(user.id)
+      expect(scores.keys).not_to include(user.id)
     end
 
     it "doesn't score users who haven't posted in two topics" do
@@ -104,7 +116,7 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
       old_user = Fabricate(:user, created_at: 6.months.ago)
       PostActionCreator.like(old_user, p)
 
-      expect(granter.scores.keys).not_to include(user.id)
+      expect(scores.keys).not_to include(user.id)
     end
 
     it "doesn't count private topics" do
@@ -117,7 +129,7 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
       old_user = Fabricate(:user, created_at: 6.months.ago)
       PostActionCreator.like(old_user, p)
 
-      expect(granter.scores.keys).not_to include(user.id)
+      expect(scores.keys).not_to include(user.id)
     end
 
     it "requires at least two likes to be considered" do
@@ -127,7 +139,7 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
       old_user = Fabricate(:user, created_at: 6.months.ago)
       PostActionCreator.like(old_user, p)
 
-      expect(granter.scores.keys).not_to include(user.id)
+      expect(scores.keys).not_to include(user.id)
     end
 
     it "returns scores for accounts created within the last month" do
@@ -139,7 +151,7 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
       old_user = Fabricate(:user, created_at: 6.months.ago)
       PostActionCreator.like(old_user, p)
 
-      expect(granter.scores.keys).to include(user.id)
+      expect(scores.keys).to include(user.id)
     end
 
     it "likes from older accounts are scored higher" do
@@ -163,11 +175,11 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
       PostActionCreator.like(um, p)
       PostActionCreator.like(ua, p)
       PostActionCreator.like(Discourse.system_user, p)
-      expect(granter.scores[user.id]).to eq(1.55)
+      expect(scores[user.id]).to eq(1.55)
 
       # It goes down the more they post
       Fabricate(:post, user: user)
-      expect(granter.scores[user.id]).to eq(1.35625)
+      expect(scores[user.id]).to eq(1.35625)
     end
 
     it "is limited to two accounts" do
@@ -193,7 +205,7 @@ describe Jobs::GrantNewUserOfTheMonthBadges do
       PostActionCreator.like(ou1, p)
       PostActionCreator.like(ou2, p)
 
-      expect(granter.scores.keys.size).to eq(2)
+      expect(scores.keys.size).to eq(2)
     end
 
   end