unify user notifications for permission/level changes #2470

app/controllers/users_controller.rb

@@ -49,8 +49,6 @@ class UsersController < ApplicationController
   def update
     @user = User.find(params[:id])
     check_privilege(@user)
-    sanitize_params!
-    handle_promotion!
     @user.update_attributes(params[:user].except(:name), :as => CurrentUser.role)
     cookies.delete(:favorite_tags)
     cookies.delete(:favorite_tags_with_categories)
@@ -69,19 +67,6 @@ class UsersController < ApplicationController
   end
 
 private
-  def sanitize_params!
-    return if CurrentUser.is_admin?
-    
-    if params[:user] && params[:user][:level].to_i >= User::Levels::MODERATOR
-      params[:user][:level] = User::Levels::JANITOR
-    end
-  end
-
-  def handle_promotion!
-    if params[:user] && params[:user][:level]
-      UserPromotion.new(@user, CurrentUser.user, params[:user].delete(:level)).promote!
-    end
-  end
 
   def check_privilege(user)
     raise User::PrivilegeError unless (user.id == CurrentUser.id || CurrentUser.is_admin?)

app/logical/user_promotion.rb

@@ -23,22 +23,6 @@ class UserPromotion
     user.save
   end
 
-  def create_user_feedback
-    if user.level > user.level_was
-      body_prefix = "Promoted"
-    elsif user.level < user.level_was
-      body_prefix = "Demoted"
-    else
-      body_prefix = "Updated"
-    end
-
-    user.feedback.create(
-      :category => "neutral",
-      :body => "#{body_prefix} from #{user.level_string_was} to #{user.level_string}",
-      :disable_dmail_notification => true
-    )
-  end
-
 private
   
   def validate
@@ -56,39 +40,45 @@ private
     TransactionLogItem.record_account_upgrade(user)
   end
 
-  def create_dmail
-    if user.level >= user.level_was || user.bit_prefs_changed?
-      create_promotion_dmail
+  def build_messages
+    messages = []
+
+    if user.can_approve_posts?
+      messages << "You can approve posts."
     else
-      create_demotion_dmail
+      messages << "You cannot approve posts."
     end
+
+    if user.can_upload_free?
+      messages << "You can upload posts without limit."
+    else
+      messages << "You cannot upload posts without limit."
+    end
+
+    if user.level_changed?
+      if user.level > user.level_was
+        messages << "You have been promoted to a #{user.level_string} level account."
+      elsif user.level < user.level_was
+        messages << "You have been demoted to a #{user.level_string} level account."
+      end
+    end
+
+    messages.join(" ")
   end
 
-  def create_promotion_dmail
-    approval_text = if user.can_approve_posts?
-      "You can approve posts."
-    else
-      ""
-    end
-
-    upload_text = if user.can_upload_free?
-      "You can upload posts without limit."
-    else
-      ""
-    end
-
+  def create_dmail
     Dmail.create_split(
       :to_id => user.id,
       :title => "You have been promoted",
-      :body => "You have been promoted to a #{user.level_string} level account. #{approval_text} #{upload_text}"
+      :body => build_messages
     )
   end
 
-  def create_demotion_dmail
-    Dmail.create_split(
-      :to_id => user.id,
-      :title => "You have been demoted",
-      :body => "You have been demoted to a #{user.level_string} level account. #{approval_text} #{upload_text}"
+  def create_user_feedback
+    user.feedback.create(
+      :category => "neutral",
+      :body => build_messages,
+      :disable_dmail_notification => true
     )
   end
 end

test/functional/users_controller_test.rb

@@ -76,25 +76,11 @@ class UsersControllerTest < ActionController::TestCase
           @cuser = FactoryGirl.create(:user)
         end
 
-        should "not work if the current user is not an admin" do
+        should "not work" do
           post :update, {:id => @user.id, :user => {:level => 40}}, {:user_id => @cuser.id}
           @user.reload
           assert_equal(20, @user.level)
         end
-
-        context "where the current user is an admin" do
-          setup do
-            @admin = FactoryGirl.create(:admin_user)
-          end
-
-          should "create a user feedback" do
-            assert_difference("UserFeedback.count") do
-              post :update, {:id => @user.id, :user => {:level => 40}}, {:user_id => @admin.id}
-            end
-            @user.reload
-            assert_equal(40, @user.level)
-          end
-        end
       end
     end
   end