diff options
author | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2016-07-26 15:27:42 -0300 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2016-07-26 16:31:55 -0300 |
commit | 9865b8e6bdfa33be8206b620670bce2dd6c9bb51 (patch) | |
tree | 86b645c9f036cf57770f47d3cf9680411d2ae3fa | |
parent | e20a92dc5f6d2d066092863d1e97a6c0613183ad (diff) | |
download | gitlab-ce-fix-19538.tar.gz |
Fix Project#to_param to keep invalid project suitable for use in URLsfix-19538
-rw-r--r-- | app/models/project.rb | 6 | ||||
-rw-r--r-- | app/services/projects/update_service.rb | 10 | ||||
-rw-r--r-- | spec/features/projects/project_settings_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 20 | ||||
-rw-r--r-- | spec/services/projects/update_service_spec.rb | 21 |
5 files changed, 27 insertions, 34 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 023b1dc3725..12b95127c1d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -584,7 +584,11 @@ class Project < ActiveRecord::Base end def to_param - path + if persisted? && errors.include?(:path) + path_was + else + path + end end def to_reference(_from_project = nil) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 032c3b182fb..921ca6748d3 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -23,17 +23,7 @@ module Projects if project.previous_changes.include?('path') project.rename_repo end - else - restore_attributes - false end end - - private - - def restore_attributes - project.path = project.path_was if project.errors.include?(:path) - project.name = project.name_was if project.errors.include?(:name) - end end end diff --git a/spec/features/projects/project_settings_spec.rb b/spec/features/projects/project_settings_spec.rb index 5562680d6cc..3de25d7af7d 100644 --- a/spec/features/projects/project_settings_spec.rb +++ b/spec/features/projects/project_settings_spec.rb @@ -32,8 +32,8 @@ describe 'Edit Project Settings', feature: true do click_button 'Rename project' - expect(page).to have_field 'Project name', with: 'sample' - expect(page).to have_field 'Path', with: 'gitlab' + expect(page).to have_field 'Project name', with: 'foo&bar' + expect(page).to have_field 'Path', with: 'foo&bar' expect(page).to have_content "Name can contain only letters, digits, '_', '.', dash and space. It must start with letter, digit or '_'." expect(page).to have_content "Path can contain only letters, digits, '_', '-' and '.'. Cannot start with '-', end in '.git' or end in '.atom'" end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9b017288488..40e2186d591 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -372,6 +372,26 @@ describe Project, models: true do it { expect(@project.to_param).to eq('gitlabhq') } end + + context 'with invalid path' do + it 'returns previous path to keep project suitable for use in URLs when persisted' do + project = create(:empty_project, path: 'gitlab') + project.path = 'foo&bar' + + project.valid? + + expect(project.to_param).to eq 'gitlab' + end + + it 'returns current path when new record' do + project = build(:empty_project, path: 'gitlab') + project.path = 'foo&bar' + + project.valid? + + expect(project.to_param).to eq 'foo&bar' + end + end end describe '#repository' do diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 6337daca9c7..e8b9e6b9238 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -139,27 +139,6 @@ describe Projects::UpdateService, services: true do end end - context 'for invalid project path/name' do - let(:user) { create(:user, admin: true) } - let(:project) { create(:empty_project, path: 'gitlab', name: 'sample') } - let(:params) { { path: 'foo&bar', name: 'foo&bar' } } - - it 'resets to previous values to keep project in a valid state' do - update_project(project, user, params) - - expect(project.path).to eq 'gitlab' - expect(project.name).to eq 'sample' - end - - it 'keeps error messages' do - update_project(project, user, params) - - expect(project.errors).not_to be_blank - expect(project.errors[:name]).to include("can contain only letters, digits, '_', '.', dash and space. It must start with letter, digit or '_'.") - expect(project.errors[:path]).to include("can contain only letters, digits, '_', '-' and '.'. Cannot start with '-', end in '.git' or end in '.atom'") - end - end - def update_project(project, user, opts) Projects::UpdateService.new(project, user, opts).execute end |