Skip to content

Commit

Permalink
Write back to file lines that have been processed
Browse files Browse the repository at this point in the history
This might have some performance implications as it reads the entire file into memory and writes at the same time... We will see how it performs as the text files should not be too intense, but if it performs badly in integration we should switch it out for a different way, maybe writing to a temporary file or something.
  • Loading branch information
minhngocd authored and lauraghiorghisor-tw committed Nov 29, 2024
1 parent 70dbbd8 commit ab776ef
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 48 deletions.
89 changes: 51 additions & 38 deletions lib/tasks/assets.rake
Original file line number Diff line number Diff line change
Expand Up @@ -55,45 +55,58 @@ namespace :assets do
desc "Delete and update replacement draft"
task :delete_and_update_replacement_draft, %i[csv_path] => :environment do |_t, args|
csv_path = args.fetch(:csv_path)
CSV.foreach(csv_path, headers: false) do |row|
original_asset_id = row[0]
original_asset = Asset.where(_id: original_asset_id)&.first
replacement_asset = original_asset&.replacement

if original_asset.nil?
puts "Asset ID: #{original_asset_id} - SKIPPED. No asset found."
next
File.open(csv_path, "r+") do |file|
lines = file.readlines
file.rewind

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

original_asset_id = row[0]
original_asset = Asset.where(_id: original_asset_id)&.first
replacement_asset = original_asset&.replacement

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

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

unless replacement_asset.draft?
puts "Asset ID: #{original_asset_id} - SKIPPED. Replacement #{replacement_asset.id} is not in draft."
next
end

unless replacement_asset.replacement.nil?
puts "Asset ID: #{original_asset_id} - SKIPPED. Replacement #{replacement_asset.id} is further replaced."
next
end

message = "Asset ID: #{original_asset_id} - OK."

if replacement_asset.deleted?
message += " Replacement #{replacement_asset.id} is already deleted."
else
replacement_asset.destroy!
message += " Replacement #{replacement_asset.id} has been deleted."
end

replacement_asset.draft = false
replacement_asset.save!
message += " Replacement draft updated to false."

puts message

file.puts "#{original_asset_id},DONE\n"
end

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

unless replacement_asset.draft?
puts "Asset ID: #{original_asset_id} - SKIPPED. Replacement #{replacement_asset.id} is not in draft."
next
end

unless replacement_asset.replacement.nil?
puts "Asset ID: #{original_asset_id} - SKIPPED. Replacement #{replacement_asset.id} is further replaced."
next
end

message = "Asset ID: #{original_asset_id} - OK."

if replacement_asset.deleted?
message += " Replacement #{replacement_asset.id} is already deleted."
else
replacement_asset.destroy!
message += " Replacement #{replacement_asset.id} has been deleted."
end

replacement_asset.draft = false
replacement_asset.save!
message += " Replacement draft updated to false."

puts message
end
end

Expand Down
43 changes: 33 additions & 10 deletions spec/lib/tasks/assets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@
describe "assets:bulk_fix:delete_and_update_replacement_draft" do
let(:task) { Rake::Task["assets:bulk_fix:delete_and_update_replacement_draft"] }

# TODO: add batch parameter tests
# it "should start from where last left off if kicked" do
# end

it "skips the asset and prints exception if asset is not found" do
expected_output = <<~OUTPUT
Asset ID: #{asset_id} - SKIPPED. No asset found.
Expand Down Expand Up @@ -113,17 +109,44 @@
expect(replacement.reload.deleted_at).not_to be_nil
expect(replacement.reload.draft).to be false
end

it "should mark a line as 'done' in the CSV if line is processed" do

Check failure on line 113 in spec/lib/tasks/assets_spec.rb

View workflow job for this annotation

GitHub Actions / Lint Ruby / Run RuboCop

RSpec/ExampleWording: Do not use should when describing your tests. (https://rspec.rubystyle.guide/#should-in-example-docstrings, https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExampleWording)
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil, replacement_id: nil)
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)
task.invoke(filepath)
expect(File.read(filepath)).to eq "6592008029c8c3e4dc76256c,DONE\n"
end

it "should skip processing a line if it is marked as DONE" do

Check failure on line 120 in spec/lib/tasks/assets_spec.rb

View workflow job for this annotation

GitHub Actions / Lint Ruby / Run RuboCop

RSpec/ExampleWording: Do not use should when describing your tests. (https://rspec.rubystyle.guide/#should-in-example-docstrings, https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExampleWording)
file.open
csv_file = <<~CSV
6592008029c8c3e4dc76256c,DONE
6592008029c8c3e4dc76256d
CSV
file.write(csv_file)
file.close

skipped_replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil, replacement_id: nil)
FactoryBot.create(:asset, id: "6592008029c8c3e4dc76256c", replacement_id: skipped_replacement.id)
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil, replacement_id: nil)
FactoryBot.create(:asset, id: "6592008029c8c3e4dc76256d", replacement_id: replacement.id)

expected_output = <<~OUTPUT
Asset ID: 6592008029c8c3e4dc76256d - OK. Replacement #{replacement.id} has been deleted. Replacement draft updated to false.
OUTPUT
expect { task.invoke(filepath) }.to output(expected_output).to_stdout
expect(skipped_replacement.reload.deleted_at).to be_nil
expect(skipped_replacement.reload.draft).to be true
expect(replacement.reload.deleted_at).not_to be_nil
expect(replacement.reload.draft).to be false
expect(File.read(filepath)).to eq "6592008029c8c3e4dc76256c,DONE\n6592008029c8c3e4dc76256d,DONE\n"
end
end

# 30k assets in this category - we're updating the assets themselves (these are the ones that have AD not deleted and not replaced in WH)
# TODO: can we check if the asset is live by curling the URL?
describe "assets:bulk_fix:delete_and_update_draft" do
let(:task) { Rake::Task["assets:bulk_fix:delete_and_update_draft"] }

# TODO: add batch parameter tests
# it "should start from where last left off if kicked" do
# end

it "skips the asset if asset is not found" do
expected_output = <<~OUTPUT
Asset ID: #{asset_id} - SKIPPED. No asset found.
Expand Down Expand Up @@ -172,7 +195,7 @@
end
end

# TODO: This category has 3.5k assets, for which we'd need to extract the corresponding replacement value (i.e. the end of the line AD's corresponding asset variant's ID).
# 3.5k assets, for which we need to set the corresponding replacement value
describe "assets:bulk_fix:update_replacement" do
let(:task) { Rake::Task["assets:bulk_fix:update_replacement"] }
let(:replacement_id) { BSON::ObjectId("6592008029c8c3e4dc76256d") }
Expand Down

0 comments on commit ab776ef

Please sign in to comment.