Skip to content

Commit

Permalink
Fix invalid state and reset parent document url when updating assets
Browse files Browse the repository at this point in the history
- Add a clause to remove parent document url when fixing draft states when deleting superseded assets
Some assets are incorrectly staying in the draft state. But when we try and fix the draft state, it cannot be linking to a draft parent_document_url. We are removing the parent document url link since it is difficult to figure out what the parent_document_url should be, and is largely irrelevant for a superseded asset.
- Fix correct state for assets that have gone in a weird invalid state over the years.
"deleted" should no longer be a valid state. patching any data that we come across that has a wrong state.
- Log when we've patched an invalid state or parent document URL
  • Loading branch information
minhngocd authored and lauraghiorghisor-tw committed Dec 5, 2024
1 parent 45fcca1 commit 1b1297e
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 8 deletions.
19 changes: 16 additions & 3 deletions lib/tasks/assets.rake
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace :assets do
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."
puts "Asset ID: #{original_asset_id} - OK. Asset 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
Expand All @@ -93,7 +93,7 @@ namespace :assets do
end

begin
original_asset.destroy!
delete_and_update_draft(original_asset, should_update_draft: false)
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}."
Expand All @@ -103,8 +103,21 @@ namespace :assets do
end
end

def delete_and_update_draft(asset)
def delete_and_update_draft(asset, should_update_draft: true)
if asset.state.to_s == "deleted"
asset.state = "uploaded"
puts "Patched state: #{asset.id}"
end

if asset.parent_document_url&.include?("draft-origin")
asset.parent_document_url = nil
puts "Patched Parent URL: #{asset.id}"
end

asset.destroy! unless asset.deleted?

return unless should_update_draft

asset.draft = false
asset.save!
end
Expand Down
72 changes: 67 additions & 5 deletions spec/lib/tasks/assets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,40 +66,102 @@
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)
it "deletes the asset replacement, updates the draft state, and turn draft parent_url to nil if the replacement is not deleted" do
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil, replacement_id: nil, parent_document_url: "https://draft-origin.publishing.service.gov.uk/example")
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)

expected_output = <<~OUTPUT
Patched Parent URL: #{replacement.id}
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
expect(replacement.reload.parent_document_url).to be_nil
end

it "deletes the asset replacement, updates the draft state, and leave live parent_url alone if the replacement is not deleted" do
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil, replacement_id: nil, parent_document_url: 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
expect(replacement.reload.parent_document_url).to be_nil
end

it "deletes the asset replacement and fixes invalid upload state if the replacement is not deleted" do
replacement = FactoryBot.create(:asset, draft: true, deleted_at: nil)
FactoryBot.create(:asset, id: asset_id, replacement_id: replacement.id)
replacement.state = "deleted"
replacement.save!(validate: false)

expected_output = <<~OUTPUT
Patched state: #{replacement.id}
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
expect(replacement.reload.state).to eq "uploaded"
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.
Asset ID: #{asset_id} - OK. Asset 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, updates the draft state, and turn draft parent_url into nil if the asset is not deleted (asset is a replacement)" do
asset = FactoryBot.create(:asset, id: asset_id, draft: true, deleted_at: nil, parent_document_url: "https://draft-origin.publishing.service.gov.uk/example")
FactoryBot.create(:asset, replacement_id: asset.id)

expected_output = <<~OUTPUT
Patched Parent URL: #{asset_id}
Asset ID: #{asset_id} - OK. Asset 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
expect(asset.reload.parent_document_url).to be_nil
end

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

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - OK. Asset 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
expect(asset.reload.parent_document_url).to be_nil
end

it "deletes the asset and updates the draft state if the asset is not deleted (asset is a replacement)" do
it "deletes the asset and fixes invalid upload 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)
asset.state = "deleted"
asset.save!(validate: false)

expected_output = <<~OUTPUT
Asset ID: #{asset_id} - is a replacement. Asset deleted and updated to false.
Patched state: #{asset_id}
Asset ID: #{asset_id} - OK. Asset 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
expect(asset.reload.state).to eq "uploaded"
end

it "skips the asset if asset is itself redirected (and not replaced by draft or itself a replacement)" do
Expand Down

0 comments on commit 1b1297e

Please sign in to comment.