From 61002a2b336eae736227d3b174a0d0dfa0901ac4 Mon Sep 17 00:00:00 2001
From: Minno Dang <mndang@thoughtworks.com>
Date: Fri, 6 Dec 2024 14:46:45 +0000
Subject: [PATCH] Remove also assets which are in draft but not deleted,
 replaced, or redirected

These are relatively harmless to the public, as the links will redirect to SignOn and require a login to preview. However it would still be available to other users with a SignOn account. For assets that are superseded, it's probably best we also remove them so users are certain that as long as assets are not linked to their live or preview editions, it should not be accessible anywhere.
---
 lib/tasks/assets.rake         |  2 +-
 spec/lib/tasks/assets_spec.rb | 19 ++++++++++---------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake
index 2a022e13..05190a01 100644
--- a/lib/tasks/assets.rake
+++ b/lib/tasks/assets.rake
@@ -104,7 +104,7 @@ namespace :assets do
           next
         end
 
-        if original_asset.draft? || original_asset.deleted? || replacement_asset || original_asset.redirect_url
+        if 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
diff --git a/spec/lib/tasks/assets_spec.rb b/spec/lib/tasks/assets_spec.rb
index ff3b77a3..01627899 100644
--- a/spec/lib/tasks/assets_spec.rb
+++ b/spec/lib/tasks/assets_spec.rb
@@ -137,15 +137,6 @@
           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)
 
@@ -382,6 +373,16 @@
         expect { task.invoke(filepath) }.to output(expected_output).to_stdout
       end
 
+      it "deletes but not update draft status of the asset if asset is itself in draft, not deleted, not redirected, not replaced" do
+        asset = FactoryBot.create(:asset, id: asset_id, draft: true)
+
+        expected_output = <<~OUTPUT
+          Asset ID: #{asset_id} - OK. Asset has been deleted.
+        OUTPUT
+        expect { task.invoke(filepath) }.to output(expected_output).to_stdout
+        expect(asset.reload.draft).to be true
+      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)