Merge branch '2584-namespace-export-csv' into 'master'

Namespace license checks for exporting issues (EES)

Closes #2584

See merge request !2164

app/controllers/ee/projects/issues_controller.rb

+module EE
+  module Projects
+    module IssuesController
+      extend ActiveSupport::Concern
+
+      included do
+        before_action :check_export_issues_available!, only: [:export_csv]
+      end
+
+      def export_csv
+        ExportCsvWorker.perform_async(current_user.id, project.id, filter_params)
+
+        index_path = namespace_project_issues_path(project.namespace, project)
+        redirect_to(index_path, notice: "Your CSV export has started. It will be emailed to #{current_user.notification_email} when complete.")
+      end
+    end
+  end
+end

app/controllers/projects/application_controller.rb

@@ -53,9 +53,16 @@ class Projects::ApplicationController < ApplicationController
     end
   end
 
+  def check_project_feature_available!(feature)
+    render_404 unless project.feature_available?(feature, current_user)
+  end
+
   def method_missing(method_sym, *arguments, &block)
-    if method_sym.to_s =~ /\Aauthorize_(.*)!\z/
+    case method_sym.to_s
+    when /\Aauthorize_(.*)!\z/
       authorize_action!($1.to_sym)
+    when /\Acheck_(.*)_available!\z/
+      check_project_feature_available!($1.to_sym)
     else
       super
     end

app/controllers/projects/issues_controller.rb

@@ -6,6 +6,8 @@ class Projects::IssuesController < Projects::ApplicationController
   include IssuableCollections
   include SpammableActions
 
+  include ::EE::Projects::IssuesController
+
   prepend_before_action :authenticate_user!, only: [:new, :export_csv]
 
   before_action :redirect_to_external_issue_tracker, only: [:index, :new]
@@ -156,13 +158,6 @@ class Projects::IssuesController < Projects::ApplicationController
     render_conflict_response
   end
 
-  def export_csv
-    ExportCsvWorker.perform_async(@current_user.id, @project.id, filter_params)
-
-    index_path = namespace_project_issues_path(@project.namespace, @project)
-    redirect_to(index_path, notice: "Your CSV export has started. It will be emailed to #{current_user.notification_email} when complete.")
-  end
-
   def referenced_merge_requests
     @merge_requests = @issue.referenced_merge_requests(current_user)
     @closed_by_merge_requests = @issue.closed_by_merge_requests(current_user)

app/models/license.rb

@@ -9,6 +9,7 @@ class License < ActiveRecord::Base
   OBJECT_STORAGE_FEATURE = 'GitLab_ObjectStorage'.freeze
   ELASTIC_SEARCH_FEATURE = 'GitLab_ElasticSearch'.freeze
   RELATED_ISSUES_FEATURE = 'RelatedIssues'.freeze
+  EXPORT_ISSUES_FEATURE  = 'GitLab_ExportIssues'.freeze
 
   FEATURE_CODES = {
     geo: GEO_FEATURE,
@@ -19,7 +20,8 @@ class License < ActiveRecord::Base
     related_issues: RELATED_ISSUES_FEATURE,
     # Features that make sense to Namespace:
     deploy_board: DEPLOY_BOARD_FEATURE,
-    file_lock: FILE_LOCK_FEATURE
+    file_lock: FILE_LOCK_FEATURE,
+    export_issues: EXPORT_ISSUES_FEATURE
   }.freeze
 
   STARTER_PLAN = 'starter'.freeze
@@ -29,7 +31,8 @@ class License < ActiveRecord::Base
 
   EES_FEATURES = [
     { ELASTIC_SEARCH_FEATURE => 1 },
-    { RELATED_ISSUES_FEATURE => 1 }
+    { RELATED_ISSUES_FEATURE => 1 },
+    { EXPORT_ISSUES_FEATURE => 1 }
   ].freeze
 
   EEP_FEATURES = [
@@ -61,7 +64,8 @@ class License < ActiveRecord::Base
     { GEO_FEATURE => 1 },
     { AUDITOR_USER_FEATURE => 1 },
     { SERVICE_DESK_FEATURE => 1 },
-    { OBJECT_STORAGE_FEATURE => 1 }
+    { OBJECT_STORAGE_FEATURE => 1 },
+    { EXPORT_ISSUES_FEATURE => 1 }
   ].freeze
 
   FEATURES_BY_PLAN = {

app/views/projects/issues/export_issues/_button.html.haml

+- if current_user && @project.feature_available?(:export_issues)
+  %button.csv_download_link.btn.append-right-10.has-tooltip{ title: 'Export as CSV' }
+    = icon('download')

app/views/projects/issues/export_issues/_csv_download.html.haml

+- return unless current_user && @project.feature_available?(:export_issues)
+
 .issues-export-modal.modal
   .modal-dialog
     .modal-content

app/views/projects/issues/index.html.haml

@@ -15,17 +15,18 @@
   = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@project.name} issues")
 
 - if project_issues(@project).exists?
-  - if current_user
-    = render "projects/issues/export_issues/csv_download"
+
+  = render 'projects/issues/export_issues/csv_download'
+
   %div{ class: (container_class) }
     .top-area
       = render 'shared/issuable/nav', type: :issues
       .nav-controls.inline
         = link_to params.merge(rss_url_options), class: 'btn append-right-10 has-tooltip', title: 'Subscribe' do
           = icon('rss')
-        - if current_user
-          %button.csv_download_link.btn.append-right-10.has-tooltip{ title: 'Export as CSV' }
-            = icon('download')
+
+        = render 'projects/issues/export_issues/button'
+
         - if @can_bulk_update
           = button_tag "Edit Issues", class: "btn btn-default js-bulk-update-toggle"
         = link_to new_namespace_project_issue_path(@project.namespace,

changelogs/unreleased-ee/2584-namespace-export-csv.yml

+---
+title: Namespace license checks for exporting issues (EES)
+merge_request: 2164
+author:

spec/controllers/projects/issues_controller_ee_spec.rb

+require('spec_helper')
+
+describe Projects::IssuesController do
+  let(:namespace) { create(:namespace) }
+  let(:project)   { create(:project_empty_repo, namespace: namespace) }
+  let(:user)      { create(:user) }
+  let(:viewer)    { user }
+  let(:issue)     { create(:issue, project: project) }
+
+  describe 'POST export_csv' do
+    let(:globally_licensed) { false }
+
+    before do
+      project.add_developer(user)
+
+      sign_in(viewer) if viewer
+
+      allow(License).to receive(:feature_available?).and_call_original
+      allow(License).to receive(:feature_available?).with(:export_issues).and_return(globally_licensed)
+    end
+
+    def request_csv
+      post :export_csv, namespace_id: project.namespace.to_param, project_id: project.to_param
+    end
+
+    context 'unlicensed' do
+      it 'returns 404' do
+        expect(ExportCsvWorker).not_to receive(:perform_async)
+
+        request_csv
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'globally licensed' do
+      let(:globally_licensed) { true }
+
+      it 'allows CSV export' do
+        expect(ExportCsvWorker).to receive(:perform_async).with(viewer.id, project.id, anything)
+
+        request_csv
+
+        expect(response).to redirect_to(namespace_project_issues_path(project.namespace, project))
+        expect(response.flash[:notice]).to match(/\AYour CSV export has started/i)
+      end
+
+      context 'anonymous user' do
+        let(:project) { create(:project_empty_repo, :public, namespace: namespace) }
+        let(:viewer) { nil }
+
+        it 'redirects to the sign in page' do
+          request_csv
+
+          expect(ExportCsvWorker).not_to receive(:perform_async)
+          expect(response).to redirect_to(new_user_session_path)
+        end
+      end
+    end
+
+    context 'licensed by namespace' do
+      let(:globally_licensed) { true }
+      let(:namespace) { create(:group, :private, plan: Namespace::BRONZE_PLAN) }
+
+      before do
+        stub_application_setting(check_namespace_plan: true)
+      end
+
+      it 'allows CSV export' do
+        expect(ExportCsvWorker).to receive(:perform_async).with(viewer.id, project.id, anything)
+
+        request_csv
+
+        expect(response).to redirect_to(namespace_project_issues_path(project.namespace, project))
+        expect(response.flash[:notice]).to match(/\AYour CSV export has started/i)
+      end
+    end
+  end
+end

spec/models/ee/project_spec.rb

@@ -22,73 +22,77 @@ describe Project, models: true do
       before do
         stub_application_setting('check_namespace_plan?' => check_namespace_plan)
         allow(Gitlab).to receive(:com?) { true }
-        expect_any_instance_of(License).to receive(:feature_available?).with(feature) { allowed_on_global_license }
+        expect(License).to receive(:feature_available?).with(feature) { allowed_on_global_license }
         allow(namespace).to receive(:plan) { plan_license }
       end
 
       License::FEATURE_CODES.each do |feature_sym, feature_code|
-        let(:feature) { feature_sym }
-        let(:feature_code) { feature_code }
+        context feature_sym.to_s do
+          let(:feature) { feature_sym }
+          let(:feature_code) { feature_code }
 
-        context "checking #{feature} availabily both on Global and Namespace license" do
-          let(:check_namespace_plan) { true }
+          context "checking #{feature_sym} availability both on Global and Namespace license" do
+            let(:check_namespace_plan) { true }
 
-          context 'allowed by Plan License AND Global License' do
-            let(:allowed_on_global_license) { true }
-            let(:plan_license) { Namespace::GOLD_PLAN }
+            context 'allowed by Plan License AND Global License' do
+              let(:allowed_on_global_license) { true }
+              let(:plan_license) { Namespace::GOLD_PLAN }
 
-            it 'returns true' do
-              is_expected.to eq(true)
+              it 'returns true' do
+                is_expected.to eq(true)
+              end
             end
-          end
 
-          context 'not allowed by Plan License but project and namespace are public' do
-            let(:allowed_on_global_license) { true }
-            let(:plan_license) { Namespace::BRONZE_PLAN }
+            context 'not allowed by Plan License but project and namespace are public' do
+              let(:allowed_on_global_license) { true }
+              let(:plan_license) { Namespace::BRONZE_PLAN }
 
-            it 'returns true' do
-              allow(namespace).to receive(:public?) { true }
-              allow(project).to receive(:public?) { true }
+              it 'returns true' do
+                allow(namespace).to receive(:public?) { true }
+                allow(project).to receive(:public?) { true }
 
-              is_expected.to eq(true)
+                is_expected.to eq(true)
+              end
             end
-          end
 
-          context 'not allowed by Plan License' do
-            let(:allowed_on_global_license) { true }
-            let(:plan_license) { Namespace::BRONZE_PLAN }
+            unless License.plan_includes_feature?(License::STARTER_PLAN, feature_sym)
+              context 'not allowed by Plan License' do
+                let(:allowed_on_global_license) { true }
+                let(:plan_license) { Namespace::BRONZE_PLAN }
 
-            it 'returns false' do
-              is_expected.to eq(false)
+                it 'returns false' do
+                  is_expected.to eq(false)
+                end
+              end
             end
-          end
 
-          context 'not allowed by Global License' do
-            let(:allowed_on_global_license) { false }
-            let(:plan_license) { Namespace::GOLD_PLAN }
+            context 'not allowed by Global License' do
+              let(:allowed_on_global_license) { false }
+              let(:plan_license) { Namespace::GOLD_PLAN }
 
-            it 'returns false' do
-              is_expected.to eq(false)
+              it 'returns false' do
+                is_expected.to eq(false)
+              end
             end
           end
-        end
 
-        context "when checking #{feature_code} only for Global license" do
-          let(:check_namespace_plan) { false }
+          context "when checking #{feature_code} only for Global license" do
+            let(:check_namespace_plan) { false }
 
-          context 'allowed by Global License' do
-            let(:allowed_on_global_license) { true }
+            context 'allowed by Global License' do
+              let(:allowed_on_global_license) { true }
 
-            it 'returns true' do
-              is_expected.to eq(true)
+              it 'returns true' do
+                is_expected.to eq(true)
+              end
             end
-          end
 
-          context 'not allowed by Global License' do
-            let(:allowed_on_global_license) { false }
+            context 'not allowed by Global License' do
+              let(:allowed_on_global_license) { false }
 
-            it 'returns false' do
-              is_expected.to eq(false)
+              it 'returns false' do
+                is_expected.to eq(false)
+              end
             end
           end
         end