Currently when a user adds a package to a new template, a temporary template gets created for use until the form is submitted. If the user cancels the form, the template still exists when it should not. This patch simply changes the create template logic to only create the template when the form is submitted
This patch now works both when javascript is enabled and disabled.
Also took care of some more edge cases since the last revision. --- src/app/controllers/templates_controller.rb | 128 ++++++++++++++--------- src/app/models/template.rb | 18 +-- src/app/util/image_descriptor_xml.rb | 4 + src/app/views/templates/_content_selection.haml | 52 --------- src/app/views/templates/_hidden_fields.haml | 8 ++ src/app/views/templates/_managed_content.haml | 16 +-- src/app/views/templates/add_software_form.haml | 1 - src/app/views/templates/content_selection.haml | 45 ++++++++ src/app/views/templates/managed_content.haml | 1 + src/app/views/templates/new.haml | 67 +++++++----- 10 files changed, 186 insertions(+), 154 deletions(-) delete mode 100644 src/app/views/templates/_content_selection.haml create mode 100644 src/app/views/templates/_hidden_fields.haml delete mode 100644 src/app/views/templates/add_software_form.haml create mode 100644 src/app/views/templates/content_selection.haml create mode 100644 src/app/views/templates/managed_content.haml
diff --git a/src/app/controllers/templates_controller.rb b/src/app/controllers/templates_controller.rb index 8d97c05..9c77c9b 100644 --- a/src/app/controllers/templates_controller.rb +++ b/src/app/controllers/templates_controller.rb @@ -38,71 +38,92 @@ class TemplatesController < ApplicationController end end
+ # Since the template form submission can mean multiple things, + # we dispatch based on form parameters here + def dispatch + if params[:save] + create + + elsif params[:cancel] + redirect_to :action => 'index' + + elsif params[:add_software_form] + content_selection + + elsif pkg = params.keys.find { |k| k =~ /^remove_package_(.*)$/ } + # actually remove the package from list + params[:packages].delete($1) if params[:packages] + new + + elsif params[:add_selected] + new + + elsif params[:cancel_add_software] + params[:packages] = params[:selected_packages] + new + + end + end + + # FIXME at some point split edit/update out from new/create + # to conform to web standards def new # can't use @template variable - is used by compass (or something other) - @tpl = Template.find_or_create(params[:id]) - @repository_manager = RepositoryManager.new - @groups = @repository_manager.all_groups(params[:repository]) + @id = params[:id] + @tpl = @id.nil? ? Template.new : Template.find(@id) + @tpl.attributes = params[:tpl] unless params[:tpl].nil? + get_selected_packages(@tpl) + render :action => :new end
def create - if params[:cancel] - redirect_to :action => 'index' - return - end - - @tpl = (params[:tpl] && !params[:tpl][:id].to_s.empty?) ? Template.find(params[:tpl][:id]) : Template.new(params[:tpl]) + @id = params[:tpl][:id] + @tpl = @id.nil? || @id == "" ? Template.new(params[:tpl]) : Template.find(@id)
- unless params[:add_software_form] and request.xhr? - # this is crazy, but we have most attrs in xml and also in model, - # synchronize it at first to xml - @tpl.update_xml_attributes!(params[:tpl]) - end + @tpl.xml.clear_packages
- # if remove pkg, we only update xml and render 'new' template - # again - params.keys.each do |param| - if param =~ /^remove_package_(.*)$/ - update_group_or_package(:remove_package, $1) - render :action => 'new' - return - end - end + params[:groups].to_a.each { |group| @tpl.xml.add_group(group) } + params[:packages].to_a.each { |pkg| @tpl.xml.add_package(pkg) }
- if params[:add_software_form] - @repository_manager = RepositoryManager.new - @groups = @repository_manager.all_groups_with_tagged_selected_packages(@tpl.xml.packages, params[:repository]) - render :action => 'add_software_form' - return - end + # this is crazy, but we have most attrs in xml and also in model, + # synchronize it at first to xml + @tpl.update_xml_attributes(params[:tpl])
if @tpl.save flash[:notice] = "Template saved." @tpl.set_complete redirect_to :action => 'index' else - @repository_manager = RepositoryManager.new - @groups = @repository_manager.all_groups(params[:repository]) + get_selected_packages(@tpl) render :action => 'new' end end
- def add_software - @tpl = params[:template_id].to_s.empty? ? Template.new : Template.find(params[:template_id]) + def content_selection @repository_manager = RepositoryManager.new - @groups = @repository_manager.all_groups(params[:repository]) - if params[:add_selected] - params[:groups].to_a.each { |group| @tpl.xml.add_group(group) } - params[:packages].to_a.each { |pkg| @tpl.xml.add_package(pkg) } - @tpl.save_xml! - end - if params[:ajax] - render :partial => 'managed_content' + @id = params[:id] if params[:id] + @tpl = @id.nil? ? Template.new : Template.find(@id) + @tpl.attributes = params[:tpl] unless params[:tpl].nil? + @packages = [] + @packages = params[:packages].collect{ |p| { :name => p } } if params[:packages] + @groups = @repository_manager.all_groups_with_tagged_selected_packages(@packages, params[:repository]) + @embed = params[:embed] + if @embed + render :layout => false else - render :action => 'new' + render :action => :content_selection end end
+ def managed_content + @repository_manager = RepositoryManager.new + @groups = @repository_manager.all_groups(params[:repository]) + @selected_packages = params[:selected_packages] || [] + @selected_packages.uniq! + @selected_groups = params[:selected_groups] || [] + render :layout => false + end + def build_form raise "select template to build" unless params[:image] and params[:image][:template_id] @image = Image.new(params[:image]) @@ -162,15 +183,6 @@ class TemplatesController < ApplicationController
private
- def update_group_or_package(method, *args) - @repository_manager = RepositoryManager.new - @groups = @repository_manager.all_groups(params[:repository]) - @tpl.xml.send(method, *args) - # we save template w/o validation (we can add package before name,... is - # set) - @tpl.save_xml! - end - def check_permission require_privilege(Privilege::IMAGE_MODIFY) end @@ -183,4 +195,18 @@ class TemplatesController < ApplicationController end return ids.first end + + def get_selected_packages(tpl) + @repository_manager = RepositoryManager.new + @groups = @repository_manager.all_groups(params[:repository]) + + if params[:packages] + @selected_packages = params[:packages] + @selected_packages.uniq! + else + @selected_packages = tpl.xml.packages.collect { |p| p[:name] } + end + + @selected_groups = [] + end end diff --git a/src/app/models/template.rb b/src/app/models/template.rb index 6457f7a..2e76a53 100644 --- a/src/app/models/template.rb +++ b/src/app/models/template.rb @@ -18,21 +18,15 @@ class Template < ActiveRecord::Base validates_presence_of :platform_version validates_presence_of :architecture
- def update_xml_attributes!(opts = {}) - doc = xml - doc.name = opts[:name] if opts[:name] - doc.platform = opts[:platform] if opts[:platform] - doc.description = opts[:summary] if opts[:summary] - doc.platform_version = opts[:platform_version] if opts[:platform_version] - doc.architecture = opts[:architecture] if opts[:architecture] - save_xml! - end - - def save_xml! + def update_xml_attributes(opts = {}) + xml.name = opts[:name] if opts[:name] + xml.platform = opts[:platform] if opts[:platform] + xml.description = opts[:summary] if opts[:summary] + xml.platform_version = opts[:platform_version] if opts[:platform_version] + xml.architecture = opts[:architecture] if opts[:architecture] self[:xml] = xml.to_xml @xml = nil update_attrs - save_without_validation! end
def xml diff --git a/src/app/util/image_descriptor_xml.rb b/src/app/util/image_descriptor_xml.rb index e8fe552..034d943 100644 --- a/src/app/util/image_descriptor_xml.rb +++ b/src/app/util/image_descriptor_xml.rb @@ -141,6 +141,10 @@ class ImageDescriptorXML end end
+ def clear_packages + @root.xpath('/image/packages').each { |s| s.remove } + end + private
def add_group_node(parent, group) diff --git a/src/app/views/templates/_content_selection.haml b/src/app/views/templates/_content_selection.haml deleted file mode 100644 index 2372a4a..0000000 --- a/src/app/views/templates/_content_selection.haml +++ /dev/null @@ -1,52 +0,0 @@ -.grid_16 - %h3 Managed Content Selection - - - form_tag :action => "add_software" do - %fieldset.clearfix - .search.grid_4.alpha - %input{:type => "search", :placeholder => "Search for package", :disabled => "disabled"} - %button.action - = hidden_field_tag :template_id, @tpl.id - .grid_8 - %p - Repositories to Search: - %a (Check all) - %fieldset - - @repository_manager.repositories.each do |repo| - = check_box_tag 'repositories[]', repo.id, true - = label_tag 'repositories[]', repo.name - %a.grid_4.omega Advanced Search - -#%fieldset.clearfix - -# .grid_3.alpha - -# = radio_button_tag :show_mode, 'group', true - -# = label_tag :show_mode, 'Show by Group' - -# .grid_2.omega - -# = radio_button_tag :show_mode, 'type', false, :disabled => true - -# = label_tag :show_mode, 'Show by Type' - - -#%fieldset.clearfix - -# = submit_tag "Add Selected", :name => "add_selected", :class => "grid_2 alpha", :id => "do_add_software" - -# = submit_tag "Cancel", :name => "cancel", :class => "grid_2", :id => "cancel_add_software" - %ul.softwaregroups - - groups = @groups.keys.sort - - unsorted = groups.delete('unsorted') - - groups.push('unsorted') if params[:show_unsorted] and unsorted - - groups.each do |group| - - group_sel = @groups[group][:selected] - - group_id = group.gsub(/\s/, '_') - %li - = check_box_tag 'groups[]', group, group_sel, :disabled => group_sel, :id => "group_#{group_id}" - = label_tag "group_#{group_id}", group - %ul{:class => "packages group_#{group_id}"} - - pkgs = @groups[group][:packages] - - pkgs.keys.sort.each do |pkg| - - pkg_sel = pkgs[pkg][:selected] ? true : false - - pkg_id = pkg.gsub(/\s/, '_') - %li - = check_box_tag 'packages[]', pkg, pkg_sel, :disabled => pkg_sel, :id => "package_#{pkg_id}" - = label_tag "package_#{pkg_id}", pkg - - = link_to "Show unsorted packages", {:action => 'create', :add_software_form => true, :show_unsorted => true, 'tpl[id]' => @tpl.id}, {:id => 'switch_all_link'} unless params[:show_unsorted] - %fieldset.clearfix - = submit_tag "Add Selected", :name => "add_selected", :class => "grid_2 alpha", :id => "do_add_software" - = submit_tag "Cancel", :name => "cancel", :class => "grid_2", :id => "cancel_add_software" diff --git a/src/app/views/templates/_hidden_fields.haml b/src/app/views/templates/_hidden_fields.haml new file mode 100644 index 0000000..3fcab65 --- /dev/null +++ b/src/app/views/templates/_hidden_fields.haml @@ -0,0 +1,8 @@ +- # the template fields as hidden form inputs +<input type="hidden" id="id" name="id" value="#{@id}" /> += hidden_field :tpl, :id += hidden_field :tpl, :name += hidden_field :tpl, :summary += hidden_field :tpl, :platform += hidden_field :tpl, :platform_version += hidden_field :tpl, :architecture diff --git a/src/app/views/templates/_managed_content.haml b/src/app/views/templates/_managed_content.haml index f6c8f7b..97c4046 100644 --- a/src/app/views/templates/_managed_content.haml +++ b/src/app/views/templates/_managed_content.haml @@ -1,26 +1,20 @@ #selected_packages - / we place template id into this partial, because - / if we want to add software with ajax, it's possible - / that template is not saved in db yet -> in this case - / template is saved when software is added and we have - / to pass template id back to new form - = hidden_field :tpl, :id %h3.gap Managed Content to Bundle %hr %label.header.alpha.prefix_2.grid_7 Name: %label.header.omega.grid_2.suffix_5 Repository: %label.grid_2.alpha.clear Managed: .grid_14.omega - - if @tpl.xml.packages.empty? + - if @selected_packages.empty? No selected packages - else - repos = @repository_manager.repositories_hash - - @tpl.xml.packages.each do |pkg| - - pkg_group = @groups.keys.find {|g| @groups[g][:packages][pkg[:name]]} + - @selected_packages.each do |pkg| + - pkg_group = @groups.keys.find {|g| @groups[g][:packages][pkg]} %fieldset.clearfix - = text_field_tag 'packages[]', pkg[:name], :disabled => true, :id => "selected_package_#{pkg[:name]}", :class => "alpha grid_7 packagename" + = text_field_tag 'packages[]', pkg, :id => "selected_package_#{pkg}", :class => "alpha grid_7 packagename" .grid_2= (pkg_group and repo = repos[@groups[pkg_group][:repository_id]]) ? repo.name.to_s : ' ' .grid_5.omega %button{:type => 'button', :disabled => 'disabled'} Config %button{:type => 'button', :disabled => 'disabled'} Metadata - = submit_tag "Remove", :name => "remove_package_#{pkg[:name]}", :id => "remove_package_#{pkg[:name]}" + = submit_tag "Remove", :name => "remove_package_#{pkg}", :id => "remove_package_#{pkg}", :class => 'remove_package' diff --git a/src/app/views/templates/add_software_form.haml b/src/app/views/templates/add_software_form.haml deleted file mode 100644 index d32729a..0000000 --- a/src/app/views/templates/add_software_form.haml +++ /dev/null @@ -1 +0,0 @@ -= render :partial => 'content_selection' diff --git a/src/app/views/templates/content_selection.haml b/src/app/views/templates/content_selection.haml new file mode 100644 index 0000000..ddd104e --- /dev/null +++ b/src/app/views/templates/content_selection.haml @@ -0,0 +1,45 @@ +.grid_16 + %h3 Managed Content Selection + + - form_tag :action => "dispatch" do + - unless @embed + = render :partial => 'hidden_fields' + - @packages.each do |p| + <input type="hidden" id="selected_package_#{p[:name]}" name="selected_packages[]" value="#{p[:name]}" /> + %fieldset.clearfix + .search.grid_4.alpha + %input{:type => "search", :placeholder => "Search for package", :disabled => "disabled"} + %button.action + .grid_8 + %p + Repositories to Search: + %a (Check all) + %fieldset + - @repository_manager.repositories.each do |repo| + = check_box_tag 'repositories[]', repo.id, true + = label_tag 'repositories[]', repo.name + %a.grid_4.omega Advanced Search + + %ul.softwaregroups + - groups = @groups.keys.sort + - unsorted = groups.delete('unsorted') + - groups.push('unsorted') if params[:show_unsorted] and unsorted + - groups.each do |group| + - group_sel = @groups[group][:selected] + - group_id = group.gsub(/\s/, '_') + %li + = check_box_tag 'groups[]', group, group_sel, :id => "group_#{group_id}" + = label_tag "group_#{group_id}", group + %ul{:class => "packages group_#{group_id}"} + - pkgs = @groups[group][:packages] + - pkgs.keys.sort.each do |pkg| + - pkg_sel = pkgs[pkg][:selected] ? true : false + - pkg_id = pkg.gsub(/\s/, '_') + %li + = check_box_tag 'packages[]', pkg, pkg_sel, :id => "package_#{pkg_id}" + = label_tag "package_#{pkg_id}", pkg + = link_to "Show unsorted packages", {:action => 'create', :add_software_form => true, :show_unsorted => true, 'tpl[id]' => @tpl.id}, {:id => 'switch_all_link'} unless params[:show_unsorted] + + %fieldset.clearfix + = submit_tag "Add Selected", :name => "add_selected", :class => "grid_2 alpha", :id => "do_add_software" + = submit_tag "Cancel", :name => "cancel_add_software", :class => "grid_2", :id => "cancel_add_software" diff --git a/src/app/views/templates/managed_content.haml b/src/app/views/templates/managed_content.haml new file mode 100644 index 0000000..926142d --- /dev/null +++ b/src/app/views/templates/managed_content.haml @@ -0,0 +1 @@ += render :partial => 'managed_content' diff --git a/src/app/views/templates/new.haml b/src/app/views/templates/new.haml index 42fc85d..d255c7d 100644 --- a/src/app/views/templates/new.haml +++ b/src/app/views/templates/new.haml @@ -1,38 +1,45 @@ :javascript $(document).ready(function() { - var $container = $('#package_selection_list'), - $submit = $('#add_software_button'); - $submit.click(function(e, show_all) { - var list_url = '#{url_for :action => 'create', :add_software_form => true}'; - var list_all_url = '#{url_for :action => 'create', :add_software_form => true, :show_unsorted => true}'; - var list_data = {'tpl[id]': $("input[name='tpl[id]']").val() || '', ajax: true}; + var $content_container = $('#managed_content'); + var $sel_pkg_container = $('#package_selection_list'); + var $submit = $('#add_software_button'); + $submit.click(function(e) { e.preventDefault(); - $(this).hide(); - $container.empty().show().addClass('loading'); - $container.load(show_all ? list_all_url : list_url, list_data, function() { - $container.removeClass('loading'); + $submit.hide(); + var data = { + 'packages[]': $("input[name='packages[]']").map(function() {return $(this).val()}).get() + } + var url = '#{url_for :action => 'content_selection', :embed => true, :id => @id}'; + $content_container.empty().show(); + $sel_pkg_container.empty().show().addClass('loading'); + $sel_pkg_container.load(url, data, function(){ + $sel_pkg_container.removeClass('loading');; $('#do_add_software').click(function(e) { - var url = '#{url_for :action => 'add_software', :ajax => true, :add_selected => true}'; + e.preventDefault(); + var url = '#{url_for :action => 'managed_content'}'; var data = { - 'packages[]': $("input:checked[name='packages[]']").map(function() {return $(this).val()}).get(), - 'groups[]': $("input:checked[name='groups[]']").map(function() {return $(this).val()}).get(), - 'template_id': $("input[name='tpl[id]']").val() || '' + 'selected_packages[]': $("input:checked[name='packages[]']").map(function() {return $(this).val()}).get(), + 'selected_groups[]': $("input:checked[name='groups[]']").map(function() {return $(this).val()}).get(), + 'template_id' : '#{@id.nil? ? nil : @id}' }; - e.preventDefault(); - $(this).replaceWith('<span class="loading grid_2 alpha">Adding Packages</span>'); - $('#selected_packages').load(url, data, function() { - $container.hide(); + $content_container.load(url, data, function(){ + $sel_pkg_container.empty().show(); $submit.show(); + $('.remove_package').click(function() { $(this).parent().parent().remove(); }); }); }); - $('#switch_all_link').click(function(e) { - e.preventDefault(); - $submit.trigger('click', [true]); - }); $('#cancel_add_software').click(function(e) { e.preventDefault(); - $container.hide(); - $submit.show(); + var data = { + 'selected_packages[]': $("input[name='selected_packages[]']").map(function() {return $(this).val()}).get(), + 'selected_groups[]': $("input:checked[name='groups[]']").map(function() {return $(this).val()}).get(), + 'template_id' : '#{@id.nil? ? nil : @id}' + } + var url = '#{url_for :action => 'managed_content'}'; + $content_container.load(url, data, function(){ + $sel_pkg_container.empty().show(); + $submit.show(); + }); }); $(".softwaregroups input[type='checkbox']").click(function() { if ($(this).attr("checked") === true) { @@ -43,12 +50,15 @@ }); }); }); + $('.remove_package').click(function() { $(this).parent().parent().remove(); }); });
.grid_16 %h2 Template - - form_for @tpl, :url => { :action => "create" } do - = error_messages_for 'tpl' + - form_for @tpl, :url => { :action => "dispatch" } do + <input type="hidden" id="id" name="id" value="#{@id}" /> + = hidden_field :tpl, :id + = error_messages_for :tpl = render :partial => 'basics'
%h3.disabled.gap Local Content to Bundle @@ -74,12 +84,15 @@ %a{:href => '#'} Remove )
- = render :partial => 'managed_content' + #managed_content + = render :partial => 'managed_content' .clearfix .grid_14.alpha.prefix_2 = submit_tag "Add Software", :name => "add_software_form", :id => "add_software_button", :class => "iconbutton"
+ #package_selection_list{:style => 'display: none'} + %h3.gap.clear Preboot Configuration %hr %fieldset.clearfix