Skip to content

Commit 7eda9be

Browse files
authored
Merge pull request #1206 from Crown-Commercial-Service/feature/nrmi-178-clean-up-non-active-val-failed-entries
Feature/nrmi 178 clean up non active val failed entries
2 parents 7d6019f + b3832c5 commit 7eda9be

9 files changed

Lines changed: 137 additions & 9 deletions

.rubocop.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ Metrics/AbcSize:
122122
- 'app/controllers/admin/suppliers_controller.rb'
123123
- 'app/controllers/v1/tasks_controller.rb'
124124
- 'app/controllers/admin/users_controller.rb'
125+
- 'app/jobs/cleanup_submission_entries_job.rb'
125126

126127
Layout/LineLength:
127128
Max: 120
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
class CleanupSubmissionEntriesJob < ApplicationJob
2+
ENTRIES_BATCH_SIZE = 1000
3+
4+
# rubocop:disable Metrics/CyclomaticComplexity
5+
# rubocop:disable Metrics/PerceivedComplexity
6+
def perform(dry_run: false, max_run_time: 5.hours, task_batch_sixe: 100)
7+
start_time = Time.current
8+
total_deleted_entries = 0
9+
10+
tasks_with_unprocessed_submissions = Task.joins(:submissions)
11+
.where(submissions: { aasm_state: 'validation_failed',
12+
cleanup_processed: false })
13+
.distinct
14+
15+
tasks_with_unprocessed_submissions.find_in_batches(batch_size: task_batch_sixe) do |tasks_batch|
16+
tasks_batch.each do |task|
17+
active_id = task.active_submission&.id
18+
19+
failed_submissions = task.submissions
20+
.where(aasm_state: 'validation_failed', cleanup_processed: false)
21+
.where.not(id: active_id)
22+
23+
next if failed_submissions.empty?
24+
25+
deleted_for_task = 0
26+
27+
failed_submissions.find_each do |submission|
28+
begin
29+
submission.entries.in_batches(of: ENTRIES_BATCH_SIZE) do |entries_batch|
30+
if dry_run
31+
Rollbar.info("Dry run: would delete #{entries_batch.count} entries for Submission ID #{submission.id}.")
32+
else
33+
deleted_count = entries_batch.delete_all
34+
deleted_for_task += deleted_count
35+
total_deleted_entries += deleted_count
36+
# rubocop:disable Layout/LineLength
37+
Rollbar.info("Task ID #{task.id}: Processed #{failed_submissions.count} failed submissions, deleted #{deleted_for_task} entries.")
38+
# rubocop:enable Layout/LineLength
39+
end
40+
end
41+
42+
submission.update(cleanup_processed: true) unless dry_run
43+
rescue StandardError => e
44+
Rollbar.error(e, "Error processing Submission ID #{submission.id} for Task ID #{task.id}: #{e.message}")
45+
end
46+
47+
break if Time.current - start_time >= max_run_time
48+
end
49+
end
50+
51+
break if Time.current - start_time >= max_run_time
52+
53+
sleep 1 # To avoid overwhelming the database
54+
end
55+
end
56+
# rubocop:enable Metrics/CyclomaticComplexity
57+
# rubocop:enable Metrics/PerceivedComplexity
58+
end

config/application.rb

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ class Application < Rails::Application
2626

2727
# config.active_record.yaml_column_permitted_classes = [Time]
2828

29-
console do
30-
require './lib/console_helpers'
31-
if defined?(Pry)
32-
TOPLEVEL_BINDING.eval('self').extend ConsoleHelpers
33-
else
34-
Rails::ConsoleMethods.include ConsoleHelpers
35-
end
36-
end
29+
# TODO: this is deprecated, figure out alternative
30+
# console do
31+
# require './lib/console_helpers'
32+
# if defined?(Pry)
33+
# TOPLEVEL_BINDING.eval('self').extend ConsoleHelpers
34+
# else
35+
# Rails::ConsoleMethods.include ConsoleHelpers
36+
# end
37+
# end
3738
end
3839
end

config/sidekiq_schedule.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,7 @@ kill_stuck_submissions:
1010
cron: '*/5 * * * *'
1111
class: KillStuckSubmissionsJob
1212
queue: default
13+
cleanup_submission_entries:
14+
cron: '0 1 * * *'
15+
class: CleanupSubmissionEntriesJob
16+
queue: default
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddCleanupProcessedToSubmissions < ActiveRecord::Migration[8.0]
2+
def change
3+
add_column :submissions, :cleanup_processed, :boolean, default: false, null: false
4+
end
5+
end
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddIndexToSubmissionsOnTaskIdAndAasmState < ActiveRecord::Migration[8.0]
2+
def change
3+
add_index :submissions, [:task_id, :aasm_state], name: 'index_submissions_on_task_id_and_aasm_state'
4+
end
5+
end
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddIndexToSubmissionsCleanupProcessed < ActiveRecord::Migration[8.0]
2+
def change
3+
add_index :submissions, :cleanup_processed
4+
end
5+
end

db/schema.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#
1111
# It's strongly recommended that you check this file into your version control system.
1212

13-
ActiveRecord::Schema[8.0].define(version: 2025_09_22_123744) do
13+
ActiveRecord::Schema[8.0].define(version: 2025_10_14_121707) do
1414
# These are extensions that must be enabled in order to support this database
1515
enable_extension "citext"
1616
enable_extension "pg_catalog.plpgsql"
@@ -250,6 +250,7 @@
250250
t.datetime "submitted_at", precision: nil
251251
t.decimal "management_charge_total", precision: 18, scale: 4
252252
t.decimal "invoice_total", precision: 18, scale: 4
253+
t.boolean "cleanup_processed", default: false, null: false
253254
t.index ["aasm_state"], name: "index_submissions_on_aasm_state"
254255
t.index ["created_at"], name: "index_submissions_on_created_at", order: :desc
255256
t.index ["created_by_id"], name: "index_submissions_on_created_by_id"
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
require 'rails_helper'
2+
3+
RSpec.describe CleanupSubmissionEntriesJob, type: :job do
4+
describe '#perform' do
5+
let!(:task) { FactoryBot.create(:task) }
6+
let!(:failed_submission1) { FactoryBot.create(:submission_with_invalid_entries, task: task) }
7+
let!(:failed_submission2) { FactoryBot.create(:submission_with_invalid_entries, task: task) }
8+
let!(:submission_entry) { FactoryBot.create(:submission_entry, submission: failed_submission1) }
9+
10+
context 'when dry_run is true' do
11+
it 'does not delete entries and logs the intended deletions' do
12+
# rubocop:disable Layout/LineLength
13+
expect(Rollbar).to receive(:info).with("Dry run: would delete 1 entries for Submission ID #{failed_submission1.id}.")
14+
# rubocop:enable Layout/LineLength
15+
expect do
16+
described_class.perform_now(dry_run: true)
17+
end.not_to change(SubmissionEntry, :count)
18+
expect(failed_submission1.reload.cleanup_processed).to be_falsey
19+
expect(failed_submission2.reload.cleanup_processed).to be_falsey
20+
end
21+
end
22+
23+
context 'when dry_run is false' do
24+
it 'deletes entries and marks submissions as processed' do
25+
expect(Rollbar).to receive(:info).with("Task ID #{task.id}: Processed 1 failed submissions, deleted 1 entries.")
26+
expect do
27+
described_class.perform_now(dry_run: false)
28+
end.to change(SubmissionEntry, :count).by(-1)
29+
expect(failed_submission1.reload.cleanup_processed).to be_truthy
30+
end
31+
end
32+
33+
context 'when there are no validation failed submissions' do
34+
before do
35+
failed_submission1.update(aasm_state: 'ingest_failed')
36+
failed_submission2.update(aasm_state: 'completed')
37+
end
38+
39+
it 'does not perform any deletions or updates' do
40+
expect(Rollbar).not_to receive(:info)
41+
expect do
42+
described_class.perform_now(dry_run: false)
43+
end.not_to change(SubmissionEntry, :count)
44+
expect(failed_submission1.reload.cleanup_processed).to be_falsey
45+
end
46+
end
47+
end
48+
end

0 commit comments

Comments
 (0)