diff options
30 files changed, 315 insertions, 148 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e7b28e540a5..afe9da08495 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -350,25 +350,6 @@ retrieve-tests-metadata: - wget -O $FLAKY_RSPEC_SUITE_REPORT_PATH http://${TESTS_METADATA_S3_BUCKET}.s3.amazonaws.com/$FLAKY_RSPEC_SUITE_REPORT_PATH || rm $FLAKY_RSPEC_SUITE_REPORT_PATH - '[[ -f $FLAKY_RSPEC_SUITE_REPORT_PATH ]] || echo "{}" > ${FLAKY_RSPEC_SUITE_REPORT_PATH}' -danger-review: - image: registry.gitlab.com/gitlab-org/gitaly/dangercontainer:latest - stage: prepare - allow_failure: true - before_script: - - source scripts/utils.sh - - retry gem install danger --no-ri --no-rdoc - cache: {} - only: - refs: - - branches@gitlab-org/gitlab-ce - - branches@gitlab-org/gitlab-ee - except: - variables: - - $CI_COMMIT_REF_NAME =~ /^ce-to-ee-.*/ - script: - - git version - - danger --fail-on-errors=true - update-tests-metadata: <<: *tests-metadata-state <<: *only-canonical-masters @@ -457,6 +438,27 @@ setup-test-env: - config/secrets.yml - vendor/gitaly-ruby +danger-review: + image: registry.gitlab.com/gitlab-org/gitaly/dangercontainer:latest + stage: test + allow_failure: true + before_script: + - source scripts/utils.sh + - retry gem install danger --no-ri --no-rdoc + cache: {} + only: + refs: + - branches@gitlab-org/gitlab-ce + - branches@gitlab-org/gitlab-ee + except: + refs: + - master + variables: + - $CI_COMMIT_REF_NAME =~ /^ce-to-ee-.*/ + script: + - git version + - danger --fail-on-errors=true + rspec-pg 0 30: *rspec-metadata-pg rspec-pg 1 30: *rspec-metadata-pg rspec-pg 2 30: *rspec-metadata-pg diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index c01066c688a..9dc0c31be49 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -116,7 +116,12 @@ class Projects::WikisController < Projects::ApplicationController # Call #wiki to make sure the Wiki Repo is initialized @project_wiki.wiki - @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages(limit: 15)) + + @sidebar_page = @project_wiki.find_sidebar(params[:version_id]) + + unless @sidebar_page # Fallback to default sidebar + @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages(limit: 15)) + end rescue ProjectWiki::CouldNotCreateWikiError flash[:notice] = "Could not create Wiki Repository at this time. Please try again later." redirect_to project_path(@project) diff --git a/app/models/project.rb b/app/models/project.rb index e29bca365a4..ae0a13b17dc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2171,10 +2171,13 @@ class Project < ActiveRecord::Base merge_requests = source_of_merge_requests.opened .where(allow_collaboration: true) - if branch_name - merge_requests.find_by(source_branch: branch_name)&.can_be_merged_by?(user) - else - merge_requests.any? { |merge_request| merge_request.can_be_merged_by?(user) } + # Issue for N+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/49322 + Gitlab::GitalyClient.allow_n_plus_1_calls do + if branch_name + merge_requests.find_by(source_branch: branch_name)&.can_be_merged_by?(user) + else + merge_requests.any? { |merge_request| merge_request.can_be_merged_by?(user) } + end end end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 9ae2fb0013a..3aa56b3983f 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ProjectWiki include Gitlab::ShellAdapter include Storage::LegacyProjectWiki @@ -9,6 +11,7 @@ class ProjectWiki }.freeze unless defined?(MARKUPS) CouldNotCreateWikiError = Class.new(StandardError) + SIDEBAR = '_sidebar' # Returns a string describing what went wrong after # an operation fails. @@ -98,6 +101,10 @@ class ProjectWiki end end + def find_sidebar(version = nil) + find_page(SIDEBAR, version) + end + def find_file(name, version = nil) wiki.file(name, version) end diff --git a/app/views/projects/wikis/_sidebar.html.haml b/app/views/projects/wikis/_sidebar.html.haml index a23396dc0d8..28353927135 100644 --- a/app/views/projects/wikis/_sidebar.html.haml +++ b/app/views/projects/wikis/_sidebar.html.haml @@ -11,9 +11,11 @@ .blocks-container .block.block-first - %ul.wiki-pages - = render @sidebar_wiki_entries, context: 'sidebar' - + - if @sidebar_page + = render_wiki_content(@sidebar_page) + - else + %ul.wiki-pages + = render @sidebar_wiki_entries, context: 'sidebar' .block = link_to project_wikis_pages_path(@project), class: 'btn btn-block' do = s_("Wiki|More Pages") diff --git a/changelogs/custom_wiki_sidebar.yml b/changelogs/custom_wiki_sidebar.yml new file mode 100644 index 00000000000..988fccc929c --- /dev/null +++ b/changelogs/custom_wiki_sidebar.yml @@ -0,0 +1,5 @@ +--- +title: "Custom Wiki Sidebar Support Issue 14995" +merge_request: +author: Josh Sooter +type: added diff --git a/changelogs/unreleased/feature-gb-email-delivery-metrics.yml b/changelogs/unreleased/feature-gb-email-delivery-metrics.yml new file mode 100644 index 00000000000..9d0d08a471d --- /dev/null +++ b/changelogs/unreleased/feature-gb-email-delivery-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Add emails delivery Prometheus metrics +merge_request: 20638 +author: +type: added diff --git a/config/initializers/action_mailer_hooks.rb b/config/initializers/action_mailer_hooks.rb new file mode 100644 index 00000000000..f1b3c1f8ae8 --- /dev/null +++ b/config/initializers/action_mailer_hooks.rb @@ -0,0 +1,12 @@ +unless Gitlab.config.gitlab.email_enabled + ActionMailer::Base.register_interceptor(::Gitlab::Email::Hook::DisableEmailInterceptor) + ActionMailer::Base.logger = nil +end + +ActionMailer::Base.register_interceptors( + ::Gitlab::Email::Hook::AdditionalHeadersInterceptor, + ::Gitlab::Email::Hook::EmailTemplateInterceptor, + ::Gitlab::Email::Hook::DeliveryMetricsObserver +) + +ActionMailer::Base.register_observer(::Gitlab::Email::Hook::DeliveryMetricsObserver) diff --git a/config/initializers/additional_headers_interceptor.rb b/config/initializers/additional_headers_interceptor.rb deleted file mode 100644 index b9159e7c06c..00000000000 --- a/config/initializers/additional_headers_interceptor.rb +++ /dev/null @@ -1 +0,0 @@ -ActionMailer::Base.register_interceptor(AdditionalEmailHeadersInterceptor) diff --git a/config/initializers/disable_email_interceptor.rb b/config/initializers/disable_email_interceptor.rb deleted file mode 100644 index e8770c8d460..00000000000 --- a/config/initializers/disable_email_interceptor.rb +++ /dev/null @@ -1,5 +0,0 @@ -# Interceptor in lib/disable_email_interceptor.rb -unless Gitlab.config.gitlab.email_enabled - ActionMailer::Base.register_interceptor(DisableEmailInterceptor) - ActionMailer::Base.logger = nil -end diff --git a/config/initializers/email_template_interceptor.rb b/config/initializers/email_template_interceptor.rb deleted file mode 100644 index f195ca9bcd6..00000000000 --- a/config/initializers/email_template_interceptor.rb +++ /dev/null @@ -1,2 +0,0 @@ -# Interceptor in lib/email_template_interceptor.rb -ActionMailer::Base.register_interceptor(EmailTemplateInterceptor) diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index 0374de24520..a1f94dc6004 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -2,15 +2,13 @@ require 'yaml' -NO_CHANGELOG_LABELS = %w[backstage QA test].freeze +NO_CHANGELOG_LABELS = %w[backstage Documentation QA test].freeze SEE_DOC = "See [the documentation](https://docs.gitlab.com/ce/development/changelog.html).".freeze -MISSING_CHANGELOG_MESSAGE = <<~MSG.freeze -**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html).** - +CREATE_CHANGELOG_MESSAGE = <<~MSG.freeze You can create one with: ``` -bin/changelog -m %<mr_iid>s +bin/changelog -m %<mr_iid>s "%<mr_title>s" ``` If your merge request doesn't warrant a CHANGELOG entry, @@ -56,13 +54,15 @@ changelog_needed = (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty? changelog_found = git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} } if git.modified_files.include?("CHANGELOG.md") - fail "CHANGELOG.md was edited. Please remove the additions and create an entry with `bin/changelog -m #{gitlab.mr_json["iid"]}` instead." + 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: gitlab.mr_json["title"], labels: presented_no_changelog_labels) end if changelog_needed if changelog_found check_changelog(changelog_found) else - warn format(MISSING_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], labels: presented_no_changelog_labels) + warn "**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html).**\n\n" + + format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: gitlab.mr_json["title"], labels: presented_no_changelog_labels) end end diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile index 934ea0beadb..97188df8785 100644 --- a/danger/specs/Dangerfile +++ b/danger/specs/Dangerfile @@ -1,12 +1,18 @@ +NO_SPECS_LABELS = %w[backstage Documentation QA].freeze NO_NEW_SPEC_MESSAGE = <<~MSG.freeze You've made some app changes, but didn't add any tests. That's OK as long as you're refactoring existing code, -but please consider adding the ~backstage label in that case. +but please consider adding any of the %<labels>s labels. MSG +def presented_no_changelog_labels + NO_SPECS_LABELS.map { |label| "~#{label}" }.join(', ') +end + has_app_changes = !git.modified_files.grep(%r{\A(ee/)?(app|lib|db/(geo/)?(post_)?migrate)/}).empty? -has_spec_changes = !git.modified_files.grep(/spec/).empty? +has_spec_changes = !git.modified_files.grep(%r{\A(ee/)?spec/}).empty? +new_specs_needed = (gitlab.mr_labels & NO_SPECS_LABELS).empty? -if has_app_changes && !has_spec_changes - warn NO_NEW_SPEC_MESSAGE, sticky: false +if has_app_changes && !has_spec_changes && new_specs_needed + warn format(NO_NEW_SPEC_MESSAGE, labels: presented_no_changelog_labels), sticky: false end diff --git a/doc/development/sql.md b/doc/development/sql.md index 974b1d99dff..e1e1d31a85f 100644 --- a/doc/development/sql.md +++ b/doc/development/sql.md @@ -243,3 +243,45 @@ WHERE EXISTS ( ``` [gin-index]: http://www.postgresql.org/docs/current/static/gin.html + +## `.find_or_create_by` is not atomic + +The inherent pattern with methods like `.find_or_create_by` and +`.first_or_create` and others is that they are not atomic. This means, +it first runs a `SELECT`, and if there are no results an `INSERT` is +performed. With concurrent processes in mind, there is a race condition +which may lead to trying to insert two similar records. This may not be +desired, or may cause one of the queries to fail due to a constraint +violation, for example. + +Using transactions does not solve this problem. + +The following pattern should be used to avoid the problem: + +```ruby +Project.transaction do + begin + User.find_or_create_by(username: "foo") + rescue ActiveRecord::RecordNotUnique + retry + end +end +``` + +If the above block is run inside a transaction and hits the race +condition, the transaction is aborted and we cannot simply retry (any +further queries inside the aborted transaction are going to fail). We +can employ [nested transactions](http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Nested+transactions) +here to only rollback the "inner transaction". Note that `requires_new: true` is required here. + +```ruby +Project.transaction do + begin + User.transaction(requires_new: true) do + User.find_or_create_by(username: "foo") + end + rescue ActiveRecord::RecordNotUnique + retry + end +end +``` diff --git a/doc/user/project/import/manifest.md b/doc/user/project/import/manifest.md index 812ecf05faf..06171f11e12 100644 --- a/doc/user/project/import/manifest.md +++ b/doc/user/project/import/manifest.md @@ -1,7 +1,8 @@ # Import multiple repositories by uploading a manifest file GitLab allows you to import all the required git repositories -based a manifest file like the one used by the Android repository. +based a manifest file like the one used by the [Android repository](https://android.googlesource.com/platform/manifest/+/2d6f081a3b05d8ef7a2b1b52b0d536b2b74feab4/default.xml). +This feature can be very handy when you need to import a project with many repositories like Android Open Source Project (AOSP). >**Note:** diff --git a/doc/user/project/wiki/index.md b/doc/user/project/wiki/index.md index d084ee41d8a..ad0ef60373c 100644 --- a/doc/user/project/wiki/index.md +++ b/doc/user/project/wiki/index.md @@ -107,3 +107,10 @@ On the right sidebar, click on **Clone repository** and follow the on-screen instructions. [permissions]: ../../permissions.md + +## Customizing sidebar + +By default, the wiki would render a sidebar which lists all the pages for the +wiki. You could as well provide a `_sidebar` page to replace this default +sidebar. When this customized sidebar page is provided, the default sidebar +would not be rendered, but the customized one. diff --git a/lib/additional_email_headers_interceptor.rb b/lib/additional_email_headers_interceptor.rb deleted file mode 100644 index 3cb1694b9f1..00000000000 --- a/lib/additional_email_headers_interceptor.rb +++ /dev/null @@ -1,6 +0,0 @@ -class AdditionalEmailHeadersInterceptor - def self.delivering_email(message) - message.header['Auto-Submitted'] ||= 'auto-generated' - message.header['X-Auto-Response-Suppress'] ||= 'All' - end -end diff --git a/lib/disable_email_interceptor.rb b/lib/disable_email_interceptor.rb deleted file mode 100644 index cee664b8951..00000000000 --- a/lib/disable_email_interceptor.rb +++ /dev/null @@ -1,7 +0,0 @@ -# Read about interceptors in http://guides.rubyonrails.org/action_mailer_basics.html#intercepting-emails -class DisableEmailInterceptor - def self.delivering_email(message) - message.perform_deliveries = false - Rails.logger.info "Emails disabled! Interceptor prevented sending mail #{message.subject}" - end -end diff --git a/lib/email_template_interceptor.rb b/lib/email_template_interceptor.rb deleted file mode 100644 index 3978a6d9fe4..00000000000 --- a/lib/email_template_interceptor.rb +++ /dev/null @@ -1,11 +0,0 @@ -# Read about interceptors in http://guides.rubyonrails.org/action_mailer_basics.html#intercepting-emails -class EmailTemplateInterceptor - def self.delivering_email(message) - # Remove HTML part if HTML emails are disabled. - unless Gitlab::CurrentSettings.html_emails_enabled - message.parts.delete_if do |part| - part.content_type.start_with?('text/html') - end - end - end -end diff --git a/lib/gitlab/email/hook/additional_headers_interceptor.rb b/lib/gitlab/email/hook/additional_headers_interceptor.rb new file mode 100644 index 00000000000..064cb5e659a --- /dev/null +++ b/lib/gitlab/email/hook/additional_headers_interceptor.rb @@ -0,0 +1,12 @@ +module Gitlab + module Email + module Hook + class AdditionalHeadersInterceptor + def self.delivering_email(message) + message.header['Auto-Submitted'] ||= 'auto-generated' + message.header['X-Auto-Response-Suppress'] ||= 'All' + end + end + end + end +end diff --git a/lib/gitlab/email/hook/delivery_metrics_observer.rb b/lib/gitlab/email/hook/delivery_metrics_observer.rb new file mode 100644 index 00000000000..1c2985f6045 --- /dev/null +++ b/lib/gitlab/email/hook/delivery_metrics_observer.rb @@ -0,0 +1,31 @@ +module Gitlab + module Email + module Hook + class DeliveryMetricsObserver + extend Gitlab::Utils::StrongMemoize + + def self.delivering_email(_message) + delivery_attempts_counter.increment + end + + def self.delivered_email(_message) + delivered_emails_counter.increment + end + + def self.delivery_attempts_counter + strong_memoize(:delivery_attempts_counter) do + Gitlab::Metrics.counter(:gitlab_emails_delivery_attempts_total, + 'Counter of total emails delivery attempts') + end + end + + def self.delivered_emails_counter + strong_memoize(:delivered_emails_counter) do + Gitlab::Metrics.counter(:gitlab_emails_delivered_total, + 'Counter of total emails delievered') + end + end + end + end + end +end diff --git a/lib/gitlab/email/hook/disable_email_interceptor.rb b/lib/gitlab/email/hook/disable_email_interceptor.rb new file mode 100644 index 00000000000..7bb8b53f0c8 --- /dev/null +++ b/lib/gitlab/email/hook/disable_email_interceptor.rb @@ -0,0 +1,13 @@ +module Gitlab + module Email + module Hook + class DisableEmailInterceptor + def self.delivering_email(message) + message.perform_deliveries = false + + Rails.logger.info "Emails disabled! Interceptor prevented sending mail #{message.subject}" + end + end + end + end +end diff --git a/lib/gitlab/email/hook/email_template_interceptor.rb b/lib/gitlab/email/hook/email_template_interceptor.rb new file mode 100644 index 00000000000..be0c4dd862e --- /dev/null +++ b/lib/gitlab/email/hook/email_template_interceptor.rb @@ -0,0 +1,18 @@ +module Gitlab + module Email + module Hook + class EmailTemplateInterceptor + ## + # Remove HTML part if HTML emails are disabled. + # + def self.delivering_email(message) + unless Gitlab::CurrentSettings.html_emails_enabled + message.parts.delete_if do |part| + part.content_type.start_with?('text/html') + end + end + end + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 3c23b588f77..2cbd9c218d4 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -443,12 +443,8 @@ module Gitlab # Returns the SHA of the most recent common ancestor of +from+ and +to+ def merge_base(from, to) - gitaly_migrate(:merge_base) do |is_enabled| - if is_enabled - gitaly_repository_client.find_merge_base(from, to) - else - rugged_merge_base(from, to) - end + wrapped_gitaly_errors do + gitaly_repository_client.find_merge_base(from, to) end end @@ -464,12 +460,8 @@ module Gitlab return [] unless root_sha - branches = gitaly_migrate(:merged_branch_names) do |is_enabled| - if is_enabled - gitaly_merged_branch_names(branch_names, root_sha) - else - git_merged_branch_names(branch_names, root_sha) - end + branches = wrapped_gitaly_errors do + gitaly_merged_branch_names(branch_names, root_sha) end Set.new(branches) @@ -848,12 +840,8 @@ module Gitlab def write_ref(ref_path, ref, old_ref: nil, shell: true) ref_path = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref_path}" unless ref_path.start_with?("refs/") || ref_path == "HEAD" - gitaly_migrate(:write_ref) do |is_enabled| - if is_enabled - gitaly_repository_client.write_ref(ref_path, ref, old_ref, shell) - else - local_write_ref(ref_path, ref, old_ref: old_ref, shell: shell) - end + wrapped_gitaly_errors do + gitaly_repository_client.write_ref(ref_path, ref, old_ref, shell) end end @@ -1188,37 +1176,6 @@ module Gitlab end end - def local_write_ref(ref_path, ref, old_ref: nil, shell: true) - if shell - shell_write_ref(ref_path, ref, old_ref) - else - rugged_write_ref(ref_path, ref) - end - end - - def rugged_write_config(full_path:) - rugged.config['gitlab.fullpath'] = full_path - end - - def shell_write_ref(ref_path, ref, old_ref) - raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ') - raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00") - raise ArgumentError, "invalid old_ref #{old_ref.inspect}" if !old_ref.nil? && old_ref.include?("\x00") - - input = "update #{ref_path}\x00#{ref}\x00#{old_ref}\x00" - run_git!(%w[update-ref --stdin -z]) { |stdin| stdin.write(input) } - end - - def rugged_write_ref(ref_path, ref) - rugged.references.create(ref_path, ref, force: true) - rescue Rugged::ReferenceError => ex - Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}" - rescue Rugged::OSError => ex - raise unless ex.message =~ /Failed to create locked file/ && ex.message =~ /File exists/ - - Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}" - end - def run_git(args, chdir: path, env: {}, nice: false, lazy_block: nil, &block) cmd = [Gitlab.config.git.bin_path, *args] cmd.unshift("nice") if nice @@ -1289,20 +1246,6 @@ module Gitlab } end - def git_merged_branch_names(branch_names, root_sha) - git_arguments = - %W[branch --merged #{root_sha} - --format=%(refname:short)\ %(objectname)] + branch_names - - lines = run_git(git_arguments).first.lines - - lines.each_with_object([]) do |line, branches| - name, sha = line.strip.split(' ', 2) - - branches << name if sha != root_sha - end - end - def gitaly_merged_branch_names(branch_names, root_sha) qualified_branch_names = branch_names.map { |b| "refs/heads/#{b}" } @@ -1448,12 +1391,6 @@ module Gitlab raise CommandError, @gitlab_projects.output end - def rugged_merge_base(from, to) - rugged.merge_base(from, to) - rescue Rugged::ReferenceError - nil - end - def rev_list_param(spec) spec == :all ? ['--all'] : spec end diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb index 830565620d6..149eeb4f9ba 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -2,16 +2,22 @@ require "spec_helper" describe "User creates wiki page" do let(:user) { create(:user) } + let(:wiki) { ProjectWiki.new(project, user) } + let(:project) { create(:project) } before do project.add_maintainer(user) - sign_in(user) - visit(project_wikis_path(project)) - click_link "Create your first page" + sign_in(user) end context "when wiki is empty" do + before do + visit(project_wikis_path(project)) + + click_link "Create your first page" + end + context "in a user namespace" do let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } @@ -165,7 +171,9 @@ describe "User creates wiki page" do context "when wiki is not empty", :js do before do - create(:wiki_page, wiki: create(:project, :wiki_repo, namespace: user.namespace).wiki, attrs: { title: "home", content: "Home page" }) + create(:wiki_page, wiki: wiki, attrs: { title: 'home', content: 'Home page' }) + + visit(project_wikis_path(project)) end context "in a user namespace" do @@ -290,4 +298,34 @@ describe "User creates wiki page" do end end end + + describe 'sidebar feature' do + context 'when there are some existing pages' do + before do + create(:wiki_page, wiki: wiki, attrs: { title: 'home', content: 'home' }) + create(:wiki_page, wiki: wiki, attrs: { title: 'another', content: 'another' }) + end + + it 'renders a default sidebar when there is no customized sidebar' do + visit(project_wikis_path(project)) + + expect(page).to have_content('Another') + expect(page).to have_content('More Pages') + end + + context 'when there is a customized sidebar' do + before do + create(:wiki_page, wiki: wiki, attrs: { title: '_sidebar', content: 'My customized sidebar' }) + end + + it 'renders my customized sidebar instead of the default one' do + visit(project_wikis_path(project)) + + expect(page).to have_content('My customized sidebar') + expect(page).to have_content('More Pages') + expect(page).not_to have_content('Another') + end + end + end + end end diff --git a/spec/lib/additional_email_headers_interceptor_spec.rb b/spec/lib/gitlab/email/hook/additional_headers_interceptor_spec.rb index b5c1a360ba9..ae61ece8029 100644 --- a/spec/lib/additional_email_headers_interceptor_spec.rb +++ b/spec/lib/gitlab/email/hook/additional_headers_interceptor_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe AdditionalEmailHeadersInterceptor do +describe Gitlab::Email::Hook::AdditionalHeadersInterceptor do let(:mail) do ActionMailer::Base.mail(to: 'test@mail.com', from: 'info@mail.com', body: 'hello') end diff --git a/spec/lib/gitlab/email/hook/delivery_metrics_observer_spec.rb b/spec/lib/gitlab/email/hook/delivery_metrics_observer_spec.rb new file mode 100644 index 00000000000..4497d4002da --- /dev/null +++ b/spec/lib/gitlab/email/hook/delivery_metrics_observer_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Gitlab::Email::Hook::DeliveryMetricsObserver do + let(:email) do + ActionMailer::Base.mail(to: 'test@example.com', + from: 'info@example.com', + body: 'hello') + end + + context 'when email has been delivered' do + it 'increments both email delivery metrics' do + expect(described_class.delivery_attempts_counter).to receive(:increment) + expect(described_class.delivered_emails_counter).to receive(:increment) + + email.deliver_now + end + end + + context 'when email has not been delivered due to an error' do + before do + allow(email.delivery_method).to receive(:deliver!) + .and_raise(StandardError, 'Some SMTP error') + end + + it 'increments only delivery attempt metric' do + expect(described_class.delivery_attempts_counter) + .to receive(:increment) + expect(described_class.delivered_emails_counter) + .not_to receive(:increment) + + expect { email.deliver_now } + .to raise_error(StandardError, 'Some SMTP error') + end + end +end diff --git a/spec/lib/disable_email_interceptor_spec.rb b/spec/lib/gitlab/email/hook/disable_email_interceptor_spec.rb index 3652d928c43..91aa3bc7c2e 100644 --- a/spec/lib/disable_email_interceptor_spec.rb +++ b/spec/lib/gitlab/email/hook/disable_email_interceptor_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe DisableEmailInterceptor do +describe Gitlab::Email::Hook::DisableEmailInterceptor do before do Mail.register_interceptor(described_class) end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 581132b6672..ff1a5aa2536 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1366,7 +1366,8 @@ describe Notify do it 'only sends the text template' do stub_application_setting(html_emails_enabled: false) - EmailTemplateInterceptor.delivering_email(multipart_mail) + Gitlab::Email::Hook::EmailTemplateInterceptor + .delivering_email(multipart_mail) expect(multipart_mail).to have_part_with('text/plain') expect(multipart_mail).not_to have_part_with('text/html') @@ -1377,7 +1378,8 @@ describe Notify do it 'sends a multipart message' do stub_application_setting(html_emails_enabled: true) - EmailTemplateInterceptor.delivering_email(multipart_mail) + Gitlab::Email::Hook::EmailTemplateInterceptor + .delivering_email(multipart_mail) expect(multipart_mail).to have_part_with('text/plain') expect(multipart_mail).to have_part_with('text/html') diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index a544940800a..528f5b610d7 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -189,6 +189,22 @@ describe ProjectWiki do end end + describe '#find_sidebar' do + before do + create_page(described_class::SIDEBAR, 'This is an awesome Sidebar') + end + + after do + subject.pages.each { |page| destroy_page(page.page) } + end + + it 'finds the page defined as _sidebar' do + page = subject.find_page('_sidebar') + + expect(page.content).to eq('This is an awesome Sidebar') + end + end + describe '#find_file' do shared_examples 'finding a wiki file' do let(:image) { File.open(Rails.root.join('spec', 'fixtures', 'big-image.png')) } |