diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-28 03:14:06 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-28 03:14:06 +0300 |
commit | 3235221bc498ca3c80eeca505fb32bf9f237778a (patch) | |
tree | 30437bc7bd635e911cd6d35c33eb05cf68f3cfa9 /spec | |
parent | ad928016f48a2f78168d1994c6012622af77f045 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
26 files changed, 355 insertions, 105 deletions
diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 93c560b4753..bc4eaf029e9 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -103,7 +103,7 @@ RSpec.describe Groups::ClustersController do it('is denied for admin when admin mode is disabled') { expect { go }.to be_denied_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(group) } it { expect { go }.to be_allowed_for(:maintainer).of(group) } - it { expect { go }.to be_denied_for(:developer).of(group) } + it { expect { go }.to be_allowed_for(:developer).of(group) } it { expect { go }.to be_denied_for(:reporter).of(group) } it { expect { go }.to be_denied_for(:guest).of(group) } it { expect { go }.to be_denied_for(:user) } @@ -673,7 +673,7 @@ RSpec.describe Groups::ClustersController do it('is denied for admin when admin mode is disabled') { expect { go }.to be_denied_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(group) } it { expect { go }.to be_allowed_for(:maintainer).of(group) } - it { expect { go }.to be_denied_for(:developer).of(group) } + it { expect { go }.to be_allowed_for(:developer).of(group) } it { expect { go }.to be_denied_for(:reporter).of(group) } it { expect { go }.to be_denied_for(:guest).of(group) } it { expect { go }.to be_denied_for(:user) } diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 2a8feb09780..cf6bd70f2d2 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -101,7 +101,7 @@ RSpec.describe Projects::ClustersController do it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_allowed_for(:developer).of(project) } it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } @@ -711,7 +711,7 @@ RSpec.describe Projects::ClustersController do end it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:maintainer).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_allowed_for(:developer).of(project) } it { expect { go }.to be_denied_for(:reporter).of(project) } it { expect { go }.to be_denied_for(:guest).of(project) } it { expect { go }.to be_denied_for(:user) } diff --git a/spec/features/monitor_sidebar_link_spec.rb b/spec/features/monitor_sidebar_link_spec.rb index bb5e581a034..fcef0fa0eff 100644 --- a/spec/features/monitor_sidebar_link_spec.rb +++ b/spec/features/monitor_sidebar_link_spec.rb @@ -117,9 +117,8 @@ RSpec.describe 'Monitor dropdown sidebar', :aggregate_failures do expect(page).to have_link('Error Tracking', href: project_error_tracking_index_path(project)) expect(page).to have_link('Product Analytics', href: project_product_analytics_path(project)) expect(page).to have_link('Logs', href: project_logs_path(project)) - - expect(page).not_to have_link('Serverless', href: project_serverless_functions_path(project)) - expect(page).not_to have_link('Kubernetes', href: project_clusters_path(project)) + expect(page).to have_link('Serverless', href: project_serverless_functions_path(project)) + expect(page).to have_link('Kubernetes', href: project_clusters_path(project)) end it_behaves_like 'shows Monitor menu based on the access level' diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 7ccd5c51493..a65d2d15c12 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -615,7 +615,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do end context 'when the user is not able to view the cluster' do - let(:user_access_level) { :developer } + let(:user_access_level) { :reporter } it 'includes only the name of the cluster without a link' do expect(page).to have_content 'using cluster the-cluster' diff --git a/spec/finders/clusters/agents_finder_spec.rb b/spec/finders/clusters/agents_finder_spec.rb index 0996d76b723..4ec798daa99 100644 --- a/spec/finders/clusters/agents_finder_spec.rb +++ b/spec/finders/clusters/agents_finder_spec.rb @@ -15,7 +15,11 @@ RSpec.describe Clusters::AgentsFinder do it { is_expected.to contain_exactly(matching_agent) } context 'user does not have permission' do - let(:user) { create(:user, developer_projects: [project]) } + let(:user) { create(:user) } + + before do + project.add_reporter(user) + end it { is_expected.to be_empty } end diff --git a/spec/frontend/clusters_list/components/agent_table_spec.js b/spec/frontend/clusters_list/components/agent_table_spec.js index 887c17bb4ad..27de9b18f4e 100644 --- a/spec/frontend/clusters_list/components/agent_table_spec.js +++ b/spec/frontend/clusters_list/components/agent_table_spec.js @@ -1,6 +1,6 @@ import { GlLink, GlIcon } from '@gitlab/ui'; import AgentTable from '~/clusters_list/components/agent_table.vue'; -import AgentOptions from '~/clusters_list/components/agent_options.vue'; +import DeleteAgentButton from '~/clusters_list/components/delete_agent_button.vue'; import { ACTIVE_CONNECTION_TIME } from '~/clusters_list/constants'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import { stubComponent } from 'helpers/stub_component'; @@ -56,7 +56,7 @@ const propsData = { ], }; -const AgentOptionsStub = stubComponent(AgentOptions, { +const DeleteAgentButtonStub = stubComponent(DeleteAgentButton, { template: `<div></div>`, }); @@ -69,14 +69,14 @@ describe('AgentTable', () => { const findLastContactText = (at) => wrapper.findAllByTestId('cluster-agent-last-contact').at(at); const findConfiguration = (at) => wrapper.findAllByTestId('cluster-agent-configuration-link').at(at); - const findAgentOptions = () => wrapper.findAllComponents(AgentOptions); + const findDeleteAgentButton = () => wrapper.findAllComponents(DeleteAgentButton); beforeEach(() => { wrapper = mountExtended(AgentTable, { propsData, provide: provideData, stubs: { - AgentOptions: AgentOptionsStub, + DeleteAgentButton: DeleteAgentButtonStub, }, }); }); @@ -128,7 +128,7 @@ describe('AgentTable', () => { }); it('displays actions menu for each agent', () => { - expect(findAgentOptions()).toHaveLength(3); + expect(findDeleteAgentButton()).toHaveLength(3); }); }); }); diff --git a/spec/frontend/clusters_list/components/clusters_actions_spec.js b/spec/frontend/clusters_list/components/clusters_actions_spec.js index cb8303ca4b2..331690fc642 100644 --- a/spec/frontend/clusters_list/components/clusters_actions_spec.js +++ b/spec/frontend/clusters_list/components/clusters_actions_spec.js @@ -10,9 +10,10 @@ describe('ClustersActionsComponent', () => { const newClusterPath = 'path/to/create/cluster'; const addClusterPath = 'path/to/connect/existing/cluster'; - const provideData = { + const defaultProvide = { newClusterPath, addClusterPath, + canAddCluster: true, }; const findDropdown = () => wrapper.findComponent(GlDropdown); @@ -21,13 +22,21 @@ describe('ClustersActionsComponent', () => { const findConnectClusterLink = () => wrapper.findByTestId('connect-cluster-link'); const findConnectNewAgentLink = () => wrapper.findByTestId('connect-new-agent-link'); - beforeEach(() => { + const createWrapper = (provideData = {}) => { wrapper = shallowMountExtended(ClustersActions, { - provide: provideData, + provide: { + ...defaultProvide, + ...provideData, + }, directives: { GlModalDirective: createMockDirective(), + GlTooltip: createMockDirective(), }, }); + }; + + beforeEach(() => { + createWrapper(); }); afterEach(() => { @@ -52,4 +61,24 @@ describe('ClustersActionsComponent', () => { expect(binding.value).toBe(INSTALL_AGENT_MODAL_ID); }); + + it('shows tooltip', () => { + const tooltip = getBinding(findDropdown().element, 'gl-tooltip'); + expect(tooltip.value).toBe(CLUSTERS_ACTIONS.connectWithAgent); + }); + + describe('when user cannot add clusters', () => { + beforeEach(() => { + createWrapper({ canAddCluster: false }); + }); + + it('disables dropdown', () => { + expect(findDropdown().props('disabled')).toBe(true); + }); + + it('shows tooltip explaining why dropdown is disabled', () => { + const tooltip = getBinding(findDropdown().element, 'gl-tooltip'); + expect(tooltip.value).toBe(CLUSTERS_ACTIONS.dropdownDisabledHint); + }); + }); }); diff --git a/spec/frontend/clusters_list/components/clusters_view_all_spec.js b/spec/frontend/clusters_list/components/clusters_view_all_spec.js index 81084443889..2c1e3d909cc 100644 --- a/spec/frontend/clusters_list/components/clusters_view_all_spec.js +++ b/spec/frontend/clusters_list/components/clusters_view_all_spec.js @@ -32,8 +32,9 @@ describe('ClustersViewAllComponent', () => { defaultBranchName, }; - const provideData = { + const defaultProvide = { addClusterPath, + canAddCluster: true, }; const entryData = { @@ -45,31 +46,43 @@ describe('ClustersViewAllComponent', () => { const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findAgentsComponent = () => wrapper.findComponent(Agents); const findClustersComponent = () => wrapper.findComponent(Clusters); + const findInstallAgentButtonTooltip = () => wrapper.findByTestId('install-agent-button-tooltip'); + const findConnectExistingClusterButtonTooltip = () => + wrapper.findByTestId('connect-existing-cluster-button-tooltip'); const findCardsContainer = () => wrapper.findByTestId('clusters-cards-container'); const findAgentCardTitle = () => wrapper.findByTestId('agent-card-title'); const findRecommendedBadge = () => wrapper.findComponent(GlBadge); const findClustersCardTitle = () => wrapper.findByTestId('clusters-card-title'); const findFooterButton = (line) => findCards().at(line).findComponent(GlButton); + const getTooltipText = (el) => { + const binding = getBinding(el, 'gl-tooltip'); + + return binding.value; + }; const createStore = (initialState) => new Vuex.Store({ state: initialState, }); - const createWrapper = ({ initialState }) => { + const createWrapper = ({ initialState = entryData, provideData } = {}) => { wrapper = shallowMountExtended(ClustersViewAll, { store: createStore(initialState), propsData, - provide: provideData, + provide: { + ...defaultProvide, + ...provideData, + }, directives: { GlModalDirective: createMockDirective(), + GlTooltip: createMockDirective(), }, stubs: { GlCard, GlSprintf }, }); }; beforeEach(() => { - createWrapper({ initialState: entryData }); + createWrapper(); }); afterEach(() => { @@ -125,15 +138,20 @@ describe('ClustersViewAllComponent', () => { expect(findAgentsComponent().props('defaultBranchName')).toBe(defaultBranchName); }); + it('should show install new Agent button in the footer', () => { + expect(findFooterButton(0).exists()).toBe(true); + expect(findFooterButton(0).props('disabled')).toBe(false); + }); + + it('does not show tooltip for install new Agent button', () => { + expect(getTooltipText(findInstallAgentButtonTooltip().element)).toBe(''); + }); + describe('when there are no agents', () => { it('should show the empty title', () => { expect(findAgentCardTitle().text()).toBe(AGENT_CARD_INFO.emptyTitle); }); - it('should show install new Agent button in the footer', () => { - expect(findFooterButton(0).exists()).toBe(true); - }); - it('should render correct modal id for the agent link', () => { const binding = getBinding(findFooterButton(0).element, 'gl-modal-directive'); @@ -173,6 +191,22 @@ describe('ClustersViewAllComponent', () => { }); }); }); + + describe('when the user cannot add clusters', () => { + beforeEach(() => { + createWrapper({ provideData: { canAddCluster: false } }); + }); + + it('should disable the button', () => { + expect(findFooterButton(0).props('disabled')).toBe(true); + }); + + it('should show a tooltip explaining why the button is disabled', () => { + expect(getTooltipText(findInstallAgentButtonTooltip().element)).toBe( + AGENT_CARD_INFO.installAgentDisabledHint, + ); + }); + }); }); describe('clusters tab', () => { @@ -189,13 +223,34 @@ describe('ClustersViewAllComponent', () => { expect(findClustersCardTitle().text()).toBe(CERTIFICATE_BASED_CARD_INFO.emptyTitle); }); - it('should show install new Agent button in the footer', () => { + it('should show install new cluster button in the footer', () => { expect(findFooterButton(1).exists()).toBe(true); + expect(findFooterButton(1).props('disabled')).toBe(false); }); it('should render correct href for the button in the footer', () => { expect(findFooterButton(1).attributes('href')).toBe(addClusterPath); }); + + it('does not show tooltip for install new cluster button', () => { + expect(getTooltipText(findConnectExistingClusterButtonTooltip().element)).toBe(''); + }); + }); + + describe('when the user cannot add clusters', () => { + beforeEach(() => { + createWrapper({ provideData: { canAddCluster: false } }); + }); + + it('should disable the button', () => { + expect(findFooterButton(1).props('disabled')).toBe(true); + }); + + it('should show a tooltip explaining why the button is disabled', () => { + expect(getTooltipText(findConnectExistingClusterButtonTooltip().element)).toBe( + CERTIFICATE_BASED_CARD_INFO.connectExistingClusterDisabledHint, + ); + }); }); describe('when the clusters are present', () => { diff --git a/spec/frontend/clusters_list/components/agent_options_spec.js b/spec/frontend/clusters_list/components/delete_agent_button_spec.js index 1d973ab39cc..82850b9dea4 100644 --- a/spec/frontend/clusters_list/components/agent_options_spec.js +++ b/spec/frontend/clusters_list/components/delete_agent_button_spec.js @@ -1,4 +1,4 @@ -import { GlDropdown, GlDropdownItem, GlModal, GlFormInput } from '@gitlab/ui'; +import { GlButton, GlModal, GlFormInput } from '@gitlab/ui'; import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; @@ -7,8 +7,9 @@ import getAgentsQuery from '~/clusters_list/graphql/queries/get_agents.query.gra import deleteAgentMutation from '~/clusters_list/graphql/mutations/delete_agent.mutation.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import AgentOptions from '~/clusters_list/components/agent_options.vue'; -import { MAX_LIST_COUNT } from '~/clusters_list/constants'; +import DeleteAgentButton from '~/clusters_list/components/delete_agent_button.vue'; +import { MAX_LIST_COUNT, DELETE_AGENT_BUTTON } from '~/clusters_list/constants'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import { getAgentResponse, mockDeleteResponse, mockErrorDeleteResponse } from '../mocks/apollo'; Vue.use(VueApollo); @@ -22,18 +23,23 @@ const agent = { webPath: 'agent-webPath', }; -describe('AgentOptions', () => { +describe('DeleteAgentButton', () => { let wrapper; let toast; let apolloProvider; let deleteResponse; const findModal = () => wrapper.findComponent(GlModal); - const findDropdown = () => wrapper.findComponent(GlDropdown); - const findDeleteBtn = () => wrapper.findComponent(GlDropdownItem); + const findDeleteBtn = () => wrapper.findComponent(GlButton); const findInput = () => wrapper.findComponent(GlFormInput); const findPrimaryAction = () => findModal().props('actionPrimary'); const findPrimaryActionAttributes = (attr) => findPrimaryAction().attributes[0][attr]; + const findDeleteAgentButtonTooltip = () => wrapper.findByTestId('delete-agent-button-tooltip'); + const getTooltipText = (el) => { + const binding = getBinding(el, 'gl-tooltip'); + + return binding.value; + }; const createMockApolloProvider = ({ mutationResponse }) => { deleteResponse = jest.fn().mockResolvedValue(mutationResponse); @@ -54,10 +60,14 @@ describe('AgentOptions', () => { }); }; - const createWrapper = async ({ mutationResponse = mockDeleteResponse } = {}) => { + const createWrapper = async ({ + mutationResponse = mockDeleteResponse, + provideData = {}, + } = {}) => { apolloProvider = createMockApolloProvider({ mutationResponse }); - const provide = { + const defaultProvide = { projectPath, + canAdminCluster: true, }; const propsData = { defaultBranchName, @@ -67,9 +77,15 @@ describe('AgentOptions', () => { toast = jest.fn(); - wrapper = shallowMountExtended(AgentOptions, { + wrapper = shallowMountExtended(DeleteAgentButton, { apolloProvider, - provide, + provide: { + ...defaultProvide, + ...provideData, + }, + directives: { + GlTooltip: createMockDirective(), + }, propsData, mocks: { $toast: { show: toast } }, stubs: { GlModal }, @@ -100,7 +116,13 @@ describe('AgentOptions', () => { describe('delete agent action', () => { it('displays a delete button', () => { - expect(findDeleteBtn().text()).toBe('Delete agent'); + expect(findDeleteBtn().attributes('aria-label')).toBe(DELETE_AGENT_BUTTON.deleteButton); + }); + + it('shows a tooltip for the button', () => { + expect(getTooltipText(findDeleteAgentButtonTooltip().element)).toBe( + DELETE_AGENT_BUTTON.deleteButton, + ); }); describe('when clicking the delete button', () => { @@ -113,6 +135,22 @@ describe('AgentOptions', () => { }); }); + describe('when user cannot delete clusters', () => { + beforeEach(() => { + createWrapper({ provideData: { canAdminCluster: false } }); + }); + + it('disables the button', () => { + expect(findDeleteBtn().attributes('disabled')).toBe('true'); + }); + + it('shows a disabled tooltip', () => { + expect(getTooltipText(findDeleteAgentButtonTooltip().element)).toBe( + DELETE_AGENT_BUTTON.disabledHint, + ); + }); + }); + describe.each` condition | agentName | isDisabled | mutationCalled ${'the input with agent name is missing'} | ${''} | ${true} | ${false} @@ -191,14 +229,14 @@ describe('AgentOptions', () => { await submitAgentToDelete(); }); - it('reenables the options dropdown', async () => { + it('reenables the button', async () => { expect(findPrimaryActionAttributes('loading')).toBe(true); - expect(findDropdown().attributes('disabled')).toBe('true'); + expect(findDeleteBtn().attributes('disabled')).toBe('true'); await findModal().vm.$emit('hide'); expect(findPrimaryActionAttributes('loading')).toBe(false); - expect(findDropdown().attributes('disabled')).toBeUndefined(); + expect(findDeleteBtn().attributes('disabled')).toBeUndefined(); }); it('clears the agent name input', async () => { diff --git a/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb b/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb index 9b54d466681..866f4ce7b5a 100644 --- a/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb +++ b/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Resolvers::Clusters::AgentTokensResolver do describe '#resolve' do let(:agent) { create(:cluster_agent) } - let(:user) { create(:user, maintainer_projects: [agent.project]) } + let(:user) { create(:user, developer_projects: [agent.project]) } let(:ctx) { Hash(current_user: user) } let!(:matching_token1) { create(:cluster_agent_token, agent: agent, last_used_at: 5.days.ago) } @@ -33,7 +33,11 @@ RSpec.describe Resolvers::Clusters::AgentTokensResolver do end context 'user does not have permission' do - let(:user) { create(:user, developer_projects: [agent.project]) } + let(:user) { create(:user) } + + before do + agent.project.add_reporter(user) + end it { is_expected.to be_empty } end diff --git a/spec/graphql/resolvers/clusters/agents_resolver_spec.rb b/spec/graphql/resolvers/clusters/agents_resolver_spec.rb index 70f40748e1d..152d7fa22c4 100644 --- a/spec/graphql/resolvers/clusters/agents_resolver_spec.rb +++ b/spec/graphql/resolvers/clusters/agents_resolver_spec.rb @@ -15,10 +15,14 @@ RSpec.describe Resolvers::Clusters::AgentsResolver do describe '#resolve' do let_it_be(:project) { create(:project) } - let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) } - let_it_be(:developer) { create(:user, developer_projects: [project]) } + let_it_be(:maintainer) { create(:user, developer_projects: [project]) } + let_it_be(:reporter) { create(:user) } let_it_be(:agents) { create_list(:cluster_agent, 2, project: project) } + before do + project.add_reporter(reporter) + end + let(:ctx) { { current_user: current_user } } subject { resolve_agents } @@ -32,7 +36,7 @@ RSpec.describe Resolvers::Clusters::AgentsResolver do end context 'the current user does not have access to clusters' do - let(:current_user) { developer } + let(:current_user) { reporter } it 'returns an empty result' do expect(subject).to be_empty diff --git a/spec/graphql/types/clusters/agent_activity_event_type_spec.rb b/spec/graphql/types/clusters/agent_activity_event_type_spec.rb index 7773bad749d..cae75485846 100644 --- a/spec/graphql/types/clusters/agent_activity_event_type_spec.rb +++ b/spec/graphql/types/clusters/agent_activity_event_type_spec.rb @@ -6,6 +6,6 @@ RSpec.describe GitlabSchema.types['ClusterAgentActivityEvent'] do let(:fields) { %i[recorded_at kind level user agent_token] } it { expect(described_class.graphql_name).to eq('ClusterAgentActivityEvent') } - it { expect(described_class).to require_graphql_authorizations(:admin_cluster) } + it { expect(described_class).to require_graphql_authorizations(:read_cluster) } it { expect(described_class).to have_graphql_fields(fields) } end diff --git a/spec/graphql/types/clusters/agent_token_type_spec.rb b/spec/graphql/types/clusters/agent_token_type_spec.rb index 3f0720cb4b5..1ca6d690c80 100644 --- a/spec/graphql/types/clusters/agent_token_type_spec.rb +++ b/spec/graphql/types/clusters/agent_token_type_spec.rb @@ -7,7 +7,7 @@ RSpec.describe GitlabSchema.types['ClusterAgentToken'] do it { expect(described_class.graphql_name).to eq('ClusterAgentToken') } - it { expect(described_class).to require_graphql_authorizations(:admin_cluster) } + it { expect(described_class).to require_graphql_authorizations(:read_cluster) } it { expect(described_class).to have_graphql_fields(fields) } end diff --git a/spec/graphql/types/clusters/agent_type_spec.rb b/spec/graphql/types/clusters/agent_type_spec.rb index a1e5952bf73..3f4faccf15d 100644 --- a/spec/graphql/types/clusters/agent_type_spec.rb +++ b/spec/graphql/types/clusters/agent_type_spec.rb @@ -7,7 +7,7 @@ RSpec.describe GitlabSchema.types['ClusterAgent'] do it { expect(described_class.graphql_name).to eq('ClusterAgent') } - it { expect(described_class).to require_graphql_authorizations(:admin_cluster) } + it { expect(described_class).to require_graphql_authorizations(:read_cluster) } it { expect(described_class).to have_graphql_fields(fields) } end diff --git a/spec/helpers/clusters_helper_spec.rb b/spec/helpers/clusters_helper_spec.rb index 51f111917d1..06be3c58e55 100644 --- a/spec/helpers/clusters_helper_spec.rb +++ b/spec/helpers/clusters_helper_spec.rb @@ -93,8 +93,9 @@ RSpec.describe ClustersHelper do end context 'user has no permissions to create a cluster' do - it 'displays that user can\t add cluster' do + it 'displays that user can\'t add cluster' do expect(subject[:can_add_cluster]).to eq("false") + expect(subject[:can_admin_cluster]).to eq("false") end end @@ -105,6 +106,7 @@ RSpec.describe ClustersHelper do it 'displays that the user can add cluster' do expect(subject[:can_add_cluster]).to eq("true") + expect(subject[:can_admin_cluster]).to eq("true") end end diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb index fc154738f11..7e08f47fb5a 100644 --- a/spec/models/concerns/resolvable_discussion_spec.rb +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -584,4 +584,14 @@ RSpec.describe Discussion, ResolvableDiscussion do expect(subject.last_resolved_note).to eq(second_note) end end + + describe '#clear_memoized_values' do + it 'resets the memoized values' do + described_class.memoized_values.each do |memo| + subject.instance_variable_set("@#{memo}", 'memoized') + expect { subject.clear_memoized_values }.to change { subject.instance_variable_get("@#{memo}") } + .from('memoized').to(nil) + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 0b3fb80f97f..70dd1cbe240 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3577,21 +3577,38 @@ RSpec.describe MergeRequest, factory_default: :keep do end describe '#update_diff_discussion_positions' do - let(:discussion) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject).to_discussion } - let(:commit) { subject.project.commit(sample_commit.id) } - let(:old_diff_refs) { subject.diff_refs } + subject { create(:merge_request, source_project: project) } - before do - # Update merge_request_diff so that #diff_refs will return commit.diff_refs - allow(subject).to receive(:create_merge_request_diff) do - subject.merge_request_diffs.create!( - base_commit_sha: commit.parent_id, - start_commit_sha: commit.parent_id, - head_commit_sha: commit.sha - ) + let(:project) { create(:project, :repository) } + let(:create_commit) { project.commit("913c66a37b4a45b9769037c55c2d238bd0942d2e") } + let(:modify_commit) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e") } + let(:edit_commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") } + let(:discussion) { create(:diff_note_on_merge_request, noteable: subject, project: project, position: old_position).to_discussion } + let(:path) { "files/ruby/popen.rb" } + let(:new_line) { 9 } + + let(:old_diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: create_commit.parent_id, + head_sha: modify_commit.sha + ) + end - subject.reload_merge_request_diff - end + let(:new_diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: create_commit.parent_id, + head_sha: edit_commit.sha + ) + end + + let(:old_position) do + Gitlab::Diff::Position.new( + old_path: path, + new_path: path, + old_line: nil, + new_line: new_line, + diff_refs: old_diff_refs + ) end it "updates diff discussion positions" do @@ -3599,36 +3616,67 @@ RSpec.describe MergeRequest, factory_default: :keep do subject.project, subject.author, old_diff_refs: old_diff_refs, - new_diff_refs: commit.diff_refs, + new_diff_refs: new_diff_refs, paths: discussion.position.paths ).and_call_original expect_any_instance_of(Discussions::UpdateDiffPositionService).to receive(:execute).with(discussion).and_call_original - expect_any_instance_of(DiffNote).to receive(:save).once subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs, - new_diff_refs: commit.diff_refs, + new_diff_refs: new_diff_refs, current_user: subject.author) end - context 'when resolve_outdated_diff_discussions is set' do - let(:project) { create(:project, :repository) } + it 'does not call the resolve method' do + expect(MergeRequests::ResolvedDiscussionNotificationService).not_to receive(:new) - subject { create(:merge_request, source_project: project) } + subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + current_user: subject.author) + end + context 'when resolve_outdated_diff_discussions is set' do before do discussion subject.project.update!(resolve_outdated_diff_discussions: true) end - it 'calls MergeRequests::ResolvedDiscussionNotificationService' do - expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService) - .to receive(:execute).with(subject) + context 'when the active discussion is resolved in the update' do + it 'calls MergeRequests::ResolvedDiscussionNotificationService' do + expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService) + .to receive(:execute).with(subject) + + subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + current_user: subject.author) + end + end + + context 'when the active discussion does not have resolved in the update' do + let(:new_line) { 16 } - subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs, - new_diff_refs: commit.diff_refs, - current_user: subject.author) + it 'does not call the resolve method' do + expect(MergeRequests::ResolvedDiscussionNotificationService).not_to receive(:new) + + subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + current_user: subject.author) + end + end + + context 'when the active discussion was already resolved' do + before do + discussion.resolve!(subject.author) + end + + it 'does not call the resolve method' do + expect(MergeRequests::ResolvedDiscussionNotificationService).not_to receive(:new) + + subject.update_diff_discussion_positions(old_diff_refs: old_diff_refs, + new_diff_refs: new_diff_refs, + current_user: subject.author) + end end end end diff --git a/spec/policies/clusters/agent_token_policy_spec.rb b/spec/policies/clusters/agent_token_policy_spec.rb index 9ae99e66f59..f5ac8bd67e6 100644 --- a/spec/policies/clusters/agent_token_policy_spec.rb +++ b/spec/policies/clusters/agent_token_policy_spec.rb @@ -10,13 +10,22 @@ RSpec.describe Clusters::AgentTokenPolicy do let(:project) { token.agent.project } describe 'rules' do + context 'when reporter' do + before do + project.add_reporter(user) + end + + it { expect(policy).to be_disallowed :admin_cluster } + it { expect(policy).to be_disallowed :read_cluster } + end + context 'when developer' do before do project.add_developer(user) end it { expect(policy).to be_disallowed :admin_cluster } - it { expect(policy).to be_disallowed :read_cluster } + it { expect(policy).to be_allowed :read_cluster } end context 'when maintainer' do diff --git a/spec/policies/clusters/agents/activity_event_policy_spec.rb b/spec/policies/clusters/agents/activity_event_policy_spec.rb index 1262fcfd9f2..365168de79f 100644 --- a/spec/policies/clusters/agents/activity_event_policy_spec.rb +++ b/spec/policies/clusters/agents/activity_event_policy_spec.rb @@ -10,13 +10,22 @@ RSpec.describe Clusters::Agents::ActivityEventPolicy do let(:project) { event.agent.project } describe 'rules' do + context 'reporter' do + before do + project.add_reporter(user) + end + + it { expect(policy).to be_disallowed :admin_cluster } + it { expect(policy).to be_disallowed :read_cluster } + end + context 'developer' do before do project.add_developer(user) end it { expect(policy).to be_disallowed :admin_cluster } - it { expect(policy).to be_disallowed :read_cluster } + it { expect(policy).to be_allowed :read_cluster } end context 'maintainer' do diff --git a/spec/presenters/clusterable_presenter_spec.rb b/spec/presenters/clusterable_presenter_spec.rb index d19abd4e4d8..7c2e19728d5 100644 --- a/spec/presenters/clusterable_presenter_spec.rb +++ b/spec/presenters/clusterable_presenter_spec.rb @@ -79,6 +79,30 @@ RSpec.describe ClusterablePresenter do end end + describe '#can_admin_cluster?' do + let(:user) { create(:user) } + + subject { described_class.new(clusterable).can_admin_cluster? } + + before do + clusterable.add_maintainer(user) + + allow(clusterable).to receive(:current_user).and_return(user) + end + + context 'when clusterable is a group' do + let(:clusterable) { create(:group) } + + it_behaves_like 'appropriate member permissions' + end + + context 'when clusterable is a project' do + let(:clusterable) { create(:project, :repository) } + + it_behaves_like 'appropriate member permissions' + end + end + describe '#environments_cluster_path' do subject { described_class.new(clusterable).environments_cluster_path(cluster) } diff --git a/spec/requests/api/group_clusters_spec.rb b/spec/requests/api/group_clusters_spec.rb index f65f9384efa..c48b5007f91 100644 --- a/spec/requests/api/group_clusters_spec.rb +++ b/spec/requests/api/group_clusters_spec.rb @@ -6,11 +6,11 @@ RSpec.describe API::GroupClusters do include KubernetesHelpers let(:current_user) { create(:user) } - let(:developer_user) { create(:user) } + let(:unauthorized_user) { create(:user) } let(:group) { create(:group, :private) } before do - group.add_developer(developer_user) + group.add_reporter(unauthorized_user) group.add_maintainer(current_user) end @@ -24,7 +24,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - get api("/groups/#{group.id}/clusters", developer_user) + get api("/groups/#{group.id}/clusters", unauthorized_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -68,7 +68,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - get api("/groups/#{group.id}/clusters/#{cluster_id}", developer_user) + get api("/groups/#{group.id}/clusters/#{cluster_id}", unauthorized_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -183,7 +183,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + post api("/groups/#{group.id}/clusters/user", unauthorized_user), params: cluster_params expect(response).to have_gitlab_http_status(:forbidden) end @@ -290,7 +290,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do before do - post api("/groups/#{group.id}/clusters/user", developer_user), params: cluster_params + post api("/groups/#{group.id}/clusters/user", unauthorized_user), params: cluster_params end it 'responds with 403' do @@ -364,7 +364,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - put api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: update_params + put api("/groups/#{group.id}/clusters/#{cluster.id}", unauthorized_user), params: update_params expect(response).to have_gitlab_http_status(:forbidden) end @@ -505,7 +505,7 @@ RSpec.describe API::GroupClusters do context 'non-authorized user' do it 'responds with 403' do - delete api("/groups/#{group.id}/clusters/#{cluster.id}", developer_user), params: cluster_params + delete api("/groups/#{group.id}/clusters/#{cluster.id}", unauthorized_user), params: cluster_params expect(response).to have_gitlab_http_status(:forbidden) end diff --git a/spec/requests/api/project_clusters_spec.rb b/spec/requests/api/project_clusters_spec.rb index 253b61e5865..b83b41a881a 100644 --- a/spec/requests/api/project_clusters_spec.rb +++ b/spec/requests/api/project_clusters_spec.rb @@ -5,13 +5,15 @@ require 'spec_helper' RSpec.describe API::ProjectClusters do include KubernetesHelpers - let_it_be(:current_user) { create(:user) } + let_it_be(:maintainer_user) { create(:user) } let_it_be(:developer_user) { create(:user) } + let_it_be(:reporter_user) { create(:user) } let_it_be(:project) { create(:project) } before do - project.add_maintainer(current_user) + project.add_maintainer(maintainer_user) project.add_developer(developer_user) + project.add_reporter(reporter_user) end describe 'GET /projects/:id/clusters' do @@ -24,7 +26,7 @@ RSpec.describe API::ProjectClusters do context 'non-authorized user' do it 'responds with 403' do - get api("/projects/#{project.id}/clusters", developer_user) + get api("/projects/#{project.id}/clusters", reporter_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -32,7 +34,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - get api("/projects/#{project.id}/clusters", current_user) + get api("/projects/#{project.id}/clusters", developer_user) end it 'includes pagination headers' do @@ -61,13 +63,13 @@ RSpec.describe API::ProjectClusters do let(:cluster) do create(:cluster, :project, :provided_by_gcp, :with_domain, platform_kubernetes: platform_kubernetes, - user: current_user, + user: maintainer_user, projects: [project]) end context 'non-authorized user' do it 'responds with 403' do - get api("/projects/#{project.id}/clusters/#{cluster_id}", developer_user) + get api("/projects/#{project.id}/clusters/#{cluster_id}", reporter_user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -75,7 +77,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - get api("/projects/#{project.id}/clusters/#{cluster_id}", current_user) + get api("/projects/#{project.id}/clusters/#{cluster_id}", developer_user) end it 'returns specific cluster' do @@ -111,8 +113,8 @@ RSpec.describe API::ProjectClusters do it 'returns user information' do user = json_response['user'] - expect(user['id']).to eq(current_user.id) - expect(user['username']).to eq(current_user.username) + expect(user['id']).to eq(maintainer_user.id) + expect(user['username']).to eq(maintainer_user.username) end it 'returns GCP provider information' do @@ -156,7 +158,7 @@ RSpec.describe API::ProjectClusters do let(:management_project_id) { management_project.id } before do - management_project.add_maintainer(current_user) + management_project.add_maintainer(maintainer_user) end let(:platform_kubernetes_attributes) do @@ -190,7 +192,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params + post api("/projects/#{project.id}/clusters/user", maintainer_user), params: cluster_params end context 'with valid params' do @@ -317,7 +319,7 @@ RSpec.describe API::ProjectClusters do create(:cluster, :provided_by_gcp, :project, projects: [project]) - post api("/projects/#{project.id}/clusters/user", current_user), params: cluster_params + post api("/projects/#{project.id}/clusters/user", maintainer_user), params: cluster_params end it 'responds with 201' do @@ -369,9 +371,9 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - management_project.add_maintainer(current_user) + management_project.add_maintainer(maintainer_user) - put api("/projects/#{project.id}/clusters/#{cluster.id}", current_user), params: update_params + put api("/projects/#{project.id}/clusters/#{cluster.id}", maintainer_user), params: update_params cluster.reload end @@ -501,7 +503,7 @@ RSpec.describe API::ProjectClusters do context 'authorized user' do before do - delete api("/projects/#{project.id}/clusters/#{cluster.id}", current_user), params: cluster_params + delete api("/projects/#{project.id}/clusters/#{cluster.id}", maintainer_user), params: cluster_params end it 'deletes the cluster' do diff --git a/spec/requests/projects/cluster_agents_controller_spec.rb b/spec/requests/projects/cluster_agents_controller_spec.rb index e4c4f537699..914d5b17ba8 100644 --- a/spec/requests/projects/cluster_agents_controller_spec.rb +++ b/spec/requests/projects/cluster_agents_controller_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Projects::ClusterAgentsController do let_it_be(:user) { create(:user) } before do - project.add_developer(user) + project.add_reporter(user) sign_in(user) subject end diff --git a/spec/serializers/deployment_cluster_entity_spec.rb b/spec/serializers/deployment_cluster_entity_spec.rb index 95f2f8ce6fc..419ae746b74 100644 --- a/spec/serializers/deployment_cluster_entity_spec.rb +++ b/spec/serializers/deployment_cluster_entity_spec.rb @@ -7,7 +7,7 @@ RSpec.describe DeploymentClusterEntity do subject { described_class.new(deployment, request: request).as_json } let(:maintainer) { create(:user) } - let(:developer) { create(:user) } + let(:reporter) { create(:user) } let(:current_user) { maintainer } let(:request) { double(:request, current_user: current_user) } let(:project) { create(:project) } @@ -17,7 +17,7 @@ RSpec.describe DeploymentClusterEntity do before do project.add_maintainer(maintainer) - project.add_developer(developer) + project.add_reporter(reporter) end it 'matches deployment_cluster entity schema' do @@ -31,7 +31,7 @@ RSpec.describe DeploymentClusterEntity do end context 'when the user does not have permission to view the cluster' do - let(:current_user) { developer } + let(:current_user) { reporter } it 'does not include the path nor the namespace' do expect(subject[:path]).to be_nil diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index 0dfd76de79c..76db2bd82f1 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -47,6 +47,7 @@ RSpec.shared_context 'GroupPolicy context' do create_custom_emoji create_package create_package_settings + read_cluster ] end @@ -54,7 +55,7 @@ RSpec.shared_context 'GroupPolicy context' do %i[ destroy_package create_projects - read_cluster create_cluster update_cluster admin_cluster add_cluster + create_cluster update_cluster admin_cluster add_cluster ] end diff --git a/spec/support/shared_examples/policies/clusterable_shared_examples.rb b/spec/support/shared_examples/policies/clusterable_shared_examples.rb index b96aa71acbe..faf283f9059 100644 --- a/spec/support/shared_examples/policies/clusterable_shared_examples.rb +++ b/spec/support/shared_examples/policies/clusterable_shared_examples.rb @@ -6,12 +6,24 @@ RSpec.shared_examples 'clusterable policies' do subject { described_class.new(current_user, clusterable) } + context 'with a reporter' do + before do + clusterable.add_reporter(current_user) + end + + it { expect_disallowed(:read_cluster) } + it { expect_disallowed(:add_cluster) } + it { expect_disallowed(:create_cluster) } + it { expect_disallowed(:update_cluster) } + it { expect_disallowed(:admin_cluster) } + end + context 'with a developer' do before do clusterable.add_developer(current_user) end - it { expect_disallowed(:read_cluster) } + it { expect_allowed(:read_cluster) } it { expect_disallowed(:add_cluster) } it { expect_disallowed(:create_cluster) } it { expect_disallowed(:update_cluster) } |