From f58f4f98bb496a647be060c9ecc75387861f04a9 Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <mamhoff@gmail.com>
Date: Wed, 13 May 2020 23:43:33 +0200
Subject: [PATCH] Add errors when node cant be deleted (#1828)

* Add error before destroying node with attached essence

Nodes that are used on e.g. content pages can not be simply destroyed.
This commit adds an error message so the user knows what's going on.

* Do not destroy nodes with dependent nodes that are in use on a page

This moves the validation up in the file so that awesome_nested_set does
not run its `delete_all` callback before we validate that it can do
that.

It also extends the validation such that we collect all the pages that
are referenced in the subtree below this node.
---
 app/models/alchemy/node.rb       | 10 ++++++++++
 config/locales/alchemy.en.yml    |  4 ++++
 spec/models/alchemy/node_spec.rb | 29 +++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/app/models/alchemy/node.rb b/app/models/alchemy/node.rb
index 60da0e003c..d9a3a3dd49 100644
--- a/app/models/alchemy/node.rb
+++ b/app/models/alchemy/node.rb
@@ -4,6 +4,8 @@ module Alchemy
   class Node < BaseRecord
     VALID_URL_REGEX = /\A(\/|\D[a-z\+\d\.\-]+:)/
 
+    before_destroy :check_if_related_essence_nodes_present
+
     acts_as_nested_set scope: "language_id", touch: true
     stampable stamper_class_name: Alchemy.user_class_name
 
@@ -75,5 +77,13 @@ def to_partial_path
     def view_folder_name
       "alchemy/menus/#{name.parameterize.underscore}"
     end
+
+    def check_if_related_essence_nodes_present
+      dependent_essence_nodes = self_and_descendants.flat_map(&:essence_nodes)
+      if dependent_essence_nodes.any?
+        errors.add(:base, :essence_nodes_present, page_names: dependent_essence_nodes.map(&:page).map(&:name).to_sentence)
+        throw(:abort)
+      end
+    end
   end
 end
diff --git a/config/locales/alchemy.en.yml b/config/locales/alchemy.en.yml
index 4bf4efef30..785066d4fa 100644
--- a/config/locales/alchemy.en.yml
+++ b/config/locales/alchemy.en.yml
@@ -711,6 +711,10 @@ en:
   activerecord:
     errors:
       models:
+        alchemy/node:
+          attributes:
+            base:
+              essence_nodes_present: "This menu item is in use inside an Alchemy element on the following pages: %{page_names}."
         alchemy/site:
           attributes:
             languages:
diff --git a/spec/models/alchemy/node_spec.rb b/spec/models/alchemy/node_spec.rb
index 9a8979b374..defceb870e 100644
--- a/spec/models/alchemy/node_spec.rb
+++ b/spec/models/alchemy/node_spec.rb
@@ -135,4 +135,33 @@ module Alchemy
       end
     end
   end
+
+  describe "#destroy" do
+    context "if there are essence nodes present" do
+      let(:node) { create(:alchemy_node) }
+      let(:page) { create(:alchemy_page, :layoutpage, page_layout: :footer) }
+      let(:element) { create(:alchemy_element, name: "menu", page: page) }
+      let(:content) { create(:alchemy_content, name: "menu", element: element) }
+
+      before do
+        node.essence_nodes.create(content: content)
+      end
+
+      it "does not destroy the node but adds an error" do
+        node.destroy
+        expect(node).not_to be_destroyed
+        expect(node.errors.full_messages).to eq(["This menu item is in use inside an Alchemy element on the following pages: #{page.name}."])
+      end
+
+      context "if there are essence nodes present on a child node" do
+        let!(:parent_node) { create(:alchemy_node, children: [node]) }
+
+        it "does not destroy the node and children either but adds an error" do
+          parent_node.destroy
+          expect(parent_node).not_to be_destroyed
+          expect(parent_node.errors.full_messages).to eq(["This menu item is in use inside an Alchemy element on the following pages: #{page.name}."])
+        end
+      end
+    end
+  end
 end