diff options
| -rw-r--r-- | CONTRIBUTING.md | 2 | ||||
| -rw-r--r-- | app/assets/javascripts/commit/pipelines/pipelines_store.js.es6 | 2 | ||||
| -rw-r--r-- | app/assets/javascripts/vue_realtime_listener/index.js.es6 | 11 | ||||
| -rw-r--r-- | app/models/namespace.rb | 15 | ||||
| -rw-r--r-- | app/views/projects/commits/show.html.haml | 4 | ||||
| -rw-r--r-- | app/views/projects/compare/_form.html.haml | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/20495-plus-icon-button.yml | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/23104-remove-public-param-for-projects.yml | 4 | ||||
| -rw-r--r-- | doc/api/projects.md | 3 | ||||
| -rw-r--r-- | lib/api/projects.rb | 19 | ||||
| -rw-r--r-- | spec/javascripts/commit/pipelines/pipelines_store_spec.js.es6 | 3 | ||||
| -rw-r--r-- | spec/models/namespace_spec.rb | 33 | ||||
| -rw-r--r-- | spec/requests/api/projects_spec.rb | 61 |
13 files changed, 75 insertions, 90 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 315cd1e598c..7c08c29d8a3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -430,7 +430,7 @@ merge request: 1. [Newlines styleguide][newlines-styleguide] 1. [Testing](doc/development/testing.md) 1. [JavaScript (ES6)](https://github.com/airbnb/javascript) -1. [JavaScript (ES5)](https://github.com/airbnb/javascript/tree/master/es5) +1. [JavaScript (ES5)](https://github.com/airbnb/javascript/tree/es5-deprecated/es5) 1. [SCSS styleguide][scss-styleguide] 1. [Shell commands](doc/development/shell_commands.md) created by GitLab contributors to enhance security diff --git a/app/assets/javascripts/commit/pipelines/pipelines_store.js.es6 b/app/assets/javascripts/commit/pipelines/pipelines_store.js.es6 index 11a3449d99a..f1b41911b73 100644 --- a/app/assets/javascripts/commit/pipelines/pipelines_store.js.es6 +++ b/app/assets/javascripts/commit/pipelines/pipelines_store.js.es6 @@ -26,7 +26,7 @@ class PipelinesStore { */ startTimeAgoLoops() { const startTimeLoops = () => { - this.timeLoopInterval = setInterval(function timeloopInterval() { + this.timeLoopInterval = setInterval(() => { this.$children[0].$children.reduce((acc, component) => { const timeAgoComponent = component.$children.filter(el => el.$options._componentTag === 'time-ago')[0]; acc.push(timeAgoComponent); diff --git a/app/assets/javascripts/vue_realtime_listener/index.js.es6 b/app/assets/javascripts/vue_realtime_listener/index.js.es6 index 95564152cce..30f6680a673 100644 --- a/app/assets/javascripts/vue_realtime_listener/index.js.es6 +++ b/app/assets/javascripts/vue_realtime_listener/index.js.es6 @@ -14,5 +14,16 @@ window.addEventListener('focus', startIntervals); window.addEventListener('blur', removeIntervals); document.addEventListener('beforeunload', removeAll); + + // add removeAll methods to stack + const stack = gl.VueRealtimeListener.reset; + gl.VueRealtimeListener.reset = () => { + gl.VueRealtimeListener.reset = stack; + removeAll(); + stack(); + }; }; + + // remove all event listeners and intervals + gl.VueRealtimeListener.reset = () => undefined; // noop })(window.gl || (window.gl = {})); diff --git a/app/models/namespace.rb b/app/models/namespace.rb index c5713fb7818..8352565da1c 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -7,6 +7,11 @@ class Namespace < ActiveRecord::Base include Gitlab::CurrentSettings include Routable + # Prevent users from creating unreasonably deep level of nesting. + # The number 20 was taken based on maximum nesting level of + # Android repo (15) + some extra backup. + NUMBER_OF_ANCESTORS_ALLOWED = 20 + cache_markdown_field :description, pipeline: :description has_many :projects, dependent: :destroy @@ -29,6 +34,8 @@ class Namespace < ActiveRecord::Base length: { maximum: 255 }, namespace: true + validate :nesting_level_allowed + delegate :name, to: :owner, allow_nil: true, prefix: true after_update :move_dir, if: :path_changed? @@ -194,7 +201,7 @@ class Namespace < ActiveRecord::Base # Scopes the model on ancestors of the record def ancestors if parent_id - path = route.path + path = route ? route.path : full_path paths = [] until path.blank? @@ -274,4 +281,10 @@ class Namespace < ActiveRecord::Base path_was end end + + def nesting_level_allowed + if ancestors.count > Group::NUMBER_OF_ANCESTORS_ALLOWED + errors.add(:parent_id, "has too deep level of nesting") + end + end end diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index d94f23f5a38..08cb8a04413 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -22,9 +22,7 @@ = link_to "View Open Merge Request", namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn' - elsif create_mr_button?(@repository.root_ref, @ref) .control - = link_to create_mr_path(@repository.root_ref, @ref), class: 'btn btn-success' do - = icon('plus') - Create Merge Request + = link_to "Create Merge Request", create_mr_path(@repository.root_ref, @ref), class: 'btn btn-success' .control = form_tag(namespace_project_commits_path(@project.namespace, @project, @id), method: :get, class: 'commits-search-form') do diff --git a/app/views/projects/compare/_form.html.haml b/app/views/projects/compare/_form.html.haml index d76d48187cd..08236216421 100644 --- a/app/views/projects/compare/_form.html.haml +++ b/app/views/projects/compare/_form.html.haml @@ -23,6 +23,4 @@ - if @merge_request.present? = link_to "View Open Merge Request", namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'prepend-left-10 btn' - elsif create_mr_button? - = link_to create_mr_path, class: 'prepend-left-10 btn' do - = icon("plus") - Create Merge Request + = link_to "Create Merge Request", create_mr_path, class: 'prepend-left-10 btn' diff --git a/changelogs/unreleased/20495-plus-icon-button.yml b/changelogs/unreleased/20495-plus-icon-button.yml new file mode 100644 index 00000000000..0f8650eb7b6 --- /dev/null +++ b/changelogs/unreleased/20495-plus-icon-button.yml @@ -0,0 +1,4 @@ +--- +title: Remove plus icon from MR button on compare view +merge_request: +author: diff --git a/changelogs/unreleased/23104-remove-public-param-for-projects.yml b/changelogs/unreleased/23104-remove-public-param-for-projects.yml new file mode 100644 index 00000000000..78eb785279f --- /dev/null +++ b/changelogs/unreleased/23104-remove-public-param-for-projects.yml @@ -0,0 +1,4 @@ +--- +title: 'API: remove `public` param for projects' +merge_request: 8736 +author: diff --git a/doc/api/projects.md b/doc/api/projects.md index 122075bbd11..040153ac880 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -642,7 +642,6 @@ Parameters: | `snippets_enabled` | boolean | no | Enable snippets for this project | | `container_registry_enabled` | boolean | no | Enable container registry for this project | | `shared_runners_enabled` | boolean | no | Enable shared runners for this project | -| `public` | boolean | no | If `true`, the same as setting `visibility_level` to 20 | | `visibility_level` | integer | no | See [project visibility level](#project-visibility-level) | | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members | @@ -676,7 +675,6 @@ Parameters: | `snippets_enabled` | boolean | no | Enable snippets for this project | | `container_registry_enabled` | boolean | no | Enable container registry for this project | | `shared_runners_enabled` | boolean | no | Enable shared runners for this project | -| `public` | boolean | no | If `true`, the same as setting `visibility_level` to 20 | | `visibility_level` | integer | no | See [project visibility level](#project-visibility-level) | | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members | @@ -709,7 +707,6 @@ Parameters: | `snippets_enabled` | boolean | no | Enable snippets for this project | | `container_registry_enabled` | boolean | no | Enable container registry for this project | | `shared_runners_enabled` | boolean | no | Enable shared runners for this project | -| `public` | boolean | no | If `true`, the same as setting `visibility_level` to 20 | | `visibility_level` | integer | no | See [project visibility level](#project-visibility-level) | | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members | diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 92a70faf1c2..bd4b23195ac 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -16,7 +16,6 @@ module API optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project' optional :container_registry_enabled, type: Boolean, desc: 'Flag indication if the container registry is enabled for that project' optional :lfs_enabled, type: Boolean, desc: 'Flag indication if Git LFS is enabled for that project' - optional :public, type: Boolean, desc: 'Create a public project. The same as visibility_level = 20.' optional :visibility_level, type: Integer, values: [ Gitlab::VisibilityLevel::PRIVATE, Gitlab::VisibilityLevel::INTERNAL, @@ -26,16 +25,6 @@ module API optional :only_allow_merge_if_build_succeeds, type: Boolean, desc: 'Only allow to merge if builds succeed' optional :only_allow_merge_if_all_discussions_are_resolved, type: Boolean, desc: 'Only allow to merge if all discussions are resolved' end - - def map_public_to_visibility_level(attrs) - publik = attrs.delete(:public) - if !publik.nil? && !attrs[:visibility_level].present? - # Since setting the public attribute to private could mean either - # private or internal, use the more conservative option, private. - attrs[:visibility_level] = (publik == true) ? Gitlab::VisibilityLevel::PUBLIC : Gitlab::VisibilityLevel::PRIVATE - end - attrs - end end resource :projects do @@ -161,7 +150,7 @@ module API use :create_params end post do - attrs = map_public_to_visibility_level(declared_params(include_missing: false)) + attrs = declared_params(include_missing: false) project = ::Projects::CreateService.new(current_user, attrs).execute if project.saved? @@ -190,7 +179,7 @@ module API user = User.find_by(id: params.delete(:user_id)) not_found!('User') unless user - attrs = map_public_to_visibility_level(declared_params(include_missing: false)) + attrs = declared_params(include_missing: false) project = ::Projects::CreateService.new(user, attrs).execute if project.saved? @@ -268,14 +257,14 @@ module API at_least_one_of :name, :description, :issues_enabled, :merge_requests_enabled, :wiki_enabled, :builds_enabled, :snippets_enabled, :shared_runners_enabled, :container_registry_enabled, - :lfs_enabled, :public, :visibility_level, :public_builds, + :lfs_enabled, :visibility_level, :public_builds, :request_access_enabled, :only_allow_merge_if_build_succeeds, :only_allow_merge_if_all_discussions_are_resolved, :path, :default_branch end put ':id' do authorize_admin_project - attrs = map_public_to_visibility_level(declared_params(include_missing: false)) + attrs = declared_params(include_missing: false) authorize! :rename_project, user_project if attrs[:name].present? authorize! :change_visibility_level, user_project if attrs[:visibility_level].present? diff --git a/spec/javascripts/commit/pipelines/pipelines_store_spec.js.es6 b/spec/javascripts/commit/pipelines/pipelines_store_spec.js.es6 index 559723bafcc..789f5dc9f49 100644 --- a/spec/javascripts/commit/pipelines/pipelines_store_spec.js.es6 +++ b/spec/javascripts/commit/pipelines/pipelines_store_spec.js.es6 @@ -7,6 +7,9 @@ describe('Store', () => { store = new gl.commits.pipelines.PipelinesStore(); }); + // unregister intervals and event handlers + afterEach(() => gl.VueRealtimeListener.reset()); + it('should start with a blank state', () => { expect(store.state.pipelines.length).toBe(0); }); diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 7bb1657bc3a..ec389f20d78 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -3,21 +3,32 @@ require 'spec_helper' describe Namespace, models: true do let!(:namespace) { create(:namespace) } - it { is_expected.to have_many :projects } - it { is_expected.to have_many :project_statistics } - it { is_expected.to belong_to :parent } - it { is_expected.to have_many :children } + describe 'associations' do + it { is_expected.to have_many :projects } + it { is_expected.to have_many :project_statistics } + it { is_expected.to belong_to :parent } + it { is_expected.to have_many :children } + end - it { is_expected.to validate_presence_of(:name) } - it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) } - it { is_expected.to validate_length_of(:name).is_at_most(255) } + describe 'validations' do + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_length_of(:description).is_at_most(255) } + it { is_expected.to validate_presence_of(:path) } + it { is_expected.to validate_length_of(:path).is_at_most(255) } + it { is_expected.to validate_presence_of(:owner) } - it { is_expected.to validate_length_of(:description).is_at_most(255) } + it 'does not allow too deep nesting' do + ancestors = (1..21).to_a + nested = build(:namespace, parent: namespace) - it { is_expected.to validate_presence_of(:path) } - it { is_expected.to validate_length_of(:path).is_at_most(255) } + allow(nested).to receive(:ancestors).and_return(ancestors) - it { is_expected.to validate_presence_of(:owner) } + expect(nested).not_to be_valid + expect(nested.errors[:parent_id].first).to eq('has too deep level of nesting') + end + end describe "Respond to" do it { is_expected.to respond_to(:human_name) } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 225e2e005df..ac0bbec44e0 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -359,13 +359,6 @@ describe API::Projects, api: true do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PUBLIC) end - it 'sets a project as public using :public' do - project = attributes_for(:project, { public: true }) - post api('/projects', user), project - expect(json_response['public']).to be_truthy - expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PUBLIC) - end - it 'sets a project as internal' do project = attributes_for(:project, :internal) post api('/projects', user), project @@ -373,13 +366,6 @@ describe API::Projects, api: true do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::INTERNAL) end - it 'sets a project as internal overriding :public' do - project = attributes_for(:project, :internal, { public: true }) - post api('/projects', user), project - expect(json_response['public']).to be_falsey - expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::INTERNAL) - end - it 'sets a project as private' do project = attributes_for(:project, :private) post api('/projects', user), project @@ -387,13 +373,6 @@ describe API::Projects, api: true do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE) end - it 'sets a project as private using :public' do - project = attributes_for(:project, { public: false }) - post api('/projects', user), project - expect(json_response['public']).to be_falsey - expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE) - end - it 'sets a project as allowing merge even if build fails' do project = attributes_for(:project, { only_allow_merge_if_build_succeeds: false }) post api('/projects', user), project @@ -431,13 +410,14 @@ describe API::Projects, api: true do end context 'when a visibility level is restricted' do + let(:project_param) { attributes_for(:project, :public) } + before do - @project = attributes_for(:project, { public: true }) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) end it 'does not allow a non-admin to use a restricted visibility level' do - post api('/projects', user), @project + post api('/projects', user), project_param expect(response).to have_http_status(400) expect(json_response['message']['visibility_level'].first).to( @@ -446,7 +426,8 @@ describe API::Projects, api: true do end it 'allows an admin to override restricted visibility settings' do - post api('/projects', admin), @project + post api('/projects', admin), project_param + expect(json_response['public']).to be_truthy expect(json_response['visibility_level']).to( eq(Gitlab::VisibilityLevel::PUBLIC) @@ -499,15 +480,6 @@ describe API::Projects, api: true do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PUBLIC) end - it 'sets a project as public using :public' do - project = attributes_for(:project, { public: true }) - post api("/projects/user/#{user.id}", admin), project - - expect(response).to have_http_status(201) - expect(json_response['public']).to be_truthy - expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PUBLIC) - end - it 'sets a project as internal' do project = attributes_for(:project, :internal) post api("/projects/user/#{user.id}", admin), project @@ -517,14 +489,6 @@ describe API::Projects, api: true do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::INTERNAL) end - it 'sets a project as internal overriding :public' do - project = attributes_for(:project, :internal, { public: true }) - post api("/projects/user/#{user.id}", admin), project - expect(response).to have_http_status(201) - expect(json_response['public']).to be_falsey - expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::INTERNAL) - end - it 'sets a project as private' do project = attributes_for(:project, :private) post api("/projects/user/#{user.id}", admin), project @@ -532,13 +496,6 @@ describe API::Projects, api: true do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE) end - it 'sets a project as private using :public' do - project = attributes_for(:project, { public: false }) - 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 'sets a project as allowing merge even if build fails' do project = attributes_for(:project, { only_allow_merge_if_build_succeeds: false }) post api("/projects/user/#{user.id}", admin), project @@ -865,7 +822,7 @@ describe API::Projects, api: true do it 'creates a new project snippet' do post api("/projects/#{project.id}/snippets", user), title: 'api test', file_name: 'sample.rb', code: 'test', - visibility_level: '0' + visibility_level: Gitlab::VisibilityLevel::PRIVATE expect(response).to have_http_status(201) expect(json_response['title']).to eq('api test') end @@ -1114,7 +1071,7 @@ describe API::Projects, api: true do end it 'updates visibility_level' do - project_param = { visibility_level: 20 } + project_param = { visibility_level: Gitlab::VisibilityLevel::PUBLIC } put api("/projects/#{project3.id}", user), project_param expect(response).to have_http_status(200) project_param.each_pair do |k, v| @@ -1124,7 +1081,7 @@ describe API::Projects, api: true do it 'updates visibility_level from public to private' do project3.update_attributes({ visibility_level: Gitlab::VisibilityLevel::PUBLIC }) - project_param = { public: false } + project_param = { visibility_level: Gitlab::VisibilityLevel::PRIVATE } put api("/projects/#{project3.id}", user), project_param expect(response).to have_http_status(200) project_param.each_pair do |k, v| @@ -1197,7 +1154,7 @@ describe API::Projects, api: true do end it 'does not update visibility_level' do - project_param = { visibility_level: 20 } + project_param = { visibility_level: Gitlab::VisibilityLevel::PUBLIC } put api("/projects/#{project3.id}", user4), project_param expect(response).to have_http_status(403) end |