how to change inbox messages mark from unread to read

Question

I'm building a inbox messaging system. All my messages are appearing unread, when most should be marked as 'read'. I have it set in my messages model to check if unread or read. So I'm not understanding why the code is not working for unread/read.

message.rb:

belongs_to :sender,
    :class_name => 'User',
    :foreign_key => 'sender_id'
belongs_to :recepient,
    :class_name => 'User',
    :foreign_key => 'recepient_id'

    def mark_message_deleted(id,user_id)
        self.sender_deleted = true if self.sender_id == user_id and self.id=id
        self.recepient_deleted = true if self.recepient_id == user_id and self.id=id
        self.sender_deleted && self.recepient_deleted ? self.destroy : save!
    end

    # Read message and if it is read by recepient then mark it is read
    def self.readingmessage(id, reader)
        message = find(id, :conditions => ["sender_id = ? OR recepient_id = ?", reader, reader])
        if message.read_at.nil? && (message.recepient.user_id==reader)
            message.read_at = Time.now
            message.save!
        end
        message
    end

    # Based on if a message has been read by it's recepient returns true or false.
    def read?
        self.read_at.nil? ? false : true
    end

    def self.received_by(user)
       where(:recepient_id => user.id)
     end

     def self.not_recepient_deleted
       where("recepient_deleted = ?", false)
     end

end

messages controller:

class MessagesController < ApplicationController

  before_filter :set_user

  def index
    if params[:mailbox] == "sent"
      @messages = @user.sent_messages
    elsif params[:mailbox] == "inbox"
      @messages = @user.received_messages
    #elsif params[:mailbox] == "archieved"
     # @messages = @user.archived_messages
    end
  end

  def new
    @message = Message.new
    if params[:reply_to]
      @reply_to = User.find_by_user_id(params[:reply_to])
      unless @reply_to.nil?
        @message.recepient_id = @reply_to.user_id
      end
    end
  end

  def create
    @message = Message.new(params[:message])
    @message.sender_id = @user_id
    if @message.save
      flash[:notice] = "Message has been sent"
      redirect_to user_messages_path(current_user, :mailbox=>:inbox)
    else
      render :action => :new
    end
  end

  def show
    @message = Message.find(params[:id])
  end

  def delete_multiple
      if params[:delete]
        params[:delete].each { |id|
          @message = Message.find(id)
          @message.mark_message_deleted(@message.id,@user_id) unless @message.nil?
        }
        flash[:notice] = "Messages deleted"
      end
      redirect_to user_messages_path(@user, @messages)
  end

  private
    def set_user
      @user = current_user
    end
end

index view:

<h2>Your Inbox</h2>
<% if @messages.size == 0 %>
No messages in your Inbox
<% end %>
<% else %>
        <%= form_tag delete_multiple_user_messages_path, :method=>:post do %>
            <table  class="table table-bordered">
                    <tr>
                    <th>Delete?</th>
                    <th>Sent</th>
                    <th>Sender</th>
                    <th>Sent</th>
                </tr>
                    <% for message in @messages %>
                        <tr>
                            <td><%= check_box_tag "delete[]", message.id %></td>
                            <td>
                                <% if message.read? %>
                                  <%= link_to h(message.subject), user_message_path(@user, message) %>
                                <% else %>
                                  <%= link_to "#{h(message.subject)} (unread)", user_message_path(@user, message) %>
                                <% end %>
                            </td>

                            <td><%=h message.created_at.to_s(:long) %></td>
                        </tr>
            <% end %>
            </table>
        <%= submit_tag "Delete selected" %> |
        <% end %>
<%= link_to "Compose New Message", new_user_message_path(@user)%> | 
<%= link_to "View Sent Messages", user_messages_path(current_user, :mailbox => :sent)%>

Answer 1

Your messsages are all appearing as unread because the read_at field is never changed fromnil.

It looks like your readingmessage method is supposed to change the value, but it's never called.

(It would make sense to call it in the show method in your controller.)

Model:

def readingmessage
  self.read_at ||= Time.now
  save
end

Controller:

  def show
    @message = Message.find(params[:id])
    @message.readingmessage if @message.recipient == current_user
  end

Answer 2

This method is doing too much:

# Read message and if it is read by recepient then mark it is read
def self.readingmessage(id, reader)
    message = find(id, :conditions => ["sender_id = ? OR recepient_id = ?", reader, reader])
    if message.read_at.nil? && (message.recepient.user_id==reader)
        message.read_at = Time.now
        message.save!
    end
    message
end

I don't understand why you're calling this method when the sender is reading the message, but if that's what you need, I'd go for something like:

# Read message and if it is read by recepient then mark it is read
def self.readingmessage(id, reader)
    message = find(id, :conditions => ["sender_id = ? OR recepient_id = ?", reader, reader])
    message
end

def self.mark_read(id, reader)
      # Read message and if it is read by recepient then mark it is read
    def self.readingmessage(id, reader)
        message = find(id, :conditions => [recepient_id = ? and not null read_at", reader])
        if message
            message.read_at = Time.now
            message.save!
        end
        message
    end
end

I assume your controller/view knows when the recipient is reading a message and the id of the message in question, in which case mark_read can be simplified considerably