From 03e373931edae9ac08667d4b2dc55a0cb50891ea Mon Sep 17 00:00:00 2001 From: tooyosi Date: Wed, 4 Jun 2025 00:41:45 +0100 Subject: [PATCH 1/5] add logic to notify project owner on subject retirement --- app/mailers/application_mailer.rb | 4 ++ app/mailers/project_notification_mailer.rb | 11 +++++ app/services/prediction_results/process.rb | 13 +++++ app/sidekiq/notify_project_owner_job.rb | 49 +++++++++++++++++++ .../notify_subject_completion.html.erb | 6 +++ config/application.rb | 1 + 6 files changed, 84 insertions(+) create mode 100644 app/mailers/application_mailer.rb create mode 100644 app/mailers/project_notification_mailer.rb create mode 100644 app/sidekiq/notify_project_owner_job.rb create mode 100644 app/views/project_notification_mailer/notify_subject_completion.html.erb diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb new file mode 100644 index 0000000..7dffcf4 --- /dev/null +++ b/app/mailers/application_mailer.rb @@ -0,0 +1,4 @@ +class ApplicationMailer < ActionMailer::Base + default from: "no-reply@zooniverse.org" + layout 'mailer' +end diff --git a/app/mailers/project_notification_mailer.rb b/app/mailers/project_notification_mailer.rb new file mode 100644 index 0000000..d80eef3 --- /dev/null +++ b/app/mailers/project_notification_mailer.rb @@ -0,0 +1,11 @@ +class ProjectNotificationMailer < ApplicationMailer + layout false + + def notify_subject_completion(user, context, completion_percentage) + @completion_percentage = completion_percentage + @project_name = context.module_name + @workflow_name = context.extractor_name + @email_to = user['email'] + mail(to: @email_to, subject: "Your subjects are almost retired ") + end + end diff --git a/app/services/prediction_results/process.rb b/app/services/prediction_results/process.rb index 37d9584..7920736 100755 --- a/app/services/prediction_results/process.rb +++ b/app/services/prediction_results/process.rb @@ -5,6 +5,7 @@ module PredictionResults class Process SUBJECT_ACTION_API_BATCH_SIZE = ENV.fetch('SUBJECT_ACTION_API_BATCH_SIZE', '10').to_i + COMPLETION_NOTIFICATION_THRESHOLD = ENV.fetch('COMPLETION_NOTIFICATION_THRESHOLD', '0.95').to_f attr_accessor :results_url, :subject_set_id, :probability_threshold, :over_threshold_subject_ids, :under_threshold_subject_ids, @@ -19,6 +20,7 @@ def initialize(results_url:, subject_set_id:, probability_threshold: 0.8, random @under_threshold_subject_ids = [] @random_spice_subject_ids = [] @prediction_data = nil + @total_subjects = 0 end def run @@ -50,6 +52,8 @@ def partition_results @over_threshold_subject_ids << subject_id if probability >= probability_threshold @under_threshold_subject_ids << subject_id if probability < probability_threshold end + @total_subjects = @over_threshold_subject_ids.count + @under_threshold_subject_ids.count + check_completion_and_notify # now add some 'spice' to the results by adding some random under threshold subject ids # but don't skew the prediction results by adding too many under threshold images # ensure we only use apply the randomisation factor to the count of over threshold subject ids @@ -84,5 +88,14 @@ def api_batch_bulk_job_args(subject_ids) .each_slice(SUBJECT_ACTION_API_BATCH_SIZE) .map { |batch_subject_ids| [batch_subject_ids, subject_set_id] } end + + private + def check_completion_and_notify + total_under_threshold_subjects = @under_threshold_subject_ids.count + completion_rate = (total_under_threshold_subjects.to_f / @total_subjects.to_f) + if completion_rate >= COMPLETION_NOTIFICATION_THRESHOLD + NotifyProjectOwnerJob.perform_async(subject_set_id, completion_rate) + end + end end end diff --git a/app/sidekiq/notify_project_owner_job.rb b/app/sidekiq/notify_project_owner_job.rb new file mode 100644 index 0000000..d0362a3 --- /dev/null +++ b/app/sidekiq/notify_project_owner_job.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true +require 'panoptes/api' + +class NotifyProjectOwnerJob + include Sidekiq::Job + sidekiq_options retry: 5 + + def perform(subject_set_id, completion_rate) + @context = Context.find_by!(active_subject_set_id: subject_set_id) + + owner_link = fetch_project_owner + owner_user = fetch_owner_user(owner_link['id']) + + ProjectNotificationMailer + .notify_subject_completion(owner_user, @context, (completion_rate * 100)) + .deliver_now + end + + private + def fetch_project_owner(max_retries = 3) + with_api_retry(max_retries) do + resp = Panoptes::Api.client.project(@context.project_id) + resp['links']['owner'] + end + end + + def fetch_owner_user(owner_id, max_retries = 3) + with_api_retry(max_retries) do + Panoptes::Api.client.user(owner_id) + end + end + + + def with_api_retry(max_retries) + attempts = 0 + + begin + yield + rescue Panoptes::Client::ServerError => e + attempts += 1 + raise if attempts > max_retries + + sleep(rand(max_retries)) + retry + ensure + Faraday.default_connection.close + end + end +end diff --git a/app/views/project_notification_mailer/notify_subject_completion.html.erb b/app/views/project_notification_mailer/notify_subject_completion.html.erb new file mode 100644 index 0000000..869de80 --- /dev/null +++ b/app/views/project_notification_mailer/notify_subject_completion.html.erb @@ -0,0 +1,6 @@ + +

Subjects belonging to <%= @workflow_name %> workflow of project: <%= @project_name %> have hit <%= @completion_percentage %>% completion

+ +

Thanks,

+ +

The Zooniverse Team

diff --git a/config/application.rb b/config/application.rb index 31fc618..4c991bd 100644 --- a/config/application.rb +++ b/config/application.rb @@ -6,6 +6,7 @@ require 'active_record/railtie' require 'action_controller/railtie' require 'active_storage/engine' +require "action_mailer/railtie" # Require the gems listed in Gemfile, including any gems # you've limited to :test, :development, or :production. From 26da3bb4923f55267097cbe4f90ac214d5e2ecb2 Mon Sep 17 00:00:00 2001 From: tooyosi Date: Thu, 5 Jun 2025 00:18:40 +0100 Subject: [PATCH 2/5] add tests --- app/mailers/project_notification_mailer.rb | 2 +- app/services/prediction_results/process.rb | 2 +- .../project_notification_mailer_spec.rb | 35 +++++++++++++++ .../prediction_results/process_spec.rb | 12 +++++ spec/sidekiq/notify_project_owner_job_spec.rb | 45 +++++++++++++++++++ 5 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 spec/mailers/project_notification_mailer_spec.rb create mode 100755 spec/sidekiq/notify_project_owner_job_spec.rb diff --git a/app/mailers/project_notification_mailer.rb b/app/mailers/project_notification_mailer.rb index d80eef3..4e209dc 100644 --- a/app/mailers/project_notification_mailer.rb +++ b/app/mailers/project_notification_mailer.rb @@ -6,6 +6,6 @@ def notify_subject_completion(user, context, completion_percentage) @project_name = context.module_name @workflow_name = context.extractor_name @email_to = user['email'] - mail(to: @email_to, subject: "Your subjects are almost retired ") + mail(to: @email_to, subject: "Your subjects are almost retired") end end diff --git a/app/services/prediction_results/process.rb b/app/services/prediction_results/process.rb index 7920736..4ea8fb0 100755 --- a/app/services/prediction_results/process.rb +++ b/app/services/prediction_results/process.rb @@ -41,6 +41,7 @@ def run end def partition_results + @total_subjects = prediction_data.count prediction_data.each do |subject_id, prediction_samples| # data schema format is published in the file # and https://github.com/zooniverse/bajor/blob/main/azure/batch/scripts/predict_on_catalog.py @@ -52,7 +53,6 @@ def partition_results @over_threshold_subject_ids << subject_id if probability >= probability_threshold @under_threshold_subject_ids << subject_id if probability < probability_threshold end - @total_subjects = @over_threshold_subject_ids.count + @under_threshold_subject_ids.count check_completion_and_notify # now add some 'spice' to the results by adding some random under threshold subject ids # but don't skew the prediction results by adding too many under threshold images diff --git a/spec/mailers/project_notification_mailer_spec.rb b/spec/mailers/project_notification_mailer_spec.rb new file mode 100644 index 0000000..0f0719b --- /dev/null +++ b/spec/mailers/project_notification_mailer_spec.rb @@ -0,0 +1,35 @@ +require "spec_helper" + +RSpec.describe ProjectNotificationMailer, :type => :mailer do + let(:user) { { 'email' => 'test@project@owner.com' } } + let(:context) { Context.first } + let(:completion_percentage) { 96 } + let(:mail) { ProjectNotificationMailer.notify_subject_completion(user, context, completion_percentage)} + + describe "#notify_subject_completion" do + + it "mails the correct user" do + expect(mail.to).to include(user['email']) + end + + it 'comes from no-reply@zooniverse.org' do + expect(mail.from).to include('no-reply@zooniverse.org') + end + + it 'has the correct subject' do + expect(mail.subject).to eq("Your subjects are almost retired") + end + + it 'has the project name in the body' do + expect(mail.body.encoded).to match("#{context.module_name}") + end + + it 'has the workflow name in the body' do + expect(mail.body.encoded).to match("#{context.extractor_name}") + end + + it 'has the completion percentage in the body' do + expect(mail.body.encoded).to match("#{completion_percentage}%") + end + end +end diff --git a/spec/services/prediction_results/process_spec.rb b/spec/services/prediction_results/process_spec.rb index 1789bad..a65a2e3 100755 --- a/spec/services/prediction_results/process_spec.rb +++ b/spec/services/prediction_results/process_spec.rb @@ -93,6 +93,18 @@ expect(process_results_service.under_threshold_subject_ids).to be_empty expect(process_results_service.random_spice_subject_ids).to match_array([under_threshold_subject_id]) end + + context 'when completion hits the notification threshold' do + before do + stub_const("PredictionResults::Process::COMPLETION_NOTIFICATION_THRESHOLD", 0.5) + allow(NotifyProjectOwnerJob).to receive(:perform_async) + end + + it 'calls NotifyProjectOwnerJob for almost retired subjects' do + process_results_service.partition_results + expect(NotifyProjectOwnerJob).to have_received(:perform_async).with(active_subject_set_id, 0.5) + end + end end describe '#move_over_threshold_subjects_to_active_set' do diff --git a/spec/sidekiq/notify_project_owner_job_spec.rb b/spec/sidekiq/notify_project_owner_job_spec.rb new file mode 100755 index 0000000..1033556 --- /dev/null +++ b/spec/sidekiq/notify_project_owner_job_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe NotifyProjectOwnerJob, type: :job do + describe 'perform' do + let(:context) { Context.first } + let(:job) { described_class.new } + let(:panoptes_client_double) { instance_double(Panoptes::Client) } + let(:fake_owner_id) { 1 } + let(:fake_user_hash) { { "id" => fake_owner_id, "email" => "foo@example.com", "display_name" => "Foo Bar" } } + let(:fake_project_hash) { { "links" => { "owner" => { "id" => fake_owner_id } } } } + let(:completion_rate) { 0.9 } + + let(:mailer_double) { instance_double("ActionMailer::MessageDelivery", deliver_now: true) } + before do + allow(ENV).to receive(:fetch).with('PANOPTES_OAUTH_CLIENT_ID').and_return('fake-client-id') + allow(ENV).to receive(:fetch).with('PANOPTES_OAUTH_CLIENT_SECRET').and_return('fake-client-sekreto') + allow(panoptes_client_double).to receive(:project).and_return(fake_project_hash) + allow(panoptes_client_double).to receive(:user).and_return(fake_user_hash) + allow(Panoptes::Client).to receive(:new).and_return(panoptes_client_double) + allow(ProjectNotificationMailer) + .to receive(:notify_subject_completion) + .and_return(mailer_double) + job.perform(context.active_subject_set_id, completion_rate) + end + + it 'calls the api client to fetch project details' do + expect(panoptes_client_double).to have_received(:project).with(context.project_id) + end + + it 'calls the api client to fetch user details' do + expect(panoptes_client_double).to have_received(:user).with(fake_owner_id) + end + + it 'calls ProjectNotificationMailer notify_subject_completion' do + allow(ProjectNotificationMailer).to receive(:notify_subject_completion) + expect(ProjectNotificationMailer).to have_received(:notify_subject_completion).with(fake_user_hash, context, (completion_rate * 100)) + end + + it 'attempts to deliver the mail' do + expect(mailer_double).to have_received(:deliver_now) + end + end +end From b102c451182eb84bcac1d218ad62a1b59c0f8418 Mon Sep 17 00:00:00 2001 From: tooyosi Date: Mon, 9 Jun 2025 16:29:38 +0100 Subject: [PATCH 3/5] add logic to notify project owner on model change --- app/mailers/project_notification_mailer.rb | 8 +++++++ app/services/prediction_results/process.rb | 21 ++++++++++++++--- app/sidekiq/notify_project_owner_job.rb | 23 +++++++++++++++---- .../notify_prediction_change.html.erb | 6 +++++ ...25_add_last_completion_rate_to_contexts.rb | 5 ++++ db/schema.rb | 3 ++- 6 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 app/views/project_notification_mailer/notify_prediction_change.html.erb create mode 100644 db/migrate/20250609121625_add_last_completion_rate_to_contexts.rb diff --git a/app/mailers/project_notification_mailer.rb b/app/mailers/project_notification_mailer.rb index 4e209dc..4f163ea 100644 --- a/app/mailers/project_notification_mailer.rb +++ b/app/mailers/project_notification_mailer.rb @@ -8,4 +8,12 @@ def notify_subject_completion(user, context, completion_percentage) @email_to = user['email'] mail(to: @email_to, subject: "Your subjects are almost retired") end + + def notify_prediction_change(user, context, change_difference) + @change_difference = change_difference + @project_name = context.module_name + @workflow_name = context.extractor_name + @email_to = user['email'] + mail(to: @email_to, subject: "Significant change in Predictions") + end end diff --git a/app/services/prediction_results/process.rb b/app/services/prediction_results/process.rb index 4ea8fb0..71fe010 100755 --- a/app/services/prediction_results/process.rb +++ b/app/services/prediction_results/process.rb @@ -6,6 +6,7 @@ module PredictionResults class Process SUBJECT_ACTION_API_BATCH_SIZE = ENV.fetch('SUBJECT_ACTION_API_BATCH_SIZE', '10').to_i COMPLETION_NOTIFICATION_THRESHOLD = ENV.fetch('COMPLETION_NOTIFICATION_THRESHOLD', '0.95').to_f + PREDICTION_CHANGE_THRESHOLD = ENV.fetch('COMPLETION_NOTIFICATION_THRESHOLD', '0.4').to_f attr_accessor :results_url, :subject_set_id, :probability_threshold, :over_threshold_subject_ids, :under_threshold_subject_ids, @@ -54,6 +55,7 @@ def partition_results @under_threshold_subject_ids << subject_id if probability < probability_threshold end check_completion_and_notify + check_prediction_change_and_notify # now add some 'spice' to the results by adding some random under threshold subject ids # but don't skew the prediction results by adding too many under threshold images # ensure we only use apply the randomisation factor to the count of over threshold subject ids @@ -90,12 +92,25 @@ def api_batch_bulk_job_args(subject_ids) end private - def check_completion_and_notify + def completion_rate total_under_threshold_subjects = @under_threshold_subject_ids.count - completion_rate = (total_under_threshold_subjects.to_f / @total_subjects.to_f) + @completion_rate ||= (total_under_threshold_subjects.to_f / @total_subjects.to_f) + end + + def check_completion_and_notify if completion_rate >= COMPLETION_NOTIFICATION_THRESHOLD - NotifyProjectOwnerJob.perform_async(subject_set_id, completion_rate) + NotifyProjectOwnerJob.perform_async(subject_set_id, @completion_rate) end end + + def check_prediction_change_and_notify + context = Context.find_by!(active_subject_set_id: subject_set_id) + difference = (@completion_rate - context.last_completion_rate.to_f).abs + if (difference > PREDICTION_CHANGE_THRESHOLD) && context.last_completion_rate != 0 + NotifyProjectOwnerJob.perform_async(subject_set_id, difference, 'model_result_change') + end + + context.update(last_completion_rate: @completion_rate) + end end end diff --git a/app/sidekiq/notify_project_owner_job.rb b/app/sidekiq/notify_project_owner_job.rb index d0362a3..e3ff013 100644 --- a/app/sidekiq/notify_project_owner_job.rb +++ b/app/sidekiq/notify_project_owner_job.rb @@ -5,18 +5,31 @@ class NotifyProjectOwnerJob include Sidekiq::Job sidekiq_options retry: 5 - def perform(subject_set_id, completion_rate) + def perform(subject_set_id, completion_rate, action='subject_completion') @context = Context.find_by!(active_subject_set_id: subject_set_id) + @completion_rate = completion_rate + handle_notify(action) + end + private + def handle_notify(action) owner_link = fetch_project_owner owner_user = fetch_owner_user(owner_link['id']) - ProjectNotificationMailer - .notify_subject_completion(owner_user, @context, (completion_rate * 100)) - .deliver_now + case action + when 'subject_completion' + ProjectNotificationMailer + .notify_subject_completion(owner_user, @context, (@completion_rate * 100)) + .deliver_now + when 'model_result_change' + ProjectNotificationMailer + .notify_prediction_change(owner_user, @context, (@completion_rate * 100)) + .deliver_now + else + raise StandardError.new('No NotifyProjectOwnerJob action specified') + end end - private def fetch_project_owner(max_retries = 3) with_api_retry(max_retries) do resp = Panoptes::Api.client.project(@context.project_id) diff --git a/app/views/project_notification_mailer/notify_prediction_change.html.erb b/app/views/project_notification_mailer/notify_prediction_change.html.erb new file mode 100644 index 0000000..f7d6e86 --- /dev/null +++ b/app/views/project_notification_mailer/notify_prediction_change.html.erb @@ -0,0 +1,6 @@ + +

Prediction run for <%= @workflow_name %> workflow of project: <%= @project_name %> have a significant change of <%= @change_difference %>% from last week's run

+ +

Thanks,

+ +

The Zooniverse Team

diff --git a/db/migrate/20250609121625_add_last_completion_rate_to_contexts.rb b/db/migrate/20250609121625_add_last_completion_rate_to_contexts.rb new file mode 100644 index 0000000..3ef3948 --- /dev/null +++ b/db/migrate/20250609121625_add_last_completion_rate_to_contexts.rb @@ -0,0 +1,5 @@ +class AddLastCompletionRateToContexts < ActiveRecord::Migration[7.0] + def change + add_column :contexts, :last_completion_rate, :float, default: 0.0 + end +end diff --git a/db/schema.rb b/db/schema.rb index b7d5d01..720ca7c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2025_04_27_195821) do +ActiveRecord::Schema[7.0].define(version: 2025_06_09_121625) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -52,6 +52,7 @@ t.string "module_name" t.string "extractor_name" t.jsonb "metadata" + t.float "last_completion_rate", default: 0.0 t.index ["workflow_id", "project_id"], name: "index_contexts_on_workflow_id_and_project_id", unique: true end From 92e1caa9697550debe41fcd80244d88920fe02d1 Mon Sep 17 00:00:00 2001 From: tooyosi Date: Tue, 8 Jul 2025 21:21:38 +0100 Subject: [PATCH 4/5] add tests for model_result_change logic --- spec/fixtures/contexts.yml | 4 ++ .../project_notification_mailer_spec.rb | 30 +++++++++- .../prediction_results/process_spec.rb | 34 +++++++++-- spec/sidekiq/notify_project_owner_job_spec.rb | 58 ++++++++++++++----- 4 files changed, 103 insertions(+), 23 deletions(-) diff --git a/spec/fixtures/contexts.yml b/spec/fixtures/contexts.yml index 8b74e77..719b5dd 100644 --- a/spec/fixtures/contexts.yml +++ b/spec/fixtures/contexts.yml @@ -7,6 +7,7 @@ galaxy_zoo_cosmic_active_learning_project: module_name: 'galaxy_zoo' extractor_name: 'cosmic_dawn' metadata: None + last_completion_rate: 0.1 galaxy_zoo_euclid_active_learning_project: id: 2 @@ -17,6 +18,7 @@ galaxy_zoo_euclid_active_learning_project: module_name: 'galaxy_zoo' extractor_name: 'euclid' metadata: None + last_completion_rate: 0.1 galaxy_zoo_cosmos_active_learning_project: id: 3 @@ -26,7 +28,9 @@ galaxy_zoo_cosmos_active_learning_project: pool_subject_set_id: 67 module_name: 'galaxy_zoo' extractor_name: 'jwst_cosmos' + last_completion_rate: 0.1 metadata: { + 'n_blocks': 2, 'fixed_crop':{ 'lower_left_x': 30, 'lower_left_y': 30, diff --git a/spec/mailers/project_notification_mailer_spec.rb b/spec/mailers/project_notification_mailer_spec.rb index 0f0719b..7cfc4b0 100644 --- a/spec/mailers/project_notification_mailer_spec.rb +++ b/spec/mailers/project_notification_mailer_spec.rb @@ -4,10 +4,9 @@ let(:user) { { 'email' => 'test@project@owner.com' } } let(:context) { Context.first } let(:completion_percentage) { 96 } - let(:mail) { ProjectNotificationMailer.notify_subject_completion(user, context, completion_percentage)} describe "#notify_subject_completion" do - + let(:mail) { ProjectNotificationMailer.notify_subject_completion(user, context, completion_percentage)} it "mails the correct user" do expect(mail.to).to include(user['email']) end @@ -32,4 +31,31 @@ expect(mail.body.encoded).to match("#{completion_percentage}%") end end + + describe "#notify_prediction_change" do + let(:mail) { ProjectNotificationMailer.notify_prediction_change(user, context, completion_percentage)} + it "mails the correct user" do + expect(mail.to).to include(user['email']) + end + + it 'comes from no-reply@zooniverse.org' do + expect(mail.from).to include('no-reply@zooniverse.org') + end + + it 'has the correct subject' do + expect(mail.subject).to eq("Significant change in Predictions") + end + + it 'has the project name in the body' do + expect(mail.body.encoded).to match("#{context.module_name}") + end + + it 'has the workflow name in the body' do + expect(mail.body.encoded).to match("#{context.extractor_name}") + end + + it 'has the completion percentage in the body' do + expect(mail.body.encoded).to match("#{completion_percentage}%") + end +end end diff --git a/spec/services/prediction_results/process_spec.rb b/spec/services/prediction_results/process_spec.rb index a65a2e3..ed34104 100755 --- a/spec/services/prediction_results/process_spec.rb +++ b/spec/services/prediction_results/process_spec.rb @@ -4,13 +4,16 @@ require 'remote_file/reader' RSpec.describe PredictionResults::Process do + fixtures :contexts + let(:confidence_threshold) { 0.8 } let(:remote_file) do # build a fake file we double as a result of the downloader Tempfile.new('remote-file-test') end let(:results_url) { 'https://fake.com/results.json' } - let(:active_subject_set_id) { 1 } + let(:context){ contexts(:galaxy_zoo_cosmic_active_learning_project) } + let(:active_subject_set_id) { context.active_subject_set_id } let(:process_results_service) { described_class.new(results_url: results_url, subject_set_id: active_subject_set_id) } let(:over_threshold_subject_id) { 1 } let(:under_threshold_subject_id) { 2 } @@ -94,15 +97,34 @@ expect(process_results_service.random_spice_subject_ids).to match_array([under_threshold_subject_id]) end - context 'when completion hits the notification threshold' do + context 'Notification triggers' do before do - stub_const("PredictionResults::Process::COMPLETION_NOTIFICATION_THRESHOLD", 0.5) allow(NotifyProjectOwnerJob).to receive(:perform_async) end - it 'calls NotifyProjectOwnerJob for almost retired subjects' do - process_results_service.partition_results - expect(NotifyProjectOwnerJob).to have_received(:perform_async).with(active_subject_set_id, 0.5) + context 'when completion hits the notification threshold' do + before do + stub_const("PredictionResults::Process::COMPLETION_NOTIFICATION_THRESHOLD", 0.5) + end + + it 'calls NotifyProjectOwnerJob for almost retired subjects' do + process_results_service.partition_results + expect(NotifyProjectOwnerJob).to have_received(:perform_async).with(active_subject_set_id, 0.5) + end + end + + context 'when model changes significantly' do + before do + stub_const("PredictionResults::Process::PREDICTION_CHANGE_THRESHOLD", 0.1) + end + + it 'calls NotifyProjectOwnerJob for model changes' do + process_results_service.partition_results + + completion_rate = (process_results_service.under_threshold_subject_ids.count.to_f / process_results_service.prediction_data.size.to_f).to_f + difference = (completion_rate - context.last_completion_rate.to_f).abs + expect(NotifyProjectOwnerJob).to have_received(:perform_async).with(active_subject_set_id, difference, 'model_result_change') + end end end end diff --git a/spec/sidekiq/notify_project_owner_job_spec.rb b/spec/sidekiq/notify_project_owner_job_spec.rb index 1033556..a5505eb 100755 --- a/spec/sidekiq/notify_project_owner_job_spec.rb +++ b/spec/sidekiq/notify_project_owner_job_spec.rb @@ -19,27 +19,55 @@ allow(panoptes_client_double).to receive(:project).and_return(fake_project_hash) allow(panoptes_client_double).to receive(:user).and_return(fake_user_hash) allow(Panoptes::Client).to receive(:new).and_return(panoptes_client_double) - allow(ProjectNotificationMailer) - .to receive(:notify_subject_completion) - .and_return(mailer_double) - job.perform(context.active_subject_set_id, completion_rate) end - it 'calls the api client to fetch project details' do - expect(panoptes_client_double).to have_received(:project).with(context.project_id) - end + context 'subject_completion' do + before do + allow(ProjectNotificationMailer) + .to receive(:notify_subject_completion) + .and_return(mailer_double) + job.perform(context.active_subject_set_id, completion_rate) + end + it 'calls the api client to fetch project details' do + expect(panoptes_client_double).to have_received(:project).with(context.project_id) + end - it 'calls the api client to fetch user details' do - expect(panoptes_client_double).to have_received(:user).with(fake_owner_id) - end + it 'calls the api client to fetch user details' do + expect(panoptes_client_double).to have_received(:user).with(fake_owner_id) + end + + it 'calls ProjectNotificationMailer notify_subject_completion' do + expect(ProjectNotificationMailer).to have_received(:notify_subject_completion).with(fake_user_hash, context, (completion_rate * 100)) + end + + it 'attempts to deliver the mail' do + expect(mailer_double).to have_received(:deliver_now) + end - it 'calls ProjectNotificationMailer notify_subject_completion' do - allow(ProjectNotificationMailer).to receive(:notify_subject_completion) - expect(ProjectNotificationMailer).to have_received(:notify_subject_completion).with(fake_user_hash, context, (completion_rate * 100)) end - it 'attempts to deliver the mail' do - expect(mailer_double).to have_received(:deliver_now) + context 'model_result_change' do + before do + allow(ProjectNotificationMailer) + .to receive(:notify_prediction_change) + .and_return(mailer_double) + job.perform(context.active_subject_set_id, completion_rate, 'model_result_change') + end + it 'calls the api client to fetch project details' do + expect(panoptes_client_double).to have_received(:project).with(context.project_id) + end + + it 'calls the api client to fetch user details' do + expect(panoptes_client_double).to have_received(:user).with(fake_owner_id) + end + + it 'calls ProjectNotificationMailer notify_prediction_change' do + expect(ProjectNotificationMailer).to have_received(:notify_prediction_change).with(fake_user_hash, context, (completion_rate * 100)) + end + + it 'attempts to deliver the mail' do + expect(mailer_double).to have_received(:deliver_now) + end end end end From d6235fcd8bd3d0d9b6b1251558d1282d47fe5319 Mon Sep 17 00:00:00 2001 From: tooyosi Date: Tue, 23 Sep 2025 15:48:31 +0100 Subject: [PATCH 5/5] remove duplicate check_completion_and_notify --- app/services/prediction_results/process.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/services/prediction_results/process.rb b/app/services/prediction_results/process.rb index dd90f75..cc43d70 100755 --- a/app/services/prediction_results/process.rb +++ b/app/services/prediction_results/process.rb @@ -113,11 +113,5 @@ def check_prediction_change_and_notify context.update(last_completion_rate: @completion_rate) end - def check_completion_and_notify - total_under_threshold_subjects = @under_threshold_subject_ids.count - if completion_rate >= COMPLETION_NOTIFICATION_THRESHOLD - NotifyProjectOwnerJob.perform_async(subject_set_id, completion_rate) - end - end end end