#638: some refactoring

app/controllers/platforms/platforms_controller.rb

@@ -86,17 +86,12 @@ class Platforms::PlatformsController < Platforms::BaseController
   end
 
   def remove_members
-    all_user_ids = params['user_remove'].inject([]) {|a, (k, v)| a << k if v.first == '1'; a}
+    Relation.remove_members(params[:user_remove], @platform)
-    all_user_ids.each do |uid|
-      Relation.by_target(@platform).where(:actor_id => uid, :actor_type => 'User').each{|r| r.destroy}
-    end
     redirect_to members_platform_path(@platform)
   end
 
   def remove_member
-    u = User.find(params[:member_id])
+    Relation.remove_member(params[:member_id], @platform)
-    Relation.by_actor(u).by_target(@platform).each{|r| r.destroy}
-
     redirect_to members_platform_path(@platform)
   end
 

app/controllers/platforms/repositories_controller.rb

@@ -34,17 +34,12 @@ class Platforms::RepositoriesController < Platforms::BaseController
   end
 
   def remove_members
-    all_user_ids = params['user_remove'].inject([]) {|a, (k, v)| a << k if v.first == '1'; a}
+    Relation.remove_members(params[:user_remove], @repository)
-    all_user_ids.each do |uid|
-      Relation.by_target(@repository).where(:actor_id => uid, :actor_type => 'User').each{|r| r.destroy}
-    end
     redirect_to edit_platform_repository_path(@platform, @repository)
   end
 
   def remove_member
-    u = User.find(params[:member_id])
+    Relation.remove_member(params[:member_id], @repository)
-    Relation.by_actor(u).by_target(@repository).each{|r| r.destroy}
-
     redirect_to edit_platform_repository_path(@platform, @repository)
   end
 

app/models/relation.rb

@@ -10,9 +10,9 @@ class Relation < ActiveRecord::Base
   before_validation :add_default_role
 
   scope :by_user_through_groups, lambda {|u| where("actor_type = 'User' AND actor_id = ? OR actor_type = 'Group' AND actor_id IN (?)", u.id, u.group_ids)}
-  scope :by_actor, lambda {|obj| {:conditions => ['actor_id = ? AND actor_type = ?', obj.id, obj.class.to_s]}}
+  scope :by_actor, lambda {|obj| where(:actor_id => obj.id, :actor_type => obj.class.to_s)}
-  scope :by_target, lambda {|tar| {:conditions => ['target_id = ? AND target_type = ?', tar.id, tar.class.to_s]}}
+  scope :by_target, lambda {|tar| where(:target_id => tar.id, :target_type => tar.class.to_s)}
-  scope :by_role, lambda {|role| {:conditions => ['role = ?', role]}}
+  scope :by_role, lambda {|role| where(:role => role)}
 
   def self.create_with_role(actor, target, role)
     r = self.new
@@ -22,6 +22,18 @@ class Relation < ActiveRecord::Base
     r.save
   end
 
+  def self.remove_member(member_id, target)
+    user = User.find(member_id)
+    Relation.by_actor(user).by_target(target).each{|r| r.destroy}
+  end
+
+  # @param user_remove looks like {"9"=>["1"], "32"=>["1"]}
+  def self.remove_members(user_remove, target)
+    user_ids = user_remove ? user_remove.map{ |k, v| k if v.first == '1' }.compact : []
+    Relation.by_target(target).where(:actor_id => user_ids, :actor_type => 'User').
+      each{|r| r.destroy}
+  end
+
   protected
 
   def add_default_role