From 265fcc379c499ebdb331458442224720a2e3a90b Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 30 Oct 2023 15:58:18 +0100 Subject: [PATCH 1/7] Create task that destroys activities --- lib/tasks/remove_activities.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 lib/tasks/remove_activities.rb diff --git a/lib/tasks/remove_activities.rb b/lib/tasks/remove_activities.rb new file mode 100644 index 0000000000..206417eed0 --- /dev/null +++ b/lib/tasks/remove_activities.rb @@ -0,0 +1,23 @@ +task :destroy_removed_activities => :environment do + # permanently remove activities that match the following criteria: + # - status is 'removed' + # - updated_at is more than 1 month ago + # - one of the following is true: + # - draft is true (never published) + # - series_memberships is empty and less then 25 submissions and latest submission is more than 1 month ago + + ContentPage.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| + if activity.draft? || activity.series_memberships.empty? + activity.destroy + end + end + + Exercise.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| + if activity.draft? || + ( activity.series_memberships.empty? && + ( activity.submissions.empty? || + (activity.submissions.count < 25 && activity.submissions.order(:created_at).last.created_at < 1.month.ago))) + activity.destroy + end + end +end From 6e7d130df6d40102749eee2f9f2329051c0f952d Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 6 Nov 2023 15:00:46 +0100 Subject: [PATCH 2/7] Improve documentation --- lib/tasks/remove_activities.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/tasks/remove_activities.rb b/lib/tasks/remove_activities.rb index 206417eed0..02574e4094 100644 --- a/lib/tasks/remove_activities.rb +++ b/lib/tasks/remove_activities.rb @@ -1,10 +1,14 @@ task :destroy_removed_activities => :environment do - # permanently remove activities that match the following criteria: + # permanently remove activities that match all of the following criteria: # - status is 'removed' # - updated_at is more than 1 month ago # - one of the following is true: # - draft is true (never published) # - series_memberships is empty and less then 25 submissions and latest submission is more than 1 month ago + # + # Destroy is called on each activity individually to ensure that callbacks are run + # This means the activity will be removed from any series, evaluations it is a member of + # and any submissions will be removed ContentPage.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| if activity.draft? || activity.series_memberships.empty? From c2f3df6535793753d4e58306d686b8e0ace000ab Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 6 Nov 2023 15:50:47 +0100 Subject: [PATCH 3/7] Make active job with tests --- app/jobs/remove_activities_job.rb | 42 ++++++++++ lib/tasks/remove_activities.rb | 27 ------ test/jobs/remove_activities_job_test.rb | 106 ++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 27 deletions(-) create mode 100644 app/jobs/remove_activities_job.rb delete mode 100644 lib/tasks/remove_activities.rb create mode 100644 test/jobs/remove_activities_job_test.rb diff --git a/app/jobs/remove_activities_job.rb b/app/jobs/remove_activities_job.rb new file mode 100644 index 0000000000..1e95faed35 --- /dev/null +++ b/app/jobs/remove_activities_job.rb @@ -0,0 +1,42 @@ +class RemoveActivitiesJob < ApplicationJob + # permanently remove activities that match all of the following criteria: + # - status is 'removed' + # - updated_at is more than 1 month ago + # - one of the following is true: + # - draft is true (never published) + # - series_memberships is empty and less then 25 submissions and latest submission is more than 1 month ago + # + # Destroy is called on each activity individually to ensure that callbacks are run + # This means the activity will be removed from any series, evaluations it is a member of + # and any submissions will be removed + queue_as :cron + + def perform + ContentPage.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| + if activity.draft? || activity.series_memberships.empty? + # destroy series memberships first explicitly, as they are dependent: :restrict_with_error + activity.series_memberships.destroy_all + + activity.destroy + end + end + + Exercise.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| + if activity.draft? || + (activity.series_memberships.empty? && + (activity.submissions.empty? || + (activity.submissions.count < 25 && activity.submissions.reorder(:created_at).last.created_at < 1.month.ago))) + # destroy submissions first explicitly, as they are dependent: :restrict_with_error + activity.submissions.destroy_all + + # destroy series memberships first explicitly, as they are dependent: :restrict_with_error + activity.series_memberships.destroy_all + + activity.destroy + end + end + + # rerun this job in 1 month + RemoveActivitiesJob.set(wait: 1.month).perform_later + end +end diff --git a/lib/tasks/remove_activities.rb b/lib/tasks/remove_activities.rb deleted file mode 100644 index 02574e4094..0000000000 --- a/lib/tasks/remove_activities.rb +++ /dev/null @@ -1,27 +0,0 @@ -task :destroy_removed_activities => :environment do - # permanently remove activities that match all of the following criteria: - # - status is 'removed' - # - updated_at is more than 1 month ago - # - one of the following is true: - # - draft is true (never published) - # - series_memberships is empty and less then 25 submissions and latest submission is more than 1 month ago - # - # Destroy is called on each activity individually to ensure that callbacks are run - # This means the activity will be removed from any series, evaluations it is a member of - # and any submissions will be removed - - ContentPage.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| - if activity.draft? || activity.series_memberships.empty? - activity.destroy - end - end - - Exercise.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| - if activity.draft? || - ( activity.series_memberships.empty? && - ( activity.submissions.empty? || - (activity.submissions.count < 25 && activity.submissions.order(:created_at).last.created_at < 1.month.ago))) - activity.destroy - end - end -end diff --git a/test/jobs/remove_activities_job_test.rb b/test/jobs/remove_activities_job_test.rb new file mode 100644 index 0000000000..6d5b18abeb --- /dev/null +++ b/test/jobs/remove_activities_job_test.rb @@ -0,0 +1,106 @@ +require 'test_helper' + +class RemoveActivitiesJobTest < ActiveJob::TestCase + + test 'should remove "removed" draft activities' do + c = create :content_page + create :activity_read_state, activity: c + c.update status: :removed, draft: true, updated_at: 2.months.ago + e = create :exercise + create :submission, exercise: e + e.update status: :removed, draft: true, updated_at: 2.months.ago + s = create :series + s.activities << c + s.activities << e + + assert_difference 'ContentPage.count', -1 do + assert_difference 'Exercise.count', -1 do + RemoveActivitiesJob.perform_now + end + end + end + + test 'should remove "removed" activities with no series memberships and no submissions' do + create :content_page, status: :removed, draft: false, updated_at: 2.months.ago + create :exercise, status: :removed, draft: false, updated_at: 2.months.ago + + assert_difference 'ContentPage.count', -1 do + assert_difference 'Exercise.count', -1 do + RemoveActivitiesJob.perform_now + end + end + end + + test 'should not remove "removed" activities with series memberships' do + c = create :content_page, status: :removed, draft: false, updated_at: 2.months.ago + e = create :exercise, status: :removed, draft: false, updated_at: 2.months.ago + s = create :series + s.activities << c + s.activities << e + + assert_no_difference 'ContentPage.count' do + assert_no_difference 'Exercise.count' do + RemoveActivitiesJob.perform_now + end + end + end + + test 'should not remove "removed" activities with more than 25 submissions' do + e = create :exercise, status: :removed, draft: false, updated_at: 2.months.ago + create_list :submission, 26, exercise: e + + assert_no_difference 'Exercise.count' do + RemoveActivitiesJob.perform_now + end + end + + test 'should remove "removed" activities with less than 25 submissions and last submission more than 1 month ago' do + e = create :exercise, status: :removed, draft: false, updated_at: 2.months.ago + create_list :submission, 21, exercise: e, created_at: 2.months.ago + + assert_difference 'Submission.count', -21 do + assert_difference 'Exercise.count', -1 do + RemoveActivitiesJob.perform_now + end + end + end + + test 'should not remove "removed" activities with less than 25 submissions and last submission less than 1 month ago' do + e = create :exercise, status: :removed, draft: false, updated_at: 2.months.ago + create_list :submission, 5, exercise: e, created_at: 2.months.ago + create_list :submission, 3, exercise: e, created_at: 2.weeks.ago + + assert_no_difference 'Submission.count' do + assert_no_difference 'Exercise.count' do + RemoveActivitiesJob.perform_now + end + end + end + + test 'should not removed non removed activities' do + create :exercise, updated_at: 2.months.ago + create :content_page, updated_at: 2.months.ago + create :exercise, draft: true, updated_at: 2.months.ago + create :content_page, draft: true, updated_at: 2.months.ago + + assert_no_difference 'Exercise.count' do + assert_no_difference 'ContentPage.count' do + RemoveActivitiesJob.perform_now + end + end + end + + test 'should not remove activities updated less than 1 month ago' do + create :exercise, status: :removed, draft: true, updated_at: 2.weeks.ago + + assert_no_difference 'Exercise.count' do + RemoveActivitiesJob.perform_now + end + end + + test 'should reschedule itself' do + assert_enqueued_with(job: RemoveActivitiesJob) do + RemoveActivitiesJob.perform_now + end + end +end From e09cc1ce835023644bce342f0133d6c1ec3374f3 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 6 Nov 2023 15:53:38 +0100 Subject: [PATCH 4/7] Fix linting --- test/jobs/remove_activities_job_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/jobs/remove_activities_job_test.rb b/test/jobs/remove_activities_job_test.rb index 6d5b18abeb..9f283fbf58 100644 --- a/test/jobs/remove_activities_job_test.rb +++ b/test/jobs/remove_activities_job_test.rb @@ -1,7 +1,6 @@ require 'test_helper' class RemoveActivitiesJobTest < ActiveJob::TestCase - test 'should remove "removed" draft activities' do c = create :content_page create :activity_read_state, activity: c From 9e43f721981d6285b2d679b9ce10fb35a7f22c1c Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 6 Nov 2023 16:01:43 +0100 Subject: [PATCH 5/7] Give cron a queue --- config/deploy/production.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/deploy/production.rb b/config/deploy/production.rb index c3bd9aed59..30eee15c6e 100644 --- a/config/deploy/production.rb +++ b/config/deploy/production.rb @@ -13,7 +13,7 @@ set :delayed_job_pools_per_server, 'dodona' => { 'default,statistics,exports,cleaning' => 2, - 'git' => 1, + 'git,cron' => 1, }, 'sisyphus' => { 'submissions,low_priority_submissions,high_priority_submissions' => 6 From e7527aa3957ed88e68e3e6f4c222645a8dbbc076 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 7 Nov 2023 16:43:22 +0100 Subject: [PATCH 6/7] Use cleaning queue --- app/jobs/remove_activities_job.rb | 2 +- config/deploy/production.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/jobs/remove_activities_job.rb b/app/jobs/remove_activities_job.rb index 1e95faed35..7353fbe33f 100644 --- a/app/jobs/remove_activities_job.rb +++ b/app/jobs/remove_activities_job.rb @@ -9,7 +9,7 @@ class RemoveActivitiesJob < ApplicationJob # Destroy is called on each activity individually to ensure that callbacks are run # This means the activity will be removed from any series, evaluations it is a member of # and any submissions will be removed - queue_as :cron + queue_as :cleaning def perform ContentPage.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| diff --git a/config/deploy/production.rb b/config/deploy/production.rb index 30eee15c6e..c3bd9aed59 100644 --- a/config/deploy/production.rb +++ b/config/deploy/production.rb @@ -13,7 +13,7 @@ set :delayed_job_pools_per_server, 'dodona' => { 'default,statistics,exports,cleaning' => 2, - 'git,cron' => 1, + 'git' => 1, }, 'sisyphus' => { 'submissions,low_priority_submissions,high_priority_submissions' => 6 From 4641be3727dec5cb24cd1fab6f582b8d231931b0 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 7 Nov 2023 16:53:56 +0100 Subject: [PATCH 7/7] Simplify if --- app/jobs/remove_activities_job.rb | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/app/jobs/remove_activities_job.rb b/app/jobs/remove_activities_job.rb index 7353fbe33f..14f7a8c612 100644 --- a/app/jobs/remove_activities_job.rb +++ b/app/jobs/remove_activities_job.rb @@ -22,18 +22,19 @@ def perform end Exercise.where(status: 'removed').where('updated_at < ?', 1.month.ago).find_each do |activity| - if activity.draft? || - (activity.series_memberships.empty? && - (activity.submissions.empty? || - (activity.submissions.count < 25 && activity.submissions.reorder(:created_at).last.created_at < 1.month.ago))) - # destroy submissions first explicitly, as they are dependent: :restrict_with_error - activity.submissions.destroy_all + unless activity.draft? + next if activity.series_memberships.present? + next if activity.submissions.count >= 25 + next if activity.submissions.present? && activity.submissions.reorder(:created_at).last.created_at > 1.month.ago + end - # destroy series memberships first explicitly, as they are dependent: :restrict_with_error - activity.series_memberships.destroy_all + # destroy submissions first explicitly, as they are dependent: :restrict_with_error + activity.submissions.destroy_all - activity.destroy - end + # destroy series memberships first explicitly, as they are dependent: :restrict_with_error + activity.series_memberships.destroy_all + + activity.destroy end # rerun this job in 1 month