Merge remote-tracking branch 'upstream/master' into ce-to-ee-2017-06-15

app/models/ee/project.rb

@@ -55,8 +55,13 @@ module EE
       end
     end
 
+    def service_desk_enabled
+      ::EE::Gitlab::ServiceDesk.enabled?(project: self) && super
+    end
+    alias_method :service_desk_enabled?, :service_desk_enabled
+
     def service_desk_address
-      return nil unless service_desk_available?
+      return nil unless service_desk_enabled?
 
       config = ::Gitlab.config.incoming_email
       wildcard = ::Gitlab::IncomingEmail::WILDCARD_PLACEHOLDER
@@ -80,7 +85,7 @@ module EE
     private
 
     def licensed_feature_available?(feature)
-      globally_available = License.current&.feature_available?(feature)
+      globally_available = License.feature_available?(feature)
 
       if current_application_settings.should_check_namespace_plan?
         globally_available &&
@@ -93,11 +98,5 @@ module EE
     def destroy_mirror_data
       mirror_data.destroy
     end
-
-    def service_desk_available?
-      return @service_desk_available if defined?(@service_desk_available)
-
-      @service_desk_available = EE::Gitlab::ServiceDesk.enabled? && service_desk_enabled?
-    end
   end
 end

app/models/license.rb

@@ -203,6 +203,10 @@ class License < ActiveRecord::Base
      restricted_attr(:trueup_to)].all?(&:present?)
   end
 
+  def trial?
+    restricted_attr(:trial)
+  end
+
   private
 
   def restricted_attr(name, default = nil)

app/views/admin/audit_logs/index.html.haml

@@ -28,7 +28,7 @@
               placeholder: admin_namespace_dropdown_label('Search groups'), idAttribute: 'id', data: { order_by: 'last_activity_at', idattribute: 'id', all_available: true} })
 
   - if @events.present?
-    %table.table
+    %table#events-table.table
       %thead
         %tr
           %th Author

app/views/projects/edit.html.haml

@@ -130,7 +130,7 @@
 
         = render 'merge_request_settings', form: f
 
-        - if EE::Gitlab::ServiceDesk.enabled?
+        - if EE::Gitlab::ServiceDesk.enabled?(project: @project)
           %hr
           %fieldset.js-service-desk-setting-wrapper.features.append-bottom-default
             %h5.prepend-top-0

app/workers/historical_data_worker.rb

@@ -4,6 +4,8 @@ class HistoricalDataWorker
 
   def perform
     return if Gitlab::Geo.secondary?
+    return if License.current.nil? || License.current&.trial?
+
     HistoricalData.track!
   end
 end

changelogs/unreleased-ee/2565-namespace-service-desk.yml

+---
+title: 'Add namespace license checks for Service Desk (EEP)'
+merge_request: 2109
+author:

lib/ee/gitlab/ldap/sync/group.rb

@@ -145,7 +145,7 @@ module EE
               elsif group.last_owner?(user)
                 warn_cannot_remove_last_owner(user, group)
               else
-                group.users.delete(user)
+                group.users.destroy(user)
               end
             end
           end

lib/ee/gitlab/service_desk.rb

 module EE
   module Gitlab
     module ServiceDesk
-      def self.enabled?
-        ::License.current&.feature_available?(:service_desk) &&
-          ::Gitlab::IncomingEmail.enabled? &&
-          ::Gitlab::IncomingEmail.supports_wildcard?
+      # Check whether a project or GitLab instance can support the Service Desk
+      # feature. Use `project.service_desk_enabled?` to check whether it is
+      # enabled for a particular project.
+      def self.enabled?(project: nil)
+        return unless ::Gitlab::IncomingEmail.enabled? && ::Gitlab::IncomingEmail.supports_wildcard?
+
+        (project || ::License).feature_available?(:service_desk)
       end
     end
   end

lib/gitlab/email/handler/ee/service_desk_handler.rb

@@ -31,9 +31,11 @@ module Gitlab
           def project
             return @project if instance_variable_defined?(:@project)
 
-            @project =
+            found_project =
               Project.where(service_desk_enabled: true)
                 .find_by_full_path(service_desk_key)
+
+            @project = found_project&.service_desk_enabled? ? found_project : nil
           end
 
           def create_issue!

lib/gitlab/usage_data.rb

@@ -55,7 +55,7 @@ module Gitlab
       end
 
       def service_desk_counts
-        return {} unless ::License.current&.feature_available?(:service_desk)
+        return {} unless ::License.feature_available?(:service_desk)
 
         projects_with_service_desk = Project.where(service_desk_enabled: true)
 

spec/controllers/projects/service_desk_controller_spec.rb

 require 'spec_helper'
 
 describe Projects::ServiceDeskController do
-  let(:project) { create(:project_empty_repo, :private) }
+  let(:project) { create(:project_empty_repo, :private, service_desk_enabled: true) }
   let(:user)    { create(:user) }
 
   before do
-    allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-    allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { true }
+    allow(License).to receive(:feature_available?).and_call_original
+    allow(License).to receive(:feature_available?).with(:service_desk) { true }
     allow(Gitlab::IncomingEmail).to receive(:enabled?) { true }
     allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
-    project.update(service_desk_enabled: true)
+
     project.add_master(user)
     sign_in(user)
   end
@@ -41,7 +41,7 @@ describe Projects::ServiceDeskController do
 
   describe 'PUT service desk properties' do
     it 'toggles services desk incoming email' do
-      project.update(service_desk_enabled: false)
+      project.update!(service_desk_enabled: false)
 
       put :update, namespace_id: project.namespace.to_param, project_id: project, service_desk_enabled: true, format: :json
 

spec/factories/licenses.rb

@@ -15,9 +15,19 @@ FactoryGirl.define do
     end
     notify_users_at   { |l| l.expires_at }
     notify_admins_at  { |l| l.expires_at }
+
+    trait :trial do
+      restrictions do
+        { trial: true }
+      end
+    end
   end
 
   factory :license do
     data { build(:gitlab_license).export }
   end
+
+  factory :trial_license, class: License do
+    data { build(:gitlab_license, :trial).export }
+  end
 end

spec/features/admin/admin_audit_logs_spec.rb

@@ -51,6 +51,8 @@ describe 'Admin::AuditLogs', feature: true, js: true do
       wait_for_requests
       find('.select2-results').click
 
+      find('#events-table td', match: :first)
+
       expect(page).to have_content('Added user access as Owner')
     end
   end
@@ -73,6 +75,8 @@ describe 'Admin::AuditLogs', feature: true, js: true do
       wait_for_requests
       find('.select2-results').click
 
+      find('#events-table td', match: :first)
+
       expect(page).to have_content('Removed user access')
     end
   end

spec/features/projects/settings/ee/service_desk_setting_spec.rb

@@ -7,8 +7,8 @@ describe 'Service Desk Setting', js: true, feature: true do
   before do
     project.add_master(user)
     login_as(user)
-    allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-    allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { true }
+
+    allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(true)
     allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
     allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
 

spec/lib/ee/gitlab/ldap/sync/group_spec.rb

@@ -235,6 +235,15 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
           expect(group.members.find_by(user_id: user.id).access_level)
             .to eq(::Gitlab::Access::OWNER)
         end
+
+        it 'updates projects authorizations' do
+          project = create(:empty_project, namespace: group)
+          group.add_user(user, Gitlab::Access::MASTER)
+
+          sync_group.update_permissions
+
+          expect(project.authorized_users.find_by(id: user.id)).to be_nil
+        end
       end
 
       context 'when the user is the last owner' do

spec/lib/ee/gitlab/service_desk_spec.rb

@@ -2,8 +2,8 @@ require 'spec_helper'
 
 describe EE::Gitlab::ServiceDesk, lib: true do
   before do
-    allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-    allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { true }
+    allow(License).to receive(:feature_available?).and_call_original
+    allow(License).to receive(:feature_available?).with(:service_desk) { true }
     allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
     allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
   end
@@ -14,7 +14,7 @@ describe EE::Gitlab::ServiceDesk, lib: true do
 
   context 'when license does not support service desk' do
     before do
-      allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { false }
+      allow(License).to receive(:feature_available?).with(:service_desk) { false }
     end
 
     it { is_expected.to be_falsy }

spec/lib/gitlab/email/handler/ee/service_desk_handler_spec.rb

@@ -10,19 +10,16 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
 
   let(:email_raw) { fixture_file('emails/service_desk.eml') }
   let(:namespace) { create(:namespace, name: "email") }
-  let(:project) { create(:project, :public, namespace: namespace, path: "test") }
 
-  context 'when service desk is enabled' do
-    before do
-      project.update(service_desk_enabled: true)
+  context 'service desk is enabled for the project' do
+    let(:project) { create(:empty_project, :public, namespace: namespace, path: 'test', service_desk_enabled: true) }
 
+    before do
       allow(Notify).to receive(:service_desk_thank_you_email)
         .with(kind_of(Integer)).and_return(double(deliver_later!: true))
 
-      allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-      allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { true }
-      allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
-      allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
+      allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(true)
+      allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(true)
     end
 
     it 'sends thank you the email and creates issue' do
@@ -56,8 +53,7 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
 
     context 'when license does not support service desk' do
       before do
-        allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-        allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { false }
+        allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
       end
 
       it 'does not create an issue or send email' do
@@ -88,16 +84,14 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
     end
   end
 
-  context 'when service desk is not enabled' do
-    before do
-      project.update_attributes(service_desk_enabled: false)
-    end
+  context 'service desk is disabled for the project' do
+    let(:project) { create(:empty_project, :public, namespace: namespace, path: 'test') }
 
     it 'bounces the email' do
       expect { receiver.execute }.to raise_error(Gitlab::Email::ProcessingError)
     end
 
-    it 'doesn\'t create an issue' do
+    it "doesn't create an issue" do
       expect { receiver.execute rescue nil }.not_to change { Issue.count }
     end
   end

spec/lib/gitlab/email/handler_spec.rb

@@ -13,15 +13,15 @@ describe Gitlab::Email::Handler, lib: true do
 
     context 'a Service Desk email' do
       it 'uses the Service Desk handler when Service Desk is enabled' do
-        allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-        allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk).and_return(true)
+        allow(License).to receive(:feature_available?).and_call_original
+        allow(License).to receive(:feature_available?).with(:service_desk).and_return(true)
 
         expect(handler_for('emails/service_desk.eml', 'some/project')).to be_instance_of(Gitlab::Email::Handler::EE::ServiceDeskHandler)
       end
 
       it 'uses no handler when Service Desk is disabled' do
-        allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-        allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk).and_return(false)
+        allow(License).to receive(:feature_available?).and_call_original
+        allow(License).to receive(:feature_available?).with(:service_desk).and_return(false)
 
         expect(handler_for('emails/service_desk.eml', 'some/project')).to be_nil
       end
@@ -31,15 +31,15 @@ describe Gitlab::Email::Handler, lib: true do
       let!(:user) { create(:user, email: 'jake@adventuretime.ooo', incoming_email_token: 'auth_token') }
 
       it 'uses the create issue handler when Service Desk is enabled' do
-        allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-        allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk).and_return(true)
+        allow(License).to receive(:feature_available?).and_call_original
+        allow(License).to receive(:feature_available?).with(:service_desk).and_return(true)
 
         expect(handler_for('emails/valid_new_issue.eml', 'some/project+auth_token')).to be_instance_of(Gitlab::Email::Handler::CreateIssueHandler)
       end
 
       it 'uses the create issue handler when Service Desk is disabled' do
-        allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-        allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk).and_return(false)
+        allow(License).to receive(:feature_available?).and_call_original
+        allow(License).to receive(:feature_available?).with(:service_desk).and_return(false)
 
         expect(handler_for('emails/valid_new_issue.eml', 'some/project+auth_token')).to be_instance_of(Gitlab::Email::Handler::CreateIssueHandler)
       end

spec/lib/gitlab/usage_data_spec.rb

@@ -94,12 +94,13 @@ describe Gitlab::UsageData do
   describe '.service_desk_counts' do
     subject { described_class.service_desk_counts }
 
-    let!(:project3) { create(:empty_project, service_desk_enabled: true) }
-    let!(:project4) { create(:empty_project, service_desk_enabled: true) }
+    before do
+      Project.update_all(service_desk_enabled: true)
+    end
 
     context 'when Service Desk is disabled' do
       it 'returns an empty hash' do
-        allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk).and_return(false)
+        allow(License).to receive(:feature_available?).with(:service_desk).and_return(false)
 
         expect(subject).to eq({})
       end
@@ -115,8 +116,10 @@ describe Gitlab::UsageData do
 
     context 'when Service Desk is enabled' do
       it 'gathers Service Desk data' do
-        create_list(:issue, 3, confidential: true, author: User.support_bot, project: [project3, project4].sample)
-        allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk).and_return(true)
+        create_list(:issue, 3, confidential: true, author: User.support_bot, project: project)
+
+        allow(License).to receive(:feature_available?).with(:service_desk).and_return(true)
+        allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).with(anything).and_return(true)
 
         expect(subject).to eq(service_desk_enabled_projects: 2,
                               service_desk_issues: 3)

spec/models/ee/project_spec.rb

@@ -211,12 +211,49 @@ describe Project, models: true do
     end
   end
 
+  describe '#service_desk_enabled?' do
+    let!(:license) { create(:license, data: build(:gitlab_license, restrictions: { plan: License::PREMIUM_PLAN }).export) }
+    let(:namespace) { create(:namespace) }
+
+    subject(:project) { build(:empty_project, :private, namespace: namespace, service_desk_enabled: true) }
+
+    before do
+      allow(::Gitlab).to receive(:com?).and_return(true)
+      allow(::Gitlab::IncomingEmail).to receive(:enabled?).and_return(true)
+      allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?).and_return(true)
+    end
+
+    it 'is enabled' do
+      expect(project.service_desk_enabled?).to be_truthy
+      expect(project.service_desk_enabled).to be_truthy
+    end
+
+    context 'namespace plans active' do
+      before do
+        stub_application_setting(check_namespace_plan: true)
+      end
+
+      it 'is disabled' do
+        expect(project.service_desk_enabled?).to be_falsy
+        expect(project.service_desk_enabled).to be_falsy
+      end
+
+      context 'Service Desk available in namespace plan' do
+        let(:namespace) { create(:namespace, plan: Namespace::SILVER_PLAN) }
+
+        it 'is enabled' do
+          expect(project.service_desk_enabled?).to be_truthy
+          expect(project.service_desk_enabled).to be_truthy
+        end
+      end
+    end
+  end
+
   describe '#service_desk_address' do
     let(:project) { create(:empty_project, service_desk_enabled: true) }
 
     before do
-      allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-      allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { true }
+      allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(true)
       allow(Gitlab.config.incoming_email).to receive(:enabled).and_return(true)
       allow(Gitlab.config.incoming_email).to receive(:address).and_return("test+%{key}@mail.com")
     end

spec/services/ee/notification_service_spec.rb

@@ -7,8 +7,7 @@ describe EE::NotificationService do
     allow(Notify).to receive(:service_desk_new_note_email)
       .with(kind_of(Integer), kind_of(Integer)).and_return(double(deliver_later: true))
 
-    allow_any_instance_of(License).to receive(:feature_available?).and_call_original
-    allow_any_instance_of(License).to receive(:feature_available?).with(:service_desk) { true }
+    allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(true)
     allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
     allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
   end

spec/workers/historical_data_worker_spec.rb

+require 'spec_helper'
+
+describe HistoricalDataWorker do
+  subject { described_class.new }
+
+  describe '#perform' do
+    context 'with a trial license' do
+      before do
+        FactoryGirl.create(:trial_license)
+      end
+
+      it 'does not track historical data' do
+        expect(HistoricalData).not_to receive(:track!)
+
+        subject.perform
+      end
+    end
+
+    context 'with a non trial license' do
+      before do
+        FactoryGirl.create(:license)
+      end
+
+      it 'tracks historical data' do
+        expect(HistoricalData).to receive(:track!)
+
+        subject.perform
+      end
+    end
+
+    context 'with a Geo secondary node' do
+      it 'does not track historical data' do
+        allow(Gitlab::Geo).to receive(:secondary?).and_return(true)
+
+        expect(HistoricalData).not_to receive(:track!)
+
+        subject.perform
+      end
+    end
+
+    context 'when there is not a license key' do
+      it 'does not track historical data' do
+        License.destroy_all
+
+        expect(HistoricalData).not_to receive(:track!)
+
+        subject.perform
+      end
+    end
+  end
+end