From ab5a4755b82f3d405936e2895305a31fbee90738 Mon Sep 17 00:00:00 2001 From: Didier Lafforgue Date: Wed, 21 Mar 2012 03:21:29 +0100 Subject: [PATCH] make the editable elements more robust when working with localization + write tests to prove it --- app/models/locomotive/editable_element.rb | 25 +++++++- app/models/locomotive/editable_short_text.rb | 1 + .../extensions/page/editable_elements.rb | 14 +++-- .../locomotive/extensions/page/parse.rb | 2 + config/locales/default.en.yml | 2 +- config/locales/default.fr.yml | 2 +- lib/locomotive/liquid/errors.rb | 2 + lib/locomotive/liquid/tags/extends.rb | 4 +- .../locomotive/liquid/tags/extends_spec.rb | 46 ++++++++++++++ .../locomotive/editable_short_text_spec.rb | 60 +++++++++++++++++++ 10 files changed, 146 insertions(+), 12 deletions(-) create mode 100644 spec/lib/locomotive/liquid/tags/extends_spec.rb diff --git a/app/models/locomotive/editable_element.rb b/app/models/locomotive/editable_element.rb index 8ace61b22e..1cf75b4a4d 100644 --- a/app/models/locomotive/editable_element.rb +++ b/app/models/locomotive/editable_element.rb @@ -9,9 +9,9 @@ class EditableElement field :hint field :priority, :type => Integer, :default => 0 field :fixed, :type => Boolean, :default => false - field :disabled, :type => Boolean, :default => false + field :disabled, :type => Boolean, :default => false, :localize => true field :from_parent, :type => Boolean, :default => false - # field :locales, :type => Array TODO + field :locales, :type => Array, :default => [] ## associations ## embedded_in :page, :class_name => 'Locomotive::Page', :inverse_of => :editable_elements @@ -27,6 +27,16 @@ class EditableElement ## methods ## + def disabled? + !!self.disabled # the original method does not work quite well with the localization + end + + # Determines if the current element can be edited in the back-office + # + def editable? + !self.disabled? && self.locales.include?(::Mongoid::Fields::I18n.locale.to_s) && (!self.fixed? || !self.from_parent?) + end + def _run_rearrange_callbacks # callback from page/tree. not needed in the editable elements end @@ -52,10 +62,19 @@ def copy_attributes(attributes) # @param [ EditableElement] el The source element # def copy_attributes_from(el) - self.attributes = el.attributes.reject { |attr| !%w(slug block hint priority fixed disabled from_parent).include?(attr) } + self.attributes = el.attributes.reject { |attr| !%w(slug block hint priority fixed disabled locales from_parent).include?(attr) } self.from_parent = true end + # Make sure the current locale is added to the list + # of locales for the current element so that we know + # in which languages the element was translated. + # + def add_current_locale + locale = ::Mongoid::Fields::I18n.locale.to_s + self.locales << locale unless self.locales.include?(locale) + end + protected def _selector diff --git a/app/models/locomotive/editable_short_text.rb b/app/models/locomotive/editable_short_text.rb index f424e61e33..b519258458 100644 --- a/app/models/locomotive/editable_short_text.rb +++ b/app/models/locomotive/editable_short_text.rb @@ -8,6 +8,7 @@ class EditableShortText < EditableElement ## methods ## def content=(value) + self.add_current_locale self.default_content = false unless self.new_record? super end diff --git a/app/models/locomotive/extensions/page/editable_elements.rb b/app/models/locomotive/extensions/page/editable_elements.rb index f96d2f8080..da3ce22a40 100644 --- a/app/models/locomotive/extensions/page/editable_elements.rb +++ b/app/models/locomotive/extensions/page/editable_elements.rb @@ -26,7 +26,7 @@ def editable_element_blocks end def enabled_editable_elements - self.editable_elements.by_priority.reject { |el| el.disabled? || (el.fixed? && el.from_parent?) } + self.editable_elements.by_priority.find_all(&:editable?) # { |el| !el.editable? } end def editable_elements_grouped_by_blocks @@ -43,15 +43,17 @@ def find_editable_files end def add_or_update_editable_element(attributes, type) - # locale = attributes.delete(:locale) # TODO - element = self.find_editable_element(attributes[:block], attributes[:slug]) if element element.copy_attributes(attributes) else - self.editable_elements.build(attributes, type) + element = self.editable_elements.build(attributes, type) end + + element.add_current_locale + + element end def enable_editable_elements(block) @@ -68,7 +70,7 @@ def merge_editable_elements_from_page(source) new_el = self.editable_elements.build({}, el.class) new_el.copy_attributes_from(el) else - existing_el.attributes = { :disabled => false } + existing_el.disabled = false # = { :disabled => false } end end end @@ -77,7 +79,7 @@ def remove_disabled_editable_elements return unless self.editable_elements.any? { |el| el.disabled? } # super fast way to remove useless elements all in once - self.collection.update(self.atomic_selector, '$pull' => { 'editable_elements' => { 'disabled' => true } }) + self.collection.update(self.atomic_selector, '$pull' => { 'editable_elements' => { "disabled.#{::Mongoid::Fields::I18n.locale}" => true } }) end end diff --git a/app/models/locomotive/extensions/page/parse.rb b/app/models/locomotive/extensions/page/parse.rb index d73c3da5eb..fb1c933751 100644 --- a/app/models/locomotive/extensions/page/parse.rb +++ b/app/models/locomotive/extensions/page/parse.rb @@ -38,6 +38,8 @@ def serialize_template @parsing_errors << I18n.t(:liquid_syntax, :fullpath => self.fullpath, :error => error.to_s, :scope => [:errors, :messages, :page]) rescue ::Locomotive::Liquid::PageNotFound => error @parsing_errors << I18n.t(:liquid_extend, :fullpath => self.fullpath, :scope => [:errors, :messages, :page]) + rescue ::Locomotive::Liquid::PageNotTranslated => error + @parsing_errors << I18n.t(:liquid_translation, :fullpath => self.fullpath, :scope => [:errors, :messages, :page]) end end end diff --git a/config/locales/default.en.yml b/config/locales/default.en.yml index 9e16dfed7f..016cadf0fe 100644 --- a/config/locales/default.en.yml +++ b/config/locales/default.en.yml @@ -11,11 +11,11 @@ en: protected_page: "You can not remove index or 404 pages" extname_changed: "New file does not have the original extension" array_too_short: "is too small (minimum element number is %{count})" - liquid_syntax: "Liquid Syntax error ('%{error}')" invalid_theme_file: "can't be blank or isn't a zip file" page: liquid_syntax: "Liquid Syntax error ('%{error}' on '%{fullpath}')" liquid_extend: "The page '%{fullpath}' extends a template which does not exist" + liquid_translation: "The page '%{fullpath}' extends a template which is not translated" too_few_custom_fields: "At least, one custom field is required" security: "presents a security problem" diff --git a/config/locales/default.fr.yml b/config/locales/default.fr.yml index 0b9d318725..f8b0d25704 100644 --- a/config/locales/default.fr.yml +++ b/config/locales/default.fr.yml @@ -32,11 +32,11 @@ fr: protected_page: "Vous ne pouvez pas supprimer les pages index et 404" extname_changed: "Nouveau fichier n'a pas l'extension original" array_too_short: "est trop petit (le nombre minimum d'éléments est %{count})" - liquid_syntax: "Erreur de syntaxe ('%{error}')" security: "présente un problème de sécurité" page: liquid_syntax: "Erreur de syntaxe dans les sections de page, veuillez vérifier la syntaxe ('%{error}'/'%{fullpath}')" liquid_extend: "La page '%{fullpath}' étend le contenu d'une page qui n'existe pas" + liquid_translation: "La page '%{fullpath}' étend le contenu d'une page qui n'a pas été traduite" attributes: defaults: diff --git a/lib/locomotive/liquid/errors.rb b/lib/locomotive/liquid/errors.rb index 0f75090b29..a6e69f80de 100644 --- a/lib/locomotive/liquid/errors.rb +++ b/lib/locomotive/liquid/errors.rb @@ -1,5 +1,7 @@ module Locomotive module Liquid class PageNotFound < ::Liquid::Error; end + + class PageNotTranslated < ::Liquid::Error; end end end \ No newline at end of file diff --git a/lib/locomotive/liquid/tags/extends.rb b/lib/locomotive/liquid/tags/extends.rb index 64420593a1..3c49a4f16f 100644 --- a/lib/locomotive/liquid/tags/extends.rb +++ b/lib/locomotive/liquid/tags/extends.rb @@ -34,7 +34,9 @@ def parse_parent_template raise PageNotFound.new("Page with fullpath '#{@template_name}' was not found") if @context[:parent_page].nil? # be sure to work with a copy of the parent template otherwise there will be conflicts - parent_template = @context[:parent_page].template.clone + parent_template = @context[:parent_page].template.try(:clone) + + raise PageNotTranslated.new("Page with fullpath '#{@template_name}' was not translated") if parent_template.nil? @context[:parent_page].instance_variable_set(:@template, parent_template) diff --git a/spec/lib/locomotive/liquid/tags/extends_spec.rb b/spec/lib/locomotive/liquid/tags/extends_spec.rb new file mode 100644 index 0000000000..0a53f54d88 --- /dev/null +++ b/spec/lib/locomotive/liquid/tags/extends_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe Locomotive::Liquid::Tags::Extends do + + before(:each) do + @home = FactoryGirl.build(:page, :raw_template => 'Hello world') + @home.send :serialize_template + @home.instance_variable_set(:@template, nil) + + @site = FactoryGirl.build(:site) + @site.stubs(:pages).returns([@home]) + end + + it 'works' do + lambda { + page = FactoryGirl.build(:page, :slug => 'sub_page_1', :parent => @home) + parse('parent', page) + }.should_not raise_error + end + + context '#errors' do + + it 'raises an error if the source page does not exist' do + lambda { + @site.pages.expects(:where).with(:fullpath => 'foo').returns([]) + parse('foo') + }.should raise_error(Locomotive::Liquid::PageNotFound, "Page with fullpath 'foo' was not found") + end + + it 'raises an error if the source page is not translated' do + lambda { + ::Mongoid::Fields::I18n.with_locale 'fr' do + page = FactoryGirl.build(:page, :slug => 'sub_page_1', :parent => @home) + parse('parent', page) + end + }.should raise_error(Locomotive::Liquid::PageNotTranslated, "Page with fullpath 'parent' was not translated") + end + + end + + def parse(source = 'index', page = nil) + page ||= @home + Liquid::Template.parse("{% extends #{source} %}", { :site => @site, :page => page }) + end + +end diff --git a/spec/models/locomotive/editable_short_text_spec.rb b/spec/models/locomotive/editable_short_text_spec.rb index fcecaf3e8d..8b8c83903d 100644 --- a/spec/models/locomotive/editable_short_text_spec.rb +++ b/spec/models/locomotive/editable_short_text_spec.rb @@ -18,6 +18,66 @@ @sub_page_1_1 = FactoryGirl.create(:page, :slug => 'sub_page_1_1', :parent => @sub_page_1, :raw_template => "{% extends 'parent' %}") end + context '#locales' do + + it 'assigns the default locale' do + @sub_page_1_el.locales.should == ['en'] + end + + it 'adds new locale' do + ::Mongoid::Fields::I18n.with_locale 'fr' do + page = Locomotive::Page.find(@home._id) + page.editable_elements.first.content = 'Lorem ipsum (FR)' + page.save + page.editable_elements.first.locales.should == %w(en fr) + end + end + + it 'adds new locale within sub page elements' do + ::Mongoid::Fields::I18n.with_locale 'fr' do + @home.update_attributes :title => 'Accueil', :raw_template => "{% block body %}{% editable_short_text 'body' %}Lorem ipsum{% endeditable_short_text %}{% endblock %}" + page = Locomotive::Page.find(@sub_page_1._id) + page.editable_elements.first.content = 'Lorem ipsum (FR)' + page.save + page.editable_elements.first.locales.should == %w(en fr) + end + end + + end + + context '#editable?' do + + it 'is editable' do + @sub_page_1_el.editable?.should be_true + end + + it 'is not editable if the element is disabled' do + @sub_page_1_el.disabled = true + @sub_page_1_el.editable?.should be_false + end + + it 'is not editable if it is a fixed one' do + @sub_page_1_el.fixed = true + @sub_page_1_el.editable?.should be_false + end + + it 'is not editable if it does exist for the current locale' do + ::Mongoid::Fields::I18n.with_locale 'fr' do + @sub_page_1_el.editable?.should be_false + end + end + + it 'is among the editable elements of the page' do + @sub_page_1.enabled_editable_elements.map(&:slug).should == %w(body) + end + + it 'is not among the editable elements of the page if disabled' do + @sub_page_1_el.disabled = true + @sub_page_1.enabled_editable_elements.map(&:slug).should == [] + end + + end + context 'in sub pages level #1' do before(:each) do