Support for subforums (#3831).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10142 e93f8b46-1217-0410-a6f0-8f06a7374b81

18 files changed, 210 insertions, 14 deletions

diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 6b154a81c..7ac7aa365 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 eb6257784..85d526c22 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 ? '&nbsp;' * 2 * level + '&#187; ' : '').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 0b7d36667..42b5499e1 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 736028bb1..daaecee53 100644
--- a/app/views/boards/_form.html.erb
+++ b/app/views/boards/_form.html.erb
@@ -3,4 +3,7 @@
 <div class="box tabular">
 <p><%= f.text_field :name, :required => true %></p>
 <p><%= f.text_field :description, :required => true, :size => 80 %></p>
+<% if @board.valid_parents.any? %>
+<p><%= f.select :parent_id, boards_options_for_select(@board.valid_parents), :include_blank => true, :label => :field_board_parent %></p>
+<% end %>
 </div>
diff --git a/app/views/boards/index.html.erb b/app/views/boards/index.html.erb
index c516b947f..3caa08d3c 100644
--- a/app/views/boards/index.html.erb
+++ b/app/views/boards/index.html.erb
@@ -8,9 +8,9 @@
     <th><%= l(:label_message_last) %></th>
   </tr></thead>
   <tbody>
-<% for board in @boards %>
+<% Board.board_tree(@boards) do |board, level| %>
   <tr class="<%= cycle 'odd', 'even' %>">
-    <td>
+    <td style="padding-left: <%= level * 18 %>px;">
       <%= link_to h(board.name), {:action => 'show', :id => board}, :class => "board"  %><br />
       <%=h board.description %>
     </td>
diff --git a/app/views/boards/show.html.erb b/app/views/boards/show.html.erb
index 1effcb3f9..784c58a52 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) %>
 
 <div class="contextual">
 <%= 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 f7cd8e367..d9f48af5c 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') %>
   <p><label><%= l(:label_board) %></label><br />
-  <%= f.select :board_id, @project.boards.collect {|b| [b.name, b.id]} %></p>
+  <%= f.select :board_id, boards_options_for_select(@message.project.boards) %></p>
 <% end %>
 
 <p>
diff --git a/app/views/messages/edit.html.erb b/app/views/messages/edit.html.erb
index fdea28e87..7ce0560bb 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) %>
 
 <h2><%= avatar(@topic.author, :size => "24") %><%=h @topic.subject %></h2>
 
diff --git a/app/views/messages/show.html.erb b/app/views/messages/show.html.erb
index 8e9ecceca..db9a2c8af 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) %>
 
 <div class="contextual">
     <%= watcher_tag(@topic, User.current) %>
diff --git a/app/views/projects/settings/_boards.html.erb b/app/views/projects/settings/_boards.html.erb
index 1d095c232..1050ef70c 100644
--- a/app/views/projects/settings/_boards.html.erb
+++ b/app/views/projects/settings/_boards.html.erb
@@ -7,10 +7,10 @@
     <th></th>
   </tr></thead>
   <tbody>
-<% @project.boards.each do |board|
+<% Board.board_tree(@project.boards) do |board, level|
   next if board.new_record? %>
   <tr class="<%= cycle 'odd', 'even' %>">
-    <td><%= link_to board.name, project_board_path(@project, board) %></td>
+    <td style="padding-left: <%= level * 18 %>px;"><%= link_to board.name, project_board_path(@project, board) %></td>
     <td><%=h board.description %></td>
     <td align="center">
     <% if authorize_for("boards", "edit") %>
diff --git a/config/initializers/10-patches.rb b/config/initializers/10-patches.rb
index dc847de50..672f1c0a0 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 a6bbfa1fd..f35ba5745 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 007aecc7c..9afb86ed3 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 000000000..c9ce47f78
--- /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 54b4373ef..79d164485 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 1c4ac67d4..8afe4f748 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 a2d7c958f..85c6d139c 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 86a8e6468..0326cbfb5 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