diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 6b154a8..7ac7aa3 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -22,6 +22,7 @@ class MessagesController < ApplicationController before_filter :find_message, :except => [:new, :preview] before_filter :authorize, :except => [:preview, :edit, :destroy] + helper :boards helper :watchers helper :attachments include AttachmentsHelper diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index eb62577..85d526c 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -18,4 +18,24 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. module BoardsHelper + def board_breadcrumb(item) + board = item.is_a?(Message) ? item.board : item + links = [link_to(l(:label_board_plural), project_boards_path(item.project))] + boards = board.ancestors.reverse + if item.is_a?(Message) + boards << board + end + links += boards.map {|ancestor| link_to(h(ancestor.name), project_board_path(ancestor.project, ancestor))} + breadcrumb links + end + + def boards_options_for_select(boards) + options = [] + Board.board_tree(boards) do |board, level| + label = (level > 0 ? ' ' * 2 * level + '» ' : '').html_safe + label << board.name + options << [label, board.id] + end + options + end end diff --git a/app/models/board.rb b/app/models/board.rb index 0b7d366..42b5499 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -21,26 +21,37 @@ class Board < ActiveRecord::Base has_many :topics, :class_name => 'Message', :conditions => "#{Message.table_name}.parent_id IS NULL", :order => "#{Message.table_name}.created_on DESC" has_many :messages, :dependent => :destroy, :order => "#{Message.table_name}.created_on DESC" belongs_to :last_message, :class_name => 'Message', :foreign_key => :last_message_id - acts_as_list :scope => :project_id + acts_as_tree :dependent => :nullify + acts_as_list :scope => '(project_id = #{project_id} AND parent_id #{parent_id ? "= #{parent_id}" : "IS NULL"})' acts_as_watchable validates_presence_of :name, :description validates_length_of :name, :maximum => 30 validates_length_of :description, :maximum => 255 + validate :validate_board scope :visible, lambda {|*args| { :include => :project, :conditions => Project.allowed_to_condition(args.shift || User.current, :view_messages, *args) } } - safe_attributes 'name', 'description', 'move_to' + safe_attributes 'name', 'description', 'parent_id', 'move_to' def visible?(user=User.current) !user.nil? && user.allowed_to?(:view_messages, project) end + def reload(*args) + @valid_parents = nil + super + end + def to_s name end + def valid_parents + @valid_parents ||= project.boards - self_and_descendants + end + def reset_counters! self.class.reset_counters!(id) end @@ -53,4 +64,26 @@ class Board < ActiveRecord::Base " last_message_id = (SELECT MAX(id) FROM #{Message.table_name} WHERE board_id=#{board_id})", ["id = ?", board_id]) end + + def self.board_tree(boards, parent_id=nil, level=0) + tree = [] + boards.select {|board| board.parent_id == parent_id}.sort_by(&:position).each do |board| + tree << [board, level] + tree += board_tree(boards, board.id, level+1) + end + if block_given? + tree.each do |board, level| + yield board, level + end + end + tree + end + + protected + + def validate_board + if parent_id && parent_id_changed? + errors.add(:parent_id, :invalid) unless valid_parents.include?(parent) + end + end end diff --git a/app/views/boards/_form.html.erb b/app/views/boards/_form.html.erb index 736028b..daaecee 100644 --- a/app/views/boards/_form.html.erb +++ b/app/views/boards/_form.html.erb @@ -3,4 +3,7 @@

<%= f.text_field :name, :required => true %>

<%= f.text_field :description, :required => true, :size => 80 %>

+<% if @board.valid_parents.any? %> +

<%= f.select :parent_id, boards_options_for_select(@board.valid_parents), :include_blank => true, :label => :field_board_parent %>

+<% end %>
diff --git a/app/views/boards/index.html.erb b/app/views/boards/index.html.erb index c516b94..3caa08d 100644 --- a/app/views/boards/index.html.erb +++ b/app/views/boards/index.html.erb @@ -8,9 +8,9 @@ <%= l(:label_message_last) %> -<% for board in @boards %> +<% Board.board_tree(@boards) do |board, level| %> - + <%= link_to h(board.name), {:action => 'show', :id => board}, :class => "board" %>
<%=h board.description %> diff --git a/app/views/boards/show.html.erb b/app/views/boards/show.html.erb index 1effcb3..784c58a 100644 --- a/app/views/boards/show.html.erb +++ b/app/views/boards/show.html.erb @@ -1,4 +1,4 @@ -<%= breadcrumb link_to(l(:label_board_plural), project_boards_path(@project)) %> +<%= board_breadcrumb(@board) %>
<%= link_to_if_authorized l(:label_message_new), diff --git a/app/views/messages/_form.html.erb b/app/views/messages/_form.html.erb index f7cd8e3..d9f48af 100644 --- a/app/views/messages/_form.html.erb +++ b/app/views/messages/_form.html.erb @@ -18,7 +18,7 @@ <% if !replying && !@message.new_record? && @message.safe_attribute?('board_id') %>


- <%= f.select :board_id, @project.boards.collect {|b| [b.name, b.id]} %>

+ <%= f.select :board_id, boards_options_for_select(@message.project.boards) %>

<% end %>

diff --git a/app/views/messages/edit.html.erb b/app/views/messages/edit.html.erb index fdea28e..7ce0560 100644 --- a/app/views/messages/edit.html.erb +++ b/app/views/messages/edit.html.erb @@ -1,5 +1,4 @@ -<%= breadcrumb link_to(l(:label_board_plural), project_boards_path(@project)), - link_to(h(@board.name), project_board_path(@project, @board)) %> +<%= board_breadcrumb(@message) %>

<%= avatar(@topic.author, :size => "24") %><%=h @topic.subject %>

diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb index 8e9ecce..db9a2c8 100644 --- a/app/views/messages/show.html.erb +++ b/app/views/messages/show.html.erb @@ -1,5 +1,4 @@ -<%= breadcrumb link_to(l(:label_board_plural), project_boards_path(@project)), - link_to(h(@board.name), project_board_path(@project, @board)) %> +<%= board_breadcrumb(@message) %>
<%= watcher_tag(@topic, User.current) %> diff --git a/app/views/projects/settings/_boards.html.erb b/app/views/projects/settings/_boards.html.erb index 1d095c2..1050ef7 100644 --- a/app/views/projects/settings/_boards.html.erb +++ b/app/views/projects/settings/_boards.html.erb @@ -7,10 +7,10 @@ -<% @project.boards.each do |board| +<% Board.board_tree(@project.boards) do |board, level| next if board.new_record? %> - <%= link_to board.name, project_board_path(@project, board) %> + <%= link_to board.name, project_board_path(@project, board) %> <%=h board.description %> <% if authorize_for("boards", "edit") %> diff --git a/config/initializers/10-patches.rb b/config/initializers/10-patches.rb index dc847de..672f1c0 100644 --- a/config/initializers/10-patches.rb +++ b/config/initializers/10-patches.rb @@ -5,7 +5,9 @@ module ActiveRecord include Redmine::I18n # Translate attribute names for validation errors display def self.human_attribute_name(attr, *args) - l("field_#{attr.to_s.gsub(/_id$/, '')}", :default => attr) + attr = attr.to_s.sub(/_id$/, '') + + l("field_#{name.underscore.gsub('/', '_')}_#{attr}", :default => ["field_#{attr}".to_sym, attr]) end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index a6bbfa1..f35ba57 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -330,6 +330,7 @@ en: field_ldap_filter: LDAP filter field_core_fields: Standard fields field_timeout: "Timeout (in seconds)" + field_board_parent: Parent forum setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 007aecc..9afb86e 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -329,6 +329,7 @@ fr: field_ldap_filter: Filtre LDAP field_core_fields: Champs standards field_timeout: "Timeout (en secondes)" + field_board_parent: Forum parent setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application diff --git a/db/migrate/20120731164049_add_boards_parent_id.rb b/db/migrate/20120731164049_add_boards_parent_id.rb new file mode 100644 index 0000000..c9ce47f --- /dev/null +++ b/db/migrate/20120731164049_add_boards_parent_id.rb @@ -0,0 +1,9 @@ +class AddBoardsParentId < ActiveRecord::Migration + def up + add_column :boards, :parent_id, :integer + end + + def down + remove_column :boards, :parent_id + end +end diff --git a/lib/plugins/acts_as_tree/lib/active_record/acts/tree.rb b/lib/plugins/acts_as_tree/lib/active_record/acts/tree.rb index 54b4373..79d1644 100644 --- a/lib/plugins/acts_as_tree/lib/active_record/acts/tree.rb +++ b/lib/plugins/acts_as_tree/lib/active_record/acts/tree.rb @@ -74,7 +74,7 @@ module ActiveRecord # # root.descendants # => [child1, subchild1, subchild2] def descendants - children + children.collect(&:children).flatten + children + children.collect(&:descendants).flatten end # Returns list of descendants and a reference to the current node. diff --git a/test/functional/boards_controller_test.rb b/test/functional/boards_controller_test.rb index 1c4ac67..8afe4f7 100644 --- a/test/functional/boards_controller_test.rb +++ b/test/functional/boards_controller_test.rb @@ -98,6 +98,23 @@ class BoardsControllerTest < ActionController::TestCase get :new, :project_id => 1 assert_response :success assert_template 'new' + + assert_select 'select[name=?]', 'board[parent_id]' do + assert_select 'option', (Project.find(1).boards.size + 1) + assert_select 'option[value=]', :text => '' + assert_select 'option[value=1]', :text => 'Help' + end + end + + def test_new_without_project_boards + Project.find(1).boards.delete_all + @request.session[:user_id] = 2 + + get :new, :project_id => 1 + assert_response :success + assert_template 'new' + + assert_select 'select[name=?]', 'board[parent_id]', 0 end def test_create @@ -111,6 +128,16 @@ class BoardsControllerTest < ActionController::TestCase assert_equal 'Testing board creation', board.description end + def test_create_with_parent + @request.session[:user_id] = 2 + assert_difference 'Board.count' do + post :create, :project_id => 1, :board => { :name => 'Testing', :description => 'Testing', :parent_id => 2} + end + assert_redirected_to '/projects/ecookbook/settings/boards' + board = Board.first(:order => 'id DESC') + assert_equal Board.find(2), board.parent + end + def test_create_with_failure @request.session[:user_id] = 2 assert_no_difference 'Board.count' do @@ -127,6 +154,18 @@ class BoardsControllerTest < ActionController::TestCase assert_template 'edit' end + def test_edit_with_parent + board = Board.generate!(:project_id => 1, :parent_id => 2) + @request.session[:user_id] = 2 + get :edit, :project_id => 1, :id => board.id + assert_response :success + assert_template 'edit' + + assert_select 'select[name=?]', 'board[parent_id]' do + assert_select 'option[value=2][selected=selected]' + end + end + def test_update @request.session[:user_id] = 2 assert_no_difference 'Board.count' do diff --git a/test/object_helpers.rb b/test/object_helpers.rb index a2d7c95..85c6d13 100644 --- a/test/object_helpers.rb +++ b/test/object_helpers.rb @@ -99,4 +99,15 @@ module ObjectHelpers source.save! source end + + def Board.generate!(attributes={}) + @generated_board_name ||= 'Forum 0' + @generated_board_name.succ! + board = Board.new(attributes) + board.name = @generated_board_name if board.name.blank? + board.description = @generated_board_name if board.description.blank? + yield board if block_given? + board.save! + board + end end diff --git a/test/unit/board_test.rb b/test/unit/board_test.rb index 86a8e64..0326cbf 100644 --- a/test/unit/board_test.rb +++ b/test/unit/board_test.rb @@ -1,3 +1,22 @@ +# encoding: utf-8 +# +# Redmine - project management software +# Copyright (C) 2006-2012 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + require File.expand_path('../../test_helper', __FILE__) class BoardTest < ActiveSupport::TestCase @@ -21,6 +40,54 @@ class BoardTest < ActiveSupport::TestCase assert_equal @project.boards.size, board.position end + def test_parent_should_be_in_same_project + board = Board.new(:project_id => 3, :name => 'Test', :description => 'Test', :parent_id => 1) + assert !board.save + assert_include "Parent forum is invalid", board.errors.full_messages + end + + def test_valid_parents_should_not_include_self_nor_a_descendant + board1 = Board.generate!(:project_id => 3) + board2 = Board.generate!(:project_id => 3, :parent => board1) + board3 = Board.generate!(:project_id => 3, :parent => board2) + board4 = Board.generate!(:project_id => 3) + + assert_equal [board4], board1.reload.valid_parents.sort_by(&:id) + assert_equal [board1, board4], board2.reload.valid_parents.sort_by(&:id) + assert_equal [board1, board2, board4], board3.reload.valid_parents.sort_by(&:id) + assert_equal [board1, board2, board3], board4.reload.valid_parents.sort_by(&:id) + end + + def test_position_should_be_assigned_with_parent_scope + parent1 = Board.generate!(:project_id => 3) + parent2 = Board.generate!(:project_id => 3) + child1 = Board.generate!(:project_id => 3, :parent => parent1) + child2 = Board.generate!(:project_id => 3, :parent => parent1) + + assert_equal 1, parent1.reload.position + assert_equal 1, child1.reload.position + assert_equal 2, child2.reload.position + assert_equal 2, parent2.reload.position + end + + def test_board_tree_should_yield_boards_with_level + parent1 = Board.generate!(:project_id => 3) + parent2 = Board.generate!(:project_id => 3) + child1 = Board.generate!(:project_id => 3, :parent => parent1) + child2 = Board.generate!(:project_id => 3, :parent => parent1) + child3 = Board.generate!(:project_id => 3, :parent => child1) + + tree = Board.board_tree(Project.find(3).boards) + + assert_equal [ + [parent1, 0], + [child1, 1], + [child3, 2], + [child2, 1], + [parent2, 0] + ], tree + end + def test_destroy board = Board.find(1) assert_difference 'Message.count', -6 do @@ -32,4 +99,15 @@ class BoardTest < ActiveSupport::TestCase end assert_equal 0, Message.count(:conditions => {:board_id => 1}) end + + def test_destroy_should_nullify_children + parent = Board.generate!(:project => @project) + child = Board.generate!(:project => @project, :parent => parent) + assert_equal parent, child.parent + + assert parent.destroy + child.reload + assert_nil child.parent + assert_nil child.parent_id + end end