Merge branch 'improve/project_transfer'

app/contexts/projects/transfer_context.rb

+module Projects
+  class TransferContext < BaseContext
+    def execute(role = :default)
+      namespace_id = params[:project].delete(:namespace_id)
+      allowed_transfer = can?(current_user, :change_namespace, project) || role == :admin
+
+      if allowed_transfer && namespace_id.present?
+        if namespace_id == Namespace.global_id
+          if project.namespace.present?
+            # Transfer to global namespace from anyone
+            project.transfer(nil)
+          end
+        elsif namespace_id.to_i != project.namespace_id
+          # Transfer to someone namespace
+          namespace = Namespace.find(namespace_id)
+          project.transfer(namespace)
+        end
+      end
+
+    rescue ProjectTransferService::TransferError => ex
+      project.reload
+      project.errors.add(:namespace_id, ex.message)
+      false
+    end
+  end
+end
+

app/contexts/projects/update_context.rb

 module Projects
   class UpdateContext < BaseContext
     def execute(role = :default)
-      namespace_id = params[:project].delete(:namespace_id)
+      params[:project].delete(:namespace_id)
       params[:project].delete(:public) unless can?(current_user, :change_public_mode, project)
-
-      allowed_transfer = can?(current_user, :change_namespace, project) || role == :admin
-
-      if allowed_transfer && namespace_id.present?
-        if namespace_id == Namespace.global_id
-          if project.namespace.present?
-            # Transfer to global namespace from anyone
-            project.transfer(nil)
-          end
-        elsif namespace_id.to_i != project.namespace_id
-          # Transfer to someone namespace
-          namespace = Namespace.find(namespace_id)
-          project.transfer(namespace)
-        end
-      end
-
       project.update_attributes(params[:project], as: role)
     end
   end

app/controllers/admin/projects_controller.rb

@@ -19,34 +19,6 @@ class Admin::ProjectsController < Admin::ApplicationController
     @users = @users.all
   end
-  def edit
-  end
-
-  def team_update
-    @project.team.add_users_ids(params[:user_ids], params[:project_access])
-
-    redirect_to [:admin, @project], notice: 'Project was successfully updated.'
-  end
-
-  def update
-    project.creator = current_user unless project.creator
-
-    status = ::Projects::UpdateContext.new(project, current_user, params).execute(:admin)
-
-    if status
-      redirect_to [:admin, @project], notice: 'Project was successfully updated.'
-    else
-      render action: "edit"
-    end
-  end
-
-  def destroy
-    @project.team.truncate
-    @project.destroy
-
-    redirect_to admin_projects_path, notice: 'Project was successfully deleted.'
-  end
-
   protected
   def project

app/controllers/projects_controller.rb

@@ -4,7 +4,7 @@ class ProjectsController < ProjectResourceController
   # Authorize
   before_filter :authorize_read_project!, except: [:index, :new, :create]
-  before_filter :authorize_admin_project!, only: [:edit, :update, :destroy]
+  before_filter :authorize_admin_project!, only: [:edit, :update, :destroy, :transfer]
   before_filter :require_non_empty_project, only: [:blob, :tree, :graph]
   layout 'application', only: [:new, :create]
@@ -45,10 +45,10 @@ class ProjectsController < ProjectResourceController
         format.js
       end
     end
-  rescue Project::TransferError => ex
-    @error = ex
-    render :update_failed
+  end
+  def transfer
+    ::Projects::TransferContext.new(project, current_user, params).execute
   end
   def show

app/models/ability.rb

@@ -41,7 +41,7 @@ class Ability
         rules << project_guest_rules
       end
-      if project.owner == user
+      if project.owner == user || user.admin?
         rules << project_admin_rules
       end

app/models/project.rb

@@ -26,8 +26,6 @@ class Project < ActiveRecord::Base
   include Gitlab::ShellAdapter
   extend Enumerize
-  class TransferError < StandardError; end
-
   attr_accessible :name, :path, :description, :default_branch, :issues_tracker,
     :issues_enabled, :wall_enabled, :merge_requests_enabled, :snippets_enabled, :issues_tracker_id,
     :wiki_enabled, :public, :import_url, as: [:default, :admin]

app/services/project_transfer_service.rb

@@ -5,6 +5,8 @@
 class ProjectTransferService
   include Gitlab::ShellAdapter
+  class TransferError < StandardError; end
+
   attr_accessor :project
   def transfer(project, new_namespace)
@@ -19,14 +21,18 @@ class ProjectTransferService
       project.namespace = new_namespace
       project.save!
+      # Move main repository
       unless gitlab_shell.mv_repository(old_path, new_path)
         raise TransferError.new('Cannot move project')
       end
+      # Move wiki repo also if present
+      if project.wikis.any?
+        gitlab_shell.mv_repository("#{old_path}.wiki", "#{new_path}.wiki")
+      end
+
       true
     end
-  rescue => ex
-    raise Project::TransferError.new(ex.message)
   end
 end

app/views/admin/projects/_form.html.haml

-= form_for [:admin, project] do |f|
-  -if project.errors.any?
-    .alert.alert-error
-      %ul
-        - project.errors.full_messages.each do |msg|
-          %li= msg
-
-  .clearfix.project_name_holder
-    = f.label :name do
-      Project name is
-    .input
-      = f.text_field :name, placeholder: "Example Project", class: "xxlarge"
-
-  - if project.repo_exists?
-    %fieldset.adv_settings
-      %legend Advanced settings:
-      .clearfix
-        = f.label :path do
-          Path
-        .input
-          = text_field_tag :ppath, @project.repository.path_to_repo, class: "xlarge", disabled: true
-
-      .clearfix
-        = f.label :default_branch, "Default Branch"
-        .input= f.select(:default_branch, @project.repository.heads.map(&:name), {}, style: "width:210px;")
-
-  %fieldset.adv_settings
-    %legend Features:
-
-    .clearfix
-      = f.label :issues_enabled, "Issues"
-      .input= f.check_box :issues_enabled
-
-    - if Project.issues_tracker.values.count > 1
-      .clearfix
-        = f.label :issues_tracker, "Issues tracker", class: 'control-label'
-        .input= f.select(:issues_tracker, Project.issues_tracker.values, {}, { disabled: !@project.issues_enabled })
-
-      .clearfix
-        = f.label :issues_tracker_id, "Project name or id in issues tracker", class: 'control-label'
-        .input= f.text_field :issues_tracker_id, class: "xxlarge", disabled: !@project.can_have_issues_tracker_id?
-
-    .clearfix
-      = f.label :merge_requests_enabled, "Merge Requests"
-      .input= f.check_box :merge_requests_enabled
-
-    .clearfix
-      = f.label :wall_enabled, "Wall"
-      .input= f.check_box :wall_enabled
-
-    .clearfix
-      = f.label :wiki_enabled, "Wiki"
-      .input= f.check_box :wiki_enabled
-
-  %fieldset.features
-    %legend Public mode:
-    .clearfix
-      = f.label :public do
-        %span Allow public http clone
-      .input= f.check_box :public
-
-  %fieldset.features
-    %legend Transfer:
-    .control-group
-      = f.label :namespace_id do
-        %span Namespace
-      .controls
-        = f.select :namespace_id, namespaces_options(@project.namespace_id, :all), {}, {class: 'chosen'}
-        %br
-        %ul.prepend-top-10.cred
-          %li Be careful. Changing project namespace can have unintended side effects
-          %li You can transfer project only to namespaces you can manage
-          %li You will need to update your local repositories to point to the new location.
-
-
-  .actions
-    = f.submit 'Save Project', class: "btn btn-save"
-    = link_to 'Cancel', admin_projects_path, class: "btn btn-cancel"
-
-
-
-:javascript
-  $(function(){
-    new Projects();
-  })
-

app/views/admin/projects/edit.html.haml

-%h3.page_title #{@project.name} → Edit project
-%hr
-= render 'form', project: @project

app/views/admin/projects/index.html.haml

@@ -52,8 +52,8 @@
               %i.icon-lock.cgreen
             = link_to project.name_with_namespace, [:admin, project]
             .pull-right
-              = link_to 'Edit', edit_admin_project_path(project), id: "edit_#{dom_id(project)}", class: "btn btn-small"
-              = link_to 'Destroy', [:admin, project], confirm: "REMOVE #{project.name}? Are you sure?", method: :delete, class: "btn btn-small btn-remove"
+              = link_to 'Edit', edit_project_path(project), id: "edit_#{dom_id(project)}", class: "btn btn-small"
+              = link_to 'Destroy', [project], confirm: "REMOVE #{project.name}? Are you sure?", method: :delete, class: "btn btn-small btn-remove"
         - if @projects.blank?
           %p.nothing_here_message 0 projects matches
         - else

app/views/admin/projects/show.html.haml

 %h3.page_title
   Project: #{@project.name_with_namespace}
-  = link_to edit_admin_project_path(@project), class: "btn pull-right" do
+  = link_to edit_project_path(@project), class: "btn pull-right" do
     %i.icon-edit
     Edit
-%br
-%table.zebra-striped
-  %thead
-    %tr
-      %th Project
-      %th
-  %tr
-    %td
-      %b
-        Name:
-    %td
-      = @project.name
-  %tr
-    %td
-      %b
-        Namespace:
-    %td
-      - if @project.namespace
-        = @project.namespace.human_name
-      - else
-        Global
-  %tr
-    %td
-      %b
-        Owned by:
-    %td
-      - if @project.owner
-        = link_to @project.owner_name, admin_user_path(@project.owner)
-      - else
-        (deleted)
-  %tr
-    %td
-      %b
-        Created by:
-    %td
-      = @project.creator.try(:name) || '(deleted)'
-  %tr
-    %td
-      %b
-        Created at:
-    %td
-      = @project.created_at.stamp("March 1, 1999")
-    %tr
-      %td
-        %b
-          Smart HTTP:
-      %td
-        = link_to @project.http_url_to_repo
-    %tr
-      %td
-        %b
-          SSH:
-      %td
-        = link_to @project.ssh_url_to_repo
-  - if @project.public
-    %tr.bgred
-      %td
-        %b
-          Public Read-Only Code access:
-      %td
-        = check_box_tag 'public', nil, @project.public
-- if @repository
-  %table.zebra-striped
-    %thead
-      %tr
-        %th Repository
-        %th
-    %tr
-      %td
-        %b
-          FS Path:
-      %td
-        %code= @repository.path_to_repo
-    %tr
-      %td
-        %b
-          Last commit at:
-      %td
-        = last_commit(@project)
-%br
-%h5
-  Team
-  %small
-    (#{@project.users.count})
-%br
-%table.zebra-striped.team_members
-  %thead
-    %tr
-      %th Name
-      %th Project Access
-      %th Repository Access
-      %th
-  - @project.users.each do |tm|
-    %tr
-      %td
-        = link_to tm.name, admin_user_path(tm)
-      %td= @project.project_access_human(tm)
-      %td= link_to 'Edit Access', edit_admin_project_member_path(@project, tm), class: "btn btn-small"
-      %td= link_to 'Remove from team', admin_project_member_path(@project, tm), confirm: 'Are you sure?', method: :delete, class: "btn btn-remove small"
-
-%br
-%h5 Add new team member
-%br
-= form_tag team_update_admin_project_path(@project), class: "bulk_import", method: :put  do
-  %table.zebra-striped
-    %thead
-      %tr
-        %th Users
-        %th Project Access:
-
-    %tr
-      %td= select_tag :user_ids, options_from_collection_for_select(@users , :id, :name), multiple: true, data: {placeholder: 'Select users'}, class: 'chosen span5'
-      %td= select_tag :project_access, options_for_select(Project.access_options), {class: "project-access-select chosen span3"}
-
-    %tr
-      %td= submit_tag 'Add', class: "btn btn-primary"
-      %td
-        Read more about project permissions
-        %strong= link_to "here", help_permissions_path, class: "vlink"
+%hr
+.row
+  .span6
+    .ui-box
+      %h5.title
+        Project info:
+      %ul.well-list
+        %li
+          %span.light Name:
+          %strong= @project.name
+        %li
+          %span.light Namespace:
+          %strong
+            - if @project.namespace
+              = link_to @project.namespace.human_name, [:admin, @project.group || @project.owner]
+            - else
+              Global
+        %li
+          %span.light Owned by:
+          %strong
+            - if @project.owner
+              = link_to @project.owner_name, admin_user_path(@project.owner)
+            - else
+              (deleted)
+        %li
+          %span.light Created by:
+          %strong
+            = @project.creator.try(:name) || '(deleted)'
+        %li
+          %span.light Created at:
+          %strong
+            = @project.created_at.stamp("March 1, 1999")
+        %li
+          %span.light http:
+          %strong
+            = link_to @project.http_url_to_repo
+        %li
+          %span.light ssh:
+          %strong
+            = link_to @project.ssh_url_to_repo
+        %li
+          %span.light fs:
+          %strong
+            = @repository.path_to_repo
+        %li
+          %span.light last commit:
+          %strong
+            - if @repository
+              = last_commit(@project)
+            - else
+              never
+        %li
+          %span.light access:
+          %strong
+            - if @project.public
+              %span.cblue
+                %i.icon-share
+                Public
+            - else
+              %span.cgreen
+                %i.icon-lock
+                Private
+  .span6
+    .ui-box
+      %h5.title
+        Team
+        %small
+          (#{@project.users.count})
+        = link_to project_team_index_path(@project), class: "btn btn-tiny" do
+          %i.icon-edit
+          Edit Team
+      %ul.well-list.team_members
+        - @project.users.each do |tm|
+          %li
+            %strong
+              = link_to tm.name, admin_user_path(tm)
+            %span.pull-right.light= @project.project_access_human(tm)

app/views/projects/_form.html.haml

@@ -107,8 +107,9 @@
         - if can?(current_user, :change_namespace, @project)
           .ui-box.ui-box-danger
             %h5.title Transfer project
+            .errors-holder
             .form-holder
-              = form_for(@project, remote: true, html: { class: 'transfer-project' }) do |f|
+              = form_for(@project, url: transfer_project_path(@project), remote: true, html: { class: 'transfer-project' }) do |f|
                 .control-group
                   = f.label :namespace_id do
                     %span Namespace

app/views/projects/transfer.js.haml

+- if @project.errors[:namespace_id].present?
+  :plain
+    $("#tab-transfer .errors-holder").replaceWith(errorMessage('#{escape_javascript(@project.errors[:namespace_id].first)}'));
+    $("#tab-transfer .form-actions input").removeAttr('disabled').removeClass('disabled');
+- else
+  :plain
+    location.href = "#{edit_project_path(@project)}";

app/views/projects/update_failed.js.haml

-:plain
-  $(".save-project-loader").replaceWith(errorMessage('#{escape_javascript(@error.message)}'));

config/routes.rb

@@ -85,11 +85,7 @@ Gitlab::Application.routes.draw do
     resource :logs, only: [:show]
     resource :resque, controller: 'resque', only: [:show]
-    resources :projects, constraints: { id: /[a-zA-Z.\/0-9_\-]+/ }, except: [:new, :create] do
-      member do
-        get :team
-        put :team_update
-      end
+    resources :projects, constraints: { id: /[a-zA-Z.\/0-9_\-]+/ }, only: [:index, :show] do
       scope module: :projects, constraints: { id: /[a-zA-Z.\/0-9_\-]+/ } do
         resources :members, only: [:edit, :update, :destroy]
       end
@@ -167,6 +163,10 @@ Gitlab::Application.routes.draw do
   # Project Area
   #
   resources :projects, constraints: { id: /(?:[a-zA-Z.0-9_\-]+\/)?[a-zA-Z.0-9_\-]+/ }, except: [:new, :create, :index], path: "/" do
+    member do
+      put :transfer
+    end
+
     resources :blob,    only: [:show], constraints: {id: /.+/}
     resources :tree,    only: [:show], constraints: {id: /.+/, format: /(html|js)/ }
     resources :edit_tree,    only: [:show, :update], constraints: {id: /.+/}, path: 'edit'

features/steps/admin/admin_projects.rb

@@ -16,9 +16,7 @@ class AdminProjects < Spinach::FeatureSteps
   Then 'I should see project details' do
     project = Project.first
     current_path.should == admin_project_path(project)
-
     page.should have_content(project.name_with_namespace)
     page.should have_content(project.creator.name)
-    page.should have_content('Add new team member')
   end
 end

lib/tasks/gitlab/check.rake

@@ -637,8 +637,8 @@ namespace :gitlab do
   def check_gitlab_shell
     print "GitLab Shell version? ... "
-    if gitlab_shell_version.strip == '1.1.0'
-      puts 'OK (1.1.0)'.green
+    if gitlab_shell_version.strip == '1.2.0'
+      puts 'OK (1.2.0)'.green
     else
       puts 'FAIL. Please update gitlab-shell to v1.1.0'.red
     end

spec/features/admin/admin_projects_spec.rb

@@ -31,46 +31,4 @@ describe "Admin::Projects" do
       page.should have_content(@project.name)
     end
   end
-
-  describe "GET /admin/projects/:id/edit" do
-    before do
-      visit admin_projects_path
-      click_link "edit_project_#{@project.id}"
-    end
-
-    it "should have project edit page" do
-      page.should have_content("Edit project")
-      page.should have_button("Save Project")
-    end
-
-    describe "Update project" do
-      before do
-        fill_in "project_name", with: "Big Bang"
-        click_button "Save Project"
-        @project.reload
-      end
-
-      it "should show page with  new data" do
-        page.should have_content("Big Bang")
-      end
-
-      it "should change project entry" do
-        @project.name.should == "Big Bang"
-      end
-    end
-  end
-
-  describe "Add new team member" do
-    before do
-      @new_user = create(:user)
-      visit admin_project_path(@project)
-    end
-
-    it "should create new user" do
-      select @new_user.name, from: "user_ids"
-      expect { click_button "Add" }.to change { UsersProject.count }.by(1)
-      page.should have_content @new_user.name
-      current_path.should == admin_project_path(@project)
-    end
-  end
 end

spec/features/security/project_access_spec.rb

@@ -33,7 +33,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -44,7 +44,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -55,7 +55,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -66,7 +66,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -77,7 +77,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -88,7 +88,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -99,7 +99,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -114,7 +114,7 @@ describe "Application access" do
       it { @blob_path.should be_allowed_for master }
       it { @blob_path.should be_allowed_for reporter }
-      it { @blob_path.should be_denied_for :admin }
+      it { @blob_path.should be_allowed_for :admin }
       it { @blob_path.should be_denied_for guest }
       it { @blob_path.should be_denied_for :user }
       it { @blob_path.should be_denied_for :visitor }
@@ -125,7 +125,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_denied_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -136,7 +136,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_denied_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -147,7 +147,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -158,7 +158,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -169,7 +169,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -180,7 +180,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -196,7 +196,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -212,7 +212,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }
@@ -223,7 +223,7 @@ describe "Application access" do
       it { should be_allowed_for master }
       it { should be_allowed_for reporter }
-      it { should be_denied_for :admin }
+      it { should be_allowed_for :admin }
       it { should be_denied_for guest }
       it { should be_denied_for :user }
       it { should be_denied_for :visitor }