Store message receivers in a new table instead of in a serializied value

db/migrate/20181201000100_create_message_recipients.foodsoft_messages.rb

@@ -0,0 +1,51 @@
+class CreateMessageRecipients < ActiveRecord::Migration
+  class Message < ActiveRecord::Base
+    has_many :message_recipients
+  end
+
+  class MessageRecipient < ActiveRecord::Base
+  end
+
+  def up
+    create_table :message_recipients do |t|
+      t.references :message, index: true, null: false
+      t.references :user, null: false
+      t.integer :email_state, default: 0, null: false
+      t.datetime :read_at
+    end
+
+    add_index :message_recipients, [:user_id, :read_at]
+
+    Message.all.each do |m|
+      recipients = YAML.load(m.recipients_ids).map do |r|
+        {
+          message_id: m.id,
+          user_id: r,
+          email_state: m.email_state
+        }
+      end
+      MessageRecipient.create! recipients
+    end
+
+    change_table :messages do |t|
+      t.remove :recipients_ids
+      t.remove :email_state
+    end
+  end
+
+  def down
+    change_table :messages do |t|
+      t.text :recipients_ids
+      t.integer :email_state, default: 0, null: false
+    end
+
+    messages = Message.all.includes(:message_recipients).map do |m|
+      recipients = m.message_recipients.map(&:user_id)
+      m.recipients_ids = recipients.to_yaml
+      m.email_state = m.email_state
+      m.save!
+    end
+
+    drop_table :message_recipients
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181201000000) do
+ActiveRecord::Schema.define(version: 20181201000100) do
 
   create_table "article_categories", force: :cascade do |t|
     t.string "name",        limit: 255, default: "", null: false
@@ -248,12 +248,20 @@ ActiveRecord::Schema.define(version: 20181201000000) do
 
   add_index "memberships", ["user_id", "group_id"], name: "index_memberships_on_user_id_and_group_id", unique: true, using: :btree
 
+  create_table "message_recipients", force: :cascade do |t|
+    t.integer  "message_id",              null: false
+    t.integer  "user_id",                 null: false
+    t.integer  "email_state", default: 0, null: false
+    t.datetime "read_at"
+  end
+
+  add_index "message_recipients", ["message_id"], name: "index_message_recipients_on_message_id", using: :btree
+  add_index "message_recipients", ["user_id", "read_at"], name: "index_message_recipients_on_user_id_and_read_at", using: :btree
+
   create_table "messages", force: :cascade do |t|
     t.integer  "sender_id",      limit: 4
-    t.text     "recipients_ids", limit: 65535
     t.string   "subject",        limit: 255,                   null: false
     t.text     "body",           limit: 65535
-    t.integer  "email_state",    limit: 4,     default: 0,     null: false
     t.boolean  "private",                      default: false
     t.datetime "created_at"
     t.integer  "reply_to",       limit: 4

plugins/messages/app/controllers/messages_controller.rb

@@ -17,7 +17,7 @@ class MessagesController < ApplicationController
         @message.reply_to = original_message.reply_to
       end
       if original_message.is_readable_for?(current_user)
-        @message.add_recipients [original_message.sender]
+        @message.add_recipients [original_message.sender_id]
         @message.group_id = original_message.group_id
         @message.private = original_message.private
         @message.subject = I18n.t('messages.model.reply_subject', :subject => original_message.subject)

plugins/messages/app/mail_receivers/messages_mail_receiver.rb

@@ -49,8 +49,8 @@ class MessagesMailReceiver
     else
       message.subject = I18n.t('messages.model.reply_subject', subject: message.reply_to_message.subject)
     end
-    message.add_recipients @message.recipients
-    message.add_recipients [@message.sender]
+    message.add_recipients @message.recipients.pluck(:id)
+    message.add_recipients [@message.sender_id]
 
     message.save!
     Resque.enqueue(MessageNotifier, FoodsoftConfig.scope, "message_deliver", message.id)

plugins/messages/app/models/message.rb

@@ -4,44 +4,39 @@ class Message < ApplicationRecord
   belongs_to :sender, :class_name => "User", :foreign_key => "sender_id"
   belongs_to :group, :class_name => "Group", :foreign_key => "group_id"
   belongs_to :reply_to_message, :class_name => "Message", :foreign_key => "reply_to"
+  has_many :message_recipients, dependent: :destroy
+  has_many :recipients, through: :message_recipients, source: :user
 
-  serialize :recipients_ids, Array
   attr_accessor :send_method, :recipient_tokens, :order_id
 
-  scope :pending, -> { where(:email_state => 0) }
-  scope :sent, -> { where(:email_state => 1) }
   scope :pub, -> { where(:private => false) }
   scope :threads, -> { where(:reply_to => nil) }
   scope :thread, -> (id) { where("id = ? OR reply_to = ?", id, id) }
 
-  # Values for the email_state attribute: :none, :pending, :sent, :failed
-  EMAIL_STATE = {
-    :pending => 0,
-    :sent => 1,
-    :failed => 2
-  }
-
-  validates_presence_of :recipients_ids, :subject, :body
+  validates_presence_of :message_recipients, :subject, :body
   validates_length_of :subject, :in => 1..255
-  validates_inclusion_of :email_state, :in => EMAIL_STATE.values
 
   after_initialize do
     @send_method ||= 'recipients'
   end
 
   before_create :create_salt
-  before_validation :clean_up_recipient_ids, :on => :create
+  before_validation :create_message_recipients, on: :create
 
-  def clean_up_recipient_ids
-    add_recipients Group.find(group_id).users unless group_id.blank?
-    add_recipients Order.find(order_id).users_ordered if send_method == 'order'
-    self.recipients_ids = recipients_ids.uniq.reject { |id| id.blank? } unless recipients_ids.nil?
-    self.recipients_ids = User.undeleted.collect(&:id) if send_method == 'all'
+  def create_message_recipients
+    user_ids = @recipients_ids
+    user_ids += User.undeleted.pluck(:id) if send_method == 'all'
+    user_ids += Group.find(group_id).users.pluck(:id) unless group_id.blank?
+    user_ids += Order.find(order_id).users_ordered.pluck(:id) if send_method == 'order'
+
+    user_ids.uniq.each do |user_id|
+      recipient = MessageRecipient.new message: self, user_id: user_id
+      message_recipients << recipient
+    end
   end
 
   def add_recipients(users)
-    self.recipients_ids = [] if recipients_ids.blank?
-    self.recipients_ids += users.collect(&:id) unless users.blank?
+    @recipients_ids += users
   end
 
   def group_id=(group_id)
@@ -85,12 +80,11 @@ class Message < ApplicationRecord
 
   def recipient_tokens=(ids)
     @recipient_tokens = ids
-    add_recipients ids.split(",").collect { |id| User.find(id) }
+    @recipients_ids = ids.split(',').map(&:to_i)
   end
 
   def mail_to=(user_id)
-    user = User.find(user_id)
-    add_recipients([user])
+    @recipients_ids = [user_id]
   end
 
   def mail_hash_for_user(user)
@@ -112,8 +106,8 @@ class Message < ApplicationRecord
     system_message? ? I18n.t('layouts.foodsoft') : sender.display rescue "?"
   end
 
-  def recipients
-    User.where(id: recipients_ids)
+  def recipients_ids
+    @recipients_ids
   end
 
   def last_reply
@@ -121,7 +115,7 @@ class Message < ApplicationRecord
   end
 
   def is_readable_for?(user)
-    !private || sender == user || recipients_ids.include?(user.id)
+    !private || sender == user || message_recipients.where(user: user).any?
   end
 
   def can_toggle_private?(user)

plugins/messages/app/models/message_recipient.rb

@@ -0,0 +1,6 @@
+class MessageRecipient < ActiveRecord::Base
+  belongs_to :message
+  belongs_to :user
+
+  enum email_state: [:pending, :sent, :skipped]
+end

plugins/messages/app/workers/message_notifier.rb

@@ -5,14 +5,16 @@ class MessageNotifier < UserNotifier
     message_id = args.first
     message = Message.find(message_id)
 
-    message.recipients.each do |recipient|
+    message.message_recipients.each do |message_recipient|
+      recipient = message_recipient.user
       if recipient.receive_email?
         Mailer.deliver_now_with_user_locale recipient do
           MessagesMailer.foodsoft_message(recipient, message)
         end
+        message_recipient.update_attribute :email_state, :sent
+      else
+        message_recipient.update_attribute :email_state, :skipped
       end
     end
-
-    message.update_attribute(:email_state, 1)
   end
 end