Commit b7a9e41bd269e2b5519928c35592aad96d3707c6
1 parent
fd0aca12
Exists in
master
and in
4 other branches
Handle perfomance issue with team import. Model specs
Showing
2 changed files
with
57 additions
and
11 deletions
Show diff stats
app/models/users_project.rb
| @@ -22,19 +22,32 @@ class UsersProject < ActiveRecord::Base | @@ -22,19 +22,32 @@ class UsersProject < ActiveRecord::Base | ||
| 22 | 22 | ||
| 23 | class << self | 23 | class << self |
| 24 | def import_team(source_project, target_project) | 24 | def import_team(source_project, target_project) |
| 25 | - UsersProject.transaction do | ||
| 26 | - team = source_project.users_projects.all | ||
| 27 | - | ||
| 28 | - team.each do |tm| | ||
| 29 | - # Skip if user already present in team | ||
| 30 | - next if target_project.users.include?(tm.user) | ||
| 31 | - | ||
| 32 | - new_tm = tm.dup | ||
| 33 | - new_tm.id = nil | ||
| 34 | - new_tm.project_id = target_project.id | ||
| 35 | - new_tm.save | 25 | + UsersProject.without_repository_callback do |
| 26 | + UsersProject.transaction do | ||
| 27 | + team = source_project.users_projects.all | ||
| 28 | + | ||
| 29 | + team.each do |tm| | ||
| 30 | + # Skip if user already present in team | ||
| 31 | + next if target_project.users.include?(tm.user) | ||
| 32 | + | ||
| 33 | + new_tm = tm.dup | ||
| 34 | + new_tm.id = nil | ||
| 35 | + new_tm.project_id = target_project.id | ||
| 36 | + new_tm.save | ||
| 37 | + end | ||
| 36 | end | 38 | end |
| 37 | end | 39 | end |
| 40 | + | ||
| 41 | + target_project.update_repository | ||
| 42 | + true | ||
| 43 | + rescue | ||
| 44 | + false | ||
| 45 | + end | ||
| 46 | + | ||
| 47 | + def without_repository_callback | ||
| 48 | + UsersProject.skip_callback(:destroy, :after, :update_repository) | ||
| 49 | + yield | ||
| 50 | + UsersProject.set_callback(:destroy, :after, :update_repository) | ||
| 38 | end | 51 | end |
| 39 | 52 | ||
| 40 | def bulk_delete(project, user_ids) | 53 | def bulk_delete(project, user_ids) |
spec/models/users_project_spec.rb
| @@ -35,4 +35,37 @@ describe UsersProject do | @@ -35,4 +35,37 @@ describe UsersProject do | ||
| 35 | it { should respond_to(:user_name) } | 35 | it { should respond_to(:user_name) } |
| 36 | it { should respond_to(:user_email) } | 36 | it { should respond_to(:user_email) } |
| 37 | end | 37 | end |
| 38 | + | ||
| 39 | + describe :import_team do | ||
| 40 | + before do | ||
| 41 | + @abilities = Six.new | ||
| 42 | + @abilities << Ability | ||
| 43 | + | ||
| 44 | + @project_1 = create :project | ||
| 45 | + @project_2 = create :project | ||
| 46 | + | ||
| 47 | + @user_1 = create :user | ||
| 48 | + @user_2 = create :user | ||
| 49 | + | ||
| 50 | + @project_1.add_access @user_1, :write | ||
| 51 | + @project_2.add_access @user_2, :read | ||
| 52 | + | ||
| 53 | + @status = UsersProject.import_team(@project_1, @project_2) | ||
| 54 | + end | ||
| 55 | + | ||
| 56 | + it { @status.should be_true } | ||
| 57 | + | ||
| 58 | + describe 'project 2 should get user 1 as developer. user_2 should not be changed' do | ||
| 59 | + it { @project_2.users.should include(@user_1) } | ||
| 60 | + it { @project_2.users.should include(@user_2) } | ||
| 61 | + | ||
| 62 | + it { @abilities.allowed?(@user_1, :write_project, @project_2).should be_true } | ||
| 63 | + it { @abilities.allowed?(@user_2, :read_project, @project_2).should be_true } | ||
| 64 | + end | ||
| 65 | + | ||
| 66 | + describe 'project 1 should not be changed' do | ||
| 67 | + it { @project_1.users.should include(@user_1) } | ||
| 68 | + it { @project_1.users.should_not include(@user_2) } | ||
| 69 | + end | ||
| 70 | + end | ||
| 38 | end | 71 | end |