diff options
29 files changed, 195 insertions, 38 deletions
diff --git a/Dangerfile b/Dangerfile index d0a605f8d8e..094d55e8652 100644 --- a/Dangerfile +++ b/Dangerfile @@ -19,4 +19,5 @@ unless helper.release_automation? danger.import_dangerfile(path: 'danger/single_codebase') danger.import_dangerfile(path: 'danger/gitlab_ui_wg') danger.import_dangerfile(path: 'danger/ce_ee_vue_templates') + danger.import_dangerfile(path: 'danger/only_documentation') end diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 228bb652597..30971ad5227 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -105,8 +105,8 @@ export default { :commit="commit" :help-page-path="helpPagePath" :show-reply-button="userCanReply" - @handle-delete-note="$emit('deleteNote')" - @start-replying="$emit('startReplying')" + @handleDeleteNote="$emit('deleteNote')" + @startReplying="$emit('startReplying')" > <note-edited-text v-if="discussion.resolved" @@ -132,7 +132,7 @@ export default { :note="componentData(note)" :help-page-path="helpPagePath" :line="line" - @handle-delete-note="$emit('deleteNote')" + @handleDeleteNote="$emit('deleteNote')" /> </template> </template> @@ -144,7 +144,7 @@ export default { :note="componentData(note)" :help-page-path="helpPagePath" :line="diffLine" - @handle-delete-note="$emit('deleteNote')" + @handleDeleteNote="$emit('deleteNote')" > <slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot> </component> diff --git a/app/assets/stylesheets/framework/system_messages.scss b/app/assets/stylesheets/framework/system_messages.scss index 6205ccaa52f..5c298d5a588 100644 --- a/app/assets/stylesheets/framework/system_messages.scss +++ b/app/assets/stylesheets/framework/system_messages.scss @@ -98,14 +98,4 @@ top: auto; bottom: auto; } - - .content-wrapper { - .with-system-header & { - margin-top: 0; - } - - .with-system-footer & { - margin-top: 0; - } - } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7321f719deb..75108bf2646 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -516,4 +516,10 @@ class ApplicationController < ActionController::Base def sentry_context Gitlab::Sentry.context(current_user) end + + def allow_gitaly_ref_name_caching + ::Gitlab::GitalyClient.allow_ref_name_caching do + yield + end + end end diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 80e4f54bbf4..90396c15375 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -87,10 +87,4 @@ class Projects::ApplicationController < ApplicationController def check_issues_available! return render_404 unless @project.feature_available?(:issues, current_user) end - - def allow_gitaly_ref_name_caching - ::Gitlab::GitalyClient.allow_ref_name_caching do - yield - end - end end diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index a80ab3bcd28..8c674be58c5 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -5,6 +5,8 @@ class SearchController < ApplicationController include SearchHelper include RendersCommits + around_action :allow_gitaly_ref_name_caching + skip_before_action :authenticate_user! requires_cross_project_access if: -> do search_term_present = params[:search].present? || params[:term].present? diff --git a/app/models/postgresql/replication_slot.rb b/app/models/postgresql/replication_slot.rb index 74ccf23cf69..7a123deb719 100644 --- a/app/models/postgresql/replication_slot.rb +++ b/app/models/postgresql/replication_slot.rb @@ -28,7 +28,7 @@ module Postgresql # We force the use of a transaction here so the query always goes to the # primary, even when using the EE DB load balancer. sizes = transaction { pluck(lag_function) } - too_great = sizes.count { |size| size >= max } + too_great = sizes.compact.count { |size| size >= max } # If too many replicas are falling behind too much, the availability of a # GitLab instance might suffer. To prevent this from happening we require diff --git a/app/views/layouts/fullscreen.html.haml b/app/views/layouts/fullscreen.html.haml index e29f646ed4f..fa04b5be9f2 100644 --- a/app/views/layouts/fullscreen.html.haml +++ b/app/views/layouts/fullscreen.html.haml @@ -10,5 +10,5 @@ = render "layouts/broadcast" = yield :flash_message = render "layouts/flash" - .content-wrapper{ id: "content-body", class: "d-flex flex-column align-items-stretch" } + .content-wrapper{ id: "content-body", class: "d-flex flex-column align-items-stretch mt-0" } = yield diff --git a/changelogs/unreleased/63200-reply-button-broken.yml b/changelogs/unreleased/63200-reply-button-broken.yml new file mode 100644 index 00000000000..11f81dbd925 --- /dev/null +++ b/changelogs/unreleased/63200-reply-button-broken.yml @@ -0,0 +1,5 @@ +--- +title: Fix unresponsive reply button in discussions +merge_request: 29936 +author: +type: fixed diff --git a/changelogs/unreleased/add-metrics-dashboard-permission-check.yml b/changelogs/unreleased/add-metrics-dashboard-permission-check.yml new file mode 100644 index 00000000000..0ea2c4c8e41 --- /dev/null +++ b/changelogs/unreleased/add-metrics-dashboard-permission-check.yml @@ -0,0 +1,5 @@ +--- +title: Add permission check to metrics dashboards endpoint +merge_request: 30017 +author: +type: added diff --git a/changelogs/unreleased/po-raw-changes-encoding.yml b/changelogs/unreleased/po-raw-changes-encoding.yml new file mode 100644 index 00000000000..051d18f50c7 --- /dev/null +++ b/changelogs/unreleased/po-raw-changes-encoding.yml @@ -0,0 +1,5 @@ +--- +title: Expect bytes from Gitaly RPC GetRawChanges +merge_request: 28164 +author: +type: fixed diff --git a/changelogs/unreleased/sh-add-gitaly-ref-caching-search-controller.yml b/changelogs/unreleased/sh-add-gitaly-ref-caching-search-controller.yml new file mode 100644 index 00000000000..d4be28e9883 --- /dev/null +++ b/changelogs/unreleased/sh-add-gitaly-ref-caching-search-controller.yml @@ -0,0 +1,5 @@ +--- +title: Enable Gitaly ref caching for SearchController +merge_request: 30105 +author: +type: performance diff --git a/changelogs/unreleased/sh-handle-nil-replication-lag.yml b/changelogs/unreleased/sh-handle-nil-replication-lag.yml new file mode 100644 index 00000000000..5638d7e79e3 --- /dev/null +++ b/changelogs/unreleased/sh-handle-nil-replication-lag.yml @@ -0,0 +1,5 @@ +--- +title: Fix background migrations failing with unused replication slot +merge_request: 30042 +author: +type: fixed diff --git a/danger/only_documentation/Dangerfile b/danger/only_documentation/Dangerfile new file mode 100644 index 00000000000..8e4564f22b6 --- /dev/null +++ b/danger/only_documentation/Dangerfile @@ -0,0 +1,24 @@ +# rubocop:disable Style/SignalException +# frozen_string_literal: true + +has_only_docs_changes = helper.all_changed_files.all? { |file| file.start_with?('doc/') } +is_docs_only_branch = gitlab.branch_for_head =~ /(^docs[\/-].*|.*-docs$)/ + +if is_docs_only_branch && !has_only_docs_changes + fail "It seems like your branch name has a `docs` prefix or suffix. "\ + "The CI won't run the full pipeline, but you also have changed non-docs files. "\ + "Please recreate this MR with a new branch name." +end + +if has_only_docs_changes && !is_docs_only_branch + markdown(<<~MARKDOWN) + + ## Documentation only changes + + Hey! Seems your merge request contains only docs changes. + Tired of waiting 2 hours for the pipeline to finish? + Next time, prepend `docs-` to [your branch name](https://docs.gitlab.com/ee/development/documentation/#branch-naming) + and the pipeline will finish before you say GitLab (x300)! + + MARKDOWN +end diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index df32242a522..64c793cfd64 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -5,8 +5,16 @@ [Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools] are useful for testing for potential accessibility problems in GitLab. -Accessibility best-practices and more in-depth information is available on -[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. +The [axe][axe-website] browser extension (available for [Firefox][axe-firefox-extension] and [Chrome][axe-chrome-extension]) is +also a handy tool for running audits and getting feedback on markup, CSS and even potentially problematic color usages. + +Accessibility best-practices and more in-depth information are available on +[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. The "[awesome a11y][awesome-a11y]" list is also a +useful compilation of accessibility-related material. [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules +[axe-website]: https://www.deque.com/axe/ +[axe-firefox-extension]: https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/ +[axe-chrome-extension]: https://chrome.google.com/webstore/detail/axe/lhdoppojpmngadmnindnejefpokejbdd +[awesome-a11y]: https://github.com/brunopulis/awesome-a11y diff --git a/doc/development/testing_guide/end_to_end/quick_start_guide.md b/doc/development/testing_guide/end_to_end/quick_start_guide.md index f96c85be1ba..670d2b31a29 100644 --- a/doc/development/testing_guide/end_to_end/quick_start_guide.md +++ b/doc/development/testing_guide/end_to_end/quick_start_guide.md @@ -242,7 +242,7 @@ module QA issue = Resource::Issue.fabricate_via_api! do |issue| issue.title = 'Issue to test the scoped labels' - issue.labels = @initial_label + issue.labels = [@initial_label] end [@new_label_same_scope, @new_label_different_scope].each do |label| @@ -365,6 +365,14 @@ Add the following `attribute :id` and `attribute :labels` right above the [`attr > We add the attributes above the existing attribute to keep them alphabetically organized. +Then, let's initialize an instance variable for labels to allow an empty array as default value when such information is not passed during the resource fabricatioin, since this optional. [Between the attributes and the `fabricate!` method](https://gitlab.com/gitlab-org/gitlab-ee/blob/1a1f1408728f19b2aa15887cd20bddab7e70c8bd/qa/qa/resource/issue.rb#L18), add the following: + +```ruby +def initialize + @labels = [] +end +``` + Next, add the following code right below the [`fabricate!`](https://gitlab.com/gitlab-org/gitlab-ee/blob/d3584e80b4236acdf393d815d604801573af72cc/qa/qa/resource/issue.rb#L27) method. ```ruby @@ -378,7 +386,7 @@ end def api_post_body { - labels: [labels], + labels: labels, title: title } end diff --git a/lib/gitlab/git/raw_diff_change.rb b/lib/gitlab/git/raw_diff_change.rb index e1002af40f6..9a41f04a4db 100644 --- a/lib/gitlab/git/raw_diff_change.rb +++ b/lib/gitlab/git/raw_diff_change.rb @@ -11,8 +11,8 @@ module Gitlab if raw_change.is_a?(Gitaly::GetRawChangesResponse::RawChange) @blob_id = raw_change.blob_id @blob_size = raw_change.size - @old_path = raw_change.old_path.presence - @new_path = raw_change.new_path.presence + @old_path = raw_change.old_path_bytes.presence + @new_path = raw_change.new_path_bytes.presence @operation = raw_change.operation&.downcase || :unknown else parse(raw_change) diff --git a/lib/gitlab/metrics/dashboard/base_service.rb b/lib/gitlab/metrics/dashboard/base_service.rb index 90895eb237a..0628e82e592 100644 --- a/lib/gitlab/metrics/dashboard/base_service.rb +++ b/lib/gitlab/metrics/dashboard/base_service.rb @@ -10,6 +10,8 @@ module Gitlab NOT_FOUND_ERROR = Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError def get_dashboard + return error('Insufficient permissions.', :unauthorized) unless allowed? + success(dashboard: process_dashboard) rescue NOT_FOUND_ERROR error("#{dashboard_path} could not be found.", :not_found) @@ -30,6 +32,12 @@ module Gitlab private + # Determines whether users should be able to view + # dashboards at all. + def allowed? + Ability.allowed?(current_user, :read_environment, project) + end + # Returns a new dashboard Hash, supplemented with DB info def process_dashboard Gitlab::Metrics::Dashboard::Processor diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 4500597a212..5a5c0a1f6ac 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -17,6 +17,10 @@ describe SearchController do set(:project) { create(:project, :public, :repository, :wiki_repo) } + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + end + subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } where(:partial, :scope) do diff --git a/spec/frontend/notes/components/discussion_notes_spec.js b/spec/frontend/notes/components/discussion_notes_spec.js index c3204b3aaa0..394666403ee 100644 --- a/spec/frontend/notes/components/discussion_notes_spec.js +++ b/spec/frontend/notes/components/discussion_notes_spec.js @@ -112,6 +112,44 @@ describe('DiscussionNotes', () => { }); }); + describe('events', () => { + describe('with groupped notes and replies expanded', () => { + const findNoteAtIndex = index => wrapper.find(`.note:nth-of-type(${index + 1}`); + + beforeEach(() => { + createComponent({ shouldGroupReplies: true, isExpanded: true }); + }); + + it('emits deleteNote when first note emits handleDeleteNote', () => { + findNoteAtIndex(0).vm.$emit('handleDeleteNote'); + expect(wrapper.emitted().deleteNote).toBeTruthy(); + }); + + it('emits startReplying when first note emits startReplying', () => { + findNoteAtIndex(0).vm.$emit('startReplying'); + expect(wrapper.emitted().startReplying).toBeTruthy(); + }); + + it('emits deleteNote when second note emits handleDeleteNote', () => { + findNoteAtIndex(1).vm.$emit('handleDeleteNote'); + expect(wrapper.emitted().deleteNote).toBeTruthy(); + }); + }); + + describe('with ungroupped notes', () => { + let note; + beforeEach(() => { + createComponent(); + note = wrapper.find('.note'); + }); + + it('emits deleteNote when first note emits handleDeleteNote', () => { + note.vm.$emit('handleDeleteNote'); + expect(wrapper.emitted().deleteNote).toBeTruthy(); + }); + }); + }); + describe('componentData', () => { beforeEach(() => { createComponent(); diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index ac2fb16bd10..30e0504e4e1 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -473,7 +473,7 @@ describe('mrWidgetOptions', () => { vm.mr.relatedLinks = { assignToMe: null, closing: ` - <a class="close-related-link" href="#'> + <a class="close-related-link" href="#"> Close </a> `, diff --git a/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb index eecd257b38d..79a78df44ae 100644 --- a/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb @@ -6,13 +6,19 @@ describe Gitlab::Metrics::Dashboard::DynamicDashboardService, :use_clean_rails_m include MetricsDashboardHelpers set(:project) { build(:project) } + set(:user) { create(:user) } set(:environment) { create(:environment, project: project) } + before do + project.add_maintainer(user) + end + describe '#get_dashboard' do - let(:service_params) { [project, nil, { environment: environment, dashboard_path: nil }] } + let(:service_params) { [project, user, { environment: environment, dashboard_path: nil }] } let(:service_call) { described_class.new(*service_params).get_dashboard } it_behaves_like 'valid embedded dashboard service response' + it_behaves_like 'raises error for users with insufficient permissions' it 'caches the unprocessed dashboard for subsequent calls' do expect(YAML).to receive(:safe_load).once.and_call_original diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index b9a5ee9c2b3..d8ed54c0248 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -6,12 +6,17 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi include MetricsDashboardHelpers set(:project) { build(:project) } + set(:user) { create(:user) } set(:environment) { create(:environment, project: project) } let(:system_dashboard_path) { Gitlab::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH} + before do + project.add_maintainer(user) + end + describe '.find' do let(:dashboard_path) { '.gitlab/dashboards/test.yml' } - let(:service_call) { described_class.find(project, nil, environment, dashboard_path: dashboard_path) } + let(:service_call) { described_class.find(project, user, environment, dashboard_path: dashboard_path) } it_behaves_like 'misconfigured dashboard service response', :not_found @@ -41,13 +46,13 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi end context 'when no dashboard is specified' do - let(:service_call) { described_class.find(project, nil, environment) } + let(:service_call) { described_class.find(project, user, environment) } it_behaves_like 'valid dashboard service response' end context 'when the dashboard is expected to be embedded' do - let(:service_call) { described_class.find(project, nil, environment, dashboard_path: nil, embedded: true) } + let(:service_call) { described_class.find(project, user, environment, dashboard_path: nil, embedded: true) } it_behaves_like 'valid embedded dashboard service response' end diff --git a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb index 57d82421b5d..468e8ec9ef2 100644 --- a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb @@ -5,8 +5,8 @@ require 'rails_helper' describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers - set(:user) { build(:user) } - set(:project) { build(:project) } + set(:user) { create(:user) } + set(:project) { create(:project) } set(:environment) { create(:environment, project: project) } before do @@ -22,6 +22,8 @@ describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_m it_behaves_like 'misconfigured dashboard service response', :not_found end + it_behaves_like 'raises error for users with insufficient permissions' + context 'when the dashboard exists' do let(:project) { project_with_dashboard(dashboard_path) } diff --git a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb index 2af745bd4d7..13f22dd01c5 100644 --- a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb @@ -5,15 +5,21 @@ require 'spec_helper' describe Gitlab::Metrics::Dashboard::SystemDashboardService, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers - set(:project) { build(:project) } + set(:user) { create(:user) } + set(:project) { create(:project) } set(:environment) { create(:environment, project: project) } + before do + project.add_maintainer(user) + end + describe 'get_dashboard' do let(:dashboard_path) { described_class::SYSTEM_DASHBOARD_PATH } - let(:service_params) { [project, nil, { environment: environment, dashboard_path: dashboard_path }] } + let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] } let(:service_call) { described_class.new(*service_params).get_dashboard } it_behaves_like 'valid dashboard service response' + it_behaves_like 'raises error for users with insufficient permissions' it 'caches the unprocessed dashboard for subsequent calls' do expect(YAML).to receive(:safe_load).once.and_call_original diff --git a/spec/models/postgresql/replication_slot_spec.rb b/spec/models/postgresql/replication_slot_spec.rb index e100af7ddc7..95ae204a8a8 100644 --- a/spec/models/postgresql/replication_slot_spec.rb +++ b/spec/models/postgresql/replication_slot_spec.rb @@ -47,5 +47,13 @@ describe Postgresql::ReplicationSlot, :postgresql do expect(described_class.lag_too_great?).to eq(false) end + + it 'returns false when there is a nil replication lag' do + expect(described_class) + .to receive(:pluck) + .and_return([0.megabytes, nil]) + + expect(described_class.lag_too_great?).to eq(false) + end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index c5ab7e57272..0bd17dbacd7 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -844,6 +844,19 @@ describe Repository do end end + describe '#get_raw_changes' do + context `with non-UTF8 bytes in paths` do + let(:old_rev) { 'd0888d297eadcd7a345427915c309413b1231e65' } + let(:new_rev) { '19950f03c765f7ac8723a73a0599764095f52fc0' } + let(:changes) { repository.raw_changes_between(old_rev, new_rev) } + + it 'returns the changes' do + expect { changes }.not_to raise_error + expect(changes.first.new_path.bytes).to eq("hello\x80world".bytes) + end + end + end + describe '#create_ref' do it 'redirects the call to write_ref' do ref, ref_path = '1', '2' diff --git a/spec/support/helpers/metrics_dashboard_helpers.rb b/spec/support/helpers/metrics_dashboard_helpers.rb index 6de00eea474..1511a2f6b49 100644 --- a/spec/support/helpers/metrics_dashboard_helpers.rb +++ b/spec/support/helpers/metrics_dashboard_helpers.rb @@ -50,4 +50,12 @@ module MetricsDashboardHelpers it_behaves_like 'valid dashboard service response for schema' end + + shared_examples_for 'raises error for users with insufficient permissions' do + context 'when the user does not have sufficient access' do + let(:user) { build(:user) } + + it_behaves_like 'misconfigured dashboard service response', :unauthorized + end + end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 77f22d9dd24..e63099d89b7 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -64,7 +64,8 @@ module TestEnv 'with-codeowners' => '219560e', 'submodule_inside_folder' => 'b491b92', 'png-lfs' => 'fe42f41', - 'sha-starting-with-large-number' => '8426165' + 'sha-starting-with-large-number' => '8426165', + 'invalid-utf8-diff-paths' => '99e4853' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily |