Merge branch 'master' into per-project-pipeline-iid

4 files changed, 81 insertions, 72 deletions

diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb
index 4ddcbd7eb66..19028495f52 100644
--- a/spec/lib/gitlab/current_settings_spec.rb
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -1,17 +1,15 @@
 require 'spec_helper'
 
 describe Gitlab::CurrentSettings do
-  include StubENV
-
   before do
     stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
   end
 
-  describe '#current_application_settings' do
+  describe '#current_application_settings', :use_clean_rails_memory_store_caching do
     it 'allows keys to be called directly' do
       db_settings = create(:application_setting,
-                           home_page_url: 'http://mydomain.com',
-                           signup_enabled: false)
+        home_page_url: 'http://mydomain.com',
+        signup_enabled: false)
 
       expect(described_class.home_page_url).to eq(db_settings.home_page_url)
       expect(described_class.signup_enabled?).to be_falsey
@@ -19,46 +17,54 @@ describe Gitlab::CurrentSettings do
       expect(described_class.metrics_sample_interval).to be(15)
     end
 
-    context 'with DB available' do
+    context 'when ENV["IN_MEMORY_APPLICATION_SETTINGS"] is true' do
       before do
-        # For some reason, `allow(described_class).to receive(:connect_to_db?).and_return(true)` causes issues
-        # during the initialization phase of the test suite, so instead let's mock the internals of it
-        allow(ActiveRecord::Base.connection).to receive(:active?).and_return(true)
-        allow(ActiveRecord::Base.connection).to receive(:table_exists?).and_call_original
-        allow(ActiveRecord::Base.connection).to receive(:table_exists?).with('application_settings').and_return(true)
+        stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'true')
       end
 
-      it 'attempts to use cached values first' do
-        expect(ApplicationSetting).to receive(:cached)
+      it 'returns an in-memory ApplicationSetting object' do
+        expect(ApplicationSetting).not_to receive(:current)
 
         expect(described_class.current_application_settings).to be_a(ApplicationSetting)
+        expect(described_class.current_application_settings).not_to be_persisted
       end
+    end
 
-      it 'falls back to DB if Redis returns an empty value' do
-        expect(ApplicationSetting).to receive(:cached).and_return(nil)
-        expect(ApplicationSetting).to receive(:last).and_call_original.twice
-
-        expect(described_class.current_application_settings).to be_a(ApplicationSetting)
+    context 'with DB unavailable' do
+      before do
+        # For some reason, `allow(described_class).to receive(:connect_to_db?).and_return(false)` causes issues
+        # during the initialization phase of the test suite, so instead let's mock the internals of it
+        allow(ActiveRecord::Base.connection).to receive(:active?).and_return(false)
       end
 
-      it 'falls back to DB if Redis fails' do
-        db_settings = ApplicationSetting.create!(ApplicationSetting.defaults)
+      it 'returns an in-memory ApplicationSetting object' do
+        expect(ApplicationSetting).not_to receive(:current)
 
-        expect(ApplicationSetting).to receive(:cached).and_raise(::Redis::BaseError)
-        expect(Rails.cache).to receive(:fetch).with(ApplicationSetting::CACHE_KEY).and_raise(Redis::BaseError)
+        expect(described_class.current_application_settings).to be_a(Gitlab::FakeApplicationSettings)
+      end
+    end
 
-        expect(described_class.current_application_settings).to eq(db_settings)
+    context 'with DB available' do
+      # This method returns the ::ApplicationSetting.defaults hash
+      # but with respect of custom attribute accessors of ApplicationSetting model
+      def settings_from_defaults
+        ar_wrapped_defaults = ::ApplicationSetting.build_from_defaults.attributes
+        ar_wrapped_defaults.slice(*::ApplicationSetting.defaults.keys)
       end
 
-      it 'creates default ApplicationSettings if none are present' do
-        expect(ApplicationSetting).to receive(:cached).and_raise(::Redis::BaseError)
-        expect(Rails.cache).to receive(:fetch).with(ApplicationSetting::CACHE_KEY).and_raise(Redis::BaseError)
+      before do
+        # For some reason, `allow(described_class).to receive(:connect_to_db?).and_return(true)` causes issues
+        # during the initialization phase of the test suite, so instead let's mock the internals of it
+        allow(ActiveRecord::Base.connection).to receive(:active?).and_return(true)
+        allow(ActiveRecord::Base.connection).to receive(:cached_table_exists?).with('application_settings').and_return(true)
+      end
 
+      it 'creates default ApplicationSettings if none are present' do
         settings = described_class.current_application_settings
 
         expect(settings).to be_a(ApplicationSetting)
         expect(settings).to be_persisted
-        expect(settings).to have_attributes(ApplicationSetting.defaults)
+        expect(settings).to have_attributes(settings_from_defaults)
       end
 
       context 'with migrations pending' do
@@ -69,7 +75,7 @@ describe Gitlab::CurrentSettings do
         it 'returns an in-memory ApplicationSetting object' do
           settings = described_class.current_application_settings
 
-          expect(settings).to be_a(OpenStruct)
+          expect(settings).to be_a(Gitlab::FakeApplicationSettings)
           expect(settings.sign_in_enabled?).to eq(settings.sign_in_enabled)
           expect(settings.sign_up_enabled?).to eq(settings.sign_up_enabled)
         end
@@ -81,7 +87,7 @@ describe Gitlab::CurrentSettings do
           settings = described_class.current_application_settings
           app_defaults = ApplicationSetting.last
 
-          expect(settings).to be_a(OpenStruct)
+          expect(settings).to be_a(Gitlab::FakeApplicationSettings)
           expect(settings.home_page_url).to eq(db_settings.home_page_url)
           expect(settings.signup_enabled?).to be_falsey
           expect(settings.signup_enabled).to be_falsey
@@ -91,34 +97,29 @@ describe Gitlab::CurrentSettings do
           settings.each { |key, _| expect(settings[key]).to eq(app_defaults[key]) }
         end
       end
-    end
-
-    context 'with DB unavailable' do
-      before do
-        # For some reason, `allow(described_class).to receive(:connect_to_db?).and_return(false)` causes issues
-        # during the initialization phase of the test suite, so instead let's mock the internals of it
-        allow(ActiveRecord::Base.connection).to receive(:active?).and_return(false)
-      end
 
-      it 'returns an in-memory ApplicationSetting object' do
-        expect(ApplicationSetting).not_to receive(:current)
-        expect(ApplicationSetting).not_to receive(:last)
+      context 'when ApplicationSettings.current is present' do
+        it 'returns the existing application settings' do
+          expect(ApplicationSetting).to receive(:current).and_return(:current_settings)
 
-        expect(described_class.current_application_settings).to be_a(OpenStruct)
+          expect(described_class.current_application_settings).to eq(:current_settings)
+        end
       end
-    end
 
-    context 'when ENV["IN_MEMORY_APPLICATION_SETTINGS"] is true' do
-      before do
-        stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'true')
+      context 'when the application_settings table does not exists' do
+        it 'returns an in-memory ApplicationSetting object' do
+          expect(ApplicationSetting).to receive(:create_from_defaults).and_raise(ActiveRecord::StatementInvalid)
+
+          expect(described_class.current_application_settings).to be_a(Gitlab::FakeApplicationSettings)
+        end
       end
 
-      it 'returns an in-memory ApplicationSetting object' do
-        expect(ApplicationSetting).not_to receive(:current)
-        expect(ApplicationSetting).not_to receive(:last)
+      context 'when the application_settings table is not fully migrated' do
+        it 'returns an in-memory ApplicationSetting object' do
+          expect(ApplicationSetting).to receive(:create_from_defaults).and_raise(ActiveRecord::UnknownAttributeError)
 
-        expect(described_class.current_application_settings).to be_a(ApplicationSetting)
-        expect(described_class.current_application_settings).not_to be_persisted
+          expect(described_class.current_application_settings).to be_a(Gitlab::FakeApplicationSettings)
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
index 452249210b0..154ab4b3856 100644
--- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
@@ -46,6 +46,20 @@ describe Gitlab::Email::Handler::CreateIssueHandler do
         expect(issue.description).to eq('')
       end
     end
+
+    context "when there are quotes in email" do
+      let(:email_raw) { fixture_file("emails/valid_new_issue_with_quote.eml") }
+
+      it "creates a new issue" do
+        expect { receiver.execute }.to change { project.issues.count }.by(1)
+        issue = project.issues.last
+
+        expect(issue.author).to eq(user)
+        expect(issue.title).to eq('New Issue by email')
+        expect(issue.description).to include('reply by email')
+        expect(issue.description).to include('> this is a quote')
+      end
+    end
   end
 
   context "something is wrong" do
diff --git a/spec/lib/gitlab/email/reply_parser_spec.rb b/spec/lib/gitlab/email/reply_parser_spec.rb
index e21a998adfe..0989188f7ee 100644
--- a/spec/lib/gitlab/email/reply_parser_spec.rb
+++ b/spec/lib/gitlab/email/reply_parser_spec.rb
@@ -3,8 +3,8 @@ require "spec_helper"
 # Inspired in great part by Discourse's Email::Receiver
 describe Gitlab::Email::ReplyParser do
   describe '#execute' do
-    def test_parse_body(mail_string)
-      described_class.new(Mail::Message.new(mail_string)).execute
+    def test_parse_body(mail_string, params = {})
+      described_class.new(Mail::Message.new(mail_string), params).execute
     end
 
     it "returns an empty string if the message is blank" do
@@ -212,5 +212,19 @@ describe Gitlab::Email::ReplyParser do
     it "does not wrap links with no href in unnecessary brackets" do
       expect(test_parse_body(fixture_file("emails/html_empty_link.eml"))).to eq("no brackets!")
     end
+
+    it "does not trim reply if trim_reply option is false" do
+      expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { trim_reply: false }))
+        .to eq(
+          <<-BODY.strip_heredoc.chomp
+          The reply by email functionality should be extended to allow creating a new issue by email.
+          even when the email is forwarded to the project which may include lines that begin with ">"
+
+          there should be a quote below this line:
+
+          > this is a quote
+          BODY
+        )
+    end
   end
 end
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 6d63749296e..4f64f2bd6b4 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -6760,26 +6760,6 @@
       "wiki_page_events": true
     },
     {
-      "id": 92,
-      "title": "Gemnasium",
-      "project_id": 5,
-      "created_at": "2016-06-14T15:01:51.202Z",
-      "updated_at": "2016-06-14T15:01:51.202Z",
-      "active": false,
-      "properties": {},
-      "template": false,
-      "push_events": true,
-      "issues_events": true,
-      "merge_requests_events": true,
-      "tag_push_events": true,
-      "note_events": true,
-      "job_events": true,
-      "type": "GemnasiumService",
-      "category": "common",
-      "default": false,
-      "wiki_page_events": true
-    },
-    {
       "id": 91,
       "title": "Flowdock",
       "project_id": 5,