diff options
| -rw-r--r-- | app/controllers/assets_controller.rb | 2 | ||||
| -rw-r--r-- | app/helpers/node_actions_helper.rb | 11 | ||||
| -rw-r--r-- | app/models/asset.rb | 30 | ||||
| -rw-r--r-- | app/models/node_action.rb | 9 | ||||
| -rw-r--r-- | config/locales/de.yml | 3 | ||||
| -rw-r--r-- | config/locales/en.yml | 3 | ||||
| -rw-r--r-- | test/controllers/assets_controller_test.rb | 10 | ||||
| -rw-r--r-- | test/models/asset_destroy_test.rb | 51 |
8 files changed, 118 insertions, 1 deletions
diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index fbede0a7..8df4c94d 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb | |||
| @@ -81,7 +81,7 @@ class AssetsController < ApplicationController | |||
| 81 | # DELETE /assets/1.xml | 81 | # DELETE /assets/1.xml |
| 82 | def destroy | 82 | def destroy |
| 83 | @asset = Asset.find(params[:id]) | 83 | @asset = Asset.find(params[:id]) |
| 84 | @asset.destroy | 84 | @asset.destroy_witnessed!(:user => current_user) |
| 85 | 85 | ||
| 86 | respond_to do |format| | 86 | respond_to do |format| |
| 87 | format.html { redirect_to(assets_url) } | 87 | format.html { redirect_to(assets_url) } |
diff --git a/app/helpers/node_actions_helper.rb b/app/helpers/node_actions_helper.rb index fd8cc36e..02d1ba83 100644 --- a/app/helpers/node_actions_helper.rb +++ b/app/helpers/node_actions_helper.rb | |||
| @@ -180,4 +180,15 @@ module NodeActionsHelper | |||
| 180 | t("node_actions.destroy", :actor => actor_ref(action), :subject => subject_ref(action), | 180 | t("node_actions.destroy", :actor => actor_ref(action), :subject => subject_ref(action), |
| 181 | :path => h(action.metadata["path"])).html_safe | 181 | :path => h(action.metadata["path"])).html_safe |
| 182 | end | 182 | end |
| 183 | |||
| 184 | def summarize_asset_destroy action | ||
| 185 | m = action.metadata | ||
| 186 | parts = [t("node_actions.asset_destroy", :actor => actor_ref(action), | ||
| 187 | :asset => h(m["asset_name"].presence || m["path"]))] | ||
| 188 | parts << t("node_actions.asset_destroy_detached", | ||
| 189 | :paths => h(Array(m["detached_from"]).join(", "))) if m["detached_from"].present? | ||
| 190 | parts << t("node_actions.asset_destroy_headlines", | ||
| 191 | :paths => h(Array(m["headline_removed_from"]).join(", "))) if m["headline_removed_from"].present? | ||
| 192 | safe_join(parts, " ") | ||
| 193 | end | ||
| 183 | end | 194 | end |
diff --git a/app/models/asset.rb b/app/models/asset.rb index 73970e21..8cec4371 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb | |||
| @@ -40,4 +40,34 @@ class Asset < ApplicationRecord | |||
| 40 | Node.where("head_id IN (:ids) OR draft_id IN (:ids) OR autosave_id IN (:ids)", | 40 | Node.where("head_id IN (:ids) OR draft_id IN (:ids) OR autosave_id IN (:ids)", |
| 41 | :ids => page_ids).distinct | 41 | :ids => page_ids).distinct |
| 42 | end | 42 | end |
| 43 | |||
| 44 | # Witnessed destruction. Destroying an asset is a public-facing act | ||
| 45 | # even when unattached. The original and its variants are publicly | ||
| 46 | # reachable under /system/uploads, so an entry is always written, | ||
| 47 | # before the row and its files die. Every currently-attached node | ||
| 48 | # participates so its zoomed history shows the loss; the asset itself | ||
| 49 | # participates as the first non-Node subject (its participant row | ||
| 50 | # dangles after destroy, by design, the name lives on in metadata). | ||
| 51 | def destroy_witnessed! user: | ||
| 52 | ActiveRecord::Base.transaction do | ||
| 53 | affected = attached_nodes.to_a | ||
| 54 | headline_losses = affected.select do |node| | ||
| 55 | [node.head, node.draft, node.autosave].compact.any? do |row| | ||
| 56 | row.related_assets.exists?(:asset_id => id, :headline => true) | ||
| 57 | end | ||
| 58 | end | ||
| 59 | |||
| 60 | metadata = { | ||
| 61 | :asset_name => name, | ||
| 62 | :content_type => upload_content_type, | ||
| 63 | :path => upload.url.sub(/\?\d+$/, ""), | ||
| 64 | } | ||
| 65 | metadata[:detached_from] = affected.map(&:unique_name) if affected.any? | ||
| 66 | metadata[:headline_removed_from] = headline_losses.map(&:unique_name) if headline_losses.any? | ||
| 67 | |||
| 68 | NodeAction.record!(:participants => [self] + affected, :user => user, | ||
| 69 | :action => "asset_destroy", **metadata) | ||
| 70 | destroy! | ||
| 71 | end | ||
| 72 | end | ||
| 43 | end | 73 | end |
diff --git a/app/models/node_action.rb b/app/models/node_action.rb index 8a3dd8b7..9ed0b628 100644 --- a/app/models/node_action.rb +++ b/app/models/node_action.rb | |||
| @@ -69,6 +69,15 @@ class NodeAction < ApplicationRecord | |||
| 69 | # "path" -- final path, flat string (create-symmetric) | 69 | # "path" -- final path, flat string (create-symmetric) |
| 70 | # "destroyed_descendants" -- integer, only when positive; one entry | 70 | # "destroyed_descendants" -- integer, only when positive; one entry |
| 71 | # at the root, per the subtree rule. | 71 | # at the root, per the subtree rule. |
| 72 | # "asset_destroy" (witnessed asset deletion; always written, even for | ||
| 73 | # unattached assets -- the files were publicly reachable; node column | ||
| 74 | # nil, subjects via participants: the asset plus every then-attached | ||
| 75 | # node): | ||
| 76 | # "asset_name" -- flat string | ||
| 77 | # "content_type" -- flat string | ||
| 78 | # "path" -- public original path, flat string | ||
| 79 | # "detached_from" -- array of unique_names, only when any | ||
| 80 | # "headline_removed_from" -- array of unique_names, only when any | ||
| 72 | # | 81 | # |
| 73 | # Reserved: "demote" (via "trash" | "depublish") for an explicit | 82 | # Reserved: "demote" (via "trash" | "depublish") for an explicit |
| 74 | # depublish workflow, if ever built. | 83 | # depublish workflow, if ever built. |
diff --git a/config/locales/de.yml b/config/locales/de.yml index dd22cef7..d6241d51 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml | |||
| @@ -116,6 +116,9 @@ de: | |||
| 116 | revision_created: "angelegt am %{date} von %{actor}" | 116 | revision_created: "angelegt am %{date} von %{actor}" |
| 117 | revision_published: "veröffentlicht am %{date} von %{actor}" | 117 | revision_published: "veröffentlicht am %{date} von %{actor}" |
| 118 | revision_restored: "wiederhergestellt am %{date} von %{actor}" | 118 | revision_restored: "wiederhergestellt am %{date} von %{actor}" |
| 119 | asset_destroy: "%{actor} hat das Asset „%{asset}“ gelöscht" | ||
| 120 | asset_destroy_detached: "— entfernt von %{paths}" | ||
| 121 | asset_destroy_headlines: "(war Aufmacher von %{paths})" | ||
| 119 | 122 | ||
| 120 | open_gallery: "Gallerie anzeigen" | 123 | open_gallery: "Gallerie anzeigen" |
| 121 | asset_licenses: | 124 | asset_licenses: |
diff --git a/config/locales/en.yml b/config/locales/en.yml index 9be01524..bd59915b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml | |||
| @@ -68,6 +68,9 @@ en: | |||
| 68 | revision_created: "created %{date} by %{actor}" | 68 | revision_created: "created %{date} by %{actor}" |
| 69 | revision_published: "published %{date} by %{actor}" | 69 | revision_published: "published %{date} by %{actor}" |
| 70 | revision_restored: "restored %{date} by %{actor}" | 70 | revision_restored: "restored %{date} by %{actor}" |
| 71 | asset_destroy: "%{actor} destroyed asset “%{asset}”" | ||
| 72 | asset_destroy_detached: "— detached from %{paths}" | ||
| 73 | asset_destroy_headlines: "(was the headline of %{paths})" | ||
| 71 | 74 | ||
| 72 | open_gallery: "Open gallery" | 75 | open_gallery: "Open gallery" |
| 73 | asset_licenses: | 76 | asset_licenses: |
diff --git a/test/controllers/assets_controller_test.rb b/test/controllers/assets_controller_test.rb index 05fc6ded..0e251aad 100644 --- a/test/controllers/assets_controller_test.rb +++ b/test/controllers/assets_controller_test.rb | |||
| @@ -182,6 +182,16 @@ class AssetsControllerTest < ActionController::TestCase | |||
| 182 | assert !Dir.exist?(upload_dir), "Upload directory should be removed after destroy" | 182 | assert !Dir.exist?(upload_dir), "Upload directory should be removed after destroy" |
| 183 | end | 183 | end |
| 184 | 184 | ||
| 185 | test "destroy is witnessed in the action log with the current user" do | ||
| 186 | asset = Asset.create!(:name => 'Witness me', | ||
| 187 | :upload_file_name => 'w.png', | ||
| 188 | :upload_content_type => 'image/png') | ||
| 189 | assert_difference 'NodeAction.where(:action => "asset_destroy").count' do | ||
| 190 | delete :destroy, params: { id: asset.id } | ||
| 191 | end | ||
| 192 | assert_equal users(:quentin), NodeAction.last.user | ||
| 193 | end | ||
| 194 | |||
| 185 | # --- URL helpers --- | 195 | # --- URL helpers --- |
| 186 | 196 | ||
| 187 | test "upload url returns correct path for original" do | 197 | test "upload url returns correct path for original" do |
diff --git a/test/models/asset_destroy_test.rb b/test/models/asset_destroy_test.rb new file mode 100644 index 00000000..5583f685 --- /dev/null +++ b/test/models/asset_destroy_test.rb | |||
| @@ -0,0 +1,51 @@ | |||
| 1 | require "test_helper" | ||
| 2 | |||
| 3 | class AssetDestroyTest < ActiveSupport::TestCase | ||
| 4 | def setup | ||
| 5 | @user = users(:quentin) | ||
| 6 | @asset = Asset.create!(:name => "Doomed asset", | ||
| 7 | :upload_file_name => "doomed.png", | ||
| 8 | :upload_content_type => "image/png") | ||
| 9 | end | ||
| 10 | |||
| 11 | test "destroying an attached asset logs nodes and asset as participants" do | ||
| 12 | node = Node.root.children.create!(:slug => "asset_destroy_attached") | ||
| 13 | node.attach_asset!(@asset, :user => @user) | ||
| 14 | |||
| 15 | @asset.destroy_witnessed!(:user => @user) | ||
| 16 | |||
| 17 | action = NodeAction.where(:action => "asset_destroy").last | ||
| 18 | subjects = action.action_participants.map { |p| [p.subject_type, p.subject_id] } | ||
| 19 | assert_includes subjects, ["Asset", @asset.id] | ||
| 20 | assert_includes subjects, ["Node", node.id] | ||
| 21 | assert_equal [node.unique_name], action.metadata["detached_from"] | ||
| 22 | end | ||
| 23 | |||
| 24 | test "records which nodes lost their headline" do | ||
| 25 | node = Node.root.children.create!(:slug => "asset_destroy_headline") | ||
| 26 | node.attach_asset!(@asset, :user => @user, :headline => true) | ||
| 27 | |||
| 28 | @asset.destroy_witnessed!(:user => @user) | ||
| 29 | |||
| 30 | action = NodeAction.where(:action => "asset_destroy").last | ||
| 31 | assert_equal [node.unique_name], action.metadata["headline_removed_from"] | ||
| 32 | end | ||
| 33 | |||
| 34 | test "an unattached asset is still witnessed" do | ||
| 35 | @asset.destroy_witnessed!(:user => @user) | ||
| 36 | |||
| 37 | action = NodeAction.where(:action => "asset_destroy").last | ||
| 38 | assert_equal [["Asset", @asset.id]], | ||
| 39 | action.action_participants.map { |p| [p.subject_type, p.subject_id] } | ||
| 40 | assert_nil action.node_id | ||
| 41 | end | ||
| 42 | |||
| 43 | test "the entry outlives the asset" do | ||
| 44 | @asset.destroy_witnessed!(:user => @user) | ||
| 45 | action = NodeAction.where(:action => "asset_destroy").last | ||
| 46 | |||
| 47 | assert_not Asset.exists?(@asset.id) | ||
| 48 | assert_equal "Doomed asset", action.metadata["asset_name"] | ||
| 49 | assert_nil action.action_participants.first.subject | ||
| 50 | end | ||
| 51 | end | ||
