diff --git a/app/controllers/dmails_controller.rb b/app/controllers/dmails_controller.rb
index de6619fc7..97e19a1d7 100644
--- a/app/controllers/dmails_controller.rb
+++ b/app/controllers/dmails_controller.rb
@@ -9,7 +9,7 @@ class DmailsController < ApplicationController
check_privilege(parent)
@dmail = parent.build_response(:forward => params[:forward])
else
- @dmail = Dmail.new(params[:dmail])
+ @dmail = Dmail.new(create_params)
end
respond_with(@dmail)
@@ -39,7 +39,7 @@ class DmailsController < ApplicationController
end
def create
- @dmail = Dmail.create_split(params[:dmail].merge(:creator_ip_addr => request.remote_ip))
+ @dmail = Dmail.create_split(create_params)
respond_with(@dmail)
end
@@ -66,4 +66,8 @@ private
raise User::PrivilegeError
end
end
+
+ def create_params
+ params.fetch(:dmail, {}).permit(:title, :body, :to_name, :to_id)
+ end
end
diff --git a/app/helpers/dmails_helper.rb b/app/helpers/dmails_helper.rb
index 869a21820..3e700b95d 100644
--- a/app/helpers/dmails_helper.rb
+++ b/app/helpers/dmails_helper.rb
@@ -2,11 +2,23 @@ module DmailsHelper
def dmails_current_folder_path
case cookies[:dmail_folder]
when "sent"
- dmails_path(:search => {:owner_id => CurrentUser.id, :from_id => CurrentUser.id}, :folder => "sent")
- when "all"
- dmails_path(:search => {:owner_id => CurrentUser.id}, :folder => "all")
+ sent_dmails_path
+ when "received"
+ received_dmails_path
else
- dmails_path(:search => {:owner_id => CurrentUser.id, :to_id => CurrentUser.id}, :folder => "received")
+ all_dmails_path
end
end
+
+ def all_dmails_path(params = {})
+ dmails_path(folder: "all", **params)
+ end
+
+ def sent_dmails_path(params = {})
+ dmails_path(search: {from_id: CurrentUser.id}, folder: "sent", **params)
+ end
+
+ def received_dmails_path(params = {})
+ dmails_path(search: {to_id: CurrentUser.id}, folder: "received", **params)
+ end
end
diff --git a/app/logical/approver_pruner.rb b/app/logical/approver_pruner.rb
index 089f615d3..8d9300bfc 100644
--- a/app/logical/approver_pruner.rb
+++ b/app/logical/approver_pruner.rb
@@ -22,7 +22,7 @@ class ApproverPruner
user.save
end
- Dmail.create_split(
+ Dmail.create_automated(
:to_id => user.id,
:title => "Approver inactivity",
:body => "You haven't approved a post in the past three months. In order to make sure the list of active approvers is up-to-date, you have lost your approver privileges. Please reply to this message if you want to be reinstated."
diff --git a/app/logical/user_promotion.rb b/app/logical/user_promotion.rb
index 981c9e2fd..7a28f47c1 100644
--- a/app/logical/user_promotion.rb
+++ b/app/logical/user_promotion.rb
@@ -84,7 +84,7 @@ private
end
def create_dmail
- Dmail.create_split(
+ Dmail.create_automated(
:to_id => user.id,
:title => "You have been promoted",
:body => build_messages
diff --git a/app/models/dmail.rb b/app/models/dmail.rb
index c363cc0f5..d6263d1d6 100644
--- a/app/models/dmail.rb
+++ b/app/models/dmail.rb
@@ -1,19 +1,22 @@
require 'digest/sha1'
class Dmail < ActiveRecord::Base
- validates_presence_of :to_id
- validates_presence_of :from_id
- validates_format_of :title, :with => /\S/
- validates_format_of :body, :with => /\S/
- validate :validate_sender_is_not_banned
- before_validation :initialize_from_id, :on => :create
+ with_options on: :create do
+ validates_presence_of :to_id
+ validates_presence_of :from_id
+ validates_format_of :title, :with => /\S/
+ validates_format_of :body, :with => /\S/
+ validate :validate_sender_is_not_banned
+ end
+
belongs_to :owner, :class_name => "User"
belongs_to :to, :class_name => "User"
belongs_to :from, :class_name => "User"
+
+ after_initialize :initialize_attributes, if: :new_record?
before_create :auto_read_if_filtered
after_create :update_recipient
after_create :send_dmail
- attr_accessible :title, :body, :is_deleted, :to_id, :to, :to_name, :creator_ip_addr
module AddressMethods
def to_name
@@ -25,13 +28,12 @@ class Dmail < ActiveRecord::Base
end
def to_name=(name)
- user = User.find_by_name(name)
- return if user.nil?
- self.to_id = user.id
+ self.to_id = User.name_to_id(name)
end
- def initialize_from_id
- self.from_id = CurrentUser.id
+ def initialize_attributes
+ self.from_id ||= CurrentUser.id
+ self.creator_ip_addr ||= CurrentUser.ip_addr
end
end
@@ -43,14 +45,14 @@ class Dmail < ActiveRecord::Base
copy = nil
Dmail.transaction do
+ # recipient's copy
copy = Dmail.new(params)
copy.owner_id = copy.to_id
- unless copy.to_id == CurrentUser.id
- copy.save
- end
+ copy.save unless copy.to_id == copy.from_id
+ # sender's copy
copy = Dmail.new(params)
- copy.owner_id = CurrentUser.id
+ copy.owner_id = copy.from_id
copy.is_read = true
copy.save
end
@@ -58,10 +60,8 @@ class Dmail < ActiveRecord::Base
copy
end
- def new_blank
- Dmail.new do |dmail|
- dmail.from_id = CurrentUser.id
- end
+ def create_automated(params)
+ create_split(from: Danbooru.config.system_user, **params)
end
end
@@ -91,18 +91,6 @@ class Dmail < ActiveRecord::Base
end
module SearchMethods
- def for(user)
- where("owner_id = ?", user)
- end
-
- def inbox
- where("to_id = owner_id")
- end
-
- def sent
- where("from_id = owner_id")
- end
-
def active
where("is_deleted = ?", false)
end
@@ -144,10 +132,6 @@ class Dmail < ActiveRecord::Base
q = q.search_message(params[:message_matches])
end
- if params[:owner_id].present?
- q = q.for(params[:owner_id].to_i)
- end
-
if params[:to_name].present?
q = q.to_name_matches(params[:to_name])
end
@@ -206,6 +190,10 @@ class Dmail < ActiveRecord::Base
end
end
+ def is_automated?
+ from == Danbooru.config.system_user
+ end
+
def filtered?
CurrentUser.dmail_filter.try(:filtered?, self)
end
diff --git a/app/models/janitor_trial.rb b/app/models/janitor_trial.rb
index 481cbf815..d7e1a55cb 100644
--- a/app/models/janitor_trial.rb
+++ b/app/models/janitor_trial.rb
@@ -73,7 +73,7 @@ class JanitorTrial < ActiveRecord::Base
def send_dmail
body = "You have been selected as a test janitor. You can now approve pending posts and have access to the moderation interface. You should reacquaint yourself with the [[howto:upload]] guide to make sure you understand the site rules.\n\nOver the next several weeks your approvals will be monitored. If the majority of them are not quality uploads you will fail the trial period and lose your approval privileges. You will also receive a negative user record indicating you previously attempted and failed a test janitor trial.\n\nThere is a minimum quota of 1 approval a month to indicate that you are being active. Remember, the goal isn't to approve as much as possible. It's to filter out borderline-quality art.\n\nIf you have any questions please respond to this message."
- Dmail.create_split(:title => "Test Janitor Trial Period", :body => body, :to_id => user_id)
+ Dmail.create_automated(:title => "Test Janitor Trial Period", :body => body, :to_id => user_id)
end
def promote_user
diff --git a/app/models/post_disapproval.rb b/app/models/post_disapproval.rb
index b1045b054..052764503 100644
--- a/app/models/post_disapproval.rb
+++ b/app/models/post_disapproval.rb
@@ -17,27 +17,20 @@ class PostDisapproval < ActiveRecord::Base
end
def self.dmail_messages!
- admin = User.admins.first
- disapprovals = {}
-
- PostDisapproval.with_message.where("created_at >= ?", 1.day.ago).find_each do |disapproval|
- disapprovals[disapproval.post.uploader_id] ||= []
- disapprovals[disapproval.post.uploader_id] << disapproval
+ disapprovals = PostDisapproval.with_message.where("created_at >= ?", 1.day.ago).group_by do |pd|
+ pd.post.uploader
end
- disapprovals.each do |user_id, list|
- user = User.find(user_id)
- CurrentUser.scoped(admin, "127.0.0.1") do
- message = list.map do |x|
- "* post ##{x.post_id}: #{x.message}"
- end.join("\n")
+ disapprovals.each do |uploader, list|
+ message = list.map do |x|
+ "* post ##{x.post_id}: #{x.message}"
+ end.join("\n")
- Dmail.create_split(
- :to_id => user.id,
- :title => "Some of your uploads have been critiqued by the moderators",
- :body => message
- )
- end
+ Dmail.create_automated(
+ :to_id => uploader.id,
+ :title => "Some of your uploads have been critiqued by the moderators",
+ :body => message
+ )
end
end
diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb
index a2e06b102..143c88dd2 100644
--- a/app/models/user_feedback.rb
+++ b/app/models/user_feedback.rb
@@ -92,7 +92,7 @@ class UserFeedback < ActiveRecord::Base
def create_dmail
unless disable_dmail_notification
body = %{#{creator_name} created a "#{category} record":/user_feedbacks?search[user_id]=#{user_id} for your account. #{body}}
- Dmail.create_split(:to_id => user_id, :title => "Your user record has been updated", :body => body)
+ Dmail.create_automated(:to_id => user_id, :title => "Your user record has been updated", :body => body)
end
end
diff --git a/app/models/user_name_change_request.rb b/app/models/user_name_change_request.rb
index 5920c3d20..eaf8cd372 100644
--- a/app/models/user_name_change_request.rb
+++ b/app/models/user_name_change_request.rb
@@ -8,7 +8,6 @@ class UserNameChangeRequest < ActiveRecord::Base
validates_length_of :desired_name, :within => 2..100, :on => :create
validates_format_of :desired_name, :with => /\A[^\s:]+\Z/, :on => :create, :message => "cannot have whitespace or colons"
before_validation :normalize_name
- # after_create :notify_admins
attr_accessible :status, :user_id, :original_name, :desired_name, :change_reason, :rejection_reason, :approver_id
def self.pending
@@ -49,19 +48,11 @@ class UserNameChangeRequest < ActiveRecord::Base
UserFeedback.for_user(user_id).order("id desc")
end
- def notify_admins
- title = "#{original_name} is requesting a name change to #{desired_name}"
- body = title + "\n\n\"See request\":/user_name_change_requests/#{id}"
- User.admins.find_each do |user|
- Dmail.create_split(:title => title, :body => body, :to_id => user.id)
- end
- end
-
def approve!
update_attributes(:status => "approved", :approver_id => CurrentUser.user.id)
user.update_attribute(:name, desired_name)
body = "Your name change request has been approved. Be sure to log in with your new user name."
- Dmail.create_split(:title => "Name change request approved", :body => body, :to_id => user_id)
+ Dmail.create_automated(:title => "Name change request approved", :body => body, :to_id => user_id)
UserFeedback.create(:user_id => user_id, :category => "neutral", :body => "Name changed from #{original_name} to #{desired_name}")
ModAction.log("Name changed from #{original_name} to #{desired_name}")
end
@@ -69,7 +60,7 @@ class UserNameChangeRequest < ActiveRecord::Base
def reject!(reason)
update_attributes(:status => "rejected", :rejection_reason => reason)
body = "Your name change request has been rejected for the following reason: #{rejection_reason}"
- Dmail.create_split(:title => "Name change request rejected", :body => body, :to_id => user_id)
+ Dmail.create_automated(:title => "Name change request rejected", :body => body, :to_id => user_id)
end
def not_limited
diff --git a/app/views/dmails/_secondary_links.html.erb b/app/views/dmails/_secondary_links.html.erb
index 7b7206f31..06a9d39dd 100644
--- a/app/views/dmails/_secondary_links.html.erb
+++ b/app/views/dmails/_secondary_links.html.erb
@@ -1,11 +1,14 @@
<% content_for(:secondary_links) do %>
<% end %>
diff --git a/app/views/dmails/edit.html.erb b/app/views/dmails/edit.html.erb
deleted file mode 100644
index 2777708ad..000000000
--- a/app/views/dmails/edit.html.erb
+++ /dev/null
@@ -1,12 +0,0 @@
-
-
-
Edit Message
- <%= render "form", :dmail => @dmail %>
-
-
Body
<%= format_text(@dmail.body, :ragel => true) %>
+
+ <% if @dmail.is_automated? %>
+
+ This is an automated message. Post in the forums if you have any questions.
+
+ <% end %>
diff --git a/config/danbooru_default_config.rb b/config/danbooru_default_config.rb
index 2d84aecd4..bbc0f771b 100644
--- a/config/danbooru_default_config.rb
+++ b/config/danbooru_default_config.rb
@@ -30,6 +30,11 @@ module Danbooru
"webmaster@#{server_host}"
end
+ # System actions, such as sending automated dmails, will be performed with this account.
+ def system_user
+ User.find_by_name("DanbooruBot") || User.admins.first
+ end
+
def upgrade_account_email
contact_email
end
diff --git a/config/routes.rb b/config/routes.rb
index 2af146ac7..2a5321886 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -110,7 +110,7 @@ Rails.application.routes.draw do
end
end
resources :delayed_jobs, :only => [:index]
- resources :dmails do
+ resources :dmails, :only => [:new, :create, :index, :show, :destroy] do
collection do
get :search
post :mark_all_as_read
diff --git a/test/factories/post_disapproval.rb b/test/factories/post_disapproval.rb
new file mode 100644
index 000000000..2f81d8783
--- /dev/null
+++ b/test/factories/post_disapproval.rb
@@ -0,0 +1,6 @@
+FactoryGirl.define do
+ factory(:post_disapproval) do
+ reason { %w(breaks_rules poor_quality disinterest).sample }
+ message { FFaker::Lorem.sentence }
+ end
+end
diff --git a/test/functional/dmails_controller_test.rb b/test/functional/dmails_controller_test.rb
index a89f1764c..a697a0b75 100644
--- a/test/functional/dmails_controller_test.rb
+++ b/test/functional/dmails_controller_test.rb
@@ -48,17 +48,17 @@ class DmailsControllerTest < ActionController::TestCase
context "index action" do
should "show dmails owned by the current user" do
- get :index, {:owner_id_equals => @dmail.owner_id, :folder => "sent"}, {:user_id => @dmail.owner_id}
+ get :index, {:search => {:owner_id => @dmail.owner_id, :folder => "sent"}}, {:user_id => @dmail.owner_id}
assert_response :success
assert_equal(1, assigns[:dmails].size)
- get :index, {:owner_id_equals => @dmail.owner_id, :folder => "received"}, {:user_id => @dmail.owner_id}
+ get :index, {:search => {:owner_id => @dmail.owner_id, :folder => "received"}}, {:user_id => @dmail.owner_id}
assert_response :success
assert_equal(1, assigns[:dmails].size)
end
should "not show dmails not owned by the current user" do
- get :index, {:owner_id_equals => @dmail.owner_id}, {:user_id => @unrelated_user.id}
+ get :index, {:search => {:owner_id => @dmail.owner_id}}, {:user_id => @unrelated_user.id}
assert_response :success
assert_equal(0, assigns[:dmails].size)
end
diff --git a/test/unit/post_disapproval_test.rb b/test/unit/post_disapproval_test.rb
index 76c75574a..33331e31a 100644
--- a/test/unit/post_disapproval_test.rb
+++ b/test/unit/post_disapproval_test.rb
@@ -64,6 +64,33 @@ class PostDisapprovalTest < ActiveSupport::TestCase
end
end
end
+
+ context "when sending dmails" do
+ setup do
+ @uploaders = FactoryGirl.create_list(:user, 2)
+ @disapprovers = FactoryGirl.create_list(:mod_user, 2)
+
+ # 2 uploaders, with 2 uploads each, and 2 disapprovals on each upload.
+ @uploaders.each do |uploader|
+ FactoryGirl.create_list(:post, 2, uploader: uploader).each do |post|
+ FactoryGirl.create(:post_disapproval, post: post, user: @disapprovers[0])
+ FactoryGirl.create(:post_disapproval, post: post, user: @disapprovers[1])
+ end
+ end
+ end
+
+ should "dmail the uploaders" do
+ bot = FactoryGirl.create(:user)
+ Danbooru.config.stubs(:system_user).returns(bot)
+
+ assert_difference(["@uploaders[0].dmails.count", "@uploaders[1].dmails.count"], 1) do
+ PostDisapproval.dmail_messages!
+ end
+
+ assert(@uploaders[0].dmails.exists?(from: bot, to: @uploaders[0]))
+ assert(@uploaders[1].dmails.exists?(from: bot, to: @uploaders[1]))
+ end
+ end
end
end
end
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index a16e817d9..ffb505616 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -13,7 +13,7 @@ class UserTest < ActiveSupport::TestCase
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
-
+
context "promoting a user" do
setup do
CurrentUser.user = FactoryGirl.create(:moderator_user)
@@ -27,10 +27,16 @@ class UserTest < ActiveSupport::TestCase
assert_equal("You have been promoted to a Gold level account from Member.", @user.feedback.last.body)
end
- should "create a dmail" do
+ should "send an automated dmail to the user" do
+ bot = FactoryGirl.create(:user)
+ Danbooru.config.stubs(:system_user).returns(bot)
+
assert_difference("Dmail.count", 2) do
@user.promote_to!(User::Levels::GOLD)
end
+
+ assert(@user.dmails.exists?(from: bot, to: @user, title: "You have been promoted"))
+ assert(bot.dmails.exists?(from: bot, to: @user, title: "You have been promoted"))
end
end