Skip to content

Commit ba9c7e5

Browse files
committed
[BUG] Deleting addressable content
* Fix issue where deleting addressable block from the sitemap throws error and breaks the UI.
1 parent 7a3f82e commit ba9c7e5

File tree

7 files changed

+30
-13
lines changed

7 files changed

+30
-13
lines changed

app/controllers/cms/content_block_controller.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ def update
7575

7676
def destroy
7777
do_command("deleted") { @block.destroy }
78-
redirect_to_first params[:_redirect_to], blocks_path
78+
respond_to do |format|
79+
format.html {redirect_to_first params[:_redirect_to], blocks_path}
80+
format.json {render :json => {:success => true }}
81+
end
82+
7983
end
8084

8185
# Additional CMS Action

app/models/cms/link.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ class Cms::Link < ActiveRecord::Base
55

66
validates_presence_of :name
77

8-
is_addressable(dependent: :destroy)
8+
is_addressable
99
include Cms::Concerns::Addressable::DeprecatedPageAccessors
1010

1111
#needed by menu_helper

app/models/cms/page.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ def actual_path
6767
end
6868
}
6969

70-
is_addressable(dependent: :destroy, no_dynamic_path: true)
70+
is_addressable(no_dynamic_path: true)
7171
include Cms::Concerns::Addressable::DeprecatedPageAccessors
7272

7373

app/models/cms/section.rb

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ module Cms
22
class Section < ActiveRecord::Base
33
flush_cache_on_change
44

5-
is_addressable no_dynamic_path: true
5+
is_addressable no_dynamic_path: true, destroy_if: :deletable?
66
# Cannot use dependent => :destroy to do this. Ancestry's callbacks trigger before the before_destroy callback.
77
# So sections would always get deleted since deletable? would return true
88
after_destroy :destroy_node
@@ -131,11 +131,6 @@ def deletable?
131131
!root? && empty?
132132
end
133133

134-
# Callback to clean up related nodes
135-
def destroy_node
136-
node.destroy
137-
end
138-
139134
def editable_by_group?(group)
140135
group.editable_by_section(self)
141136
end

lib/cms/concerns/can_be_addressable.rb

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,16 @@ module CanBeAddressable
88
# @params [Hash] options
99
# @option options [String] :path The base path where instances will be placed.
1010
# @option options [String] :no_dynamic_path Set as true if the Record has a :path attribute managed as a column in the db. (Default: false)
11+
# @option options [Symbol] :destroy_if Name of a custom method used to determine when this object should be destrotyed. Rather than dependant: destroy to determine if the section node should be destroyed when this object is.
1112
def is_addressable(options={})
1213
has_one_options = {as: :node, inverse_of: :node, class_name: 'Cms::SectionNode'}
13-
if options[:dependent]
14-
has_one_options[:dependent] = options[:dependent]
14+
unless options[:destroy_if]
15+
has_one_options[:dependent] = :destroy
16+
else
17+
before_destroy options[:destroy_if]
18+
after_destroy :destroy_node
1519
end
20+
1621
has_one :section_node, has_one_options
1722

1823
include Cms::Concerns::Addressable
@@ -144,6 +149,10 @@ def self.descendants
144149
ObjectSpace.each_object(::Class).select { |klass| klass < Cms::Concerns::Addressable }
145150
end
146151

152+
# Allows for manual destruction of node
153+
def destroy_node
154+
node.destroy
155+
end
147156

148157
# Returns the value that will appear in the <title> element of the page when this content is rendered.
149158
# Subclasses can override this.

test/unit/concerns/addressable_test.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ class UsingDefaultTemplate < ActiveRecord::Base
9494
end
9595
end
9696

97+
describe ".destroy" do
98+
it "should also delete the section node" do
99+
add = IsAddressable.create(slug: "coke", parent_id: root_section)
100+
before = Cms::SectionNode.count
101+
add.destroy
102+
(Cms::SectionNode.count - before).must_equal -1
103+
end
104+
end
105+
97106
describe ".path" do
98107
it "should join #path and .slug" do
99108
addressable.expects(:slug).returns("one")

todo_ui_Revamp.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ Tasks:
22

33
## UI Merge
44

5-
* [BUG] Deleting addressable content from the sitemap throws error and breaks the UI.
6-
* Select something else after deleting.
5+
* Sections can be hidden (but sitemap doesn't show it)
6+
* Select something else after deleting.
77
* Handle non-editable sections (from security)
88
* Audit sitemap.js.erb for remaining features.
99
* Have something selected at the start (last selected or Root)

0 commit comments

Comments
 (0)