Commit 13cbc82aa164a6d2885546dc8fe1d0885700fe83
Exists in
spb-stable
and in
2 other branches
Merge branch 'master-create-group-projects' into 'master'
Master can create group projects It also includes Project transfer refactoring Fixes #1284
Showing
16 changed files
with
116 additions
and
200 deletions
Show diff stats
app/controllers/projects_controller.rb
| ... | ... | @@ -46,7 +46,7 @@ class ProjectsController < ApplicationController |
| 46 | 46 | end |
| 47 | 47 | |
| 48 | 48 | def transfer |
| 49 | - ::Projects::TransferService.new(project, current_user, params).execute | |
| 49 | + ::Projects::TransferService.new(project, current_user, params[:project]).execute | |
| 50 | 50 | end |
| 51 | 51 | |
| 52 | 52 | def show | ... | ... |
app/helpers/namespaces_helper.rb
| 1 | 1 | module NamespacesHelper |
| 2 | 2 | def namespaces_options(selected = :current_user, scope = :default) |
| 3 | - groups = current_user.owned_groups | |
| 3 | + groups = current_user.owned_groups + current_user.masters_groups | |
| 4 | 4 | users = [current_user.namespace] |
| 5 | 5 | |
| 6 | 6 | group_opts = ["Groups", groups.sort_by(&:human_name).map {|g| [g.human_name, g.id]} ] | ... | ... |
app/models/ability.rb
| ... | ... | @@ -188,6 +188,13 @@ class Ability |
| 188 | 188 | rules << :read_group |
| 189 | 189 | end |
| 190 | 190 | |
| 191 | + # Only group masters and group owners can create new projects in group | |
| 192 | + if group.has_master?(user) || group.has_owner?(user) || user.admin? | |
| 193 | + rules += [ | |
| 194 | + :create_projects, | |
| 195 | + ] | |
| 196 | + end | |
| 197 | + | |
| 191 | 198 | # Only group owner and administrators can manage group |
| 192 | 199 | if group.has_owner?(user) || user.admin? |
| 193 | 200 | rules += [ |
| ... | ... | @@ -205,6 +212,7 @@ class Ability |
| 205 | 212 | # Only namespace owner and administrators can manage it |
| 206 | 213 | if namespace.owner == user || user.admin? |
| 207 | 214 | rules += [ |
| 215 | + :create_projects, | |
| 208 | 216 | :manage_namespace |
| 209 | 217 | ] |
| 210 | 218 | end | ... | ... |
app/models/group.rb
| ... | ... | @@ -26,7 +26,7 @@ class Group < Namespace |
| 26 | 26 | validates :avatar, file_size: { maximum: 100.kilobytes.to_i } |
| 27 | 27 | |
| 28 | 28 | mount_uploader :avatar, AttachmentUploader |
| 29 | - | |
| 29 | + | |
| 30 | 30 | def self.accessible_to(user) |
| 31 | 31 | accessible_ids = Project.accessible_to(user).pluck(:namespace_id) |
| 32 | 32 | accessible_ids += user.groups.pluck(:id) if user |
| ... | ... | @@ -60,6 +60,10 @@ class Group < Namespace |
| 60 | 60 | owners.include?(user) |
| 61 | 61 | end |
| 62 | 62 | |
| 63 | + def has_master?(user) | |
| 64 | + members.masters.where(user_id: user).any? | |
| 65 | + end | |
| 66 | + | |
| 63 | 67 | def last_owner?(user) |
| 64 | 68 | has_owner?(user) && owners.size == 1 |
| 65 | 69 | end | ... | ... |
app/models/project.rb
| ... | ... | @@ -387,10 +387,6 @@ class Project < ActiveRecord::Base |
| 387 | 387 | end |
| 388 | 388 | end |
| 389 | 389 | |
| 390 | - def transfer(new_namespace) | |
| 391 | - ProjectTransferService.new.transfer(self, new_namespace) | |
| 392 | - end | |
| 393 | - | |
| 394 | 390 | def execute_hooks(data, hooks_scope = :push_hooks) |
| 395 | 391 | hooks.send(hooks_scope).each do |hook| |
| 396 | 392 | hook.async_execute(data) | ... | ... |
app/models/user.rb
| ... | ... | @@ -90,6 +90,8 @@ class User < ActiveRecord::Base |
| 90 | 90 | has_many :users_groups, dependent: :destroy |
| 91 | 91 | has_many :groups, through: :users_groups |
| 92 | 92 | has_many :owned_groups, -> { where users_groups: { group_access: UsersGroup::OWNER } }, through: :users_groups, source: :group |
| 93 | + has_many :masters_groups, -> { where users_groups: { group_access: UsersGroup::MASTER } }, through: :users_groups, source: :group | |
| 94 | + | |
| 93 | 95 | # Projects |
| 94 | 96 | has_many :groups_projects, through: :groups, source: :projects |
| 95 | 97 | has_many :personal_projects, through: :namespace, source: :projects | ... | ... |
app/services/project_transfer_service.rb
| ... | ... | @@ -1,46 +0,0 @@ |
| 1 | -# ProjectTransferService class | |
| 2 | -# | |
| 3 | -# Used for transfer project to another namespace | |
| 4 | -# | |
| 5 | -class ProjectTransferService | |
| 6 | - include Gitlab::ShellAdapter | |
| 7 | - | |
| 8 | - class TransferError < StandardError; end | |
| 9 | - | |
| 10 | - attr_accessor :project | |
| 11 | - | |
| 12 | - def transfer(project, new_namespace) | |
| 13 | - Project.transaction do | |
| 14 | - old_path = project.path_with_namespace | |
| 15 | - new_path = File.join(new_namespace.try(:path) || '', project.path) | |
| 16 | - | |
| 17 | - if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? | |
| 18 | - raise TransferError.new("Project with same path in target namespace already exists") | |
| 19 | - end | |
| 20 | - | |
| 21 | - # Remove old satellite | |
| 22 | - project.satellite.destroy | |
| 23 | - | |
| 24 | - # Apply new namespace id | |
| 25 | - project.namespace = new_namespace | |
| 26 | - project.save! | |
| 27 | - | |
| 28 | - # Move main repository | |
| 29 | - unless gitlab_shell.mv_repository(old_path, new_path) | |
| 30 | - raise TransferError.new('Cannot move project') | |
| 31 | - end | |
| 32 | - | |
| 33 | - # Move wiki repo also if present | |
| 34 | - gitlab_shell.mv_repository("#{old_path}.wiki", "#{new_path}.wiki") | |
| 35 | - | |
| 36 | - # Create a new satellite (reload project from DB) | |
| 37 | - Project.find(project.id).ensure_satellite_exists | |
| 38 | - | |
| 39 | - # clear project cached events | |
| 40 | - project.reset_events_cache | |
| 41 | - | |
| 42 | - true | |
| 43 | - end | |
| 44 | - end | |
| 45 | -end | |
| 46 | - |
app/services/projects/create_service.rb
| ... | ... | @@ -97,7 +97,7 @@ module Projects |
| 97 | 97 | |
| 98 | 98 | def allowed_namespace?(user, namespace_id) |
| 99 | 99 | namespace = Namespace.find_by(id: namespace_id) |
| 100 | - current_user.can?(:manage_namespace, namespace) | |
| 100 | + current_user.can?(:create_projects, namespace) | |
| 101 | 101 | end |
| 102 | 102 | end |
| 103 | 103 | end | ... | ... |
app/services/projects/transfer_service.rb
| 1 | +# Projects::TransferService class | |
| 2 | +# | |
| 3 | +# Used for transfer project to another namespace | |
| 4 | +# | |
| 5 | +# Ex. | |
| 6 | +# # Move projects to namespace with ID 17 by user | |
| 7 | +# Projects::TransferService.new(project, user, namespace_id: 17).execute | |
| 8 | +# | |
| 1 | 9 | module Projects |
| 2 | 10 | class TransferService < BaseService |
| 3 | - def execute(role = :default) | |
| 4 | - namespace_id = params[:project].delete(:namespace_id) | |
| 5 | - allowed_transfer = can?(current_user, :change_namespace, project) || role == :admin | |
| 6 | - | |
| 7 | - if allowed_transfer && namespace_id.present? | |
| 8 | - if namespace_id.to_i != project.namespace_id | |
| 9 | - # Transfer to someone namespace | |
| 10 | - namespace = Namespace.find(namespace_id) | |
| 11 | - project.transfer(namespace) | |
| 12 | - end | |
| 13 | - end | |
| 11 | + include Gitlab::ShellAdapter | |
| 12 | + class TransferError < StandardError; end | |
| 13 | + | |
| 14 | + def execute | |
| 15 | + namespace_id = params.delete(:namespace_id) | |
| 16 | + namespace = Namespace.find_by(id: namespace_id) | |
| 14 | 17 | |
| 15 | - rescue ProjectTransferService::TransferError => ex | |
| 18 | + if allowed_transfer?(current_user, project, namespace) | |
| 19 | + transfer(project, namespace) | |
| 20 | + else | |
| 21 | + project.errors.add(:namespace, 'is invalid') | |
| 22 | + false | |
| 23 | + end | |
| 24 | + rescue Projects::TransferService::TransferError => ex | |
| 16 | 25 | project.reload |
| 17 | 26 | project.errors.add(:namespace_id, ex.message) |
| 18 | 27 | false |
| 19 | 28 | end |
| 29 | + | |
| 30 | + def transfer(project, new_namespace) | |
| 31 | + Project.transaction do | |
| 32 | + old_path = project.path_with_namespace | |
| 33 | + new_path = File.join(new_namespace.try(:path) || '', project.path) | |
| 34 | + | |
| 35 | + if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? | |
| 36 | + raise TransferError.new("Project with same path in target namespace already exists") | |
| 37 | + end | |
| 38 | + | |
| 39 | + # Remove old satellite | |
| 40 | + project.satellite.destroy | |
| 41 | + | |
| 42 | + # Apply new namespace id | |
| 43 | + project.namespace = new_namespace | |
| 44 | + project.save! | |
| 45 | + | |
| 46 | + # Move main repository | |
| 47 | + unless gitlab_shell.mv_repository(old_path, new_path) | |
| 48 | + raise TransferError.new('Cannot move project') | |
| 49 | + end | |
| 50 | + | |
| 51 | + # Move wiki repo also if present | |
| 52 | + gitlab_shell.mv_repository("#{old_path}.wiki", "#{new_path}.wiki") | |
| 53 | + | |
| 54 | + # Create a new satellite (reload project from DB) | |
| 55 | + Project.find(project.id).ensure_satellite_exists | |
| 56 | + | |
| 57 | + # clear project cached events | |
| 58 | + project.reset_events_cache | |
| 59 | + | |
| 60 | + true | |
| 61 | + end | |
| 62 | + end | |
| 63 | + | |
| 64 | + def allowed_transfer?(current_user, project, namespace) | |
| 65 | + namespace && | |
| 66 | + can?(current_user, :change_namespace, project) && | |
| 67 | + namespace.id != project.namespace_id && | |
| 68 | + current_user.can?(:create_projects, namespace) | |
| 69 | + end | |
| 20 | 70 | end |
| 21 | 71 | end |
| 22 | - | ... | ... |
app/views/groups/_projects.html.haml
doc/permissions/permissions.md
| ... | ... | @@ -40,7 +40,7 @@ If a user is a GitLab administrator they receive all permissions. |
| 40 | 40 | |------|-----|--------|---------|------|-----| |
| 41 | 41 | |Browse group|✓|✓|✓|✓|✓| |
| 42 | 42 | |Edit group|||||✓| |
| 43 | -|Create project in group|||||✓| | |
| 43 | +|Create project in group||||✓|✓| | |
| 44 | 44 | |Manage group members|||||✓| |
| 45 | 45 | |Remove group|||||✓| |
| 46 | 46 | ... | ... |
lib/api/groups.rb
| ... | ... | @@ -87,10 +87,12 @@ module API |
| 87 | 87 | # POST /groups/:id/projects/:project_id |
| 88 | 88 | post ":id/projects/:project_id" do |
| 89 | 89 | authenticated_as_admin! |
| 90 | - @group = Group.find(params[:id]) | |
| 90 | + group = Group.find(params[:id]) | |
| 91 | 91 | project = Project.find(params[:project_id]) |
| 92 | - if project.transfer(@group) | |
| 93 | - present @group | |
| 92 | + result = ::Projects::TransferService.new(project, current_user, namespace_id: group.id).execute | |
| 93 | + | |
| 94 | + if result | |
| 95 | + present group | |
| 94 | 96 | else |
| 95 | 97 | not_found! |
| 96 | 98 | end | ... | ... |
lib/tasks/gitlab/enable_namespaces.rake
| ... | ... | @@ -1,111 +0,0 @@ |
| 1 | -namespace :gitlab do | |
| 2 | - desc "GITLAB | Enable usernames and namespaces for user projects" | |
| 3 | - task enable_namespaces: :environment do | |
| 4 | - warn_user_is_not_gitlab | |
| 5 | - | |
| 6 | - migrate_user_namespaces | |
| 7 | - migrate_groups | |
| 8 | - migrate_projects | |
| 9 | - end | |
| 10 | - | |
| 11 | - def migrate_user_namespaces | |
| 12 | - puts "\nGenerate usernames for users without one: ".blue | |
| 13 | - User.find_each(batch_size: 500) do |user| | |
| 14 | - if user.namespace | |
| 15 | - print '-'.cyan | |
| 16 | - next | |
| 17 | - end | |
| 18 | - | |
| 19 | - username = if user.username.present? | |
| 20 | - # if user already has username filled | |
| 21 | - user.username | |
| 22 | - else | |
| 23 | - build_username(user) | |
| 24 | - end | |
| 25 | - | |
| 26 | - begin | |
| 27 | - User.transaction do | |
| 28 | - user.update_attributes!(username: username) | |
| 29 | - print '.'.green | |
| 30 | - end | |
| 31 | - rescue | |
| 32 | - print 'F'.red | |
| 33 | - end | |
| 34 | - end | |
| 35 | - puts "\nDone" | |
| 36 | - end | |
| 37 | - | |
| 38 | - def build_username(user) | |
| 39 | - username = nil | |
| 40 | - | |
| 41 | - # generate username | |
| 42 | - username = user.email.match(/^[^@]*/)[0] | |
| 43 | - username.gsub!("+", ".") | |
| 44 | - | |
| 45 | - # return username if no matches | |
| 46 | - return username unless User.find_by(username: username) | |
| 47 | - | |
| 48 | - # look for same username | |
| 49 | - (1..10).each do |i| | |
| 50 | - suffixed_username = "#{username}#{i}" | |
| 51 | - | |
| 52 | - return suffixed_username unless User.find_by(username: suffixed_username) | |
| 53 | - end | |
| 54 | - end | |
| 55 | - | |
| 56 | - def migrate_groups | |
| 57 | - puts "\nCreate directories for groups: ".blue | |
| 58 | - | |
| 59 | - Group.find_each(batch_size: 500) do |group| | |
| 60 | - begin | |
| 61 | - if group.dir_exists? | |
| 62 | - print '-'.cyan | |
| 63 | - else | |
| 64 | - if group.ensure_dir_exist | |
| 65 | - print '.'.green | |
| 66 | - else | |
| 67 | - print 'F'.red | |
| 68 | - end | |
| 69 | - end | |
| 70 | - rescue | |
| 71 | - print 'F'.red | |
| 72 | - end | |
| 73 | - end | |
| 74 | - puts "\nDone" | |
| 75 | - end | |
| 76 | - | |
| 77 | - def migrate_projects | |
| 78 | - git_path = Gitlab.config.gitlab_shell.repos_path | |
| 79 | - puts "\nMove projects in groups into respective directories ... ".blue | |
| 80 | - Project.where('namespace_id IS NOT NULL').find_each(batch_size: 500) do |project| | |
| 81 | - next unless project.group | |
| 82 | - | |
| 83 | - group = project.group | |
| 84 | - | |
| 85 | - print "#{project.name_with_namespace.yellow} ... " | |
| 86 | - | |
| 87 | - new_path = File.join(git_path, project.path_with_namespace + '.git') | |
| 88 | - | |
| 89 | - if File.exists?(new_path) | |
| 90 | - puts "already at #{new_path}".green | |
| 91 | - next | |
| 92 | - end | |
| 93 | - | |
| 94 | - old_path = File.join(git_path, project.path + '.git') | |
| 95 | - | |
| 96 | - unless File.exists?(old_path) | |
| 97 | - puts "couldn't find it at #{old_path}".red | |
| 98 | - next | |
| 99 | - end | |
| 100 | - | |
| 101 | - begin | |
| 102 | - project.transfer(group.path) | |
| 103 | - puts "moved to #{new_path}".green | |
| 104 | - rescue | |
| 105 | - puts "failed moving to #{new_path}".red | |
| 106 | - end | |
| 107 | - end | |
| 108 | - | |
| 109 | - puts "\nDone" | |
| 110 | - end | |
| 111 | -end |
spec/models/project_spec.rb
| ... | ... | @@ -84,7 +84,6 @@ describe Project do |
| 84 | 84 | it { should respond_to(:satellite) } |
| 85 | 85 | it { should respond_to(:update_merge_requests) } |
| 86 | 86 | it { should respond_to(:execute_hooks) } |
| 87 | - it { should respond_to(:transfer) } | |
| 88 | 87 | it { should respond_to(:name_with_namespace) } |
| 89 | 88 | it { should respond_to(:owner) } |
| 90 | 89 | it { should respond_to(:path_with_namespace) } | ... | ... |
spec/requests/api/groups_spec.rb
| ... | ... | @@ -147,7 +147,7 @@ describe API::API, api: true do |
| 147 | 147 | describe "POST /groups/:id/projects/:project_id" do |
| 148 | 148 | let(:project) { create(:project) } |
| 149 | 149 | before(:each) do |
| 150 | - project.stub(:transfer).and_return(true) | |
| 150 | + Projects::TransferService.any_instance.stub(execute: true) | |
| 151 | 151 | Project.stub(:find).and_return(project) |
| 152 | 152 | end |
| 153 | 153 | |
| ... | ... | @@ -160,8 +160,8 @@ describe API::API, api: true do |
| 160 | 160 | |
| 161 | 161 | context "when authenticated as admin" do |
| 162 | 162 | it "should transfer project to group" do |
| 163 | - project.should_receive(:transfer) | |
| 164 | 163 | post api("/groups/#{group1.id}/projects/#{project.id}", admin) |
| 164 | + response.status.should == 201 | |
| 165 | 165 | end |
| 166 | 166 | end |
| 167 | 167 | end | ... | ... |
spec/services/projects/transfer_service_spec.rb
| 1 | 1 | require 'spec_helper' |
| 2 | 2 | |
| 3 | -describe ProjectTransferService do | |
| 3 | +describe Projects::TransferService do | |
| 4 | 4 | before(:each) { enable_observers } |
| 5 | 5 | after(:each) {disable_observers} |
| 6 | 6 | |
| 7 | - context 'namespace -> namespace' do | |
| 8 | - let(:user) { create(:user) } | |
| 9 | - let(:group) { create(:group) } | |
| 10 | - let(:project) { create(:project, namespace: user.namespace) } | |
| 7 | + let(:user) { create(:user) } | |
| 8 | + let(:group) { create(:group) } | |
| 9 | + let(:group2) { create(:group) } | |
| 10 | + let(:project) { create(:project, namespace: user.namespace) } | |
| 11 | 11 | |
| 12 | + context 'namespace -> namespace' do | |
| 12 | 13 | before do |
| 13 | - @result = service.transfer(project, group) | |
| 14 | + group.add_owner(user) | |
| 15 | + @service = Projects::TransferService.new(project, user, namespace_id: group.id) | |
| 16 | + @service.gitlab_shell.stub(mv_repository: true) | |
| 17 | + @result = @service.execute | |
| 14 | 18 | end |
| 15 | 19 | |
| 16 | 20 | it { @result.should be_true } |
| ... | ... | @@ -18,16 +22,25 @@ describe ProjectTransferService do |
| 18 | 22 | end |
| 19 | 23 | |
| 20 | 24 | context 'namespace -> no namespace' do |
| 21 | - let(:user) { create(:user) } | |
| 22 | - let(:project) { create(:project, namespace: user.namespace) } | |
| 25 | + before do | |
| 26 | + group.add_owner(user) | |
| 27 | + @service = Projects::TransferService.new(project, user, namespace_id: nil) | |
| 28 | + @service.gitlab_shell.stub(mv_repository: true) | |
| 29 | + @result = @service.execute | |
| 30 | + end | |
| 23 | 31 | |
| 24 | - it { lambda{service.transfer(project, nil)}.should raise_error(ActiveRecord::RecordInvalid) } | |
| 32 | + it { @result.should be_false } | |
| 33 | + it { project.namespace.should == user.namespace } | |
| 25 | 34 | end |
| 26 | 35 | |
| 27 | - def service | |
| 28 | - service = ProjectTransferService.new | |
| 29 | - service.gitlab_shell.stub(mv_repository: true) | |
| 30 | - service | |
| 36 | + context 'namespace -> not allowed namespace' do | |
| 37 | + before do | |
| 38 | + @service = Projects::TransferService.new(project, user, namespace_id: group2.id) | |
| 39 | + @service.gitlab_shell.stub(mv_repository: true) | |
| 40 | + @result = @service.execute | |
| 41 | + end | |
| 42 | + | |
| 43 | + it { @result.should be_false } | |
| 44 | + it { project.namespace.should == user.namespace } | |
| 31 | 45 | end |
| 32 | 46 | end |
| 33 | - | ... | ... |