Don't run parameterize on project.path. Fixes #6073

e47f39b9 · Josh Frye · 3999b80e · e47f39b9 · e47f39b9 · e47f39b9
--- 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

--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -22,10 +22,6 @@ def execute
      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

--- 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)

--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -87,7 +87,8 @@ def map_public_to_visibility_level(attrs)
      # 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 @@ def map_public_to_visibility_level(attrs)
      # Example Request
      #   POST /projects
      post do
-        required_attributes! [:name]
+        required_attributes! [:path]
        attrs = attributes_for_keys [:name,
                                     :path,
                                     :description,
@@ -137,7 +138,8 @@ def map_public_to_visibility_level(attrs)
      #
      # 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 @@ def map_public_to_visibility_level(attrs)
        authenticated_as_admin!
        user = User.find(params[:user_id])
        attrs = attributes_for_keys [:name,
+                                     :path,
                                     :description,
                                     :default_branch,
                                     :issues_enabled,

--- a/spec/features/issues/filter_by_milestone_spec.rb
+++ b/spec/features/issues/filter_by_milestone_spec.rb
 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

--- a/spec/models/hooks/system_hook_spec.rb
+++ b/spec/models/hooks/system_hook_spec.rb
@@ -30,7 +30,7 @@
    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 @@

    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' }

--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -183,25 +183,25 @@
    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 @@
    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 @@
        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 @@
    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)

--- a/spec/services/issues/bulk_update_service_spec.rb
+++ b/spec/services/issues/bulk_update_service_spec.rb
@@ -2,7 +2,7 @@

 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 }


--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -6,7 +6,8 @@
      @user = create :user
      @opts = {
        name: "GitLab",
-        namespace: @user.namespace
+        namespace: @user.namespace,
+        path: 'foo'
      }
    end