From 0b71714d44de2b12bfdf6aa63339aab727ff3b7e Mon Sep 17 00:00:00 2001 From: Christophe Vilayphiou Date: Fri, 17 Feb 2012 14:54:11 +0800 Subject: [PATCH] Improve code in items, pages, links --- app/controllers/admin/items_controller.rb | 15 ------------- app/controllers/admin/links_controller.rb | 6 +---- app/controllers/admin/pages_controller.rb | 12 +++++----- app/controllers/pages_controller.rb | 17 ++++++++------ app/models/item.rb | 9 ++------ app/views/admin/links/_form.html.erb | 14 +++++++----- app/views/admin/pages/_form.html.erb | 27 +++++++++-------------- 7 files changed, 37 insertions(+), 63 deletions(-) diff --git a/app/controllers/admin/items_controller.rb b/app/controllers/admin/items_controller.rb index 64364691..33bc0fd2 100644 --- a/app/controllers/admin/items_controller.rb +++ b/app/controllers/admin/items_controller.rb @@ -15,21 +15,6 @@ class Admin::ItemsController < ApplicationController @item = get_homepage end end - - -#TODO -# Allow to move items down and up different parents -# def up -# @item = Item.find(params[:id]) -# @item.move_higher -# redirect_to admin_items_url( :parent_name => @item.parent_name ) -# end -# -# def down -# @item = Item.find(params[:id]) -# @item.move_lower -# redirect_to admin_items_url( :parent_name => @item.parent_name ) -# end protected diff --git a/app/controllers/admin/links_controller.rb b/app/controllers/admin/links_controller.rb index 57b1f775..533ff829 100644 --- a/app/controllers/admin/links_controller.rb +++ b/app/controllers/admin/links_controller.rb @@ -12,13 +12,11 @@ class Admin::LinksController < ApplicationController def new @item = Link.new - @item.is_published = true - @item.parent_id = Page.find(params[:parent_id]).id rescue nil + @item.parent = Page.find(params[:parent_id]) rescue nil end def edit @item = Link.find(params[:id]) - @i18n_variable = @item.i18n_variable end def create @@ -34,7 +32,6 @@ class Admin::LinksController < ApplicationController end else flash.now[:error] = t('admin.create_error_link') - @i18n_variable = @item.i18n_variable render :action => "new" end end @@ -52,7 +49,6 @@ class Admin::LinksController < ApplicationController end else flash.now[:error] = t('admin.update_error_link') - @i18n_variable = @item.i18n_variable render :action => "edit" end end diff --git a/app/controllers/admin/pages_controller.rb b/app/controllers/admin/pages_controller.rb index 7d896143..d1a465ba 100644 --- a/app/controllers/admin/pages_controller.rb +++ b/app/controllers/admin/pages_controller.rb @@ -19,19 +19,17 @@ class Admin::PagesController < ApplicationController def new @item = Page.new + @item.parent = Item.find(params[:parent_id]) rescue nil @apps = ModuleApp.all - @item.is_published = true - @item.parent_id = @parent_item.id rescue nil @designs = Design.all.entries - @default_design = Design.first + @design = Design.first end def edit @item = Page.find(params[:id]) @apps = ModuleApp.all - @i18n_variable = @item.i18n_variable @designs = Design.all.entries - @design = @item.design + @design = @item.design ? @item.design : @designs.first @app_frontend_urls = @item.module_app.app_pages if @item.module_app end @@ -47,7 +45,9 @@ class Admin::PagesController < ApplicationController end else flash.now[:error] = t('admin.create_error_page') - @i18n_variable = @item.i18n_variable + @apps = ModuleApp.all + @designs = Design.all.entries + @design = Design.first render :action => "new" end end diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 4af02e9e..af5f412a 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -13,13 +13,16 @@ class PagesController < ApplicationController def show #begin - item = Item.first(:conditions => {:full_name => params[:page_name]}) - case item._type - when 'Page' - @item = item - render_page(params[:id]) - when 'Link' - redirect_to "http://#{item[:url]}" + @item = Item.first(:conditions => {:full_name => params[:page_name]}) + if @item.is_published + case @item._type + when 'Page' + render_page(params[:id]) + when 'Link' + redirect_to "http://#{@item[:url]}" + end + else + render :file => "#{Rails.root}/public/404.html", :status => :not_found end #rescue # render :file => "#{Rails.root}/public/404.html", :status => :not_found diff --git a/app/models/item.rb b/app/models/item.rb index 6bda5209..44c6762d 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -5,15 +5,13 @@ class Item field :name, :index => true field :full_name, :index => true - field :parent_id, :index => true - field :parent_name field :position, :type => Integer - field :is_published, :type => Boolean, :default => true, :index => true + field :is_published, :type => Boolean, :default => false, :index => true validates_format_of :name, :with => /^[0-9a-zA-Z\-_]+$/ validates :name, :exclusion => { :in => LIST[:forbidden_item_names] } validates_uniqueness_of :name, :scope => :parent_id - validates_presence_of :name, :full_name, :position, :is_published + validates_presence_of :name, :full_name, :position belongs_to :parent, :class_name => "Item" has_one :i18n_variable, :as => :language_value, :autosave => true, :dependent => :destroy @@ -47,9 +45,6 @@ class Item self.position = (max_page)? max_page + 1 : 1 end - # Set the parent value - self.parent_name = Item.first(:conditions => {:id => self.parent_id} ).name rescue nil - # Build the full_name from the ancestors array full_node = self.ancestors.map{ |a| a.name }.push( self.name ) # Remove root node if not root diff --git a/app/views/admin/links/_form.html.erb b/app/views/admin/links/_form.html.erb index 57c5bfeb..2f03b968 100644 --- a/app/views/admin/links/_form.html.erb +++ b/app/views/admin/links/_form.html.erb @@ -1,16 +1,18 @@ <%= f.error_messages %> -<%= f.hidden_field :parent_id %> +<%= f.hidden_field :parent, :value => @item.parent.id %>

<%= f.label :name, "Name" %> <%= f.text_field :name, :class => 'text' %>

-<% @site_valid_locales.each do |locale| %> -

- <%= label_tag "link[title]", "#{t('admin.title')} #{locale}" %> - <%= text_field_tag "link[i18n_variable][#{locale}]", (@i18n_variable[locale] if @i18n_variable), :class => 'text' %> -

+<%= f.fields_for :i18n_variable, (@item.new_record? ? @item.build_i18n_variable : @item.i18n_variable) do |f| %> + <% @site_valid_locales.each do |locale| %> +

+ <%= f.label :locale, "#{t('admin.title')} #{I18nVariable.from_locale(locale)}" %> + <%= f.text_field locale %> +

+ <% end %> <% end %>

diff --git a/app/views/admin/pages/_form.html.erb b/app/views/admin/pages/_form.html.erb index 8223f0bb..ecabdd13 100644 --- a/app/views/admin/pages/_form.html.erb +++ b/app/views/admin/pages/_form.html.erb @@ -1,34 +1,27 @@ <%= f.error_messages %> -<%= f.hidden_field :parent_id %> +<%= f.hidden_field :parent, :value => @item.parent.id %>

<%= f.label :name, t('admin.name') %> <%= f.text_field :name, :class => 'text' %>

- -<% @site_valid_locales.each do |locale| %> -

- <%= label_tag "page[title]", "#{t('admin.title')} #{locale}" %> - <%= text_field_tag "page[i18n_variable][#{locale}]", (@i18n_variable[locale] if @i18n_variable), :class => 'text' %> -

+<%= f.fields_for :i18n_variable, (@item.new_record? ? @item.build_i18n_variable : @item.i18n_variable) do |f| %> + <% @site_valid_locales.each do |locale| %> +

+ <%= f.label :locale, "#{t('admin.title')} #{I18nVariable.from_locale(locale)}" %> + <%= f.text_field locale %> +

+ <% end %> <% end %>

<%= t('admin.design_name') %> - <% if @design %> - <%= f.collection_select :design_id, @designs, :id, :title, {}, :rel => admin_pages_path %> - <% else %> - <%= f.select :design_id, @designs.collect { |d| [d.title, d.id] }, {:selected => @default_design.id}, {:rel => admin_pages_path} %> - <% end %> + <%= f.collection_select :design, @designs, :id, :title, {:selected => @design.id}, {:rel => admin_pages_path} %>

<%= t('admin.theme') %> - <% if @design %> - <%= f.select :theme_id, @design.themes.collect { |t| [t.name.capitalize, t.id] }, :include_blank => true %> - <% else %> - <%= f.select :theme_id, @default_design.themes.collect { |t| [t.name.capitalize, t.id] }, :include_blank => true %> - <% end %> + <%= f.select :theme_id, @design.themes.collect { |t| [t.name.capitalize, t.id] }, :include_blank => true %>

<%= t('admin.module_app') %>