Fix raw state integers + add sentry logs (#73)

* Replace raw state integers with named enum scopes in jobs

Using state: 0 relied on knowing the integer value of the approved enum,
which breaks silently if values change. Both DailyPuzzleJob and
PuzzleInventoryCheckJob now use Puzzle.approved.where(sent_at: nil).

* Small refactor

Author: JuanVqz

app/jobs/daily_puzzle_job.rb

@@ -3,7 +3,7 @@ class DailyPuzzleJob < ApplicationJob
   retry_on StandardError, attempts: 3
 
   def perform
-    puzzle = Puzzle.where(sent_at: nil, state: 0).order("RANDOM()").first
+    puzzle = Puzzle.approved.where(sent_at: nil).order("RANDOM()").first
     return unless puzzle
 
     Server.where(active: true).each do |server|

app/jobs/puzzle_inventory_check_job.rb

@@ -3,7 +3,7 @@ class PuzzleInventoryCheckJob < ApplicationJob
   retry_on StandardError, attempts: 3
 
   def perform
-    approved_unsent_puzzle_count = Puzzle.where(state: 0, sent_at: nil).count
+    approved_unsent_puzzle_count = Puzzle.approved.where(sent_at: nil).count
 
     if approved_unsent_puzzle_count < 5
       send_low_inventory_notification(approved_unsent_puzzle_count)
@@ -13,13 +13,17 @@ def perform
   private
 
   def send_low_inventory_notification(count)
+    channel_id = ENV["SHIELD_NOTIFICATIONS_CHANNEL"]
+    return Rails.logger.warn("SHIELD_NOTIFICATIONS_CHANNEL is not configured, skipping low inventory notification") unless channel_id
+
     notification_message = SlackClient::Messages::LowPuzzleInventoryNotification.new(count).create
-    send_message(notification_message, channel_id: ENV.fetch("SHIELD_NOTIFICATIONS_CHANNEL", nil))
+    send_message(notification_message, channel_id: channel_id)
   end
 
   def send_message(message, channel_id:)
     SlackClient::Client.instance.chat_postMessage(channel: channel_id, blocks: message)
   rescue Slack::Web::Api::Errors::SlackError => e
+    Sentry.capture_exception(e)
     Rails.logger.error "Failed to send Slack message: #{e.message} #{e.response_metadata}"
   end
 end

test/jobs/daily_puzzle_job_test.rb

@@ -0,0 +1,67 @@
+require "test_helper"
+
+class DailyPuzzleJobTest < ActiveJob::TestCase
+  test "only selects approved puzzles, not pending or rejected" do
+    pending_puzzle = Puzzle.create!(
+      question: "Pending puzzle question",
+      answer: "ruby",
+      explanation: "Test explanation",
+      state: :pending,
+      sent_at: nil,
+      suggested_by: "test_user"
+    )
+    rejected_puzzle = Puzzle.create!(
+      question: "Rejected puzzle question",
+      answer: "rails",
+      explanation: "Test explanation",
+      state: :rejected,
+      sent_at: nil,
+      suggested_by: "test_user"
+    )
+
+    DailyPuzzleJob.perform_now
+
+    assert_nil pending_puzzle.reload.sent_at
+    assert_nil rejected_puzzle.reload.sent_at
+  end
+
+  test "only selects puzzles that have not been sent yet" do
+    already_sent = Puzzle.create!(
+      question: "Already sent puzzle question",
+      answer: "ruby",
+      explanation: "Test explanation",
+      state: :approved,
+      sent_at: 1.day.ago,
+      suggested_by: "test_user"
+    )
+
+    DailyPuzzleJob.perform_now
+
+    assert_equal already_sent.reload.sent_at.to_i, 1.day.ago.to_i
+  end
+
+  test "marks the selected puzzle as archived and sets sent_at" do
+    puzzle = Puzzle.create!(
+      question: "Approved unsent puzzle question",
+      answer: "ruby",
+      explanation: "Test explanation",
+      state: :approved,
+      sent_at: nil,
+      suggested_by: "test_user"
+    )
+
+    DailyPuzzleJob.perform_now
+
+    puzzle.reload
+    assert_not_nil puzzle.sent_at
+    assert_equal "archived", puzzle.state
+  end
+
+  test "does nothing when no approved unsent puzzles exist" do
+    Puzzle.approved.where(sent_at: nil).delete_all
+
+    assert_nothing_raised do
+      DailyPuzzleJob.perform_now
+    end
+  end
+end

test/jobs/puzzle_inventory_check_job_test.rb

@@ -0,0 +1,88 @@
+require "test_helper"
+
+class PuzzleInventoryCheckJobTest < ActiveJob::TestCase
+  setup do
+    Puzzle.approved.where(sent_at: nil).delete_all
+    ENV["SHIELD_NOTIFICATIONS_CHANNEL"] = "test-channel"
+  end
+
+  test "sends notification when fewer than 5 approved unsent puzzles exist" do
+    3.times do |i|
+      Puzzle.create!(
+        question: "Approved unsent puzzle #{i}",
+        answer: "ruby",
+        state: :approved,
+        sent_at: nil,
+        explanation: "Test explanation",
+        suggested_by: "test_user"
+      )
+    end
+
+    notification_sent = false
+    job = PuzzleInventoryCheckJob.new
+    job.define_singleton_method(:send_message) { |*| notification_sent = true }
+    job.perform
+
+    assert notification_sent, "Expected a low inventory notification to be sent"
+  end
+
+  test "does not send notification when 5 or more approved unsent puzzles exist" do
+    5.times do |i|
+      Puzzle.create!(
+        question: "Approved unsent puzzle #{i}",
+        answer: "ruby",
+        state: :approved,
+        sent_at: nil,
+        explanation: "Test explanation",
+        suggested_by: "test_user"
+      )
+    end
+
+    notification_sent = false
+    job = PuzzleInventoryCheckJob.new
+    job.define_singleton_method(:send_message) { |*| notification_sent = true }
+    job.perform
+
+    assert_not notification_sent, "Expected no notification to be sent"
+  end
+
+  test "only counts approved puzzles, not pending or rejected" do
+    5.times do |i|
+      Puzzle.create!(
+        question: "Pending puzzle #{i}",
+        answer: "ruby",
+        state: :pending,
+        sent_at: nil,
+        explanation: "Test explanation",
+        suggested_by: "test_user"
+      )
+    end
+
+    notification_sent = false
+    job = PuzzleInventoryCheckJob.new
+    job.define_singleton_method(:send_message) { |*| notification_sent = true }
+    job.perform
+
+    assert notification_sent, "Pending puzzles should not count toward approved inventory"
+  end
+
+  test "only counts unsent puzzles" do
+    5.times do |i|
+      Puzzle.create!(
+        question: "Already sent puzzle #{i}",
+        answer: "ruby",
+        state: :approved,
+        sent_at: 1.day.ago,
+        explanation: "Test explanation",
+        suggested_by: "test_user"
+      )
+    end
+
+    notification_sent = false
+    job = PuzzleInventoryCheckJob.new
+    job.define_singleton_method(:send_message) { |*| notification_sent = true }
+    job.perform
+
+    assert notification_sent, "Already sent puzzles should not count toward available inventory"
+  end
+end