On 08/23/2012 05:54 PM, [email protected] wrote:
From: Jozef Zigmund <[email protected]>

Moved the method 'import_xml_from_url' from DeployablesController
to ApplicationController, with fix for "undefined method `request_uri'".
ApplicationController's one is applied for ImagesController#edit_xml also.

https://bugzilla.redhat.com/show_bug.cgi?id=811852
---
  src/app/controllers/application_controller.rb | 20 ++++++++++++++++++++
  src/app/controllers/deployables_controller.rb | 18 ------------------
  src/app/controllers/images_controller.rb      | 24 ++++++++++++++----------
  src/app/views/images/new.html.haml            |  7 +++++--
  src/config/locales/en.yml                     |  8 +++++---
  5 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/src/app/controllers/application_controller.rb 
b/src/app/controllers/application_controller.rb
index 2ce15ab..3fd1884 100644
--- a/src/app/controllers/application_controller.rb
+++ b/src/app/controllers/application_controller.rb
@@ -146,6 +146,26 @@ class ApplicationController < ActionController::Base
      end
    end

+  def import_xml_from_url(url)
+    if url.blank?
+      flash[:error] = t('application_controller.flash.error.no_url_provided')
+    elsif not url =~ URI::regexp
+      flash[:error] = t('application_controller.flash.error.not_valid_url', :url 
=> url)
+    else
+      begin
+        response = RestClient.get(url, :accept => :xml)
+        if response.code == 200
+          return response
+        else
+          flash[:error] = 
t('application_controller.flash.error.download_failed')
+        end
+      rescue RestClient::Exception, SocketError, URI::InvalidURIError, 
Errno::ECONNREFUSED, Errno::ETIMEDOUT
+        flash[:error] = 
t('application_controller.flash.error.not_valid_or_reachable', :url => url)
+      end
+    end
+    return nil
+  end
+
    private
    def json_error_hash(msg, status)
      json = {}
diff --git a/src/app/controllers/deployables_controller.rb 
b/src/app/controllers/deployables_controller.rb
index e07a603..74b5fc3 100644
--- a/src/app/controllers/deployables_controller.rb
+++ b/src/app/controllers/deployables_controller.rb
@@ -302,22 +302,4 @@ class DeployablesController < ApplicationController
                                        Privilege::CREATE, Deployable).
        where('pool_family_id' => @pool_family.id)
    end
-
-  def import_xml_from_url(url)
-    begin
-      response = RestClient.get(url, :accept => :xml)
-      if response.code == 200
-        response
-      end
-    rescue RestClient::Exception, SocketError, URI::InvalidURIError, 
Errno::ECONNREFUSED, Errno::ETIMEDOUT
-      if url.present?
-        flash[:error] = t('catalog_entries.flash.warning.not_valid_or_reachable', 
:url => url)
-      else
-        flash[:error] = t('catalog_entries.flash.warning.no_url_provided')
-      end
-      nil
-    end
-  end
-
-

Not caused by your patch but it's a good chance to fix it:
no matter what I will in in URL for deployable, error message is always "XML is blank" because if import fails, create action continues in saving deployable with nil xml.

  end
diff --git a/src/app/controllers/images_controller.rb 
b/src/app/controllers/images_controller.rb
index d9ab289..9cfdd2c 100644
--- a/src/app/controllers/images_controller.rb
+++ b/src/app/controllers/images_controller.rb
@@ -183,7 +183,11 @@ class ImagesController < ApplicationController
          t("images.flash.error.no_provider_accounts_for_import") :
          t("images.flash.error.no_provider_accounts")
      end
-    render 'import' == params[:tab] ? :import : :new
+    if 'import' == params[:tab]
+      render :import
+    else
+      render :new, :locals => {:active => "file"}
+    end
    end

    def import
@@ -216,21 +220,21 @@ class ImagesController < ApplicationController
      @environment = PoolFamily.find(params[:environment])
      check_permissions
      @name = params[:name]
-
      if params.has_key? :image_url
-      url = params[:image_url]
-      begin
-        xml_source = RestClient.get(url, :accept => :xml)
-      rescue RestClient::Exception, SocketError, URI::InvalidURIError, 
Errno::ECONNREFUSED, Errno::ETIMEDOUT
-        flash.now[:error] = t('images.flash.error.invalid_url')
-        render :new and return
+      import_xml_from_url(params[:image_url])
+      if flash.has_key? :error

I get following error:
undefined method `has_key?' for #<ActionDispatch::Flash::FlashHash:0x00000007a54770>

You might check the result of import_xml_from_url call instead.

+        @accounts = @environment.provider_accounts.enabled.
+                  list_for_user(current_session, current_user, Privilege::USE)
+        render :new, :locals => {:active => "url"} and return
        end
      else
        file = params[:image_file]
        xml_source = file && file.read
        unless xml_source
-        flash.now[:error] = t('images.flash.error.no_file')
-        render :new and return
+        @accounts = @environment.provider_accounts.enabled.
+            list_for_user(current_session, current_user, Privilege::USE)
+        flash[:error] = t('images.flash.error.no_file')
+        render :new, :locals => {:active => "file"} and return
        end
      end

diff --git a/src/app/views/images/new.html.haml 
b/src/app/views/images/new.html.haml
index 79f8b6c..2ea9cba 100644
--- a/src/app/views/images/new.html.haml
+++ b/src/app/views/images/new.html.haml
@@ -1,4 +1,5 @@
  = render :partial => 'layouts/admin_nav'
+
  %header.page-header
    .obj_actions
      .return_to
@@ -15,9 +16,11 @@
        %nav#image-upload-tabs.faceted
          %ul.tabs
            %li
-            %a{ :href => '#image-file-form', :class => 'active'}= t('.upload')
+            %a{ :href => '#image-file-form', :class => "#{'active' if active == 
"file"}"}
+              = t('.upload')
            %li
-            %a{ :href => '#image-url-form'}= t('.from_url')
+            %a{ :href => '#image-url-form', :class => "#{'active' if active == 
"url"}"}
+              = t('.from_url')

        %section#image-file-form
          = form_tag(edit_xml_images_path, { :multipart => true, :class => 
'generic horizontal' }) do
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 8d2fb7d..a5213a6 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -838,8 +838,6 @@ en:
        catalog_entries_list: Deployables List
      flash:
        warning:
-        not_valid_or_reachable: "Deployable XML file is either invalid or no longer 
reachable at %{url}"
-        no_url_provided: "No URL provided for the Deployable XML file."
          not_valid: "The Deployable XML file contains syntax errors"
          not_valid_duplicate_assembly_names: "Deployable XML must contain unique 
Assembly names"
          not_valid_cyclic_reference: "Contains cyclic reference between following 
Assemblies or services: %{reference}"
@@ -963,7 +961,6 @@ en:
          delete_failed: Unable to Delete Image
          not_found: Image not found
        error:
-        invalid_url: Could not load the provided URL
          no_file: You must specify the Image Template XML file
          no_template: "The Image doesn't have an Image Template because it was 
originally imported, rather than built in Conductor."
          no_provider_accounts: "Images cannot be built. There are no enabled 
Provider Accounts associated with this Environment."
@@ -1478,6 +1475,11 @@ en:
          record_not_exist: "The record you tried to access does not exist. It may 
have been deleted"
          must_be_logged: You must be logged in to access this page
          must_not_be_logged: You must be logged out to access this page
+      error:
+        no_url_provided: No URL is provided for XML import
+        not_valid_url: "Provided URL is not valid %{url}"
+        download_failed: Download of XML file failed
+        not_valid_or_reachable: "XML file is either invalid or no longer reachable 
at %{url}"
      admin_tabs:
        catalogs: Catalogs
        realms: Realms


Hi, this looks good, though found 2 minor problems (inline).

Jan

Reply via email to