diff options
21 files changed, 421 insertions, 92 deletions
diff --git a/app/assets/javascripts/pages/projects/project.js b/app/assets/javascripts/pages/projects/project.js index b99408e3609..435e8705803 100644 --- a/app/assets/javascripts/pages/projects/project.js +++ b/app/assets/javascripts/pages/projects/project.js @@ -1,9 +1,9 @@ -/* eslint-disable func-names, no-var, no-return-assign, vars-on-top */ +/* eslint-disable func-names, no-var, no-return-assign */ import $ from 'jquery'; import Cookies from 'js-cookie'; import { __ } from '~/locale'; -import { visitUrl, mergeUrlParams } from '~/lib/utils/url_utility'; +import { mergeUrlParams } from '~/lib/utils/url_utility'; import { serializeForm } from '~/lib/utils/forms'; import axios from '~/lib/utils/axios_utils'; import flash from '~/flash'; @@ -105,6 +105,10 @@ export default class Project { var selected = $dropdown.data('selected'); var fieldName = $dropdown.data('fieldName'); var shouldVisit = Boolean($dropdown.data('visit')); + var $form = $dropdown.closest('form'); + var action = $form.attr('action'); + var linkTarget = mergeUrlParams(serializeForm($form[0]), action); + return $dropdown.glDropdown({ data(term, callback) { axios @@ -126,21 +130,18 @@ export default class Project { renderRow(ref) { var li = refListItem.cloneNode(false); - if (ref.header != null) { - li.className = 'dropdown-header'; - li.textContent = ref.header; - } else { - var link = refLink.cloneNode(false); - - if (ref === selected) { - link.className = 'is-active'; - } - - link.textContent = ref; - link.dataset.ref = ref; + var link = refLink.cloneNode(false); - li.appendChild(link); + if (ref === selected) { + link.className = 'is-active'; } + link.textContent = ref; + link.dataset.ref = ref; + if (ref.length > 0 && shouldVisit) { + link.href = mergeUrlParams({ [fieldName]: ref }, linkTarget); + } + + li.appendChild(link); return li; }, @@ -152,15 +153,11 @@ export default class Project { }, clicked(options) { const { e } = options; - e.preventDefault(); - if ($(`input[name="${fieldName}"]`).length) { - var $form = $dropdown.closest('form'); - var action = $form.attr('action'); - - if (shouldVisit) { - visitUrl(mergeUrlParams(serializeForm($form[0]), action)); - } + if (!shouldVisit) { + e.preventDefault(); } + /* The actual process is removed since `link.href` in `RenderRow` contains the full target. + * It makes the visitable link can be visited when opening on a new tab of browser */ }, }); }); diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index 514b03e23b5..f13fb4d0b3d 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -73,6 +73,10 @@ class Projects::DeployKeysController < Projects::ApplicationController @deploy_key ||= DeployKey.find(params[:id]) end + def deploy_keys_project + @deploy_keys_project ||= deploy_key.deploy_keys_project_for(@project) + end + def create_params create_params = params.require(:deploy_key) .permit(:key, :title, deploy_keys_projects_attributes: [:can_push]) @@ -81,10 +85,16 @@ class Projects::DeployKeysController < Projects::ApplicationController end def update_params - params.require(:deploy_key).permit(:title, deploy_keys_projects_attributes: [:id, :can_push]) + permitted_params = [deploy_keys_projects_attributes: [:id, :can_push]] + permitted_params << :title if can?(current_user, :update_deploy_key, deploy_key) + + params.require(:deploy_key).permit(*permitted_params) end def authorize_update_deploy_key! - access_denied! unless can?(current_user, :update_deploy_key, deploy_key) + if !can?(current_user, :update_deploy_key, deploy_key) && + !can?(current_user, :update_deploy_keys_project, deploy_keys_project) + access_denied! + end end end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 4e693a94c6b..b6c0976cd43 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -33,8 +33,6 @@ class Snippet < ApplicationRecord default_content_html_invalidator || file_name_changed? end - default_value_for(:visibility_level) { Gitlab::CurrentSettings.default_snippet_visibility } - belongs_to :author, class_name: 'User' belongs_to :project @@ -139,6 +137,24 @@ class Snippet < ApplicationRecord @link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/) end + def initialize(attributes = {}) + # We can't use default_value_for because the database has a default + # value of 0 for visibility_level. If someone attempts to create a + # private snippet, default_value_for will assume that the + # visibility_level hasn't changed and will use the application + # setting default, which could be internal or public. + # + # To fix the problem, we assign the actual snippet default if no + # explicit visibility has been initialized. + attributes ||= {} + + unless visibility_attribute_present?(attributes) + attributes[:visibility_level] = Gitlab::CurrentSettings.default_snippet_visibility + end + + super + end + def to_reference(from = nil, full: false) reference = "#{self.class.reference_prefix}#{id}" diff --git a/app/policies/deploy_keys_project_policy.rb b/app/policies/deploy_keys_project_policy.rb new file mode 100644 index 00000000000..368377048a4 --- /dev/null +++ b/app/policies/deploy_keys_project_policy.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class DeployKeysProjectPolicy < BasePolicy + delegate { @subject.project } + + with_options scope: :subject, score: 0 + condition(:public_deploy_key) { @subject.deploy_key.public? } + + rule { public_deploy_key & can?(:admin_project) }.enable :update_deploy_keys_project +end diff --git a/app/presenters/projects/settings/deploy_keys_presenter.rb b/app/presenters/projects/settings/deploy_keys_presenter.rb index 6f8c4e1f902..9bb7fe13593 100644 --- a/app/presenters/projects/settings/deploy_keys_presenter.rb +++ b/app/presenters/projects/settings/deploy_keys_presenter.rb @@ -40,7 +40,7 @@ module Projects def as_json serializer = DeployKeySerializer.new # rubocop: disable CodeReuse/Serializer - opts = { user: current_user } + opts = { user: current_user, project: project } { enabled_keys: serializer.represent(enabled_keys.with_projects, opts), diff --git a/app/serializers/deploy_key_entity.rb b/app/serializers/deploy_key_entity.rb index e47d6454780..9a558d12bec 100644 --- a/app/serializers/deploy_key_entity.rb +++ b/app/serializers/deploy_key_entity.rb @@ -20,6 +20,7 @@ class DeployKeyEntity < Grape::Entity private def can_edit - Ability.allowed?(options[:user], :update_deploy_key, object) + Ability.allowed?(options[:user], :update_deploy_key, object) || + Ability.allowed?(options[:user], :update_deploy_keys_project, object.deploy_keys_project_for(options[:project])) end end diff --git a/app/views/shared/deploy_keys/_form.html.haml b/app/views/shared/deploy_keys/_form.html.haml index bc0dc7f9631..1944c293be1 100644 --- a/app/views/shared/deploy_keys/_form.html.haml +++ b/app/views/shared/deploy_keys/_form.html.haml @@ -6,7 +6,7 @@ .form-group = form.label :title, class: 'col-form-label col-sm-2' - .col-sm-10= form.text_field :title, class: 'form-control' + .col-sm-10= form.text_field :title, class: 'form-control', readonly: ('readonly' unless can?(current_user, :update_deploy_key, deploy_key)) .form-group - if deploy_key.new_record? diff --git a/changelogs/unreleased/12819-remove-feature-flag.yml b/changelogs/unreleased/12819-remove-feature-flag.yml new file mode 100644 index 00000000000..0096a0d00ac --- /dev/null +++ b/changelogs/unreleased/12819-remove-feature-flag.yml @@ -0,0 +1,5 @@ +--- +title: Fix pod logs failure when pod contains more than 1 container +merge_request: 18574 +author: +type: fixed diff --git a/changelogs/unreleased/23079-write-permission-global-deploy-keys.yml b/changelogs/unreleased/23079-write-permission-global-deploy-keys.yml new file mode 100644 index 00000000000..04f9dfc9043 --- /dev/null +++ b/changelogs/unreleased/23079-write-permission-global-deploy-keys.yml @@ -0,0 +1,5 @@ +--- +title: Allow maintainers to toggle write permission for public deploy keys +merge_request: 17210 +author: +type: fixed diff --git a/changelogs/unreleased/64837-persist-refs-over-browser-tabs.yml b/changelogs/unreleased/64837-persist-refs-over-browser-tabs.yml new file mode 100644 index 00000000000..68042383ed6 --- /dev/null +++ b/changelogs/unreleased/64837-persist-refs-over-browser-tabs.yml @@ -0,0 +1,5 @@ +--- +title: persist the refs when open the link of refs in a new tab of browser +merge_request: 31998 +author: minghuan lei +type: added diff --git a/changelogs/unreleased/sh-fix-snippet-visibility-api.yml b/changelogs/unreleased/sh-fix-snippet-visibility-api.yml new file mode 100644 index 00000000000..837da277179 --- /dev/null +++ b/changelogs/unreleased/sh-fix-snippet-visibility-api.yml @@ -0,0 +1,5 @@ +--- +title: Fix inability to set snippet visibility via API +merge_request: 18612 +author: +type: fixed diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index ea4acf7083a..a6d3c008686 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -232,7 +232,7 @@ bundle exec rake gitlab:graphql:compile_docs In its current state, the rake task: - Generates output for GraphQL objects. -- Places the output at `docs/api/graphql/reference/index.md`. +- Places the output at `doc/api/graphql/reference/index.md`. This uses some features from `graphql-docs` gem like its schema parser and helper methods. The docs generator code comes from our side giving us more flexibility, like using Haml templates and generating Markdown files. @@ -241,5 +241,5 @@ To edit the template used, please take a look at `lib/gitlab/graphql/docs/templa The actual renderer is at `Gitlab::Graphql::Docs::Renderer`. `@parsed_schema` is an instance variable that the `graphql-docs` gem expects to have available. -`Gitlab::Graphql::Docs::Helper` defines the `object` method we currently use. This is also where you should implement any -new methods for new types you'd like to display. +`Gitlab::Graphql::Docs::Helper` defines the `object` method we currently use. This is also where you +should implement any new methods for new types you'd like to display. diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index df6d2721977..e86bcc19b2b 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -115,14 +115,20 @@ module API put ":id/deploy_keys/:key_id" do deploy_keys_project = find_by_deploy_key(user_project, params[:key_id]) - authorize!(:update_deploy_key, deploy_keys_project.deploy_key) + if !can?(current_user, :update_deploy_key, deploy_keys_project.deploy_key) && + !can?(current_user, :update_deploy_keys_project, deploy_keys_project) + forbidden!(nil) + end + + update_params = {} + update_params[:can_push] = params[:can_push] if params.key?(:can_push) + update_params[:deploy_key_attributes] = { id: params[:key_id] } - can_push = params[:can_push].nil? ? deploy_keys_project.can_push : params[:can_push] - title = params[:title] || deploy_keys_project.deploy_key.title + if can?(current_user, :update_deploy_key, deploy_keys_project.deploy_key) + update_params[:deploy_key_attributes][:title] = params[:title] if params.key?(:title) + end - result = deploy_keys_project.update(can_push: can_push, - deploy_key_attributes: { id: params[:key_id], - title: title }) + result = deploy_keys_project.update(update_params) if result present deploy_keys_project, with: Entities::DeployKeysProject diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 01b42eddb28..dae5327c398 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2376,21 +2376,15 @@ msgstr "" msgid "Billing" msgstr "" -msgid "BillingPlans|%{group_name} is currently using the %{plan_link} plan." +msgid "BillingPlans|%{group_name} is currently using the %{plan_name} plan." msgstr "" -msgid "BillingPlans|@%{user_name} you are currently using the %{plan_link} plan." +msgid "BillingPlans|@%{user_name} you are currently using the %{plan_name} plan." msgstr "" msgid "BillingPlans|Congratulations, your new trial is activated" msgstr "" -msgid "BillingPlans|Current plan" -msgstr "" - -msgid "BillingPlans|Downgrade" -msgstr "" - msgid "BillingPlans|If you would like to downgrade your plan please contact %{support_link_start}Customer Support%{support_link_end}." msgstr "" @@ -2415,15 +2409,15 @@ msgstr "" msgid "BillingPlans|To manage the plan for this group, visit the billing section of %{parent_billing_page_link}." msgstr "" -msgid "BillingPlans|Upgrade" -msgstr "" - msgid "BillingPlans|Your GitLab.com trial expired on %{expiration_date}. %{learn_more_text}" msgstr "" msgid "BillingPlans|Your GitLab.com trial will <strong>expire after %{expiration_date}</strong>. You can learn more about GitLab.com Gold by reading about our %{features_link}." msgstr "" +msgid "BillingPlans|billed annually at %{price_per_year}" +msgstr "" + msgid "BillingPlans|features" msgstr "" @@ -2433,13 +2427,10 @@ msgstr "" msgid "BillingPlans|monthly" msgstr "" -msgid "BillingPlans|paid annually at %{price_per_year}" -msgstr "" - msgid "BillingPlans|per user" msgstr "" -msgid "BillingPlan|Upgrade plan" +msgid "BillingPlan|Upgrade" msgstr "" msgid "Bitbucket Server Import" @@ -4798,6 +4789,9 @@ msgstr "" msgid "Current Branch" msgstr "" +msgid "Current Plan" +msgstr "" + msgid "Current Project" msgstr "" diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index ccad76eaddd..8b1ca2efab2 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' describe Projects::DeployKeysController do let(:project) { create(:project, :repository) } let(:user) { create(:user) } + let(:admin) { create(:admin) } before do project.add_maintainer(user) @@ -37,7 +38,7 @@ describe Projects::DeployKeysController do create(:deploy_keys_project, project: project2, deploy_key: deploy_key_internal) end - let!(:deploy_keys_actual_project) do + let!(:deploy_keys_project_actual) do create(:deploy_keys_project, project: project, deploy_key: deploy_key_actual) end @@ -154,7 +155,7 @@ describe Projects::DeployKeysController do context 'with admin' do before do - sign_in(create(:admin)) + sign_in(admin) end it 'returns 302' do @@ -219,7 +220,7 @@ describe Projects::DeployKeysController do context 'with admin' do before do - sign_in(create(:admin)) + sign_in(admin) end it 'returns 302' do @@ -234,4 +235,80 @@ describe Projects::DeployKeysController do end end end + + describe 'PUT update' do + let(:extra_params) { {} } + + subject do + put :update, params: extra_params.reverse_merge(id: deploy_key.id, + namespace_id: project.namespace, + project_id: project) + end + + def deploy_key_params(title, can_push) + deploy_keys_projects_attributes = { '0' => { id: deploy_keys_project, can_push: can_push } } + { deploy_key: { title: title, deploy_keys_projects_attributes: deploy_keys_projects_attributes } } + end + + let(:deploy_key) { create(:deploy_key, public: true) } + let(:project) { create(:project) } + let!(:deploy_keys_project) do + create(:deploy_keys_project, project: project, deploy_key: deploy_key) + end + + context 'with project maintainer' do + before do + project.add_maintainer(user) + end + + context 'public deploy key attached to project' do + let(:extra_params) { deploy_key_params('updated title', '1') } + + it 'does not update the title of the deploy key' do + expect { subject }.not_to change { deploy_key.reload.title } + end + + it 'updates can_push of deploy_keys_project' do + expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) + end + end + end + + context 'with admin' do + before do + sign_in(admin) + end + + context 'public deploy key attached to project' do + let(:extra_params) { deploy_key_params('updated title', '1') } + + it 'updates the title of the deploy key' do + expect { subject }.to change { deploy_key.reload.title }.to('updated title') + end + + it 'updates can_push of deploy_keys_project' do + expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) + end + end + end + + context 'with admin as project maintainer' do + before do + sign_in(admin) + project.add_maintainer(admin) + end + + context 'public deploy key attached to project' do + let(:extra_params) { deploy_key_params('updated title', '1') } + + it 'updates the title of the deploy key' do + expect { subject }.to change { deploy_key.reload.title }.to('updated title') + end + + it 'updates can_push of deploy_keys_project' do + expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) + end + end + end + end end diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 1294c8822b6..18031a40f15 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -66,6 +66,19 @@ describe 'Projects > Settings > Repository settings' do expect(page).to have_content('Write access allowed') end + it 'edit an existing public deploy key to be writable' do + project.deploy_keys << public_deploy_key + visit project_settings_repository_path(project) + + find('.deploy-key', text: public_deploy_key.title).find('.ic-pencil').click + + check 'deploy_key_deploy_keys_projects_attributes_0_can_push' + click_button 'Save changes' + + expect(page).to have_content('public_deploy_key') + expect(page).to have_content('Write access allowed') + end + it 'edit a deploy key from projects user has access to' do project2 = create(:project_empty_repo) project2.add_role(user, role) diff --git a/spec/features/search/user_searches_for_code_spec.rb b/spec/features/search/user_searches_for_code_spec.rb index 9451ee6eb15..9949595fddf 100644 --- a/spec/features/search/user_searches_for_code_spec.rb +++ b/spec/features/search/user_searches_for_code_spec.rb @@ -94,6 +94,13 @@ describe 'User searches for code' do expect(page).to have_selector('.results', text: 'path = gitlab-grack') end + + it 'persist refs over browser tabs' do + ref = 'feature' + find('.js-project-refs-dropdown').click + link = find_link(ref)[:href] + expect(link.include?("repository_ref=" + ref)).to be(true) + end end it 'no ref switcher shown in issue result summary', :js do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 3524cdae3b8..15dcccb37d9 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -133,6 +133,32 @@ describe Snippet do end end + describe 'when default snippet visibility set to internal' do + using RSpec::Parameterized::TableSyntax + + before do + stub_application_setting(default_snippet_visibility: Gitlab::VisibilityLevel::INTERNAL) + end + + where(:attribute_name, :value) do + :visibility | 'private' + :visibility_level | Gitlab::VisibilityLevel::PRIVATE + 'visibility' | 'private' + 'visibility_level' | Gitlab::VisibilityLevel::PRIVATE + end + + with_them do + it 'sets the visibility level' do + snippet = described_class.new(attribute_name => value, title: 'test', file_name: 'test.rb', content: 'test data') + + expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + expect(snippet.title).to eq('test') + expect(snippet.file_name).to eq('test.rb') + expect(snippet.content).to eq('test data') + end + end + end + describe '.with_optional_visibility' do context 'when a visibility level is provided' do it 'returns snippets with the given visibility' do diff --git a/spec/policies/deploy_keys_project_policy_spec.rb b/spec/policies/deploy_keys_project_policy_spec.rb new file mode 100644 index 00000000000..952da86b7a7 --- /dev/null +++ b/spec/policies/deploy_keys_project_policy_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DeployKeysProjectPolicy do + subject { described_class.new(current_user, deploy_key.deploy_keys_project_for(project)) } + + describe 'updating a deploy_keys_project' do + context 'when a project maintainer' do + let(:current_user) { create(:user) } + + context 'tries to update private deploy key attached to project' do + let(:deploy_key) { create(:deploy_key, public: false) } + let(:project) { create(:project_empty_repo) } + + before do + project.add_maintainer(current_user) + project.deploy_keys << deploy_key + end + + it { is_expected.to be_disallowed(:update_deploy_keys_project) } + end + + context 'tries to update public deploy key attached to project' do + let(:deploy_key) { create(:deploy_key, public: true) } + let(:project) { create(:project_empty_repo) } + + before do + project.add_maintainer(current_user) + project.deploy_keys << deploy_key + end + + it { is_expected.to be_allowed(:update_deploy_keys_project) } + end + end + + context 'when a non-maintainer project member' do + let(:current_user) { create(:user) } + let(:project) { create(:project_empty_repo) } + + before do + project.add_developer(current_user) + project.deploy_keys << deploy_key + end + + context 'tries to update private deploy key attached to project' do + let(:deploy_key) { create(:deploy_key, public: false) } + + it { is_expected.to be_disallowed(:update_deploy_keys_project) } + end + + context 'tries to update public deploy key attached to project' do + let(:deploy_key) { create(:deploy_key, public: true) } + + it { is_expected.to be_disallowed(:update_deploy_keys_project) } + end + end + + context 'when a user is not a project member' do + let(:current_user) { create(:user) } + let(:project) { create(:project_empty_repo) } + let(:deploy_key) { create(:deploy_key, public: true) } + + before do + project.deploy_keys << deploy_key + end + + context 'tries to update public deploy key attached to project' do + it { is_expected.to be_disallowed(:update_deploy_keys_project) } + end + end + end +end diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index b93ee148736..e0cc18abcca 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe API::DeployKeys do let(:user) { create(:user) } + let(:maintainer) { create(:user) } let(:admin) { create(:admin) } let(:project) { create(:project, creator_id: user.id) } let(:project2) { create(:project, creator_id: user.id) } @@ -124,45 +125,109 @@ describe API::DeployKeys do end describe 'PUT /projects/:id/deploy_keys/:key_id' do - let(:private_deploy_key) { create(:another_deploy_key, public: false) } - let(:project_private_deploy_key) do - create(:deploy_keys_project, project: project, deploy_key: private_deploy_key) + let(:extra_params) { {} } + + subject do + put api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", api_user), params: extra_params end - it 'updates a public deploy key as admin' do - expect do - put api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin), params: { title: 'new title' } - end.not_to change(deploy_key, :title) + context 'with non-admin' do + let(:api_user) { user } - expect(response).to have_gitlab_http_status(200) + it 'does not update a public deploy key' do + expect { subject }.not_to change(deploy_key, :title) + + expect(response).to have_gitlab_http_status(404) + end end - it 'does not update a public deploy key as non admin' do - expect do - put api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", user), params: { title: 'new title' } - end.not_to change(deploy_key, :title) + context 'with admin' do + let(:api_user) { admin } - expect(response).to have_gitlab_http_status(404) + context 'public deploy key attached to project' do + let(:extra_params) { { title: 'new title', can_push: true } } + + it 'updates the title of the deploy key' do + expect { subject }.to change { deploy_key.reload.title }.to 'new title' + expect(response).to have_gitlab_http_status(200) + end + + it 'updates can_push of deploy_keys_project' do + expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) + expect(response).to have_gitlab_http_status(200) + end + end + + context 'private deploy key' do + let(:deploy_key) { create(:another_deploy_key, public: false) } + let(:deploy_keys_project) do + create(:deploy_keys_project, project: project, deploy_key: deploy_key) + end + let(:extra_params) { { title: 'new title', can_push: true } } + + it 'updates the title of the deploy key' do + expect { subject }.to change { deploy_key.reload.title }.to 'new title' + expect(response).to have_gitlab_http_status(200) + end + + it 'updates can_push of deploy_keys_project' do + expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) + expect(response).to have_gitlab_http_status(200) + end + + context 'invalid title' do + let(:extra_params) { { title: '' } } + + it 'does not update the title of the deploy key' do + expect { subject }.not_to change { deploy_key.reload.title } + expect(response).to have_gitlab_http_status(400) + end + end + end end - it 'does not update a private key with invalid title' do - project_private_deploy_key + context 'with admin as project maintainer' do + let(:api_user) { admin } - expect do - put api("/projects/#{project.id}/deploy_keys/#{private_deploy_key.id}", admin), params: { title: '' } - end.not_to change(deploy_key, :title) + before do + project.add_maintainer(admin) + end - expect(response).to have_gitlab_http_status(400) + context 'public deploy key attached to project' do + let(:extra_params) { { title: 'new title', can_push: true } } + + it 'updates the title of the deploy key' do + expect { subject }.to change { deploy_key.reload.title }.to 'new title' + expect(response).to have_gitlab_http_status(200) + end + + it 'updates can_push of deploy_keys_project' do + expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) + expect(response).to have_gitlab_http_status(200) + end + end end - it 'updates a private ssh key with correct attributes' do - project_private_deploy_key + context 'with maintainer' do + let(:api_user) { maintainer } - put api("/projects/#{project.id}/deploy_keys/#{private_deploy_key.id}", admin), params: { title: 'new title', can_push: true } + before do + project.add_maintainer(maintainer) + end - expect(json_response['id']).to eq(private_deploy_key.id) - expect(json_response['title']).to eq('new title') - expect(json_response['can_push']).to eq(true) + context 'public deploy key attached to project' do + let(:extra_params) { { title: 'new title', can_push: true } } + + it 'does not update the title of the deploy key' do + expect { subject }.not_to change { deploy_key.reload.title } + expect(response).to have_gitlab_http_status(200) + end + + it 'updates can_push of deploy_keys_project' do + expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true) + expect(response).to have_gitlab_http_status(200) + end + end end end diff --git a/spec/serializers/deploy_key_entity_spec.rb b/spec/serializers/deploy_key_entity_spec.rb index 9e76d36c302..607adfc2488 100644 --- a/spec/serializers/deploy_key_entity_spec.rb +++ b/spec/serializers/deploy_key_entity_spec.rb @@ -8,14 +8,15 @@ describe DeployKeyEntity do let(:user) { create(:user) } let(:project) { create(:project, :internal)} let(:project_private) { create(:project, :private)} - let!(:project_pending_delete) { create(:project, :internal, pending_delete: true) } let(:deploy_key) { create(:deploy_key) } - let!(:deploy_key_internal) { create(:deploy_keys_project, project: project, deploy_key: deploy_key) } - let!(:deploy_key_private) { create(:deploy_keys_project, project: project_private, deploy_key: deploy_key) } - let!(:deploy_key_pending_delete) { create(:deploy_keys_project, project: project_pending_delete, deploy_key: deploy_key) } let(:entity) { described_class.new(deploy_key, user: user) } + before do + project.deploy_keys << deploy_key + project_private.deploy_keys << deploy_key + end + describe 'returns deploy keys with projects a user can read' do let(:expected_result) do { @@ -46,17 +47,30 @@ describe DeployKeyEntity do it { expect(entity.as_json).to eq(expected_result) } end - describe 'returns can_edit true if user is a maintainer of project' do + context 'user is an admin' do + let(:user) { create(:user, :admin) } + + it { expect(entity.as_json).to include(can_edit: true) } + end + + context 'user is a project maintainer' do before do project.add_maintainer(user) end - it { expect(entity.as_json).to include(can_edit: true) } - end + context 'project deploy key' do + it { expect(entity.as_json).to include(can_edit: true) } + end - describe 'returns can_edit true if a user admin' do - let(:user) { create(:user, :admin) } + context 'public deploy key' do + let(:deploy_key_public) { create(:deploy_key, public: true) } + let(:entity_public) { described_class.new(deploy_key_public, { user: user, project: project }) } - it { expect(entity.as_json).to include(can_edit: true) } + before do + project.deploy_keys << deploy_key_public + end + + it { expect(entity_public.as_json).to include(can_edit: true) } + end end end |