diff --git a/app/models/content_instance.rb b/app/models/content_instance.rb index 527c76ba..bef5cbd5 100644 --- a/app/models/content_instance.rb +++ b/app/models/content_instance.rb @@ -14,8 +14,7 @@ class ContentInstance ## validations ## validate :require_highlighted_field - validate :validate_uniqueness_of_slug - validates_presence_of :_slug + validates :_slug, :presence => true, :uniqueness => { :scope => :content_type_id } ## associations ## embedded_in :content_type, :inverse_of => :contents @@ -77,9 +76,29 @@ class ContentInstance protected + # Sets the slug of the instance by using the value of the highlighted field + # (if available). If a sibling content instance has the same permalink then a + # unique one will be generated def set_slug - self._slug = self.highlighted_field_value.dup if self._slug.blank? && self.highlighted_field_value.present? - self._slug.permalink! if self._slug.present? + self._slug = highlighted_field_value.dup if _slug.blank? && highlighted_field_value.present? + + if _slug.present? + self._slug.permalink! + self._slug = next_unique_slug if slug_already_taken? + end + end + + # Return the next available unique slug as a string + def next_unique_slug + slug = _slug.gsub(/-\d*$/, '') + last_slug = _parent.contents.where(:_id.ne => _id, :_slug => /^#{slug}-?\d*?$/i).order_by(:_slug).last._slug + next_number = last_slug.scan(/-(\d)$/).flatten.first.to_i + 1 + + [slug, next_number].join('-') + end + + def slug_already_taken? + _parent.contents.where(:_id.ne => _id, :_slug => _slug).any? end def set_visibility @@ -98,12 +117,6 @@ class ContentInstance end end - def validate_uniqueness_of_slug - if self._parent.contents.any? { |c| c._id != self._id && c._slug == self._slug } - self.errors.add(:_slug, :taken) - end - end - def highlighted_field_alias self.content_type.highlighted_field._alias.to_sym end diff --git a/spec/models/content_instance_spec.rb b/spec/models/content_instance_spec.rb index 9669ab61..1330f983 100644 --- a/spec/models/content_instance_spec.rb +++ b/spec/models/content_instance_spec.rb @@ -14,7 +14,6 @@ describe ContentInstance do end describe '#validation' do - it 'is valid' do build_content.should be_valid end @@ -32,14 +31,37 @@ describe ContentInstance do content.should_not be_valid content.errors[:_slug].should == ["can't be blank"] end + end - it 'has an unique permalink' do - build_content.save; @content_type = ContentType.find(@content_type._id) - content = build_content - content.should_not be_valid - content.errors[:_slug].should == ["is already taken"] + context 'setting the slug' do + before :each do + build_content(:_slug => 'dogs').tap(&:save!)._slug.should == 'dogs' end + it 'uses the given slug if it is unique' do + build_content(:_slug => 'monkeys').tap(&:save!)._slug.should == 'monkeys' + build_content(:_slug => 'cats-2').tap(&:save!)._slug.should == 'cats-2' + end + + it 'appends a number to the end of the slug if it is not unique' do + build_content(:_slug => 'dogs').tap(&:save!)._slug.should == 'dogs-1' + build_content(:_slug => 'dogs').tap(&:save!)._slug.should == 'dogs-2' + build_content(:_slug => 'dogs-2').tap(&:save!)._slug.should == 'dogs-3' + build_content(:_slug => 'dogs-2').tap(&:save!)._slug.should == 'dogs-4' + end + + it 'ignores the case of a slug' do + build_content(:_slug => 'dogs').tap(&:save!)._slug.should == 'dogs-1' + build_content(:_slug => 'DOGS').tap(&:save!)._slug.should == 'dogs-2' + end + + it 'correctly handles slugs with multiple numbers' do + build_content(:_slug => 'fish-1-2').tap(&:save!)._slug.should == 'fish-1-2' + build_content(:_slug => 'fish-1-2').tap(&:save!)._slug.should == 'fish-1-3' + + build_content(:_slug => 'fish-1-hi').tap(&:save!)._slug.should == 'fish-1-hi' + build_content(:_slug => 'fish-1-hi').tap(&:save!)._slug.should == 'fish-1-hi-1' + end end describe "#navigation" do @@ -189,4 +211,4 @@ describe ContentInstance do def fake_bson_id(id) BSON::ObjectId(id.to_s.rjust(24, '0')) end -end \ No newline at end of file +end