diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-08 12:06:07 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-11-08 12:06:07 +0300 |
commit | d23b2a0871f3ca507aafa949e0314625f1f0c6a7 (patch) | |
tree | b1e26c7460bdae25f19103e14978a3aaeef52037 | |
parent | 1ef4b65f55f4fc6524a47050b4f6d686beb81d3a (diff) |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | .gitlab/CODEOWNERS | 2 | ||||
-rw-r--r-- | app/helpers/blob_helper.rb | 6 | ||||
-rw-r--r-- | app/helpers/snippets_helper.rb | 81 | ||||
-rw-r--r-- | app/models/group.rb | 4 | ||||
-rw-r--r-- | app/services/groups/transfer_service.rb | 2 | ||||
-rw-r--r-- | app/services/groups/update_service.rb | 5 | ||||
-rw-r--r-- | app/views/search/results/_snippet_blob.html.haml | 2 | ||||
-rw-r--r-- | app/views/search/results/_snippet_title.html.haml | 5 | ||||
-rw-r--r-- | app/views/shared/snippets/_blob.html.haml | 3 | ||||
-rw-r--r-- | app/views/shared/snippets/_embed.html.haml | 2 | ||||
-rw-r--r-- | app/views/shared/snippets/_header.html.haml | 2 | ||||
-rw-r--r-- | spec/finders/container_repositories_finder_spec.rb | 2 | ||||
-rw-r--r-- | spec/helpers/snippets_helper_spec.rb | 206 | ||||
-rw-r--r-- | spec/models/personal_snippet_spec.rb | 19 | ||||
-rw-r--r-- | spec/models/project_snippet_spec.rb | 21 | ||||
-rw-r--r-- | spec/models/snippet_spec.rb | 37 | ||||
-rw-r--r-- | spec/services/groups/transfer_service_spec.rb | 30 | ||||
-rw-r--r-- | spec/services/groups/update_service_spec.rb | 61 |
18 files changed, 365 insertions, 125 deletions
diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 394ecc18338..c8283326533 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -3,7 +3,7 @@ *.rake @gitlab-org/maintainers/rails-backend # Technical writing team are the default reviewers for everything in `doc/` -/doc/ @axil @marcia @eread @mikelewis +/doc/ @gl-docsteam # Frontend maintainers should see everything in `app/assets/` app/assets/ @gitlab-org/maintainers/frontend diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index d57bce0f401..912f0b61978 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -141,11 +141,7 @@ module BlobHelper if @build && @entry raw_project_job_artifacts_url(@project, @build, path: @entry.path, **kwargs) elsif @snippet - if @snippet.project_id - raw_project_snippet_url(@project, @snippet, **kwargs) - else - raw_snippet_url(@snippet, **kwargs) - end + reliable_raw_snippet_url(@snippet) elsif @blob project_raw_url(@project, @id, **kwargs) end diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index 6ccc1fb2ed1..10e31fb8888 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -11,22 +11,40 @@ module SnippetsHelper end end - def reliable_snippet_path(snippet, opts = nil) + def reliable_snippet_path(snippet, opts = {}) + reliable_snippet_url(snippet, opts.merge(only_path: true)) + end + + def reliable_raw_snippet_path(snippet, opts = {}) + reliable_raw_snippet_url(snippet, opts.merge(only_path: true)) + end + + def reliable_snippet_url(snippet, opts = {}) if snippet.project_id? - project_snippet_path(snippet.project, snippet, opts) + project_snippet_url(snippet.project, snippet, nil, opts) else - snippet_path(snippet, opts) + snippet_url(snippet, nil, opts) end end - def download_snippet_path(snippet) - if snippet.project_id - raw_project_snippet_path(@project, snippet, inline: false) + def reliable_raw_snippet_url(snippet, opts = {}) + if snippet.project_id? + raw_project_snippet_url(snippet.project, snippet, nil, opts) else - raw_snippet_path(snippet, inline: false) + raw_snippet_url(snippet, nil, opts) end end + def download_raw_snippet_button(snippet) + link_to(icon('download'), + reliable_raw_snippet_path(snippet, inline: false), + target: '_blank', + rel: 'noopener noreferrer', + class: "btn btn-sm has-tooltip", + title: 'Download', + data: { container: 'body' }) + end + # Return the path of a snippets index for a user or for a project # # @returns String, path to snippet index @@ -114,30 +132,45 @@ module SnippetsHelper { snippet_object: snippet, snippet_chunks: snippet_chunks } end - def snippet_embed - "<script src=\"#{url_for(only_path: false, overwrite_params: nil)}.js\"></script>" + def snippet_embed_tag(snippet) + content_tag(:script, nil, src: reliable_snippet_url(snippet, format: :js, only_path: false)) + end + + def snippet_badge(snippet) + return unless attrs = snippet_badge_attributes(snippet) + + css_class, text = attrs + tag.span(class: ['badge', 'badge-gray']) do + concat(tag.i(class: ['fa', css_class])) + concat(' ') + concat(text) + end + end + + def snippet_badge_attributes(snippet) + if snippet.private? + ['fa-lock', _('private')] + end end - def embedded_snippet_raw_button + def embedded_raw_snippet_button blob = @snippet.blob return if blob.empty? || blob.binary? || blob.stored_externally? - snippet_raw_url = if @snippet.is_a?(PersonalSnippet) - raw_snippet_url(@snippet) - else - raw_project_snippet_url(@snippet.project, @snippet) - end - - link_to external_snippet_icon('doc-code'), snippet_raw_url, class: 'btn', target: '_blank', rel: 'noopener noreferrer', title: 'Open raw' + link_to(external_snippet_icon('doc-code'), + reliable_raw_snippet_url(@snippet), + class: 'btn', + target: '_blank', + rel: 'noopener noreferrer', + title: 'Open raw') end def embedded_snippet_download_button - download_url = if @snippet.is_a?(PersonalSnippet) - raw_snippet_url(@snippet, inline: false) - else - raw_project_snippet_url(@snippet.project, @snippet, inline: false) - end - - link_to external_snippet_icon('download'), download_url, class: 'btn', target: '_blank', title: 'Download', rel: 'noopener noreferrer' + link_to(external_snippet_icon('download'), + reliable_raw_snippet_url(@snippet, inline: false), + class: 'btn', + target: '_blank', + title: 'Download', + rel: 'noopener noreferrer') end end diff --git a/app/models/group.rb b/app/models/group.rb index 27e4d709823..7760a3c69ce 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -263,8 +263,8 @@ class Group < Namespace members_with_parents.maintainers.exists?(user_id: user) end - def has_container_repositories? - container_repositories.exists? + def has_container_repository_including_subgroups? + ::ContainerRepository.for_group_and_its_subgroups(self).exists? end # @deprecated diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 14f5a605633..24813f6ddf9 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -75,7 +75,7 @@ module Groups # rubocop: enable CodeReuse/ActiveRecord def group_projects_contain_registry_images? - @group.has_container_repositories? + @group.has_container_repository_including_subgroups? end def update_group_attributes diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index be7502a193e..8635b82461b 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -43,8 +43,9 @@ module Groups def renaming_group_with_container_registry_images? new_path = params[:path] - new_path && new_path != group.path && - group.has_container_repositories? + new_path && + new_path != group.path && + group.has_container_repository_including_subgroups? end def container_images_error diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index f17dae0a94c..37f4efee9d2 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -1,6 +1,7 @@ - snippet_blob = chunk_snippet(snippet_blob, @search_term) - snippet = snippet_blob[:snippet_object] - snippet_chunks = snippet_blob[:snippet_chunks] +- snippet_path = reliable_snippet_path(snippet) .search-result-row %span @@ -11,7 +12,6 @@ = snippet.author_name %span.light= time_ago_with_tooltip(snippet.created_at) %h4.snippet-title - - snippet_path = reliable_snippet_path(snippet) .file-holder .js-file-title.file-title = link_to snippet_path do diff --git a/app/views/search/results/_snippet_title.html.haml b/app/views/search/results/_snippet_title.html.haml index 1e01088d9e6..7280146720e 100644 --- a/app/views/search/results/_snippet_title.html.haml +++ b/app/views/search/results/_snippet_title.html.haml @@ -2,10 +2,7 @@ %h4.snippet-title.term = link_to reliable_snippet_path(snippet_title) do = truncate(snippet_title.title, length: 60) - - if snippet_title.private? - %span.badge.badge-gray - %i.fa.fa-lock - = _("private") + = snippet_badge(snippet_title) %span.cgray.monospace.tiny.float-right.term = snippet_title.file_name diff --git a/app/views/shared/snippets/_blob.html.haml b/app/views/shared/snippets/_blob.html.haml index 2132fcbccc5..6a5e777706c 100644 --- a/app/views/shared/snippets/_blob.html.haml +++ b/app/views/shared/snippets/_blob.html.haml @@ -8,7 +8,6 @@ .btn-group{ role: "group" }< = copy_blob_source_button(blob) = open_raw_blob_button(blob) - - = link_to icon('download'), download_snippet_path(@snippet), target: '_blank', class: "btn btn-sm has-tooltip", title: 'Download', data: { container: 'body' } + = download_raw_snippet_button(@snippet) = render 'projects/blob/content', blob: blob diff --git a/app/views/shared/snippets/_embed.html.haml b/app/views/shared/snippets/_embed.html.haml index c7f0511d1de..d2e35511b32 100644 --- a/app/views/shared/snippets/_embed.html.haml +++ b/app/views/shared/snippets/_embed.html.haml @@ -17,7 +17,7 @@ .file-actions.d-none.d-sm-block .btn-group{ role: "group" }< - = embedded_snippet_raw_button + = embedded_raw_snippet_button = embedded_snippet_download_button %article.file-holder.snippet-file-content diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 8d94a87a775..67f177288f0 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -44,7 +44,7 @@ %li %button.js-share-btn.btn.btn-transparent{ type: 'button' } %strong.embed-toggle-list-item= _("Share") - %input.js-snippet-url-area.snippet-embed-input.form-control{ type: "text", autocomplete: 'off', value: snippet_embed } + %input.js-snippet-url-area.snippet-embed-input.form-control{ type: "text", autocomplete: 'off', value: snippet_embed_tag(@snippet) } .input-group-append = clipboard_button(title: _('Copy'), class: 'js-clipboard-btn snippet-clipboard-btn btn btn-default', target: '.js-snippet-url-area') .clearfix diff --git a/spec/finders/container_repositories_finder_spec.rb b/spec/finders/container_repositories_finder_spec.rb index 43b82faa9a3..08c241186d6 100644 --- a/spec/finders/container_repositories_finder_spec.rb +++ b/spec/finders/container_repositories_finder_spec.rb @@ -8,7 +8,7 @@ describe ContainerRepositoriesFinder do let(:group) { create(:group) } let(:project) { create(:project, group: group) } - let(:project_repository) { create(:container_repository, project: project) } + let!(:project_repository) { create(:container_repository, project: project) } before do group.add_reporter(reporter) diff --git a/spec/helpers/snippets_helper_spec.rb b/spec/helpers/snippets_helper_spec.rb index 66c8d576a4c..d88e151a11c 100644 --- a/spec/helpers/snippets_helper_spec.rb +++ b/spec/helpers/snippets_helper_spec.rb @@ -3,33 +3,217 @@ require 'spec_helper' describe SnippetsHelper do + include Gitlab::Routing include IconsHelper - describe '#embedded_snippet_raw_button' do - it 'gives view raw button of embedded snippets for project snippets' do - @snippet = create(:project_snippet, :public) + let_it_be(:public_personal_snippet) { create(:personal_snippet, :public) } + let_it_be(:public_project_snippet) { create(:project_snippet, :public) } + + describe '#reliable_snippet_path' do + subject { reliable_snippet_path(snippet) } + + context 'personal snippets' do + let(:snippet) { public_personal_snippet } + + context 'public' do + it 'returns a full path' do + expect(subject).to eq("/snippets/#{snippet.id}") + end + end + end + + context 'project snippets' do + let(:snippet) { public_project_snippet } + + it 'returns a full path' do + expect(subject).to eq("/#{snippet.project.full_path}/snippets/#{snippet.id}") + end + end + end + + describe '#reliable_snippet_url' do + subject { reliable_snippet_url(snippet) } + + context 'personal snippets' do + let(:snippet) { public_personal_snippet } + + context 'public' do + it 'returns a full url' do + expect(subject).to eq("http://test.host/snippets/#{snippet.id}") + end + end + end + + context 'project snippets' do + let(:snippet) { public_project_snippet } + + it 'returns a full url' do + expect(subject).to eq("http://test.host/#{snippet.project.full_path}/snippets/#{snippet.id}") + end + end + end + + describe '#reliable_raw_snippet_path' do + subject { reliable_raw_snippet_path(snippet) } + + context 'personal snippets' do + let(:snippet) { public_personal_snippet } - expect(embedded_snippet_raw_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" rel=\"noopener noreferrer\" title=\"Open raw\" href=\"#{raw_project_snippet_url(@snippet.project, @snippet)}\">#{external_snippet_icon('doc-code')}</a>") + context 'public' do + it 'returns a full path' do + expect(subject).to eq("/snippets/#{snippet.id}/raw") + end + end end - it 'gives view raw button of embedded snippets for personal snippets' do + context 'project snippets' do + let(:snippet) { public_project_snippet } + + it 'returns a full path' do + expect(subject).to eq("/#{snippet.project.full_path}/snippets/#{snippet.id}/raw") + end + end + end + + describe '#reliable_raw_snippet_url' do + subject { reliable_raw_snippet_url(snippet) } + + context 'personal snippets' do + let(:snippet) { public_personal_snippet } + + context 'public' do + it 'returns a full url' do + expect(subject).to eq("http://test.host/snippets/#{snippet.id}/raw") + end + end + end + + context 'project snippets' do + let(:snippet) { public_project_snippet } + + it 'returns a full url' do + expect(subject).to eq("http://test.host/#{snippet.project.full_path}/snippets/#{snippet.id}/raw") + end + end + end + + describe '#embedded_raw_snippet_button' do + subject { embedded_raw_snippet_button.to_s } + + it 'returns view raw button of embedded snippets for personal snippets' do @snippet = create(:personal_snippet, :public) - expect(embedded_snippet_raw_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" rel=\"noopener noreferrer\" title=\"Open raw\" href=\"#{raw_snippet_url(@snippet)}\">#{external_snippet_icon('doc-code')}</a>") + expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw")) + end + + it 'returns view raw button of embedded snippets for project snippets' do + @snippet = create(:project_snippet, :public) + + expect(subject).to eq(download_link("http://test.host/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw")) + end + + def download_link(url) + "<a class=\"btn\" target=\"_blank\" rel=\"noopener noreferrer\" title=\"Open raw\" href=\"#{url}\">#{external_snippet_icon('doc-code')}</a>" end end describe '#embedded_snippet_download_button' do - it 'gives download button of embedded snippets for project snippets' do + subject { embedded_snippet_download_button } + + it 'returns download button of embedded snippets for personal snippets' do + @snippet = create(:personal_snippet, :public) + + expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw")) + end + + it 'returns download button of embedded snippets for project snippets' do @snippet = create(:project_snippet, :public) - expect(embedded_snippet_download_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" title=\"Download\" rel=\"noopener noreferrer\" href=\"#{raw_project_snippet_url(@snippet.project, @snippet, inline: false)}\">#{external_snippet_icon('download')}</a>") + expect(subject).to eq(download_link("http://test.host/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw")) end - it 'gives download button of embedded snippets for personal snippets' do - @snippet = create(:personal_snippet, :public) + def download_link(url) + "<a class=\"btn\" target=\"_blank\" title=\"Download\" rel=\"noopener noreferrer\" href=\"#{url}?inline=false\">#{external_snippet_icon('download')}</a>" + end + end + + describe '#snippet_embed_tag' do + subject { snippet_embed_tag(snippet) } + + context 'personal snippets' do + let(:snippet) { public_personal_snippet } + + context 'public' do + it 'returns a script tag with the snippet full url' do + expect(subject).to eq(script_embed("http://test.host/snippets/#{snippet.id}")) + end + end + end + + context 'project snippets' do + let(:snippet) { public_project_snippet } + + it 'returns a script tag with the snippet full url' do + expect(subject).to eq(script_embed("http://test.host/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}")) + end + end + + def script_embed(url) + "<script src=\"#{url}.js\"></script>" + end + end + + describe '#download_raw_snippet_button' do + subject { download_raw_snippet_button(snippet) } + + context 'with personal snippet' do + let(:snippet) { public_personal_snippet } + + it 'returns the download button' do + expect(subject).to eq(download_link("/snippets/#{snippet.id}/raw")) + end + end + + context 'with project snippet' do + let(:snippet) { public_project_snippet } + + it 'returns the download button' do + expect(subject).to eq(download_link("/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}/raw")) + end + end + + def download_link(url) + "<a target=\"_blank\" rel=\"noopener noreferrer\" class=\"btn btn-sm has-tooltip\" title=\"Download\" data-container=\"body\" href=\"#{url}?inline=false\"><i aria-hidden=\"true\" data-hidden=\"true\" class=\"fa fa-download\"></i></a>" + end + end + + describe '#snippet_badge' do + let(:snippet) { build(:personal_snippet, visibility) } + + subject { snippet_badge(snippet) } + + context 'when snippet is private' do + let(:visibility) { :private } + + it 'returns the snippet badge' do + expect(subject).to eq "<span class=\"badge badge-gray\"><i class=\"fa fa-lock\"></i> private</span>" + end + end + + context 'when snippet is public' do + let(:visibility) { :public } + + it 'does not return anything' do + expect(subject).to be_nil + end + end + + context 'when snippet is internal' do + let(:visibility) { :internal } - expect(embedded_snippet_download_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" title=\"Download\" rel=\"noopener noreferrer\" href=\"#{raw_snippet_url(@snippet, inline: false)}\">#{external_snippet_icon('download')}</a>") + it 'does not return anything' do + expect(subject).to be_nil + end end end end diff --git a/spec/models/personal_snippet_spec.rb b/spec/models/personal_snippet_spec.rb new file mode 100644 index 00000000000..276c8e22731 --- /dev/null +++ b/spec/models/personal_snippet_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PersonalSnippet do + describe '#embeddable?' do + [ + { snippet: :public, embeddable: true }, + { snippet: :internal, embeddable: false }, + { snippet: :private, embeddable: false } + ].each do |combination| + it 'returns true when snippet is public' do + snippet = build(:personal_snippet, combination[:snippet]) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end +end diff --git a/spec/models/project_snippet_spec.rb b/spec/models/project_snippet_spec.rb index e87b4f41f4d..46025507cb5 100644 --- a/spec/models/project_snippet_spec.rb +++ b/spec/models/project_snippet_spec.rb @@ -10,4 +10,25 @@ describe ProjectSnippet do describe "Validation" do it { is_expected.to validate_presence_of(:project) } end + + describe '#embeddable?' do + [ + { project: :public, snippet: :public, embeddable: true }, + { project: :internal, snippet: :public, embeddable: false }, + { project: :private, snippet: :public, embeddable: false }, + { project: :public, snippet: :internal, embeddable: false }, + { project: :internal, snippet: :internal, embeddable: false }, + { project: :private, snippet: :internal, embeddable: false }, + { project: :public, snippet: :private, embeddable: false }, + { project: :internal, snippet: :private, embeddable: false }, + { project: :private, snippet: :private, embeddable: false } + ].each do |combination| + it 'only returns true when both project and snippet are public' do + project = create(:project, combination[:project]) + snippet = build(:project_snippet, combination[:snippet], project: project) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index f4dcbfbc190..e4cc8931840 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -451,41 +451,4 @@ describe Snippet do expect(blob.data).to eq(snippet.content) end end - - describe '#embeddable?' do - context 'project snippet' do - [ - { project: :public, snippet: :public, embeddable: true }, - { project: :internal, snippet: :public, embeddable: false }, - { project: :private, snippet: :public, embeddable: false }, - { project: :public, snippet: :internal, embeddable: false }, - { project: :internal, snippet: :internal, embeddable: false }, - { project: :private, snippet: :internal, embeddable: false }, - { project: :public, snippet: :private, embeddable: false }, - { project: :internal, snippet: :private, embeddable: false }, - { project: :private, snippet: :private, embeddable: false } - ].each do |combination| - it 'only returns true when both project and snippet are public' do - project = create(:project, combination[:project]) - snippet = create(:project_snippet, combination[:snippet], project: project) - - expect(snippet.embeddable?).to eq(combination[:embeddable]) - end - end - end - - context 'personal snippet' do - [ - { snippet: :public, embeddable: true }, - { snippet: :internal, embeddable: false }, - { snippet: :private, embeddable: false } - ].each do |combination| - it 'only returns true when snippet is public' do - snippet = create(:personal_snippet, combination[:snippet]) - - expect(snippet.embeddable?).to eq(combination[:embeddable]) - end - end - end - end end diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index 5ef1fb1932f..9a490dfd779 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -427,20 +427,34 @@ describe Groups::TransferService do end end - context 'when a project in group has container images' do + context 'when a project has container images' do let(:group) { create(:group, :public, :nested) } - let!(:project) { create(:project, :repository, :public, namespace: group) } + let!(:container_repository) { create(:container_repository, project: project) } + + subject { transfer_service.execute(new_parent_group) } before do - stub_container_registry_tags(repository: /image/, tags: %w[rc1]) - create(:container_repository, project: project, name: :image) - create(:group_member, :owner, group: new_parent_group, user: user) + group.add_owner(user) + new_parent_group.add_owner(user) end - it 'does not allow group to be transferred' do - transfer_service.execute(new_parent_group) + context 'within group' do + let(:project) { create(:project, :repository, :public, namespace: group) } + + it 'does not transfer' do + expect(subject).to be false + expect(transfer_service.error).to match(/Docker images in their Container Registry/) + end + end - expect(transfer_service.error).to match(/Docker images in their Container Registry/) + context 'within subgroup' do + let(:subgroup) { create(:group, parent: group) } + let(:project) { create(:project, :repository, :public, namespace: subgroup) } + + it 'does not transfer' do + expect(subject).to be false + expect(transfer_service.error).to match(/Docker images in their Container Registry/) + end end end end diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index ca8eaf4c970..1aa7e06182b 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -32,6 +32,43 @@ describe Groups::UpdateService do expect(service.execute).to be_falsey end + + context 'when a project has container images' do + let(:params) { { path: SecureRandom.hex } } + let!(:container_repository) { create(:container_repository, project: project) } + + subject { described_class.new(public_group, user, params).execute } + + context 'within group' do + let(:project) { create(:project, group: public_group) } + + context 'with path updates' do + it 'does not allow the update' do + expect(subject).to be false + expect(public_group.errors[:base].first).to match(/Docker images in their Container Registry/) + end + end + + context 'with name updates' do + let(:params) { { name: 'new-name' } } + + it 'allows the update' do + expect(subject).to be true + expect(public_group.reload.name).to eq('new-name') + end + end + end + + context 'within subgroup' do + let(:subgroup) { create(:group, parent: public_group) } + let(:project) { create(:project, group: subgroup) } + + it 'does not allow path updates' do + expect(subject).to be false + expect(public_group.errors[:base].first).to match(/Docker images in their Container Registry/) + end + end + end end context "internal group with internal project" do @@ -148,30 +185,6 @@ describe Groups::UpdateService do end end - context 'projects in group have container images' do - let(:service) { described_class.new(public_group, user, path: SecureRandom.hex) } - let(:project) { create(:project, :internal, group: public_group) } - - before do - stub_container_registry_tags(repository: /image/, tags: %w[rc1]) - create(:container_repository, project: project, name: :image) - end - - it 'does not allow path to be changed' do - result = described_class.new(public_group, user, path: 'new-path').execute - - expect(result).to eq false - expect(public_group.errors[:base].first).to match(/Docker images in their Container Registry/) - end - - it 'allows other settings to be changed' do - result = described_class.new(public_group, user, name: 'new-name').execute - - expect(result).to eq true - expect(public_group.reload.name).to eq('new-name') - end - end - context 'for a subgroup' do let(:subgroup) { create(:group, :private, parent: private_group) } |