Merge branch 'add-sse-config-file-2' into 'master'

Introduce StaticSiteEditorConfigService

See merge request gitlab-org/gitlab!41923

app/controllers/projects/static_site_editor_controller.rb

@@ -14,19 +14,27 @@ class Projects::StaticSiteEditorController < Projects::ApplicationController
   end
 
   def show
-    config = Gitlab::StaticSiteEditor::Config::CombinedConfig.new(
-      @repository,
-      @ref,
-      @path,
-      params[:return_url]
-    )
-    @data = config.data
+    service_response = ::StaticSiteEditor::ConfigService.new(
+      container: project,
+      current_user: current_user,
+      params: {
+        ref: @ref,
+        path: @path,
+        return_url: params[:return_url]
+      }
+    ).execute
+
+    if service_response.success?
+      @data = service_response.payload
+    else
+      respond_422
+    end
   end
 
   private
 
   def assign_ref_and_path
-    @ref, @path = extract_ref(params[:id])
+    @ref, @path = extract_ref(params.fetch(:id))
 
     render_404 if @ref.blank? || @path.blank?
   end

app/services/static_site_editor/config_service.rb

+# frozen_string_literal: true
+
+module StaticSiteEditor
+  class ConfigService < ::BaseContainerService
+    ValidationError = Class.new(StandardError)
+
+    def execute
+      @project = container
+      check_access!
+
+      ServiceResponse.success(payload: data)
+    rescue ValidationError => e
+      ServiceResponse.error(message: e.message)
+    end
+
+    private
+
+    attr_reader :project
+
+    def check_access!
+      unless can?(current_user, :download_code, project)
+        raise ValidationError, 'Insufficient permissions to read configuration'
+      end
+    end
+
+    def data
+      check_for_duplicate_keys!
+      generated_data.merge(file_data)
+    end
+
+    def generated_data
+      @generated_data ||= Gitlab::StaticSiteEditor::Config::GeneratedConfig.new(
+        project.repository,
+        params.fetch(:ref),
+        params.fetch(:path),
+        params[:return_url]
+      ).data
+    end
+
+    def file_data
+      @file_data ||= Gitlab::StaticSiteEditor::Config::FileConfig.new.data
+    end
+
+    def check_for_duplicate_keys!
+      duplicate_keys = generated_data.keys & file_data.keys
+      raise ValidationError.new("Duplicate key(s) '#{duplicate_keys}' found.") if duplicate_keys.present?
+    end
+  end
+end

lib/gitlab/static_site_editor/config/combined_config.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module StaticSiteEditor
-    module Config
-      class CombinedConfig
-        def initialize(repository, ref, path, return_url)
-          @repository = repository
-          @ref = ref
-          @path = path
-          @return_url = return_url
-        end
-
-        def data
-          generated_data = Gitlab::StaticSiteEditor::Config::GeneratedConfig.new(
-            @repository,
-            @ref,
-            @path,
-            @return_url
-          ).data
-          file_data = Gitlab::StaticSiteEditor::Config::FileConfig.new.data
-          check_for_duplicate_keys(generated_data, file_data)
-          generated_data.merge(file_data)
-        end
-
-        private
-
-        def check_for_duplicate_keys(generated_data, file_data)
-          duplicate_keys = generated_data.keys & file_data.keys
-          raise StandardError.new("Duplicate key(s) '#{duplicate_keys}' found.") if duplicate_keys.present?
-        end
-      end
-    end
-  end
-end

lib/gitlab/static_site_editor/config/file_config.rb

@@ -5,9 +5,8 @@ module Gitlab
     module Config
       class FileConfig
         def data
-          merge_requests_illustration_path = ActionController::Base.helpers.image_path('illustrations/merge_requests.svg')
           {
-            merge_requests_illustration_path: merge_requests_illustration_path
+            static_site_generator: 'middleman'
           }
         end
       end

lib/gitlab/static_site_editor/config/generated_config.rb

@@ -14,6 +14,7 @@ module Gitlab
         end
 
         def data
+          merge_requests_illustration_path = ActionController::Base.helpers.image_path('illustrations/merge_requests.svg')
           {
             branch: ref,
             path: path,
@@ -23,7 +24,8 @@ module Gitlab
             namespace: project.namespace.full_path,
             return_url: sanitize_url(return_url),
             is_supported_content: supported_content?.to_s,
-            base_url: Gitlab::Routing.url_helpers.project_show_sse_path(project, full_path)
+            base_url: Gitlab::Routing.url_helpers.project_show_sse_path(project, full_path),
+            merge_requests_illustration_path: merge_requests_illustration_path
           }
         end
 

spec/controllers/projects/static_site_editor_controller_spec.rb

@@ -7,12 +7,6 @@ RSpec.describe Projects::StaticSiteEditorController do
   let_it_be(:user) { create(:user) }
   let(:data) { instance_double(Hash) }
 
-  before do
-    allow_next_instance_of(Gitlab::StaticSiteEditor::Config::CombinedConfig) do |config|
-      allow(config).to receive(:data) { data }
-    end
-  end
-
   describe 'GET show' do
     let(:default_params) do
       {
@@ -23,6 +17,16 @@ RSpec.describe Projects::StaticSiteEditorController do
       }
     end
 
+    let(:service_response) do
+      ServiceResponse.success(payload: data)
+    end
+
+    before do
+      allow_next_instance_of(::StaticSiteEditor::ConfigService) do |instance|
+        allow(instance).to receive(:execute).and_return(service_response)
+      end
+    end
+
     context 'User roles' do
       context 'anonymous' do
         before do
@@ -74,6 +78,14 @@ RSpec.describe Projects::StaticSiteEditorController do
               expect(response).to have_gitlab_http_status(:not_found)
             end
           end
+
+          context 'when invalid config file' do
+            let(:service_response) { ServiceResponse.error(message: 'invalid') }
+
+            it 'returns 422' do
+              expect(response).to have_gitlab_http_status(:unprocessable_entity)
+            end
+          end
         end
       end
     end

spec/features/static_site_editor_spec.rb

@@ -13,7 +13,11 @@ RSpec.describe 'Static Site Editor' do
     visit project_show_sse_path(project, 'master/README.md')
   end
 
-  it 'renders Static Site Editor page' do
-    expect(page).to have_selector('#static-site-editor')
+  it 'renders Static Site Editor page with generated and file attributes' do
+    # assert generated config value is present
+    expect(page).to have_css('#static-site-editor[data-branch="master"]')
+
+    # assert file config value is present
+    expect(page).to have_css('#static-site-editor[data-static-site-generator="middleman"]')
   end
 end

spec/lib/gitlab/static_site_editor/config/combined_config_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::StaticSiteEditor::Config::CombinedConfig do
-  subject(:config) { described_class.new(repository, ref, path, return_url) }
-
-  let(:repository) { double(:repository) }
-  let(:ref) { double(:ref) }
-  let(:path) { double(:path) }
-  let(:return_url) { double(:return_url) }
-  let(:generated_data) { { generated: true } }
-  let(:file_data) { { file: true } }
-
-  describe '#data' do
-    subject { config.data }
-
-    before do
-      allow_next_instance_of(Gitlab::StaticSiteEditor::Config::GeneratedConfig) do |config|
-        allow(config).to receive(:data) { generated_data }
-      end
-      allow_next_instance_of(Gitlab::StaticSiteEditor::Config::FileConfig) do |config|
-        allow(config).to receive(:data) { file_data }
-      end
-    end
-
-    it 'returns merged generated data and config file data' do
-      is_expected.to eq({ generated: true, file: true })
-    end
-
-    it 'raises an exception if any keys would be overwritten by the merge' do
-      generated_data[:duplicate_key] = true
-      file_data[:duplicate_key] = true
-      expect { subject }.to raise_error(StandardError, /duplicate key.*duplicate_key.*found/i)
-    end
-  end
-end

spec/lib/gitlab/static_site_editor/config/file_config_spec.rb

@@ -9,9 +9,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig do
     subject { config.data }
 
     it 'returns hardcoded data for now' do
-      is_expected.to match(
-        merge_requests_illustration_path: %r{illustrations/merge_requests}
-      )
+      is_expected.to match(static_site_generator: 'middleman')
     end
   end
 end

spec/lib/gitlab/static_site_editor/config/generated_config_spec.rb

@@ -20,7 +20,8 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do
     subject { config.data }
 
     it 'returns data for the frontend component' do
-      is_expected.to eq(
+      is_expected
+        .to match({
                     branch: 'master',
                     commit_id: repository.commit.id,
                     namespace: 'namespace',
@@ -29,8 +30,9 @@ RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do
                     project_id: project.id,
                     return_url: 'http://example.com',
                     is_supported_content: 'true',
-        base_url: '/namespace/project/-/sse/master%2FREADME.md'
-      )
+                    base_url: '/namespace/project/-/sse/master%2FREADME.md',
+                    merge_requests_illustration_path: %r{illustrations/merge_requests}
+                  })
     end
 
     context 'when namespace is a subgroup' do

spec/services/static_site_editor/config_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe StaticSiteEditor::ConfigService do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+
+  # params
+  let(:ref) { double(:ref) }
+  let(:path) { double(:path) }
+  let(:return_url) { double(:return_url) }
+
+  # stub data
+  let(:generated_data) { { generated: true } }
+  let(:file_data) { { file: true } }
+
+  describe '#execute' do
+    subject(:execute) do
+      described_class.new(
+        container: project,
+        current_user: user,
+        params: {
+          ref: ref,
+          path: path,
+          return_url: return_url
+        }
+      ).execute
+    end
+
+    context 'when insufficient permission' do
+      it 'returns an error' do
+        expect(execute).to be_error
+        expect(execute.message).to eq('Insufficient permissions to read configuration')
+      end
+    end
+
+    context 'for developer' do
+      before do
+        project.add_developer(user)
+
+        allow_next_instance_of(Gitlab::StaticSiteEditor::Config::GeneratedConfig) do |config|
+          allow(config).to receive(:data) { generated_data }
+        end
+
+        allow_next_instance_of(Gitlab::StaticSiteEditor::Config::FileConfig) do |config|
+          allow(config).to receive(:data) { file_data }
+        end
+      end
+
+      it 'returns merged generated data and config file data' do
+        expect(execute).to be_success
+        expect(execute.payload).to eq(generated: true, file: true)
+      end
+
+      it 'returns an error if any keys would be overwritten by the merge' do
+        generated_data[:duplicate_key] = true
+        file_data[:duplicate_key] = true
+        expect(execute).to be_error
+        expect(execute.message).to match(/duplicate key.*duplicate_key.*found/i)
+      end
+    end
+  end
+end