Forums enhancements:

* messages can now be edited/deleted (explicit permissions need to be given)
* topics can be locked so that no reply can be added (only by users allowed to edit messages)
* topics can be marked as sticky so that they always appear at the top of the list (only by users allowed to edit messages)

git-svn-id: http://redmine.rubyforge.org/svn/trunk@926 e93f8b46-1217-0410-a6f0-8f06a7374b81

19 files changed, 294 insertions, 36 deletions

diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb
index 3a8b021a3..200792370 100644
--- a/app/controllers/boards_controller.rb
+++ b/app/controllers/boards_controller.rb
@@ -41,7 +41,7 @@ class BoardsController < ApplicationController
       
     @topic_count = @board.topics.count
     @topic_pages = Paginator.new self, @topic_count, 25, params['page']
-    @topics =  @board.topics.find :all, :order => sort_clause,
+    @topics =  @board.topics.find :all, :order => "#{Message.table_name}.sticky DESC, #{sort_clause}",
                                   :include => [:author, {:last_reply => :author}],
                                   :limit  =>  @topic_pages.items_per_page,
                                   :offset =>  @topic_pages.current.offset
diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 9352c4af4..46c9adadd 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -17,22 +17,30 @@
 
 class MessagesController < ApplicationController
   layout 'base'
-  before_filter :find_project, :authorize
+  before_filter :find_board, :only => :new
+  before_filter :find_message, :except => :new
+  before_filter :authorize
 
   verify :method => :post, :only => [ :reply, :destroy ], :redirect_to => { :action => :show }
 
   helper :attachments
   include AttachmentsHelper   
 
+  # Show a topic and its replies
   def show
     @reply = Message.new(:subject => "RE: #{@message.subject}")
     render :action => "show", :layout => false if request.xhr?
   end
   
+  # Create a new topic
   def new
     @message = Message.new(params[:message])
     @message.author = User.current
-    @message.board = @board 
+    @message.board = @board
+    if params[:message] && User.current.allowed_to?(:edit_messages, @project)
+      @message.locked = params[:message]['locked']
+      @message.sticky = params[:message]['sticky']
+    end
     if request.post? && @message.save
       params[:attachments].each { |file|
         Attachment.create(:container => @message, :file => file, :author => User.current) if file.size > 0
@@ -41,24 +49,55 @@ class MessagesController < ApplicationController
     end
   end
 
+  # Reply to a topic
   def reply
     @reply = Message.new(params[:reply])
     @reply.author = User.current
     @reply.board = @board
-    @message.children << @reply
+    @topic.children << @reply
     if !@reply.new_record?
       params[:attachments].each { |file|
         Attachment.create(:container => @reply, :file => file, :author => User.current) if file.size > 0
       } if params[:attachments] and params[:attachments].is_a? Array
     end
-    redirect_to :action => 'show', :id => @message
+    redirect_to :action => 'show', :id => @topic
+  end
+
+  # Edit a message
+  def edit
+    if params[:message] && User.current.allowed_to?(:edit_messages, @project)
+      @message.locked = params[:message]['locked']
+      @message.sticky = params[:message]['sticky']
+    end
+    if request.post? && @message.update_attributes(params[:message])
+      params[:attachments].each { |file|
+        Attachment.create(:container => @message, :file => file, :author => User.current) if file.size > 0
+      } if params[:attachments] and params[:attachments].is_a? Array
+      flash[:notice] = l(:notice_successful_update)
+      redirect_to :action => 'show', :id => @topic
+    end
+  end
+  
+  # Delete a messages
+  def destroy
+    @message.destroy
+    redirect_to @message.parent.nil? ?
+      { :controller => 'boards', :action => 'show', :project_id => @project, :id => @board } :
+      { :action => 'show', :id => @message.parent }
   end
   
 private
-  def find_project
+  def find_message
+    find_board
+    @message = @board.messages.find(params[:id], :include => :parent)
+    @topic = @message.root
+  rescue ActiveRecord::RecordNotFound
+    render_404
+  end
+  
+  def find_board
     @board = Board.find(params[:board_id], :include => :project)
     @project = @board.project
-    @message = @board.topics.find(params[:id]) if params[:id]
   rescue ActiveRecord::RecordNotFound
     render_404
   end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 9c8e9c67d..f4746c627 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -34,7 +34,7 @@ module ApplicationHelper
 
   # Display a link to user's account page
   def link_to_user(user)
-    link_to user.name, :controller => 'account', :action => 'show', :id => user
+    user ? link_to(user, :controller => 'account', :action => 'show', :id => user) : 'Anonymous'
   end
   
   def link_to_issue(issue)
@@ -92,7 +92,7 @@ module ApplicationHelper
   
   def authoring(created, author)
     time_tag = content_tag('acronym', distance_of_time_in_words(Time.now, created), :title => format_time(created))
-    l(:label_added_time_by, author.name, time_tag)
+    l(:label_added_time_by, author || 'Anonymous', time_tag)
   end
   
   def day_name(day)
diff --git a/app/models/message.rb b/app/models/message.rb
index 909c06a9e..038665cce 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -30,9 +30,15 @@ class Message < ActiveRecord::Base
                 :description => :content,
                 :url => Proc.new {|o| {:controller => 'messages', :action => 'show', :board_id => o.board_id, :id => o.id}}
   
+  attr_protected :locked, :sticky
   validates_presence_of :subject, :content
   validates_length_of :subject, :maximum => 255
   
+  def validate_on_create
+    # Can not reply to a locked topic
+    errors.add_to_base 'Topic is locked' if root.locked?
+  end
+  
   def after_create
     board.update_attribute(:last_message_id, self.id)
     board.increment! :messages_count
@@ -43,6 +49,18 @@ class Message < ActiveRecord::Base
     end
   end
   
+  def after_destroy
+    # The following line is required so that the previous counter
+    # updates (due to children removal) are not overwritten
+    board.reload
+    board.decrement! :messages_count
+    board.decrement! :topics_count unless parent
+  end
+  
+  def sticky?
+    sticky == 1
+  end
+  
   def project
     board.project
   end
diff --git a/app/views/boards/index.rhtml b/app/views/boards/index.rhtml
index 3291d0194..cd4e85e9a 100644
--- a/app/views/boards/index.rhtml
+++ b/app/views/boards/index.rhtml
@@ -19,7 +19,7 @@
     <td>
     <small>
       <% if board.last_message %>
-      <%= board.last_message.author.name %>, <%= format_time(board.last_message.created_on) %><br />
+      <%= authoring board.last_message.created_on, board.last_message.author %><br />
       <%= link_to_message board.last_message %>  
       <% end %>
     </small>
diff --git a/app/views/boards/show.rhtml b/app/views/boards/show.rhtml
index 0af89fdb7..8bcf960b2 100644
--- a/app/views/boards/show.rhtml
+++ b/app/views/boards/show.rhtml
@@ -18,7 +18,7 @@
 <h2><%=h @board.name %></h2>
 
 <% if @topics.any? %>
-<table class="list">
+<table class="list messages">
   <thead><tr>
     <th><%= l(:field_subject) %></th>
     <th><%= l(:field_author) %></th>
@@ -28,18 +28,16 @@
   </tr></thead>  
   <tbody>
   <% @topics.each do |topic| %>
-    <tr class="<%= cycle 'odd', 'even' %>">
-      <td><%= link_to h(topic.subject), :controller => 'messages', :action => 'show', :board_id => @board, :id => topic %></td>
-      <td align="center"><%= link_to_user topic.author %></td>
-      <td align="center"><%= format_time(topic.created_on) %></td>
-      <td align="center"><%= topic.replies_count %></td>
-      <td>
-      <small>
+    <tr class="message <%= cycle 'odd', 'even' %> <%= topic.sticky? ? 'sticky' : '' %> <%= topic.locked? ? 'locked' : '' %>">
+      <td class="subject"><%= link_to h(topic.subject), { :controller => 'messages', :action => 'show', :board_id => @board, :id => topic }, :class => 'icon' %></td>
+      <td class="author" align="center"><%= topic.author %></td>
+      <td class="created_on" align="center"><%= format_time(topic.created_on) %></td>
+      <td class="replies" align="center"><%= topic.replies_count %></td>
+      <td class="last_message">
         <% if topic.last_reply %>
-        <%= topic.last_reply.author.name %>, <%= format_time(topic.last_reply.created_on) %><br />
+        <%= authoring topic.last_reply.created_on, topic.last_reply.author %><br />
         <%= link_to_message topic.last_reply %>
         <% end %>
-      </small>
       </td>
     </tr>
   <% end %>
diff --git a/app/views/messages/_form.rhtml b/app/views/messages/_form.rhtml
index 25d88cd44..c2f7fb569 100644
--- a/app/views/messages/_form.rhtml
+++ b/app/views/messages/_form.rhtml
@@ -3,7 +3,13 @@
 <div class="box">
 <!--[form:message]-->
 <p><label><%= l(:field_subject) %></label><br />
-<%= f.text_field :subject, :required => true, :size => 120 %></p>
+<%= f.text_field :subject, :required => true, :size => 120 %>
+
+<% if User.current.allowed_to?(:edit_messages, @project) %>
+    <label><%= f.check_box :sticky %> Sticky</label>
+    <label><%= f.check_box :locked %> Locked</label>
+<% end %>
+</p>
 
 <p><%= f.text_area :content, :required => true, :cols => 80, :rows => 15, :class => 'wiki-edit', :id => 'message_content' %></p>
 <%= wikitoolbar_for 'message_content' %>
diff --git a/app/views/messages/edit.rhtml b/app/views/messages/edit.rhtml
new file mode 100644
index 000000000..808b6ea27
--- /dev/null
+++ b/app/views/messages/edit.rhtml
@@ -0,0 +1,6 @@
+<h2><%= link_to h(@board.name), :controller => 'boards', :action => 'show', :project_id => @project, :id => @board %> &#187; <%=h @message.subject %></h2>
+
+<% form_for :message, @message, :url => {:action => 'edit'}, :html => {:multipart => true} do |f| %>
+  <%= render :partial => 'form', :locals => {:f => f} %>
+  <%= submit_tag l(:button_save) %>
+<% end %>
diff --git a/app/views/messages/show.rhtml b/app/views/messages/show.rhtml
index e39c09d50..bb7e2b7f3 100644
--- a/app/views/messages/show.rhtml
+++ b/app/views/messages/show.rhtml
@@ -1,28 +1,37 @@
-<h2><%= link_to h(@board.name), :controller => 'boards', :action => 'show', :project_id => @project, :id => @board %> &#187; <%=h @message.subject %></h2>
+<div class="contextual">
+    <%= link_to_if_authorized l(:button_edit), {:action => 'edit', :id => @topic}, :class => 'icon icon-edit' %>
+    <%= link_to_if_authorized l(:button_delete), {:action => 'destroy', :id => @topic}, :method => :post, :confirm => l(:text_are_you_sure), :class => 'icon icon-del' %>
+</div>
+
+<h2><%= link_to h(@board.name), :controller => 'boards', :action => 'show', :project_id => @project, :id => @board %> &#187; <%=h @topic.subject %></h2>
 
 <div class="message">
-<p><span class="author"><%= authoring @message.created_on, @message.author %></span></p>
+<p><span class="author"><%= authoring @topic.created_on, @topic.author %></span></p>
 <div class="wiki">
-<%= textilizable(@message.content, :attachments => @message.attachments) %>
+<%= textilizable(@topic.content, :attachments => @topic.attachments) %>
 </div>
-<%= link_to_attachments @message.attachments, :no_author => true %>
+<%= link_to_attachments @topic.attachments, :no_author => true %>
 </div>
 <br />
 
-<div class="message reply">
 <h3 class="icon22 icon22-comment"><%= l(:label_reply_plural) %></h3>
-<% @message.children.each do |message| %>
+<% @topic.children.each do |message| %>
   <a name="<%= "message-#{message.id}" %>"></a>
+  <div class="contextual">
+    <%= link_to_if_authorized l(:button_edit), {:action => 'edit', :id => message}, :class => 'icon icon-edit' %>
+    <%= link_to_if_authorized l(:button_delete), {:action => 'destroy', :id => message}, :method => :post, :confirm => l(:text_are_you_sure), :class => 'icon icon-del' %>
+  </div>
+  <div class="message reply">
   <h4><%=h message.subject %> - <%= authoring message.created_on, message.author %></h4>
   <div class="wiki"><%= textilizable message.content %></div>
   <%= link_to_attachments message.attachments, :no_author => true %>
+  </div>
 <% end %>
-</div>
 
-<% if authorize_for('messages', 'reply') %>
+<% if !@topic.locked? && authorize_for('messages', 'reply') %>
 <p><%= toggle_link l(:button_reply), "reply", :focus => 'message_content' %></p>
 <div id="reply" style="display:none;">
-<% form_for :reply, @reply, :url => {:action => 'reply', :id => @message}, :html => {:multipart => true} do |f| %>
+<% form_for :reply, @reply, :url => {:action => 'reply', :id => @topic}, :html => {:multipart => true} do |f| %>
     <%= render :partial => 'form', :locals => {:f => f} %>
     <%= submit_tag l(:button_submit) %>
 <% end %>
diff --git a/db/migrate/082_add_messages_locked.rb b/db/migrate/082_add_messages_locked.rb
new file mode 100644
index 000000000..20a172565
--- /dev/null
+++ b/db/migrate/082_add_messages_locked.rb
@@ -0,0 +1,9 @@
+class AddMessagesLocked < ActiveRecord::Migration
+  def self.up
+    add_column :messages, :locked, :boolean, :default => false
+  end
+
+  def self.down
+    remove_column :messages, :locked
+  end
+end
diff --git a/db/migrate/083_add_messages_sticky.rb b/db/migrate/083_add_messages_sticky.rb
new file mode 100644
index 000000000..8fd5d2ce3
--- /dev/null
+++ b/db/migrate/083_add_messages_sticky.rb
@@ -0,0 +1,9 @@
+class AddMessagesSticky < ActiveRecord::Migration
+  def self.up
+    add_column :messages, :sticky, :integer, :default => 0
+  end
+
+  def self.down
+    remove_column :messages, :sticky
+  end
+end
diff --git a/lib/redmine.rb b/lib/redmine.rb
index 8a79b265a..ffc1cc27e 100644
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -84,6 +84,8 @@ Redmine::AccessControl.map do |map|
     map.permission :manage_boards, {:boards => [:new, :edit, :destroy]}, :require => :member
     map.permission :view_messages, {:boards => [:index, :show], :messages => [:show]}, :public => true
     map.permission :add_messages, {:messages => [:new, :reply]}
+    map.permission :edit_messages, {:messages => :edit}, :require => :member
+    map.permission :delete_messages, {:messages => :destroy}, :require => :member
   end
 end
 
diff --git a/public/images/sticky.png b/public/images/sticky.png
new file mode 100644
index 000000000..d32ee63a4
--- /dev/null
+++ b/public/images/sticky.png
diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css
index f398895e8..1e3200135 100644
--- a/public/stylesheets/application.css
+++ b/public/stylesheets/application.css
@@ -77,6 +77,11 @@ tr.issue td.subject, tr.issue td.category { white-space: normal; }
 tr.issue td.subject { text-align: left; }
 tr.issue td.done_ratio table.progress { margin-left:auto; margin-right: auto;}
 
+tr.message { height: 2.6em; }
+tr.message td.last_message { font-size: 80%; }
+tr.message.locked td.subject a { background-image: url(../images/locked.png); }
+tr.message.sticky td.subject a { background-image: url(../images/sticky.png); font-weight: bold; }
+
 table.list tbody tr:hover { background-color:#ffffdd; }
 table td {padding:2px;}
 table p {margin:0;}
diff --git a/test/fixtures/boards.yml b/test/fixtures/boards.yml
index 7f2944fac..b6b42aaa3 100644
--- a/test/fixtures/boards.yml
+++ b/test/fixtures/boards.yml
@@ -2,12 +2,12 @@
 boards_001: 

   name: Help

   project_id: 1

-  topics_count: 1

+  topics_count: 2

   id: 1

   description: Help board

   position: 1

-  last_message_id: 2

-  messages_count: 2

+  last_message_id: 5

+  messages_count: 5

 boards_002: 

   name: Discussion

   project_id: 1

diff --git a/test/fixtures/messages.yml b/test/fixtures/messages.yml
index 88f54dbc2..5bb2438dd 100644
--- a/test/fixtures/messages.yml
+++ b/test/fixtures/messages.yml
@@ -4,8 +4,8 @@ messages_001:
   updated_on: 2007-05-12 17:15:32 +02:00

   subject: First post

   id: 1

-  replies_count: 1

-  last_reply_id: 2

+  replies_count: 2

+  last_reply_id: 3

   content: "This is the very first post\n\

     in the forum"

   author_id: 1

@@ -22,4 +22,36 @@ messages_002:
   author_id: 1

   parent_id: 1

   board_id: 1

-  
\ No newline at end of file
+messages_003: 

+  created_on: 2007-05-12 17:18:02 +02:00

+  updated_on: 2007-05-12 17:18:02 +02:00

+  subject: "RE: First post"

+  id: 3

+  replies_count: 0

+  last_reply_id: 

+  content: "An other reply"

+  author_id: 

+  parent_id: 1

+  board_id: 1

+messages_004: 

+  created_on: 2007-08-12 17:15:32 +02:00

+  updated_on: 2007-08-12 17:15:32 +02:00

+  subject: Post 2

+  id: 4

+  replies_count: 1

+  last_reply_id: 5

+  content: "This is an other post"

+  author_id: 

+  parent_id: 

+  board_id: 1

+messages_005: 

+  created_on: 2007-09-12 17:18:00 +02:00

+  updated_on: 2007-09-12 17:18:00 +02:00

+  subject: 'RE: post 2'

+  id: 5

+  replies_count: 0

+  last_reply_id: 

+  content: "Reply to the second post"

+  author_id: 1

+  parent_id: 4

+  board_id: 1

diff --git a/test/functional/boards_controller_test.rb b/test/functional/boards_controller_test.rb
new file mode 100644
index 000000000..3ff71bc4e
--- /dev/null
+++ b/test/functional/boards_controller_test.rb
@@ -0,0 +1,50 @@
+# redMine - project management software
+# Copyright (C) 2006-2007  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.dirname(__FILE__) + '/../test_helper'
+require 'boards_controller'
+
+# Re-raise errors caught by the controller.
+class BoardsController; def rescue_action(e) raise e end; end
+
+class BoardsControllerTest < Test::Unit::TestCase
+  fixtures :projects, :users, :members, :roles, :boards, :messages, :enabled_modules
+  
+  def setup
+    @controller = BoardsController.new
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    User.current = nil
+  end
+  
+  def test_index
+    get :index, :project_id => 1
+    assert_response :success
+    assert_template 'index'
+    assert_not_nil assigns(:boards)
+    assert_not_nil assigns(:project)
+  end
+  
+  def test_show
+    get :show, :project_id => 1, :id => 1
+    assert_response :success
+    assert_template 'show'
+    assert_not_nil assigns(:board)
+    assert_not_nil assigns(:project)
+    assert_not_nil assigns(:topics)
+  end
+end
diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb
new file mode 100644
index 000000000..25fc1363e
--- /dev/null
+++ b/test/functional/messages_controller_test.rb
@@ -0,0 +1,49 @@
+# redMine - project management software
+# Copyright (C) 2006-2007  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.dirname(__FILE__) + '/../test_helper'
+require 'messages_controller'
+
+# Re-raise errors caught by the controller.
+class MessagesController; def rescue_action(e) raise e end; end
+
+class MessagesControllerTest < Test::Unit::TestCase
+  fixtures :projects, :users, :members, :roles, :boards, :messages, :enabled_modules
+  
+  def setup
+    @controller = MessagesController.new
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    User.current = nil
+  end
+  
+  def test_show
+    get :show, :board_id => 1, :id => 1
+    assert_response :success
+    assert_template 'show'
+    assert_not_nil assigns(:board)
+    assert_not_nil assigns(:project)
+    assert_not_nil assigns(:topic)
+  end
+  
+  def test_reply
+    @request.session[:user_id] = 2
+    post :reply, :board_id => 1, :id => 1, :reply => { :content => 'This is a test reply', :subject => 'Test reply' }
+    assert_redirected_to 'messages/show'
+    assert Message.find_by_subject('Test reply')
+  end
+end
diff --git a/test/unit/message_test.rb b/test/unit/message_test.rb
index 6d8458bfc..82ed3fe13 100644
--- a/test/unit/message_test.rb
+++ b/test/unit/message_test.rb
@@ -41,4 +41,30 @@ class MessageTest < Test::Unit::TestCase
     assert_equal replies_count+1, @message[:replies_count]
     assert_equal reply, @message.last_reply
   end
+  
+  def test_destroy_topic
+    message = Message.find(1)
+    board = message.board
+    topics_count, messages_count = board.topics_count, board.messages_count    
+    assert message.destroy
+    board.reload
+    
+    # Replies deleted
+    assert Message.find_all_by_parent_id(1).empty?
+    # Checks counters
+    assert_equal topics_count - 1, board.topics_count
+    assert_equal messages_count - 3, board.messages_count
+  end
+  
+  def test_destroy_reply
+    message = Message.find(5)
+    board = message.board
+    topics_count, messages_count = board.topics_count, board.messages_count    
+    assert message.destroy
+    board.reload
+
+    # Checks counters
+    assert_equal topics_count, board.topics_count
+    assert_equal messages_count - 1, board.messages_count
+  end
 end