[#3647][fix][cms] edit page which applied a urgency layout (#3649)

* [fix][#3647] layout is unset if urgency layout is set on page

* [add] reproducible spec

* [modify] refactor spec to check for user experiences more

app/helpers/cms/form_helper.rb

@@ -1,5 +1,5 @@
 module Cms::FormHelper
-  def ancestral_layouts(node)
+  def ancestral_layouts(node, cur_layout = nil)
     node  = @cur_node if !node || node.new_record?
     items = []
     if node
@@ -15,6 +15,11 @@ def ancestral_layouts(node)
     Cms::Layout.site(@cur_site).where(depth: 1).sort(name: 1).each do |item|
       items << [item.name, item.id]
     end
+    if cur_layout
+      unless items.find { |_name, id| id == cur_layout.id }
+        items.prepend([cur_layout.name, cur_layout.id])
+      end
+    end
     items
   end
 

app/views/article/pages/_form.html.erb

@@ -12,7 +12,7 @@
   </dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/cms/agents/addons/node_setting/_form.html.erb

@@ -8,7 +8,7 @@ view_route = @item.view_route if @item.view_route.present?
 
 <dl class="see mod-cms-node_setting">
   <dt><%= @model.t :page_layout_id %><%= @model.tt :page_layout_id %></dt>
-  <dd><%= f.select :page_layout_id, ancestral_layouts(@item), include_blank: " " %></dd>
+  <dd><%= f.select :page_layout_id, ancestral_layouts(@item, @item.layout), include_blank: " " %></dd>
 
   <dt><%= @model.t :order %><%= @model.tt :order %></dt>
   <dd><%= f.number_field :order %></dd>

app/views/cms/nodes/_form.html.erb

@@ -17,7 +17,7 @@ file = "#{Rails.root}/app/views/" + @item.route.sub('/', '/agents/nodes/') + "/e
   <dd><%= f.text_field :basename, class: "filename" %></dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item, @item.layout), include_blank: " " %></dd>
 </dl>
 
 <%= render file: file, locals: { f: f } if File.exists?(file) %>

app/views/cms/pages/_form.html.erb

@@ -9,7 +9,7 @@
   <dd><%= f.text_field :basename, class: "filename" %></dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/event/pages/_form.html.erb

@@ -12,7 +12,7 @@
   </dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/ezine/pages/_form.html.erb

@@ -9,7 +9,7 @@
   <dd><%= f.text_field :basename, id: "ezine-pages-form-dd-input", class: "filename", style: "display: none" %></dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/facility/images/_form.html.erb

@@ -12,7 +12,7 @@
   </dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/facility/maps/_form.html.erb

@@ -12,7 +12,7 @@
   </dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/faq/pages/_form.html.erb

@@ -12,7 +12,7 @@
   </dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/mail_page/pages/_form.html.erb

@@ -12,7 +12,7 @@
   </dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

app/views/member/blog_page_locations/_form.html.erb

@@ -6,7 +6,7 @@
   <dd><%= f.text_field :basename, class: "filename" %></dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@cur_node), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@cur_node, @cur_node.layout), include_blank: " " %></dd>
 
   <dt><%= @model.t :state %><%= @model.tt :state %></dt>
   <dd><%= f.select :state, @item.state_options %></dd>

app/views/member/blogs/_form.html.erb

@@ -6,7 +6,7 @@
   <dd><%= f.text_field :basename, class: "filename" %></dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@cur_node), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@cur_node, @cur_node.layout), include_blank: " " %></dd>
 
   <dt><%= @model.t :state %><%= @model.tt :state %></dt>
   <dd><%= f.select :state, @item.state_options %></dd>

app/views/member/photos/_form.html.erb

@@ -4,7 +4,7 @@
   <dd><%= f.text_field :name %></dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <dt><%= @model.t :listable_state %><%= @model.tt :listable_state %></dt>
   <dd><%= f.select :listable_state, @item.listable_state_options %></dd>

app/views/opendata/agents/addons/list_node_setting/_form.html.erb

@@ -7,7 +7,7 @@
 %>
 <dl class="see mod-opendata-list-node-setting">
   <dt><%= @model.t :page_layout_id %><%= @model.tt :page_layout_id %></dt>
-  <dd><%= f.select :page_layout_id, ancestral_layouts(@item), include_blank: " " %></dd>
+  <dd><%= f.select :page_layout_id, ancestral_layouts(@item, @item.layout), include_blank: " " %></dd>
 
   <dt><%= @model.t :order %><%= @model.tt :order %></dt>
   <dd><%= f.number_field :order %></dd>

app/views/rss/pages/_form.html.erb

@@ -12,7 +12,7 @@
   </dd>
 
   <dt><%= @model.t :layout_id %><%= @model.tt :layout_id %></dt>
-  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent), include_blank: " " %></dd>
+  <dd><%= f.select :layout_id, ancestral_layouts(@item.parent, @item.layout), include_blank: " " %></dd>
 
   <%= render file: 'cms/pages/_form_body_parts', locals: { f: f } %>
 

spec/features/urgency/layouts_spec.rb

@@ -3,29 +3,56 @@
 describe "urgency_layouts", type: :feature, dbscope: :example do
   let!(:site) { cms_site }
 
-  let!(:node) { create_once :urgency_node_layout, filename: "urgency", urgency_default_layout_id: layout1.id }
+  let!(:node) { create :urgency_node_layout, filename: "urgency", urgency_default_layout_id: layout1.id }
   let!(:layout1) { create :cms_layout, cur_site: site, name: unique_id, filename: "layout1.layout.html" }
   let!(:layout2) { create :cms_layout, cur_site: site, name: unique_id, filename: "urgency/layout2.layout.html" }
 
-  let!(:item) { create_once :cms_page, name: "index", filename: "index.html" }
-  let!(:index_path) { urgency_layouts_path site.id, node }
+  let!(:item) { create :cms_page, name: "index", filename: "index.html" }
 
-  context "with auth" do
+  describe "apply urgency layout and vice versa" do
     before { login_cms_user }
 
-    it "#index" do
-      visit index_path
-      expect(current_path).not_to eq sns_login_path
+    it do
+      visit urgency_layouts_path(site: site, cid: node)
 
       click_on layout2.name
       click_on I18n.t('urgency.switch_layout')
-      expect(current_path).to eq index_path
-      expect(Cms::Page.find(item.id).layout_id).to eq layout2.id
+      expect(page).to have_css("#notice", text: I18n.t("ss.notice.saved"))
+
+      item.reload
+      expect(item.layout_id).to eq layout2.id
 
       click_on layout1.name
       click_on I18n.t('urgency.switch_layout')
-      expect(current_path).to eq index_path
-      expect(Cms::Page.find(item.id).layout_id).to eq layout1.id
+      expect(page).to have_css("#notice", text: I18n.t("ss.notice.saved"))
+
+      item.reload
+      expect(item.layout_id).to eq layout1.id
+    end
+  end
+
+  describe "ss-3647" do
+    before { login_cms_user }
+
+    it do
+      # change to urgency layout
+      visit urgency_layouts_path(site: site, cid: node)
+      click_on layout2.name
+      click_on I18n.t('urgency.switch_layout')
+      expect(page).to have_css("#notice", text: I18n.t("ss.notice.saved"))
+
+      item.reload
+      expect(item.layout_id).to eq layout2.id
+
+      # edit page without any modifications
+      visit cms_page_path(site: site, id: item)
+      click_on I18n.t("ss.links.edit")
+      click_on I18n.t("ss.buttons.save")
+      expect(page).to have_css("#notice", text: I18n.t("ss.notice.saved"))
+
+      # confirm that layout is still set
+      item.reload
+      expect(item.layout_id).to eq layout2.id
     end
   end
 end