From 58c4006ee3c0a5e3055f76dd7f59df0ac7f44446 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Wed, 4 Dec 2024 15:51:17 +0000 Subject: [PATCH] Fix invalid state and reset parent document url when updating assets - 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 --- lib/tasks/assets.rake | 3 ++ spec/lib/tasks/assets_spec.rb | 68 +++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index 85c1c9e2..04d0c220 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -104,7 +104,10 @@ namespace :assets do end def delete_and_update_draft asset + (asset.state = "uploaded"; puts "Patched state: #{asset.id}") if asset.state.to_s == "deleted" asset.destroy! unless asset.deleted? + + (asset.parent_document_url = nil; puts "Patched Parent URL: #{asset.id}") if asset.parent_document_url&.include?("draft-origin") asset.draft = false asset.save! end diff --git a/spec/lib/tasks/assets_spec.rb b/spec/lib/tasks/assets_spec.rb index 956399cd..06efd7bc 100644 --- a/spec/lib/tasks/assets_spec.rb +++ b/spec/lib/tasks/assets_spec.rb @@ -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 eq 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 @@ -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 eq 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