Commit 655a7057 authored by Jason Yavorska's avatar Jason Yavorska Committed by Dmytro Zaporozhets (DZ)

Implement maintainer feedback

Add canonical links helper for specs
Reduce factory usage
Remove extra description blocks
parent 3e4b0007
...@@ -40,6 +40,14 @@ module PageLayoutHelper ...@@ -40,6 +40,14 @@ module PageLayoutHelper
end end
end end
def page_canonical_link(link = nil)
if link
@page_canonical_link = link
else
@page_canonical_link
end
end
def favicon def favicon
Gitlab::Favicon.main Gitlab::Favicon.main
end end
......
...@@ -19,6 +19,7 @@ class Issue < ApplicationRecord ...@@ -19,6 +19,7 @@ class Issue < ApplicationRecord
include WhereComposite include WhereComposite
include StateEventable include StateEventable
include IdInOrdered include IdInOrdered
include Presentable
DueDateStruct = Struct.new(:title, :name).freeze DueDateStruct = Struct.new(:title, :name).freeze
NoDueDate = DueDateStruct.new('No Due Date', '0').freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze
...@@ -417,6 +418,10 @@ class Issue < ApplicationRecord ...@@ -417,6 +418,10 @@ class Issue < ApplicationRecord
IssueLink.inverse_link_type(type) IssueLink.inverse_link_type(type)
end end
def relocation_target
moved_to || duplicated_to
end
private private
def ensure_metrics def ensure_metrics
......
...@@ -42,6 +42,9 @@ ...@@ -42,6 +42,9 @@
%title= page_title(site_name) %title= page_title(site_name)
%meta{ name: "description", content: page_description } %meta{ name: "description", content: page_description }
- if page_canonical_link
%link{ rel: 'canonical', href: page_canonical_link }
= favicon_link_tag favicon, id: 'favicon', data: { original_href: favicon }, type: 'image/png' = favicon_link_tag favicon, id: 'favicon', data: { original_href: favicon }, type: 'image/png'
= render 'layouts/startup_css' = render 'layouts/startup_css'
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
- page_title "#{@issue.title} (#{@issue.to_reference})", _("Issues") - page_title "#{@issue.title} (#{@issue.to_reference})", _("Issues")
- page_description @issue.description - page_description @issue.description
- page_card_attributes @issue.card_attributes - page_card_attributes @issue.card_attributes
- if @issue.relocation_target
- page_canonical_link @issue.relocation_target.present(current_user: current_user).web_url
- can_update_issue = can?(current_user, :update_issue, @issue) - can_update_issue = can?(current_user, :update_issue, @issue)
- can_reopen_issue = can?(current_user, :reopen_issue, @issue) - can_reopen_issue = can?(current_user, :reopen_issue, @issue)
......
---
title: Add canonical links for moved/duplicated issues
merge_request: 34604
author:
type: added
...@@ -26,10 +26,6 @@ module EE ...@@ -26,10 +26,6 @@ module EE
end end
end end
def epic_path(entity, *args)
group_epic_path(entity.group, entity, *args)
end
def license_management_api_url(project) def license_management_api_url(project)
expose_path(api_v4_projects_managed_licenses_path(id: project.id)) expose_path(api_v4_projects_managed_licenses_path(id: project.id))
end end
...@@ -60,6 +56,11 @@ module EE ...@@ -60,6 +56,11 @@ module EE
end end
end end
url_helper :epic
def epic_path(entity, *args)
group_epic_path(entity.group, entity, *args)
end
url_helper :user_group_saml_omniauth_metadata url_helper :user_group_saml_omniauth_metadata
def user_group_saml_omniauth_metadata_path(group) def user_group_saml_omniauth_metadata_path(group)
params = { group_path: group.path, token: group.saml_discovery_token } params = { group_path: group.path, token: group.saml_discovery_token }
......
...@@ -16,6 +16,7 @@ module EE ...@@ -16,6 +16,7 @@ module EE
include UsageStatistics include UsageStatistics
include FromUnion include FromUnion
include EpicTreeSorting include EpicTreeSorting
include Presentable
enum state_id: { enum state_id: {
opened: ::Epic.available_states[:opened], opened: ::Epic.available_states[:opened],
......
...@@ -204,6 +204,11 @@ module EE ...@@ -204,6 +204,11 @@ module EE
update!(blocking_issues_count: blocking_count) update!(blocking_issues_count: blocking_count)
end end
override :relocation_target
def relocation_target
super || promoted_to_epic
end
private private
def blocking_issues_ids def blocking_issues_ids
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'issues canonical link' do
include Spec::Support::Helpers::Features::CanonicalLinkHelpers
let(:epic) { create(:epic) }
let(:project) { create(:project, :public, group: epic.group) }
let(:original_issue) { create(:issue, project: project) }
let(:canonical_url) { epic_url(epic) }
context 'when the issue was promoted' do
it 'shows the canonical URL' do
original_issue.promoted_to_epic = epic
original_issue.save!
visit(issue_path(original_issue))
expect(page).to have_canonical_link(canonical_url)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'issues canonical link' do
include Spec::Support::Helpers::Features::CanonicalLinkHelpers
let_it_be(:original_project) { create(:project, :public) }
let_it_be(:original_issue) { create(:issue, project: original_project) }
let_it_be(:canonical_issue) { create(:issue) }
let_it_be(:canonical_url) { issue_url(canonical_issue, Gitlab::Application.routes.default_url_options) }
it "doesn't show the canonical URL" do
visit(issue_path(original_issue))
expect(page).not_to have_any_canonical_links
end
context 'when the issue was moved' do
it 'shows the canonical URL' do
original_issue.moved_to = canonical_issue
original_issue.save!
visit(issue_path(original_issue))
expect(page).to have_canonical_link(canonical_url)
end
end
context 'when the issue was duplicated' do
it 'shows the canonical URL' do
original_issue.duplicated_to = canonical_issue
original_issue.save!
visit(issue_path(original_issue))
expect(page).to have_canonical_link(canonical_url)
end
end
end
# frozen_string_literal: true
# These helpers allow you to manipulate with notes.
#
# Usage:
# describe "..." do
# include Spec::Support::Helpers::Features::CanonicalLinkHelpers
# ...
#
# expect(page).to have_canonical_link(url)
#
module Spec
module Support
module Helpers
module Features
module CanonicalLinkHelpers
def have_canonical_link(url)
have_xpath("//link[@rel=\"canonical\" and @href=\"#{url}\"]", visible: false)
end
def have_any_canonical_links
have_xpath('//link[@rel="canonical"]', visible: false)
end
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment