Merge branch 'ee-55057-system-message-to-core' into 'master'

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

See merge request gitlab-org/gitlab-ee!9556

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 {
@@ -82,11 +82,11 @@
   // right sidebar eg: mr page
   .nav-sidebar,
   .right-sidebar,
-  // navless pages' footer eg: login page
-  // navless pages' footer border eg: login page
+    // navless pages' footer eg: login page
+    // 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
@@ -174,7 +173,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,30 +21,13 @@ 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(
-          header_message: header_message,
-          footer_message: footer_message
-        )
-      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 }
+    it 'creates appearance with footer and header message' do
+      post :create, params: { appearance: create_params }
 
-        expect(Appearance.current).to have_attributes(
-          header_message: nil,
-          footer_message: nil
-        )
-      end
+      expect(Appearance.current).to have_attributes(
+        header_message: header_message,
+        footer_message: footer_message
+      )
     end
   end
 
@@ -60,30 +45,13 @@ 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)
+    it 'updates appearance with footer and header message' do
+      put :update, params: { appearance: update_params }
 
-        put :update, params: { appearance: update_params }
-
-        expect(Appearance.current).to have_attributes(
-          header_message: header_message,
-          footer_message: footer_message
-        )
-      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
+      expect(Appearance.current).to have_attributes(
+        header_message: header_message,
+        footer_message: footer_message
+      )
     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
 
-      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
 
-      visit admin_appearances_path
+      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,28 +59,11 @@ describe 'Display system header and footer bar' do
         create(:appearance, header_message: header_message)
 
         sign_in(create(:user))
+        visit root_path
       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 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
+      it_behaves_like 'system header is configured'
+      it_behaves_like 'system footer is not configured'
     end
 
     context 'when only system footer is defined' do
@@ -86,28 +71,11 @@ describe 'Display system header and footer bar' do
         create(:appearance, footer_message: footer_message)
 
         sign_in(create(:user))
+        visit root_path
       end
 
-      context 'when licensed' do
-        before do
-          stub_licensed_features(system_header_footer: true)
-
-          visit root_path
-        end
-
-        it_behaves_like 'system header is not 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 footer is not configured'
-      end
+      it_behaves_like 'system header is not configured'
+      it_behaves_like 'system footer is configured'
     end
 
     context 'when system header and footer are defined' do
@@ -115,29 +83,11 @@ describe 'Display system header and footer bar' do
         create(:appearance, header_message: header_message, footer_message: footer_message)
 
         sign_in(create(:user))
+        visit root_path
       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
+      it_behaves_like 'system header is configured'
+      it_behaves_like 'system footer is configured'
     end
   end
 
@@ -154,83 +104,34 @@ 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
-
-        it_behaves_like 'system header is configured'
-        it_behaves_like 'system footer is not configured'
+        visit root_path
       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
+      it_behaves_like 'system header is configured'
+      it_behaves_like 'system footer is not configured'
     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
 
-        it_behaves_like 'system header is not configured'
-        it_behaves_like 'system footer is configured'
+        visit root_path
       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
+      it_behaves_like 'system header is not configured'
+      it_behaves_like 'system footer is configured'
     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
-
-        it_behaves_like 'system header is configured'
-        it_behaves_like 'system footer is configured'
+        visit root_path
       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
+      it_behaves_like 'system header is configured'
+      it_behaves_like 'system footer is configured'
     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,10 +65,10 @@ 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)
+              .and_return(nil)
 
       expect(helper.brand_title).to eq('GitLab Enterprise Edition')
     end

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