Skip to content

Commit

Permalink
Add rake task to fix assets that are not deleted in WH
Browse files Browse the repository at this point in the history
On the back of an investigation into asset deletion states, we've
produced a set of assets that are not deleted in wh, but should not be live.
In order to preserve some data integrity, we're choosing to fix the data,
rather than do a mass deletion.

We've identified some flows where draft replacements should be deleted.
Nonetheless, the original asset does not redirect to a replacement if
the replacement is in draft, so we're also switching the draft state to false.
The order is important, as the replacements themselves would become public
if we don't action the deletion first. We're checking whether each asset
that we feed through the rake task is itself or has a replacement in this state.

For assets that are not draft replacements nor have draft replacements,
we want to allow them to continue redirecting, being replaced etc.

A final catch-all step ensures remaining assets (for which the steps above
don't fire) are deleted.

We've also identified broken flows that cause missing replacement links,
but those will be fixed by running some updater logic from a wh rake task.
  • Loading branch information
lauraghiorghisor-tw committed Dec 4, 2024
1 parent 6636d85 commit 2302ab2
Show file tree
Hide file tree
Showing 2 changed files with 280 additions and 0 deletions.
76 changes: 76 additions & 0 deletions lib/tasks/assets.rake
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,80 @@ namespace :assets do
end
end
end

namespace :bulk_fix do
desc "Fix assets and draft replacements"
task :fix_assets_and_draft_replacements, %i[csv_path] => :environment do |_t, args|
csv_path = args.fetch(:csv_path)
process_file_in_memory(csv_path) do |row|
original_asset_id = row[0]
original_asset = Asset.where(_id: original_asset_id)&.first
is_replacement = Asset.where(replacement_id: original_asset_id).any?

if original_asset.nil?
puts "Asset ID: #{original_asset_id} - SKIPPED. No asset found."
next
end

replacement_asset = original_asset.replacement

if replacement_asset && replacement_asset.replacement.nil? && replacement_asset.draft?
begin
delete_and_update_draft(replacement_asset)
puts "Asset ID: #{original_asset_id} - OK. Draft replacement #{replacement_asset.id} deleted and updated to false."
rescue StandardError
puts "Asset ID: #{original_asset_id} - ERROR. Asset replacement failed to save. Error: #{replacement_asset.errors.full_messages}."
end
next
end

if is_replacement && replacement_asset.nil? && original_asset.draft?
begin
delete_and_update_draft(original_asset)
puts "Asset ID: #{original_asset_id} - is a replacement. Asset deleted and updated to false."
rescue StandardError
puts "Asset ID: #{original_asset_id} - ERROR. Asset failed to save. Error: #{original_asset.errors.full_messages}."
end
next
end

if original_asset.draft? || original_asset.deleted? || replacement_asset || original_asset.redirect_url
puts "Asset ID: #{original_asset_id} - SKIPPED. Asset is draft (#{original_asset.draft?}), deleted (#{original_asset.deleted?}), replaced (#{!replacement_asset.nil?}), or redirected (#{!original_asset.redirect_url.nil?})."
next
end

begin
original_asset.destroy!
puts "Asset ID: #{original_asset_id} - OK. Asset has been deleted."
rescue StandardError
puts "Asset ID: #{original_asset_id} - ERROR. Asset failed to save. Error: #{original_asset.errors.full_messages}."
end
end
end
end
end

def delete_and_update_draft(asset)
asset.destroy! unless asset.deleted?
asset.draft = false
asset.save!
end

def process_file_in_memory(filepath)
File.open(filepath, "r+") do |file|
lines = file.readlines
file.rewind

lines.each do |line|
row = line.split(",").map(&:strip)
if row.last == "DONE"
file.puts line
next
end

yield row

file.puts "#{line.strip},DONE\n"
end
end
end
204 changes: 204 additions & 0 deletions spec/lib/tasks/assets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,208 @@
expect { task.invoke("foo") }.to raise_error(Mongoid::Errors::DocumentNotFound)
end
end

# rubocop:disable RSpec/AnyInstance
context "when running a bulk fix" do
let(:asset_id) { BSON::ObjectId("6592008029c8c3e4dc76256c") }
let(:file) { Tempfile.new("csv_file") }
let(:filepath) { file.path }

before do
task.reenable # without this, calling `invoke` does nothing after first test
csv_file = <<~CSV
6592008029c8c3e4dc76256c
CSV
file.write(csv_file)
file.close
end

describe "assets:bulk_fix:fix_assets_and_draft_replacements" do
let(:task) { Rake::Task["assets:bulk_fix:fix_assets_and_draft_replacements"] }

it "skips the asset if asset is not found" do
expected_output = <<~OUTPUT
Asset ID: #{asset_id} - SKIPPED. No asset found.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
end

it "only updates the draft state of the replacement if the asset replacement is already deleted" do
replacement = FactoryBot.create(:asset, draft: true, deleted_at: Time.zone.now, replacement_id: nil)
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - OK. Draft replacement #{replacement.id} deleted and updated to false.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(replacement.reload.draft).to be false
end

it "deletes the asset replacement and updates the draft state if the replacement is not deleted" do
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil, replacement_id: nil)
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - OK. Draft replacement #{replacement.id} deleted and updated to false.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(replacement.reload.deleted_at).not_to be_nil
expect(replacement.reload.draft).to be false
end

it "only updates the draft state if the asset is already deleted (asset is a replacement)" do
asset = FactoryBot.create(:asset, id: asset_id, draft: true, deleted_at: Time.zone.now)
FactoryBot.create(:asset, replacement_id: asset.id)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - is a replacement. Asset deleted and updated to false.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(asset.reload.deleted_at).not_to be_nil
expect(asset.reload.draft).to be false
end

it "deletes the asset and updates the draft state if the asset is not deleted (asset is a replacement)" do
asset = FactoryBot.create(:asset, id: asset_id, draft: true, deleted_at: nil)
FactoryBot.create(:asset, replacement_id: asset.id)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - is a replacement. Asset deleted and updated to false.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(asset.reload.deleted_at).not_to be_nil
expect(asset.reload.draft).to be false
end

it "skips the asset if asset is itself redirected (and not replaced by draft or itself a replacement)" do
FactoryBot.create(:asset, id: asset_id, draft: false, redirect_url: "https://example.com")

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - SKIPPED. Asset is draft (false), deleted (false), replaced (false), or redirected (true).
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
end

it "skips the asset, if the asset is further replaced (and not replaced by draft or itself a replacement)" do
replacement = FactoryBot.create(:asset, draft: false)
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - SKIPPED. Asset is draft (false), deleted (false), replaced (true), or redirected (false).
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
end

it "skips the asset if asset is itself in draft (and not replaced by draft or itself a replacement)" do
FactoryBot.create(:asset, id: asset_id, draft: true)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - SKIPPED. Asset is draft (true), deleted (false), replaced (false), or redirected (false).
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
end

it "skips the asset, if the asset is already deleted (and is not itself a replacement)" do
FactoryBot.create(:asset, id: asset_id, deleted_at: Time.zone.now)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - SKIPPED. Asset is draft (false), deleted (true), replaced (false), or redirected (false).
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
end

it "catch-all case: it deletes the asset" do
FactoryBot.create(:asset, id: asset_id, draft: false, deleted_at: nil, replacement_id: nil, redirect_url: nil)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - OK. Asset has been deleted.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
end

it "rescues and logs if asset fails to save" do
asset = FactoryBot.create(:asset, id: asset_id, draft: true, deleted_at: nil)
FactoryBot.create(:asset, replacement_id: asset.id)

allow_any_instance_of(Asset).to receive(:save!).and_raise(StandardError)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - ERROR. Asset failed to save. Error: #{asset.errors.full_messages}.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(asset.reload.draft).to be true
expect(asset.reload.deleted_at).not_to be_nil
end

it "rescues and logs if asset fails to destroy" do
asset = FactoryBot.create(:asset, id: asset_id, draft: false, deleted_at: nil, replacement_id: nil, redirect_url: nil)

allow_any_instance_of(Asset).to receive(:destroy!).and_raise(StandardError)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - ERROR. Asset failed to save. Error: #{asset.errors.full_messages}.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(asset.reload.deleted_at).to be_nil
end

it "rescues and logs if asset replacement fails to save" do
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil)
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)

allow_any_instance_of(Asset).to receive(:save!).and_raise(StandardError)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - ERROR. Asset replacement failed to save. Error: #{replacement.errors.full_messages}.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(replacement.reload.draft).to be true
expect(replacement.reload.deleted_at).not_to be_nil
end

it "rescues and logs if asset replacement fails to destroy" do
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil)
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)

allow_any_instance_of(Asset).to receive(:destroy!).and_raise(StandardError)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - ERROR. Asset replacement failed to save. Error: #{replacement.errors.full_messages}.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(replacement.reload.draft).to be true
expect(replacement.reload.deleted_at).to be_nil
end

it "marks a line as 'done' in the CSV if line is processed" do
FactoryBot.create(:asset, id: asset_id, draft: true, deleted_at: nil)
task.invoke(filepath)
expect(File.read(filepath)).to eq "6592008029c8c3e4dc76256c,DONE\n"
end

it "skips processing a line if it is marked as DONE" do
file.open
csv_file = <<~CSV
6592008029c8c3e4dc76256c,DONE
6592008029c8c3e4dc76256d
CSV
file.write(csv_file)
file.close

skipped_asset = FactoryBot.create(:asset, id: "6592008029c8c3e4dc76256c", draft: true, deleted_at: nil)
asset = FactoryBot.create(:asset, id: "6592008029c8c3e4dc76256d", draft: false, deleted_at: nil)

expected_output = <<~OUTPUT
Asset ID: 6592008029c8c3e4dc76256d - OK. Asset has been deleted.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(skipped_asset.reload.deleted_at).to be_nil
expect(skipped_asset.reload.draft).to be true
expect(asset.reload.deleted_at).not_to be_nil
expect(asset.reload.draft).to be false
expect(File.read(filepath)).to eq "6592008029c8c3e4dc76256c,DONE\n6592008029c8c3e4dc76256d,DONE\n"
end
end
end
# rubocop:enable RSpec/AnyInstance
end

0 comments on commit 2302ab2

Please sign in to comment.