attr_protect Committership#permisssions from mass attribute updates

Also ad a Committership::create_with_permission to easy expressing
things, mostly for usable for test and utility code NOT for controller
code.

4 files changed, 33 insertions, 43 deletions

diff --git a/test/functional/committerships_controller_test.rb b/test/functional/committerships_controller_test.rb
index 7a0bfa6..621033e 100644
--- a/test/functional/committerships_controller_test.rb
+++ b/test/functional/committerships_controller_test.rb
@@ -105,10 +105,9 @@ class CommittershipsControllerTest < ActionController::TestCase
 
     should "scope to the correct repository" do
       repo = repositories(:johans2)
-      repo.committerships.create!({
-          :committer => users(:mike),
-          :permissions => Committership::CAN_ADMIN
-        })
+      repo.committerships.create_with_permissions!({
+          :committer => users(:mike)
+        }, Committership::CAN_ADMIN)
       login_as :mike
       get :new, :group_id => repo.owner.to_param,
         :project_id => repo.project.to_param,
diff --git a/test/functional/repositories_controller_test.rb b/test/functional/repositories_controller_test.rb
index c93ce27..3e4b491 100644
--- a/test/functional/repositories_controller_test.rb
+++ b/test/functional/repositories_controller_test.rb
@@ -853,10 +853,9 @@ class RepositoriesControllerTest < ActionController::TestCase
       repo = repositories(:johans2)
       repo.user = users(:johan)
       repo.save!
-      repo.committerships.create!({
-          :committer => users(:johan),
-          :permissions => (Committership::CAN_ADMIN | Committership::CAN_COMMIT)
-        })
+      repo.committerships.create_with_permissions!({
+          :committer => users(:johan)
+        }, (Committership::CAN_ADMIN | Committership::CAN_COMMIT))
       assert repo.reload.admin?(users(:johan))
       delete :destroy, :project_id => repo.project.to_param,
         :group_id => repo.owner.to_param, :id => repo.to_param
@@ -877,10 +876,9 @@ class RepositoriesControllerTest < ActionController::TestCase
     should "work for user/group clones" do
       repo = repositories(:johans2)
       repo.user = users(:mike)
-      repo.committerships.create!({
-          :committer => users(:mike),
-          :permissions => (Committership::CAN_ADMIN | Committership::CAN_COMMIT)
-        })
+      repo.committerships.create_with_permissions!({
+          :committer => users(:mike)
+        }, (Committership::CAN_ADMIN | Committership::CAN_COMMIT))
       repo.save!
       login_as :mike
       get :confirm_delete, :group_id => repo.owner.to_param,
@@ -1025,10 +1023,9 @@ class RepositoriesControllerTest < ActionController::TestCase
       login_as :moe
       @repository.owner = users(:moe)
       @repository.kind = Repository::KIND_USER_REPO
-      @repository.committerships.create!({
-          :committer => users(:moe),
-          :permissions => Committership::CAN_ADMIN
-        })
+      @repository.committerships.create_with_permissions!({
+          :committer => users(:moe)
+        }, Committership::CAN_ADMIN)
       @repository.save!
       get :edit, :project_id => @repository.project.to_param,
         :user_id => users(:moe).to_param, :id => @repository.to_param
@@ -1037,10 +1034,9 @@ class RepositoriesControllerTest < ActionController::TestCase
 
     should "requires adminship on the repo" do
       login_as :mike
-      @repository.committerships.create!({
-          :committer => groups(:team_thunderbird),
-          :permissions => Committership::CAN_ADMIN
-        })
+      @repository.committerships.create_with_permissions!({
+          :committer => groups(:team_thunderbird)
+        }, Committership::CAN_ADMIN)
       @repository.kind = Repository::KIND_TEAM_REPO
       @repository.owner = groups(:team_thunderbird)
       @repository.save!
diff --git a/test/unit/merge_request_test.rb b/test/unit/merge_request_test.rb
index 5d5d0c2..f037c7b 100644
--- a/test/unit/merge_request_test.rb
+++ b/test/unit/merge_request_test.rb
@@ -37,10 +37,10 @@ class MergeRequestTest < ActiveSupport::TestCase
 
   should_validate_presence_of :user, :source_repository, :target_repository
   should_validate_presence_of :summary, :sequence_number
-  
+
   should_have_many :comments
   should_not_allow_mass_assignment_of :sequence_number
-  
+
   should "email all committers in the target_repository after the terms are accepted" do
     assert_incremented_by(Message, :count, 1) do
       mr = @merge_request.clone
@@ -236,10 +236,9 @@ class MergeRequestTest < ActiveSupport::TestCase
 
   should "knows who can resolve itself" do
     assert @merge_request.resolvable_by?(users(:johan))
-    @merge_request.target_repository.committerships.create!({
-        :committer => groups(:team_thunderbird),
-        :permissions => Committership::CAN_REVIEW
-      })
+    @merge_request.target_repository.committerships.create_with_permissions!({
+        :committer => groups(:team_thunderbird)
+      }, Committership::CAN_REVIEW)
     assert @merge_request.resolvable_by?(users(:mike))
     assert !@merge_request.resolvable_by?(users(:moe))
   end
@@ -794,5 +793,5 @@ class MergeRequestTest < ActiveSupport::TestCase
       assert_equal @merge_request.sequence_number.to_s, @merge_request.to_param
     end
   end
-  
+
 end
diff --git a/test/unit/repository_test.rb b/test/unit/repository_test.rb
index 7fa7c4b..4bf55ca 100644
--- a/test/unit/repository_test.rb
+++ b/test/unit/repository_test.rb
@@ -478,10 +478,9 @@ class RepositoryTest < ActiveSupport::TestCase
 
     should "also be deletable by users with admin privs" do
       @repository.kind = Repository::KIND_TEAM_REPO
-      cs = @repository.committerships.create!({
-          :committer => users(:mike),
-          :permissions => (Committership::CAN_ADMIN)
-        })
+      cs = @repository.committerships.create_with_permissions!({
+          :committer => users(:mike)
+        }, Committership::CAN_ADMIN)
       assert @repository.can_be_deleted_by?(users(:johan))
       assert @repository.can_be_deleted_by?(users(:mike))
     end
@@ -585,16 +584,14 @@ class RepositoryTest < ActiveSupport::TestCase
     repo.reload
     assert !repo.committers.include?(users(:moe))
 
-    repo.committerships.create!({
-        :committer => users(:johan),
-        :permissions => Committership::CAN_COMMIT
-      })
+    repo.committerships.create_with_permissions!({
+        :committer => users(:johan)
+      }, Committership::CAN_COMMIT)
     assert_equal [users(:johan).login], repo.committers.map(&:login)
 
-    repo.committerships.create!({
-        :committer => groups(:team_thunderbird),
-        :permissions => Committership::CAN_COMMIT
-      })
+    repo.committerships.create_with_permissions!({
+        :committer => groups(:team_thunderbird)
+      }, Committership::CAN_COMMIT)
     exp_users = groups(:team_thunderbird).members.unshift(users(:johan))
     assert_equal exp_users.map(&:login), repo.committers.map(&:login)
 
@@ -726,10 +723,9 @@ class RepositoryTest < ActiveSupport::TestCase
 
     should "return a list of reviewers" do
       assert !@repo.reviewers.map(&:login).include?(users(:moe).login)
-      @repo.committerships.create!({
-          :committer => users(:moe),
-          :permissions => Committership::CAN_REVIEW
-        })
+      @repo.committerships.create_with_permissions!({
+          :committer => users(:moe)
+        }, Committership::CAN_REVIEW)
       assert @repo.reviewers.map(&:login).include?(users(:moe).login)
     end