refactored user promotion code, related to #1257

app/controllers/admin/users_controller.rb

@@ -9,21 +9,8 @@ module Admin
 
     def update
       @user = User.find(params[:id])
-      sanitize_params!
       @user.promote_to!(params[:user][:level])
       redirect_to edit_admin_user_path(@user), :notice => "User updated"
     end
-
-  protected
-    def sanitize_params!
-      # admins can do anything
-      return if CurrentUser.is_admin?
-
-      # can't promote/demote moderators
-      raise User::PrivilegeError if @user.is_moderator?
-
-      # can't promote to admin      
-      raise User::PrivilegeError if params[:user] && params[:user][:level].to_i >= User::Levels::ADMIN
-    end
   end
 end

app/logical/user_promotion.rb

@@ -0,0 +1,52 @@
+class UserPromotion
+  attr_reader :user, :promoter, :new_level
+
+  def initialize(user, promoter, new_level)
+    @user = user
+    @promoter = promoter
+    @new_level = new_level
+  end
+
+  def promote!
+    user.level = new_level
+    user.inviter_id = promoter.id
+
+    validate
+    create_transaction_log_item
+    create_user_feedback
+
+    user.save
+  end
+
+private
+  
+  def validate
+    # admins can do anything
+    return if promoter.is_admin?
+
+    # can't promote/demote moderators
+    raise User::PrivilegeError if user.is_moderator?
+
+    # can't promote to admin      
+    raise User::PrivilegeError if new_level.to_i >= User::Levels::ADMIN
+  end
+
+  def create_transaction_log_item
+    TransactionLogItem.record_account_upgrade(user)
+  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} by #{promoter.name} from #{user.level_string_was} to #{user.level_string}"
+    )
+  end
+end

app/models/user.rb

@@ -243,22 +243,7 @@ class User < ActiveRecord::Base
     end
 
     def promote_to!(new_level)
-      self.level = new_level
+      UserPromotion.new(self, CurrentUser.user, new_level).promote!
-      self.inviter_id = CurrentUser.user.id
-      TransactionLogItem.record_account_upgrade(self)
-      if level > level_was
-        body_prefix = "Promoted"
-      elsif level < level_was
-        body_prefix = "Demoted"
-      else
-        body_prefix = ""
-      end
-
-      feedback.create(
-        :category => "neutral",
-        :body => "#{body_prefix} by #{inviter.name} from #{level_string(level_was)} to #{level_string(level)}"
-      )
-      save
     end
 
     def promote_to_admin_if_first_user
@@ -296,6 +281,10 @@ class User < ActiveRecord::Base
       end
     end
 
+    def level_string_was
+      level_string(level_was)
+    end
+
     def level_string(value = nil)
       case (value || level)
       when Levels::BLOCKED
@@ -372,7 +361,7 @@ class User < ActiveRecord::Base
 
     def create_mod_action
       if level_changed?
-        ModAction.create(:description => %{"#{name}":/users/#{id} level changed #{level_string(level_was)} -> #{level_string}})
+        ModAction.create(:description => %{"#{name}":/users/#{id} level changed #{level_string_was} -> #{level_string}})
       end
     end