Comments inline. -- Jacob Helwig
On Fri, 01 Oct 2010 14:08:36 -0400, Nick Lewis wrote: > > Classes and groups are sorted alphabetically in every case in which > they are listed. Nodes are sorted by the time of their last report. > > Signed-off-by: Nick Lewis <[email protected]> > --- > app/models/node.rb | 2 +- > app/models/node_class.rb | 2 ++ > app/models/node_group.rb | 2 ++ > app/views/nodes/_nodes.html.haml | 2 +- > lib/node_group_graph.rb | 4 ++-- > spec/controllers/node_classes_controller_spec.rb | 2 ++ > spec/controllers/node_groups_controller_spec.rb | 2 ++ > spec/models/node_spec.rb | 2 +- > spec/shared_behaviors/sorted_index.rb | 14 ++++++++++++++ > 9 files changed, 27 insertions(+), 5 deletions(-) > create mode 100644 spec/shared_behaviors/sorted_index.rb > > diff --git a/app/models/node.rb b/app/models/node.rb > index 42ed344..3ecbb0d 100644 > --- a/app/models/node.rb > +++ b/app/models/node.rb > @@ -97,7 +97,7 @@ class Node < ActiveRecord::Base > end > > def inherited_classes > - (node_group_list.keys - [self]).map(&:node_classes).flatten.uniq > + (node_group_list.map(&:first)- [self]).map(&:node_classes).flatten.uniq .keys seems to be the more clear option here, and Rein points out that .map(&:first) breaks in Ruby 1.9. > end > > def all_classes > diff --git a/app/models/node_class.rb b/app/models/node_class.rb > index f048599..b9b34b0 100644 > --- a/app/models/node_class.rb > +++ b/app/models/node_class.rb > @@ -14,6 +14,8 @@ class NodeClass < ActiveRecord::Base > validates_format_of :name, :with => > /\A([a-z0-9][-\w]*)(::[a-z0-9][-\w]*)*\Z/, :message => "must contain a valid > Puppet class name, e.g. 'foo' or 'foo::bar'" > validates_uniqueness_of :name > > + default_scope :order => 'name ASC' > + > named_scope :search, lambda{|q| q.blank? ? {} : {:conditions => ['name > LIKE ?', "%#{q}%"]} } > > named_scope :with_nodes_count, > diff --git a/app/models/node_group.rb b/app/models/node_group.rb > index 933ae88..e53e620 100644 > --- a/app/models/node_group.rb > +++ b/app/models/node_group.rb > @@ -24,6 +24,8 @@ class NodeGroup < ActiveRecord::Base > validates_presence_of :name > validates_uniqueness_of :name > > + default_scope :order => 'name ASC' > + > named_scope :search, lambda{|q| q.blank? ? {} : {:conditions => ['name > LIKE ?', "%#{q}%"]} } > > named_scope :with_nodes_count, > diff --git a/app/views/nodes/_nodes.html.haml > b/app/views/nodes/_nodes.html.haml > index 9636314..55d2a09 100644 > --- a/app/views/nodes/_nodes.html.haml > +++ b/app/views/nodes/_nodes.html.haml > @@ -14,7 +14,7 @@ > ↓ Latest report > %tbody > - if nodes.present? > - - nodes.each do |node| > + - nodes.sort {|a,b| (b.reported_at || Time.at(0)) <=> (a.reported_at > || Time.at(0))}.each do |node| This doesn't appear directly related to #4661. Should be in its own commit? > - sources = container.node_list[node] unless container.nil? > %tr[node]{:class => "#{'active' if node == @node}"} > %td.status{:class => node.status_class} > diff --git a/lib/node_group_graph.rb b/lib/node_group_graph.rb > index fdd169b..15671bb 100644 > --- a/lib/node_group_graph.rb > +++ b/lib/node_group_graph.rb > @@ -12,7 +12,7 @@ module NodeGroupGraph > end > group > end > - @node_group_list = all > + @node_group_list = all.sort {|a,b| a.first.name <=> b.first.name} > end > > # Returns a nested hash of all groups for this group/node, direct or > inherited. > @@ -38,7 +38,7 @@ module NodeGroupGraph > end > group > end > - @node_class_list = all > + @node_class_list = all.sort {|a,b| a.first.name <=> b.first.name} > end > > # Collects all the parameters of the node, starting at the "most distant" > groups > diff --git a/spec/controllers/node_classes_controller_spec.rb > b/spec/controllers/node_classes_controller_spec.rb > index 9fe1cbd..0ac5b2f 100644 > --- a/spec/controllers/node_classes_controller_spec.rb > +++ b/spec/controllers/node_classes_controller_spec.rb > @@ -1,10 +1,12 @@ > require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') > require 'shared_behaviors/controller_mixins' > +require 'shared_behaviors/sorted_index' > > describe NodeClassesController do > def model; NodeClass end > > it_should_behave_like "without JSON pagination" > it_should_behave_like "with search by q and tag" > + it_should_behave_like "sorted index" > > end > diff --git a/spec/controllers/node_groups_controller_spec.rb > b/spec/controllers/node_groups_controller_spec.rb > index fad7c22..f33f930 100644 > --- a/spec/controllers/node_groups_controller_spec.rb > +++ b/spec/controllers/node_groups_controller_spec.rb > @@ -1,10 +1,12 @@ > require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') > require 'shared_behaviors/controller_mixins' > +require 'shared_behaviors/sorted_index' > > describe NodeGroupsController do > def model; NodeGroup end > > it_should_behave_like "without JSON pagination" > it_should_behave_like "with search by q and tag" > + it_should_behave_like "sorted index" > > end > diff --git a/spec/models/node_spec.rb b/spec/models/node_spec.rb > index 167cf2d..2fc4b13 100644 > --- a/spec/models/node_spec.rb > +++ b/spec/models/node_spec.rb > @@ -279,7 +279,7 @@ describe Node do > end > > it "should return the correct list" do > - @node.node_group_list.should == {...@node_group_a => s...@node], > @node_group_c => s...@node_group_a,@node_group_b], @node_group_b => > s...@node], @node_group_d => s...@node_group_c]} > + @node.node_group_list.should == [...@node_group_a, s...@node]], > [...@node_group_b, s...@node]], [...@node_group_c, > s...@node_group_a,@node_group_b]], [...@node_group_d, s...@node_group_c]]] Perhaps split up this long line? Something like this? @node.node_group_list.should == [ [...@node_group_a, s...@node]], [...@node_group_b, s...@node]], [...@node_group_c, s...@node_group_a,@node_group_b]], [...@node_group_d, s...@node_group_c]] ] > end > end > > diff --git a/spec/shared_behaviors/sorted_index.rb > b/spec/shared_behaviors/sorted_index.rb > new file mode 100644 > index 0000000..639342c > --- /dev/null > +++ b/spec/shared_behaviors/sorted_index.rb > @@ -0,0 +1,14 @@ > +describe "sorted index", :shared => true do > + before do > + model.destroy_all > + end > + > + it "should be sorted by default" do > + c = model.generate! :name => "c" > + b = model.generate! :name => "b" > + d = model.generate! :name => "d" > + a = model.generate! :name => "a" > + > + model.all.should == [a,b,c,d] > + end > +end > -- > 1.7.2.1 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. >
signature.asc
Description: Digital signature
