Port of 55057-system-message-to-core to EE

remove EE specific code
remove EE licence checks
move migration from EE to CE folder structure
move specs from EE to CE folder structure
remove EE specific flag specs

app/assets/stylesheets/framework.scss

@@ -66,3 +66,4 @@
 @import 'framework/terms';
 @import 'framework/read_more';
 @import 'framework/flex_grid';
+@import 'framework/system_messages';

ee/app/assets/stylesheets/framework/system_messages.scss → app/assets/stylesheets/framework/system_messages.scss

@@ -25,8 +25,8 @@
 
 .footer-message {
   bottom: 0;
-  height: $system-header-height;
-  line-height: $system-header-height;
+  height: $system-footer-height;
+  line-height: $system-footer-height;
 }
 
 .with-performance-bar {
@@ -86,7 +86,7 @@
     // navless pages' footer border eg: login page
   &.devise-layout-html body .footer-container,
   &.devise-layout-html body hr.footer-fixed {
-    bottom: $system-header-height;
+    bottom: $system-footer-height;
   }
 }
 

app/assets/stylesheets/framework/variables.scss

@@ -277,6 +277,8 @@ $general-hover-transition-duration: 100ms;
 $general-hover-transition-curve: linear;
 $highlight-changes-color: rgb(235, 255, 232);
 $performance-bar-height: 35px;
+$system-header-height: 35px;
+$system-footer-height: $system-header-height;
 $flash-height: 52px;
 $context-header-height: 60px;
 $breadcrumb-min-height: 48px;

app/controllers/admin/appearances_controller.rb

@@ -74,8 +74,10 @@ class Admin::AppearancesController < Admin::ApplicationController
       favicon_cache
       new_project_guidelines
       updated_by
+      header_message
+      footer_message
+      message_background_color
+      message_font_color
     ]
   end
 end
-
-Admin::AppearancesController.prepend(EE::Admin::AppearancesController)

app/helpers/appearances_helper.rb

@@ -40,6 +40,38 @@ module AppearancesHelper
       render 'shared/logo_type.svg'
     end
   end
+
+  def header_message
+    return unless current_appearance&.show_header?
+
+    class_names = []
+    class_names << 'with-performance-bar' if performance_bar_enabled?
+
+    render_message(:header_message, class_names)
+  end
+
+  def footer_message
+    return unless current_appearance&.show_footer?
+
+    render_message(:footer_message)
+  end
+
+  private
+
+  def render_message(field_sym, class_names = [])
+    class_names << field_sym.to_s.dasherize
+
+    content_tag :div, class: class_names, style: message_style do
+      markdown_field(current_appearance, field_sym)
+    end
+  end
+
+  def message_style
+    style = []
+    style << "background-color: #{current_appearance.message_background_color};"
+    style << "color: #{current_appearance.message_font_color}"
+    style.join
+  end
 end
 
 AppearancesHelper.prepend(EE::AppearancesHelper)

app/helpers/application_helper.rb

@@ -214,12 +214,19 @@ module ApplicationHelper
     class_names = []
     class_names << 'issue-boards-page' if current_controller?(:boards)
     class_names << 'with-performance-bar' if performance_bar_enabled?
-
+    class_names << system_message_class
     class_names
   end
 
-  # EE feature: System header and footer, unavailable in CE
   def system_message_class
+    class_names = []
+
+    return class_names unless appearance
+
+    class_names << 'with-system-header' if appearance.show_header?
+    class_names << 'with-system-footer' if appearance.show_footer?
+
+    class_names
   end
 
   # Returns active css class when condition returns true
@@ -292,6 +299,12 @@ module ApplicationHelper
       snippets: snippets_project_autocomplete_sources_path(object)
     }
   end
+
+  private
+
+  def appearance
+    ::Appearance.current
+  end
 end
 
 ApplicationHelper.prepend(EE::ApplicationHelper)

app/models/appearance.rb

@@ -8,12 +8,19 @@ class Appearance < ActiveRecord::Base
 
   cache_markdown_field :description
   cache_markdown_field :new_project_guidelines
+  cache_markdown_field :header_message, pipeline: :broadcast_message
+  cache_markdown_field :footer_message, pipeline: :broadcast_message
 
   validates :logo,        file_size: { maximum: 1.megabyte }
   validates :header_logo, file_size: { maximum: 1.megabyte }
+  validates :message_background_color, allow_blank: true, color: true
+  validates :message_font_color, allow_blank: true, color: true
 
   validate :single_appearance_row, on: :create
 
+  default_value_for :message_background_color, '#E75E40'
+  default_value_for :message_font_color, '#FFFFFF'
+
   mount_uploader :logo,         AttachmentUploader
   mount_uploader :header_logo,  AttachmentUploader
   mount_uploader :favicon,      FaviconUploader
@@ -41,6 +48,14 @@ class Appearance < ActiveRecord::Base
     logo_system_path(favicon, 'favicon')
   end
 
+  def show_header?
+    header_message.present?
+  end
+
+  def show_footer?
+    footer_message.present?
+  end
+
   private
 
   def logo_system_path(logo, mount_type)
@@ -61,5 +76,3 @@ class Appearance < ActiveRecord::Base
     Gitlab::Utils.append_path(asset_host, local_path)
   end
 end
-
-Appearance.prepend(EE::Appearance)

ee/app/views/admin/appearances/_system_header_footer_form.html.haml → app/views/admin/appearances/_system_header_footer_form.html.haml

-- return unless License.feature_available?(:system_header_footer)
-
 - form = local_assigns.fetch(:form)
 
 %hr
@@ -16,7 +14,8 @@
       = form.label :footer_message, _('Footer message'), class: 'col-form-label label-bold'
       = form.text_area :footer_message, placeholder: _('State your message to activate'), class: "form-control js-autosize"
     .form-group.js-toggle-colors-container
-      = link_to _('Customize colors'), '#', class: 'js-toggle-colors-link'
+      %button.btn.btn-link.js-toggle-colors-link{ type: 'button' }
+        = _('Customize colors')
     .form-group.js-toggle-colors-container.hide
       = form.label :message_background_color, _('Background Color'), class: 'col-form-label label-bold'
       = form.color_field :message_background_color, class: "form-control"

changelogs/unreleased/55057-system-message-to-core.yml

+---
+title: Port System Header and Footer feature to Core
+merge_request: 25241
+author:
+type: added

ee/db/migrate/20180314145917_add_header_and_footer_banners_to_appearances_table.rb → db/migrate/20180314145917_add_header_and_footer_banners_to_appearances_table.rb

+# frozen_string_literal: true
+
 class AddHeaderAndFooterBannersToAppearancesTable < ActiveRecord::Migration[4.2]
   include Gitlab::Database::MigrationHelpers
 

doc/customization/system_header_and_footer_messages.md

@@ -16,6 +16,3 @@ After saving, all GitLab pages will contain the custom system header and/or foot
 The GitLab sign in page will also show the header and the footer messages:
 
 ![sign_up_custom_header_and_footer](system_header_and_footer_messages/sign_up_custom_header_and_footer.png)
-
-[eep]: https://about.gitlab.com/pricing/
-[ee-4972]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4972

ee/app/controllers/ee/admin/appearances_controller.rb

-# frozen_string_literal: true
-
-module EE
-  module Admin
-    module AppearancesController
-      def allowed_appearance_params
-        if License.feature_available?(:system_header_footer)
-          super + header_footer_params
-        else
-          super
-        end
-      end
-
-      private
-
-      def header_footer_params
-        %i[
-          header_message
-          footer_message
-          message_background_color
-          message_font_color
-        ]
-      end
-    end
-  end
-end

ee/app/helpers/ee/appearances_helper.rb

@@ -4,41 +4,9 @@ module EE
   module AppearancesHelper
     extend ::Gitlab::Utils::Override
 
-    def header_message
-      return unless current_appearance&.show_header?
-
-      class_names = []
-      class_names << 'with-performance-bar' if performance_bar_enabled?
-
-      render_message(:header_message, class_names)
-    end
-
-    def footer_message
-      return unless current_appearance&.show_footer?
-
-      render_message(:footer_message)
-    end
-
     override :default_brand_title
     def default_brand_title
       'GitLab Enterprise Edition'
     end
-
-    private
-
-    def render_message(field_sym, class_names = [])
-      class_names << field_sym.to_s.dasherize
-
-      content_tag :div, class: class_names, style: message_style do
-        markdown_field(current_appearance, field_sym)
-      end
-    end
-
-    def message_style
-      style = []
-      style << "background-color: #{current_appearance.message_background_color};"
-      style << "color: #{current_appearance.message_font_color}"
-      style.join
-    end
   end
 end

ee/app/helpers/ee/application_helper.rb

@@ -60,17 +60,6 @@ module EE
       class_names
     end
 
-    def system_message_class
-      class_names = []
-
-      return class_names unless appearance
-
-      class_names << 'with-system-header' if appearance.show_header?
-      class_names << 'with-system-footer' if appearance.show_footer?
-
-      class_names
-    end
-
     override :autocomplete_data_sources
     def autocomplete_data_sources(object, noteable_type)
       return {} unless object && noteable_type

ee/app/models/ee/appearance.rb

-# frozen_string_literal: true
-
-module EE
-  module Appearance
-    extend ActiveSupport::Concern
-
-    prepended do
-      cache_markdown_field :header_message, pipeline: :broadcast_message
-      cache_markdown_field :footer_message, pipeline: :broadcast_message
-
-      validates :message_background_color, allow_blank: true, color: true
-      validates :message_font_color, allow_blank: true, color: true
-
-      default_value_for :message_background_color, '#E75E40'
-      default_value_for :message_font_color, '#FFFFFF'
-    end
-
-    def show_header?
-      return unless ::License.feature_available?(:system_header_footer)
-
-      header_message.present?
-    end
-
-    def show_footer?
-      return unless ::License.feature_available?(:system_header_footer)
-
-      footer_message.present?
-    end
-  end
-end

ee/app/models/license.rb

@@ -68,7 +68,6 @@ class License < ActiveRecord::Base
     external_authorization_service
     ci_cd_projects
     protected_environments
-    system_header_footer
     custom_project_templates
     packages
     code_owner_as_approver_suggestion
@@ -175,7 +174,6 @@ class License < ActiveRecord::Base
     object_storage
     repository_size_limit
     external_authorization_service
-    system_header_footer
     custom_project_templates
   ].freeze
 

ee/spec/controllers/admin/appearances_controller_spec.rb → spec/controllers/admin/appearances_controller_spec.rb

+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe Admin::AppearancesController do
@@ -19,10 +21,7 @@ describe Admin::AppearancesController do
       sign_in(admin)
     end
 
-    context 'when system messages feature is available' do
     it 'creates appearance with footer and header message' do
-        stub_licensed_features(system_header_footer: true)
-
       post :create, params: { appearance: create_params }
 
       expect(Appearance.current).to have_attributes(
@@ -32,20 +31,6 @@ describe Admin::AppearancesController do
     end
   end
 
-    context 'when system messages feature is not available' do
-      it 'does not create appearance with footer and header message' do
-        stub_licensed_features(system_header_footer: false)
-
-        post :create, params: { appearance: create_params }
-
-        expect(Appearance.current).to have_attributes(
-          header_message: nil,
-          footer_message: nil
-        )
-      end
-    end
-  end
-
   describe 'PUT #update' do
     let(:update_params) do
       {
@@ -60,10 +45,7 @@ describe Admin::AppearancesController do
       sign_in(admin)
     end
 
-    context 'when system messages feature is available' do
     it 'updates appearance with footer and header message' do
-        stub_licensed_features(system_header_footer: true)
-
       put :update, params: { appearance: update_params }
 
       expect(Appearance.current).to have_attributes(
@@ -72,18 +54,4 @@ describe Admin::AppearancesController do
       )
     end
   end
-
-    context 'when system messages feature is not available' do
-      it 'does not update appearance with footer and header message' do
-        stub_licensed_features(system_header_footer: false)
-
-        post :create, params: { appearance: update_params }
-
-        expect(Appearance.current).to have_attributes(
-          header_message: nil,
-          footer_message: nil
-        )
-      end
-    end
-  end
 end

spec/features/admin/admin_appearance_spec.rb

@@ -41,26 +41,33 @@ describe 'Admin Appearance' do
 
   context 'Custom system header and footer' do
     before do
-      appearance.update(header_message: "Foo", footer_message: "Bar")
       sign_in(create(:admin))
     end
 
-    it 'shows custom system header and footer when licensed' do
-      stub_licensed_features(system_header_footer: true)
-
+    context 'when system header and footer messages are empty' do
+      it 'shows custom system header and footer fields' do
         visit admin_appearances_path
 
-      expect(page).to have_content appearance.header_message
-      expect(page).to have_content appearance.footer_message
+        expect(page).to have_field('appearance_header_message', with: '')
+        expect(page).to have_field('appearance_footer_message', with: '')
+        expect(page).to have_field('appearance_message_background_color')
+        expect(page).to have_field('appearance_message_font_color')
+      end
     end
 
-    it 'does not show custom system header and footer when unlicensed' do
-      stub_licensed_features(system_header_footer: false)
+    context 'when system header and footer messages are not empty' do
+      before do
+        appearance.update(header_message: 'Foo', footer_message: 'Bar')
+      end
 
+      it 'shows custom system header and footer fields' do
         visit admin_appearances_path
 
-      expect(page).not_to have_content appearance.header_message
-      expect(page).not_to have_content appearance.footer_message
+        expect(page).to have_field('appearance_header_message', with: appearance.header_message)
+        expect(page).to have_field('appearance_footer_message', with: appearance.footer_message)
+        expect(page).to have_field('appearance_message_background_color')
+        expect(page).to have_field('appearance_message_font_color')
+      end
     end
   end
 

ee/spec/features/display_system_header_and_footer_bar_spec.rb → spec/features/display_system_header_and_footer_bar_spec.rb

+# frozen_string_literal: true
+
 require 'rails_helper'
 
 describe 'Display system header and footer bar' do
@@ -57,12 +59,6 @@ describe 'Display system header and footer bar' do
         create(:appearance, header_message: header_message)
 
         sign_in(create(:user))
-      end
-
-      context 'when licensed' do
-        before do
-          stub_licensed_features(system_header_footer: true)
-
         visit root_path
       end
 
@@ -70,28 +66,11 @@ describe 'Display system header and footer bar' do
       it_behaves_like 'system footer is not configured'
     end
 
-      context 'when unlicensed' do
-        before do
-          stub_licensed_features(system_header_footer: false)
-
-          visit root_path
-        end
-
-        it_behaves_like 'system header is not configured'
-      end
-    end
-
     context 'when only system footer is defined' do
       before do
         create(:appearance, footer_message: footer_message)
 
         sign_in(create(:user))
-      end
-
-      context 'when licensed' do
-        before do
-          stub_licensed_features(system_header_footer: true)
-
         visit root_path
       end
 
@@ -99,46 +78,17 @@ describe 'Display system header and footer bar' do
       it_behaves_like 'system footer is configured'
     end
 
-      context 'when unlicensed' do
-        before do
-          stub_licensed_features(system_header_footer: false)
-
-          visit root_path
-        end
-
-        it_behaves_like 'system footer is not configured'
-      end
-    end
-
     context 'when system header and footer are defined' do
       before do
         create(:appearance, header_message: header_message, footer_message: footer_message)
 
         sign_in(create(:user))
-      end
-
-      context 'when licensed' do
-        before do
-          stub_licensed_features(system_header_footer: true)
-
         visit root_path
       end
 
       it_behaves_like 'system header is configured'
       it_behaves_like 'system footer is configured'
     end
-
-      context 'when unlicensed' do
-        before do
-          stub_licensed_features(system_header_footer: false)
-
-          visit root_path
-        end
-
-        it_behaves_like 'system header is not configured'
-        it_behaves_like 'system footer is not configured'
-      end
-    end
   end
 
   context 'when not authenticated' do
@@ -154,11 +104,6 @@ describe 'Display system header and footer bar' do
     context 'when only system header is defined' do
       before do
         create(:appearance, header_message: header_message)
-      end
-
-      context 'when licensed' do
-        before do
-          stub_licensed_features(system_header_footer: true)
 
         visit root_path
       end
@@ -167,25 +112,9 @@ describe 'Display system header and footer bar' do
       it_behaves_like 'system footer is not configured'
     end
 
-      context 'when unlicensed' do
-        before do
-          stub_licensed_features(system_header_footer: false)
-
-          visit root_path
-        end
-
-        it_behaves_like 'system header is not configured'
-      end
-    end
-
     context 'when only system footer is defined' do
       before do
         create(:appearance, footer_message: footer_message)
-      end
-
-      context 'when licensed' do
-        before do
-          stub_licensed_features(system_header_footer: true)
 
         visit root_path
       end
@@ -194,25 +123,9 @@ describe 'Display system header and footer bar' do
       it_behaves_like 'system footer is configured'
     end
 
-      context 'when unlicensed' do
-        before do
-          stub_licensed_features(system_header_footer: false)
-
-          visit root_path
-        end
-
-        it_behaves_like 'system footer is not configured'
-      end
-    end
-
     context 'when system header and footer are defined' do
       before do
         create(:appearance, header_message: header_message, footer_message: footer_message)
-      end
-
-      context 'when licensed' do
-        before do
-          stub_licensed_features(system_header_footer: true)
 
         visit root_path
       end
@@ -220,17 +133,5 @@ describe 'Display system header and footer bar' do
       it_behaves_like 'system header is configured'
       it_behaves_like 'system footer is configured'
     end
-
-      context 'when unlicensed' do
-        before do
-          stub_licensed_features(system_header_footer: false)
-
-          visit root_path
-        end
-
-        it_behaves_like 'system header is not configured'
-        it_behaves_like 'system footer is not configured'
-      end
-    end
   end
 end

ee/spec/helpers/ee/appearances_helper_spec.rb → spec/helpers/appearances_helper_spec.rb

+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe AppearancesHelper do
@@ -14,20 +16,10 @@ describe AppearancesHelper do
     end
 
     context 'when header message is set' do
-      it 'returns nil when unlicensed' do
-        create(:appearance, header_message: "Foo bar")
-
-        stub_licensed_features(system_header_footer: false)
-
-        expect(helper.header_message).to be_nil
-      end
-
-      it 'includes current message when licensed' do
+      it 'includes current message' do
         message = "Foo bar"
         create(:appearance, header_message: message)
 
-        stub_licensed_features(system_header_footer: true)
-
         expect(helper.header_message).to include(message)
       end
     end
@@ -41,20 +33,10 @@ describe AppearancesHelper do
     end
 
     context 'when footer message is set' do
-      it 'returns nil when unlicensed' do
-        create(:appearance, footer_message: "Foo bar")
-
-        stub_licensed_features(system_header_footer: false)
-
-        expect(helper.footer_message).to be_nil
-      end
-
-      it 'includes current message when licensed' do
+      it 'includes current message' do
         message = "Foo bar"
         create(:appearance, footer_message: message)
 
-        stub_licensed_features(system_header_footer: true)
-
         expect(helper.footer_message).to include(message)
       end
     end
@@ -83,7 +65,7 @@ describe AppearancesHelper do
   end
 
   describe '#brand_title' do
-    it 'returns the default EE title when no appearance is present' do
+    it 'returns the default CE title when no appearance is present' do
       allow(helper)
         .to receive(:current_appearance)
               .and_return(nil)

spec/models/appearance_spec.rb

@@ -63,4 +63,19 @@ describe Appearance do
   %i(logo header_logo favicon).each do |logo_type|
     it_behaves_like 'logo paths', logo_type
   end
+
+  describe 'validations' do
+    let(:triplet) { '#000' }
+    let(:hex)     { '#AABBCC' }
+
+    it { is_expected.to allow_value(nil).for(:message_background_color) }
+    it { is_expected.to allow_value(triplet).for(:message_background_color) }
+    it { is_expected.to allow_value(hex).for(:message_background_color) }
+    it { is_expected.not_to allow_value('000').for(:message_background_color) }
+
+    it { is_expected.to allow_value(nil).for(:message_font_color) }
+    it { is_expected.to allow_value(triplet).for(:message_font_color) }
+    it { is_expected.to allow_value(hex).for(:message_font_color) }
+    it { is_expected.not_to allow_value('000').for(:message_font_color) }
+  end
 end