Skip to content

Commit

Permalink
make the editable elements more robust when working with localization…
Browse files Browse the repository at this point in the history
… + write tests to prove it
  • Loading branch information
Didier Lafforgue committed Mar 21, 2012
1 parent e2f60f2 commit ab5a475
Show file tree
Hide file tree
Showing 10 changed files with 146 additions and 12 deletions.
25 changes: 22 additions & 3 deletions app/models/locomotive/editable_element.rb
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/models/locomotive/editable_short_text.rb
Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions app/models/locomotive/extensions/page/editable_elements.rb
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions app/models/locomotive/extensions/page/parse.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion config/locales/default.en.yml
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion config/locales/default.fr.yml
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions 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
4 changes: 3 additions & 1 deletion lib/locomotive/liquid/tags/extends.rb
Expand Up @@ -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)

Expand Down
46 changes: 46 additions & 0 deletions 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
60 changes: 60 additions & 0 deletions spec/models/locomotive/editable_short_text_spec.rb
Expand Up @@ -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
Expand Down

0 comments on commit ab5a475

Please sign in to comment.