From 188c3be355a77ff1e3c6267af19bb2790dd97fb7 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 19 Nov 2013 17:02:05 +0600 Subject: [PATCH 1/3] [#325] add abibity to create fork with different name --- app/assets/javascripts/extra/fork.js | 19 ++++++++ app/assets/stylesheets/design/git.scss | 7 +++ app/controllers/api/v1/projects_controller.rb | 2 +- .../projects/projects_controller.rb | 9 +++- app/models/project.rb | 4 +- .../projects/git/base/_choose_fork.html.haml | 5 +- app/views/projects/git/base/_fork.html.haml | 15 +++--- app/views/projects/git/base/_forks.html.haml | 5 ++ config/routes.rb | 1 + .../api/v1/projects_controller_spec.rb | 47 +++++++++++++++---- .../projects/projects_controller_spec.rb | 9 ++-- 11 files changed, 98 insertions(+), 25 deletions(-) create mode 100644 app/assets/javascripts/extra/fork.js create mode 100644 app/views/projects/git/base/_forks.html.haml diff --git a/app/assets/javascripts/extra/fork.js b/app/assets/javascripts/extra/fork.js new file mode 100644 index 000000000..1c56b269f --- /dev/null +++ b/app/assets/javascripts/extra/fork.js @@ -0,0 +1,19 @@ +$(document).ready(function() { + var fork_name = $('#fork_name'); + var forks_path = $('#possible_forks_path'); + + fork_name.keyup(function(){ + $.ajax({ + type: 'GET', + url: forks_path.val(), + data: 'name=' + fork_name.val(), + success: function(data){ + $('#forks_list').html(data); + }, + error: function(data){ + alert('error'); // TODO remove + } + }); + }); + +}); \ No newline at end of file diff --git a/app/assets/stylesheets/design/git.scss b/app/assets/stylesheets/design/git.scss index 8428c17b1..264ed598c 100644 --- a/app/assets/stylesheets/design/git.scss +++ b/app/assets/stylesheets/design/git.scss @@ -279,3 +279,10 @@ table.blame td.message .message { background: #ECECEC; color: #000; } + +.fork_name { + height: 20px; + margin-left: 10px; + width: 450px; + padding: 0 5px; +} diff --git a/app/controllers/api/v1/projects_controller.rb b/app/controllers/api/v1/projects_controller.rb index e115e54d1..1c9c82f15 100644 --- a/app/controllers/api/v1/projects_controller.rb +++ b/app/controllers/api/v1/projects_controller.rb @@ -66,7 +66,7 @@ class Api::V1::ProjectsController < Api::V1::BaseController def fork owner = (Group.find params[:group_id] if params[:group_id].present?) || current_user authorize! :write, owner if owner.class == Group - if forked = @project.fork(owner) and forked.valid? + if forked = @project.fork(owner, params[:fork_name]) and forked.valid? render_json_response forked, 'Project has been forked successfully' else render_validation_error forked, 'Project has not been forked' diff --git a/app/controllers/projects/projects_controller.rb b/app/controllers/projects/projects_controller.rb index 03d14cdc3..cd92f47fd 100644 --- a/app/controllers/projects/projects_controller.rb +++ b/app/controllers/projects/projects_controller.rb @@ -2,7 +2,7 @@ class Projects::ProjectsController < Projects::BaseController include ProjectsHelper before_filter :authenticate_user! - load_and_authorize_resource :id_param => :project_name # to force member actions load + load_and_authorize_resource :id_param => :project_name, :collection => :possible_forks # to force member actions load def index @projects = Project.accessible_by(current_ability, :membered) @@ -68,7 +68,7 @@ class Projects::ProjectsController < Projects::BaseController def fork owner = (Group.find params[:group] if params[:group].present?) || current_user authorize! :write, owner if owner.class == Group - if forked = @project.fork(owner) and forked.valid? + if forked = @project.fork(owner, params[:fork_name]) and forked.valid? redirect_to forked, :notice => t("flash.project.forked") else flash[:warning] = t("flash.project.fork_error") @@ -77,6 +77,11 @@ class Projects::ProjectsController < Projects::BaseController end end + def possible_forks + render :partial => 'projects/git/base/forks', :layout => false, + :locals => { :owner => current_user, :name => (params[:name].presence || @project.name) } + end + def sections if request.post? if @project.update_attributes(params[:project]) diff --git a/app/models/project.rb b/app/models/project.rb index b7dbd8e53..3b2be1721 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -189,8 +189,10 @@ class Project < ActiveRecord::Base build_list.save end - def fork(new_owner) + def fork(new_owner, new_name) + new_name = new_name.presence || name dup.tap do |c| + c.name = new_name c.parent_id = id c.owner = new_owner c.updated_at = nil; c.created_at = nil # :id = nil diff --git a/app/views/projects/git/base/_choose_fork.html.haml b/app/views/projects/git/base/_choose_fork.html.haml index e627dd77c..f92a438c0 100644 --- a/app/views/projects/git/base/_choose_fork.html.haml +++ b/app/views/projects/git/base/_choose_fork.html.haml @@ -1,11 +1,12 @@ -- if owner.own_projects.exists? :name => @project.name +- if owner.own_projects.exists? :name => name - is_group = owner.class == Group ? "(#{t 'activerecord.models.group'})" : '' %p.center =t 'layout.projects.already_exists' - =link_to "#{owner.uname}/#{@project.name} #{is_group}", project_path(owner, @project.name) + =link_to "#{owner.uname}/#{name} #{is_group}", project_path(owner, name) - else = form_for @project, :url => fork_project_path(@project), :html => { :class => :form, :multipart => true, :method => :post } do |f| = hidden_field_tag :group, owner.id if owner.class == Group + = hidden_field_tag :fork_name, name, :name => 'fork_name' =f.submit t('layout.projects.fork_to', :to => "#{owner.uname} #{is_group}"), :class => 'btn btn-primary disabled', 'data-loading-text' => t('layout.processing'), :id => 'create_fork' :javascript diff --git a/app/views/projects/git/base/_fork.html.haml b/app/views/projects/git/base/_fork.html.haml index 98d55a301..9571fcbeb 100644 --- a/app/views/projects/git/base/_fork.html.haml +++ b/app/views/projects/git/base/_fork.html.haml @@ -1,3 +1,5 @@ += hidden_field_tag :possible_forks_path, possible_forks_project_path(@project) + - if can? :write, @project .r{:style => "display: block"} =link_to t("projects.pull_requests.show.pull"), new_project_pull_request_path(@project, :treeish => @treeish), :id => 'send_pull_request', :class => 'button' @@ -8,11 +10,12 @@ .modal-header %a.close{"data-dismiss" => "modal"} × %h3=t 'layout.projects.fork_modal_header' - .modal-body.modal-body-fork - =render 'choose_fork', :owner => current_user - %hr.bootstrap - - Group.can_own_project(current_user).each do |group| - =render 'choose_fork', :owner => group - %hr.bootstrap + = hidden_field_tag :possible_forks, possible_forks_project_path(@project) + %div + = Project.human_attribute_name :name + = text_field_tag 'fork_name', @project.name, :id => 'fork_name', :class => 'fork_name' + #forks_list.modal-body.modal-body-fork + = render 'forks', :owner => current_user, :name => @project.name + - if @project.is_package && can?(:create, @project.build_lists.new) .r{:style => "display: block"}= link_to t('layout.projects.new_build_list'), new_project_build_list_path(@project), :class => 'button' diff --git a/app/views/projects/git/base/_forks.html.haml b/app/views/projects/git/base/_forks.html.haml new file mode 100644 index 000000000..a4d442860 --- /dev/null +++ b/app/views/projects/git/base/_forks.html.haml @@ -0,0 +1,5 @@ +=render 'projects/git/base/choose_fork', :owner => current_user, :name => name +%hr.bootstrap +- Group.can_own_project(current_user).each do |group| + =render 'projects/git/base/choose_fork', :owner => group, :name => name + %hr.bootstrap \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index d8e93236f..cdfc771e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -344,6 +344,7 @@ Rosa::Application.routes.draw do delete '/' => 'projects#destroy' # Member post '/fork' => 'projects#fork', :as => :fork_project + get '/possible_forks' => 'projects#possible_forks', :as => :possible_forks_project get '/sections' => 'projects#sections', :as => :sections_project post '/sections' => 'projects#sections' delete '/remove_user' => 'projects#remove_user', :as => :remove_user_project diff --git a/spec/controllers/api/v1/projects_controller_spec.rb b/spec/controllers/api/v1/projects_controller_spec.rb index b7991fab6..ca398bf3b 100644 --- a/spec/controllers/api/v1/projects_controller_spec.rb +++ b/spec/controllers/api/v1/projects_controller_spec.rb @@ -62,6 +62,17 @@ shared_examples_for 'api projects user with fork rights' do it 'ensures that project has been forked' do lambda { post :fork, :id => @project.id, :format => :json }.should change{ Project.count }.by(1) end + + it 'should be able to perform fork action with different name' do + post :fork, :id => @project.id, :fork_name => (@project.name + '_forked'), :format => :json + response.should be_success + end + + it 'ensures that project has been forked' do + new_name = @project.name + '_forked' + lambda { post :fork, :id => @project.id, :fork_name => new_name, :format => :json }.should + change{ Project.where(:name => new_name).count }.by(1) + end end shared_examples_for 'api projects user with fork rights for hidden project' do @@ -334,16 +345,36 @@ describe Api::V1::ProjectsController do it_should_behave_like 'api projects user without admin rights' it_should_behave_like 'api projects user without owner rights' - it 'group writer should be able to fork project to their group' do - group = FactoryGirl.create(:group) - group.actors.create(:actor_type => 'User', :actor_id => @user.id, :role => 'writer') - lambda {post :fork, :id => @project.id, :group_id => group.id}.should change{ Project.count }.by(1) + context 'group writer' do + it 'should be able to fork project to their group' do + group = FactoryGirl.create(:group) + group.actors.create(:actor_type => 'User', :actor_id => @user.id, :role => 'writer') + lambda {post :fork, :id => @project.id, :group_id => group.id}.should change{ Project.count }.by(1) + end + + it 'should be able to fork project with different name to their group' do + group = FactoryGirl.create(:group) + group.actors.create(:actor_type => 'User', :actor_id => @user.id, :role => 'writer') + new_name = @project.name + '_forked' + lambda { post :fork, :id => @project.id, :group_id => group.id, :fork_name => new_name }.should + change { Project.where(:name => new_name).count }.by(1) + end end - it 'group reader should not be able to fork project to their group' do - group = FactoryGirl.create(:group) - group.actors.create(:actor_type => 'User', :actor_id => @user.id, :role => 'reader') - lambda {post :fork, :id => @project.id, :group_id => group.id}.should change{ Project.count }.by(0) + context 'group reader' do + it 'should not be able to fork project to their group' do + group = FactoryGirl.create(:group) + group.actors.create(:actor_type => 'User', :actor_id => @user.id, :role => 'reader') + lambda {post :fork, :id => @project.id, :group_id => group.id}.should change{ Project.count }.by(0) + end + + it 'should not be able to fork project with different name to their group' do + group = FactoryGirl.create(:group) + new_name = @project.name + '_forked' + group.actors.create(:actor_type => 'User', :actor_id => @user.id, :role => 'reader') + lambda { post :fork, :id => @project.id, :group_id => group.id, :fork_name => new_name }.should + change{ Project.where(:name => new_name.count) }.by(0) + end end end diff --git a/spec/controllers/projects/projects_controller_spec.rb b/spec/controllers/projects/projects_controller_spec.rb index 07b57bb46..cc6ce6a83 100644 --- a/spec/controllers/projects/projects_controller_spec.rb +++ b/spec/controllers/projects/projects_controller_spec.rb @@ -21,11 +21,10 @@ shared_examples_for 'projects user with reader rights' do :group => group.id}.should change{ Project.count }.by(1) end - # it 'should be able to view project' do - # get :show, :owner_name => @project.owner.uname, :project_name => @project.name - # assigns(:project).should eq @project - # end - + it 'should be able to fork project with different name' do + post :fork, :owner_name => @project.owner.uname, :project_name => @project.name, :fork_name => 'another_name' + response.should redirect_to(project_path(Project.where(:name => 'another_name').last)) + end end shared_examples_for 'projects user with project admin rights' do From bf86eb22b36d83afa7fdc19dff27af26dc5be9ca Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Tue, 19 Nov 2013 18:35:07 +0600 Subject: [PATCH 2/3] [#325] try to fix specs --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 019e9fb23..0006bb175 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -200,7 +200,7 @@ class Project < ActiveRecord::Base build_list.save end - def fork(new_owner, new_name) + def fork(new_owner, new_name = name) new_name = new_name.presence || name dup.tap do |c| c.name = new_name From f29eee6d5da9c1786df94fa2965630c512489763 Mon Sep 17 00:00:00 2001 From: Alexander Machehin Date: Wed, 20 Nov 2013 16:19:41 +0600 Subject: [PATCH 3/3] [#325] try to fix input --- app/assets/stylesheets/design/custom.scss | 8 ++++++++ app/assets/stylesheets/design/git.scss | 7 ------- app/views/projects/git/base/_choose_fork.html.haml | 8 +++++--- app/views/projects/git/base/_fork.html.haml | 4 +--- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/design/custom.scss b/app/assets/stylesheets/design/custom.scss index 1a8bdbe61..c94067c43 100644 --- a/app/assets/stylesheets/design/custom.scss +++ b/app/assets/stylesheets/design/custom.scss @@ -2111,3 +2111,11 @@ table tbody { cursor: default; } } + +.fork_name { + height: 20px; + margin: 0 0 5px 10px; + width: 400px; + padding: 0 5px; + margin:auto; +} diff --git a/app/assets/stylesheets/design/git.scss b/app/assets/stylesheets/design/git.scss index 264ed598c..8428c17b1 100644 --- a/app/assets/stylesheets/design/git.scss +++ b/app/assets/stylesheets/design/git.scss @@ -279,10 +279,3 @@ table.blame td.message .message { background: #ECECEC; color: #000; } - -.fork_name { - height: 20px; - margin-left: 10px; - width: 450px; - padding: 0 5px; -} diff --git a/app/views/projects/git/base/_choose_fork.html.haml b/app/views/projects/git/base/_choose_fork.html.haml index f92a438c0..f71676821 100644 --- a/app/views/projects/git/base/_choose_fork.html.haml +++ b/app/views/projects/git/base/_choose_fork.html.haml @@ -1,13 +1,15 @@ +- is_group = owner.class == Group ? "(#{t 'activerecord.models.group'})" : '' +- full_name = "#{owner.uname}/#{name} #{is_group}" + - if owner.own_projects.exists? :name => name - - is_group = owner.class == Group ? "(#{t 'activerecord.models.group'})" : '' %p.center =t 'layout.projects.already_exists' - =link_to "#{owner.uname}/#{name} #{is_group}", project_path(owner, name) + =link_to full_name, project_path(owner, name) - else = form_for @project, :url => fork_project_path(@project), :html => { :class => :form, :multipart => true, :method => :post } do |f| = hidden_field_tag :group, owner.id if owner.class == Group = hidden_field_tag :fork_name, name, :name => 'fork_name' - =f.submit t('layout.projects.fork_to', :to => "#{owner.uname} #{is_group}"), :class => 'btn btn-primary disabled', 'data-loading-text' => t('layout.processing'), :id => 'create_fork' + =f.submit t('layout.projects.fork_to', :to => full_name), :class => 'btn btn-primary disabled', 'data-loading-text' => t('layout.processing'), :id => 'create_fork' :javascript $('#create_fork').click(function () { diff --git a/app/views/projects/git/base/_fork.html.haml b/app/views/projects/git/base/_fork.html.haml index 9571fcbeb..6f15afbcb 100644 --- a/app/views/projects/git/base/_fork.html.haml +++ b/app/views/projects/git/base/_fork.html.haml @@ -11,9 +11,7 @@ %a.close{"data-dismiss" => "modal"} × %h3=t 'layout.projects.fork_modal_header' = hidden_field_tag :possible_forks, possible_forks_project_path(@project) - %div - = Project.human_attribute_name :name - = text_field_tag 'fork_name', @project.name, :id => 'fork_name', :class => 'fork_name' + %div{:align => 'center'}= text_field_tag 'fork_name', @project.name, :id => 'fork_name', :class => 'fork_name' #forks_list.modal-body.modal-body-fork = render 'forks', :owner => current_user, :name => @project.name