Merge branch 'more-opengraph' into 'master'

Add Open Graph data for group, project and commit.

See merge request !2211

app/helpers/page_layout_helper.rb

@@ -27,35 +27,20 @@ module PageLayoutHelper
   #
   # Returns an HTML-safe String.
   def page_description(description = nil)
-    @page_description ||= page_description_default
-
     if description.present?
       @page_description = description.squish
-    else
+    elsif @page_description.present?
       sanitize(@page_description, tags: []).truncate_words(30)
     end
   end
-  # Default value for page_description when one hasn't been defined manually by
-  # a view
-  def page_description_default
-    if @project
-      @project.description || brand_title
-    else
-      brand_title
-    end
-  end
-
   def page_image
     default = image_url('gitlab_logo.png')
-    if @project
-      @project.avatar_url || default
-    elsif @user
-      avatar_icon(@user)
-    else
-      default
-    end
+    subject = @project || @user || @group
+
+    image = subject.avatar_url if subject.present?
+    image || default
   end
   # Define or get attributes to be used as Twitter card metadata

app/views/layouts/_head.html.haml

+- page_description brand_title unless page_description
+
+- site_name = "GitLab"
 %head{prefix: "og: http://ogp.me/ns#"}
   %meta{charset: "utf-8"}
   %meta{'http-equiv' => 'X-UA-Compatible', content: 'IE=edge'}
   -# Open Graph - http://ogp.me/
   %meta{property: 'og:type',        content: "object"}
-  %meta{property: 'og:site_name',   content: "GitLab"}
+  %meta{property: 'og:site_name',   content: site_name}
   %meta{property: 'og:title',       content: page_title}
   %meta{property: 'og:description', content: page_description}
   %meta{property: 'og:image',       content: page_image}
@@ -17,7 +20,7 @@
   %meta{property: 'twitter:image',        content: page_image}
   = page_card_meta_tags
-  %title= page_title('GitLab')
+  %title= page_title(site_name)
   %meta{name: "description", content: page_description}
   = favicon_link_tag 'favicon.ico'

app/views/layouts/group.html.haml

 - page_title       @group.name
+- page_description @group.description  unless page_description
 - header_title     group_title(@group) unless header_title
 - sidebar          "group"             unless sidebar

app/views/layouts/project.html.haml

 - page_title       @project.name_with_namespace
+- page_description @project.description    unless page_description
 - header_title     project_title(@project) unless header_title
 - sidebar          "project"               unless sidebar

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

 - page_title        "#{@commit.title} (#{@commit.short_id})", "Commits"
+- page_description  @commit.description
+
 = render "projects/commits/header_title"
 = render "commit_box"
 - if @ci_commit

spec/helpers/page_layout_helper_spec.rb

@@ -2,10 +2,8 @@ require 'rails_helper'
 describe PageLayoutHelper do
   describe 'page_description' do
-    it 'defaults to value returned by page_description_default helper' do
-      allow(helper).to receive(:page_description_default).and_return('Foo')
-
-      expect(helper.page_description).to eq 'Foo'
+    it 'defaults to nil' do
+      expect(helper.page_description).to eq nil
     end
     it 'returns the last-pushed description' do
@@ -42,58 +40,32 @@ describe PageLayoutHelper do
     end
   end
-  describe 'page_description_default' do
-    it 'uses Project description when available' do
-      project = double(description: 'Project Description')
-      helper.instance_variable_set(:@project, project)
-
-      expect(helper.page_description_default).to eq 'Project Description'
-    end
-
-    it 'uses brand_title when Project description is nil' do
-      project = double(description: nil)
-      helper.instance_variable_set(:@project, project)
-
-      expect(helper).to receive(:brand_title).and_return('Brand Title')
-      expect(helper.page_description_default).to eq 'Brand Title'
-    end
-
-    it 'falls back to brand_title' do
-      allow(helper).to receive(:brand_title).and_return('Brand Title')
-
-      expect(helper.page_description_default).to eq 'Brand Title'
-    end
-  end
-
   describe 'page_image' do
     it 'defaults to the GitLab logo' do
       expect(helper.page_image).to end_with 'assets/gitlab_logo.png'
     end
-    context 'with @project' do
-      it 'uses Project avatar if available' do
-        project = double(avatar_url: 'http://example.com/uploads/avatar.png')
-        helper.instance_variable_set(:@project, project)
-        expect(helper.page_image).to eq project.avatar_url
+    %w(project user group).each do |type|
+      context "with @#{type} assigned" do
+        it "uses #{type.titlecase} avatar if available" do
+          object = double(avatar_url: 'http://example.com/uploads/avatar.png')
+          assign(type, object)
+          expect(helper.page_image).to eq object.avatar_url
         end
-      it 'falls back to the default' do
-        project = double(avatar_url: nil)
-        helper.instance_variable_set(:@project, project)
+        it 'falls back to the default when avatar_url is nil' do
+          object = double(avatar_url: nil)
+          assign(type, object)
           expect(helper.page_image).to end_with 'assets/gitlab_logo.png'
         end
       end
-    context 'with @user' do
-      it 'delegates to avatar_icon helper' do
-        user = double('User')
-        helper.instance_variable_set(:@user, user)
-
-        expect(helper).to receive(:avatar_icon).with(user)
-
-        helper.page_image
+      context "with no assignments" do
+        it 'falls back to the default' do
+          expect(helper.page_image).to end_with 'assets/gitlab_logo.png'
+        end
       end
     end
   end