From 572ee360a11bbc8aa72cfe6505953ffee1f80300 Mon Sep 17 00:00:00 2001 From: dinedine Date: Mon, 3 Jan 2011 11:50:09 +0100 Subject: [PATCH] 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 | 4 +- app/controllers/admin/pages_controller.rb | 1 - app/models/extensions/page/tree.rb | 14 +++--- app/uploaders/asset_uploader.rb | 2 +- app/uploaders/theme_asset_uploader.rb | 2 +- lib/locomotive/import/pages.rb | 24 +++++++-- lib/locomotive/liquid/tags/nav.rb | 56 +++++++++++---------- lib/locomotive/routing/site_dispatcher.rb | 4 +- spec/lib/locomotive/liquid/tags/nav_spec.rb | 55 ++++++++++++++------ 9 files changed, 103 insertions(+), 59 deletions(-) diff --git a/app/controllers/admin/imports_controller.rb b/app/controllers/admin/imports_controller.rb index 8edad93e..f46e2cb5 100644 --- a/app/controllers/admin/imports_controller.rb +++ b/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') diff --git a/app/controllers/admin/pages_controller.rb b/app/controllers/admin/pages_controller.rb index 3e0db0f8..6433e000 100644 --- a/app/controllers/admin/pages_controller.rb +++ b/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 diff --git a/app/models/extensions/page/tree.rb b/app/models/extensions/page/tree.rb index c9da10bf..dc05c4f3 100644 --- a/app/models/extensions/page/tree.rb +++ b/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) - self.position = nil # make it move to bottom - self.add_to_list_bottom + + 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 diff --git a/app/uploaders/asset_uploader.rb b/app/uploaders/asset_uploader.rb index 57ec81fc..3017806e 100644 --- a/app/uploaders/asset_uploader.rb +++ b/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'], diff --git a/app/uploaders/theme_asset_uploader.rb b/app/uploaders/theme_asset_uploader.rb index a7793848..4e18cde3 100644 --- a/app/uploaders/theme_asset_uploader.rb +++ b/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 diff --git a/lib/locomotive/import/pages.rb b/lib/locomotive/import/pages.rb index 4930c4c8..fa68235b 100644 --- a/lib/locomotive/import/pages.rb +++ b/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 = {} - pages.each_with_index do |data, position| + tmp, positions = {}, Hash.new(0) + 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 diff --git a/lib/locomotive/liquid/tags/nav.rb b/lib/locomotive/liquid/tags/nav.rb index aeb496d2..b14922a6 100644 --- a/lib/locomotive/liquid/tags/nav.rb +++ b/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 = {} - markup.scan(::Liquid::TagAttributes) { |key, value| @options[key.to_sym] = value } + @options = { :id => 'nav' } + 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 ") 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 = %{
    \n#{output}
} + end + + output + end + + private + + def fetch_entries(context) @current_page = context.registers[:page] children = (case @source @@ -37,30 +61,11 @@ 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.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 = %{} - end - - output + children.delete_if { |p| !include_page?(p) } end - private - def include_page?(page) if page.templatized? || !page.published? false @@ -71,8 +76,7 @@ module Locomotive end end - def render_child_link(page, css) - # selected = @current_page._id == page._id ? ' on' : '' + def render_entry_link(page, css) selected = @current_page.fullpath =~ /^#{page.fullpath}/ ? ' on' : '' icon = @options[:icon] ? '' : '' diff --git a/lib/locomotive/routing/site_dispatcher.rb b/lib/locomotive/routing/site_dispatcher.rb index 126621b3..1678d3b3 100644 --- a/lib/locomotive/routing/site_dispatcher.rb +++ b/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 diff --git a/spec/lib/locomotive/liquid/tags/nav_spec.rb b/spec/lib/locomotive/liquid/tags/nav_spec.rb index 327eda30..47f15de7 100644 --- a/spec/lib/locomotive/liquid/tags/nav_spec.rb +++ b/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([ - Page.new(:title => 'Child #1', :fullpath => 'child_1', :slug => 'child_1'), - Page.new(:title => 'Child #2', :fullpath => 'child_2', :slug => 'child_2') - ]) - @home.children.last.stubs(:children).returns([ - Page.new(:title => 'Child #2.1', :fullpath => 'child_2/sub_child_1', :slug => 'sub_child_1'), - Page.new(:title => 'Child #2.2', :fullpath => 'child_2/sub_child_2', :slug => 'sub_child_2') - ]) + home_children = [ + Page.new(:title => 'Child #1', :fullpath => 'child_1', :slug => 'child_1', :published => true), + Page.new(:title => 'Child #2', :fullpath => 'child_2', :slug => 'child_2', :published => true) + ] + @home.stubs(:children_with_minimal_attributes).returns(home_children) + @home.stubs(:children).returns(home_children) + + 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 == '' + render_nav.should == '' end it 'renders from page' do + render_nav('page').should == '' + 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.should == '' + output = render_nav 'parent', { :page => page } + output.should == '' end it 'adds an icon before the link' do - render_nav('site', {}, 'icon: true').should match /