Add structured markup for users

This commit introduces the the Schema.org structured
markup in the user's page.

app/helpers/page_layout_helper.rb

@@ -150,6 +150,14 @@ module PageLayoutHelper
     css_class.join(' ')
   end
 
+  def page_itemtype(itemtype = nil)
+    if itemtype
+      @page_itemtype = { itemscope: true, itemtype: itemtype }
+    else
+      @page_itemtype || {}
+    end
+  end
+
   private
 
   def generic_canonical_url

app/helpers/users_helper.rb

@@ -91,18 +91,18 @@ module UsersHelper
     end
   end
 
-  def work_information(user)
+  def work_information(user, with_schema_markup: false)
     return unless user
 
     organization = user.organization
     job_title = user.job_title
 
     if organization.present? && job_title.present?
-      s_('Profile|%{job_title} at %{organization}') % { job_title: job_title, organization: organization }
+      render_job_title_and_organization(job_title, organization, with_schema_markup: with_schema_markup)
     elsif job_title.present?
-      job_title
+      render_job_title(job_title, with_schema_markup: with_schema_markup)
     elsif organization.present?
-      organization
+      render_organization(organization, with_schema_markup: with_schema_markup)
     end
   end
 
@@ -151,6 +151,35 @@ module UsersHelper
 
     items
   end
+
+  def render_job_title(job_title, with_schema_markup: false)
+    if with_schema_markup
+      content_tag :span, itemprop: 'jobTitle' do
+        job_title
+      end
+    else
+      job_title
+    end
+  end
+
+  def render_organization(organization, with_schema_markup: false)
+    if with_schema_markup
+      content_tag :span, itemprop: 'worksFor' do
+        organization
+      end
+    else
+      organization
+    end
+  end
+
+  def render_job_title_and_organization(job_title, organization, with_schema_markup: false)
+    if with_schema_markup
+      job_title = '<span itemprop="jobTitle">'.html_safe + job_title + "</span>".html_safe
+      organization = '<span itemprop="worksFor">'.html_safe + organization + "</span>".html_safe
+    end
+
+    html_escape(s_('Profile|%{job_title} at %{organization}')) % { job_title: job_title, organization: organization }
+  end
 end
 
 UsersHelper.prepend_if_ee('EE::UsersHelper')

app/views/layouts/_page.html.haml

@@ -20,6 +20,6 @@
       - unless @hide_breadcrumbs
         = render "layouts/nav/breadcrumbs"
     %div{ class: "#{(container_class unless @no_container)} #{@content_class}" }
-      .content{ id: "content-body" }
+      .content{ id: "content-body", **page_itemtype }
         = render "layouts/flash", extra_flash_class: 'limit-container-width'
         = yield

app/views/users/show.html.haml

@@ -4,6 +4,7 @@
 - page_title       @user.blocked? ? s_('UserProfile|Blocked user') : @user.name
 - page_description @user.bio_html
 - header_title     @user.name, user_path(@user)
+- page_itemtype    'http://schema.org/Person'
 - link_classes = "flex-grow-1 mx-1 "
 
 = content_for :meta_tags do
@@ -35,7 +36,7 @@
     .profile-header{ class: [('with-no-profile-tabs' if profile_tabs.empty?)] }
       .avatar-holder
         = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do
-          = image_tag avatar_icon_for_user(@user, 90), class: "avatar s90", alt: ''
+          = image_tag avatar_icon_for_user(@user, 90), class: "avatar s90", alt: '', itemprop: 'image'
 
       - if @user.blocked?
         .user-info
@@ -44,7 +45,7 @@
         = render "users/profile_basic_info"
       - else
         .user-info
-          .cover-title
+          .cover-title{ itemprop: 'name' }
             = @user.name
 
           - if @user.status
@@ -54,15 +55,15 @@
           = render "users/profile_basic_info"
           .cover-desc.cgray.mb-1.mb-sm-2
             - unless @user.location.blank?
-              .profile-link-holder.middle-dot-divider-sm.d-block.d-sm-inline.mb-1.mb-sm-0
+              .profile-link-holder.middle-dot-divider-sm.d-block.d-sm-inline.mb-1.mb-sm-0{ itemprop: 'address', itemscope: true, itemtype: 'https://schema.org/PostalAddress' }
                 = sprite_icon('location', css_class: 'vertical-align-sub fgray')
-                %span.vertical-align-middle
+                %span.vertical-align-middle{ itemprop: 'addressLocality' }
                   = @user.location
             - unless work_information(@user).blank?
               .profile-link-holder.middle-dot-divider-sm.d-block.d-sm-inline
                 = sprite_icon('work', css_class: 'vertical-align-middle fgray')
                 %span.vertical-align-middle
-                  = work_information(@user)
+                  = work_information(@user, with_schema_markup: true)
           .cover-desc.cgray.mb-1.mb-sm-2
             - unless @user.skype.blank?
               .profile-link-holder.middle-dot-divider
@@ -80,10 +81,10 @@
               .profile-link-holder.middle-dot-divider-sm.d-block.d-sm-inline.mt-1.mt-sm-0
                 - if Feature.enabled?(:security_auto_fix) && @user.bot?
                   = sprite_icon('question', css_class: 'gl-text-blue-600')
-                = link_to @user.short_website_url, @user.full_website_url, class: 'text-link', target: '_blank', rel: 'me noopener noreferrer nofollow'
+                = link_to @user.short_website_url, @user.full_website_url, class: 'text-link', target: '_blank', rel: 'me noopener noreferrer nofollow', itemprop: 'url'
             - unless @user.public_email.blank?
               .profile-link-holder.middle-dot-divider-sm.d-block.d-sm-inline.mt-1.mt-sm-0
-                = link_to @user.public_email, "mailto:#{@user.public_email}", class: 'text-link'
+                = link_to @user.public_email, "mailto:#{@user.public_email}", class: 'text-link', itemprop: 'email'
         - if @user.bio.present?
           .cover-desc.cgray
             .profile-user-bio

changelogs/unreleased/273750-fj-structured-data-for-users.yml

+---
+title: Add structured markup for users
+merge_request: 46553
+author:
+type: added

spec/features/users/show_spec.rb

@@ -5,11 +5,13 @@ require 'spec_helper'
 RSpec.describe 'User page' do
   include ExternalAuthorizationServiceHelpers
 
-  let(:user) { create(:user, bio: '**Lorem** _ipsum_ dolor sit [amet](https://example.com)') }
+  let_it_be(:user) { create(:user, bio: '**Lorem** _ipsum_ dolor sit [amet](https://example.com)') }
+
+  subject { visit(user_path(user)) }
 
   context 'with public profile' do
     it 'shows all the tabs' do
-      visit(user_path(user))
+      subject
 
       page.within '.nav-links' do
         expect(page).to have_link('Overview')
@@ -22,14 +24,12 @@ RSpec.describe 'User page' do
     end
 
     it 'does not show private profile message' do
-      visit(user_path(user))
+      subject
 
       expect(page).not_to have_content("This user has a private profile")
     end
 
     context 'work information' do
-      subject { visit(user_path(user)) }
-
       it 'shows job title and organization details' do
         user.update(organization: 'GitLab - work info test', job_title: 'Frontend Engineer')
 
@@ -57,24 +57,24 @@ RSpec.describe 'User page' do
   end
 
   context 'with private profile' do
-    let(:user) { create(:user, private_profile: true) }
+    let_it_be(:user) { create(:user, private_profile: true) }
 
     it 'shows no tab' do
-      visit(user_path(user))
+      subject
 
       expect(page).to have_css("div.profile-header")
       expect(page).not_to have_css("ul.nav-links")
     end
 
     it 'shows private profile message' do
-      visit(user_path(user))
+      subject
 
       expect(page).to have_content("This user has a private profile")
     end
 
     it 'shows own tabs' do
       sign_in(user)
-      visit(user_path(user))
+      subject
 
       page.within '.nav-links' do
         expect(page).to have_link('Overview')
@@ -88,36 +88,36 @@ RSpec.describe 'User page' do
   end
 
   context 'with blocked profile' do
-    let(:user) { create(:user, state: :blocked) }
+    let_it_be(:user) { create(:user, state: :blocked) }
 
     it 'shows no tab' do
-      visit(user_path(user))
+      subject
 
       expect(page).to have_css("div.profile-header")
       expect(page).not_to have_css("ul.nav-links")
     end
 
     it 'shows blocked message' do
-      visit(user_path(user))
+      subject
 
       expect(page).to have_content("This user is blocked")
     end
 
     it 'shows user name as blocked' do
-      visit(user_path(user))
+      subject
 
       expect(page).to have_css(".cover-title", text: 'Blocked user')
     end
 
     it 'shows no additional fields' do
-      visit(user_path(user))
+      subject
 
       expect(page).not_to have_css(".profile-user-bio")
       expect(page).not_to have_css(".profile-link-holder")
     end
 
     it 'shows username' do
-      visit(user_path(user))
+      subject
 
       expect(page).to have_content("@#{user.username}")
     end
@@ -126,7 +126,7 @@ RSpec.describe 'User page' do
   it 'shows the status if there was one' do
     create(:user_status, user: user, message: "Working hard!")
 
-    visit(user_path(user))
+    subject
 
     expect(page).to have_content("Working hard!")
   end
@@ -135,7 +135,7 @@ RSpec.describe 'User page' do
     it 'shows the sign in link' do
       stub_application_setting(signup_enabled: false)
 
-      visit(user_path(user))
+      subject
 
       page.within '.navbar-nav' do
         expect(page).to have_link('Sign in')
@@ -147,7 +147,7 @@ RSpec.describe 'User page' do
     it 'shows the sign in and register link' do
       stub_application_setting(signup_enabled: true)
 
-      visit(user_path(user))
+      subject
 
       page.within '.navbar-nav' do
         expect(page).to have_link('Sign in / Register')
@@ -157,7 +157,7 @@ RSpec.describe 'User page' do
 
   context 'most recent activity' do
     it 'shows the most recent activity' do
-      visit(user_path(user))
+      subject
 
       expect(page).to have_content('Most Recent Activity')
     end
@@ -168,7 +168,7 @@ RSpec.describe 'User page' do
       end
 
       it 'hides the most recent activity' do
-        visit(user_path(user))
+        subject
 
         expect(page).not_to have_content('Most Recent Activity')
       end
@@ -177,14 +177,14 @@ RSpec.describe 'User page' do
 
   context 'page description' do
     before do
-      visit(user_path(user))
+      subject
     end
 
     it_behaves_like 'page meta description', 'Lorem ipsum dolor sit amet'
   end
 
   context 'with a bot user' do
-    let(:user) { create(:user, user_type: :security_bot) }
+    let_it_be(:user) { create(:user, user_type: :security_bot) }
 
     describe 'feature flag enabled' do
       before do
@@ -192,7 +192,7 @@ RSpec.describe 'User page' do
       end
 
       it 'only shows Overview and Activity tabs' do
-        visit(user_path(user))
+        subject
 
         page.within '.nav-links' do
           expect(page).to have_link('Overview')
@@ -211,7 +211,7 @@ RSpec.describe 'User page' do
       end
 
       it 'only shows Overview and Activity tabs' do
-        visit(user_path(user))
+        subject
 
         page.within '.nav-links' do
           expect(page).to have_link('Overview')
@@ -224,4 +224,24 @@ RSpec.describe 'User page' do
       end
     end
   end
+
+  context 'structured markup' do
+    let_it_be(:user) { create(:user, website_url: 'https://gitlab.com', organization: 'GitLab', job_title: 'Frontend Engineer', email: 'public@example.com', public_email: 'public@example.com', location: 'Country', created_at: Time.now, updated_at: Time.now) }
+
+    it 'shows Person structured markup' do
+      subject
+
+      aggregate_failures do
+        expect(page).to have_selector('[itemscope][itemtype="http://schema.org/Person"]')
+        expect(page).to have_selector('img[itemprop="image"]')
+        expect(page).to have_selector('[itemprop="name"]')
+        expect(page).to have_selector('[itemprop="address"][itemscope][itemtype="https://schema.org/PostalAddress"]')
+        expect(page).to have_selector('[itemprop="addressLocality"]')
+        expect(page).to have_selector('[itemprop="url"]')
+        expect(page).to have_selector('[itemprop="email"]')
+        expect(page).to have_selector('span[itemprop="jobTitle"]')
+        expect(page).to have_selector('span[itemprop="worksFor"]')
+      end
+    end
+  end
 end

spec/helpers/page_layout_helper_spec.rb

@@ -208,4 +208,27 @@ RSpec.describe PageLayoutHelper do
       end
     end
   end
+
+  describe '#page_itemtype' do
+    subject { helper.page_itemtype(itemtype) }
+
+    context 'when itemtype is passed' do
+      let(:itemtype) { 'http://schema.org/Person' }
+
+      it 'stores and returns the itemtype value' do
+        attrs = { itemscope: true, itemtype: itemtype }
+
+        expect(subject).to eq attrs
+        expect(helper.page_itemtype(nil)).to eq attrs
+      end
+    end
+
+    context 'when no itemtype is provided' do
+      let(:itemtype) { nil }
+
+      it 'returns an empty hash' do
+        expect(subject).to eq({})
+      end
+    end
+  end
 end

spec/helpers/users_helper_spec.rb

@@ -204,40 +204,72 @@ RSpec.describe UsersHelper do
   end
 
   describe '#work_information' do
-    subject { helper.work_information(user) }
+    let(:with_schema_markup) { false }
 
-    context 'when both job_title and organization are present' do
-      let(:user) { build(:user, organization: 'GitLab', job_title: 'Frontend Engineer') }
+    subject { helper.work_information(user, with_schema_markup: with_schema_markup) }
 
-      it 'returns job title concatenated with organization' do
-        is_expected.to eq('Frontend Engineer at GitLab')
-      end
+    context 'when neither organization nor job_title are present' do
+      it { is_expected.to be_nil }
     end
 
-    context 'when only organization is present' do
-      let(:user) { build(:user, organization: 'GitLab') }
+    context 'when user parameter is nil' do
+      let(:user) { nil }
 
-      it "returns organization" do
-        is_expected.to eq('GitLab')
-      end
+      it { is_expected.to be_nil }
     end
 
-    context 'when only job_title is present' do
-      let(:user) { build(:user, job_title: 'Frontend Engineer') }
+    context 'without schema markup' do
+      context 'when both job_title and organization are present' do
+        let(:user) { build(:user, organization: 'GitLab', job_title: 'Frontend Engineer') }
 
-      it 'returns job title' do
-        is_expected.to eq('Frontend Engineer')
+        it 'returns job title concatenated with organization' do
+          is_expected.to eq('Frontend Engineer at GitLab')
+        end
       end
-    end
 
-    context 'when neither organization nor job_title are present' do
-      it { is_expected.to be_nil }
+      context 'when only organization is present' do
+        let(:user) { build(:user, organization: 'GitLab') }
+
+        it "returns organization" do
+          is_expected.to eq('GitLab')
+        end
+      end
+
+      context 'when only job_title is present' do
+        let(:user) { build(:user, job_title: 'Frontend Engineer') }
+
+        it 'returns job title' do
+          is_expected.to eq('Frontend Engineer')
+        end
+      end
     end
 
-    context 'when user parameter is nil' do
-      let(:user) { nil }
+    context 'with schema markup' do
+      let(:with_schema_markup) { true }
 
-      it { is_expected.to be_nil }
+      context 'when both job_title and organization are present' do
+        let(:user) { build(:user, organization: 'GitLab', job_title: 'Frontend Engineer') }
+
+        it 'returns job title concatenated with organization' do
+          is_expected.to eq('<span itemprop="jobTitle">Frontend Engineer</span> at <span itemprop="worksFor">GitLab</span>')
+        end
+      end
+
+      context 'when only organization is present' do
+        let(:user) { build(:user, organization: 'GitLab') }
+
+        it "returns organization" do
+          is_expected.to eq('<span itemprop="worksFor">GitLab</span>')
+        end
+      end
+
+      context 'when only job_title is present' do
+        let(:user) { build(:user, job_title: 'Frontend Engineer') }
+
+        it 'returns job title' do
+          is_expected.to eq('<span itemprop="jobTitle">Frontend Engineer</span>')
+        end
+      end
     end
   end
 end