Adding an archived state to messages:

- New state&migration
- Additions in User and Message
- Messages#index now uses inbox
- Another action for message archive
- Selected messages can now be read or archived
- Styling system generated messages with its own 
icon

14 files changed, 199 insertions, 17 deletions

diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index 675507d..69b380c 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -20,7 +20,7 @@ class MessagesController < ApplicationController
   renders_in_global_context
   
   def index
-    @messages = current_user.top_level_messages.paginate(:page => params[:page])
+    @messages = current_user.messages_in_inbox.paginate(:page => params[:page])
     @root = Breadcrumb::ReceivedMessages.new(current_user)
     respond_to do |wants|
       wants.html
@@ -28,6 +28,11 @@ class MessagesController < ApplicationController
     end
   end
   
+  def all
+    @messages = current_user.top_level_messages.paginate(:page => params[:page])
+    @root = Breadcrumb::ReceivedMessages.new(current_user)
+  end
+  
   def sent
     @messages = current_user.sent_messages.paginate(:all,
       :page => params[:page])
@@ -45,8 +50,15 @@ class MessagesController < ApplicationController
   def bulk_update
     message_ids = params[:message_ids].to_a
     message_ids.each do |message_id|
-      if message = current_user.received_messages.find(message_id)
-        message.read
+      # if message = current_user.all_messages.find(message_id)
+      if message = Message.find(:first, :conditions => ['(recipient_id=? OR sender_id=?) AND id=?', current_user, current_user, message_id])
+        if params[:requested_action] == 'archive'
+          message.archived_by(current_user)
+          message.save!
+        else
+          logger.info("Marking message #{message_id} as read")
+          message.read
+        end
       end
     end
     redirect_to :action => :index
diff --git a/app/models/message.rb b/app/models/message.rb
index 4194588..edd65a1 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -24,7 +24,7 @@ class Message < ActiveRecord::Base
   belongs_to :in_reply_to, :class_name => 'Message', :foreign_key => :in_reply_to_id
   belongs_to :root_message, :class_name => 'Message', :foreign_key => :root_message_id
   after_create :send_email_notification_if_required
-  after_create :flag_root_message_if_required
+  before_create :flag_root_message_if_required
   
   has_many :replies, :class_name => 'Message', :foreign_key => :in_reply_to_id
   
@@ -44,7 +44,7 @@ class Message < ActiveRecord::Base
       transition :unread => :read
     end
   end
-
+  
   include ActiveMessaging::MessageSender
   
   def build_reply(options={})
@@ -145,6 +145,14 @@ class Message < ActiveRecord::Base
     end
   end
   
+  def archived_by(a_user)
+    if a_user == sender
+      self.archived_by_sender = true
+    end
+    if a_user == recipient
+      self.archived_by_recipient = true
+    end
+  end
   protected
     def send_email_notification_if_required
       if recipient.wants_email_notifications? and (recipient != sender)
@@ -161,8 +169,11 @@ class Message < ActiveRecord::Base
       if root_message
         if root_message.sender == recipient
           root_message.has_unread_replies = true
-          root_message.save
+          root_message.archived_by_sender = false
+        else
+          root_message.archived_by_recipient = false
         end
+        root_message.save
       end
     end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 07ce9f4..c3a57a1 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -87,6 +87,10 @@ class User < ActiveRecord::Base
       count(:all, :conditions => {:aasm_state => "unread"})
     end
   end
+  
+  def all_messages
+    Message.find(:all, :conditions => ["sender_id = ? OR recipient_id = ?", self, self])
+  end
 
   Paperclip::Attachment.interpolations['login'] = lambda{|attachment,style| attachment.instance.login}
   
@@ -101,6 +105,11 @@ class User < ActiveRecord::Base
     Message.find_by_sql(["SELECT * FROM messages WHERE (has_unread_replies=? AND sender_id=?) OR recipient_id=? AND in_reply_to_id IS NULL ORDER BY created_at DESC", true,self, self])
   end
   
+  # Top level messages, excluding message threads that have been archived by me
+  def messages_in_inbox
+    Message.find_by_sql(["SELECT * FROM messages WHERE ((sender_id=? AND archived_by_sender=?) OR (recipient_id=? AND archived_by_recipient=?)) AND in_reply_to_id IS NULL ORDER BY created_at DESC", self, false, self, false])
+  end
+  
   has_many :sent_messages, :class_name => "Message", :foreign_key => "sender_id", :order => "created_at DESC" do
     def top_level
       find(:all, :conditions => {:in_reply_to_id => nil})
diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb
index b0a7712..8fb51b8 100644
--- a/app/views/messages/_message.html.erb
+++ b/app/views/messages/_message.html.erb
@@ -16,13 +16,11 @@
 #   along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #++
 %>
-<tr class="<%= message.aasm_state_for_user(current_user) -%>" id="<%= dom_id(message) -%>">
+<tr class="<%= message.aasm_state_for_user(current_user) -%> <%= message.css_class %>" id="<%= dom_id(message) -%>">
   <td>
-    <%- if message.recipient == current_user and message.unread? -%>
-      <input type="checkbox" name="message_ids[]" value="<%= message.id -%>" class="select_msg" />
-    <%- end -%>
+    <input type="checkbox" name="message_ids[]" value="<%= message.id -%>" class="select_msg" />
   </td>
-  <td>
+  <td class="who">
     <%= sender_and_recipient_display(message) %> (<%= message.number_of_messages_in_thread %>)
   </td>
   <td>
diff --git a/app/views/messages/_messages.html.erb b/app/views/messages/_messages.html.erb
index 8365ba0..f6601e8 100644
--- a/app/views/messages/_messages.html.erb
+++ b/app/views/messages/_messages.html.erb
@@ -18,7 +18,9 @@
 %>
 <%= form_tag(bulk_update_messages_path) %>
 <div>
-  <%= link_to_function("Select all unread messages", "$$('.select_msg').each(function(c){c.checked='checked'})") %>
+  Select 
+  <%= link_to_function("All messages", "$$('.select_msg').each(function(c){c.checked=(c.checked ? '' : 'checked')})") %>,
+  <%= link_to_function("All unread messages", "$$('.unread .select_msg').each(function(c){c.checked=(c.checked ? '' : 'checked')})") %>
 </div>
 <table class="message_list">
   <tr>
@@ -31,6 +33,12 @@
       <%= render :partial => message %>
   <% end %>
 </table>
-
-<input type="submit" value="Mark selected messages as read" />
+<label>
+  Action:
+</label>
+<select name="requested_action">
+  <option value="">Read</option>
+  <option value="archive">Archive</option>
+</select>
+<input type="submit" value="Apply to selected messages" />
 </form>
diff --git a/app/views/messages/_sidebar.html.erb b/app/views/messages/_sidebar.html.erb
index 1fa8cd9..b07f490 100644
--- a/app/views/messages/_sidebar.html.erb
+++ b/app/views/messages/_sidebar.html.erb
@@ -36,4 +36,9 @@
         link_to(t("views.messages.sent_messages"), sent_messages_path, :class => "current")
     end -%>
   </li>
+  <li class="all_emails">
+    <%= link_to_unless_current t("views.messages.all_messages"), all_messages_path do 
+        link_to(t("views.messages.all_messages"), all_messages_path, :class => "current")
+    end -%>
+  </li>
 </ul>
diff --git a/app/views/messages/all.html.erb b/app/views/messages/all.html.erb
new file mode 100644
index 0000000..ec10d1a
--- /dev/null
+++ b/app/views/messages/all.html.erb
@@ -0,0 +1,30 @@
+<%
+#--
+#   Copyright (C) 2009 Nokia Corporation and/or its subsidiary(-ies)
+#
+#   This program is free software: you can redistribute it and/or modify
+#   it under the terms of the GNU Affero General Public License as published by
+#   the Free Software Foundation, either version 3 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 Affero General Public License for more details.
+#
+#   You should have received a copy of the GNU Affero General Public License
+#   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#++
+%>
+
+<%= breadcrumbs_from(@root) -%>
+
+<h1><%= t "views.messages.all_messages" %></h1>
+
+<%= render :partial => "messages", :locals => {:messages => @messages} %>
+
+<% content_for(:sidebar) do %>
+  <%= render :partial => "sidebar" %>
+<% end %>
+
+<%= will_paginate(@messages) %>
\ No newline at end of file
diff --git a/config/locales/en.rb b/config/locales/en.rb
index e795d7a..5d28a4d 100644
--- a/config/locales/en.rb
+++ b/config/locales/en.rb
@@ -547,6 +547,7 @@
         :index_message => "Inbox",
         :reply => "Reply",
         :received_messages => "Inbox",
+        :all_messages => 'Message archive',
         :sent_messages => "Sent messages",
         :new => "Compose a message",
         :mark_as_read => "Mark as read"
diff --git a/config/routes.rb b/config/routes.rb
index 00798a8..e4a591a 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -137,7 +137,7 @@ ActionController::Routing::Routes.draw do |map|
   
   map.resources :messages, 
     :member => {:reply => :post, :read => :put}, 
-    :collection => {:auto_complete_for_recipient_login => :post, :sent => :get, :bulk_update => :put}
+    :collection => {:auto_complete_for_recipient_login => :post, :sent => :get, :bulk_update => :put, :all => :get}
   
   map.with_options :controller => 'sessions' do |session|
     session.login    '/login',  :action => 'new'
diff --git a/db/migrate/20090604111146_adding_archived_state_to_messages.rb b/db/migrate/20090604111146_adding_archived_state_to_messages.rb
new file mode 100644
index 0000000..a241302
--- /dev/null
+++ b/db/migrate/20090604111146_adding_archived_state_to_messages.rb
@@ -0,0 +1,11 @@
+class AddingArchivedStateToMessages < ActiveRecord::Migration
+  def self.up
+    add_column :messages, :archived_by_sender, :boolean, :default => false
+    add_column :messages, :archived_by_recipient, :boolean, :default => false
+  end
+
+  def self.down
+    remove_column :messages, :archived_by_sender
+    remove_column :messages, :archived_by_recipient
+  end
+end
diff --git a/public/stylesheets/base.css b/public/stylesheets/base.css
index e505a1d..d9db6a3 100644
--- a/public/stylesheets/base.css
+++ b/public/stylesheets/base.css
@@ -563,6 +563,7 @@ li.emails > a { background-image: url("/images/silk/email.png") !important; }
 li.new_email > a { background-image: url("/images/silk/email_add.png") !important; }
 li.sent_emails > a { background-image: url("/images/silk/email_go.png") !important; }
 li.received_emails > a { background-image: url("/images/silk/email_open.png") !important; }
+li.all_emails > a { background-image: url("/images/silk/folder.png") !important; }
 
 /* Icons used both for events and actions/breadcrums */
 li.event_instance.create_repository, li.create_repository > a { background-image: url("/images/silk/database_add.png") !important; }
@@ -2249,6 +2250,22 @@ div.archive-download-box p {
 {
   font-weight: bold;
 }
+.message_list td.who
+{
+  padding-left: 22px;
+  background-position: 2px 2px;
+  background-repeat: no-repeat;
+}
+.message_list tr.merge_request td.who,
+.message_list tr.membership td.who,
+.message_list tr.committership td.who
+{
+  background-image: url(/images/silk/server.png);
+}
+.message_list tr.message td.who
+{
+  background-image: url(/images/silk/comments.png);
+}
 .message_full
 {
   border: 1px solid #ddd;
diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb
index 9017642..886e842 100644
--- a/test/functional/messages_controller_test.rb
+++ b/test/functional/messages_controller_test.rb
@@ -223,7 +223,7 @@ class MessagesControllerTest < ActionController::TestCase
       }
     end
     
-    should 'should mark the selected messages as read' do
+    should 'mark the selected messages as read when supplying no action' do
       @request.session[:user_id] = @recipient.id
       put :bulk_update, :message_ids => @messages.collect(&:id)
       assert_response :redirect
@@ -231,6 +231,15 @@ class MessagesControllerTest < ActionController::TestCase
         assert msg.reload.read?
       end
     end
+    
+    should 'archive the selected messages for the current user' do
+      @request.session[:user_id] = @recipient.id
+      put :bulk_update, :message_ids => @messages.collect(&:id), :requested_action => 'archive'
+      assert_response :redirect
+      @messages.each do |msg|
+        assert msg.reload.archived_by_recipient?
+      end
+    end
   end
   
   context 'Unauthenticated GET to index' do
diff --git a/test/unit/message_test.rb b/test/unit/message_test.rb
index 5ab6ba3..ac321b0 100644
--- a/test/unit/message_test.rb
+++ b/test/unit/message_test.rb
@@ -251,5 +251,58 @@ class MessageTest < ActiveSupport::TestCase
       end
     end
   end
+  
+  context 'Archive state' do
+    setup do
+      @sender = Factory.create(:user)
+      @recipient = Factory.create(:user)
+      @message = Factory.create(:message, :sender => @sender, :recipient => @recipient)
+    end
+    
+    should 'be marked as archived by both sender and recipient when it is the same user' do
+      @message = Factory.create(:message, :sender => @sender, :recipient => @sender)
+      @message.archived_by(@sender)
+      assert @message.archived_by_sender?
+      assert @message.archived_by_recipient?
+    end
+    
+    should 'initially be unread_by_both' do
+      assert !@message.archived_by_sender?
+      assert !@message.archived_by_recipient?
+    end
+    
+    should 'be archived by sender' do
+      @message.archived_by(@sender)
+      assert @message.archived_by_sender?
+      assert !@message.archived_by_recipient?
+    end
+    
+    should 'be archived by recipient' do
+      @message.archived_by(@recipient)
+      assert @message.archived_by_recipient?
+      assert !@message.archived_by_sender?
+    end
+    
+    should 'be archived by both sender and recipient' do
+      @message.archived_by(@sender)
+      @message.archived_by(@recipient)
+      assert @message.archived_by_sender?
+      assert @message.archived_by_recipient?
+    end
+    
+    should 'be reset when a reply is created' do
+      @message.archived_by(@sender)
+      @message.save
+      reply = @message.build_reply(:body => "Foo")
+      assert reply.save
+      assert !@message.reload.archived_by_sender?
+      
+      @message.archived_by(@recipient)
+      @message.save
+      reply_to_reply = reply.build_reply(:body => "Kthxbye")
+      assert reply_to_reply.save
+      assert !@message.reload.archived_by_recipient?
+    end
+  end
 
 end
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index e2596da..761666d 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -277,6 +277,12 @@ class UserTest < ActiveSupport::TestCase
       assert @sender.sent_messages.include?(@message)
     end
     
+    should 'have an all_messages method that returns all messages to or from self' do
+      assert @sender.all_messages.include?(@message)
+      assert @recipient.all_messages.include?(@message)
+      assert !users(:mike).all_messages.include?(@message)
+    end
+    
     should 'know of its sent messages' do
       assert @recipient.received_messages.include?(@message)
     end
@@ -295,12 +301,22 @@ class UserTest < ActiveSupport::TestCase
       
       should 'include messages to self' do
         assert @recipient.top_level_messages.include?(@message)
+        assert_equal @recipient, @message.recipient
+        assert !@message.archived_by_recipient?
+        assert @recipient.messages_in_inbox.include?(@message)
+        @message.archived_by(@recipient)
+        assert @message.save
+        assert !@recipient.messages_in_inbox.include?(@message)
       end
       
       should 'include messages from self with unread replies' do
         reply = @message.build_reply(:body => "Thx")
         assert reply.save
-        assert @recipient.top_level_messages.include?(@message)
+        assert @sender.top_level_messages.include?(@message)
+        assert @sender.messages_in_inbox.include?(@message)
+        @message.archived_by(@sender)
+        assert @message.save
+        assert !@sender.messages_in_inbox.include?(@message)
       end
       
       should 'not include messages from someone else with unread replies' do
@@ -308,10 +324,12 @@ class UserTest < ActiveSupport::TestCase
         another_reply = another_message.build_reply(:body => "Not for you")
         assert another_reply.save
         assert !@sender.top_level_messages.include?(another_message)
+        assert !@sender.messages_in_inbox.include?(another_message)
       end
     end
   end
   
+  
   context 'Avatars' do
     setup {@user = users(:johan)}