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 4, 2024
1 parent 2302ab2 commit f47657e
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 3 deletions.
9 changes: 9 additions & 0 deletions lib/tasks/assets.rake
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,16 @@ namespace :assets do
end

def delete_and_update_draft(asset)
if asset.state.to_s == "deleted"
asset.state = "uploaded"
puts "Patched state: #{asset.id}"
end
asset.destroy! unless asset.deleted?

if asset.parent_document_url&.include?("draft-origin")
asset.parent_document_url = nil
puts "Patched Parent URL: #{asset.id}"
end
asset.draft = false
asset.save!
end
Expand Down
68 changes: 65 additions & 3 deletions spec/lib/tasks/assets_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,47 @@
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
Expand All @@ -90,16 +121,47 @@
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
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} - 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} - 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 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
Patched state: #{asset_id}
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
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 f47657e

Please sign in to comment.