Merge branch 'refactoring-around-appearance' into 'master'

Refactoring around appearance

This was cherry picked instead of manual copy-pasting.

See merge request !230

app/controllers/admin/appearances_controller.rb

@@ -26,20 +26,18 @@ class Admin::AppearancesController < Admin::ApplicationController
   end
 
   def logo
-    appearance = Appearance.last
-    appearance.remove_logo!
+    @appearance.remove_logo!
 
-    appearance.save
+    @appearance.save
 
     redirect_to admin_appearances_path, notice: 'Logo was succesfully removed.'
   end
 
   def header_logos
-    appearance = Appearance.last
-    appearance.remove_light_logo!
-    appearance.save
+    @appearance.remove_header_logo!
+    @appearance.save
 
-    redirect_to admin_appearances_path, notice: 'Header logo were succesfully removed.'
+    redirect_to admin_appearances_path, notice: 'Header logo was succesfully removed.'
   end
 
   private
@@ -52,7 +50,7 @@ class Admin::AppearancesController < Admin::ApplicationController
   # Only allow a trusted parameter "white list" through.
   def appearance_params
     params.require(:appearance).permit(
-      :title, :description, :logo, :logo_cache, :light_logo, :light_logo_cache,
+      :title, :description, :logo, :logo_cache, :header_logo, :header_logo_cache,
       :updated_by
     )
   end

app/controllers/uploads_controller.rb

@@ -63,7 +63,7 @@ class UploadsController < ApplicationController
   end
 
   def upload_mount
-    upload_mounts = %w(avatar attachment file logo light_logo)
+    upload_mounts = %w(avatar attachment file logo header_logo)
 
     if upload_mounts.include?(params[:mounted_as])
       params[:mounted_as]

app/helpers/appearances_helper.rb

@@ -24,8 +24,8 @@ module AppearancesHelper
   end
 
   def brand_header_logo
-    if brand_item && brand_item.light_logo?
-      image_tag brand_item.light_logo
+    if brand_item && brand_item.header_logo?
+      image_tag brand_item.header_logo
     else
       render 'shared/logo.svg'
     end

app/models/appearance.rb

@@ -9,16 +9,15 @@
 #  updated_by  :integer
 #  created_at  :datetime
 #  updated_at  :datetime
-#  dark_logo   :string(255)
-#  light_logo  :string(255)
+#  header_logo  :string(255)
 #
 
 class Appearance < ActiveRecord::Base
   validates :title,       presence: true
   validates :description, presence: true
-  validates :logo, file_size: { maximum: 1000.kilobytes.to_i }
-  validates :light_logo, file_size: { maximum: 1000.kilobytes.to_i }
+  validates :logo,        file_size: { maximum: 1.megabyte }
+  validates :header_logo, file_size: { maximum: 1.megabyte }
 
   mount_uploader :logo,         AttachmentUploader
-  mount_uploader :light_logo, AttachmentUploader
+  mount_uploader :header_logo,  AttachmentUploader
 end

app/views/admin/appearances/_form.html.haml

@@ -29,24 +29,24 @@
       = f.hidden_field :logo_cache
       = f.file_field :logo, class: ""
       .hint
-        Maximum logo size is 1MB, page optimized for logo size 640x360px
+        Maximum file size is 1MB. Pages are optimized for a 640x360 px logo.
 
   %fieldset.app_logo
     %legend
       Navigation bar:
   .form-group
-    = f.label :light_logo, 'Header logo', class: 'control-label'
+    = f.label :header_logo, 'Header logo', class: 'control-label'
     .col-sm-10
-      - if @appearance.light_logo?
-        = image_tag @appearance.light_logo_url, class: 'appearance-light-logo-preview'
+      - if @appearance.header_logo?
+        = image_tag @appearance.header_logo_url, class: 'appearance-light-logo-preview'
         - if @appearance.persisted?
           %br
           = link_to 'Remove header logo', header_logos_admin_appearances_path, data: { confirm: "Header logo will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-small remove-logo"
         %hr
-      = f.hidden_field :light_logo_cache
-      = f.file_field :light_logo, class: ""
+      = f.hidden_field :header_logo_cache
+      = f.file_field :header_logo, class: ""
       .hint
-        Maximum size is 1MB, page optimized for logo size 72x72px
+        Maximum file size is 1MB. Pages are optimized for a 72x72 px header logo
 
   .form-actions
     = f.submit 'Save', class: 'btn btn-save'

app/views/layouts/nav/_admin.html.haml

@@ -62,8 +62,8 @@
       %span
         Background Jobs
   = nav_link(controller: :appearances) do
-    = link_to admin_appearances_path do
-      %i.fa.fa-image
+    = link_to admin_appearances_path, title: 'Appearances' do
+      = icon('image')
       %span
         Appearance
 

config/routes.rb

@@ -159,7 +159,7 @@ Rails.application.routes.draw do
     # Appearance
     get ":model/:mounted_as/:id/:filename",
         to:           "uploads#show",
-        constraints:  { model: /appearance/, mounted_as: /logo|dark_logo|light_logo/, filename: /.+/ }
+        constraints:  { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ }
 
     # Project markdown uploads
     get ":namespace_id/:project_id/:secret/:filename",

db/migrate/20160222153918_create_appearances_ce.rb

+class CreateAppearancesCe < ActiveRecord::Migration
+  def change
+    unless table_exists?(:appearances)
+      create_table :appearances do |t|
+        t.string :title
+        t.text :description
+        t.string :header_logo
+        t.string :logo
+
+        t.timestamps null: false
+      end
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160220123949) do
+ActiveRecord::Schema.define(version: 20160222153918) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -27,12 +27,10 @@ ActiveRecord::Schema.define(version: 20160220123949) do
   create_table "appearances", force: :cascade do |t|
     t.string   "title"
     t.text     "description"
+    t.string   "header_logo"
     t.string   "logo"
-    t.integer  "updated_by"
-    t.datetime "created_at"
-    t.datetime "updated_at"
-    t.string   "dark_logo"
-    t.string   "light_logo"
+    t.datetime "created_at",  null: false
+    t.datetime "updated_at",  null: false
   end
 
   create_table "application_settings", force: :cascade do |t|

doc/customization/branded_login_page.md

 # Changing the appearance of the login page
 
-GitLab Enterprise Edition offers a way to put your company recognizible identity on the login page of your GitLab server and make it a branded login page.
+GitLab Enterprise Edition offers a way to put your company's identity on the login page of your GitLab server and make it a branded login page.
 
-By default, Enterprise Edition page shows GitLab logo and description
+By default, the page shows the GitLab logo and description.
 
 ![default_login_page](branded_login_page/default_login_page.png)
 
 ## Changing the appearance of the login page
 
-Navigate to the ![admin area](branded_login_page/admin_area.png) and go to the Appearance page.
+Navigate to the **Admin** area and go to the **Appearance** page.
 
 Fill in the required details like Title, Description and upload the company logo.
 
@@ -16,4 +16,4 @@ Fill in the required details like Title, Description and upload the company logo
 
 After saving the page, your GitLab login page will have the details you filled in:
 
-![company_login_page](branded_login_page/company_login_page.png)
+![company_login_page](branded_login_page/custom_sign_in.png)
\ No newline at end of file

doc/customization/welcome_message.md

-# Customize the complete sign-in page (GitLab Enterprise Edition only)
+# Customize the complete sign-in page
 
-Please see [Branded login page](http://doc.gitlab.com/ee/customization/branded_login_page.html)
+Please see [Branded login page](branded_login_page.md)
 
 # Add a welcome message to the sign-in page (GitLab Community Edition)
 
 It is possible to add a markdown-formatted welcome message to your GitLab
 sign-in page. Users of GitLab Enterprise Edition should use the [branded login
-page feature](/ee/customization/branded_login_page.html) instead.
+page feature](branded_login_page.md) instead.
 
 The welcome message (extra_sign_in_text) can now be set/changed in the Admin UI.  
 Admin area > Settings

features/steps/admin/appearance.rb

@@ -38,7 +38,7 @@ class Spinach::Features::AdminAppearance < Spinach::FeatureSteps
   end
 
   step 'I attach header logos' do
-    attach_file(:appearance_light_logo, Rails.root.join('spec', 'fixtures', 'dk.png'))
+    attach_file(:appearance_header_logo, Rails.root.join('spec', 'fixtures', 'dk.png'))
     click_button 'Save'
   end
 
@@ -47,7 +47,7 @@ class Spinach::Features::AdminAppearance < Spinach::FeatureSteps
   end
 
   step 'I should see header logos' do
-    expect(page).to have_xpath('//img[@src="/uploads/appearance/light_logo/1/dk.png"]')
+    expect(page).to have_xpath('//img[@src="/uploads/appearance/header_logo/1/dk.png"]')
   end
 
   step 'I remove the logo' do
@@ -63,7 +63,7 @@ class Spinach::Features::AdminAppearance < Spinach::FeatureSteps
   end
 
   step 'I should see header logos removed' do
-    expect(page).not_to have_xpath('//img[@src="/uploads/appearance/light_logo/1/header_logo_light.png"]')
+    expect(page).not_to have_xpath('//img[@src="/uploads/appearance/header_logo/1/header_logo_light.png"]')
   end
 
   def appearance

spec/models/appearance_spec.rb

-require 'spec_helper'
+require 'rails_helper'
 
-describe Appearance do
+RSpec.describe Appearance, type: :model do
   subject { create(:appearance) }
 
-  it { should be_valid }
+  it { is_expected.to be_valid }
+
+  it { is_expected.to validate_presence_of(:title) }
+  it { is_expected.to validate_presence_of(:description) }
 end