diff options
| -rw-r--r-- | app/assets/javascripts/diff_notes/components/jump_to_discussion.js | 10 | ||||
| -rw-r--r-- | app/assets/javascripts/project.js | 9 | ||||
| -rw-r--r-- | app/assets/stylesheets/framework/dropdowns.scss | 45 | ||||
| -rw-r--r-- | app/assets/stylesheets/pages/projects.scss | 2 | ||||
| -rw-r--r-- | app/views/shared/_clone_panel.html.haml | 2 | ||||
| -rw-r--r-- | app/workers/email_receiver_worker.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/28472-ignore-auto-generated-mails.yml | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/35232-next-unresolved.yml | 4 | ||||
| -rw-r--r-- | doc/administration/reply_by_email_postfix_setup.md | 14 | ||||
| -rw-r--r-- | doc/api/README.md | 63 | ||||
| -rw-r--r-- | lib/gitlab/email/handler/create_note_handler.rb | 1 | ||||
| -rw-r--r-- | lib/gitlab/email/receiver.rb | 13 | ||||
| -rw-r--r-- | lib/tasks/gitlab/gitaly.rake | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/email/handler/create_note_handler_spec.rb | 9 | ||||
| -rw-r--r-- | spec/lib/gitlab/email/receiver_spec.rb | 16 | ||||
| -rw-r--r-- | spec/spec_helper.rb | 6 | ||||
| -rw-r--r-- | spec/support/test_env.rb | 17 | ||||
| -rw-r--r-- | spec/tasks/gitlab/gitaly_rake_spec.rb | 10 | ||||
| -rw-r--r-- | spec/workers/email_receiver_worker_spec.rb | 26 |
19 files changed, 178 insertions, 77 deletions
diff --git a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js index 37ddca29e71..298f737a2bc 100644 --- a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js +++ b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js @@ -94,7 +94,7 @@ const JumpToDiscussion = Vue.extend({ hasDiscussionsToJumpTo = false; } } - } else if (activeTab !== 'notes') { + } else if (activeTab !== 'show') { // If we are on the commits or builds tabs, // there are no discussions to jump to. hasDiscussionsToJumpTo = false; @@ -103,12 +103,12 @@ const JumpToDiscussion = Vue.extend({ if (!hasDiscussionsToJumpTo) { // If there are no discussions to jump to on the current page, // switch to the notes tab and jump to the first disucssion there. - window.mrTabs.activateTab('notes'); - activeTab = 'notes'; + window.mrTabs.activateTab('show'); + activeTab = 'show'; jumpToFirstDiscussion = true; } - if (activeTab === 'notes') { + if (activeTab === 'show') { discussionsSelector = '.discussion[data-discussion-id]'; discussionIdsInScope = discussionIdsForElements($(discussionsSelector)); } @@ -156,7 +156,7 @@ const JumpToDiscussion = Vue.extend({ let $target = $(`${discussionsSelector}[data-discussion-id="${nextUnresolvedDiscussionId}"]`); - if (activeTab === 'notes') { + if (activeTab === 'show') { $target = $target.closest('.note-discussion'); // If the next discussion is closed, toggle it open. diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index a3f7d69b98d..6e1744e8e72 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -10,14 +10,19 @@ import Cookies from 'js-cookie'; const $projectCloneField = $('#project_clone'); const $cloneBtnText = $('a.clone-dropdown-btn span'); + const selectedCloneOption = $cloneBtnText.text().trim(); + if (selectedCloneOption.length > 0) { + $(`a:contains('${selectedCloneOption}')`, $cloneOptions).addClass('is-active'); + } + $('a', $cloneOptions).on('click', (e) => { const $this = $(e.currentTarget); const url = $this.attr('href'); e.preventDefault(); - $('.active', $cloneOptions).not($this).removeClass('active'); - $this.toggleClass('active'); + $('.is-active', $cloneOptions).not($this).removeClass('is-active'); + $this.toggleClass('is-active'); $projectCloneField.val(url); $cloneBtnText.text($this.text()); diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 3f934403147..572203bce34 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -722,3 +722,48 @@ @include set-invisible; overflow: hidden; } + +// TODO: change global style and remove mixin +@mixin new-style-dropdown { + .dropdown-menu { + li { + padding: 0 1px; + + &.dropdown-header { + padding: 8px 16px; + } + + a { + border-radius: 0; + padding: 8px 16px; + + &.is-focused, + &:hover, + &:active, + &:focus { + background-color: $gray-darker; + } + + &.is-active { + font-weight: inherit; + + &::before { + top: 16px; + } + } + } + } + + &.dropdown-menu-selectable { + li { + a { + padding: 8px 40px; + + &.is-active::before { + left: 16px; + } + } + } + } + } +} diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index b3a90dff89a..d29421aa1b3 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -282,6 +282,8 @@ } .project-repo-buttons { + @include new-style-dropdown; + .project-action-button .dropdown-menu { max-height: 250px; overflow-y: auto; diff --git a/app/views/shared/_clone_panel.html.haml b/app/views/shared/_clone_panel.html.haml index b4843eafdb7..3d9c90c38fe 100644 --- a/app/views/shared/_clone_panel.html.haml +++ b/app/views/shared/_clone_panel.html.haml @@ -11,7 +11,7 @@ %span = default_clone_protocol.upcase = icon('caret-down') - %ul.dropdown-menu.dropdown-menu-right.clone-options-dropdown + %ul.dropdown-menu.dropdown-menu-selectable.dropdown-menu-right.clone-options-dropdown %li = ssh_clone_button(project) %li diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index d3f7e479a8d..1afa24c8e2a 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -31,8 +31,6 @@ class EmailReceiverWorker when Gitlab::Email::EmptyEmailError can_retry = true "It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies." - when Gitlab::Email::AutoGeneratedEmailError - "The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface." when Gitlab::Email::UserNotFoundError "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface." when Gitlab::Email::UserBlockedError diff --git a/changelogs/unreleased/28472-ignore-auto-generated-mails.yml b/changelogs/unreleased/28472-ignore-auto-generated-mails.yml new file mode 100644 index 00000000000..af63b43e62e --- /dev/null +++ b/changelogs/unreleased/28472-ignore-auto-generated-mails.yml @@ -0,0 +1,4 @@ +--- +title: Don't send rejection mails for all auto-generated mails +merge_request: 13254 +author: diff --git a/changelogs/unreleased/35232-next-unresolved.yml b/changelogs/unreleased/35232-next-unresolved.yml new file mode 100644 index 00000000000..45f3fb429a8 --- /dev/null +++ b/changelogs/unreleased/35232-next-unresolved.yml @@ -0,0 +1,4 @@ +--- +title: fix jump to next discussion button +merge_request: +author: diff --git a/doc/administration/reply_by_email_postfix_setup.md b/doc/administration/reply_by_email_postfix_setup.md index 3b8c716eff5..a1bb3851951 100644 --- a/doc/administration/reply_by_email_postfix_setup.md +++ b/doc/administration/reply_by_email_postfix_setup.md @@ -177,6 +177,20 @@ Courier, which we will install later to add IMAP authentication, requires mailbo ```sh sudo apt-get install courier-imap ``` + + And start `imapd`: + ```sh + imapd start + ``` + +1. The courier-authdaemon isn't started after installation. Without it, imap authentication will fail: + ```sh + sudo service courier-authdaemon start + ``` + You can also configure courier-authdaemon to start on boot: + ```sh + sudo systemctl enable courier-authdaemon + ``` ## Configure Postfix to receive email from the internet diff --git a/doc/api/README.md b/doc/api/README.md index f63d395b10e..8acb2145f1a 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -77,6 +77,38 @@ controller-specific endpoints. GraphQL has a number of benefits: It will co-exist with the current v4 REST API. If we have a v5 API, this should be a compatibility layer on top of GraphQL. +## Basic usage + +API requests should be prefixed with `api` and the API version. The API version +is defined in [`lib/api.rb`][lib-api-url]. For example, the root of the v4 API +is at `/api/v4`. + +For endpoints that require [authentication](#authentication), you need to pass +a `private_token` parameter via query string or header. If passed as a header, +the header name must be `PRIVATE-TOKEN` (uppercase and with a dash instead of +an underscore). + +Example of a valid API request: + +``` +GET /projects?private_token=9koXpg98eAheJpvBs5tK +``` + +Example of a valid API request using cURL and authentication via header: + +```shell +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects" +``` + +Example of a valid API request using cURL and authentication via a query string: + +```shell +curl "https://gitlab.example.com/api/v4/projects?private_token=9koXpg98eAheJpvBs5tK" +``` + +The API uses JSON to serialize data. You don't need to specify `.json` at the +end of an API URL. + ## Authentication Most API requests require authentication via a session cookie or token. For @@ -207,37 +239,6 @@ GET /projects?private_token=9koXpg98eAheJpvBs5tK&sudo=23 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --header "SUDO: 23" "https://gitlab.example.com/api/v4/projects" ``` -## Basic usage - -API requests should be prefixed with `api` and the API version. The API version -is defined in [`lib/api.rb`][lib-api-url]. - -For endpoints that require [authentication](#authentication), you need to pass -a `private_token` parameter via query string or header. If passed as a header, -the header name must be `PRIVATE-TOKEN` (uppercase and with a dash instead of -an underscore). - -Example of a valid API request: - -``` -GET /projects?private_token=9koXpg98eAheJpvBs5tK -``` - -Example of a valid API request using cURL and authentication via header: - -```shell -curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects" -``` - -Example of a valid API request using cURL and authentication via a query string: - -```shell -curl "https://gitlab.example.com/api/v4/projects?private_token=9koXpg98eAheJpvBs5tK" -``` - -The API uses JSON to serialize data. You don't need to specify `.json` at the -end of an API URL. - ## Status codes The API is designed to return different status codes according to context and diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb index 31579e94a87..8eea33b9ab5 100644 --- a/lib/gitlab/email/handler/create_note_handler.rb +++ b/lib/gitlab/email/handler/create_note_handler.rb @@ -15,7 +15,6 @@ module Gitlab def execute raise SentNotificationNotFoundError unless sent_notification - raise AutoGeneratedEmailError if mail.header.to_s =~ /auto-(generated|replied)/ validate_permission!(:create_note) diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 0d6b08b5d29..c8f4591d060 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -26,6 +26,9 @@ module Gitlab raise EmptyEmailError if @raw.blank? mail = build_mail + + ignore_auto_submitted!(mail) + mail_key = extract_mail_key(mail) handler = Handler.for(mail, mail_key) @@ -87,6 +90,16 @@ module Gitlab break key if key end end + + def ignore_auto_submitted!(mail) + # Mail::Header#[] is case-insensitive + auto_submitted = mail.header['Auto-Submitted']&.value + + # Mail::Field#value would strip leading and trailing whitespace + raise AutoGeneratedEmailError if + # See also https://tools.ietf.org/html/rfc3834 + auto_submitted && auto_submitted != 'no' + end end end end diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 680e76af471..3703f9cfb5c 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -21,7 +21,7 @@ namespace :gitlab do create_gitaly_configuration # In CI we run scripts/gitaly-test-build instead of this command unless ENV['CI'].present? - Bundler.with_original_env { run_command!(%w[/usr/bin/env -u BUNDLE_GEMFILE] + [command]) } + Bundler.with_original_env { run_command!([command]) } end end end diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index 0127b012c91..d0fa16ce4d1 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -36,15 +36,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler do end end - context "when the email was auto generated" do - let!(:mail_key) { '636ca428858779856c226bb145ef4fad' } - let!(:email_raw) { fixture_file("emails/auto_reply.eml") } - - it "raises an AutoGeneratedEmailError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError) - end - end - context "when the noteable could not be found" do before do noteable.destroy diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index 88565ea5311..59f43abf26d 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -28,14 +28,6 @@ describe Gitlab::Email::Receiver do it "raises an UnknownIncomingEmail error" do expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) end - - context "and the email contains no references header" do - let(:email_raw) { fixture_file("emails/auto_reply.eml").gsub(mail_key, "!!!") } - - it "raises an UnknownIncomingEmail error" do - expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) - end - end end context "when the email is blank" do @@ -45,4 +37,12 @@ describe Gitlab::Email::Receiver do expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) end end + + context "when the email was auto generated" do + let(:email_raw) { fixture_file("emails/auto_reply.eml") } + + it "raises an AutoGeneratedEmailError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError) + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 609998d6e9c..06769b241ad 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -49,7 +49,7 @@ RSpec.configure do |config| config.include SearchHelpers, type: :feature config.include WaitForRequests, :js config.include StubConfiguration - config.include EmailHelpers, type: :mailer + config.include EmailHelpers, :mailer, type: :mailer config.include TestEnv config.include ActiveJob::TestHelper config.include ActiveSupport::Testing::TimeHelpers @@ -93,6 +93,10 @@ RSpec.configure do |config| RequestStore.clear! end + config.before(:example, :mailer) do + reset_delivered_emails! + end + if ENV['CI'] config.around(:each) do |ex| ex.run_with_retry retry: 2 diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 7995b5893e2..bed78928f14 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -63,6 +63,8 @@ module TestEnv # See gitlab.yml.example test section for paths # def init(opts = {}) + Rake.application.rake_require 'tasks/gitlab/helpers' + Rake::Task.define_task :environment # Disable mailer for spinach tests disable_mailer if opts[:mailer] == false @@ -122,11 +124,14 @@ module TestEnv end def setup_gitlab_shell - shell_needs_update = component_needs_update?(Gitlab.config.gitlab_shell.path, + gitlab_shell_dir = File.dirname(Gitlab.config.gitlab_shell.path) + gitlab_shell_needs_update = component_needs_update?(gitlab_shell_dir, Gitlab::Shell.version_required) - unless !shell_needs_update || system('rake', 'gitlab:shell:install') - raise 'Can`t clone gitlab-shell' + Rake.application.rake_require 'tasks/gitlab/shell' + unless !gitlab_shell_needs_update || Rake.application.invoke_task('gitlab:shell:install') + FileUtils.rm_rf(gitlab_shell_dir) + raise "Can't install gitlab-shell" end end @@ -142,8 +147,10 @@ module TestEnv gitaly_needs_update = component_needs_update?(gitaly_dir, Gitlab::GitalyClient.expected_server_version) - unless !gitaly_needs_update || system('rake', "gitlab:gitaly:install[#{gitaly_dir}]") - raise "Can't clone gitaly" + Rake.application.rake_require 'tasks/gitlab/gitaly' + unless !gitaly_needs_update || Rake.application.invoke_task("gitlab:gitaly:install[#{gitaly_dir}]") + FileUtils.rm_rf(gitaly_dir) + raise "Can't install gitaly" end start_gitaly(gitaly_dir) diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index 695231c7d15..a2f4ec39d89 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -41,8 +41,6 @@ describe 'gitlab:gitaly namespace rake task' do end describe 'gmake/make' do - let(:command_preamble) { %w[/usr/bin/env -u BUNDLE_GEMFILE] } - before(:all) do @old_env_ci = ENV.delete('CI') end @@ -59,12 +57,12 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is available' do before do expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) + allow_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) end it 'calls gmake in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['/usr/bin/gmake', 0]) - expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['gmake']).and_return(true) + expect_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end @@ -73,12 +71,12 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is not available' do before do expect_any_instance_of(Object).to receive(:checkout_or_clone_version) - allow_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + allow_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) end it 'calls make in the gitaly directory' do expect(Gitlab::Popen).to receive(:popen).with(%w[which gmake]).and_return(['', 42]) - expect_any_instance_of(Object).to receive(:run_command!).with(command_preamble + ['make']).and_return(true) + expect_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) run_rake_task('gitlab:gitaly:install', clone_path) end diff --git a/spec/workers/email_receiver_worker_spec.rb b/spec/workers/email_receiver_worker_spec.rb index fe70501eeac..e4e77c667b3 100644 --- a/spec/workers/email_receiver_worker_spec.rb +++ b/spec/workers/email_receiver_worker_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe EmailReceiverWorker do +describe EmailReceiverWorker, :mailer do let(:raw_message) { fixture_file('emails/valid_reply.eml') } context "when reply by email is enabled" do @@ -17,12 +17,16 @@ describe EmailReceiverWorker do context "when an error occurs" do before do - allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(Gitlab::Email::EmptyEmailError) + allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(error) end - it "sends out a rejection email" do - perform_enqueued_jobs do - described_class.new.perform(raw_message) + context 'when the error is Gitlab::Email::EmptyEmailError' do + let(:error) { Gitlab::Email::EmptyEmailError } + + it 'sends out a rejection email' do + perform_enqueued_jobs do + described_class.new.perform(raw_message) + end email = ActionMailer::Base.deliveries.last expect(email).not_to be_nil @@ -30,6 +34,18 @@ describe EmailReceiverWorker do expect(email.subject).to include("Rejected") end end + + context 'when the error is Gitlab::Email::AutoGeneratedEmailError' do + let(:error) { Gitlab::Email::AutoGeneratedEmailError } + + it 'does not send out any rejection email' do + perform_enqueued_jobs do + described_class.new.perform(raw_message) + end + + should_not_email_anyone + end + end end end |