preserve page orders when importing theme + handle favicon as theme asset + add tests for the nav tag + polish code (remove useless logs)

app/controllers/admin/imports_controller.rb

@@ -31,7 +31,9 @@ module Admin
         flash[:notice] = t("flash.admin.imports.create.#{Locomotive.config.delayed_job ? 'notice' : 'done'}")
 
         redirect_to Locomotive.config.delayed_job ? admin_import_url : new_admin_import_url
-      rescue
+      rescue Exception => e
+        logger.error "[Locomotive import] #{e.message}"
+
         @error = t('errors.messages.invalid_theme_file')
         flash[:alert] = t('flash.admin.imports.create.alert')
 

app/controllers/admin/pages_controller.rb

@@ -6,7 +6,6 @@ module Admin
     respond_to :json, :only => [:update, :sort, :get_path]
 
     def index
-      # @pages = current_site.pages.roots.minimal_attributes
       @pages = current_site.all_pages_in_once
     end
 

app/models/extensions/page/tree.rb

@@ -33,8 +33,6 @@ module Models
           def quick_tree(site)
             pages = site.pages.minimal_attributes.order_by([[:depth, :asc], [:position, :asc]]).to_a
 
-            puts "pages size = #{pages.size}"
-
             tmp = []
 
             while !pages.empty?
@@ -45,11 +43,9 @@ module Models
           end
 
           def _quick_tree(current_page, pages)
-            puts "_build_tree [current_page = #{current_page.title}] / #{pages.size}"
             i, children = 0, []
 
             while !pages.empty?
-              puts "...#{i}"
               page = pages[i]
 
               break if page.nil?
@@ -70,8 +66,6 @@ module Models
 
             current_page.children = children
 
-            puts "children size for #{current_page.title} = #{current_page.children.size}"
-
             current_page
           end
 
@@ -113,8 +107,12 @@ module Models
           def change_parent
             if self.parent_id_changed?
               self.fix_position(false)
+
+              unless self.parent_id_was.nil?
                 self.position = nil # make it move to bottom
                 self.add_to_list_bottom
+              end
+
               self.instance_variable_set :@_will_move, true
             end
           end

app/uploaders/asset_uploader.rb

@@ -60,7 +60,7 @@ class AssetUploader < CarrierWave::Uploader::Base
 
   def self.content_types
     {
-      :image      => ['image/jpeg', 'image/pjpeg', 'image/gif', 'image/png', 'image/x-png', 'image/jpg'],
+      :image      => ['image/jpeg', 'image/pjpeg', 'image/gif', 'image/png', 'image/x-png', 'image/jpg', 'image/x-icon'],
       :video      => [/^video/, 'application/x-shockwave-flash', 'application/x-swf'],
       :audio      => [/^audio/, 'application/ogg', 'application/x-mp3'],
       :pdf        => ['application/pdf', 'application/x-pdf'],

app/uploaders/theme_asset_uploader.rb

@@ -15,7 +15,7 @@ class ThemeAssetUploader < AssetUploader
   end
 
   def extension_white_list
-    %w(jpg jpeg gif png css js swf flv eot svg ttf woff otf)
+    %w(jpg jpeg gif png css js swf flv eot svg ttf woff otf ico)
   end
 
 end

lib/locomotive/import/pages.rb

@@ -36,7 +36,8 @@ module Locomotive
           :title        => fullpath.split('/').last.humanize,
           :slug         => fullpath.split('/').last,
           :parent       => parent,
-          :raw_template => template
+          :raw_template => template,
+          :published    => true
         }.merge(self.pages[fullpath] || {}).symbolize_keys
 
         # templatized ?
@@ -54,7 +55,7 @@ module Locomotive
 
         page.save!
 
-        self.log "adding #{page.fullpath}"
+        self.log "adding #{page.fullpath} / #{page.position}"
 
         site.reload
 
@@ -137,10 +138,23 @@ module Locomotive
         pages = context[:database]['site']['pages']
 
         if pages.is_a?(Array) # ordered list of pages
-          tmp = {}
+          tmp, positions = {}, Hash.new(0)
-          pages.each_with_index do |data, position|
+          pages.each do |data|
+            position = nil
+            fullpath = data.keys.first.to_s
+
+            unless %w(index 404).include?(fullpath)
+              (segments = fullpath.split('/')).pop
+              position_key = segments.empty? ? 'index' : segments.join('/')
+
+              position = positions[position_key]
+
+              positions[position_key] += 1
+            end
+
             attributes = (data.values.first || {}).merge(:position => position)
-            tmp[data.keys.first.to_s] = attributes
+
+            tmp[fullpath] = attributes
           end
           pages = tmp
         end

lib/locomotive/liquid/tags/nav.rb

@@ -17,10 +17,10 @@ module Locomotive
         def initialize(tag_name, markup, tokens, context)
           if markup =~ Syntax
             @source = ($1 || 'page').gsub(/"|'/, '')
-            @options = {}
+            @options = { :id => 'nav' }
-            markup.scan(::Liquid::TagAttributes) { |key, value| @options[key.to_sym] = value }
+            markup.scan(::Liquid::TagAttributes) { |key, value| @options[key.to_sym] = value.gsub(/"|'/, '') }
 
-            @options[:exclude] = Regexp.new(@options[:exclude].gsub(/"|'/, '')) if @options[:exclude]
+            @options[:exclude] = Regexp.new(@options[:exclude]) if @options[:exclude]
           else
             raise ::Liquid::SyntaxError.new("Syntax Error in 'nav' - Valid syntax: nav <page|site> <options>")
           end
@@ -29,6 +29,30 @@ module Locomotive
         end
 
         def render(context)
+          children_output = []
+
+          entries = fetch_entries(context)
+
+          entries.each_with_index do |p, index|
+            css = []
+            css << 'first' if index == 0
+            css << 'last' if index == entries.size - 1
+
+            children_output << render_entry_link(p, css.join(' '))
+          end
+
+          output = children_output.join("\n")
+
+          if @options[:no_wrapper] != 'true'
+            output = %{<ul id="#{@options[:id]}">\n#{output}</ul>}
+          end
+
+          output
+        end
+
+        private
+
+        def fetch_entries(context)
           @current_page = context.registers[:page]
 
           children = (case @source
@@ -37,29 +61,10 @@ module Locomotive
           when 'page'     then @current_page
           else
             context.registers[:site].pages.fullpath(@source).minimal_attributes.first
-          end).children_with_minimal_attributes
+          end).children_with_minimal_attributes.to_a
 
-          children_output = []
+          children.delete_if { |p| !include_page?(p) }
-
-          children.each_with_index do |p, index|
-            if include_page?(p)
-              css = ''
-              css = 'first' if index == 0
-              css = 'last' if index == children.size - 1
-              children_output << render_child_link(p, css)
         end
-          end
-
-          output = children_output.join("\n")
-
-           if @options[:no_wrapper] != 'true'
-             output = %{<ul id="nav">\n#{output}</ul>}
-           end
-
-          output
-        end
-
-        private
 
         def include_page?(page)
           if page.templatized? || !page.published?
@@ -71,8 +76,7 @@ module Locomotive
           end
         end
 
-        def render_child_link(page, css)
+        def render_entry_link(page, css)
-          # selected = @current_page._id == page._id ? ' on' : ''
           selected = @current_page.fullpath =~ /^#{page.fullpath}/ ? ' on' : ''
 
           icon = @options[:icon] ? '<span></span>' : ''

lib/locomotive/routing/site_dispatcher.rb

@@ -26,9 +26,11 @@ module Locomotive
         end
 
         def require_site
+          return true if current_site
+
           redirect_to admin_installation_url and return false if Account.count == 0 || Site.count == 0
 
-          render_no_site_error and return false if current_site.nil?
+          render_no_site_error and return false
         end
 
         def render_no_site_error

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

@@ -4,37 +4,62 @@ describe Locomotive::Liquid::Tags::Nav do
 
   before(:each) do
     @home = Factory.build(:page)
-    @home.stubs(:children).returns([
+    home_children = [
-      Page.new(:title => 'Child #1', :fullpath => 'child_1', :slug => 'child_1'),
+      Page.new(:title => 'Child #1', :fullpath => 'child_1', :slug => 'child_1', :published => true),
-      Page.new(:title => 'Child #2', :fullpath => 'child_2', :slug => 'child_2')
+      Page.new(:title => 'Child #2', :fullpath => 'child_2', :slug => 'child_2', :published => true)
-      ])
+    ]
-    @home.children.last.stubs(:children).returns([
+    @home.stubs(:children_with_minimal_attributes).returns(home_children)
-      Page.new(:title => 'Child #2.1', :fullpath => 'child_2/sub_child_1', :slug => 'sub_child_1'),
+    @home.stubs(:children).returns(home_children)
-      Page.new(:title => 'Child #2.2', :fullpath => 'child_2/sub_child_2', :slug => 'sub_child_2')
+
-      ])
+    other_children = [
+      Page.new(:title => 'Child #2.1', :fullpath => 'child_2/sub_child_1', :slug => 'sub_child_1', :published => true),
+      Page.new(:title => 'Child #2.2', :fullpath => 'child_2/sub_child_2', :slug => 'sub_child_2', :published => true)
+      ]
+    @home.children.last.stubs(:children_with_minimal_attributes).returns(other_children)
+    @home.children.last.stubs(:children).returns(other_children)
+
+    pages = [@home]
+    pages.stubs(:index).returns(pages)
+    pages.stubs(:minimal_attributes).returns(pages) # iso
     @site = Factory.build(:site)
-    @site.stubs(:pages).returns([@home])
+    @site.stubs(:pages).returns(pages)
   end
 
   context '#rendering' do
 
     it 'renders from site' do
-      render_nav.should == '<ul id="nav"><li id="child-1" class="link"><a href="/child_1">Child #1</a></li><li id="child-2" class="link"><a href="/child_2">Child #2</a></li></ul>'
+      render_nav.should == '<ul id="nav"><li id="child-1" class="link first"><a href="/child_1">Child #1</a></li><li id="child-2" class="link last"><a href="/child_2">Child #2</a></li></ul>'
     end
 
     it 'renders from page' do
+      render_nav('page').should == '<ul id="nav"><li id="child-1" class="link first"><a href="/child_1">Child #1</a></li><li id="child-2" class="link last"><a href="/child_2">Child #2</a></li></ul>'
+    end
+
+    it 'renders from parent' do
       (page = @home.children.last.children.first).stubs(:parent).returns(@home.children.last)
-      output = render_nav 'page', { :page => page }
+      output = render_nav 'parent', { :page => page }
-      output.should == '<ul id="nav"><li id="sub-child-1" class="link on"><a href="/child_2/sub_child_1">Child #2.1</a></li><li id="sub-child-2" class="link"><a href="/child_2/sub_child_2">Child #2.2</a></li></ul>'
+      output.should == '<ul id="nav"><li id="sub-child-1" class="link on first"><a href="/child_2/sub_child_1">Child #2.1</a></li><li id="sub-child-2" class="link last"><a href="/child_2/sub_child_2">Child #2.2</a></li></ul>'
     end
 
     it 'adds an icon before the link' do
-      render_nav('site', {}, 'icon: true').should match /<li id="child-1" class="link"><a href="\/child_1"><span><\/span>Child #1<\/a>/
+      render_nav('site', {}, 'icon: true').should match /<li id="child-1" class="link first"><a href="\/child_1"><span><\/span>Child #1<\/a>/
-      render_nav('site', {}, 'icon: before').should match /<li id="child-1" class="link"><a href="\/child_1"><span><\/span>Child #1<\/a>/
+      render_nav('site', {}, 'icon: before').should match /<li id="child-1" class="link first"><a href="\/child_1"><span><\/span>Child #1<\/a>/
     end
 
     it 'adds an icon after the link' do
-      render_nav('site', {}, 'icon: after').should match /<li id="child-1" class="link"><a href="\/child_1">Child #1<span><\/span><\/a><\/li>/
+      render_nav('site', {}, 'icon: after').should match /<li id="child-1" class="link first"><a href="\/child_1">Child #1<span><\/span><\/a><\/li>/
+    end
+
+    it 'assign a different dom id' do
+      render_nav('site', {}, 'id: "main-nav"').should match /<ul id="main-nav">/
+    end
+
+    it 'excludes entries based on a regexp' do
+      render_nav('page', {}, 'exclude: "child_1"').should == '<ul id="nav"><li id="child-2" class="link first last"><a href="/child_2">Child #2</a></li></ul>'
+    end
+
+    it 'does not render the parent ul' do
+      render_nav('site', {}, 'no_wrapper: true').should_not match /<ul id="nav">/
     end
 
   end