diff --git a/app/controllers/dmails_controller.rb b/app/controllers/dmails_controller.rb index 960503dbf..869238bec 100644 --- a/app/controllers/dmails_controller.rb +++ b/app/controllers/dmails_controller.rb @@ -33,7 +33,7 @@ class DmailsController < ApplicationController end def create - @dmail = Dmail.create_split(dmail_params(:create)) + @dmail = Dmail.create_split(from: CurrentUser.user, creator_ip_addr: CurrentUser.ip_addr, **dmail_params(:create)) respond_with(@dmail) end diff --git a/app/models/dmail.rb b/app/models/dmail.rb index 882ad7f83..491c06d72 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -10,7 +10,6 @@ class Dmail < ApplicationRecord belongs_to :from, :class_name => "User" has_many :moderation_reports, as: :model - after_initialize :initialize_attributes, if: :new_record? before_create :autoreport_spam after_save :update_unread_dmail_count after_commit :send_email, on: :create @@ -28,11 +27,6 @@ class Dmail < ApplicationRecord def to_name=(name) self.to = User.find_by_name(name) end - - def initialize_attributes - self.from_id ||= CurrentUser.id - self.creator_ip_addr ||= CurrentUser.ip_addr - end end module FactoryMethods @@ -59,12 +53,10 @@ class Dmail < ApplicationRecord end def create_automated(params) - CurrentUser.as_system do - dmail = Dmail.new(from: User.system, **params) - dmail.owner = dmail.to - dmail.save - dmail - end + dmail = Dmail.new(from: User.system, creator_ip_addr: "127.0.0.1", **params) + dmail.owner = dmail.to + dmail.save + dmail end end diff --git a/test/functional/dmails_controller_test.rb b/test/functional/dmails_controller_test.rb index 607b8aa46..f62d01c1c 100644 --- a/test/functional/dmails_controller_test.rb +++ b/test/functional/dmails_controller_test.rb @@ -166,12 +166,10 @@ class DmailsControllerTest < ActionDispatch::IntegrationTest @sender = create(:user) @recipient = create(:user) - as(@sender) do - @dmail1 = Dmail.create_split(title: "test1", body: "test", to: @recipient) - @dmail2 = Dmail.create_split(title: "test2", body: "test", to: @recipient) - @dmail3 = Dmail.create_split(title: "test3", body: "test", to: @recipient, is_read: true) - @dmail4 = Dmail.create_split(title: "test4", body: "test", to: @recipient, is_deleted: true) - end + @dmail1 = create(:dmail, from: @sender, owner: @recipient, to: @recipient) + @dmail2 = create(:dmail, from: @sender, owner: @recipient, to: @recipient) + @dmail3 = create(:dmail, from: @sender, owner: @recipient, to: @recipient, is_read: true) + @dmail4 = create(:dmail, from: @sender, owner: @recipient, to: @recipient, is_deleted: true) end should "mark all unread, undeleted dmails as read" do @@ -182,7 +180,7 @@ class DmailsControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_equal(0, @recipient.reload.unread_dmail_count) - assert_equal(true, [@dmail1, @dmail2, @dmail3, @dmail4].all?(&:is_read)) + assert_equal(true, [@dmail1, @dmail2, @dmail3, @dmail4].map(&:reload).all?(&:is_read)) end end diff --git a/test/unit/dmail_test.rb b/test/unit/dmail_test.rb index 9a85b8509..0814c2181 100644 --- a/test/unit/dmail_test.rb +++ b/test/unit/dmail_test.rb @@ -29,14 +29,11 @@ class DmailTest < ActiveSupport::TestCase end context "from a banned user" do - setup do - @user.update_attribute(:is_banned, true) - end - should "not validate" do - dmail = FactoryBot.build(:dmail, :title => "xxx", :owner => @user) - dmail.save - assert_equal(1, dmail.errors.size) + user = create(:banned_user) + dmail = build(:dmail, owner: user, from: user) + + assert_equal(false, dmail.valid?) assert_equal(["Sender is banned and cannot send messages"], dmail.errors.full_messages) end end @@ -85,15 +82,10 @@ class DmailTest < ActiveSupport::TestCase should "create a copy for each user" do @new_user = FactoryBot.create(:user) assert_difference("Dmail.count", 2) do - Dmail.create_split(:to_id => @new_user.id, :title => "foo", :body => "foo") + Dmail.create_split(from: CurrentUser.user, creator_ip_addr: "127.0.0.1", to_id: @new_user.id, title: "foo", body: "foo") end end - should "record the creator's ip addr" do - dmail = FactoryBot.create(:dmail, owner: @user) - assert_equal(CurrentUser.ip_addr, dmail.creator_ip_addr.to_s) - end - should "send an email if the user wants it" do user = create(:user, receive_email_notifications: true) assert_difference("ActionMailer::Base.deliveries.size", 1) do @@ -104,14 +96,14 @@ class DmailTest < ActiveSupport::TestCase should "create only one message for a split response" do user = FactoryBot.create(:user, :receive_email_notifications => true) assert_difference("ActionMailer::Base.deliveries.size", 1) do - Dmail.create_split(:to_id => user.id, :title => "foo", :body => "foo") + Dmail.create_split(from: CurrentUser.user, creator_ip_addr: "127.0.0.1", to_id: user.id, title: "foo", body: "foo") end end should "notify the recipient he has mail" do recipient = create(:user) - Dmail.create_split(title: "hello", body: "hello", to_id: recipient.id) + create(:dmail, owner: recipient, to: recipient) assert_equal(1, recipient.reload.unread_dmail_count) recipient.dmails.unread.last.update!(is_read: true)