From 105337e1aecfe3401db93018d1fe829af9c3038c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Thu, 5 Mar 2026 18:58:24 -0600 Subject: [PATCH 1/2] 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). --- app/jobs/daily_puzzle_job.rb | 2 +- app/jobs/puzzle_inventory_check_job.rb | 2 +- test/jobs/daily_puzzle_job_test.rb | 67 +++++++++++++++ test/jobs/puzzle_inventory_check_job_test.rb | 87 ++++++++++++++++++++ 4 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 test/jobs/daily_puzzle_job_test.rb create mode 100644 test/jobs/puzzle_inventory_check_job_test.rb diff --git a/app/jobs/daily_puzzle_job.rb b/app/jobs/daily_puzzle_job.rb index 90067a9..8b1a8a8 100644 --- a/app/jobs/daily_puzzle_job.rb +++ b/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| diff --git a/app/jobs/puzzle_inventory_check_job.rb b/app/jobs/puzzle_inventory_check_job.rb index 656e42c..02c320a 100644 --- a/app/jobs/puzzle_inventory_check_job.rb +++ b/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) diff --git a/test/jobs/daily_puzzle_job_test.rb b/test/jobs/daily_puzzle_job_test.rb new file mode 100644 index 0000000..c470ed0 --- /dev/null +++ b/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 diff --git a/test/jobs/puzzle_inventory_check_job_test.rb b/test/jobs/puzzle_inventory_check_job_test.rb new file mode 100644 index 0000000..69e3ab1 --- /dev/null +++ b/test/jobs/puzzle_inventory_check_job_test.rb @@ -0,0 +1,87 @@ +require "test_helper" + +class PuzzleInventoryCheckJobTest < ActiveJob::TestCase + setup do + Puzzle.approved.where(sent_at: nil).delete_all + 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 From 3b624ce2fa04211be8b9bbdbf18c1ed1fa0abf1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Thu, 5 Mar 2026 19:00:11 -0600 Subject: [PATCH 2/2] Small refactor --- app/jobs/puzzle_inventory_check_job.rb | 6 +++++- test/jobs/puzzle_inventory_check_job_test.rb | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/jobs/puzzle_inventory_check_job.rb b/app/jobs/puzzle_inventory_check_job.rb index 02c320a..dd64c67 100644 --- a/app/jobs/puzzle_inventory_check_job.rb +++ b/app/jobs/puzzle_inventory_check_job.rb @@ -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 diff --git a/test/jobs/puzzle_inventory_check_job_test.rb b/test/jobs/puzzle_inventory_check_job_test.rb index 69e3ab1..9e16261 100644 --- a/test/jobs/puzzle_inventory_check_job_test.rb +++ b/test/jobs/puzzle_inventory_check_job_test.rb @@ -3,6 +3,7 @@ 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