diff options
| -rw-r--r-- | app/models/public_body_category.rb | 13 | ||||
| -rw-r--r-- | app/models/public_body_category_link.rb | 19 | ||||
| -rw-r--r-- | app/models/public_body_heading.rb | 16 | ||||
| -rw-r--r-- | spec/models/public_body_category_link_spec.rb | 28 | ||||
| -rw-r--r-- | spec/models/public_body_category_spec.rb | 6 | ||||
| -rw-r--r-- | spec/models/public_body_heading_spec.rb | 54 |
6 files changed, 108 insertions, 28 deletions
diff --git a/app/models/public_body_category.rb b/app/models/public_body_category.rb index 0c5142cfd..e78b91733 100644 --- a/app/models/public_body_category.rb +++ b/app/models/public_body_category.rb @@ -118,22 +118,9 @@ class PublicBodyCategory < ActiveRecord::Base # we already have this, stop return end - - # find the last display_order for this heading - last_link = PublicBodyCategoryLink.where( - :public_body_heading_id => heading.id - ).order(:category_display_order).last - - if last_link - display_order = last_link.category_display_order + 1 - else - display_order = 1 - end - heading_link = PublicBodyCategoryLink.create( :public_body_category_id => self.id, :public_body_heading_id => heading.id, - :category_display_order => display_order ) end diff --git a/app/models/public_body_category_link.rb b/app/models/public_body_category_link.rb index bd4fe541a..e5a0d7f25 100644 --- a/app/models/public_body_category_link.rb +++ b/app/models/public_body_category_link.rb @@ -12,4 +12,21 @@ class PublicBodyCategoryLink < ActiveRecord::Base belongs_to :public_body_category belongs_to :public_body_heading -end
\ No newline at end of file + validates :category_display_order, :numericality => { :only_integer => true, + :message => N_('Display order must be a number') } + + before_validation :on => :create do + unless self.category_display_order + self.category_display_order = PublicBodyCategoryLink.next_display_order(self.public_body_heading_id) + end + end + + def PublicBodyCategoryLink.next_display_order(heading_id) + if last = where(:public_body_heading_id => heading_id).order(:category_display_order).last + last.category_display_order + 1 + else + 0 + end + end + +end diff --git a/app/models/public_body_heading.rb b/app/models/public_body_heading.rb index 62e7d8e91..e675162f8 100644 --- a/app/models/public_body_heading.rb +++ b/app/models/public_body_heading.rb @@ -18,6 +18,14 @@ class PublicBodyHeading < ActiveRecord::Base validates_uniqueness_of :name, :message => N_('Name is already taken') validates_presence_of :name, :message => N_('Name can\'t be blank') + validates :display_order, :numericality => { :only_integer => true, + :message => N_('Display order must be a number') } + + before_validation :on => :create do + unless self.display_order + self.display_order = PublicBodyHeading.next_display_order + end + end # Convenience methods for creating/editing translations via forms def find_translation_by_locale(locale) @@ -49,4 +57,12 @@ class PublicBodyHeading < ActiveRecord::Base end end end + + def PublicBodyHeading.next_display_order + if max = maximum(:display_order) + max + 1 + else + 0 + end + end end diff --git a/spec/models/public_body_category_link_spec.rb b/spec/models/public_body_category_link_spec.rb index 9fce062a3..105f97864 100644 --- a/spec/models/public_body_category_link_spec.rb +++ b/spec/models/public_body_category_link_spec.rb @@ -9,5 +9,29 @@ require 'spec_helper' -describe PublicBodyCategoryLink do -end
\ No newline at end of file +describe PublicBodyHeading, 'when validating' do + + it 'should set a default display order based on the next available display order' do + heading_with_no_categories = FactoryGirl.create(:heading_with_no_categories) + lonely_category = FactoryGirl.create(:lonely_category) + category_link = PublicBodyCategoryLink.new(:heading => heading_with_no_categories, + :category_link => lonely_category) + category_link.valid? + category_link.category_display_order.should == PublicBodyCategoryLink.next_display_order(heading_with_no_categories) + end + +end + +describe PublicBodyCategoryLink, 'when setting a category display order' do + + it 'should return 0 if there are no public body headings' do + heading_with_no_categories = FactoryGirl.create(:heading_with_no_categories) + PublicBodyCategoryLink.next_display_order(heading_with_no_categories).should == 0 + end + + it 'should return one more than the highest display order if there are public body headings' do + silly_heading = FactoryGirl.create(:silly_heading) + PublicBodyCategoryLink.next_display_order(silly_heading).should == 2 + end + +end diff --git a/spec/models/public_body_category_spec.rb b/spec/models/public_body_category_spec.rb index d892c8bda..d3c91e4f8 100644 --- a/spec/models/public_body_category_spec.rb +++ b/spec/models/public_body_category_spec.rb @@ -33,13 +33,13 @@ describe PublicBodyCategory do cat_group2[0].public_body_category_links.where( :public_body_heading_id => headings[1].id). first. - category_display_order.should eq 1 + category_display_order.should eq 0 cat_group2[1].title.should eq "Aardvark" cat_group2[1].public_body_category_links.where( :public_body_heading_id => headings[1].id). first. - category_display_order.should eq 2 + category_display_order.should eq 1 end end @@ -116,4 +116,4 @@ describe PublicBodyCategory do end end end -end
\ No newline at end of file +end diff --git a/spec/models/public_body_heading_spec.rb b/spec/models/public_body_heading_spec.rb index ec5c3ad2c..3ca01c648 100644 --- a/spec/models/public_body_heading_spec.rb +++ b/spec/models/public_body_heading_spec.rb @@ -10,18 +10,54 @@ require 'spec_helper' -describe PublicBodyHeading do +describe PublicBodyHeading, 'when loading the data' do + before do load_test_categories end - describe 'when loading the data' do - it 'should use the display_order field to preserve the original data order' do - headings = PublicBodyHeading.all - headings[0].name.should eq 'Local and regional' - headings[0].display_order.should eq 1 - headings[1].name.should eq 'Miscellaneous' - headings[1].display_order.should eq 2 - end + it 'should use the display_order field to preserve the original data order' do + headings = PublicBodyHeading.all + headings[0].name.should eq 'Local and regional' + headings[0].display_order.should eq 1 + headings[1].name.should eq 'Miscellaneous' + headings[1].display_order.should eq 2 + end + +end + +describe PublicBodyHeading, 'when validating' do + + it 'should require a name' do + heading = PublicBodyHeading.new + heading.should_not be_valid + heading.errors[:name].should == ["Name can't be blank"] + end + + it 'should require a unique name' do + heading = FactoryGirl.create(:silly_heading) + new_heading = PublicBodyHeading.new(:name => heading.name) + new_heading.should_not be_valid + new_heading.errors[:name].should == ["Name is already taken"] + end + + it 'should set a default display order based on the next available display order' do + heading = PublicBodyHeading.new + heading.valid? + heading.display_order.should == PublicBodyHeading.next_display_order end + +end + +describe PublicBodyHeading, 'when setting a display order' do + + it 'should return 0 if there are no public body headings' do + PublicBodyHeading.next_display_order.should == 0 + end + + it 'should return one more than the highest display order if there are public body headings' do + heading = FactoryGirl.create(:popular_heading) + PublicBodyHeading.next_display_order.should == 2 + end + end |