Refactoring for Static Site Editor config

Move logic out of controllers/views into library classes
  * Simplify StaticSiteEditorController and view
  * Refactor Config into CombinedConfig, GeneratedConfig
    and FileConfig
Make specs more mock-friendly
Rename refactors for consistency

app/controllers/projects/static_site_editor_controller.rb

@@ -14,7 +14,13 @@ class Projects::StaticSiteEditorController < Projects::ApplicationController
   end
 
   def show
-    @config = Gitlab::StaticSiteEditor::Config.new(@repository, @ref, @path, params[:return_url])
+    config = Gitlab::StaticSiteEditor::Config::CombinedConfig.new(
+      @repository,
+      @ref,
+      @path,
+      params[:return_url]
+    )
+    @data = config.data
   end
 
   private

app/views/projects/static_site_editor/show.html.haml

-#static-site-editor{ data: @config.payload.merge({ merge_requests_illustration_path: image_path('illustrations/merge_requests.svg') }) }
+#static-site-editor{ data: @data }

lib/gitlab/static_site_editor/config.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module StaticSiteEditor
-    class Config
-      SUPPORTED_EXTENSIONS = %w[.md].freeze
-
-      def initialize(repository, ref, file_path, return_url)
-        @repository = repository
-        @ref = ref
-        @file_path = file_path
-        @return_url = return_url
-        @commit_id = repository.commit(ref)&.id if ref
-      end
-
-      def payload
-        {
-          branch: ref,
-          path: file_path,
-          commit_id: commit_id,
-          project_id: project.id,
-          project: project.path,
-          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)
-        }
-      end
-
-      private
-
-      attr_reader :repository, :ref, :file_path, :return_url, :commit_id
-
-      delegate :project, to: :repository
-
-      def supported_content?
-        master_branch? && extension_supported? && file_exists?
-      end
-
-      def master_branch?
-        ref == 'master'
-      end
-
-      def extension_supported?
-        return true if file_path.end_with?('.md.erb') && Feature.enabled?(:sse_erb_support, project)
-
-        SUPPORTED_EXTENSIONS.any? { |ext| file_path.end_with?(ext) }
-      end
-
-      def file_exists?
-        commit_id.present? && !repository.blob_at(commit_id, file_path).nil?
-      end
-
-      def full_path
-        "#{ref}/#{file_path}"
-      end
-
-      def sanitize_url(url)
-        url if Gitlab::UrlSanitizer.valid_web?(url)
-      end
-    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

+# frozen_string_literal: true
+
+module Gitlab
+  module StaticSiteEditor
+    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
+          }
+        end
+      end
+    end
+  end
+end

lib/gitlab/static_site_editor/config/generated_config.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module StaticSiteEditor
+    module Config
+      class GeneratedConfig
+        SUPPORTED_EXTENSIONS = %w[.md].freeze
+
+        def initialize(repository, ref, path, return_url)
+          @repository = repository
+          @ref = ref
+          @path = path
+          @return_url = return_url
+        end
+
+        def data
+          {
+            branch: ref,
+            path: path,
+            commit_id: commit_id,
+            project_id: project.id,
+            project: project.path,
+            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)
+          }
+        end
+
+        private
+
+        attr_reader :repository, :ref, :path, :return_url
+
+        delegate :project, to: :repository
+
+        def commit_id
+          repository.commit(ref)&.id if ref
+        end
+
+        def supported_content?
+          master_branch? && extension_supported? && file_exists?
+        end
+
+        def master_branch?
+          ref == 'master'
+        end
+
+        def extension_supported?
+          return true if path.end_with?('.md.erb') && Feature.enabled?(:sse_erb_support, project)
+
+          SUPPORTED_EXTENSIONS.any? { |ext| path.end_with?(ext) }
+        end
+
+        def file_exists?
+          commit_id.present? && !repository.blob_at(commit_id, path).nil?
+        end
+
+        def full_path
+          "#{ref}/#{path}"
+        end
+
+        def sanitize_url(url)
+          url if Gitlab::UrlSanitizer.valid_web?(url)
+        end
+      end
+    end
+  end
+end

spec/controllers/projects/static_site_editor_controller_spec.rb

@@ -5,6 +5,13 @@ require 'spec_helper'
 RSpec.describe Projects::StaticSiteEditorController do
   let_it_be(:project) { create(:project, :public, :repository) }
   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
@@ -55,12 +62,12 @@ RSpec.describe Projects::StaticSiteEditorController do
           end
 
           it 'assigns a required variables' do
-            expect(assigns(:config)).to be_a(Gitlab::StaticSiteEditor::Config)
+            expect(assigns(:data)).to eq(data)
             expect(assigns(:ref)).to eq('master')
             expect(assigns(:path)).to eq('README.md')
           end
 
-          context 'when combination of ref and file path is incorrect' do
+          context 'when combination of ref and path is incorrect' do
             let(:default_params) { super().merge(id: 'unknown') }
 
             it 'responds with 404 page' do

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig do
+  subject(:config) { described_class.new }
+
+  describe '#data' do
+    subject { config.data }
+
+    it 'returns hardcoded data for now' do
+      is_expected.to match(
+        merge_requests_illustration_path: %r{illustrations/merge_requests}
+      )
+    end
+  end
+end

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

@@ -2,8 +2,8 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::StaticSiteEditor::Config do
-  subject(:config) { described_class.new(repository, ref, file_path, return_url) }
+RSpec.describe Gitlab::StaticSiteEditor::Config::GeneratedConfig do
+  subject(:config) { described_class.new(repository, ref, path, return_url) }
 
   let_it_be(:namespace) { create(:namespace, name: 'namespace') }
   let_it_be(:root_group) { create(:group, name: 'group') }
@@ -13,11 +13,11 @@ RSpec.describe Gitlab::StaticSiteEditor::Config do
   let_it_be(:repository) { project.repository }
 
   let(:ref) { 'master' }
-  let(:file_path) { 'README.md' }
+  let(:path) { 'README.md' }
   let(:return_url) { 'http://example.com' }
 
-  describe '#payload' do
-    subject { config.payload }
+  describe '#data' do
+    subject { config.data }
 
     it 'returns data for the frontend component' do
       is_expected.to eq(
@@ -49,7 +49,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config do
       before do
         repository.create_file(
           project.creator,
-          file_path,
+          path,
           '',
           message: 'message',
           branch_name: 'master'
@@ -57,7 +57,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config do
       end
 
       context 'when feature flag is enabled' do
-        let(:file_path) { 'FEATURE_ON.md.erb' }
+        let(:path) { 'FEATURE_ON.md.erb' }
 
         before do
           stub_feature_flags(sse_erb_support: project)
@@ -67,7 +67,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config do
       end
 
       context 'when feature flag is disabled' do
-        let(:file_path) { 'FEATURE_OFF.md.erb' }
+        let(:path) { 'FEATURE_OFF.md.erb' }
 
         before do
           stub_feature_flags(sse_erb_support: false)
@@ -78,7 +78,7 @@ RSpec.describe Gitlab::StaticSiteEditor::Config do
     end
 
     context 'when file path is nested' do
-      let(:file_path) { 'lib/README.md' }
+      let(:path) { 'lib/README.md' }
 
       it { is_expected.to include(base_url: '/namespace/project/-/sse/master%2Flib%2FREADME.md') }
     end
@@ -90,19 +90,19 @@ RSpec.describe Gitlab::StaticSiteEditor::Config do
     end
 
     context 'when file does not have a markdown extension' do
-      let(:file_path) { 'README.txt' }
+      let(:path) { 'README.txt' }
 
       it { is_expected.to include(is_supported_content: 'false') }
     end
 
     context 'when file does not have an extension' do
-      let(:file_path) { 'README' }
+      let(:path) { 'README' }
 
       it { is_expected.to include(is_supported_content: 'false') }
     end
 
     context 'when file does not exist' do
-      let(:file_path) { 'UNKNOWN.md' }
+      let(:path) { 'UNKNOWN.md' }
 
       it { is_expected.to include(is_supported_content: 'false') }
     end