Merge branch 'sandish/gitlab-ce-update_ret_val' into 'master'
Ensure updating project settings shows a flash message on success See merge request !8579
This commit is contained in:
commit
55b3ee7439
7 changed files with 85 additions and 114 deletions
|
@ -42,19 +42,16 @@ class ProjectsController < Projects::ApplicationController
|
|||
end
|
||||
|
||||
def update
|
||||
status = ::Projects::UpdateService.new(@project, current_user, project_params).execute
|
||||
result = ::Projects::UpdateService.new(@project, current_user, project_params).execute
|
||||
|
||||
# Refresh the repo in case anything changed
|
||||
@repository = project.repository
|
||||
@repository = @project.repository
|
||||
|
||||
respond_to do |format|
|
||||
if status
|
||||
if result[:status] == :success
|
||||
flash[:notice] = "Project '#{@project.name}' was successfully updated."
|
||||
format.html do
|
||||
redirect_to(
|
||||
edit_project_path(@project),
|
||||
notice: "Project '#{@project.name}' was successfully updated."
|
||||
)
|
||||
redirect_to(edit_project_path(@project))
|
||||
end
|
||||
else
|
||||
format.html { render 'edit' }
|
||||
|
|
|
@ -9,7 +9,7 @@ module Projects
|
|||
Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
|
||||
|
||||
deny_visibility_level(project, new_visibility)
|
||||
return project
|
||||
return error('Visibility level unallowed')
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -23,6 +23,10 @@ module Projects
|
|||
if project.previous_changes.include?('path')
|
||||
project.rename_repo
|
||||
end
|
||||
|
||||
success
|
||||
else
|
||||
error('Project could not be updated')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
title: Ensure updating project settings shows a flash message on success
|
||||
merge_request: 8579
|
||||
author: Sandish Chen
|
|
@ -295,13 +295,13 @@ module API
|
|||
authorize! :rename_project, user_project if attrs[:name].present?
|
||||
authorize! :change_visibility_level, user_project if attrs[:visibility_level].present?
|
||||
|
||||
::Projects::UpdateService.new(user_project, current_user, attrs).execute
|
||||
result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute
|
||||
|
||||
if user_project.errors.any?
|
||||
render_validation_error!(user_project)
|
||||
else
|
||||
if result[:status] == :success
|
||||
present user_project, with: Entities::Project,
|
||||
user_can_admin_project: can?(current_user, :admin_project, user_project)
|
||||
else
|
||||
render_validation_error!(user_project)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -245,7 +245,7 @@ describe ProjectsController do
|
|||
|
||||
expect(project.repository.path).to include(new_path)
|
||||
expect(assigns(:repository).path).to eq(project.repository.path)
|
||||
expect(response).to have_http_status(200)
|
||||
expect(response).to have_http_status(302)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -21,6 +21,16 @@ describe 'Edit Project Settings', feature: true do
|
|||
expect(page).to have_content "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'."
|
||||
expect(page).to have_button 'Save changes'
|
||||
end
|
||||
|
||||
scenario 'shows a successful notice when the project is updated' do
|
||||
visit edit_namespace_project_path(project.namespace, project)
|
||||
|
||||
fill_in 'project_name_edit', with: 'hello world'
|
||||
|
||||
click_button 'Save changes'
|
||||
|
||||
expect(page).to have_content "Project 'hello world' was successfully updated."
|
||||
end
|
||||
end
|
||||
|
||||
describe 'Rename repository' do
|
||||
|
|
|
@ -1,145 +1,101 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Projects::UpdateService, services: true do
|
||||
describe :update_by_user do
|
||||
before do
|
||||
@user = create :user
|
||||
@admin = create :user, admin: true
|
||||
@project = create :project, creator_id: @user.id, namespace: @user.namespace
|
||||
@opts = {}
|
||||
end
|
||||
let(:user) { create(:user) }
|
||||
let(:admin) { create(:admin) }
|
||||
let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
|
||||
|
||||
context 'is private when updated to private' do
|
||||
before do
|
||||
@created_private = @project.private?
|
||||
describe 'update_by_user' do
|
||||
context 'when visibility_level is INTERNAL' do
|
||||
it 'updates the project to internal' do
|
||||
result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL)
|
||||
|
||||
@opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
|
||||
update_project(@project, @user, @opts)
|
||||
expect(result).to eq({ status: :success })
|
||||
expect(project).to be_internal
|
||||
end
|
||||
|
||||
it { expect(@created_private).to be_truthy }
|
||||
it { expect(@project.private?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'is internal when updated to internal' do
|
||||
before do
|
||||
@created_private = @project.private?
|
||||
|
||||
@opts.merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
|
||||
update_project(@project, @user, @opts)
|
||||
context 'when visibility_level is PUBLIC' do
|
||||
it 'updates the project to public' do
|
||||
result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
||||
expect(result).to eq({ status: :success })
|
||||
expect(project).to be_public
|
||||
end
|
||||
|
||||
it { expect(@created_private).to be_truthy }
|
||||
it { expect(@project.internal?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'is public when updated to public' do
|
||||
context 'when visibility levels are restricted to PUBLIC only' do
|
||||
before do
|
||||
@created_private = @project.private?
|
||||
|
||||
@opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
||||
update_project(@project, @user, @opts)
|
||||
end
|
||||
|
||||
it { expect(@created_private).to be_truthy }
|
||||
it { expect(@project.public?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'respect configured visibility restrictions setting' do
|
||||
before(:each) do
|
||||
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
|
||||
end
|
||||
|
||||
context 'is private when updated to private' do
|
||||
before do
|
||||
@created_private = @project.private?
|
||||
|
||||
@opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
|
||||
update_project(@project, @user, @opts)
|
||||
context 'when visibility_level is INTERNAL' do
|
||||
it 'updates the project to internal' do
|
||||
result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL)
|
||||
expect(result).to eq({ status: :success })
|
||||
expect(project).to be_internal
|
||||
end
|
||||
|
||||
it { expect(@created_private).to be_truthy }
|
||||
it { expect(@project.private?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'is internal when updated to internal' do
|
||||
before do
|
||||
@created_private = @project.private?
|
||||
context 'when visibility_level is PUBLIC' do
|
||||
it 'does not update the project to public' do
|
||||
result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
||||
|
||||
@opts.merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
|
||||
update_project(@project, @user, @opts)
|
||||
expect(result).to eq({ status: :error, message: 'Visibility level unallowed' })
|
||||
expect(project).to be_private
|
||||
end
|
||||
|
||||
it { expect(@created_private).to be_truthy }
|
||||
it { expect(@project.internal?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'is private when updated to public' do
|
||||
before do
|
||||
@created_private = @project.private?
|
||||
|
||||
@opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
||||
update_project(@project, @user, @opts)
|
||||
context 'when updated by an admin' do
|
||||
it 'updates the project to public' do
|
||||
result = update_project(project, admin, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
||||
expect(result).to eq({ status: :success })
|
||||
expect(project).to be_public
|
||||
end
|
||||
end
|
||||
|
||||
it { expect(@created_private).to be_truthy }
|
||||
it { expect(@project.private?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'is public when updated to public by admin' do
|
||||
before do
|
||||
@created_private = @project.private?
|
||||
|
||||
@opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
||||
update_project(@project, @admin, @opts)
|
||||
end
|
||||
|
||||
it { expect(@created_private).to be_truthy }
|
||||
it { expect(@project.public?).to be_truthy }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe :visibility_level do
|
||||
let(:user) { create :user, admin: true }
|
||||
describe 'visibility_level' do
|
||||
let(:project) { create(:project, :internal) }
|
||||
let(:forked_project) { create(:forked_project_with_submodules, :internal) }
|
||||
let(:opts) { {} }
|
||||
|
||||
before do
|
||||
forked_project.build_forked_project_link(forked_to_project_id: forked_project.id, forked_from_project_id: project.id)
|
||||
forked_project.save
|
||||
|
||||
@created_internal = project.internal?
|
||||
@fork_created_internal = forked_project.internal?
|
||||
end
|
||||
|
||||
context 'updates forks visibility level when parent set to more restrictive' do
|
||||
before do
|
||||
opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
|
||||
update_project(project, user, opts).inspect
|
||||
end
|
||||
it 'updates forks visibility level when parent set to more restrictive' do
|
||||
opts = { visibility_level: Gitlab::VisibilityLevel::PRIVATE }
|
||||
|
||||
it { expect(@created_internal).to be_truthy }
|
||||
it { expect(@fork_created_internal).to be_truthy }
|
||||
it { expect(project.private?).to be_truthy }
|
||||
it { expect(project.forks.first.private?).to be_truthy }
|
||||
expect(project).to be_internal
|
||||
expect(forked_project).to be_internal
|
||||
|
||||
expect(update_project(project, admin, opts)).to eq({ status: :success })
|
||||
|
||||
expect(project).to be_private
|
||||
expect(forked_project.reload).to be_private
|
||||
end
|
||||
|
||||
context 'does not update forks visibility level when parent set to less restrictive' do
|
||||
before do
|
||||
opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
|
||||
update_project(project, user, opts).inspect
|
||||
end
|
||||
it 'does not update forks visibility level when parent set to less restrictive' do
|
||||
opts = { visibility_level: Gitlab::VisibilityLevel::PUBLIC }
|
||||
|
||||
it { expect(@created_internal).to be_truthy }
|
||||
it { expect(@fork_created_internal).to be_truthy }
|
||||
it { expect(project.public?).to be_truthy }
|
||||
it { expect(project.forks.first.internal?).to be_truthy }
|
||||
expect(project).to be_internal
|
||||
expect(forked_project).to be_internal
|
||||
|
||||
expect(update_project(project, admin, opts)).to eq({ status: :success })
|
||||
|
||||
expect(project).to be_public
|
||||
expect(forked_project.reload).to be_internal
|
||||
end
|
||||
end
|
||||
|
||||
it 'returns an error result when record cannot be updated' do
|
||||
result = update_project(project, admin, { name: 'foo&bar' })
|
||||
|
||||
expect(result).to eq({ status: :error, message: 'Project could not be updated' })
|
||||
end
|
||||
|
||||
def update_project(project, user, opts)
|
||||
Projects::UpdateService.new(project, user, opts).execute
|
||||
described_class.new(project, user, opts).execute
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue