diff options
22 files changed, 213 insertions, 34 deletions
diff --git a/app/assets/javascripts/jira_import/components/jira_import_form.vue b/app/assets/javascripts/jira_import/components/jira_import_form.vue index b5d17398f3a..5bf98682f92 100644 --- a/app/assets/javascripts/jira_import/components/jira_import_form.vue +++ b/app/assets/javascripts/jira_import/components/jira_import_form.vue @@ -23,6 +23,7 @@ import { addInProgressImportToStore } from '../utils/cache_update'; import { debounceWait, dropdownLabel, + userMappingsPageSize, previousImportsMessage, tableConfig, userMappingMessage, @@ -74,12 +75,15 @@ export default { }, data() { return { + hasMoreUsers: false, isFetching: false, + isLoadingMoreUsers: false, isSubmitting: false, searchTerm: '', selectedProject: undefined, selectState: null, userMappings: [], + userMappingsStartAt: 0, users: [], }; }, @@ -101,6 +105,9 @@ export default { ? `jira-import::${this.selectedProject}-${this.numberOfPreviousImports + 1}` : 'jira-import::KEY-1'; }, + isInitialLoadingState() { + return this.isLoadingMoreUsers && !this.hasMoreUsers; + }, }, watch: { searchTerm: debounce(function debouncedUserSearch() { @@ -108,23 +115,7 @@ export default { }, debounceWait), }, mounted() { - this.$apollo - .mutate({ - mutation: getJiraUserMappingMutation, - variables: { - input: { - projectPath: this.projectPath, - }, - }, - }) - .then(({ data }) => { - if (data.jiraImportUsers.errors.length) { - this.$emit('error', data.jiraImportUsers.errors.join('. ')); - } else { - this.userMappings = data.jiraImportUsers.jiraUsers; - } - }) - .catch(() => this.$emit('error', __('There was an error retrieving the Jira users.'))); + this.getJiraUserMapping(); this.searchUsers() .then(data => { @@ -133,6 +124,36 @@ export default { .catch(() => {}); }, methods: { + getJiraUserMapping() { + this.isLoadingMoreUsers = true; + + this.$apollo + .mutate({ + mutation: getJiraUserMappingMutation, + variables: { + input: { + projectPath: this.projectPath, + startAt: this.userMappingsStartAt, + }, + }, + }) + .then(({ data }) => { + if (data.jiraImportUsers.errors.length) { + this.$emit('error', data.jiraImportUsers.errors.join('. ')); + return; + } + + this.userMappings = this.userMappings.concat(data.jiraImportUsers.jiraUsers); + this.hasMoreUsers = data.jiraImportUsers.jiraUsers.length === userMappingsPageSize; + this.userMappingsStartAt += userMappingsPageSize; + }) + .catch(() => { + this.$emit('error', __('There was an error retrieving the Jira users.')); + }) + .finally(() => { + this.isLoadingMoreUsers = false; + }); + }, searchUsers() { const params = { active: true, @@ -187,7 +208,9 @@ export default { this.selectedProject = undefined; } }) - .catch(() => this.$emit('error', __('There was an error importing the Jira project.'))) + .catch(() => { + this.$emit('error', __('There was an error importing the Jira project.')); + }) .finally(() => { this.isSubmitting = false; }); @@ -280,9 +303,7 @@ export default { > <gl-search-box-by-type v-model.trim="searchTerm" class="m-2" /> - <div v-if="isFetching" class="gl-text-center"> - <gl-loading-icon /> - </div> + <gl-loading-icon v-if="isFetching" /> <gl-new-dropdown-item v-for="user in users" @@ -300,6 +321,17 @@ export default { </template> </gl-table> + <gl-loading-icon v-if="isInitialLoadingState" /> + + <gl-button + v-if="hasMoreUsers" + :loading="isLoadingMoreUsers" + data-testid="load-more-users-button" + @click="getJiraUserMapping" + > + {{ __('Load more users') }} + </gl-button> + <div class="footer-block row-content-block d-flex justify-content-between"> <gl-button type="submit" diff --git a/app/assets/javascripts/jira_import/utils/constants.js b/app/assets/javascripts/jira_import/utils/constants.js index 6adc3e5306c..178159be009 100644 --- a/app/assets/javascripts/jira_import/utils/constants.js +++ b/app/assets/javascripts/jira_import/utils/constants.js @@ -27,3 +27,6 @@ export const tableConfig = [ export const userMappingMessage = __(`Jira users have been imported from the configured Jira instance. They can be mapped by selecting a GitLab user from the dropdown in the "GitLab username" column. When the form appears, the dropdown defaults to the user conducting the import.`); + +// pageSize must match the MAX_USERS value in app/services/jira_import/users_mapper_service.rb +export const userMappingsPageSize = 50; diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 2cc51c65c26..b93c98a4790 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -129,6 +129,10 @@ module AuthenticatesWithTwoFactor def user_changed?(user) return false unless session[:user_updated_at] - user.updated_at != session[:user_updated_at] + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/244638 + # Rounding errors happen when the user is updated, as the Rails ActiveRecord + # object has higher precision than what is stored in the database, therefore + # using .to_i to force truncation to the timestamp + user.updated_at.to_i != session[:user_updated_at].to_i end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 661b10019ad..eb46be65858 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -426,6 +426,8 @@ class ApplicationSetting < ApplicationRecord end def self.create_from_defaults + check_schema! + transaction(requires_new: true) do super end @@ -434,6 +436,22 @@ class ApplicationSetting < ApplicationRecord current_without_cache end + # Due to the frequency with which settings are accessed, it is + # likely that during a backup restore a running GitLab process + # will insert a new `application_settings` row before the + # constraints have been added to the table. This would add an + # extra row with ID 1 and prevent the primary key constraint from + # being added, which made ActiveRecord throw a + # IrreversibleOrderError anytime the settings were accessed + # (https://gitlab.com/gitlab-org/gitlab/-/issues/36405). To + # prevent this from happening, we do a sanity check that the + # primary key constraint is present before inserting a new entry. + def self.check_schema! + return if ActiveRecord::Base.connection.primary_key(self.table_name).present? + + raise "The `#{self.table_name}` table is missing a primary key constraint in the database schema" + end + # By default, the backend is Rails.cache, which uses # ActiveSupport::Cache::RedisStore. Since loading ApplicationSetting # can cause a significant amount of load on Redis, let's cache it in diff --git a/app/services/git/process_ref_changes_service.rb b/app/services/git/process_ref_changes_service.rb index c012c61a337..d039ed00efc 100644 --- a/app/services/git/process_ref_changes_service.rb +++ b/app/services/git/process_ref_changes_service.rb @@ -42,7 +42,7 @@ module Git push_service_class = push_service_class_for(ref_type) create_bulk_push_event = changes.size > Gitlab::CurrentSettings.push_event_activities_limit - merge_request_branches = merge_request_branches_for(changes) + merge_request_branches = merge_request_branches_for(ref_type, changes) changes.each do |change| push_service_class.new( @@ -74,8 +74,10 @@ module Git Git::BranchPushService end - def merge_request_branches_for(changes) - @merge_requests_branches ||= MergeRequests::PushedBranchesService.new(project, current_user, changes: changes).execute + def merge_request_branches_for(ref_type, changes) + return [] if ref_type == :tag + + MergeRequests::PushedBranchesService.new(project, current_user, changes: changes).execute end end end diff --git a/app/services/jira_import/users_mapper_service.rb b/app/services/jira_import/users_mapper_service.rb index b5997d77215..480c034f952 100644 --- a/app/services/jira_import/users_mapper_service.rb +++ b/app/services/jira_import/users_mapper_service.rb @@ -2,6 +2,7 @@ module JiraImport class UsersMapperService + # MAX_USERS must match the pageSize value in app/assets/javascripts/jira_import/utils/constants.js MAX_USERS = 50 attr_reader :jira_service, :start_at diff --git a/changelogs/unreleased/235889-jira-importer-user-mapping-shows-50-users-max.yml b/changelogs/unreleased/235889-jira-importer-user-mapping-shows-50-users-max.yml new file mode 100644 index 00000000000..1fcf6c768e7 --- /dev/null +++ b/changelogs/unreleased/235889-jira-importer-user-mapping-shows-50-users-max.yml @@ -0,0 +1,5 @@ +--- +title: Fix Jira importer user mapping limit +merge_request: 40310 +author: +type: fixed diff --git a/changelogs/unreleased/bump-ado-image-to-v1-0-2.yml b/changelogs/unreleased/bump-ado-image-to-v1-0-2.yml new file mode 100644 index 00000000000..d24e9a1e6c3 --- /dev/null +++ b/changelogs/unreleased/bump-ado-image-to-v1-0-2.yml @@ -0,0 +1,5 @@ +--- +title: Fix auto-deploy-image external chart dependencies +merge_request: 40730 +author: +type: fixed diff --git a/changelogs/unreleased/cat-time-precision-2fa-ldap.yml b/changelogs/unreleased/cat-time-precision-2fa-ldap.yml new file mode 100644 index 00000000000..dc2cdaa8632 --- /dev/null +++ b/changelogs/unreleased/cat-time-precision-2fa-ldap.yml @@ -0,0 +1,5 @@ +--- +title: Update the 2FA user update check to account for rounding errors +merge_request: 41327 +author: +type: fixed diff --git a/changelogs/unreleased/id-remove-memoize-on-processing-ref-changes.yml b/changelogs/unreleased/id-remove-memoize-on-processing-ref-changes.yml new file mode 100644 index 00000000000..2c52d5f3f61 --- /dev/null +++ b/changelogs/unreleased/id-remove-memoize-on-processing-ref-changes.yml @@ -0,0 +1,5 @@ +--- +title: Fix wrong caching logic in ProcessRefChangesService +merge_request: 40821 +author: +type: fixed diff --git a/changelogs/unreleased/sh-coerce-object-storage-params.yml b/changelogs/unreleased/sh-coerce-object-storage-params.yml new file mode 100644 index 00000000000..872441a1064 --- /dev/null +++ b/changelogs/unreleased/sh-coerce-object-storage-params.yml @@ -0,0 +1,5 @@ +--- +title: Coerce string object storage options to booleans +merge_request: 39901 +author: +type: fixed diff --git a/changelogs/unreleased/sh-fix-backup-restore-race.yml b/changelogs/unreleased/sh-fix-backup-restore-race.yml new file mode 100644 index 00000000000..ab5d4d8fcb0 --- /dev/null +++ b/changelogs/unreleased/sh-fix-backup-restore-race.yml @@ -0,0 +1,5 @@ +--- +title: Fix ActiveRecord::IrreversibleOrderError during restore from backup +merge_request: 40789 +author: +type: fixed diff --git a/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml index f234008dad4..e9d77766db3 100644 --- a/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml @@ -1,5 +1,5 @@ .dast-auto-deploy: - image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v1.0.0" + image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v1.0.2" dast_environment_deploy: extends: .dast-auto-deploy diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml index 76fb2948144..41120750ff4 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml @@ -1,5 +1,5 @@ .auto-deploy: - image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v1.0.0" + image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v1.0.2" dependencies: [] include: diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e4af61a0f35..ed9d4b43f5a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14501,6 +14501,9 @@ msgstr "" msgid "Load more" msgstr "" +msgid "Load more users" +msgstr "" + msgid "Loading" msgstr "" diff --git a/spec/frontend/jira_import/components/jira_import_form_spec.js b/spec/frontend/jira_import/components/jira_import_form_spec.js index 7cc7b40f4c8..6ef28a71f48 100644 --- a/spec/frontend/jira_import/components/jira_import_form_spec.js +++ b/spec/frontend/jira_import/components/jira_import_form_spec.js @@ -10,6 +10,7 @@ import { imports, issuesPath, jiraProjects, + jiraUsersResponse, projectId, projectPath, userMappings as defaultUserMappings, @@ -38,7 +39,10 @@ describe('JiraImportForm', () => { const getHeader = name => getByRole(wrapper.element, 'columnheader', { name }); + const findLoadMoreUsersButton = () => wrapper.find('[data-testid="load-more-users-button"]'); + const mountComponent = ({ + hasMoreUsers = false, isSubmitting = false, loading = false, mutate = mutateSpy, @@ -55,6 +59,7 @@ describe('JiraImportForm', () => { projectPath, }, data: () => ({ + hasMoreUsers, isFetching: false, isSubmitting, searchTerm: '', @@ -300,6 +305,7 @@ describe('JiraImportForm', () => { variables: { input: { projectPath, + startAt: 0, }, }, }; @@ -318,4 +324,53 @@ describe('JiraImportForm', () => { }); }); }); + + describe('load more users button', () => { + describe('when all users have been loaded', () => { + it('is not shown', () => { + wrapper = mountComponent(); + + expect(findLoadMoreUsersButton().exists()).toBe(false); + }); + }); + + describe('when all users have not been loaded', () => { + it('is shown', () => { + wrapper = mountComponent({ hasMoreUsers: true }); + + expect(findLoadMoreUsersButton().exists()).toBe(true); + }); + }); + + describe('when clicked', () => { + beforeEach(() => { + mutateSpy = jest.fn(() => + Promise.resolve({ + data: { + jiraImportStart: { errors: [] }, + jiraImportUsers: { jiraUsers: jiraUsersResponse, errors: [] }, + }, + }), + ); + + wrapper = mountComponent({ hasMoreUsers: true }); + }); + + it('calls the GraphQL user mapping mutation', async () => { + const mutationArguments = { + mutation: getJiraUserMappingMutation, + variables: { + input: { + projectPath, + startAt: 0, + }, + }, + }; + + findLoadMoreUsersButton().vm.$emit('click'); + + expect(mutateSpy).toHaveBeenCalledWith(expect.objectContaining(mutationArguments)); + }); + }); + }); }); diff --git a/spec/frontend/jira_import/mock_data.js b/spec/frontend/jira_import/mock_data.js index 8ea40080f32..51dd939283e 100644 --- a/spec/frontend/jira_import/mock_data.js +++ b/spec/frontend/jira_import/mock_data.js @@ -1,5 +1,6 @@ import getJiraImportDetailsQuery from '~/jira_import/queries/get_jira_import_details.query.graphql'; import { IMPORT_STATE } from '~/jira_import/utils/jira_import_utils'; +import { userMappingsPageSize } from '~/jira_import/utils/constants'; export const fullPath = 'gitlab-org/gitlab-test'; @@ -87,6 +88,8 @@ export const jiraProjects = [ { text: 'Migrate to GitLab (MTG)', value: 'MTG' }, ]; +export const jiraUsersResponse = new Array(userMappingsPageSize); + export const imports = [ { jiraProjectKey: 'MTG', diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index fd4e8bc1cd0..786db23ffc4 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -115,6 +115,16 @@ RSpec.describe Gitlab::CurrentSettings do expect(settings).to have_attributes(settings_from_defaults) end + context 'when ApplicationSettings does not have a primary key' do + before do + allow(ActiveRecord::Base.connection).to receive(:primary_key).with('application_settings').and_return(nil) + end + + it 'raises an exception if ApplicationSettings does not have a primary key' do + expect { described_class.current_application_settings }.to raise_error(/table is missing a primary key constraint/) + end + end + context 'with pending migrations' do let(:current_settings) { described_class.current_application_settings } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index bcd8eccd68f..ab25608e2f0 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -650,6 +650,16 @@ RSpec.describe ApplicationSetting do end end + context 'when ApplicationSettings does not have a primary key' do + before do + allow(ActiveRecord::Base.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil) + end + + it 'raises an exception' do + expect { described_class.create_from_defaults }.to raise_error(/table is missing a primary key constraint/) + end + end + describe '#disabled_oauth_sign_in_sources=' do before do allow(Devise).to receive(:omniauth_providers).and_return([:github]) diff --git a/spec/services/git/process_ref_changes_service_spec.rb b/spec/services/git/process_ref_changes_service_spec.rb index fc313bf6eb9..087f4ba372b 100644 --- a/spec/services/git/process_ref_changes_service_spec.rb +++ b/spec/services/git/process_ref_changes_service_spec.rb @@ -172,23 +172,31 @@ RSpec.describe Git::ProcessRefChangesService do [ { index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "#{ref_prefix}/create1" }, { index: 1, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789013', ref: "#{ref_prefix}/create2" }, - { index: 2, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789014', ref: "#{ref_prefix}/create3" } + { index: 2, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789014', ref: "#{ref_prefix}/create3" }, + { index: 3, oldrev: '789015', newrev: '789016', ref: "#{ref_prefix}/changed1" }, + { index: 4, oldrev: '789017', newrev: '789018', ref: "#{ref_prefix}/changed2" }, + { index: 5, oldrev: '789019', newrev: Gitlab::Git::BLANK_SHA, ref: "#{ref_prefix}/removed1" }, + { index: 6, oldrev: '789020', newrev: Gitlab::Git::BLANK_SHA, ref: "#{ref_prefix}/removed2" } ] end let(:git_changes) { double(branch_changes: branch_changes, tag_changes: tag_changes) } - it 'schedules job for existing merge requests' do - expect_next_instance_of(MergeRequests::PushedBranchesService) do |service| - expect(service).to receive(:execute).and_return(%w(create1 create2)) - end + before do + allow(MergeRequests::PushedBranchesService).to receive(:new).and_return( + double(execute: %w(create1 create2)), double(execute: %w(changed1)), double(execute: %w(removed2)) + ) + end + it 'schedules job for existing merge requests' do expect(UpdateMergeRequestsWorker).to receive(:perform_async) .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789012', "#{ref_prefix}/create1").ordered expect(UpdateMergeRequestsWorker).to receive(:perform_async) .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789013', "#{ref_prefix}/create2").ordered - expect(UpdateMergeRequestsWorker).not_to receive(:perform_async) - .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789014', "#{ref_prefix}/create3").ordered + expect(UpdateMergeRequestsWorker).to receive(:perform_async) + .with(project.id, user.id, '789015', '789016', "#{ref_prefix}/changed1").ordered + expect(UpdateMergeRequestsWorker).to receive(:perform_async) + .with(project.id, user.id, '789020', Gitlab::Git::BLANK_SHA, "#{ref_prefix}/removed2").ordered subject.execute end diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore index 259148fa18f..259148fa18f 100755..100644 --- a/vendor/gitignore/C++.gitignore +++ b/vendor/gitignore/C++.gitignore diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore index a1c2a238a96..a1c2a238a96 100755..100644 --- a/vendor/gitignore/Java.gitignore +++ b/vendor/gitignore/Java.gitignore |