Merge branch '273751-fj-structured-data-for-groups' into 'master'

Resolve "Provide structured data for groups"

See merge request gitlab-org/gitlab!47374

app/assets/javascripts/groups/components/group_item.vue

@@ -74,6 +74,9 @@ export default {
     visibilityTooltip() {
       return GROUP_VISIBILITY_TYPE[this.group.visibility];
     },
+    microdata() {
+      return this.group.microdata || {};
+    },
   },
   mounted() {
     if (this.group.name === 'Learn GitLab') {
@@ -99,7 +102,15 @@ export default {
 </script>
 
 <template>
-  <li :id="groupDomId" :class="rowClass" class="group-row" @click.stop="onClickRowGroup">
+  <li
+    :id="groupDomId"
+    :class="rowClass"
+    class="group-row"
+    :itemprop="microdata.itemprop"
+    :itemtype="microdata.itemtype"
+    :itemscope="microdata.itemscope"
+    @click.stop="onClickRowGroup"
+  >
     <div
       :class="{ 'project-row-contents': !isGroup }"
       class="group-row-contents d-flex align-items-center py-2 pr-3"
@@ -118,7 +129,13 @@ export default {
         class="avatar-container rect-avatar s32 d-none flex-grow-0 flex-shrink-0 "
       >
         <a :href="group.relativePath" class="no-expand">
-          <img v-if="hasAvatar" :src="group.avatarUrl" class="avatar s40" />
+          <img
+            v-if="hasAvatar"
+            :src="group.avatarUrl"
+            data-testid="group-avatar"
+            class="avatar s40"
+            :itemprop="microdata.imageItemprop"
+          />
           <identicon v-else :entity-id="group.id" :entity-name="group.name" size-class="s40" />
         </a>
       </div>
@@ -127,9 +144,11 @@ export default {
           <div class="d-flex align-items-center flex-wrap title namespace-title gl-mr-3">
             <a
               v-gl-tooltip.bottom
+              data-testid="group-name"
               :href="group.relativePath"
               :title="group.fullName"
               class="no-expand gl-mt-3 gl-mr-3 gl-text-gray-900!"
+              :itemprop="microdata.nameItemprop"
               >{{
                 // ending bracket must be by closing tag to prevent
                 // link hover text-decoration from over-extending
@@ -146,7 +165,12 @@ export default {
             </span>
           </div>
           <div v-if="group.description" class="description">
-            <span v-html="group.description"> </span>
+            <span
+              :itemprop="microdata.descriptionItemprop"
+              data-testid="group-description"
+              v-html="group.description"
+            >
+            </span>
           </div>
         </div>
         <div v-if="isGroupPendingRemoval">

app/assets/javascripts/groups/index.js

@@ -47,8 +47,9 @@ export default (containerId = 'js-groups-tree', endpoint, action = '') => {
     data() {
       const { dataset } = dataEl || this.$options.el;
       const hideProjects = parseBoolean(dataset.hideProjects);
+      const showSchemaMarkup = parseBoolean(dataset.showSchemaMarkup);
       const service = new GroupsService(endpoint || dataset.endpoint);
-      const store = new GroupsStore(hideProjects);
+      const store = new GroupsStore({ hideProjects, showSchemaMarkup });
 
       return {
         action,

app/assets/javascripts/groups/store/groups_store.js

 import { normalizeHeaders, parseIntPagination } from '../../lib/utils/common_utils';
+import { getGroupItemMicrodata } from './utils';
 
 export default class GroupsStore {
-  constructor(hideProjects) {
+  constructor({ hideProjects = false, showSchemaMarkup = false } = {}) {
     this.state = {};
     this.state.groups = [];
     this.state.pageInfo = {};
     this.hideProjects = hideProjects;
+    this.showSchemaMarkup = showSchemaMarkup;
   }
 
   setGroups(rawGroups) {
@@ -94,6 +96,7 @@ export default class GroupsStore {
       starCount: rawGroupItem.star_count,
       updatedAt: rawGroupItem.updated_at,
       pendingRemoval: rawGroupItem.marked_for_deletion,
+      microdata: this.showSchemaMarkup ? getGroupItemMicrodata(rawGroupItem) : {},
     };
   }
 

app/assets/javascripts/groups/store/utils.js

+export const getGroupItemMicrodata = ({ type }) => {
+  const defaultMicrodata = {
+    itemscope: true,
+    itemtype: 'https://schema.org/Thing',
+    itemprop: 'owns',
+    imageItemprop: 'image',
+    nameItemprop: 'name',
+    descriptionItemprop: 'description',
+  };
+
+  switch (type) {
+    case 'group':
+      return {
+        ...defaultMicrodata,
+        itemtype: 'https://schema.org/Organization',
+        itemprop: 'subOrganization',
+        imageItemprop: 'logo',
+      };
+    case 'project':
+      return {
+        ...defaultMicrodata,
+        itemtype: 'https://schema.org/SoftwareSourceCode',
+      };
+    default:
+      return defaultMicrodata;
+  }
+};

app/views/groups/_home_panel.html.haml

@@ -6,10 +6,10 @@
   .row.mb-3
     .home-panel-title-row.col-md-12.col-lg-6.d-flex
       .avatar-container.rect-avatar.s64.home-panel-avatar.gl-mr-3.float-none
-        = group_icon(@group, class: 'avatar avatar-tile s64', width: 64, height: 64)
+        = group_icon(@group, class: 'avatar avatar-tile s64', width: 64, height: 64, itemprop: 'logo')
       .d-flex.flex-column.flex-wrap.align-items-baseline
         .d-inline-flex.align-items-baseline
-          %h1.home-panel-title.gl-mt-3.gl-mb-2
+          %h1.home-panel-title.gl-mt-3.gl-mb-2{ itemprop: 'name' }
             = @group.name
             %span.visibility-icon.text-secondary.gl-ml-2.has-tooltip{ data: { container: 'body' }, title: visibility_icon_description(@group) }
               = visibility_level_icon(@group.visibility_level, options: {class: 'icon'})
@@ -34,7 +34,7 @@
   - if @group.description.present?
     .group-home-desc.mt-1
       .home-panel-description
-        .home-panel-description-markdown.read-more-container
+        .home-panel-description-markdown.read-more-container{ itemprop: 'description' }
           = markdown_field(@group, :description)
         %button.btn.btn-blank.btn-link.js-read-more-trigger.d-lg-none{ type: "button" }
           = _("Read more")

app/views/groups/_subgroups_and_projects.html.haml

@@ -3,6 +3,6 @@
     = render "shared/groups/empty_state"
 
   %section{ data: { hide_projects: 'false', group_id: group.id, path: group_path(group) } }
-    .js-groups-list-holder
+    .js-groups-list-holder{ data: { show_schema_markup: 'true'} }
       .loading-container.text-center.prepend-top-20
         .spinner.spinner-md

app/views/groups/show.html.haml

 - breadcrumb_title _("Details")
 - @content_class = "limit-container-width" unless fluid_layout
+- page_itemtype    'https://schema.org/Organization'
 
 - if show_thanks_for_purchase_banner?
   = render_if_exists 'shared/thanks_for_purchase_banner', plan_title: plan_title, quantity: params[:purchased_quantity].to_i

changelogs/unreleased/273751-fj-structured-data-for-groups.yml

+---
+title: Add SEO structured markup for groups
+merge_request: 47374
+author:
+type: added

spec/features/groups/show_spec.rb

@@ -193,4 +193,69 @@ RSpec.describe 'Group show page' do
 
     it_behaves_like 'page meta description', 'Lorem ipsum dolor sit amet'
   end
+
+  context 'structured schema markup' do
+    let_it_be(:group) { create(:group, :public, :with_avatar, description: 'foo') }
+    let_it_be(:subgroup) { create(:group, :public, :with_avatar, parent: group, description: 'bar') }
+    let_it_be_with_reload(:project) { create(:project, :public, :with_avatar, namespace: group, description: 'foo') }
+    let_it_be(:subproject) { create(:project, :public, :with_avatar, namespace: subgroup, description: 'bar') }
+
+    it 'shows Organization structured markup', :js do
+      visit path
+      wait_for_all_requests
+
+      aggregate_failures do
+        expect(page).to have_selector('.content[itemscope][itemtype="https://schema.org/Organization"]')
+
+        page.within('.group-home-panel') do
+          expect(page).to have_selector('img.avatar[itemprop="logo"]')
+          expect(page).to have_selector('[itemprop="name"]', text: group.name)
+          expect(page).to have_selector('[itemprop="description"]', text: group.description)
+        end
+
+        page.within('[itemprop="owns"][itemtype="https://schema.org/SoftwareSourceCode"]') do
+          expect(page).to have_selector('img.avatar[itemprop="image"]')
+          expect(page).to have_selector('[itemprop="name"]', text: project.name)
+          expect(page).to have_selector('[itemprop="description"]', text: project.description)
+        end
+
+        # Finding the subgroup row and expanding it
+        el = find('[itemprop="subOrganization"][itemtype="https://schema.org/Organization"]')
+        el.click
+        wait_for_all_requests
+        page.within(el) do
+          expect(page).to have_selector('img.avatar[itemprop="logo"]')
+          expect(page).to have_selector('[itemprop="name"]', text: subgroup.name)
+          expect(page).to have_selector('[itemprop="description"]', text: subgroup.description)
+
+          page.within('[itemprop="owns"][itemtype="https://schema.org/SoftwareSourceCode"]') do
+            expect(page).to have_selector('img.avatar[itemprop="image"]')
+            expect(page).to have_selector('[itemprop="name"]', text: subproject.name)
+            expect(page).to have_selector('[itemprop="description"]', text: subproject.description)
+          end
+        end
+      end
+    end
+
+    it 'does not include structured markup in shared projects tab', :js do
+      other_project = create(:project, :public)
+      other_project.project_group_links.create!(group: group)
+
+      visit group_shared_path(group)
+      wait_for_all_requests
+
+      expect(page).to have_selector('li.group-row')
+      expect(page).not_to have_selector('[itemprop="owns"][itemtype="https://schema.org/SoftwareSourceCode"]')
+    end
+
+    it 'does not include structured markup in archived projects tab', :js do
+      project.update!(archived: true)
+
+      visit group_archived_path(group)
+      wait_for_all_requests
+
+      expect(page).to have_selector('li.group-row')
+      expect(page).not_to have_selector('[itemprop="owns"][itemtype="https://schema.org/SoftwareSourceCode"]')
+    end
+  end
 end

spec/frontend/groups/components/app_spec.js

@@ -35,7 +35,7 @@ describe('AppComponent', () => {
   let mock;
   let getGroupsSpy;
 
-  const store = new GroupsStore(false);
+  const store = new GroupsStore({ hideProjects: false });
   const service = new GroupsService(mockEndpoint);
 
   const createShallowComponent = (hideProjects = false) => {

spec/frontend/groups/components/group_item_spec.js

@@ -2,6 +2,7 @@ import Vue from 'vue';
 import mountComponent from 'helpers/vue_mount_component_helper';
 import groupItemComponent from '~/groups/components/group_item.vue';
 import groupFolderComponent from '~/groups/components/group_folder.vue';
+import { getGroupItemMicrodata } from '~/groups/store/utils';
 import eventHub from '~/groups/event_hub';
 import * as urlUtilities from '~/lib/utils/url_utility';
 import { mockParentGroupItem, mockChildren } from '../mock_data';
@@ -30,6 +31,11 @@ describe('GroupItemComponent', () => {
     vm.$destroy();
   });
 
+  const withMicrodata = group => ({
+    ...group,
+    microdata: getGroupItemMicrodata(group),
+  });
+
   describe('computed', () => {
     describe('groupDomId', () => {
       it('should return ID string suffixed with group ID', () => {
@@ -212,4 +218,47 @@ describe('GroupItemComponent', () => {
       expect(vm.$el.querySelector('.group-list-tree')).toBeDefined();
     });
   });
+  describe('schema.org props', () => {
+    describe('when showSchemaMarkup is disabled on the group', () => {
+      it.each(['itemprop', 'itemtype', 'itemscope'], 'it does not set %s', attr => {
+        expect(vm.$el.getAttribute(attr)).toBeNull();
+      });
+      it.each(
+        ['.js-group-avatar', '.js-group-name', '.js-group-description'],
+        'it does not set `itemprop` on sub-nodes',
+        selector => {
+          expect(vm.$el.querySelector(selector).getAttribute('itemprop')).toBeNull();
+        },
+      );
+    });
+    describe('when group has microdata', () => {
+      beforeEach(() => {
+        const group = withMicrodata({
+          ...mockParentGroupItem,
+          avatarUrl: 'http://foo.bar',
+          description: 'Foo Bar',
+        });
+
+        vm = createComponent(group);
+      });
+
+      it.each`
+        attr           | value
+        ${'itemscope'} | ${'itemscope'}
+        ${'itemtype'}  | ${'https://schema.org/Organization'}
+        ${'itemprop'}  | ${'subOrganization'}
+      `('it does set correct $attr', ({ attr, value } = {}) => {
+        expect(vm.$el.getAttribute(attr)).toBe(value);
+      });
+
+      it.each`
+        selector                               | propValue
+        ${'[data-testid="group-avatar"]'}      | ${'logo'}
+        ${'[data-testid="group-name"]'}        | ${'name'}
+        ${'[data-testid="group-description"]'} | ${'description'}
+      `('it does set correct $selector', ({ selector, propValue } = {}) => {
+        expect(vm.$el.querySelector(selector).getAttribute('itemprop')).toBe(propValue);
+      });
+    });
+  });
 });

spec/frontend/groups/store/groups_store_spec.js

 import GroupsStore from '~/groups/store/groups_store';
+import { getGroupItemMicrodata } from '~/groups/store/utils';
 import {
   mockGroups,
   mockSearchedGroups,
@@ -17,9 +18,9 @@ describe('ProjectsStore', () => {
       expect(Object.keys(store.state).length).toBe(2);
       expect(Array.isArray(store.state.groups)).toBeTruthy();
       expect(Object.keys(store.state.pageInfo).length).toBe(0);
-      expect(store.hideProjects).not.toBeDefined();
+      expect(store.hideProjects).toBeFalsy();
 
-      store = new GroupsStore(true);
+      store = new GroupsStore({ hideProjects: true });
 
       expect(store.hideProjects).toBeTruthy();
     });
@@ -86,22 +87,30 @@ describe('ProjectsStore', () => {
 
   describe('formatGroupItem', () => {
     it('should parse group item object and return updated object', () => {
-      let store;
-      let updatedGroupItem;
-
-      store = new GroupsStore();
-      updatedGroupItem = store.formatGroupItem(mockRawChildren[0]);
+      const store = new GroupsStore();
+      const updatedGroupItem = store.formatGroupItem(mockRawChildren[0]);
 
       expect(Object.keys(updatedGroupItem).indexOf('fullName')).toBeGreaterThan(-1);
       expect(updatedGroupItem.childrenCount).toBe(mockRawChildren[0].children_count);
       expect(updatedGroupItem.isChildrenLoading).toBe(false);
       expect(updatedGroupItem.isBeingRemoved).toBe(false);
+      expect(updatedGroupItem.microdata).toEqual({});
+    });
 
-      store = new GroupsStore(true);
-      updatedGroupItem = store.formatGroupItem(mockRawChildren[0]);
+    it('with hideProjects', () => {
+      const store = new GroupsStore({ hideProjects: true });
+      const updatedGroupItem = store.formatGroupItem(mockRawChildren[0]);
 
       expect(Object.keys(updatedGroupItem).indexOf('fullName')).toBeGreaterThan(-1);
       expect(updatedGroupItem.childrenCount).toBe(mockRawChildren[0].subgroup_count);
+      expect(updatedGroupItem.microdata).toEqual({});
+    });
+
+    it('with showSchemaMarkup', () => {
+      const store = new GroupsStore({ showSchemaMarkup: true });
+      const updatedGroupItem = store.formatGroupItem(mockRawChildren[0]);
+
+      expect(updatedGroupItem.microdata).toEqual(getGroupItemMicrodata(mockRawChildren[0]));
     });
   });
 

spec/frontend/groups/store/utils_spec.js

+import { getGroupItemMicrodata } from '~/groups/store/utils';
+
+describe('~/groups/store/utils', () => {
+  describe('getGroupItemMetadata', () => {
+    it('has default type', () => {
+      expect(getGroupItemMicrodata({ type: 'silly' })).toMatchInlineSnapshot(`
+        Object {
+          "descriptionItemprop": "description",
+          "imageItemprop": "image",
+          "itemprop": "owns",
+          "itemscope": true,
+          "itemtype": "https://schema.org/Thing",
+          "nameItemprop": "name",
+        }
+      `);
+    });
+
+    it('has group props', () => {
+      expect(getGroupItemMicrodata({ type: 'group' })).toMatchInlineSnapshot(`
+        Object {
+          "descriptionItemprop": "description",
+          "imageItemprop": "logo",
+          "itemprop": "subOrganization",
+          "itemscope": true,
+          "itemtype": "https://schema.org/Organization",
+          "nameItemprop": "name",
+        }
+      `);
+    });
+
+    it('has project props', () => {
+      expect(getGroupItemMicrodata({ type: 'project' })).toMatchInlineSnapshot(`
+        Object {
+          "descriptionItemprop": "description",
+          "imageItemprop": "image",
+          "itemprop": "owns",
+          "itemscope": true,
+          "itemtype": "https://schema.org/SoftwareSourceCode",
+          "nameItemprop": "name",
+        }
+      `);
+    });
+  });
+});