Merge branch '281378-fj-add-absolute-urls-to-schema-breadcrumb' into 'master'

Make schema breadcrumb urls absolute

See merge request gitlab-org/gitlab!47523

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