Don't run parameterize on project.path. Fixes #6073fix-api-project-name-case

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