fix issue #317 + with_scope works now with has_many relationships + clean code

Gemfile.lock

@@ -17,10 +17,10 @@ GIT
 
 GIT
   remote: git://github.com/locomotivecms/custom_fields.git
-  revision: c30c7df5a502b888f63bb908c146cb0fc61ef204
+  revision: ec157c3941d1530eb49d08ee385d25c3d3f34f15
   branch: 2.0.0.rc
   specs:
-    custom_fields (2.0.0.rc6)
+    custom_fields (2.0.0.rc7)
       activesupport (~> 3.2.1)
       carrierwave-mongoid (~> 0.1.3)
       mongoid (~> 2.4.5)
@@ -36,7 +36,7 @@ PATH
       carrierwave-mongoid (~> 0.1.3)
       cells (~> 3.8.0)
       codemirror-rails (~> 2.21)
-      custom_fields (~> 2.0.0.rc6)
+      custom_fields (~> 2.0.0.rc7)
       devise (~> 1.5.3)
       dragonfly (~> 0.9.8)
       flash_cookie_session (~> 1.1.1)

app/models/locomotive/content_type.rb

@@ -5,7 +5,9 @@ module Locomotive
 
     ## extensions ##
     include CustomFields::Source
+    include Extensions::ContentType::DefaultValues
     include Extensions::ContentType::ItemTemplate
+    include Extensions::ContentType::Sync
 
     ## fields ##
     field :name
@@ -32,7 +34,6 @@ module Locomotive
     ## callbacks ##
     before_validation   :normalize_slug
     after_validation    :bubble_fields_errors_up
-    before_save         :set_default_values
     before_update       :update_label_field_name_in_entries
 
     ## validations ##
@@ -142,25 +143,6 @@ module Locomotive
       self.entries_custom_fields.find(self.order_by).name rescue 'created_at'
     end
 
-    def set_default_values
-      self.order_by ||= 'created_at'
-
-      if @new_label_field_name.present?
-        self.label_field_id = self.entries_custom_fields.detect { |f| f.name == @new_label_field_name.underscore }._id
-      end
-
-      if self.label_field_id.blank?
-        self.label_field_id = self.entries_custom_fields.first._id
-      end
-
-      field = self.entries_custom_fields.find(self.label_field_id)
-
-      # the label field should always be required
-      field.required = true
-
-      self.label_field_name = field.name
-    end
-
     def normalize_slug
       self.slug = self.name.clone if self.slug.blank? && self.name.present?
       self.slug.permalink! if self.slug.present?

app/models/locomotive/extensions/content_type/default_values.rb

@@ -0,0 +1,59 @@
+module Locomotive
+  module Extensions
+    module ContentType
+      module DefaultValues
+
+        extend ActiveSupport::Concern
+
+        included do
+          before_save :set_order_by
+          before_save :set_label_field
+          before_save :set_default_order_by_for_has_many_fields
+        end
+
+        protected
+
+        def set_order_by
+          unless self.order_by.nil? || %w(created_at updated_at _position).include?(self.order_by)
+            field = self.entries_custom_fields.where(:name => self.order_by).first || self.entries_custom_fields.find(self.order_by)
+
+            if field
+              self.order_by = field._id
+            end
+          end
+
+          self.order_by ||= 'created_at'
+        end
+
+        def set_label_field
+          if @new_label_field_name.present?
+            self.label_field_id = self.entries_custom_fields.detect { |f| f.name == @new_label_field_name.underscore }._id
+          end
+
+          # unknown label_field_name, get the first one instead
+          if self.label_field_id.blank?
+            self.label_field_id = self.entries_custom_fields.first._id
+          end
+
+          field = self.entries_custom_fields.find(self.label_field_id)
+
+          # the label field should always be required
+          field.required = true
+
+          self.label_field_name = field.name
+        end
+
+        def set_default_order_by_for_has_many_fields
+          self.entries_custom_fields.where(:type.in => %w(has_many many_to_many)).each do |field|
+            if field.ui_enabled?
+              field.order_by = nil
+            else
+              field.order_by = field.class_name_to_content_type.order_by_definition
+            end
+          end
+        end
+
+      end
+    end
+  end
+end

app/models/locomotive/extensions/content_type/sync.rb

@@ -0,0 +1,62 @@
+module Locomotive
+  module Extensions
+    module ContentType
+      module Sync
+
+        extend ActiveSupport::Concern
+
+        included do
+          after_save :sync_relationships_order_by
+        end
+
+        protected
+
+        # If the user changes the order of the content type, we have to make
+        # sure that other related content types tied to the current one through
+        # a belongs_to / has_many relationship also gets updated.
+        #
+        def sync_relationships_order_by
+          current_class_name = self.klass_with_custom_fields(:entries).name
+
+          self.entries_custom_fields.where(:type => 'belongs_to').each do |field|
+            target_content_type = self.class_name_to_content_type(field.class_name)
+
+            operations = { '$set' => {} }
+
+            target_content_type.entries_custom_fields.where(:type.in => %w(has_many many_to_many), :ui_enabled => false, :class_name => current_class_name).each do |target_field|
+              if target_field.order_by != self.order_by_definition
+                target_field.order_by = self.order_by_definition # needed by the custom_fields_recipe_for method in order to be up to date
+
+                operations['$set']["entries_custom_fields.#{target_field._index}.order_by"] = self.order_by_definition
+              end
+            end
+
+            unless operations['$set'].empty?
+              persist_content_type_changes target_content_type, operations
+            end
+          end
+        end
+
+        # Save the changes for the content type passed in parameter without forgetting
+        # to bump the version.. It also updates the recipe for related entries.
+        # That method does not call the Mongoid API but directly MongoDB.
+        #
+        # @param [ ContentType ] content_type The content type to update
+        # @param [ Hash ] operations The MongoDB atomic operations
+        #
+        def persist_content_type_changes(content_type, operations)
+          content_type.entries_custom_fields_version += 1
+
+          operations['$set']['entries_custom_fields_version'] = content_type.entries_custom_fields_version
+
+          self.collection.update({ '_id' => content_type._id }, operations)
+
+          collection, selector = content_type.entries.collection, content_type.entries.criteria.selector
+
+          collection.update selector, { '$set' => { 'custom_fields_recipe' => content_type.custom_fields_recipe_for(:entries) } }, :multi => true
+        end
+
+      end
+    end
+  end
+end

lib/locomotive/liquid/drops/content_entry.rb

@@ -46,8 +46,7 @@ module Locomotive
             value = self._source.send(meth)
 
             if value.respond_to?(:all)
-              # returns a mongoid criterion in order to chain pagination criteria
+              filter_and_order_list(value)
-              value.all
             else
               value
             end
@@ -56,6 +55,24 @@ module Locomotive
           end
         end
 
+        protected
+
+        def filter_and_order_list(list)
+          # filter ?
+          if @context['with_scope']
+            conditions  = HashWithIndifferentAccess.new(@context['with_scope'])
+            order_by    = conditions.delete(:order_by).try(:split)
+
+            if order_by.nil?
+              list.where(conditions).ordered
+            else
+              list.where(conditions).order_by(order_by)
+            end
+          else
+            list.ordered
+          end.all
+        end
+
       end
     end
   end

locomotive_cms.gemspec

@@ -30,7 +30,7 @@ Gem::Specification.new do |s|
   s.add_dependency 'mongoid',                         '~> 2.4.5'
   s.add_dependency 'locomotive-mongoid-tree',         '~> 0.6.2'
 
-  s.add_dependency 'custom_fields',                   '~> 2.0.0.rc6'
+  s.add_dependency 'custom_fields',                   '~> 2.0.0.rc7'
 
   s.add_dependency 'kaminari',                        '~> 0.13.0'
 

spec/lib/locomotive/liquid/drops/content_entry_spec.rb

@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe Locomotive::Liquid::Drops::ContentEntry do
+
+  before(:each) do
+    @list = mock('list')
+    @list.stubs(:all).returns(true)
+    @category = Locomotive::Liquid::Drops::ContentEntry.new(mock('category', :projects => @list))
+  end
+
+  context '#accessing a has_many relationship' do
+
+    it 'loops through the list' do
+      template = %({% for project in category.projects %}{{ project }},{% endfor %})
+
+      @list.expects(:ordered).returns(mock('criteria', :all => %w(a b)))
+
+      render(template, { 'category' => @category }).should == 'a,b,'
+    end
+
+    it 'filters the list' do
+      template = %({% with_scope order_by: 'name ASC', active: true %}{% for project in category.projects %}{{ project }},{% endfor %}{% endwith_scope %})
+
+      @list.expects(:order_by).with(['name', 'ASC']).returns(mock('criteria', :all => %w(a b)))
+      @list.expects(:where).with({ 'active' => true }).returns(@list)
+
+      render(template, { 'category' => @category }).should == 'a,b,'
+    end
+
+    it 'filters the list and uses the default order' do
+      template = %({% with_scope active: true %}{% for project in category.projects %}{{ project }},{% endfor %}{% endwith_scope %})
+
+      @list.expects(:ordered).returns(mock('criteria', :all => %w(a b)))
+      @list.expects(:where).with({ 'active' => true }).returns(@list)
+
+      render(template, { 'category' => @category }).should == 'a,b,'
+    end
+
+  end
+
+  def render(template, assigns = {})
+    liquid_context = ::Liquid::Context.new(assigns, {}, {})
+
+    output = ::Liquid::Template.parse(template).render(liquid_context)
+    output.gsub(/\n\s{0,}/, '')
+  end
+
+end

spec/lib/locomotive/liquid/drops/page_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Locomotive::Liquid::Drops::Page do
 
   before(:each) do
-    site = FactoryGirl.build(:site)
+    site  = FactoryGirl.build(:site)
     @home = FactoryGirl.build(:page, :site => site, :meta_keywords => 'Libidinous, Angsty', :meta_description => "Quite the combination.")
   end
 

spec/lib/locomotive/liquid/drops/site_spec.rb

@@ -3,9 +3,9 @@ require 'spec_helper'
 describe Locomotive::Liquid::Drops::Site do
 
   before(:each) do
-    @site = FactoryGirl.build(:site)
+    @site   = FactoryGirl.build(:site)
-    page_1 = FactoryGirl.build(:page, :site => @site)
+    page_1  = FactoryGirl.build(:page, :site => @site)
-    page_2 = FactoryGirl.build(:page, :site => @site, :title => 'About us', :slug => 'about_us')
+    page_2  = FactoryGirl.build(:page, :site => @site, :title => 'About us', :slug => 'about_us')
     @site.stubs(:pages).returns([page_1, page_2])
   end
 

spec/lib/locomotive/liquid/tags/with_scope_spec.rb

@@ -36,5 +36,4 @@ describe Locomotive::Liquid::Tags::WithScope do
     text.should == "true-foo"
   end
 
-
 end

spec/models/locomotive/content_entry_spec.rb

@@ -12,7 +12,7 @@ describe Locomotive::ContentEntry do
     @content_type.entries_custom_fields.build :label => 'Visible ?', :type => 'boolean', :name => 'visible'
     @content_type.entries_custom_fields.build :label => 'File', :type => 'file'
     @content_type.valid?
-    @content_type.send(:set_default_values)
+    @content_type.send(:set_label_field)
   end
 
   describe '#validation' do

spec/models/locomotive/content_type_spec.rb

@@ -108,46 +108,72 @@ describe Locomotive::ContentType do
 
   end
 
-  context '#group_by belongs_to field' do
+  describe 'belongs_to/has_many relationship' do
 
     before(:each) do
-      (@category_content_type = build_content_type(:name => 'Categories')).save!
+      build_belongs_to_has_many_relationship
-      @category_1 = @category_content_type.entries.create :name => 'A-developer'
-      @category_2 = @category_content_type.entries.create :name => 'B-designer'
 
-      @content_type = build_content_type.tap do |content_type|
+      @category_1 = @category_content_type.entries.create :name => 'Gems'
-        field = content_type.entries_custom_fields.build :label => 'Category', :type => 'belongs_to', :class_name => @category_1.class.to_s
+      @category_2 = @category_content_type.entries.create :name => 'Service'
-        content_type.group_by_field_id = field._id
+
-        content_type.save!
+      @content_1 = @content_type.entries.create :name => 'Github',        :category => @category_2
+      @content_2 = @content_type.entries.create :name => 'LocomotiveCMS', :category => @category_1, :description => 'Lorem ipsum',  :_position_in_category => 1
+      @content_3 = @content_type.entries.create :name => 'RubyOnRails',   :category => @category_1, :description => 'Zzzzzz',       :_position_in_category => 2
+    end
+
+    context '#ordering in a belongs_to/has_many relationship' do
+
+      it 'orders projects based on the default order of the Project content type' do
+        @category_1.projects.metadata.order.should == %w(name desc)
+        @category_1.projects.map(&:name).should == %w(RubyOnRails LocomotiveCMS)
+        @category_1.projects.ordered.all.map(&:name).should == %w(RubyOnRails LocomotiveCMS)
       end
-      @content_1 = @content_type.entries.create :name => 'Sacha', :category => @category_2
+
-      @content_2 = @content_type.entries.create :name => 'Did',   :category => @category_1
+      it 'updates the information about the order of a has_many relationship if the target class changes its order' do
-      @content_3 = @content_type.entries.create :name => 'Mario', :category => @category_1
+        @content_type.order_by = 'description'; @content_type.order_direction = 'ASC'; @content_type.save!
+        @category_1 = @category_1.class.find(@category_1._id)
+
+        @category_1.projects.metadata.order.should == %w(description ASC)
+        @category_1.projects.map(&:name).should == %w(LocomotiveCMS RubyOnRails)
+      end
+
+      it 'uses the order by position if the UI option is enabled' do
+        field = @category_content_type.entries_custom_fields.where(:name => 'projects').first
+        field.ui_enabled = true; @category_content_type.save!; @category_1 = @category_1.class.find(@category_1._id)
+
+        @category_1.projects.metadata.order.to_s.should == 'position_in_category'
+        @category_1.projects.map(&:name).should == %w(LocomotiveCMS RubyOnRails)
+      end
+
     end
 
-    it 'groups entries' do
+    context '#group_by belongs_to field' do
-      groups = @content_type.send(:group_by_belongs_to_field, @content_type.group_by_field)
 
-      groups.map { |h| h[:name] }.should == %w(A-developer B-designer)
+      it 'groups entries' do
+        groups = @content_type.send(:group_by_belongs_to_field, @content_type.group_by_field)
 
-      groups.first[:entries].map(&:name).should == %w(Did Mario)
+        groups.map { |h| h[:name] }.should == %w(Gems Service)
-      groups.last[:entries].map(&:name).should == %w(Sacha)
-    end
 
-    it 'groups entries with a different columns order' do
+        groups.first[:entries].map(&:name).should == %w(RubyOnRails LocomotiveCMS)
-      @category_content_type.update_attributes :order_by => @category_content_type.entries_custom_fields.first._id, :order_direction => 'desc'
+        groups.last[:entries].map(&:name).should == %w(Github)
-      groups = @content_type.send(:group_by_belongs_to_field, @content_type.group_by_field)
+      end
 
-      groups.map { |h| h[:name] }.should == %w(B-designer A-developer)
+      it 'groups entries with a different columns order' do
-    end
+        @category_content_type.update_attributes :order_by => @category_content_type.entries_custom_fields.first._id, :order_direction => 'desc'
+        groups = @content_type.send(:group_by_belongs_to_field, @content_type.group_by_field)
 
-    it 'deals with entries without a value for the group_by field (orphans)' do
+        groups.map { |h| h[:name] }.should == %w(Service Gems)
-      @content_type.entries.create :name => 'Paul'
+      end
-      groups = @content_type.send(:group_by_belongs_to_field, @content_type.group_by_field)
 
-      groups.map { |h| h[:name] }.should == ['A-developer', 'B-designer', nil]
+      it 'deals with entries without a value for the group_by field (orphans)' do
+        @content_type.entries.create :name => 'MacOsX'
+        groups = @content_type.send(:group_by_belongs_to_field, @content_type.group_by_field)
+
+        groups.map { |h| h[:name] }.should == ['Gems', 'Service', nil]
+
+        groups.last[:entries].map(&:name).should == %w(MacOsX)
+      end
 
-      groups.last[:entries].map(&:name).should == %w(Paul)
     end
 
   end
@@ -301,12 +327,13 @@ describe Locomotive::ContentType do
 
   end
 
-  def build_content_type(options = {})
+  def build_content_type(options = {}, &block)
     FactoryGirl.build(:content_type, options).tap do |content_type|
       content_type.entries_custom_fields.build :label => 'Name',        :type => 'string'
       content_type.entries_custom_fields.build :label => 'Description', :type => 'text'
       content_type.entries_custom_fields.build :label => 'Active',      :type => 'boolean'
       content_type.entries_custom_fields.build :label => 'Active at',   :type => 'date'
+      block.call(content_type) if block_given?
     end
   end
 
@@ -314,6 +341,23 @@ describe Locomotive::ContentType do
     content_type.entries.build(:name => 'Asset on steroids', :description => 'Lorem ipsum', :active => true)
   end
 
+  def build_belongs_to_has_many_relationship
+    (@category_content_type = build_content_type(:name => 'Categories')).save!
+    category_klass = @category_content_type.klass_with_custom_fields(:entries).name
+
+    @content_type = build_content_type.tap do |content_type|
+      field = content_type.entries_custom_fields.build :label => 'Category', :type => 'belongs_to', :class_name => category_klass
+      content_type.order_by           = 'name'
+      content_type.order_direction    = 'desc'
+      content_type.group_by_field_id  = field._id
+      content_type.save!
+    end
+    project_klass = @content_type.klass_with_custom_fields(:entries).name
+
+    field = @category_content_type.entries_custom_fields.build :label => 'Projects', :type => 'has_many', :class_name => project_klass, :inverse_of => :category, :ui_enabled => false
+    @category_content_type.save!
+  end
+
   def lookup_field_id(index)
     @content_type.entries_custom_fields.all[index].id.to_s
   end