From e47f39b9562768b20cb23f3a8f5cf714b11a2013 Mon Sep 17 00:00:00 2001 From: Josh Frye Date: Tue, 12 Jan 2016 12:05:52 -0500 Subject: Don't run parameterize on project.path. Fixes #6073 --- CHANGELOG | 5 ++++ app/services/projects/create_service.rb | 4 --- doc/api/projects.md | 4 +-- lib/api/projects.rb | 9 ++++--- spec/features/issues/filter_by_milestone_spec.rb | 2 +- spec/models/hooks/system_hook_spec.rb | 4 +-- spec/requests/api/projects_spec.rb | 31 ++++++++++++------------ spec/services/issues/bulk_update_service_spec.rb | 2 +- spec/services/projects/create_service_spec.rb | 3 ++- 9 files changed, 35 insertions(+), 29 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2431ad6b05c..0362226be74 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -172,6 +172,11 @@ v 8.9.1 v 8.9.0 - Fix group visibility form layout in application settings +v 9.0.0 (unreleased) + - Fix: Don't downcase project path when using the `name` attribute in api requests + - API: Require path to be set when creating projects, with name optional. + +v 8.9.0 (unreleased) - Fix builds API response not including commit data - Fix error when CI job variables key specified but not defined - Fix pipeline status when there are no builds in pipeline diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 55956be2844..cb360de2f96 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -22,10 +22,6 @@ module Projects elsif @project.path.present? # Set project name from path @project.name = @project.path.dup - elsif @project.name.present? - # For compatibility - set path from name - # TODO: remove this in 8.0 - @project.path = @project.name.dup.parameterize end # get namespace id diff --git a/doc/api/projects.md b/doc/api/projects.md index dceee7b4ea7..573fe28e05a 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -433,8 +433,8 @@ POST /projects Parameters: -- `name` (required) - new project name -- `path` (optional) - custom repository name for new project. By default generated based on name +- `name` (optional) - new project name. By default generated based on path +- `path` (required) - custom repository path for new project - `namespace_id` (optional) - namespace for the new project (defaults to user) - `description` (optional) - short project description - `issues_enabled` (optional) diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 0cc1edd65c8..e87880136e4 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -87,7 +87,8 @@ module API # Create new project # # Parameters: - # name (required) - name for new project + # path (required) + # name (optional) - name for new project # description (optional) - short project description # issues_enabled (optional) # merge_requests_enabled (optional) @@ -104,7 +105,7 @@ module API # Example Request # POST /projects post do - required_attributes! [:name] + required_attributes! [:path] attrs = attributes_for_keys [:name, :path, :description, @@ -137,7 +138,8 @@ module API # # Parameters: # user_id (required) - The ID of a user - # name (required) - name for new project + # path (required) - path for the new project + # name (optional) - name for new project # description (optional) - short project description # default_branch (optional) - 'master' by default # issues_enabled (optional) @@ -157,6 +159,7 @@ module API authenticated_as_admin! user = User.find(params[:user_id]) attrs = attributes_for_keys [:name, + :path, :description, :default_branch, :issues_enabled, diff --git a/spec/features/issues/filter_by_milestone_spec.rb b/spec/features/issues/filter_by_milestone_spec.rb index 99445185893..d6c060836c5 100644 --- a/spec/features/issues/filter_by_milestone_spec.rb +++ b/spec/features/issues/filter_by_milestone_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' feature 'Issue filtering by Milestone', feature: true do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, path: 'foo') } let(:milestone) { create(:milestone, project: project) } scenario 'filters by no Milestone', js: true do diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 4078b9e4ff5..397edd302cb 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -30,7 +30,7 @@ describe SystemHook, models: true do end it "project_create hook" do - Projects::CreateService.new(user, name: 'empty').execute + Projects::CreateService.new(user, path: 'empty').execute expect(WebMock).to have_requested(:post, system_hook.url).with( body: /project_create/, headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } @@ -48,7 +48,7 @@ describe SystemHook, models: true do it "user_create hook" do create(:user) - + expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_create/, headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 8a52725a893..154bf96a331 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -183,25 +183,25 @@ describe API::API, api: true do context 'maximum number of projects reached' do it 'should not create new project and respond with 403' do allow_any_instance_of(User).to receive(:projects_limit_left).and_return(0) - expect { post api('/projects', user2), name: 'foo' }. + expect { post api('/projects', user2), path: 'foo' }. to change {Project.count}.by(0) expect(response).to have_http_status(403) end end - it 'should create new project without path and return 201' do - expect { post api('/projects', user), name: 'foo' }. + it 'should create new project without name and return 201' do + expect { post api('/projects', user), path: 'foo' }. to change { Project.count }.by(1) expect(response).to have_http_status(201) end it 'should create last project before reaching project limit' do allow_any_instance_of(User).to receive(:projects_limit_left).and_return(1) - post api('/projects', user2), name: 'foo' + post api('/projects', user2), path: 'foo' expect(response).to have_http_status(201) end - it 'should not create new project without name and return 400' do + it 'should not create new project without path and return 400' do expect { post api('/projects', user) }.not_to change { Project.count } expect(response).to have_http_status(400) end @@ -293,9 +293,9 @@ describe API::API, api: true do before { project } before { admin } - it 'should create new project without path and return 201' do - expect { post api("/projects/user/#{user.id}", admin), name: 'foo' }.to change {Project.count}.by(1) - expect(response).to have_http_status(201) + it 'should create new project without name and return 201' do + expect { post api("/projects/user/#{user.id}", admin), path: 'foo' }.to change {Project.count}.by(1) + expect(response.status).to eq(201) end it 'should respond with 400 on failure and not project' do @@ -320,7 +320,8 @@ describe API::API, api: true do description: FFaker::Lorem.sentence, issues_enabled: false, merge_requests_enabled: false, - wiki_enabled: false + wiki_enabled: false, + path: 'foo' }) post api("/projects/user/#{user.id}", admin), project @@ -332,42 +333,42 @@ describe API::API, api: true do end it 'should set a project as public' do - project = attributes_for(:project, :public) + project = attributes_for(:project, :public, path: 'foo') post api("/projects/user/#{user.id}", admin), project expect(json_response['public']).to be_truthy expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PUBLIC) end it 'should set a project as public using :public' do - project = attributes_for(:project, { public: true }) + project = attributes_for(:project, { public: true, path: 'foo' }) post api("/projects/user/#{user.id}", admin), project expect(json_response['public']).to be_truthy expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PUBLIC) end it 'should set a project as internal' do - project = attributes_for(:project, :internal) + project = attributes_for(:project, :internal, path: 'foo') post api("/projects/user/#{user.id}", admin), project expect(json_response['public']).to be_falsey expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::INTERNAL) end it 'should set a project as internal overriding :public' do - project = attributes_for(:project, :internal, { public: true }) + project = attributes_for(:project, :internal, { public: true, path: 'foo' }) post api("/projects/user/#{user.id}", admin), project expect(json_response['public']).to be_falsey expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::INTERNAL) end it 'should set a project as private' do - project = attributes_for(:project, :private) + project = attributes_for(:project, :private, path: 'foo') post api("/projects/user/#{user.id}", admin), project expect(json_response['public']).to be_falsey expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE) end it 'should set a project as private using :public' do - project = attributes_for(:project, { public: false }) + project = attributes_for(:project, { public: false, path: 'foo' }) post api("/projects/user/#{user.id}", admin), project expect(json_response['public']).to be_falsey expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE) diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issues/bulk_update_service_spec.rb index 4a689e64dc5..86036a4b855 100644 --- a/spec/services/issues/bulk_update_service_spec.rb +++ b/spec/services/issues/bulk_update_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Issues::BulkUpdateService, services: true do let(:user) { create(:user) } - let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute } + let(:project) { Projects::CreateService.new(user, namespace: user.namespace, path: 'test').execute } let!(:result) { Issues::BulkUpdateService.new(project, user, params).execute } diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index fd114359467..ef81a46c70d 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -6,7 +6,8 @@ describe Projects::CreateService, services: true do @user = create :user @opts = { name: "GitLab", - namespace: @user.namespace + namespace: @user.namespace, + path: 'foo' } end -- cgit v1.2.1