Skip to content

Fixes GH-1892 by properly updating the form action when the title is changed during save-and-continue #1944

Closed
wants to merge 1 commit into from

3 participants

@tsemana
tsemana commented Sep 14, 2012

Passes the admin_pages_path url using the items current title back to the view using a hidden field tag. The save-and-continue javascript then updates the form action with the new url.
Includes request spec for the first save-and-continue and regression test for the bug described in GH-1892.

@parndt parndt commented on an outdated diff Sep 14, 2012
core/app/views/refinery/_message.html.erb
<% flash.each do |key, value| %>
-<div id='flash' class="flash flash_<%= key %>" style='visibility: hidden;' >
- <%= value %>
- <%= link_to t(".close#{'_this_message' if key.to_s == "message"}"), "",
- :id => "flash_close" %>
-</div>
-<% end %>
+ <div id='flash' class="flash flash_<%= key %>" style='visibility: hidden;' >
+ <%= value %>
+ <%= link_to t(".close#{'_this_message' if key.to_s == "message"}"), "",
+ :id => "flash_close" %>
+ </div>
@parndt
Refinery member
parndt added a note Sep 14, 2012

indentation doesn't match opening <div

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@parndt parndt commented on an outdated diff Sep 14, 2012
core/app/views/refinery/_message.html.erb
<% flash.each do |key, value| %>
-<div id='flash' class="flash flash_<%= key %>" style='visibility: hidden;' >
- <%= value %>
- <%= link_to t(".close#{'_this_message' if key.to_s == "message"}"), "",
- :id => "flash_close" %>
-</div>
-<% end %>
+ <div id='flash' class="flash flash_<%= key %>" style='visibility: hidden;' >
+ <%= value %>
+ <%= link_to t(".close#{'_this_message' if key.to_s == "message"}"), "",
+ :id => "flash_close" %>
@parndt
Refinery member
parndt added a note Sep 14, 2012

can you line up the :id with the t( as before?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@parndt parndt and 1 other commented on an outdated diff Sep 14, 2012
core/lib/refinery/crud.rb
@@ -125,7 +125,9 @@ def update
unless request.xhr?
redirect_to :back
else
- render :partial => '/refinery/message'
+ render :partial => '/refinery/message',
+ #pass the new update url in case the title was updated
+ :locals => {:new_url => refinery.admin_page_path(@#{singular_name}.uncached_nested_url) }
@parndt
Refinery member
parndt added a note Sep 14, 2012

this shouldn't go in crud because it only applies to admin/pages_controller

@tsemana
tsemana added a note Sep 14, 2012

Thanks, should I carry over the whole update method over to admin/pages_controller? or is there a way to narrow it down to when it's an xhr request?

@parndt
Refinery member
parndt added a note Sep 14, 2012
@parndt
Refinery member
parndt added a note Sep 14, 2012
$ rake app:refinery:uncrudify controller=refinery/admin/pages action=update
def update
  if @page.update_attributes(params[:page])
    flash.notice = t(
      'refinery.crudify.updated',
      :what => "'#{@page.title}'"
    )

    unless from_dialog?
      unless params[:continue_editing] =~ /true|on|1/
        redirect_back_or_default(refinery.admin_pages_path)
      else
        unless request.xhr?
          redirect_to :back
        else
          render :partial => '/refinery/message', :locals => {
            :new_url => refinery.admin_page_path(@page.uncached_nested_url) 
          }
        end
      end
    else
      self.index
      @dialog_successful = true
      render :index
    end
  else
    unless request.xhr?
      render :action => 'edit'
    else
      render :partial => '/refinery/admin/error_messages', :locals => {
               :object => @page,
               :include_object_name => true
             }
    end
  end
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tsemana tsemana Fixes and adds regression test for GH-1892. Adds request spec for
save-and-continue editing.
52ef640
@ugisozols
Refinery member

Looking good.

@tsemana will you tackle preview bug we were talking about in irc?

@tsemana
tsemana commented Sep 15, 2012

Thanks, yes I will. It's weird in that the regression spec I wrote works passes even though it fails when done manually... Either way I'll give it a crack

@parndt
Refinery member
parndt commented Sep 18, 2012

Merged to 6d5c910 thanks!

@parndt parndt closed this Sep 18, 2012
@ugisozols ugisozols added a commit that referenced this pull request Sep 19, 2012
@ugisozols ugisozols Add note to changelog about #1948 and #1944. 7d3d33c
@ugisozols ugisozols added a commit that referenced this pull request Sep 19, 2012
@ugisozols ugisozols Add note to changelog about #1948 and #1944. 493e9aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.