[PATCH conductor 1/3] Fixing autoupdating issues
by Imre Farkas
From: Imre Farkas <ifarkas(a)redhat.com>
The order of Pools/Deployments/Instances was messed up after autoupdate
The Properties and Role Assignemt tabs on deployments#show was empty after autoupdate
---
src/app/controllers/instances_controller.rb | 4 ++--
src/app/controllers/pools_controller.rb | 4 ++--
src/app/models/deployment.rb | 2 ++
src/app/models/pool.rb | 2 ++
src/app/views/pools/_pretty_list.html.haml | 2 +-
src/public/javascripts/backbone/routers.js | 2 ++
src/public/javascripts/backbone/views.js | 6 ++++++
7 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/app/controllers/instances_controller.rb b/src/app/controllers/instances_controller.rb
index 5979a1c..3f5917f 100644
--- a/src/app/controllers/instances_controller.rb
+++ b/src/app/controllers/instances_controller.rb
@@ -215,9 +215,9 @@ class InstancesController < ApplicationController
def load_instances
if params[:deployment_id].blank?
- @instances = Instance.includes(:owner).apply_filters(:preset_filter_id => params[:instances_preset_filter], :search_filter => params[:instances_search]).where("instances.pool_id" => @pools)
+ @instances = Instance.includes(:owner).apply_filters(:preset_filter_id => params[:instances_preset_filter], :search_filter => params[:instances_search]).list(sort_column(Instance), sort_direction).where("instances.pool_id" => @pools)
else
- @instances = Instance.includes(:owner).apply_filters(:preset_filter_id => params[:instances_preset_filter], :search_filter => params[:instances_search]).where("instances.pool_id" => @pools, "instances.deployment_id" => params[:deployment_id])
+ @instances = Instance.includes(:owner).apply_filters(:preset_filter_id => params[:instances_preset_filter], :search_filter => params[:instances_search]).list(sort_column(Instance), sort_direction).where("instances.pool_id" => @pools, "instances.deployment_id" => params[:deployment_id])
end
end
diff --git a/src/app/controllers/pools_controller.rb b/src/app/controllers/pools_controller.rb
index 272110c..37f238e 100644
--- a/src/app/controllers/pools_controller.rb
+++ b/src/app/controllers/pools_controller.rb
@@ -45,12 +45,12 @@ class PoolsController < ApplicationController
@details_tab = @tabs.find {|t| t[:id] == details_tab_name} || @tabs.first[:name].downcase
case @details_tab[:id]
when 'pools'
- @pools = Pool.list_for_user(current_user, Privilege::VIEW).apply_filters(:preset_filter_id => params[:pools_preset_filter], :search_filter => params[:pools_search])
+ @pools = Pool.list_for_user(current_user, Privilege::VIEW).apply_filters(:preset_filter_id => params[:pools_preset_filter], :search_filter => params[:pools_search]).list(sort_column(Pool), sort_direction)
when 'instances'
params[:instances_preset_filter] = "other_than_stopped" unless params[:instances_preset_filter]
@instances = Instance.apply_filters(:preset_filter_id => params[:instances_preset_filter], :search_filter => params[:instances_search]).list(sort_column(Instance), sort_direction)
when 'deployments'
- @deployments = Deployment.apply_filters(:preset_filter_id => params[:deployments_preset_filter], :search_filter => params[:deployments_search])
+ @deployments = Deployment.apply_filters(:preset_filter_id => params[:deployments_preset_filter], :search_filter => params[:deployments_search]).list(sort_column(Deployment), sort_direction)
end
else
@pools = Pool.list(sort_column(Pool), sort_direction)
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 7b5ce76..8ebd703 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -57,6 +57,8 @@ class Deployment < ActiveRecord::Base
after_create "assign_owner_roles(owner)"
+ scope :ascending_by_name, :order => 'name ASC'
+
validates_presence_of :pool_id
validates_presence_of :name
validates_uniqueness_of :name, :scope => :pool_id
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 287bc70..977e61e 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -65,6 +65,8 @@ class Pool < ActiveRecord::Base
before_destroy :destroyable?
+ scope :ascending_by_name, :order => 'name ASC'
+
def cloud_accounts
accounts = []
instances.each do |instance|
diff --git a/src/app/views/pools/_pretty_list.html.haml b/src/app/views/pools/_pretty_list.html.haml
index 555dd6f..ac2af4d 100644
--- a/src/app/views/pools/_pretty_list.html.haml
+++ b/src/app/views/pools/_pretty_list.html.haml
@@ -25,7 +25,7 @@
%a.control{:href => '#'}
%span Expand/Collapse
%div.content
- = render :partial => 'deployments', :locals => {:deployments => pool.deployments}
+ = render :partial => 'deployments', :locals => {:deployments => pool.deployments.ascending_by_name}
%ul.content.actions
%li
= link_to ("#{html_escape(pool.name)} " + "#{t'pools.index.pool_details'}" + " »").html_safe, pool_path(pool), :class =>'pool_details'
diff --git a/src/public/javascripts/backbone/routers.js b/src/public/javascripts/backbone/routers.js
index a766b83..cb55730 100644
--- a/src/public/javascripts/backbone/routers.js
+++ b/src/public/javascripts/backbone/routers.js
@@ -74,6 +74,8 @@ Conductor.Routers.Deployments = Backbone.Router.extend({
var view = new Conductor.Views.DeploymentsShow({ model: deployment,
collection: deployment.instances });
+ if(view.currentTab() !== 'instances') return;
+
deployment.bind('change', function() { view.render() });
deployment.instances.fetch({success: function(instances) {
diff --git a/src/public/javascripts/backbone/views.js b/src/public/javascripts/backbone/views.js
index 3e2776d..dd0cd38 100644
--- a/src/public/javascripts/backbone/views.js
+++ b/src/public/javascripts/backbone/views.js
@@ -102,6 +102,12 @@ Conductor.Views.DeploymentsShow = Backbone.View.extend({
el: '#content',
+ currentTab: function() {
+ if($('#details_instances.active').length > 0) {
+ return 'instances';
+ }
+ },
+
render: function() {
var $instances = this.$('ul.instances-array');
if($instances.length === 0) {
--
1.7.6.4
12 years, 3 months
[PATCH conductor] Error message fixed for creating new deployable without providing url for the xml
by Imre Farkas
From: Imre Farkas <ifarkas(a)redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=770809
---
src/app/controllers/deployables_controller.rb | 6 +++++-
src/config/locales/en.yml | 1 +
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
index 13d4f3e..05473b2 100644
--- a/src/app/controllers/deployables_controller.rb
+++ b/src/app/controllers/deployables_controller.rb
@@ -258,7 +258,11 @@ class DeployablesController < ApplicationController
response
end
rescue RestClient::Exception, SocketError, URI::InvalidURIError
- flash[:error] = t('catalog_entries.flash.warning.not_valid_or_reachable', :url => url)
+ 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
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 0275275..94e9873 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -631,6 +631,7 @@ en:
flash:
warning:
not_valid_or_reachable: "Catalog entry XML file is either invalid or no longer reachable at %{url}"
+ no_url_provided: "No URL provided for the catalog entry XML file."
not_valid: "Deployable XML file doesn't resolve valid XML"
failed: Catalog entry was not created.
notice:
--
1.7.6.4
12 years, 3 months
[PATCH conductor] Deployable XML parsing switched to strict mode
by Jan Provazník
From: Jan Provaznik <jprovazn(a)redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=766993
It's not possible to have now wrongly formatted document (missing tags),
DeployableXML raises exception now in this case.
---
src/app/models/deployable.rb | 10 +++++++---
src/app/models/deployment.rb | 2 ++
src/app/util/deployable_xml.rb | 2 +-
src/spec/models/deployable_spec.rb | 8 ++++++++
4 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
index 59c4378..c9446f2 100644
--- a/src/app/models/deployable.rb
+++ b/src/app/models/deployable.rb
@@ -41,9 +41,13 @@ class Deployable < ActiveRecord::Base
PRESET_FILTERS_OPTIONS = []
def valid_deployable_xml?
- deployable_xml = DeployableXML.new(xml)
- unless deployable_xml.validate!
- errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid'))
+ begin
+ deployable_xml = DeployableXML.new(xml)
+ unless deployable_xml.validate!
+ errors.add(:xml, I18n.t('catalog_entries.flash.warning.not_valid'))
+ end
+ rescue Nokogiri::XML::SyntaxError => e
+ errors.add(:base, I18n.t("deployments.errors.not_valid_deployable_xml", :msg => "#{e.message}"))
end
end
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 7b5ce76..ea62c33 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -79,6 +79,8 @@ class Deployment < ActiveRecord::Base
deployable_xml.validate!
rescue DeployableXML::ValidationError => e
errors.add(:deployable_xml, e.message)
+ rescue Nokogiri::XML::SyntaxError => e
+ errors.add(:base, I18n.t("deployments.errors.not_valid_deployable_xml", :msg => "#{e.message}"))
end
end
diff --git a/src/app/util/deployable_xml.rb b/src/app/util/deployable_xml.rb
index 8295340..995cfff 100644
--- a/src/app/util/deployable_xml.rb
+++ b/src/app/util/deployable_xml.rb
@@ -173,7 +173,7 @@ class DeployableXML
if xmlstr_or_node.is_a? Nokogiri::XML::Node
@root = xmlstr_or_node
elsif xmlstr_or_node.is_a? String
- @doc = Nokogiri::XML(xmlstr_or_node)
+ @doc = Nokogiri::XML(xmlstr_or_node) { |config| config.strict }
@root = @doc.root.at_xpath("/deployable") if @doc.root
end
@relax_file = "#{File.dirname(File.expand_path(__FILE__))}/deployable-rng.xml"
diff --git a/src/spec/models/deployable_spec.rb b/src/spec/models/deployable_spec.rb
index 9f7cbaf..cf4bb1d 100644
--- a/src/spec/models/deployable_spec.rb
+++ b/src/spec/models/deployable_spec.rb
@@ -30,4 +30,12 @@ describe Deployable do
doc.at_xpath('/deployable/assemblies/assembly')[:name].should eql(image.name)
doc.at_xpath('/deployable/assemblies/assembly/image')[:id].should eql(image.uuid)
end
+
+ it "should not be valid if xml is not parsable" do
+ deployable = FactoryGirl.build(:deployable)
+ deployable.should be_valid
+ deployable.xml << "</deployable>"
+ deployable.should_not be_valid
+ end
+
end
--
1.7.6
12 years, 3 months