Make schema breadcrumb urls absolute

It seems is better for crawlers if in the schema
markup for breadcrumb, urls are absolute instead of
relative.

In this commit we change that.

app/helpers/breadcrumbs_helper.rb

@@ -71,7 +71,14 @@ module BreadcrumbsHelper
       '@type' => 'ListItem',
       'position' => position,
       'name' => text,
-      'item' => link
+      'item' => ensure_absolute_link(link)
     }
   end
+
+  def ensure_absolute_link(link)
+    url = URI.parse(link)
+    url.absolute? ? link : URI.join(request.base_url, link).to_s
+  rescue URI::InvalidURIError
+    "#{request.base_url}#{request.path}"
+  end
 end

changelogs/unreleased/281378-fj-add-absolute-urls-to-schema-breadcrumb.yml

+---
+title: Make schema breadcrumb urls absolute
+merge_request: 47523
+author:
+type: changed

spec/features/breadcrumbs_schema_markup_spec.rb

@@ -17,13 +17,13 @@ RSpec.describe 'Breadcrumbs schema markup', :aggregate_failures do
 
     expect(item_list.size).to eq 3
     expect(item_list[0]['name']).to eq project.namespace.name
-    expect(item_list[0]['item']).to eq user_path(project.owner)
+    expect(item_list[0]['item']).to eq user_url(project.owner)
 
     expect(item_list[1]['name']).to eq project.name
-    expect(item_list[1]['item']).to eq project_path(project)
+    expect(item_list[1]['item']).to eq project_url(project)
 
     expect(item_list[2]['name']).to eq 'Details'
-    expect(item_list[2]['item']).to eq project_path(project)
+    expect(item_list[2]['item']).to eq project_url(project)
   end
 
   it 'generates the breadcrumb schema for group projects' do
@@ -33,16 +33,16 @@ RSpec.describe 'Breadcrumbs schema markup', :aggregate_failures do
 
     expect(item_list.size).to eq 4
     expect(item_list[0]['name']).to eq group.name
-    expect(item_list[0]['item']).to eq group_path(group)
+    expect(item_list[0]['item']).to eq group_url(group)
 
     expect(item_list[1]['name']).to eq subgroup.name
-    expect(item_list[1]['item']).to eq group_path(subgroup)
+    expect(item_list[1]['item']).to eq group_url(subgroup)
 
     expect(item_list[2]['name']).to eq group_project.name
-    expect(item_list[2]['item']).to eq project_path(group_project)
+    expect(item_list[2]['item']).to eq project_url(group_project)
 
     expect(item_list[3]['name']).to eq 'Details'
-    expect(item_list[3]['item']).to eq project_path(group_project)
+    expect(item_list[3]['item']).to eq project_url(group_project)
   end
 
   it 'generates the breadcrumb schema for group' do
@@ -52,13 +52,13 @@ RSpec.describe 'Breadcrumbs schema markup', :aggregate_failures do
 
     expect(item_list.size).to eq 3
     expect(item_list[0]['name']).to eq group.name
-    expect(item_list[0]['item']).to eq group_path(group)
+    expect(item_list[0]['item']).to eq group_url(group)
 
     expect(item_list[1]['name']).to eq subgroup.name
-    expect(item_list[1]['item']).to eq group_path(subgroup)
+    expect(item_list[1]['item']).to eq group_url(subgroup)
 
     expect(item_list[2]['name']).to eq 'Details'
-    expect(item_list[2]['item']).to eq group_path(subgroup)
+    expect(item_list[2]['item']).to eq group_url(subgroup)
   end
 
   it 'generates the breadcrumb schema for issues' do
@@ -68,13 +68,13 @@ RSpec.describe 'Breadcrumbs schema markup', :aggregate_failures do
 
     expect(item_list.size).to eq 3
     expect(item_list[0]['name']).to eq project.namespace.name
-    expect(item_list[0]['item']).to eq user_path(project.owner)
+    expect(item_list[0]['item']).to eq user_url(project.owner)
 
     expect(item_list[1]['name']).to eq project.name
-    expect(item_list[1]['item']).to eq project_path(project)
+    expect(item_list[1]['item']).to eq project_url(project)
 
     expect(item_list[2]['name']).to eq 'Issues'
-    expect(item_list[2]['item']).to eq project_issues_path(project)
+    expect(item_list[2]['item']).to eq project_issues_url(project)
   end
 
   it 'generates the breadcrumb schema for specific issue' do
@@ -84,16 +84,16 @@ RSpec.describe 'Breadcrumbs schema markup', :aggregate_failures do
 
     expect(item_list.size).to eq 4
     expect(item_list[0]['name']).to eq project.namespace.name
-    expect(item_list[0]['item']).to eq user_path(project.owner)
+    expect(item_list[0]['item']).to eq user_url(project.owner)
 
     expect(item_list[1]['name']).to eq project.name
-    expect(item_list[1]['item']).to eq project_path(project)
+    expect(item_list[1]['item']).to eq project_url(project)
 
     expect(item_list[2]['name']).to eq 'Issues'
-    expect(item_list[2]['item']).to eq project_issues_path(project)
+    expect(item_list[2]['item']).to eq project_issues_url(project)
 
     expect(item_list[3]['name']).to eq issue.to_reference
-    expect(item_list[3]['item']).to eq project_issue_path(project, issue)
+    expect(item_list[3]['item']).to eq project_issue_url(project, issue)
   end
 
   def get_schema_content

spec/helpers/breadcrumbs_helper_spec.rb

@@ -4,16 +4,49 @@ require 'spec_helper'
 
 RSpec.describe BreadcrumbsHelper do
   describe '#push_to_schema_breadcrumb' do
-    it 'enqueue element name, link and position' do
-      element = %w(element1 link1)
-      helper.push_to_schema_breadcrumb(element[0], element[1])
+    let(:element_name) { 'BreadCrumbElement' }
+    let(:link) { 'http://test.host/foo' }
+    let(:breadcrumb_list) { helper.instance_variable_get(:@schema_breadcrumb_list) }
+
+    subject { helper.push_to_schema_breadcrumb(element_name, link) }
 
-      list = helper.instance_variable_get(:@schema_breadcrumb_list)
+    it 'enqueue element name, link and position' do
+      subject
 
       aggregate_failures do
-        expect(list[0]['name']).to eq element[0]
-        expect(list[0]['item']).to eq element[1]
-        expect(list[0]['position']).to eq(1)
+        expect(breadcrumb_list[0]['name']).to eq element_name
+        expect(breadcrumb_list[0]['item']).to eq link
+        expect(breadcrumb_list[0]['position']).to eq(1)
+      end
+    end
+
+    context 'when link is relative' do
+      let(:link) { '/foo' }
+
+      it 'converts the url into absolute' do
+        subject
+
+        expect(breadcrumb_list[0]['item']).to eq "http://test.host#{link}"
+      end
+    end
+
+    describe 'when link is invalid' do
+      let(:link) { 'invalid://foo[]' }
+
+      it 'returns the current url' do
+        subject
+
+        expect(breadcrumb_list[0]['item']).to eq 'http://test.host'
+      end
+    end
+
+    describe 'when link is nil' do
+      let(:link) { nil }
+
+      it 'returns the current url' do
+        subject
+
+        expect(breadcrumb_list[0]['item']).to eq 'http://test.host'
       end
     end
   end
@@ -21,8 +54,8 @@ RSpec.describe BreadcrumbsHelper do
   describe '#schema_breadcrumb_json' do
     let(:elements) do
       [
-        %w(element1 link1),
-        %w(element2 link2)
+        %w(element1 http://test.host/link1),
+        %w(element2 http://test.host/link2)
       ]
     end
 
@@ -56,8 +89,8 @@ RSpec.describe BreadcrumbsHelper do
     context 'when extra breadcrumb element is added' do
       let(:extra_elements) do
         [
-          %w(extra_element1 extra_link1),
-          %w(extra_element2 extra_link2)
+          %w(extra_element1 http://test.host/extra_link1),
+          %w(extra_element2 http://test.host/extra_link2)
         ]
       end