[PATCH] Deploaybles and catalog_entries cleanups

jprovazn at redhat.com jprovazn at redhat.com
Thu Dec 1 14:22:40 UTC 2011


From: Jan Provaznik <jprovazn at redhat.com>

Hi, after yesterday's renaming of catalog_entries and deployables there is
still mess in current views and controllers - this should be cleaned up before building
rpms. I started with it, but apparently won't be able to finish this today.

This patch is in the middle of renaming and cleaning, if you have time, feel free to
continue on this.

Jan
---
 src/app/controllers/deployables_controller.rb      |  116 ++++++++++----------
 src/app/views/deployables/_form.html.haml          |   35 +++---
 src/app/views/deployables/_from_url.html.haml      |   21 ++--
 src/app/views/deployables/_list.html.haml          |   10 +-
 .../views/deployables/_new_from_image.html.haml    |   14 +-
 src/app/views/deployables/_upload.html.haml        |   29 +++---
 src/app/views/deployables/show.html.haml           |   10 +-
 7 files changed, 116 insertions(+), 119 deletions(-)

diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
index c5e7a4e..46d48cf 100644
--- a/src/app/controllers/deployables_controller.rb
+++ b/src/app/controllers/deployables_controller.rb
@@ -26,18 +26,18 @@ class DeployablesController < ApplicationController
     @deployables = Deployable.list_for_user(current_user, Privilege::VIEW)
     @catalog_entries = @deployables.collect { |d| d.catalog_entries.first }
     #@catalog_entries = CatalogEntry.list_for_user(current_user, Privilege::VIEW).apply_filters(:preset_filter_id => params[:catalog_entries_preset_filter], :search_filter => params[:catalog_entries_search])
-    @catalog = @catalog_entries.first.catalog unless @catalog_entries.empty?
+    @catalog = Catalog.find(params[:catalog_id]) #@catalog_entries.first.catalog unless @catalog_entries.empty?
     set_header
   end
 
   def new
-    @catalog_entry = params[:catalog_entry].nil? ? CatalogEntry.new() : CatalogEntry.new(params[:catalog_entry])
-    @catalog_entry.deployable = Deployable.new unless @catalog_entry.deployable
+    @deployable = Deployable.new(params[:deployable])
     require_privilege(Privilege::CREATE, Deployable)
     if params[:create_from_image]
       @image = Aeolus::Image::Warehouse::Image.find(params[:create_from_image])
       @hw_profiles = HardwareProfile.frontend.list_for_user(current_user, Privilege::VIEW)
-      @catalog_entry.deployable.name = @image.name
+      @deployable.name = @image.name
+      @selected_catalogs = params[:catalog_id].to_a
       load_catalogs
     else
       @catalog = Catalog.find(params[:catalog_id])
@@ -51,13 +51,14 @@ class DeployablesController < ApplicationController
   end
 
   def show
-    @catalog_entry = CatalogEntry.find(params[:id])
-    require_privilege(Privilege::VIEW, @catalog_entry.deployable)
-    save_breadcrumb(catalog_deployable_path(@catalog_entry.catalog, @catalog_entry), @catalog_entry.deployable.name)
+    @deployable = Deployable.find(params[:id])
+    @catalog = params[:catalog_id]
+    require_privilege(Privilege::VIEW, @deployable)
+    save_breadcrumb(catalog_deployable_path(@catalog, @deployable), @deployable.name)
     @providers = Provider.all
-    @catalogs_options = Catalog.all.map {|c| [c.name, c.id] unless c == @catalog_entry.catalog}.compact
-    add_permissions_inline(@catalog_entry.deployable)
-    @image_details = @catalog_entry.deployable.get_image_details
+    @catalogs_options = Catalog.all.map {|c| [c.name, c.id] unless c == @catalog}.compact
+    add_permissions_inline(@deployable)
+    @image_details = @deployable.get_image_details
     @image_details.each do |assembly|
       assembly.keys.each do |key|
         flash[:error] = assembly[key] if key.to_s =~ /^error\w+/
@@ -71,69 +72,71 @@ class DeployablesController < ApplicationController
       return
     end
 
-
-    @catalog_entry = CatalogEntry.new(params[:catalog_entry])
-    if params[:create_from_image].present?
-      @catalog = @catalog_entry.catalog
-      @catalog_entry.deployable = Deployable.new unless @catalog_entry.deployable
-    else
-      @catalog = Catalog.find(params[:catalog_id])
-      @catalog_entry.catalog = @catalog
-    end
-    require_privilege(Privilege::MODIFY, @catalog)
     require_privilege(Privilege::CREATE, Deployable)
-    @catalog_entry.deployable.owner = current_user
+    @deployable = Deployable.new(params[:deployable])
+    @selected_catalogs = params[:catalog_id].to_a
+    @deployable.owner = current_user
 
     if params.has_key? :url
         xml = import_xml_from_url(params[:url])
         unless xml.nil?
           #store xml_filename for url (i.e. url ends to: foo || foo.xml)
-          @catalog_entry.deployable.xml_filename =  File.basename(URI.parse(params[:url]).path)
-          @catalog_entry.deployable.xml = xml
+          @deployable.xml_filename =  File.basename(URI.parse(params[:url]).path)
+          @deployable.xml = xml
         end
     elsif params[:create_from_image].present?
       hw_profile = HardwareProfile.frontend.find(params[:hardware_profile])
       require_privilege(Privilege::VIEW, hw_profile)
-      @catalog_entry.deployable.set_from_image(params[:create_from_image], hw_profile)
+      @deployable.set_from_image(params[:create_from_image], hw_profile)
     end
 
-    if @catalog_entry.save
-      flash[:notice] = t "catalog_entries.flash.notice.added"
-      if params[:edit_xml]
-        redirect_to edit_catalog_deployable_path @catalog_entry.catalog.id, @catalog_entry.id, :edit_xml =>true
-      else
-        redirect_to catalog_deployables_path(@catalog)
+    begin
+      @deployable.transaction do
+        raise t("catalog_entries.flash.error.no_catalog") if @selected_catalogs.empty?
+        @deployable.save!
+        @selected_catalogs.each do |cid|
+          catalog = Catalog.find(cid)
+          require_privilege(Privilege::MODIFY, catalog)
+          CatalogEntry.create!(:catalog_id => cid, :deploayble_id => @deployable.id)
+        end
+        flash[:notice] = t "catalog_entries.flash.notice.added"
+        if params[:edit_xml]
+          redirect_to edit_catalog_deployable_path @selected_catalogs.first.id, @deployable.id, :edit_xml =>true
+        else
+          redirect_to catalog_deployables_path(@selected_catalogs.first)
+        end
       end
-    else
-      flash[:warning]= t('catalog_entries.flash.warning.not_valid') if @catalog_entry.errors.has_key?(:xml)
+    rescue => e
+      flash[:warning]= t('catalog_entries.flash.warning.create_failed', :message => e.message)
+      flash[:warning]= t('catalog_entries.flash.warning.not_valid') if @deployable.errors.has_key?(:xml)
       if params[:create_from_image].present?
         load_catalogs
         @image = Aeolus::Image::Warehouse::Image.find(params[:create_from_image])
         @hw_profiles = HardwareProfile.frontend.list_for_user(current_user, Privilege::VIEW)
-        @catalog_entry.deployable.name = @image.name
+        @deployable.name = @image.name
       else
+        @catalog = Catalog.find(@selected_catalogs.first)
         params.delete(:edit_xml) if params[:edit_xml]
-        @form_option = params[:catalog_entry].has_key?(:xml) ? 'upload' : 'from_url'
-        @form_option = params[:catalog_entry][:deployable].has_key?(:xml) ? 'upload' : 'from_url'
+        @form_option = params[:deployable].has_key?(:xml) ? 'upload' : 'from_url'
       end
       render :new
     end
   end
 
   def edit
-    @catalog_entry = CatalogEntry.find(params[:id])
-    require_privilege(Privilege::MODIFY, @catalog_entry.deployable)
-    @catalog = @catalog_entry.catalog
+    @deployable = Deployable.find(params[:id])
+    require_privilege(Privilege::MODIFY, @deployable)
+    @catalog = params[:catalog_id]
   end
 
   def update
-    @catalog_entry = CatalogEntry.find(params[:id])
-    require_privilege(Privilege::MODIFY, @catalog_entry.deployable)
-    params[:catalog_entry][:deployable].delete(:owner_id) if params[:catalog_entry] and params[:catalog_entry][:deployable]
+    @deployable = Deployable.find(params[:id])
+    require_privilege(Privilege::MODIFY, @deployable)
+    params[:deployable].delete(:owner_id) if params[:deployable]
 
-    if @catalog_entry.update_attributes(params[:catalog_entry])
+    if @deployable.update_attributes(params[:deployable])
       flash[:notice] = t"catalog_entries.flash.notice.updated"
-      redirect_to catalog_deployable_path(@catalog_entry.catalog, @catalog_entry)
+      redirect_to catalog_deployable_path(params[:catalog_id], @deployable)
     else
       render :action => 'edit'
     end
@@ -141,28 +144,25 @@ class DeployablesController < ApplicationController
 
   def multi_destroy
     @catalog = nil
-    CatalogEntry.find(params[:catalog_entries_selected]).to_a.each do |d|
-      require_privilege(Privilege::MODIFY, d.catalog)
-      require_privilege(Privilege::MODIFY, d.deployable)
-      @catalog = d.catalog
-      # Don't do this when we're managing deployables independently
-      d.deployable.destroy
+    Deployable.find(params[:deployables_selected]).to_a.each do |d|
+      # TODO: delete only in catalogs where I have permission to
+      #require_privilege(Privilege::MODIFY, d.catalog)
+      require_privilege(Privilege::MODIFY, d)
+      #@catalog = d.catalog
       d.destroy
     end
-    redirect_to catalog_path(@catalog)
+    redirect_to catalog_path(params[:catalog_id])
   end
 
   def destroy
-    catalog_entry = CatalogEntry.find(params[:id])
-    require_privilege(Privilege::MODIFY, catalog_entry.catalog)
-    require_privilege(Privilege::MODIFY, catalog_entry.deployable)
-    @catalog = catalog_entry.catalog
-    # Don't do this when we're managing deployables independently
-    catalog_entry.deployable.destroy
-    catalog_entry.destroy
+    deployable = Deployable.find(params[:id])
+    # TODO: delete only in catalogs where I have permission to
+    #require_privilege(Privilege::MODIFY, catalog_entry.catalog)
+    require_privilege(Privilege::MODIFY, deployable)
+    destroy
 
     respond_to do |format|
-      format.html { redirect_to catalog_path(@catalog) }
+      format.html { redirect_to catalog_path(params[:catalog_id]) }
     end
   end
 
diff --git a/src/app/views/deployables/_form.html.haml b/src/app/views/deployables/_form.html.haml
index d638684..c53eb61 100644
--- a/src/app/views/deployables/_form.html.haml
+++ b/src/app/views/deployables/_form.html.haml
@@ -1,26 +1,25 @@
-- if @catalog_entry.errors.any?
-  = render 'layouts/error_messages', :object => @catalog_entry
+- if @deployable.errors.any?
+  = render 'layouts/error_messages', :object => @deployable
 %fieldset
+  = form_for @deployable do |form|
   - if params[:edit_xml]
-    = form.fields_for :deployable, @catalog_entry.deployable do |deployable_fields|
-      %p
-        = deployable_fields.label :xml, t('.deployable_xml')
-        = deployable_fields.text_area :xml, :class => 'long'
+    %p
+      = form.label :xml, t('.deployable_xml')
+      = form.text_area :xml, :class => 'long'
   - else
     %p
-      = form.label :catalog_id, t('catalog_entries.form.catalog')
-      = @catalog ? @catalog.name : @catalog_entry.catalog.name
+      = label_tag :catalog_id, t('catalog_entries.form.catalog')
+      = @catalog.name
       .clear
-    = form.fields_for :deployable, @catalog_entry.deployable do |deployable_fields|
-      %p
-        = deployable_fields.label :name, t('catalog_entries.index.name')
-        = deployable_fields.text_field :name, :class => 'em long'
-      %p
-        = deployable_fields.label :description, t('.description')
-        = deployable_fields.text_area :description, :class => 'long'
-      %p
-        = deployable_fields.label :xml, t('.deployable_xml')
-        = link_to t("catalog_entries.properties.edit_deployable_xml"), edit_catalog_deployable_path(@catalog_entry.catalog.id, @catalog_entry.id, :edit_xml=>true)
+    %p
+      = form.label :name, t('catalog_entries.index.name')
+      = form.text_field :name, :class => 'em long'
+    %p
+      = form.label :description, t('.description')
+      = form.text_area :description, :class => 'long'
+    %p
+      = form.label :xml, t('.deployable_xml')
+      = link_to t("catalog_entries.properties.edit_deployable_xml"), edit_catalog_deployable_path(@catalog.id, @deployable.id, :edit_xml=>true)
 
 %fieldset.options
   = link_to t('cancel'), catalog_path(@catalog), :class => 'button danger'
diff --git a/src/app/views/deployables/_from_url.html.haml b/src/app/views/deployables/_from_url.html.haml
index 22b0099..d351ecb 100644
--- a/src/app/views/deployables/_from_url.html.haml
+++ b/src/app/views/deployables/_from_url.html.haml
@@ -1,19 +1,18 @@
-= form_for @catalog_entry, :url => catalog_deployables_path(@catalog), :html => {:class => 'generic', :multipart => true } do |form|
-  - if @catalog_entry.errors.any?
-    = render 'layouts/error_messages', :object => @catalog_entry
+= form_for @deployable, :url => catalog_deployables_path(@catalog), :html => {:class => 'generic', :multipart => true } do |form|
+  - if @deployable.errors.any?
+    = render 'layouts/error_messages', :object => @deployable
   %fieldset
     %p
       = form.label :catalog_id, t('catalog_entries.form.catalog')
       %span.text_instead_input= @catalog.name
       .clear
-  =form.fields_for :deployable, @catalog_entry.deployable do |deployable_fields|
-    %fieldset
-      %p
-        = deployable_fields.label :name, t('catalog_entries.index.name')
-        = deployable_fields.text_field :name, :class => 'em long'
-      %p
-        = deployable_fields.label :description, t('catalog_entries.form.description')
-        = deployable_fields.text_area :description, :class => 'long'
+  %fieldset
+    %p
+      = form.label :name, t('catalog_entries.index.name')
+      = form.text_field :name, :class => 'em long'
+    %p
+      = form.label :description, t('catalog_entries.form.description')
+      = form.text_area :description, :class => 'long'
   %fieldset
     %p
       = label_tag :url, t('catalog_entries.form.deployable_xml_url')
diff --git a/src/app/views/deployables/_list.html.haml b/src/app/views/deployables/_list.html.haml
index 117e636..350ad4f 100644
--- a/src/app/views/deployables/_list.html.haml
+++ b/src/app/views/deployables/_list.html.haml
@@ -25,14 +25,14 @@
           });
         });
 
-    = filter_table(@header, @catalog_entries) do |catalog_entry|
+    = filter_table(@header, @deployables) do |deployable|
       %tr{:class => cycle('nostripe','stripe')}
         %td{:class => 'checkbox'}
           - selected = params[:select] == 'all'
-          = check_box_tag "catalog_entries_selected[]", catalog_entry.id, selected, :id => "catalog_entry_checkbox_#{catalog_entry.id}"
+          = check_box_tag "deployables_selected[]", deployable.id, selected, :id => "deployable_checkbox_#{deployable.id}"
         %td
-          = link_to catalog_entry.deployable.name, catalog_deployable_path(@catalog, catalog_entry.id)
+          = link_to deployable.name, catalog_deployable_path(@catalog, deployable.id)
         %td
-          = link_to catalog_entry.catalog.name, catalog_entry.catalog
+          = link_to @catalog.name, @catalog
         %td
-          = link_to catalog_entry.deployable.xml_filename, catalog_deployable_path(@catalog, catalog_entry.id)
+          = link_to deployable.xml_filename, catalog_deployable_path(@catalog, deployable.id)
diff --git a/src/app/views/deployables/_new_from_image.html.haml b/src/app/views/deployables/_new_from_image.html.haml
index f546698..95ffc7d 100644
--- a/src/app/views/deployables/_new_from_image.html.haml
+++ b/src/app/views/deployables/_new_from_image.html.haml
@@ -1,12 +1,12 @@
 %header.admin-page-header
-  %h1.deployables= t('.catalog_entry_name', :name => @catalog_entry.deployable.name)
+  %h1.deployables= t('.catalog_entry_name', :name => @deployable.name)
 
 %section.admin-content-section
   .content.buildpush
-    = form_for @catalog_entry, :html => {:class => 'generic'} do |form|
+    = form_for @deployable, :html => {:class => 'generic'} do |form|
       = hidden_field_tag :create_from_image, params[:create_from_image]
-      - if @catalog_entry.errors.any?
-        = render 'layouts/error_messages', :object => @catalog_entry
+      - if @deployable.errors.any?
+        = render 'layouts/error_messages', :object => @deployable
       .lefted.new-catalog-entry-from-image
         %h2= t('.title')
         %p= t('.description', :name => @image.name)
@@ -17,8 +17,8 @@
             .clear
           %div
             = form.label :catalog_id, t('catalog_entries.form.catalog')
-            = form.select :catalog_id, @catalogs.collect{|c| [c.name, c.id]}
-            %span.text_instead_input= "Catalog1"
+            /= form.select :catalog_id, @catalogs.collect{|c| [c.name, c.id]}
+            %span.text_instead_input= @selected_catalogs.empty? ? t('deployables.form.none') : @selected_catalogs.count > 1 ? t('deployables.form.multiple') : @selected_catalogs.first
             %span.catalog_link
               &nbsp
               %div.catalog_list
@@ -26,7 +26,7 @@
                   - @catalogs.each do |catalog|
                     %li.catalog
                       .catalog_with_checkbox
-                        = check_box_tag :catalog_ids
+                        = check_box_tag :catalog_id, @selected_catalogs.include?(catalog.id)
                         = catalog.name
                         .clear
             .clear
diff --git a/src/app/views/deployables/_upload.html.haml b/src/app/views/deployables/_upload.html.haml
index 475d492..6afaa8f 100644
--- a/src/app/views/deployables/_upload.html.haml
+++ b/src/app/views/deployables/_upload.html.haml
@@ -1,22 +1,21 @@
-= form_for @catalog_entry, :url => catalog_deployables_path(@catalog), :html => {:class => 'generic', :multipart => true } do |form|
-  - if @catalog_entry.errors.any?
-    = render 'layouts/error_messages', :object => @catalog_entry
+= form_for @deployable, :url => catalog_deployables_path(@catalog), :html => {:class => 'generic', :multipart => true } do |form|
+  - if @deployable.errors.any?
+    = render 'layouts/error_messages', :object => @deployable
   %fieldset
     %p
-      = form.label :catalog_id, t('catalog_entries.form.catalog')
+      = label_tag :catalog_id, t('catalog_entries.form.catalog')
       %span.text_instead_input= @catalog.name
       .clear
-  = form.fields_for :deployable, @catalog_entry.deployable do |deployable_fields|
-    %fieldset
-      %p
-        = deployable_fields.label :name, t('catalog_entries.index.name')
-        = deployable_fields.text_field :name, :class => 'em long'
-      %p
-        = deployable_fields.label :description, t('catalog_entries.form.description')
-        = deployable_fields.text_area :description, :class => 'long'
-      %p
-        = deployable_fields.label :xml, t('catalog_entries.form.choose_xml')
-        = deployable_fields.file_field :xml
+  %fieldset
+    %p
+      = form.label :name, t('catalog_entries.index.name')
+      = form.text_field :name, :class => 'em long'
+    %p
+      = form.label :description, t('catalog_entries.form.description')
+      = form.text_area :description, :class => 'long'
+    %p
+      = form.label :xml, t('catalog_entries.form.choose_xml')
+      = form.file_field :xml
   %fieldset
     %p
       = check_box_tag(:edit_xml, true, false)
diff --git a/src/app/views/deployables/show.html.haml b/src/app/views/deployables/show.html.haml
index 1eab46e..fa5ec42 100644
--- a/src/app/views/deployables/show.html.haml
+++ b/src/app/views/deployables/show.html.haml
@@ -1,12 +1,12 @@
 = render :partial => 'layouts/admin_nav'
 %header.admin-page-header
-  %h1{:class => controller.controller_name}= @catalog_entry.deployable.name
+  %h1{:class => controller.controller_name}= @deployable.name
   #obj_actions.button-container
     .button-group
-      = button_to t('catalog_entries.show.delete'), catalog_deployable_path(@catalog_entry.catalog, @catalog_entry), :method => 'delete', :confirm => "#{t'catalog_entries.show.confirm_delete'}", :class => 'button danger', :id => 'delete'
-      = link_to t('catalog_entries.show.edit'), edit_catalog_deployable_path(@catalog_entry.catalog, @catalog_entry), :class => 'button', :id => 'edit_button'
+      = button_to t('catalog_entries.show.delete'), catalog_deployable_path(@catalog, @deployable), :method => 'delete', :confirm => "#{t'catalog_entries.show.confirm_delete'}", :class => 'button danger', :id => 'delete'
+      = link_to t('catalog_entries.show.edit'), edit_catalog_deployable_path(@catalog, @deployable), :class => 'button', :id => 'edit_button'
 
-    = link_to t('deployments.overview.launch'), launch_new_deployments_path(:deployable_id => @catalog_entry.deployable.id, :pool_id => @catalog_entry.catalog.pool), :id => :launch_deployment
+    = link_to t('deployments.overview.launch'), launch_new_deployments_path(:deployable_id => @deployable.id, :pool_id => @catalog.pool), :id => :launch_deployment
 
 %section.admin-content-section.deployable
   %section.image
@@ -61,7 +61,7 @@
       %p= t('.choose_catalog')
       = form_for(:catalog_entry, :url => catalog_entries_path) do |f|
         = f.select :catalog_id, options_for_select(@catalogs_options)
-        = f.hidden_field(:deployable_id, :value => @catalog_entry.deployable.id)
+        = f.hidden_field(:deployable_id, :value => @deployable.id)
         = f.submit t('catalog_entries.show.add_catalog'), :id => :add_catalog
 
       %table
-- 
1.7.6.4




More information about the aeolus-devel mailing list