FIX: CheckPolicy job was expiring all user policies

FIX: CheckPolicy job was expiring all user policies

UPDATE policy_users SET expired_at = now() FROM policy_users pu JOIN post_policies pp ON pp.id = pu.post_policy_id WHERE …

is not how you join tables when doing an UPDATE.

In this case, the query would update the whole table once at least one row matches the conditions of the query.

The proper syntax is the following

UPDATE policy_users pu SET expired_at = now() FROM post_policies pp WHERE pp.id = pu.post_policy_id AND …

diff --git a/jobs/scheduled/check_policy.rb b/jobs/scheduled/check_policy.rb
index b63fb94..fb29070 100644
--- a/jobs/scheduled/check_policy.rb
+++ b/jobs/scheduled/check_policy.rb
@@ -8,32 +8,24 @@ module Jobs
 
     def execute(args = nil)
       sql = <<~SQL
-        SELECT p.id FROM post_policies pp
-        JOIN posts p ON p.id = pp.post_id
-        JOIN topics t ON t.id = p.topic_id
-        WHERE t.deleted_at IS NULL
-          AND p.deleted_at IS NULL
-          AND t.archetype = 'regular'
-          AND (
-          (
-            reminder = 'weekly' AND
-            last_reminded_at < :weekly
-          ) OR
-          (
-            reminder = 'daily' AND
-            last_reminded_at < :daily
-          ))
+        SELECT p.id
+          FROM post_policies pp
+          JOIN posts p ON p.id = pp.post_id
+          JOIN topics t ON t.id = p.topic_id
+         WHERE t.deleted_at IS NULL
+           AND p.deleted_at IS NULL
+           AND t.archetype = 'regular'
+           AND (
+             (reminder = 'weekly' AND last_reminded_at < :weekly)
+             OR
+             (reminder = 'daily' AND last_reminded_at < :daily)
+           )
       SQL
 
-      post_ids = DB.query_single(
-        sql,
-        weekly: 1.week.ago,
-        daily: 1.day.ago
-      )
+      post_ids = DB.query_single(sql, weekly: 1.week.ago, daily: 1.day.ago)
 
-      if post_ids.length > 0
+      if post_ids.size > 0
         Post.where(id: post_ids).find_each do |post|
-
           post.post_policy.update(last_reminded_at: Time.zone.now)
 
           missing_users(post).each do |user|
@@ -47,7 +39,7 @@ module Jobs
         end
       end
 
-      PostPolicy.where('next_renew_at < ?', Time.zone.now).find_each do |policy|
+      PostPolicy.where("next_renew_at < ?", Time.zone.now).find_each do |policy|
         policy.policy_users.accepted.update_all(expired_at: Time.zone.now)
         next_renew = policy.renew_start
         if policy.renew_days < 1
@@ -60,23 +52,18 @@ module Jobs
         policy.update(next_renew_at: next_renew)
       end
 
-      sql = <<~SQL
-      UPDATE policy_users
-      SET expired_at = :now
-      FROM policy_users pu
-      INNER JOIN post_policies pp ON pp.id = pu.post_policy_id
-      WHERE pp.renew_start IS NULL AND
-      pp.renew_days IS NOT NULL AND
-      pu.accepted_at IS NOT NULL AND
-      pu.expired_at IS NULL AND
-      pu.revoked_at IS NULL AND
-      pu.accepted_at < :now::timestamp - ( INTERVAL '1 day' *  pp.renew_days )
+      DB.exec <<~SQL
+        UPDATE policy_users pu
+           SET expired_at = now()
+          FROM post_policies pp
+         WHERE pp.id = pu.post_policy_id
+           AND pp.renew_start IS NULL
+           AND pp.renew_days  IS NOT NULL
+           AND pu.accepted_at IS NOT NULL
+           AND pu.expired_at  IS NULL
+           AND pu.revoked_at  IS NULL
+           AND pu.accepted_at < now() - (INTERVAL '1 day' * pp.renew_days)
       SQL
-
-      DB.exec(
-        sql,
-        now: Time.zone.now
-      )
     end
 
     def missing_users(post)

GitHub sha: 330c3375

No tests for this stuff?

Was planning to but @lis2 got there first :wink:

1 Like