Merge branch '30529-remove-pages-tab-if-pages-isn-t-enabled' into 'master'

Disable navigation to Pages config if Pages is disabled

Closes #30529

See merge request !11008

13 files changed, 145 insertions, 48 deletions

diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index e2f81b09adc..89f1128ec36 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -89,4 +89,8 @@ class Projects::ApplicationController < ApplicationController
   def builds_enabled
     return render_404 unless @project.feature_available?(:builds, current_user)
   end
+
+  def require_pages_enabled!
+    not_found unless Gitlab.config.pages.enabled
+  end
 end
diff --git a/app/controllers/projects/pages_controller.rb b/app/controllers/projects/pages_controller.rb
index fbd18b68141..93b2c180810 100644
--- a/app/controllers/projects/pages_controller.rb
+++ b/app/controllers/projects/pages_controller.rb
@@ -1,6 +1,7 @@
 class Projects::PagesController < Projects::ApplicationController
   layout 'project_settings'
 
+  before_action :require_pages_enabled!
   before_action :authorize_read_pages!, only: [:show]
   before_action :authorize_update_pages!, except: [:show]
 
diff --git a/app/controllers/projects/pages_domains_controller.rb b/app/controllers/projects/pages_domains_controller.rb
index b8c253f6ae3..3a93977fd27 100644
--- a/app/controllers/projects/pages_domains_controller.rb
+++ b/app/controllers/projects/pages_domains_controller.rb
@@ -1,6 +1,7 @@
 class Projects::PagesDomainsController < Projects::ApplicationController
   layout 'project_settings'
 
+  before_action :require_pages_enabled!
   before_action :authorize_update_pages!, except: [:show]
   before_action :domain, only: [:show, :destroy]
 
diff --git a/app/views/projects/pages/_disabled.html.haml b/app/views/projects/pages/_disabled.html.haml
deleted file mode 100644
index ad51fbc6cab..00000000000
--- a/app/views/projects/pages/_disabled.html.haml
+++ /dev/null
@@ -1,4 +0,0 @@
-.panel.panel-default
-  .nothing-here-block
-    GitLab Pages are disabled.
-    Ask your system's administrator to enable it.
diff --git a/app/views/projects/pages/show.html.haml b/app/views/projects/pages/show.html.haml
index 259d5bd63d6..b22a54d75c8 100644
--- a/app/views/projects/pages/show.html.haml
+++ b/app/views/projects/pages/show.html.haml
@@ -16,13 +16,10 @@
 
 %hr.clearfix
 
-- if Gitlab.config.pages.enabled
-  = render 'access'
-  = render 'use'
-  - if Gitlab.config.pages.external_http || Gitlab.config.pages.external_https
-    = render 'list'
-  - else
-    = render 'no_domains'
-  = render 'destroy'
+= render 'access'
+= render 'use'
+- if Gitlab.config.pages.external_http || Gitlab.config.pages.external_https
+  = render 'list'
 - else
-  = render 'disabled'
+  = render 'no_domains'
+= render 'destroy'
diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml
index 5a5ade03624..8c7f9e0191e 100644
--- a/app/views/projects/settings/_head.html.haml
+++ b/app/views/projects/settings/_head.html.haml
@@ -27,7 +27,8 @@
               = link_to namespace_project_settings_ci_cd_path(@project.namespace, @project), title: 'CI/CD Pipelines' do
                 %span
                   CI/CD Pipelines
-          = nav_link(controller: :pages) do
-            = link_to namespace_project_pages_path(@project.namespace, @project), title: 'Pages' do
-              %span
-                Pages
+          - if Gitlab.config.pages.enabled
+            = nav_link(controller: :pages) do
+              = link_to namespace_project_pages_path(@project.namespace, @project), title: 'Pages' do
+                %span
+                  Pages
diff --git a/changelogs/unreleased/30529-remove-pages-tab-if-pages-isn-t-enabled.yml b/changelogs/unreleased/30529-remove-pages-tab-if-pages-isn-t-enabled.yml
new file mode 100644
index 00000000000..16938f05326
--- /dev/null
+++ b/changelogs/unreleased/30529-remove-pages-tab-if-pages-isn-t-enabled.yml
@@ -0,0 +1,4 @@
+---
+title: Disable navigation to Project-level pages configuration when Pages disabled
+merge_request: 11008
+author:
diff --git a/features/project/active_tab.feature b/features/project/active_tab.feature
index 0d6f7350181..34201cd8486 100644
--- a/features/project/active_tab.feature
+++ b/features/project/active_tab.feature
@@ -63,13 +63,6 @@ Feature: Project Active Tab
     And no other sub tabs should be active
     And the active main tab should be Settings
 
-  Scenario: On Project Settings/Pages
-    Given I visit my project's settings page
-    And I click the "Pages" tab
-    Then the active sub tab should be Pages
-    And no other sub tabs should be active
-    And the active main tab should be Settings
-
   Scenario: On Project Members
     Given I visit my project's members page
     Then the active sub tab should be Members
diff --git a/features/project/pages.feature b/features/project/pages.feature
index 87d88348d09..56e47287b5c 100644
--- a/features/project/pages.feature
+++ b/features/project/pages.feature
@@ -3,10 +3,15 @@ Feature: Project Pages
     Given I sign in as a user
     And I own a project
 
-  Scenario: Pages are disabled
+  Scenario: I cannot navigate to Pages settings if pages enabled
     Given pages are disabled
-    When I visit the Project Pages
-    Then I should see that GitLab Pages are disabled
+    And I visit my project's settings page
+    Then I should not see the "Pages" tab
+
+  Scenario: I can navigate to Pages settings if pages enabled
+    Given pages are enabled
+    And I visit my project's settings page
+    Then I should see the "Pages" tab
 
   Scenario: I can see the pages usage if not deployed
     Given pages are enabled
diff --git a/features/steps/project/active_tab.rb b/features/steps/project/active_tab.rb
index 4befd49ac81..5cd9bd38c9d 100644
--- a/features/steps/project/active_tab.rb
+++ b/features/steps/project/active_tab.rb
@@ -39,12 +39,6 @@ class Spinach::Features::ProjectActiveTab < Spinach::FeatureSteps
     end
   end
 
-  step 'I click the "Pages" tab' do
-    page.within '.sub-nav' do
-      click_link('Pages')
-    end
-  end
-
   step 'I click the "Activity" tab' do
     page.within '.sub-nav' do
       click_link('Activity')
diff --git a/features/steps/project/pages.rb b/features/steps/project/pages.rb
index 4045955a8b9..fea82d9fb57 100644
--- a/features/steps/project/pages.rb
+++ b/features/steps/project/pages.rb
@@ -18,14 +18,22 @@ class Spinach::Features::ProjectPages < Spinach::FeatureSteps
     visit namespace_project_pages_path(@project.namespace, @project)
   end
 
-  step 'I should see that GitLab Pages are disabled' do
-    expect(page).to have_content('GitLab Pages are disabled')
-  end
-
   step 'I should see the usage of GitLab Pages' do
     expect(page).to have_content('Configure pages')
   end
 
+  step 'I should see the "Pages" tab' do
+    page.within '.sub-nav' do
+      expect(page).to have_link('Pages')
+    end
+  end
+
+  step 'I should not see the "Pages" tab' do
+    page.within '.sub-nav' do
+      expect(page).not_to have_link('Pages')
+    end
+  end
+
   step 'pages are deployed' do
     pipeline = @project.ensure_pipeline('HEAD', @project.commit('HEAD').sha)
     build = build(:ci_build,
diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb
new file mode 100644
index 00000000000..df35d8e86b9
--- /dev/null
+++ b/spec/controllers/projects/pages_controller_spec.rb
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe Projects::PagesController do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project, :public, :access_requestable) }
+
+  let(:request_params) do
+    {
+      namespace_id: project.namespace,
+      project_id: project
+    }
+  end
+
+  before do
+    allow(Gitlab.config.pages).to receive(:enabled).and_return(true)
+    sign_in(user)
+    project.add_master(user)
+  end
+
+  describe 'GET show' do
+    it 'returns 200 status' do
+      get :show, request_params
+
+      expect(response).to have_http_status(200)
+    end
+  end
+
+  describe 'DELETE destroy' do
+    it 'returns 302 status' do
+      delete :destroy, request_params
+
+      expect(response).to have_http_status(302)
+    end
+  end
+
+  context 'pages disabled' do
+    before do
+      allow(Gitlab.config.pages).to receive(:enabled).and_return(false)
+    end
+
+    describe 'GET show' do
+      it 'returns 404 status' do
+        get :show, request_params
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    describe 'DELETE destroy' do
+      it 'returns 404 status' do
+        delete :destroy, request_params
+
+        expect(response).to have_http_status(404)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb
index 2362df895a8..33853c4b9d0 100644
--- a/spec/controllers/projects/pages_domains_controller_spec.rb
+++ b/spec/controllers/projects/pages_domains_controller_spec.rb
@@ -1,8 +1,9 @@
 require 'spec_helper'
 
 describe Projects::PagesDomainsController do
-  let(:user)    { create(:user) }
-  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+  let!(:pages_domain) { create(:pages_domain, project: project) }
 
   let(:request_params) do
     {
@@ -11,14 +12,17 @@ describe Projects::PagesDomainsController do
     }
   end
 
+  let(:pages_domain_params) do
+    build(:pages_domain, :with_certificate, :with_key, domain: 'my.otherdomain.com').slice(:key, :certificate, :domain)
+  end
+
   before do
+    allow(Gitlab.config.pages).to receive(:enabled).and_return(true)
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe 'GET show' do
-    let!(:pages_domain)   { create(:pages_domain, project: project) }
-
     it "displays the 'show' page" do
       get(:show, request_params.merge(id: pages_domain.domain))
 
@@ -37,10 +41,6 @@ describe Projects::PagesDomainsController do
   end
 
   describe 'POST create' do
-    let(:pages_domain_params) do
-      build(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate, :domain)
-    end
-
     it "creates a new pages domain" do
       expect do
         post(:create, request_params.merge(pages_domain: pages_domain_params))
@@ -51,8 +51,6 @@ describe Projects::PagesDomainsController do
   end
 
   describe 'DELETE destroy' do
-    let!(:pages_domain)   { create(:pages_domain, project: project) }
-
     it "deletes the pages domain" do
       expect do
         delete(:destroy, request_params.merge(id: pages_domain.domain))
@@ -61,4 +59,42 @@ describe Projects::PagesDomainsController do
       expect(response).to redirect_to(namespace_project_pages_path(project.namespace, project))
     end
   end
+
+  context 'pages disabled' do
+    before do
+      allow(Gitlab.config.pages).to receive(:enabled).and_return(false)
+    end
+
+    describe 'GET show' do
+      it 'returns 404 status' do
+        get(:show, request_params.merge(id: pages_domain.domain))
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    describe 'GET new' do
+      it 'returns 404 status' do
+        get :new, request_params
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    describe 'POST create' do
+      it "returns 404 status" do
+        post(:create, request_params.merge(pages_domain: pages_domain_params))
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    describe 'DELETE destroy' do
+      it "deletes the pages domain" do
+        delete(:destroy, request_params.merge(id: pages_domain.domain))
+
+        expect(response).to have_http_status(404)
+      end
+    end
+  end
 end