diff options
19 files changed, 299 insertions, 93 deletions
diff --git a/app/assets/javascripts/ide/stores/modules/merge_requests/actions.js b/app/assets/javascripts/ide/stores/modules/merge_requests/actions.js index cdd8076952f..6ef938b0ae2 100644 --- a/app/assets/javascripts/ide/stores/modules/merge_requests/actions.js +++ b/app/assets/javascripts/ide/stores/modules/merge_requests/actions.js @@ -31,7 +31,7 @@ export const fetchMergeRequests = ({ dispatch, state: { state } }, { type, searc dispatch('requestMergeRequests', type); dispatch('resetMergeRequests', type); - Api.mergeRequests({ scope, state, search }) + return Api.mergeRequests({ scope, state, search }) .then(({ data }) => dispatch('receiveMergeRequestsSuccess', { type, data })) .catch(() => dispatch('receiveMergeRequestsError', { type, search })); }; diff --git a/app/assets/javascripts/ide/stores/modules/pipelines/actions.js b/app/assets/javascripts/ide/stores/modules/pipelines/actions.js index 8cb01f25223..3e67b222e66 100644 --- a/app/assets/javascripts/ide/stores/modules/pipelines/actions.js +++ b/app/assets/javascripts/ide/stores/modules/pipelines/actions.js @@ -102,7 +102,7 @@ export const receiveJobsSuccess = ({ commit }, { id, data }) => export const fetchJobs = ({ dispatch }, stage) => { dispatch('requestJobs', stage.id); - axios + return axios .get(stage.dropdownPath) .then(({ data }) => dispatch('receiveJobsSuccess', { id: stage.id, data })) .catch(() => dispatch('receiveJobsError', stage)); diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 9195408551f..1933c46ee44 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -32,6 +32,7 @@ class NotificationSetting < ActiveRecord::Base :reopen_issue, :close_issue, :reassign_issue, + :issue_due, :new_merge_request, :push_to_merge_request, :reopen_merge_request, diff --git a/changelogs/unreleased/fix-performance-problem-of-tags-query.yml b/changelogs/unreleased/fix-performance-problem-of-tags-query.yml new file mode 100644 index 00000000000..4649775be9c --- /dev/null +++ b/changelogs/unreleased/fix-performance-problem-of-tags-query.yml @@ -0,0 +1,5 @@ +--- +title: Fix performance problem of accessing tag list for projects api endpoints +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/issue_47709.yml b/changelogs/unreleased/issue_47709.yml new file mode 100644 index 00000000000..c3ef55fd692 --- /dev/null +++ b/changelogs/unreleased/issue_47709.yml @@ -0,0 +1,5 @@ +--- +title: 'Allow to toggle notifications for issues due soon' +merge_request: +author: +type: fixed diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 66b62d9ee2e..b256c33c631 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -135,10 +135,13 @@ module API expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes def self.preload_relation(projects_relation, options = {}) + # Preloading tags, should be done with using only `:tags`, + # as `:tags` are defined as: `has_many :tags, through: :taggings` + # N+1 is solved then by using `subject.tags.map(&:name)` + # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20555 projects_relation.preload(:project_feature, :route) - .preload(:import_state) - .preload(namespace: [:route, :owner], - tags: :taggings) + .preload(:import_state, :tags) + .preload(namespace: [:route, :owner]) end end @@ -212,11 +215,15 @@ module API expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics def self.preload_relation(projects_relation, options = {}) + # Preloading tags, should be done with using only `:tags`, + # as `:tags` are defined as: `has_many :tags, through: :taggings` + # N+1 is solved then by using `subject.tags.map(&:name)` + # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20555 super(projects_relation).preload(:group) .preload(project_group_links: :group, fork_network: :root_project, forked_project_link: :forked_from_project, - forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) + forked_from_project: [:route, :forks, :tags, namespace: :route]) end def self.forks_counting_projects(projects_relation) @@ -1,5 +1,7 @@ $: << File.expand_path(File.dirname(__FILE__)) +Encoding.default_external = 'UTF-8' + module QA ## # GitLab QA runtime classes, mostly singletons. diff --git a/qa/qa/factory/resource/kubernetes_cluster.rb b/qa/qa/factory/resource/kubernetes_cluster.rb index f32cf985e9d..1c9e5f94b22 100644 --- a/qa/qa/factory/resource/kubernetes_cluster.rb +++ b/qa/qa/factory/resource/kubernetes_cluster.rb @@ -36,6 +36,9 @@ module QA if @install_helm_tiller Page::Project::Operations::Kubernetes::Show.perform do |page| + # We must wait a few seconds for permissions to be setup correctly for new cluster + sleep 10 + # Helm must be installed before everything else page.install!(:helm) page.await_installed(:helm) diff --git a/qa/qa/runtime/namespace.rb b/qa/qa/runtime/namespace.rb index 28f17d1160b..f1c8ef11f94 100644 --- a/qa/qa/runtime/namespace.rb +++ b/qa/qa/runtime/namespace.rb @@ -8,7 +8,7 @@ module QA end def name - "qa-test-#{time.strftime('%Y-%m-%d-%Y-%H-%M-%S')}" + "qa-test-#{time.strftime('%Y-%m-%d-%H-%M-%S')}" end def path diff --git a/spec/features/projects/show/user_manages_notifications_spec.rb b/spec/features/projects/show/user_manages_notifications_spec.rb index 31b105229be..546619e88ec 100644 --- a/spec/features/projects/show/user_manages_notifications_spec.rb +++ b/spec/features/projects/show/user_manages_notifications_spec.rb @@ -16,4 +16,36 @@ describe 'Projects > Show > User manages notifications', :js do expect(page).to have_content 'On mention' end end + + context 'custom notification settings' do + let(:email_events) do + [ + :new_note, + :new_issue, + :reopen_issue, + :close_issue, + :reassign_issue, + :issue_due, + :new_merge_request, + :push_to_merge_request, + :reopen_merge_request, + :close_merge_request, + :reassign_merge_request, + :merge_merge_request, + :failed_pipeline, + :success_pipeline + ] + end + + it 'shows notification settings checkbox' do + first('.notifications-btn').click + page.find('a[data-notification-level="custom"]').click + + page.within('.custom-notifications-form') do + email_events.each do |event_name| + expect(page).to have_selector("input[name='notification_setting[#{event_name}]']") + end + end + end + end end diff --git a/spec/javascripts/frequent_items/store/actions_spec.js b/spec/javascripts/frequent_items/store/actions_spec.js index 0cdd033d38f..0a8525e77d6 100644 --- a/spec/javascripts/frequent_items/store/actions_spec.js +++ b/spec/javascripts/frequent_items/store/actions_spec.js @@ -205,7 +205,7 @@ describe('Frequent Items Dropdown Store Actions', () => { actions.setSearchQuery, { query: 'test' }, mockedState, - [{ type: types.SET_SEARCH_QUERY }], + [{ type: types.SET_SEARCH_QUERY, payload: { query: 'test' } }], [{ type: 'fetchSearchedItems', payload: { query: 'test' } }], done, ); @@ -216,7 +216,7 @@ describe('Frequent Items Dropdown Store Actions', () => { actions.setSearchQuery, null, mockedState, - [{ type: types.SET_SEARCH_QUERY }], + [{ type: types.SET_SEARCH_QUERY, payload: null }], [{ type: 'fetchFrequentItems' }], done, ); diff --git a/spec/javascripts/helpers/vuex_action_helper.js b/spec/javascripts/helpers/vuex_action_helper.js index d6ab0aeeed7..4ca7015184e 100644 --- a/spec/javascripts/helpers/vuex_action_helper.js +++ b/spec/javascripts/helpers/vuex_action_helper.js @@ -1,71 +1,103 @@ +const noop = () => {}; + /** - * helper for testing action with expected mutations inspired in + * Helper for testing action with expected mutations inspired in * https://vuex.vuejs.org/en/testing.html * + * @param {Function} action to be tested + * @param {Object} payload will be provided to the action + * @param {Object} state will be provided to the action + * @param {Array} [expectedMutations=[]] mutations expected to be committed + * @param {Array} [expectedActions=[]] actions expected to be dispatched + * @param {Function} [done=noop] to be executed after the tests + * @return {Promise} + * * @example * testAction( * actions.actionName, // action - * { }, // mocked response - * state, // state + * { }, // mocked payload + * state, //state + * // expected mutations * [ * { type: types.MUTATION} - * { type: types.MUTATION_1, payload: {}} - * ], // mutations + * { type: types.MUTATION_1, payload: jasmine.any(Number)} + * ], + * // expected actions * [ - * { type: 'actionName', payload: {}}, - * { type: 'actionName1', payload: {}} - * ] //actions + * { type: 'actionName', payload: {param: 'foobar'}}, + * { type: 'actionName1'} + * ] * done, * ); + * + * @example + * testAction( + * actions.actionName, // action + * { }, // mocked payload + * state, //state + * [ { type: types.MUTATION} ], // expected mutations + * [], // expected actions + * ).then(done) + * .catch(done.fail); */ -export default (action, payload, state, expectedMutations, expectedActions, done) => { - let mutationsCount = 0; - let actionsCount = 0; +export default ( + action, + payload, + state, + expectedMutations = [], + expectedActions = [], + done = noop, +) => { + const mutations = []; + const actions = []; // mock commit const commit = (type, mutationPayload) => { - const mutation = expectedMutations[mutationsCount]; - - expect(mutation.type).toEqual(type); + const mutation = { type }; - if (mutation.payload) { - expect(mutation.payload).toEqual(mutationPayload); + if (typeof mutationPayload !== 'undefined') { + mutation.payload = mutationPayload; } - mutationsCount += 1; - if (mutationsCount >= expectedMutations.length) { - done(); - } + mutations.push(mutation); }; // mock dispatch const dispatch = (type, actionPayload) => { - const actionExpected = expectedActions[actionsCount]; - - expect(actionExpected.type).toEqual(type); + const dispatchedAction = { type }; - if (actionExpected.payload) { - expect(actionExpected.payload).toEqual(actionPayload); + if (typeof actionPayload !== 'undefined') { + dispatchedAction.payload = actionPayload; } - actionsCount += 1; - if (actionsCount >= expectedActions.length) { - done(); - } + actions.push(dispatchedAction); }; - // call the action with mocked store and arguments - action({ commit, state, dispatch, rootState: state }, payload); - - // check if no mutations should have been dispatched - if (expectedMutations.length === 0) { - expect(mutationsCount).toEqual(0); + const validateResults = () => { + expect({ + mutations, + actions, + }).toEqual({ + mutations: expectedMutations, + actions: expectedActions, + }); done(); - } + }; - // check if no mutations should have been dispatched - if (expectedActions.length === 0) { - expect(actionsCount).toEqual(0); - done(); - } + return new Promise((resolve, reject) => { + try { + const result = action({ commit, state, dispatch, rootState: state }, payload); + resolve(result); + } catch (e) { + reject(e); + } + }) + .catch(error => { + validateResults(); + throw error; + }) + .then(data => { + validateResults(); + return data; + }); }; diff --git a/spec/javascripts/helpers/vuex_action_helper_spec.js b/spec/javascripts/helpers/vuex_action_helper_spec.js new file mode 100644 index 00000000000..8d6ad6750c0 --- /dev/null +++ b/spec/javascripts/helpers/vuex_action_helper_spec.js @@ -0,0 +1,141 @@ +import MockAdapter from 'axios-mock-adapter'; +import { TEST_HOST } from 'spec/test_constants'; +import axios from '~/lib/utils/axios_utils'; +import testAction from './vuex_action_helper'; + +describe('VueX test helper (testAction)', () => { + let originalExpect; + let assertion; + let mock; + const noop = () => {}; + + beforeAll(() => { + mock = new MockAdapter(axios); + /* + In order to test the helper properly, we need to overwrite the jasmine `expect` helper. + We test that the testAction helper properly passes the dispatched actions/committed mutations + to the jasmine helper. + */ + originalExpect = expect; + assertion = null; + global.expect = actual => ({ + toEqual: () => { + originalExpect(actual).toEqual(assertion); + }, + }); + }); + + afterAll(() => { + mock.restore(); + global.expect = originalExpect; + }); + + it('should properly pass on state and payload', () => { + const exampleState = { FOO: 12, BAR: 3 }; + const examplePayload = { BAZ: 73, BIZ: 55 }; + + const action = ({ state }, payload) => { + originalExpect(state).toEqual(exampleState); + originalExpect(payload).toEqual(examplePayload); + }; + + assertion = { mutations: [], actions: [] }; + + testAction(action, examplePayload, exampleState); + }); + + describe('should work with synchronous actions', () => { + it('committing mutation', () => { + const action = ({ commit }) => { + commit('MUTATION'); + }; + + assertion = { mutations: [{ type: 'MUTATION' }], actions: [] }; + + testAction(action, null, {}, assertion.mutations, assertion.actions, noop); + }); + + it('dispatching action', () => { + const action = ({ dispatch }) => { + dispatch('ACTION'); + }; + + assertion = { actions: [{ type: 'ACTION' }], mutations: [] }; + + testAction(action, null, {}, assertion.mutations, assertion.actions, noop); + }); + + it('work with jasmine done once finished', done => { + assertion = { mutations: [], actions: [] }; + + testAction(noop, null, {}, assertion.mutations, assertion.actions, done); + }); + + it('provide promise interface', done => { + assertion = { mutations: [], actions: [] }; + + testAction(noop, null, {}, assertion.mutations, assertion.actions) + .then(done) + .catch(done.fail); + }); + }); + + describe('should work with promise based actions (fetch action)', () => { + let lastError; + const data = { FOO: 'BAR' }; + + const promiseAction = ({ commit, dispatch }) => { + dispatch('ACTION'); + + return axios + .get(TEST_HOST) + .catch(error => { + commit('ERROR'); + lastError = error; + throw error; + }) + .then(() => { + commit('SUCCESS'); + return data; + }); + }; + + beforeEach(() => { + lastError = null; + }); + + it('work with jasmine done once finished', done => { + mock.onGet(TEST_HOST).replyOnce(200, 42); + + assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; + + testAction(promiseAction, null, {}, assertion.mutations, assertion.actions, done); + }); + + it('return original data of successful promise while checking actions/mutations', done => { + mock.onGet(TEST_HOST).replyOnce(200, 42); + + assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; + + testAction(promiseAction, null, {}, assertion.mutations, assertion.actions) + .then(res => { + originalExpect(res).toEqual(data); + done(); + }) + .catch(done.fail); + }); + + it('return original error of rejected promise while checking actions/mutations', done => { + mock.onGet(TEST_HOST).replyOnce(500, ''); + + assertion = { mutations: [{ type: 'ERROR' }], actions: [{ type: 'ACTION' }] }; + + testAction(promiseAction, null, {}, assertion.mutations, assertion.actions) + .then(done.fail) + .catch(error => { + originalExpect(error).toBe(lastError); + done(); + }); + }); + }); +}); diff --git a/spec/javascripts/ide/stores/actions/file_spec.js b/spec/javascripts/ide/stores/actions/file_spec.js index 58d3ffc6d94..f570c0b16bd 100644 --- a/spec/javascripts/ide/stores/actions/file_spec.js +++ b/spec/javascripts/ide/stores/actions/file_spec.js @@ -601,10 +601,7 @@ describe('IDE store file actions', () => { actions.unstageChange, 'path', store.state, - [ - { type: types.UNSTAGE_CHANGE, payload: 'path' }, - { type: types.SET_LAST_COMMIT_MSG, payload: '' }, - ], + [{ type: types.UNSTAGE_CHANGE, payload: 'path' }], [], done, ); diff --git a/spec/javascripts/ide/stores/actions/project_spec.js b/spec/javascripts/ide/stores/actions/project_spec.js index ca79edafb7e..6a85968e199 100644 --- a/spec/javascripts/ide/stores/actions/project_spec.js +++ b/spec/javascripts/ide/stores/actions/project_spec.js @@ -73,6 +73,7 @@ describe('IDE store project actions', () => { branchId: store.state.currentBranchId, }, store.state, + // mutations [ { type: 'SET_BRANCH_COMMIT', @@ -82,17 +83,9 @@ describe('IDE store project actions', () => { commit: { id: '123' }, }, }, - ], // mutations - [ - { - type: 'getLastCommitPipeline', - payload: { - projectId: 'abc/def', - projectIdNumber: store.state.projects['abc/def'].id, - branchId: 'master', - }, - }, - ], // action + ], + // action + [], done, ); }); diff --git a/spec/javascripts/ide/stores/actions/tree_spec.js b/spec/javascripts/ide/stores/actions/tree_spec.js index 6860e6cdb91..9f098eded08 100644 --- a/spec/javascripts/ide/stores/actions/tree_spec.js +++ b/spec/javascripts/ide/stores/actions/tree_spec.js @@ -192,11 +192,8 @@ describe('Multi-file store tree actions', () => { showTreeEntry, 'grandparent/parent/child.txt', store.state, - [ - { type: types.SET_TREE_OPEN, payload: 'grandparent/parent' }, - { type: types.SET_TREE_OPEN, payload: 'grandparent' }, - ], - [{ type: 'showTreeEntry' }], + [{ type: types.SET_TREE_OPEN, payload: 'grandparent/parent' }], + [{ type: 'showTreeEntry', payload: 'grandparent/parent' }], done, ); }); diff --git a/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js index d21f33eaf6d..d063f1ea860 100644 --- a/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js @@ -122,21 +122,6 @@ describe('IDE merge requests actions', () => { }); }); - it('dispatches request', done => { - testAction( - fetchMergeRequests, - { type: 'created' }, - mockedState, - [], - [ - { type: 'requestMergeRequests' }, - { type: 'resetMergeRequests' }, - { type: 'receiveMergeRequestsSuccess' }, - ], - done, - ); - }); - it('dispatches success with received data', done => { testAction( fetchMergeRequests, @@ -144,8 +129,8 @@ describe('IDE merge requests actions', () => { mockedState, [], [ - { type: 'requestMergeRequests' }, - { type: 'resetMergeRequests' }, + { type: 'requestMergeRequests', payload: 'created' }, + { type: 'resetMergeRequests', payload: 'created' }, { type: 'receiveMergeRequestsSuccess', payload: { type: 'created', data: mergeRequests }, @@ -168,9 +153,9 @@ describe('IDE merge requests actions', () => { mockedState, [], [ - { type: 'requestMergeRequests' }, - { type: 'resetMergeRequests' }, - { type: 'receiveMergeRequestsError' }, + { type: 'requestMergeRequests', payload: 'created' }, + { type: 'resetMergeRequests', payload: 'created' }, + { type: 'receiveMergeRequestsError', payload: { type: 'created', search: '' } }, ], done, ); diff --git a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js index 836ba72b5d8..91edb388791 100644 --- a/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/pipelines/actions_spec.js @@ -315,7 +315,7 @@ describe('IDE pipelines actions', () => { 'job', mockedState, [{ type: types.SET_DETAIL_JOB, payload: 'job' }], - [{ type: 'setRightPane' }], + [{ type: 'setRightPane', payload: 'jobs-detail' }], done, ); }); @@ -325,7 +325,7 @@ describe('IDE pipelines actions', () => { setDetailJob, null, mockedState, - [{ type: types.SET_DETAIL_JOB }], + [{ type: types.SET_DETAIL_JOB, payload: null }], [{ type: 'setRightPane', payload: rightSidebarViews.pipelines }], done, ); @@ -336,7 +336,7 @@ describe('IDE pipelines actions', () => { setDetailJob, 'job', mockedState, - [{ type: types.SET_DETAIL_JOB }], + [{ type: types.SET_DETAIL_JOB, payload: 'job' }], [{ type: 'setRightPane', payload: rightSidebarViews.jobsDetail }], done, ); diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index d7c5f26ab67..77c475b9f52 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -93,4 +93,10 @@ RSpec.describe NotificationSetting do end end end + + context 'email events' do + it 'includes EXCLUDED_WATCHER_EVENTS in EMAIL_EVENTS' do + expect(described_class::EMAIL_EVENTS).to include(*described_class::EXCLUDED_WATCHER_EVENTS) + end + end end |