diff options
23 files changed, 243 insertions, 122 deletions
diff --git a/app/assets/javascripts/notes/components/note_awards_list.vue b/app/assets/javascripts/notes/components/note_awards_list.vue index d4a57d5d58d..df62e379017 100644 --- a/app/assets/javascripts/notes/components/note_awards_list.vue +++ b/app/assets/javascripts/notes/components/note_awards_list.vue @@ -116,16 +116,20 @@ export default { // We have 10+ awarded user, join them with comma and add `and x more`. if (remainingAwardList.length) { - title = sprintf(__(`%{listToShow}, and %{awardsListLength} more.`), { - listToShow: namesToShow.join(', '), - awardsListLength: remainingAwardList.length, - }); + title = sprintf( + __(`%{listToShow}, and %{awardsListLength} more.`), + { + listToShow: namesToShow.join(', '), + awardsListLength: remainingAwardList.length, + }, + false, + ); } else if (namesToShow.length > 1) { // Join all names with comma but not the last one, it will be added with and text. title = namesToShow.slice(0, namesToShow.length - 1).join(', '); // If we have more than 2 users we need an extra comma before and text. title += namesToShow.length > 2 ? ',' : ''; - title += sprintf(__(` and %{sliced}`), { sliced: namesToShow.slice(-1) }); // Append and text + title += sprintf(__(` and %{sliced}`), { sliced: namesToShow.slice(-1) }, false); // Append and text } else { // We have only 2 users so join them with and. title = namesToShow.join(__(' and ')); diff --git a/app/models/repository.rb b/app/models/repository.rb index b9f57169ea5..5e547cf509b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1052,18 +1052,19 @@ class Repository return rebase_deprecated(user, merge_request) end - MergeRequest.transaction do - raw.rebase( - user, - merge_request.id, - branch: merge_request.source_branch, - branch_sha: merge_request.source_branch_sha, - remote_repository: merge_request.target_project.repository.raw, - remote_branch: merge_request.target_branch - ) do |commit_id| - merge_request.update!(rebase_commit_sha: commit_id, merge_error: nil) - end + raw.rebase( + user, + merge_request.id, + branch: merge_request.source_branch, + branch_sha: merge_request.source_branch_sha, + remote_repository: merge_request.target_project.repository.raw, + remote_branch: merge_request.target_branch + ) do |commit_id| + merge_request.update!(rebase_commit_sha: commit_id, merge_error: nil) end + rescue StandardError => error + merge_request.update!(rebase_commit_sha: nil) + raise error end def squash(user, merge_request, message) diff --git a/app/policies/personal_snippet_policy.rb b/app/policies/personal_snippet_policy.rb index 91a8f3a7133..03869412d0c 100644 --- a/app/policies/personal_snippet_policy.rb +++ b/app/policies/personal_snippet_policy.rb @@ -13,7 +13,6 @@ class PersonalSnippetPolicy < BasePolicy rule { is_author | admin }.policy do enable :read_personal_snippet enable :update_personal_snippet - enable :destroy_personal_snippet enable :admin_personal_snippet enable :create_note end diff --git a/changelogs/unreleased/30036-award-emoji-tooltip-is-escaped-twice.yml b/changelogs/unreleased/30036-award-emoji-tooltip-is-escaped-twice.yml new file mode 100644 index 00000000000..6a8ee6e889c --- /dev/null +++ b/changelogs/unreleased/30036-award-emoji-tooltip-is-escaped-twice.yml @@ -0,0 +1,5 @@ +--- +title: Fix award emoji tooltip being escaped twice if multiple people voted +merge_request: 19273 +author: Brian T +type: fixed diff --git a/changelogs/unreleased/fj-37254-remove-delete-personal-snippet.yml b/changelogs/unreleased/fj-37254-remove-delete-personal-snippet.yml new file mode 100644 index 00000000000..2a9f26c870d --- /dev/null +++ b/changelogs/unreleased/fj-37254-remove-delete-personal-snippet.yml @@ -0,0 +1,5 @@ +--- +title: Remove destroy_personal_snippet ability +merge_request: 20717 +author: +type: fixed diff --git a/changelogs/unreleased/osw-remove-transaction-while-rebasing.yml b/changelogs/unreleased/osw-remove-transaction-while-rebasing.yml new file mode 100644 index 00000000000..9cb9e26287d --- /dev/null +++ b/changelogs/unreleased/osw-remove-transaction-while-rebasing.yml @@ -0,0 +1,5 @@ +--- +title: Remove DB transaction from Rebase operation +merge_request: 20739 +author: +type: fixed diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index c3ca9bf3e05..35e67e04bfb 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -31,7 +31,7 @@ enable_json_logs = Gitlab.config.sidekiq.log_format == 'json' enable_sidekiq_memory_killer = ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS'].to_i.nonzero? use_sidekiq_daemon_memory_killer = ENV["SIDEKIQ_DAEMON_MEMORY_KILLER"].to_i.nonzero? use_sidekiq_legacy_memory_killer = !use_sidekiq_daemon_memory_killer -use_request_store = ENV['SIDEKIQ_REQUEST_STORE'].to_i.nonzero? +use_request_store = ENV.fetch('SIDEKIQ_REQUEST_STORE', 1).to_i.nonzero? Sidekiq.configure_server do |config| config.redis = queues_config_hash diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index f83dc556964..af95f9d6f76 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -51,14 +51,16 @@ def presented_no_changelog_labels NO_CHANGELOG_LABELS.map { |label| "~#{label}" }.join(', ') end +def sanitized_mr_title + gitlab.mr_json["title"].gsub(/^WIP: */, '').gsub(/`/, '\\\`') +end + changelog_needed = (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty? changelog_found = git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} } -mr_title = gitlab.mr_json["title"].gsub(/^WIP: */, '') - if git.modified_files.include?("CHANGELOG.md") fail "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" + - format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: mr_title, labels: presented_no_changelog_labels) + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: presented_no_changelog_labels) end if changelog_needed @@ -66,6 +68,6 @@ if changelog_needed check_changelog(changelog_found) else message "**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html)**: If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.\n\n" + - format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: mr_title, labels: presented_no_changelog_labels) + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: presented_no_changelog_labels) end end diff --git a/doc/push_rules/push_rules.md b/doc/push_rules/push_rules.md index 0771a3e4225..d778d6b929c 100644 --- a/doc/push_rules/push_rules.md +++ b/doc/push_rules/push_rules.md @@ -49,6 +49,13 @@ branch names globally in Push Rules, you can now sleep without the anxiety of your developers' mistakes. Every branch that doesn't match your push rule will get rejected. +### Custom Push Rules **(CORE ONLY)** + +It's possible to create custom push rules rather than the push rules available in +**Admin area > Push Rules** by using more advanced server-side Git hooks. + +See [custom server-side Git hooks](../administration/custom_hooks.md) for more information. + ## Enabling push rules NOTE: **Note:** diff --git a/doc/user/project/operations/feature_flags.md b/doc/user/project/operations/feature_flags.md index c05f8fa8bc4..723f9d69995 100644 --- a/doc/user/project/operations/feature_flags.md +++ b/doc/user/project/operations/feature_flags.md @@ -75,7 +75,7 @@ To define specs for each environment: 1. Set the status and rollout strategy of the additional spec. This status and rollout strategy combination takes precedence over the default spec since we always use the most specific match available. 1. Click **Create feature flag** or **Update feature flag**. - + NOTE: **NOTE** We'd highly recommend you to use the [Environment](../../../ci/environments.md) @@ -119,17 +119,15 @@ CAUTION: **Caution:** If this strategy is selected, then the Unleash client **must** be given a user ID for the feature to be enabled. See the [Ruby example](#ruby-application-example) below. -### Target users strategy +#### User IDs -> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/8240) in GitLab 12.2. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/8240) in GitLab 12.2. [Updated](https://gitlab.com/gitlab-org/gitlab/issues/34363) to be defined per environment in GitLab 12.6. A feature flag may be enabled for a list of target users. It is implemented using the Unleash [`userWithId`](https://unleash.github.io/docs/activation_strategy#userwithid) activation strategy. -The feature will always be enabled for all users in the list across all environments even if the matching environment spec **Status** is disabled. - - +User IDs should be a comma separated list of values. For example, `user@example.com, user2@example.com`, or `username1,username2,username3`, etc. CAUTION: **Caution:** The Unleash client **must** be given a user ID for the feature to be enabled for diff --git a/doc/user/project/operations/img/specs_list.png b/doc/user/project/operations/img/specs_list.png Binary files differdeleted file mode 100644 index 43d069c09ce..00000000000 --- a/doc/user/project/operations/img/specs_list.png +++ /dev/null diff --git a/doc/user/project/operations/img/specs_list_v12_6.png b/doc/user/project/operations/img/specs_list_v12_6.png Binary files differnew file mode 100644 index 00000000000..ea429802a40 --- /dev/null +++ b/doc/user/project/operations/img/specs_list_v12_6.png diff --git a/doc/user/project/operations/img/target_users_v12_2.png b/doc/user/project/operations/img/target_users_v12_2.png Binary files differdeleted file mode 100644 index c88d2b7be97..00000000000 --- a/doc/user/project/operations/img/target_users_v12_2.png +++ /dev/null diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index f759b3607a8..fd5422f2e2c 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -131,7 +131,7 @@ module API snippet = snippets_for_current_user.find_by_id(params.delete(:id)) break not_found!('Snippet') unless snippet - authorize! :destroy_personal_snippet, snippet + authorize! :admin_personal_snippet, snippet destroy_conditionally!(snippet) end diff --git a/spec/frontend/environments/environment_monitoring_spec.js b/spec/frontend/environments/environment_monitoring_spec.js new file mode 100644 index 00000000000..8e67f799dc0 --- /dev/null +++ b/spec/frontend/environments/environment_monitoring_spec.js @@ -0,0 +1,39 @@ +import { shallowMount } from '@vue/test-utils'; +import MonitoringComponent from '~/environments/components/environment_monitoring.vue'; +import Icon from '~/vue_shared/components/icon.vue'; + +describe('Monitoring Component', () => { + let wrapper; + + const monitoringUrl = 'https://gitlab.com'; + + const createWrapper = () => { + wrapper = shallowMount(MonitoringComponent, { + sync: false, + attachToDocument: true, + propsData: { + monitoringUrl, + }, + }); + }; + + const findIcons = () => wrapper.findAll(Icon); + const findIconsByName = name => findIcons().filter(icon => icon.props('name') === name); + + beforeEach(() => { + createWrapper(); + }); + + describe('computed', () => { + it('title', () => { + expect(wrapper.vm.title).toBe('Monitoring'); + }); + }); + + it('should render a link to environment monitoring page', () => { + expect(wrapper.attributes('href')).toEqual(monitoringUrl); + expect(findIconsByName('chart').length).toBe(1); + expect(wrapper.attributes('data-original-title')).toBe('Monitoring'); + expect(wrapper.attributes('aria-label')).toBe('Monitoring'); + }); +}); diff --git a/spec/frontend/environments/environment_stop_spec.js b/spec/frontend/environments/environment_stop_spec.js new file mode 100644 index 00000000000..ab714728311 --- /dev/null +++ b/spec/frontend/environments/environment_stop_spec.js @@ -0,0 +1,40 @@ +import $ from 'jquery'; +import { shallowMount } from '@vue/test-utils'; +import StopComponent from '~/environments/components/environment_stop.vue'; +import LoadingButton from '~/vue_shared/components/loading_button.vue'; +import eventHub from '~/environments/event_hub'; + +$.fn.tooltip = () => {}; + +describe('Stop Component', () => { + let wrapper; + + const createWrapper = () => { + wrapper = shallowMount(StopComponent, { + sync: false, + attachToDocument: true, + propsData: { + environment: {}, + }, + }); + }; + + const findButton = () => wrapper.find(LoadingButton); + + beforeEach(() => { + jest.spyOn(window, 'confirm'); + + createWrapper(); + }); + + it('should render a button to stop the environment', () => { + expect(findButton().exists()).toBe(true); + expect(wrapper.attributes('data-original-title')).toEqual('Stop environment'); + }); + + it('emits requestStopEnvironment in the event hub when button is clicked', () => { + jest.spyOn(eventHub, '$emit'); + findButton().vm.$emit('click'); + expect(eventHub.$emit).toHaveBeenCalledWith('requestStopEnvironment', wrapper.vm.environment); + }); +}); diff --git a/spec/frontend/environments/environment_terminal_button_spec.js b/spec/frontend/environments/environment_terminal_button_spec.js new file mode 100644 index 00000000000..9aa2b82736c --- /dev/null +++ b/spec/frontend/environments/environment_terminal_button_spec.js @@ -0,0 +1,36 @@ +import { shallowMount } from '@vue/test-utils'; +import TerminalComponent from '~/environments/components/environment_terminal_button.vue'; + +describe('Stop Component', () => { + let wrapper; + const terminalPath = '/path'; + + const mountWithProps = props => { + wrapper = shallowMount(TerminalComponent, { + sync: false, + attachToDocument: true, + propsData: props, + }); + }; + + beforeEach(() => { + mountWithProps({ terminalPath }); + }); + + describe('computed', () => { + it('title', () => { + expect(wrapper.vm.title).toEqual('Terminal'); + }); + }); + + it('should render a link to open a web terminal with the provided path', () => { + expect(wrapper.is('a')).toBe(true); + expect(wrapper.attributes('data-original-title')).toBe('Terminal'); + expect(wrapper.attributes('aria-label')).toBe('Terminal'); + expect(wrapper.attributes('href')).toBe(terminalPath); + }); + + it('should render a non-disabled button', () => { + expect(wrapper.classes()).not.toContain('disabled'); + }); +}); diff --git a/spec/javascripts/environments/environment_monitoring_spec.js b/spec/javascripts/environments/environment_monitoring_spec.js deleted file mode 100644 index f8d8223967a..00000000000 --- a/spec/javascripts/environments/environment_monitoring_spec.js +++ /dev/null @@ -1,32 +0,0 @@ -import Vue from 'vue'; -import monitoringComp from '~/environments/components/environment_monitoring.vue'; - -describe('Monitoring Component', () => { - let MonitoringComponent; - let component; - - const monitoringUrl = 'https://gitlab.com'; - - beforeEach(() => { - MonitoringComponent = Vue.extend(monitoringComp); - - component = new MonitoringComponent({ - propsData: { - monitoringUrl, - }, - }).$mount(); - }); - - describe('computed', () => { - it('title', () => { - expect(component.title).toEqual('Monitoring'); - }); - }); - - it('should render a link to environment monitoring page', () => { - expect(component.$el.getAttribute('href')).toEqual(monitoringUrl); - expect(component.$el.querySelector('.fa-area-chart')).toBeDefined(); - expect(component.$el.getAttribute('data-original-title')).toEqual('Monitoring'); - expect(component.$el.getAttribute('aria-label')).toEqual('Monitoring'); - }); -}); diff --git a/spec/javascripts/environments/environment_stop_spec.js b/spec/javascripts/environments/environment_stop_spec.js deleted file mode 100644 index 4d9caa57566..00000000000 --- a/spec/javascripts/environments/environment_stop_spec.js +++ /dev/null @@ -1,23 +0,0 @@ -import Vue from 'vue'; -import stopComp from '~/environments/components/environment_stop.vue'; - -describe('Stop Component', () => { - let StopComponent; - let component; - - beforeEach(() => { - StopComponent = Vue.extend(stopComp); - spyOn(window, 'confirm').and.returnValue(true); - - component = new StopComponent({ - propsData: { - environment: {}, - }, - }).$mount(); - }); - - it('should render a button to stop the environment', () => { - expect(component.$el.tagName).toEqual('BUTTON'); - expect(component.$el.getAttribute('data-original-title')).toEqual('Stop environment'); - }); -}); diff --git a/spec/javascripts/environments/environment_terminal_button_spec.js b/spec/javascripts/environments/environment_terminal_button_spec.js deleted file mode 100644 index fc98e656efe..00000000000 --- a/spec/javascripts/environments/environment_terminal_button_spec.js +++ /dev/null @@ -1,35 +0,0 @@ -import Vue from 'vue'; -import terminalComp from '~/environments/components/environment_terminal_button.vue'; - -describe('Stop Component', () => { - let component; - const terminalPath = '/path'; - - const mountWithProps = props => { - const TerminalComponent = Vue.extend(terminalComp); - component = new TerminalComponent({ - propsData: props, - }).$mount(); - }; - - beforeEach(() => { - mountWithProps({ terminalPath }); - }); - - describe('computed', () => { - it('title', () => { - expect(component.title).toEqual('Terminal'); - }); - }); - - it('should render a link to open a web terminal with the provided path', () => { - expect(component.$el.tagName).toEqual('A'); - expect(component.$el.getAttribute('data-original-title')).toEqual('Terminal'); - expect(component.$el.getAttribute('aria-label')).toEqual('Terminal'); - expect(component.$el.getAttribute('href')).toEqual(terminalPath); - }); - - it('should render a non-disabled button', () => { - expect(component.$el.classList).not.toContain('disabled'); - }); -}); diff --git a/spec/javascripts/notes/components/note_awards_list_spec.js b/spec/javascripts/notes/components/note_awards_list_spec.js index ede541a5247..90aa1684272 100644 --- a/spec/javascripts/notes/components/note_awards_list_spec.js +++ b/spec/javascripts/notes/components/note_awards_list_spec.js @@ -61,6 +61,66 @@ describe('note_awards_list component', () => { expect(vm.$el.querySelector('.js-add-award')).toBeDefined(); }); + describe('when the user name contains special HTML characters', () => { + const createAwardEmoji = (_, index) => ({ + name: 'art', + user: { id: index, name: `&<>"\`'-${index}`, username: `user-${index}` }, + }); + + const mountComponent = () => { + const Component = Vue.extend(awardsNote); + vm = new Component({ + store, + propsData: { + awards: awardsMock, + noteAuthorId: 0, + noteId: '545', + canAwardEmoji: true, + toggleAwardPath: '/gitlab-org/gitlab-foss/notes/545/toggle_award_emoji', + }, + }).$mount(); + }; + + const findTooltip = () => + vm.$el.querySelector('[data-original-title]').getAttribute('data-original-title'); + + it('should only escape & and " characters', () => { + awardsMock = [...new Array(1)].map(createAwardEmoji); + mountComponent(); + const escapedName = awardsMock[0].user.name.replace(/&/g, '&').replace(/"/g, '"'); + + expect(vm.$el.querySelector('[data-original-title]').outerHTML).toContain(escapedName); + }); + + it('should not escape special HTML characters twice when only 1 person awarded', () => { + awardsMock = [...new Array(1)].map(createAwardEmoji); + mountComponent(); + + awardsMock.forEach(award => { + expect(findTooltip()).toContain(award.user.name); + }); + }); + + it('should not escape special HTML characters twice when 2 people awarded', () => { + awardsMock = [...new Array(2)].map(createAwardEmoji); + mountComponent(); + + awardsMock.forEach(award => { + expect(findTooltip()).toContain(award.user.name); + }); + }); + + it('should not escape special HTML characters twice when more than 10 people awarded', () => { + awardsMock = [...new Array(11)].map(createAwardEmoji); + mountComponent(); + + // Testing only the first 10 awards since 11 onward will not be displayed. + awardsMock.slice(0, 10).forEach(award => { + expect(findTooltip()).toContain(award.user.name); + }); + }); + }); + describe('when the user cannot award emoji', () => { beforeEach(() => { const Component = Vue.extend(awardsNote); diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9cb3229aeb1..bad05990965 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1530,7 +1530,7 @@ describe Repository do expect(merge_request.reload.rebase_commit_sha).to eq(new_sha) end - it 'does rollback when an error is encountered in the second step' do + it 'does rollback when a PreReceiveError is encountered in the second step' do second_response = double(pre_receive_error: 'my_error', git_error: nil) mock_gitaly(second_response) @@ -1541,6 +1541,17 @@ describe Repository do expect(merge_request.reload.rebase_commit_sha).to be_nil end + it 'does rollback when a GitError is encountered in the second step' do + second_response = double(pre_receive_error: nil, git_error: 'git error') + mock_gitaly(second_response) + + expect do + repository.rebase(user, merge_request) + end.to raise_error(Gitlab::Git::Repository::GitError) + + expect(merge_request.reload.rebase_commit_sha).to be_nil + end + def mock_gitaly(second_response) responses = [ double(rebase_sha: new_sha).as_null_object, diff --git a/spec/policies/personal_snippet_policy_spec.rb b/spec/policies/personal_snippet_policy_spec.rb index 36b4ac16cf0..0abdf7382be 100644 --- a/spec/policies/personal_snippet_policy_spec.rb +++ b/spec/policies/personal_snippet_policy_spec.rb @@ -11,8 +11,7 @@ describe PersonalSnippetPolicy do let(:author_permissions) do [ :update_personal_snippet, - :admin_personal_snippet, - :destroy_personal_snippet + :admin_personal_snippet ] end |
