refactor the code (move some pieces of code into the custom_fields gem) + solve a ton of bugs: circular dependencies inside the content_entry presenter, fix Locomotive.mounted_on (broken in rails 3.2), js errors when creating a page, locale switcher broken if a page was not translated in the target locale

app/assets/javascripts/locomotive/models/content_entry.js.coffee

@@ -7,12 +7,12 @@ class Locomotive.Models.ContentEntry extends Backbone.Model
   initialize: ->
     @urlRoot = @urlRoot.replace(':slug', @get('content_type_slug'))
 
-    _.each @get('_has_many_fields'), (field) =>
+    _.each @get('has_many_custom_fields'), (field) =>
       name = field[0]
       collection = new Locomotive.Models.ContentEntriesCollection(@get(name))
       @set_attribute name, collection
 
-    _.each @get('_many_to_many_fields'), (field) =>
+    _.each @get('many_to_many_custom_fields'), (field) =>
       name = field[0]
       collection = new Locomotive.Models.ContentEntriesCollection(@get(name))
       collection.comparator = (entry) -> entry.get('__position') || 0
@@ -24,7 +24,7 @@ class Locomotive.Models.ContentEntry extends Backbone.Model
     @set data
 
   update_attributes: (attributes) ->
-    _.each attributes._file_fields, (field) => # special treatment for files
+    _.each attributes.file_custom_fields, (field) => # special treatment for files
       attribute = "#{field}_url"
       @set_attribute attribute, attributes[attribute]
       @set_attribute "remove_#{field}", false
@@ -42,11 +42,11 @@ class Locomotive.Models.ContentEntry extends Backbone.Model
         unless _.include(@get('safe_attributes'), key)
           delete hash[key]
 
-      _.each @get('_has_many_fields'), (field) => # include the has_many relationships
+      _.each @get('has_many_custom_fields'), (field) => # include the has_many relationships
         name = field[0]
         hash["#{name}_attributes"] = @get(name).toMinJSON()
 
-      _.each @get('_many_to_many_fields'), (field) => # include the many_to_many relationships
+      _.each @get('many_to_many_custom_fields'), (field) => # include the many_to_many relationships
         name = field[0]; setter_name = field[1]
         hash[setter_name] = @get(name).sort().map (entry) => entry.id
 

app/assets/javascripts/locomotive/models/page.js.coffee

@@ -12,10 +12,13 @@ class Locomotive.Models.Page extends Backbone.Model
 
   _normalize: ->
     @set
-      editable_elements: new Locomotive.Models.EditableElementsCollection(@get('editable_elements'))
+      editable_elements: new Locomotive.Models.EditableElementsCollection(@get('editable_elements') || [])
 
   toJSON: ->
     _.tap super, (hash) =>
-      hash.editable_elements = @get('editable_elements').toJSONForSave() if @get('editable_elements')
+      _.each ['content_type_id_text', 'edit_url', 'parent_id_text'], (key) => delete hash[key]
+
+      delete hash['editable_elements']
+      hash.editable_elements = @get('editable_elements').toJSONForSave() if @get('editable_elements')? && @get('editable_elements').length > 0
 
 class Locomotive.Models.PagesCollection extends Backbone.Collection

app/assets/javascripts/locomotive/views/content_entries/_form_view.js.coffee

@@ -56,7 +56,7 @@ class Locomotive.Views.ContentEntries.FormView extends Locomotive.Views.Shared.F
       $(textarea).tinymce(settings)
 
   enable_file_fields: ->
-    _.each @model.get('_file_fields'), (name) =>
+    _.each @model.get('file_custom_fields'), (name) =>
       view = new Locomotive.Views.Shared.Fields.FileView model: @model, name: name
 
       @_file_field_views.push(view)
@@ -64,7 +64,7 @@ class Locomotive.Views.ContentEntries.FormView extends Locomotive.Views.Shared.F
       @$("##{@model.paramRoot}_#{name}_input").append(view.render().el)
 
   enable_has_many_fields: ->
-    _.each @model.get('_has_many_fields'), (field) =>
+    _.each @model.get('has_many_custom_fields'), (field) =>
       name = field[0]; inverse_of = field[1]
       new_entry = new Locomotive.Models.ContentEntry(@options["#{name}_new_entry"])
       view      = new Locomotive.Views.Shared.Fields.HasManyView model: @model, name: name, new_entry: new_entry, inverse_of: inverse_of
@@ -74,7 +74,7 @@ class Locomotive.Views.ContentEntries.FormView extends Locomotive.Views.Shared.F
       @$("##{@model.paramRoot}_#{name}_input label").after(view.render().el)
 
   enable_many_to_many_fields: ->
-    _.each @model.get('_many_to_many_fields'), (field) =>
+    _.each @model.get('many_to_many_custom_fields'), (field) =>
       name = field[0]
       view = new Locomotive.Views.Shared.Fields.ManyToManyView model: @model, name: name, all_entries: @options["all_#{name}_entries"]
 

app/controllers/locomotive/pages_controller.rb

@@ -52,7 +52,7 @@ module Locomotive
 
     def get_path
       page = current_site.pages.build(:parent => current_site.pages.find(params[:parent_id]), :slug => params[:slug].permalink)
-
+      page.send(:build_fullpath)
       render :json => { :url => public_page_url(page), :slug => page.slug }
     end
 

app/helpers/locomotive/accounts_helper.rb

@@ -1,4 +1,5 @@
-module Locomotive::AccountsHelper
+module Locomotive
+  module AccountsHelper
 
     def admin_on?(site = current_site)
       site.memberships.detect { |m| m.admin? && m.account == current_locomotive_account }
@@ -9,4 +10,4 @@ module Locomotive::AccountsHelper
     end
 
   end
-
+end

app/helpers/locomotive/base_helper.rb

@@ -1,4 +1,5 @@
-module Locomotive::BaseHelper
+module Locomotive
+  module BaseHelper
 
     def title(title = nil)
       if title.nil?
@@ -112,3 +113,4 @@ module Locomotive::BaseHelper
 
 
   end
+end

app/helpers/locomotive/content_entries_helper.rb

@@ -1,4 +1,5 @@
-module Locomotive::ContentEntriesHelper
+module Locomotive
+  module ContentEntriesHelper
 
     def options_for_belongs_to_custom_field(class_name)
       content_type = Locomotive::ContentType.class_name_to_content_type(class_name, current_site)
@@ -11,3 +12,4 @@ module Locomotive::ContentEntriesHelper
     end
 
   end
+end

app/helpers/locomotive/content_types_helper.rb

@@ -1,4 +1,5 @@
-module Locomotive::ContentTypesHelper
+module Locomotive
+  module ContentTypesHelper
 
     # Iterates over the content types with the following rules
     # - content types are ordered by the updated_at date (DESC)
@@ -64,3 +65,4 @@ module Locomotive::ContentTypesHelper
     end
 
   end
+end

app/helpers/locomotive/custom_fields_helper.rb

@@ -1,4 +1,5 @@
-module Locomotive::CustomFieldsHelper
+module Locomotive
+  module CustomFieldsHelper
 
     def options_for_custom_field_type
       %w(string text select boolean date file belongs_to has_many many_to_many).map do |type|
@@ -64,103 +65,5 @@ module Locomotive::CustomFieldsHelper
       end
     end
 
-  # def options_for_association_target
+  end
-  #   current_site.reload.content_types.collect { |c| [c.name, c.content_klass.to_s] }
-  # end
-  #
-  # def options_for_reverse_lookups(my_content_type)
-  #   klass_name = my_content_type.content_klass.to_s
-  #
-  #   [].tap do |options|
-  #     ContentType.where(:'entries_custom_fields.kind' => 'has_one', :'entries_custom_fields.target' => klass_name).each do |content_type|
-  #       content_type.entries_custom_fields.find_all { |f| f.has_one? && f.target == klass_name }.each do |field|
-  #         options << {
-  #           :klass  => content_type.content_klass.to_s,
-  #           :label  => field.label,
-  #           :name   => field._name
-  #         }
-  #       end
-  #     end
-  #   end
-  # end
-  #
-  # def filter_options_for_reverse_has_many(contents, reverse_lookup, object)
-  #   # Only display items which don't belong to a different object
-  #   contents.reject do |c|
-  #     owner = c.send(reverse_lookup.to_sym)
-  #     !(owner.nil? || owner == object._id)
-  #   end
-  # end
-  #
-  # def options_for_has_one(field, value)
-  #   self.options_for_has_one_or_has_many(field) do |groups|
-  #     grouped_options_for_select(groups.collect do |g|
-  #       if g[:items].empty?
-  #         nil
-  #       else
-  #         [g[:name], g[:items].collect { |c| [c._label, c._id] }]
-  #       end
-  #     end.compact, value)
-  #   end
-  # end
-  #
-  # def options_for_has_many(field, content = nil)
-  #   self.options_for_has_one_or_has_many(field, content)
-  # end
-  #
-  # def options_for_has_one_or_has_many(field, content = nil, &block)
-  #   content_type = field.target.constantize._parent.reload
-  #
-  #   if content_type.groupable?
-  #     grouped_contents = content_type.list_or_group_contents
-  #
-  #     grouped_contents.each do |g|
-  #       g[:items] = filter_options_for_reverse_has_many(g[:items], field.reverse_lookup, content)
-  #     end if field.reverse_has_many?
-  #
-  #     if block_given?
-  #       block.call(grouped_contents)
-  #     else
-  #       grouped_contents.collect do |g|
-  #         if g[:items].empty?
-  #           nil
-  #         else
-  #           { :name => g[:name], :items => g[:items].collect { |c| [c._label, c._id] } }
-  #         end
-  #       end.compact
-  #     end
-  #   else
-  #     contents = content_type.ordered_contents
-  #
-  #     if field.reverse_has_many?
-  #       contents = filter_options_for_reverse_has_many(contents, field.reverse_lookup, content)
-  #     end
-  #
-  #     contents.collect { |c| [c._label, c._id] }
-  #   end
-  # end
-  #
-  # def has_many_data_to_js(field, content)
-  #   options = {
-  #     :taken_ids => content.send(field._alias.to_sym).ids
-  #   }
-  #
-  #   if !content.new_record? && field.reverse_has_many?
-  #     url_options =  {
-  #       :breadcrumb_alias => field.reverse_lookup_alias,
-  #       "content[#{field.reverse_lookup_alias}]" => content._id
-  #     }
-  #
-  #     options.merge!(
-  #       :new_item   => {
-  #         :label  => t('locomotive.contents.form.has_many.new_item'),
-  #         :url    => new_content_url(field.target_klass._parent.slug, url_options)
-  #       },
-  #       :edit_item_url => edit_content_url(field.target_klass._parent.slug, 42, url_options)
-  #     )
-  #   end
-  #
-  #   collection_to_js(options_for_has_many(field, content), options)
-  # end
-
 end

app/helpers/locomotive/installation_helper.rb

@@ -1,7 +1,9 @@
-module Locomotive::InstallationHelper
+module Locomotive
+  module InstallationHelper
 
     def next_installation_step_link(step = 1, label = nil)
       link_to(content_tag(:span, label || t('admin.installation.common.next')), installation_step_url(step), :class => 'button')
     end
 
   end
+end

app/helpers/locomotive/pages_helper.rb

@@ -1,4 +1,5 @@
-module Locomotive::PagesHelper
+module Locomotive
+  module PagesHelper
 
     def css_for_page(page)
       %w(index not_found templatized redirect).inject([]) do |memo, state|
@@ -22,7 +23,7 @@ module Locomotive::PagesHelper
     end
 
     def add_children_to_options(page, list)
-    return list if page.path.include?(@page.id) || page == @page
+      return list if page.parent_ids.include?(@page.id) || page == @page
 
       offset = '- ' * (page.depth || 0) * 2
 
@@ -43,3 +44,4 @@ module Locomotive::PagesHelper
     end
 
   end
+end

app/helpers/locomotive/sites_helper.rb

@@ -1,7 +1,9 @@
-module Locomotive::SitesHelper
+module Locomotive
+  module SitesHelper
 
     def ordered_current_site_locales
       current_site.locales + (Locomotive.config.site_locales - current_site.locales)
     end
 
   end
+end

app/helpers/locomotive/theme_assets_helper.rb

@@ -1,4 +1,5 @@
-module Locomotive::ThemeAssetsHelper
+module Locomotive
+  module ThemeAssetsHelper
 
     def image_dimensions_and_size(asset)
       content_tag(:small, "#{asset.width}px x #{asset.height}px | #{number_to_human_size(asset.size)}")
@@ -22,3 +23,4 @@ module Locomotive::ThemeAssetsHelper
 
 
   end
+end

app/models/locomotive/extensions/site/locales.rb

@@ -36,7 +36,7 @@ module Locomotive
           #
           def localized_page_fullpath(page, locale = nil)
             locale = (locale || I18n.locale).to_s
-            fullpath = page.fullpath_translations[locale]
+            fullpath = page.fullpath_translations[locale] || page.fullpath_translations[self.default_locale]
 
             locale == self.default_locale ? fullpath : File.join(locale, fullpath)
           end

app/presenters/locomotive/base_presenter.rb

@@ -5,15 +5,16 @@ class Locomotive::BasePresenter
   include ActionView::Helpers::TextHelper
   include ActionView::Helpers::NumberHelper
 
-  attr_reader :source, :options, :ability
+  attr_reader :source, :options, :ability, :depth
 
   delegate :created_at, :updated_at, :to => :source
 
   def initialize(object, options = {})
     @source   = object
-    @options  = options
+    @options  = options || {}
+    @depth    = options[:depth] || 0
 
-    if @options && @options[:current_account] && @options[:current_site]
+    if @options[:current_account] && @options[:current_site]
       @ability = Locomotive::Ability.new @options[:current_account], @options[:current_site]
     end
   end

app/presenters/locomotive/content_entry_presenter.rb

@@ -1,44 +1,25 @@
 module Locomotive
   class ContentEntryPresenter < BasePresenter
 
-    delegate :_label, :_slug, :_position, :seo_title, :meta_keywords, :meta_description, :to => :source
+    delegate :_label, :_slug, :_position, :seo_title, :meta_keywords, :meta_description, :file_custom_fields, :has_many_custom_fields, :many_to_many_custom_fields, :to => :source
 
-    # Returns the value of a field in the context of the current entry.
+    # Lists of all the attributes editable thru the html form for instance
-    #
-    # @params [ CustomFields::Field ] field The field
-    #
-    # @returns [ Object ] The value of the field for the entry
-    #
-    def value_for(field)
-      getter = [*self.getters_for(field.name, field.type)].first.to_sym
-      self.source.send(getter)
-    end
-
-    # Returns the list of getters for an entry
-    #
-    # @returns [ List ] a list of method names (string)
-    #
-    def custom_fields_methods
-      self.source.custom_fields_recipe['rules'].map do |rule|
-        self.getters_for rule['name'], rule['type']
-      end.flatten
-    end
-
-    # Lists of all the attributes editable by a html form
     #
     # @returns [ List ] a list of attributes (string)
     #
     def safe_attributes
-      self.source.custom_fields_recipe['rules'].map do |rule|
+      self.source.custom_fields_safe_attributes + %w(_slug seo_title meta_keywords meta_description _destroy)
-        case rule['type'].to_sym
+    end
-        when :date                    then "formatted_#{rule['name']}"
+
-        when :file                    then [rule['name'], "remove_#{rule['name']}"]
+    def filtered_custom_fields_methods
-        when :select, :belongs_to     then ["#{rule['name']}_id", "position_in_#{rule['name']}"]
+      self.source.custom_fields_methods do |rule|
-        when :has_many, :many_to_many then nil
+        if self.source.is_a_custom_field_many_relationship?(rule['name'])
-        else
+          # avoid circular dependencies, it should accept only one deep level
-          rule['name']
+          self.depth == 0
+        else
+          true
+        end
       end
-      end.compact.flatten + %w(_slug seo_title meta_keywords meta_description _destroy)
     end
 
     def errors
@@ -49,59 +30,24 @@ module Locomotive
       self.source.content_type.slug
     end
 
-    def _file_fields
-      group_fields 'file'
-    end
-
-    def _has_many_fields
-      group_fields('has_many') { |rule| [rule['name'], rule['inverse_of']] }
-    end
-
-    def _many_to_many_fields
-      group_fields('many_to_many') { |rule| [rule['name'], "#{rule['name'].singularize}_ids"] }
-    end
-
     def included_methods
-      default_list = %w(_label _slug _position content_type_slug _file_fields _has_many_fields _many_to_many_fields safe_attributes)
+      default_list = %w(_label _slug _position content_type_slug file_custom_fields has_many_custom_fields many_to_many_custom_fields safe_attributes)
       default_list << 'errors' if !!self.options[:include_errors]
-      super + self.custom_fields_methods + default_list
+      super + self.filtered_custom_fields_methods + default_list
     end
 
     def method_missing(meth, *arguments, &block)
-      if self.custom_fields_methods.include?(meth.to_s)
+      if self.source.custom_fields_methods.include?(meth.to_s)
+        if self.source.is_a_custom_field_many_relationship?(meth.to_s)
+          # go deeper
+          self.source.send(meth).map { |entry| entry.to_presenter(:depth => self.depth + 1) }
+        else
           self.source.send(meth) rescue nil
+        end
       else
         super
       end
     end
 
-    protected
-
-    # Gets the names of the getter methods for a field.
-    # The names depend on the field type.
-    #
-    # @params [ String ] name Name of the field
-    # @params [ String ] type Type of the field
-    #
-    # @returns [ Object ] A string or an array of names
-    def getters_for(name, type)
-      case type.to_sym
-      when :select      then [name, "#{name}_id"]
-      when :date        then "formatted_#{name}"
-      when :file        then "#{name}_url"
-      when :belongs_to  then "#{name}_id"
-      else
-        name
-      end
-    end
-
-    def group_fields(type, &block)
-      unless block_given?
-        block = lambda { |rule| rule['name'] }
-      end
-
-      self.source.custom_fields_recipe['rules'].find_all { |rule| rule['type'] == type }.map(&block)
-    end
-
   end
 end

app/presenters/locomotive/page_presenter.rb

@@ -12,7 +12,7 @@ module Locomotive
     end
 
     def included_methods
-      super + %w(title slug fullpath raw_template published listed templatized redirect redirect_url cache_strategy template_changed editable_elements edit_url localized_fullpaths)
+      super + %w(title slug fullpath raw_template published listed templatized redirect redirect_url cache_strategy template_changed editable_elements localized_fullpaths)
     end
 
     def localized_fullpaths

doc/TODO

@@ -104,11 +104,12 @@ x deployment
     x data ?
 x script to migrate existing site
   x i18n
-- upgrade to rails 3.2 (https://github.com/locomotivecms/engine/pull/281/files)
+x upgrade to rails 3.2 (https://github.com/locomotivecms/engine/pull/281/files)
-- missing custom_fields
+x missing custom_fields
   x belongs_to
   x has_many
-  - many_to_many
+  x many_to_many
+- simplify cells integration when modifying a menu from the main app
 - heroku module for locomotive
 - refactoring
   x remove the import / export scripts

lib/locomotive.rb

@@ -122,7 +122,7 @@ module Locomotive
   end
 
   def self.mounted_on
-    Rails.application.routes.named_routes[:locomotive].path
+    Rails.application.routes.named_routes[:locomotive].path.spec.to_s
   end
 
   protected

lib/locomotive/liquid/tags/locale_switcher.rb

@@ -61,7 +61,7 @@ module Locomotive
           case @options[:label]
           when :iso     then locale
           when :locale  then I18n.t("locomotive.locales.#{locale}", :locale => locale)
-          when :title   then @page.title
+          when :title   then @page.title # FIXME: this returns nil if the page has not been translated in the locale
           else
             locale
           end

lib/locomotive/render.rb

@@ -91,7 +91,7 @@ module Locomotive
           :current_locomotive_account => current_locomotive_account
         }
 
-        ::Liquid::Context.new({}, assigns, registers, false) # pass false to true to enable the re-thrown exception flag
+        ::Liquid::Context.new({}, assigns, registers, false) # switch from false to true to enable the re-thrown exception flag
       end
 
       def prepare_and_set_response(output)