diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-15 03:07:23 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-15 03:07:23 +0300 |
commit | afd8f58f2d0d42d21496fe4652c1664add9b68b7 (patch) | |
tree | 38741f62cac6fafb42d30632596db951ec955850 /spec | |
parent | beabc7d164276a8bb35c2b497a0c4dc0dc824e3c (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
13 files changed, 381 insertions, 38 deletions
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 2241a7cf590..4a5283f1127 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -775,32 +775,40 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : end describe 'PUT update' do + let(:note_params) { { note: "New comment" } } + let(:request_params) do { namespace_id: project.namespace, project_id: project, id: note, format: :json, - note: { - note: "New comment" - } + note: note_params } end - specify { expect(put(:update, params: request_params)).to have_request_urgency(:low) } + subject(:update_note) { put :update, params: request_params } - context "should update the note with a valid issue" do - before do - sign_in(note.author) - project.add_developer(note.author) - end + before do + sign_in(note.author) + project.add_developer(note.author) + end + + specify { expect(update_note).to have_request_urgency(:low) } + context "when the note is valid" do it "updates the note" do - expect { put :update, params: request_params }.to change { note.reload.note } + expect { update_note }.to change { note.reload.note } + end + + it "returns status 200" do + update_note + + expect(response).to have_gitlab_http_status(:ok) end end - context "doesnt update the note" do + context "when the issue is confidential and the user has guest permissions" do let(:issue) { create(:issue, :confidential, project: project) } let(:note) { create(:note, noteable: issue, project: project) } @@ -809,20 +817,38 @@ RSpec.describe Projects::NotesController, type: :controller, feature_category: : project.add_guest(user) end - it "disallows edits when the issue is confidential and the user has guest permissions" do - request_params = { - namespace_id: project.namespace, - project_id: project, - id: note, - format: :json, - note: { - note: "New comment" - } - } - expect { put :update, params: request_params }.not_to change { note.reload.note } + it "disallows edits" do + expect { update_note }.not_to change { note.reload.note } + end + + it "returns status 404" do + update_note + expect(response).to have_gitlab_http_status(:not_found) end end + + context "when there are ActiveRecord validation errors" do + before do + allow(note).to receive_message_chain(:errors, :full_messages) + .and_return(['Error 1', 'Error 2']) + + allow_next_instance_of(Notes::UpdateService) do |service| + allow(service).to receive(:execute).and_return(note) + end + end + + it "does not update the note" do + expect { update_note }.not_to change { note.reload.note } + end + + it "returns status 422", :aggregate_failures do + update_note + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('{"errors":"Error 1 and Error 2"}') + end + end end describe 'DELETE destroy' do diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 00d99b46d0b..578973d5b3d 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Snippets::NotesController do +RSpec.describe Snippets::NotesController, feature_category: :team_planning do let(:user) { create(:user) } let(:private_snippet) { create(:personal_snippet, :private) } @@ -256,6 +256,59 @@ RSpec.describe Snippets::NotesController do end end + describe 'PUT update' do + let(:note_params) { { note: "New comment" } } + + let(:request_params) do + { + snippet_id: public_snippet, + id: note_on_public, + format: :json, + note: note_params + } + end + + before do + sign_in(note_on_public.author) + end + + subject(:update_note) { put :update, params: request_params } + + context "when the note is valid" do + it "updates the note" do + expect { update_note }.to change { note_on_public.reload.note } + end + + it "returns status 200" do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context "when there are ActiveRecord validation errors" do + before do + allow(note_on_public).to receive_message_chain(:errors, :full_messages) + .and_return(['Error 1', 'Error 2']) + + allow_next_instance_of(Notes::UpdateService) do |service| + allow(service).to receive(:execute).and_return(note_on_public) + end + end + + it "does not update the note" do + expect { update_note }.not_to change { note_on_public.reload.note } + end + + it "returns status 422", :aggregate_failures do + update_note + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('{"errors":"Error 1 and Error 2"}') + end + end + end + describe 'DELETE destroy' do let(:request_params) do { diff --git a/spec/finders/groups/environment_scopes_finder_spec.rb b/spec/finders/groups/environment_scopes_finder_spec.rb new file mode 100644 index 00000000000..dfa32725e4a --- /dev/null +++ b/spec/finders/groups/environment_scopes_finder_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::EnvironmentScopesFinder, feature_category: :secrets_management do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group, :public) } + + let!(:environment1) { create(:ci_group_variable, group: group, key: 'var1', environment_scope: 'environment1') } + let!(:environment2) { create(:ci_group_variable, group: group, key: 'var2', environment_scope: 'environment2') } + let!(:environment3) { create(:ci_group_variable, group: group, key: 'var2', environment_scope: 'environment3') } + let(:finder) { described_class.new(group: group, params: params) } + + subject { finder.execute } + + context 'with default no arguments' do + let(:params) { {} } + + it do + expected_result = group.variables.environment_scope_names + + expect(subject.map(&:name)) + .to match_array(expected_result) + end + end + + context 'with search' do + let(:params) { { search: 'ment1' } } + + it do + expected_result = ['environment1'] + + expect(subject.map(&:name)) + .to match_array(expected_result) + end + end + + context 'with specific name' do + let(:params) { { name: 'environment3' } } + + it do + expect(subject.map(&:name)) + .to match_array([environment3.environment_scope]) + end + end + end +end diff --git a/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap b/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap index 807bb116f2e..8cad483e27e 100644 --- a/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap +++ b/spec/frontend/comment_templates/components/__snapshots__/list_item_spec.js.snap @@ -58,7 +58,7 @@ exports[`Comment templates list item component renders list item 1`] = ` </button> <div - class="gl-new-dropdown-panel gl-w-31! gl-absolute" + class="gl-new-dropdown-panel gl-w-31!" data-testid="base-dropdown-menu" id="base-dropdown-7" > diff --git a/spec/frontend/issues/show/components/header_actions_spec.js b/spec/frontend/issues/show/components/header_actions_spec.js index c14da65a236..9a503a2d882 100644 --- a/spec/frontend/issues/show/components/header_actions_spec.js +++ b/spec/frontend/issues/show/components/header_actions_spec.js @@ -103,7 +103,8 @@ describe('HeaderActions component', () => { }, }; - const findToggleIssueStateButton = () => wrapper.find(`[data-testid="toggle-button"]`); + const findToggleIssueStateButton = () => + wrapper.find(`[data-testid="toggle-issue-state-button"]`); const findEditButton = () => wrapper.find(`[data-testid="edit-button"]`); const findDropdownBy = (dataTestId) => wrapper.find(`[data-testid="${dataTestId}"]`); diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js index 5d81a7a9a0f..d50fb130a69 100644 --- a/spec/frontend/notes/components/noteable_note_spec.js +++ b/spec/frontend/notes/components/noteable_note_spec.js @@ -1,6 +1,7 @@ import Vue, { nextTick } from 'vue'; import Vuex from 'vuex'; import { GlAvatar } from '@gitlab/ui'; +import { clone } from 'lodash'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import DiffsModule from '~/diffs/store/modules'; @@ -10,9 +11,13 @@ import NoteHeader from '~/notes/components/note_header.vue'; import issueNote from '~/notes/components/noteable_note.vue'; import NotesModule from '~/notes/stores/modules'; import { NOTEABLE_TYPE_MAPPING } from '~/notes/constants'; +import { createAlert } from '~/alert'; +import { UPDATE_COMMENT_FORM } from '~/notes/i18n'; +import { sprintf } from '~/locale'; import { noteableDataMock, notesDataMock, note } from '../mock_data'; Vue.use(Vuex); +jest.mock('~/alert'); const singleLineNotePosition = { line_range: { @@ -54,10 +59,13 @@ describe('issue_note', () => { store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNotesData', notesDataMock); + // the component overwrites the `note` prop with every action, hence create a copy + const noteCopy = clone(props.note || note); + wrapper = mountExtended(issueNote, { store, propsData: { - note, + note: noteCopy, ...props, }, stubs: [ @@ -252,7 +260,7 @@ describe('issue_note', () => { }); it('should render issue body', () => { - expect(findNoteBody().props().note).toBe(note); + expect(findNoteBody().props().note).toMatchObject(note); expect(findNoteBody().props().line).toBe(null); expect(findNoteBody().props().canEdit).toBe(note.current_user.can_edit); expect(findNoteBody().props().isEditing).toBe(false); @@ -297,7 +305,7 @@ describe('issue_note', () => { }); it('does not have internal note class for external notes', () => { - createWrapper({ note }); + createWrapper(); expect(wrapper.classes()).not.toContain('internal-note'); }); @@ -327,7 +335,6 @@ describe('issue_note', () => { }); await nextTick(); - expect(findNoteBody().props().note.note_html).toBe(`<p dir="auto">${updatedText}</p>\n`); findNoteBody().vm.$emit('cancelForm', {}); @@ -340,7 +347,7 @@ describe('issue_note', () => { describe('formUpdateHandler', () => { const updateNote = jest.fn(); const params = { - noteText: '', + noteText: 'updated note text', parentElement: null, callback: jest.fn(), resolveDiscussion: false, @@ -359,28 +366,38 @@ describe('issue_note', () => { }); }; + beforeEach(() => { + createWrapper(); + updateActions(); + }); + afterEach(() => updateNote.mockReset()); it('responds to handleFormUpdate', () => { - createWrapper(); - updateActions(); findNoteBody().vm.$emit('handleFormUpdate', params); + expect(wrapper.emitted('handleUpdateNote')).toHaveLength(1); }); + it('updates note content', async () => { + findNoteBody().vm.$emit('handleFormUpdate', params); + + await nextTick(); + + expect(findNoteBody().props().note.note_html).toBe(`<p dir="auto">${params.noteText}</p>\n`); + expect(findNoteBody().props('isEditing')).toBe(false); + }); + it('should not update note with sensitive token', () => { const sensitiveMessage = 'token: glpat-1234567890abcdefghij'; - - createWrapper(); - updateActions(); findNoteBody().vm.$emit('handleFormUpdate', { ...params, noteText: sensitiveMessage }); + expect(updateNote).not.toHaveBeenCalled(); }); it('does not stringify empty position', () => { - createWrapper(); - updateActions(); findNoteBody().vm.$emit('handleFormUpdate', params); + expect(updateNote.mock.calls[0][1].note.note.position).toBeUndefined(); }); @@ -388,10 +405,35 @@ describe('issue_note', () => { const position = { test: true }; const expectation = JSON.stringify(position); createWrapper({ note: { ...note, position } }); + updateActions(); findNoteBody().vm.$emit('handleFormUpdate', params); + expect(updateNote.mock.calls[0][1].note.note.position).toBe(expectation); }); + + describe('when updateNote returns errors', () => { + beforeEach(() => { + updateNote.mockRejectedValue({ + response: { status: 422, data: { errors: 'error 1 and error 2' } }, + }); + }); + + beforeEach(() => { + findNoteBody().vm.$emit('handleFormUpdate', { ...params, noteText: 'invalid note' }); + }); + + it('renders error message and restores content of updated note', async () => { + await waitForPromises(); + expect(createAlert).toHaveBeenCalledWith({ + message: sprintf(UPDATE_COMMENT_FORM.error, { reason: 'error 1 and error 2' }, false), + parent: wrapper.vm.$el, + }); + + expect(findNoteBody().props('isEditing')).toBe(true); + expect(findNoteBody().props().note.note_html).toBe(note.note_html); + }); + }); }); describe('diffFile', () => { diff --git a/spec/graphql/resolvers/group_environment_scopes_resolver_spec.rb b/spec/graphql/resolvers/group_environment_scopes_resolver_spec.rb new file mode 100644 index 00000000000..71561137356 --- /dev/null +++ b/spec/graphql/resolvers/group_environment_scopes_resolver_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Resolvers::GroupEnvironmentScopesResolver, feature_category: :secrets_management do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let(:group) { create(:group) } + + context "with a group" do + let(:expected_environment_scopes) do + %w[environment1 environment2 environment3 environment4 environment5 environment6] + end + + before do + group.add_developer(current_user) + expected_environment_scopes.each_with_index do |env, index| + create(:ci_group_variable, group: group, key: "var#{index + 1}", environment_scope: env) + end + end + + describe '#resolve' do + it 'finds all environment scopes' do + expect(resolve_environment_scopes.map(&:name)).to match_array( + expected_environment_scopes + ) + end + end + end + + context 'without a group' do + describe '#resolve' do + it 'rails to find any environment scopes' do + expect(resolve_environment_scopes.map(&:name)).to match_array( + [] + ) + end + end + end + + def resolve_environment_scopes(args = {}, context = { current_user: current_user }) + resolve(described_class, obj: group, args: args, ctx: context) + end +end diff --git a/spec/graphql/types/ci/group_environment_scope_type_spec.rb b/spec/graphql/types/ci/group_environment_scope_type_spec.rb new file mode 100644 index 00000000000..3e3f52ca4bb --- /dev/null +++ b/spec/graphql/types/ci/group_environment_scope_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['CiGroupEnvironmentScope'], feature_category: :secrets_management do + specify do + expect(described_class).to have_graphql_fields( + :name + ).at_least + end +end diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb index a2751b9fb20..5a8a2b391e1 100644 --- a/spec/models/ci/group_variable_spec.rb +++ b/spec/models/ci/group_variable_spec.rb @@ -54,6 +54,36 @@ RSpec.describe Ci::GroupVariable, feature_category: :secrets_management do it { expect(described_class.for_groups([group.id])).to eq([group_variable]) } end + describe '.for_environment_scope_like' do + let_it_be(:group) { create(:group) } + let_it_be(:variable1_on_staging1) { create(:ci_group_variable, group: group, environment_scope: 'staging1') } + let_it_be(:variable2_on_staging2) { create(:ci_group_variable, group: group, environment_scope: 'staging2') } + let_it_be(:variable3_on_production) { create(:ci_group_variable, group: group, environment_scope: 'production') } + + it { + expect(described_class.for_environment_scope_like('staging')) + .to match_array([variable1_on_staging1, variable2_on_staging2]) + } + + it { + expect(described_class.for_environment_scope_like('production')) + .to match_array([variable3_on_production]) + } + end + + describe '.environment_scope_names' do + let_it_be(:group) { create(:group) } + let_it_be(:variable1_on_staging1) { create(:ci_group_variable, group: group, environment_scope: 'staging1') } + let_it_be(:variable2_on_staging2) { create(:ci_group_variable, group: group, environment_scope: 'staging2') } + let_it_be(:variable3_on_staging2) { create(:ci_group_variable, group: group, environment_scope: 'staging2') } + let_it_be(:variable4_on_production) { create(:ci_group_variable, group: group, environment_scope: 'production') } + + it 'groups and orders' do + expect(described_class.environment_scope_names) + .to match_array(%w[production staging1 staging2]) + end + end + it_behaves_like 'cleanup by a loose foreign key' do let!(:model) { create(:ci_group_variable) } diff --git a/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb b/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb new file mode 100644 index 00000000000..13a3a128979 --- /dev/null +++ b/spec/requests/api/graphql/ci/group_environment_scopes_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.group(fullPath).environmentScopes', feature_category: :secrets_management do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let(:expected_environment_scopes) do + %w[ + group1_environment1 + group1_environment2 + group2_environment3 + group2_environment4 + group2_environment5 + group2_environment6 + ] + end + + let(:query) do + %( + query { + group(fullPath: "#{group.full_path}") { + environmentScopes#{environment_scopes_params} { + nodes { + name + } + } + } + } + ) + end + + before do + group.add_developer(user) + expected_environment_scopes.each_with_index do |env, index| + create(:ci_group_variable, group: group, key: "var#{index + 1}", environment_scope: env) + end + end + + context 'when query has no parameters' do + let(:environment_scopes_params) { "" } + + it 'returns all avaiable environment scopes' do + post_graphql(query, current_user: user) + + expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq( + expected_environment_scopes.map { |env_scope| { 'name' => env_scope } } + ) + end + end + + context 'when query has search parameters' do + let(:environment_scopes_params) { "(search: \"group1\")" } + + it 'returns only environment scopes with group1 prefix' do + post_graphql(query, current_user: user) + + expect(graphql_data.dig('group', 'environmentScopes', 'nodes')).to eq( + [ + { 'name' => 'group1_environment1' }, + { 'name' => 'group1_environment2' } + ] + ) + end + end +end diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb index 01b61acb476..e109bfbcd0b 100644 --- a/spec/services/notes/update_service_spec.rb +++ b/spec/services/notes/update_service_spec.rb @@ -47,6 +47,24 @@ RSpec.describe Notes::UpdateService, feature_category: :team_planning do end end + context 'when the note is invalid' do + let(:edit_note_text) { { note: 'new text' } } + + before do + allow(note).to receive(:valid?).and_return(false) + end + + it 'does not update the note' do + travel_to(1.day.from_now) do + expect { update_note(edit_note_text) }.not_to change { note.reload.updated_at } + end + end + + it 'returns the note' do + expect(update_note(edit_note_text)).to eq(note) + end + end + describe 'event tracking', :snowplow do let(:event) { Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_EDITED } diff --git a/spec/support/finder_collection_allowlist.yml b/spec/support/finder_collection_allowlist.yml index 8fcb4ee7b9c..7ac7e88867a 100644 --- a/spec/support/finder_collection_allowlist.yml +++ b/spec/support/finder_collection_allowlist.yml @@ -5,6 +5,7 @@ # FooFinder # Reason: It uses a memory backend - Namespaces::BilledUsersFinder # Reason: There is no need to have anything else besides the ids is current structure - Namespaces::FreeUserCap::UsersFinder # Reason: There is no need to have anything else besides the count +- Groups::EnvironmentScopesFinder # Reason: There is no need to have anything else besides the simple strucutre with the scope name # Temporary excludes (aka TODOs) # For example: diff --git a/spec/support/view_component.rb b/spec/support/view_component.rb index 912bfda6d33..cddd39bbb1e 100644 --- a/spec/support/view_component.rb +++ b/spec/support/view_component.rb @@ -7,7 +7,7 @@ RSpec.configure do |config| config.include Devise::Test::ControllerHelpers, type: :component config.before(:each, type: :component) do - @request = controller.request + @request = vc_test_controller.request end config.include_context 'when page has no HTML escapes', type: :component |