Commit 768263f3 authored by Marcin Sedlak-Jakubowski's avatar Marcin Sedlak-Jakubowski

Merge branch '15694-allow-expanded-gfm-references' into 'master'

Allow issuable references to show titles

See merge request gitlab-org/gitlab!74369
parents 99594281 cdf83616
......@@ -2539,12 +2539,10 @@ Rails/IncludeUrlHelper:
- 'ee/app/presenters/merge_request_approver_presenter.rb'
- 'ee/spec/helpers/ee/projects/security/configuration_helper_spec.rb'
- 'ee/spec/lib/banzai/filter/cross_project_issuable_information_filter_spec.rb'
- 'ee/spec/lib/banzai/filter/issuable_state_filter_spec.rb'
- 'lib/gitlab/ci/badge/metadata.rb'
- 'spec/helpers/merge_requests_helper_spec.rb'
- 'spec/helpers/nav/top_nav_helper_spec.rb'
- 'spec/helpers/notify_helper_spec.rb'
- 'spec/lib/banzai/filter/issuable_state_filter_spec.rb'
- 'spec/lib/banzai/filter/reference_redactor_filter_spec.rb'
- 'spec/lib/banzai/reference_redactor_spec.rb'
......
......@@ -21,7 +21,7 @@ module PreviewMarkdown
def projects_filter_params
{
issuable_state_filter_enabled: true,
issuable_reference_expansion_enabled: true,
suggestions_filter_enabled: params[:preview_suggestions].present?
}
end
......
......@@ -110,7 +110,7 @@ class Projects::MergeRequests::DraftsController < Projects::MergeRequests::Appli
def render_draft_note(note)
params = { target_id: merge_request.id, target_type: 'MergeRequest', text: note.note }
result = PreviewMarkdownService.new(@project, current_user, params).execute
markdown_params = { markdown_engine: result[:markdown_engine], issuable_state_filter_enabled: true }
markdown_params = { markdown_engine: result[:markdown_engine], issuable_reference_expansion_enabled: true }
note.rendered_note = view_context.markdown(result[:text], markdown_params)
note.users_referenced = result[:users]
......
......@@ -181,7 +181,7 @@ module MarkupHelper
wiki: wiki,
repository: wiki.repository,
page_slug: wiki_page.slug,
issuable_state_filter_enabled: true
issuable_reference_expansion_enabled: true
).merge(render_wiki_content_context_container(wiki))
end
......
......@@ -43,7 +43,7 @@ module Issuable
included do
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description, issuable_state_filter_enabled: true
cache_markdown_field :description, issuable_reference_expansion_enabled: true
redact_field :description
......
......@@ -506,7 +506,7 @@ class MergeRequest < ApplicationRecord
def self.reference_pattern
@reference_pattern ||= %r{
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}(?<merge_request>\d+)
#{Regexp.escape(reference_prefix)}(?<merge_request>\d+)(?<format>\+)?
}x
end
......
......@@ -23,7 +23,7 @@ class Note < ApplicationRecord
include FromUnion
include Sortable
cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true
cache_markdown_field :note, pipeline: :note, issuable_reference_expansion_enabled: true
redact_field :note
......
......@@ -136,7 +136,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
pipeline: :gfm,
author: author,
project: project,
issuable_state_filter_enabled: true
issuable_reference_expansion_enabled: true
)
end
......@@ -146,7 +146,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
pipeline: :gfm,
author: author,
project: project,
issuable_state_filter_enabled: true
issuable_reference_expansion_enabled: true
)
end
......
......@@ -516,30 +516,30 @@ version to reference other projects from the same namespace.
GitLab Flavored Markdown recognizes the following:
| references | input | cross-project reference | shortcut inside same namespace |
| :------------------------------ | :------------------------- | :-------------------------------------- | :----------------------------- |
| specific user | `@user_name` | | |
| specific group | `@group_name` | | |
| entire team | `@all` | | |
| project | `namespace/project>` | | |
| issue | ``#123`` | `namespace/project#123` | `project#123` |
| merge request | `!123` | `namespace/project!123` | `project!123` |
| snippet | `$123` | `namespace/project$123` | `project$123` |
| epic **(ULTIMATE)** | `&123` | `group1/subgroup&123` | |
| vulnerability **(ULTIMATE)** (1)| `[vulnerability:123]` | `[vulnerability:namespace/project/123]` | `[vulnerability:project/123]` |
| feature flag | `[feature_flag:123]` | `[feature_flag:namespace/project/123]` | `[feature_flag:project/123]` |
| label by ID | `~123` | `namespace/project~123` | `project~123` |
| one-word label by name | `~bug` | `namespace/project~bug` | `project~bug` |
| multi-word label by name | `~"feature request"` | `namespace/project~"feature request"` | `project~"feature request"` |
| scoped label by name | `~"priority::high"` | `namespace/project~"priority::high"` | `project~"priority::high"` |
| project milestone by ID | `%123` | `namespace/project%123` | `project%123` |
| one-word milestone by name | `%v1.23` | `namespace/project%v1.23` | `project%v1.23` |
| multi-word milestone by name | `%"release candidate"` | `namespace/project%"release candidate"` | `project%"release candidate"` |
| specific commit | `9ba12248` | `namespace/project@9ba12248` | `project@9ba12248` |
| commit range comparison | `9ba12248...b19a04f5` | `namespace/project@9ba12248...b19a04f5` | `project@9ba12248...b19a04f5` |
| repository file references | `[README](doc/README.md)` | | |
| repository file line references | `[README](doc/README.md#L13)` | | |
| [alert](../operations/incident_management/alerts.md) | `^alert#123` | `namespace/project^alert#123` | `project^alert#123` |
| references | input | cross-project reference | shortcut inside same namespace |
| :--------------------------------------------------- | :---------------------------- | :----------------------------------------- | :------------------------------- |
| specific user | `@user_name` | | |
| specific group | `@group_name` | | |
| entire team | `@all` | | |
| project | `namespace/project>` | | |
| issue | ``#123`` | `namespace/project#123` | `project#123` |
| merge request | `!123` | `namespace/project!123` | `project!123` |
| snippet | `$123` | `namespace/project$123` | `project$123` |
| [epic](group/epics/index.md) | `&123` | `group1/subgroup&123` | |
| vulnerability **(ULTIMATE)** <sup>1</sup> | `[vulnerability:123]` | `[vulnerability:namespace/project/123]` | `[vulnerability:project/123]` |
| feature flag | `[feature_flag:123]` | `[feature_flag:namespace/project/123]` | `[feature_flag:project/123]` |
| label by ID | `~123` | `namespace/project~123` | `project~123` |
| one-word label by name | `~bug` | `namespace/project~bug` | `project~bug` |
| multi-word label by name | `~"feature request"` | `namespace/project~"feature request"` | `project~"feature request"` |
| scoped label by name | `~"priority::high"` | `namespace/project~"priority::high"` | `project~"priority::high"` |
| project milestone by ID | `%123` | `namespace/project%123` | `project%123` |
| one-word milestone by name | `%v1.23` | `namespace/project%v1.23` | `project%v1.23` |
| multi-word milestone by name | `%"release candidate"` | `namespace/project%"release candidate"` | `project%"release candidate"` |
| specific commit | `9ba12248` | `namespace/project@9ba12248` | `project@9ba12248` |
| commit range comparison | `9ba12248...b19a04f5` | `namespace/project@9ba12248...b19a04f5` | `project@9ba12248...b19a04f5` |
| repository file references | `[README](doc/README.md)` | | |
| repository file line references | `[README](doc/README.md#L13)` | | |
| [alert](../operations/incident_management/alerts.md) | `^alert#123` | `namespace/project^alert#123` | `project^alert#123` |
1. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/222483) in GitLab 13.7.
......@@ -554,6 +554,16 @@ In addition to this, links to some objects are also recognized and formatted. So
- The issues designs tab: `"https://gitlab.com/gitlab-org/gitlab/-/issues/1234/designs"`, which are rendered as `#1234 (designs)`.
- Links to individual designs: `"https://gitlab.com/gitlab-org/gitlab/-/issues/1234/designs/layout.png"`, which are rendered as `#1234[layout.png]`.
### Show the issue, merge request, or epic title in the reference
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/15694) in GitLab 14.6.
To include the title in the rendered link of an issue, merge request, or epic, add a plus (`+`)
at the end of the reference. For example, a reference like `#123+` is rendered as
`The issue title (#123)`.
Expanding titles does not apply to URL references, like `https://gitlab.com/gitlab-org/gitlab/-/issues/1234`.
### Embedding metrics in GitLab Flavored Markdown
Metric charts can be embedded in GitLab Flavored Markdown. Read
......
......@@ -221,7 +221,7 @@ module EE
}x
%r{
(#{group_regexp})?
(?:#{combined_prefix})(?<epic>\d+)
(?:#{combined_prefix})(?<epic>\d+)(?<format>\+)?
}x
end
end
......
......@@ -23,7 +23,7 @@ module EE
PASSIVE_STATES = %w(dismissed resolved).freeze
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description, issuable_state_filter_enabled: true
cache_markdown_field :description, issuable_reference_expansion_enabled: true
strip_attributes! :title
......
......@@ -14,7 +14,7 @@ module RequirementsManagement
self.table_name = 'requirements'
cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description, issuable_state_filter_enabled: true
cache_markdown_field :description, issuable_reference_expansion_enabled: true
strip_attributes! :title
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Banzai::Filter::IssuableStateFilter do
include ActionView::Helpers::UrlHelper
RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter do
include FilterSpecHelper
let(:user) { create(:user) }
let(:context) { { current_user: user, issuable_state_filter_enabled: true, group: group } }
let(:epic) { create(:epic, :opened, group: group) }
let(:closed_epic) { create(:epic, :closed, group: group) }
let(:group) { create(:group) }
let(:other_group) { create(:group) }
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:other_group) { create(:group) }
let_it_be(:epic) { create(:epic, :opened, group: group, title: 'Some epic') }
let_it_be(:closed_epic) { create(:epic, :closed, group: group) }
let_it_be(:context) { { current_user: user, issuable_reference_expansion_enabled: true, group: group } }
def create_link(text, data)
link_to(text, '', class: 'gfm has-tooltip', data: data)
ActionController::Base.helpers.link_to(text, '', class: 'gfm has-tooltip', data: data)
end
it 'ignores open epic references' do
......@@ -41,4 +41,12 @@ RSpec.describe Banzai::Filter::IssuableStateFilter do
expect(doc.css('a').last.text).to eq("#{closed_epic.to_reference(other_group)}")
end
it 'shows title for references with +' do
link = create_link(epic.to_reference, epic: epic.id, reference_type: 'epic', reference_format: '+')
doc = filter(link, context)
expect(doc.css('a').last.text).to eq("#{epic.title} (#{epic.to_reference})")
end
end
......@@ -65,6 +65,13 @@ RSpec.describe Banzai::Filter::References::EpicReferenceFilter do
expect(link.attr('data-original')).to eq(CGI.escapeHTML(reference))
end
it 'includes a data-reference-format attribute' do
link = doc("&#{epic.iid}+").css('a').first
expect(link).to have_attribute('data-reference-format')
expect(link.attr('data-reference-format')).to eq('+')
end
it 'ignores invalid epic IIDs' do
text = "Check &#{non_existing_record_iid}"
......
......@@ -2,16 +2,18 @@
module Banzai
module Filter
# HTML filter that appends state information to issuable links.
# Runs as a post-process filter as issuable state might change while
# HTML filter that appends extra information to issuable links.
# Runs as a post-process filter as issuable might change while
# Markdown is in the cache.
#
# This filter supports cross-project references.
class IssuableStateFilter < HTML::Pipeline::Filter
class IssuableReferenceExpansionFilter < HTML::Pipeline::Filter
include Gitlab::Utils::StrongMemoize
VISIBLE_STATES = %w(closed merged).freeze
def call
return doc unless context[:issuable_state_filter_enabled]
return doc unless context[:issuable_reference_expansion_enabled]
context = RenderContext.new(project, current_user)
extractor = Banzai::IssuableExtractor.new(context)
......@@ -19,10 +21,13 @@ module Banzai
issuables.each do |node, issuable|
next if !can_read_cross_project? && cross_referenced?(issuable)
next unless should_expand?(node, issuable)
if VISIBLE_STATES.include?(issuable.state) && issuable_reference?(node.inner_html, issuable)
state = moved_issue?(issuable) ? s_("IssuableStatus|moved") : issuable.state
node.content += " (#{state})"
case node.attr('data-reference-format')
when '+'
expand_reference_with_title_and_state(node, issuable)
else
expand_reference_with_state(node, issuable)
end
end
......@@ -31,12 +36,31 @@ module Banzai
private
# Example: Issue Title (#123 - closed)
def expand_reference_with_title_and_state(node, issuable)
node.content = "#{issuable.title.truncate(50)} (#{node.content}"
node.content += " - #{issuable_state_text(issuable)}" if VISIBLE_STATES.include?(issuable.state)
node.content += ')'
end
# Example: #123 (closed)
def expand_reference_with_state(node, issuable)
node.content += " (#{issuable_state_text(issuable)})"
end
def issuable_state_text(issuable)
moved_issue?(issuable) ? s_("IssuableStatus|moved") : issuable.state
end
def moved_issue?(issuable)
issuable.instance_of?(Issue) && issuable.moved?
end
def issuable_reference?(text, issuable)
CGI.unescapeHTML(text) == issuable.reference_link_text(project || group)
def should_expand?(node, issuable)
# We add this extra check to avoid unescaping HTML and generating reference link text for every reference
return unless node.attr('data-reference-format').present? || VISIBLE_STATES.include?(issuable.state)
CGI.unescapeHTML(node.inner_html) == issuable.reference_link_text(project || group)
end
def cross_referenced?(issuable)
......@@ -47,7 +71,9 @@ module Banzai
end
def can_read_cross_project?
Ability.allowed?(current_user, :read_cross_project)
strong_memoize(:can_read_cross_project) do
Ability.allowed?(current_user, :read_cross_project)
end
end
def current_user
......
......@@ -205,6 +205,8 @@ module Banzai
data_attributes = data_attributes_for(link_content || match, parent, object,
link_content: !!link_content,
link_reference: link_reference)
data_attributes[:reference_format] = matches[:format] if matches.names.include?("format")
data = data_attribute(data_attributes)
url =
......
......@@ -19,7 +19,7 @@ module Banzai
# prevent unnecessary Gitaly calls from being made.
Filter::UploadLinkFilter,
Filter::RepositoryLinkFilter,
Filter::IssuableStateFilter,
Filter::IssuableReferenceExpansionFilter,
Filter::SuggestionFilter
]
end
......
......@@ -413,7 +413,7 @@ module Gitlab
end
def issue
@issue ||= /(?<issue>\d+\b)/
@issue ||= /(?<issue>\d+)(?<format>\+)?(?=\W|\z)/
end
def base64_regex
......
......@@ -321,7 +321,7 @@ RSpec.describe MarkupHelper do
let(:context) do
{
pipeline: :wiki, project: project, wiki: wiki,
page_slug: 'nested/page', issuable_state_filter_enabled: true,
page_slug: 'nested/page', issuable_reference_expansion_enabled: true,
repository: wiki_repository
}
end
......
......@@ -2,28 +2,27 @@
require 'spec_helper'
RSpec.describe Banzai::Filter::IssuableStateFilter do
include ActionView::Helpers::UrlHelper
RSpec.describe Banzai::Filter::IssuableReferenceExpansionFilter do
include FilterSpecHelper
let(:user) { create(:user) }
let(:context) { { current_user: user, issuable_state_filter_enabled: true } }
let(:closed_issue) { create_issue(:closed) }
let(:project) { create(:project, :public) }
let(:group) { create(:group) }
let(:other_project) { create(:project, :public) }
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public) }
let_it_be(:group) { create(:group) }
let_it_be(:other_project) { create(:project, :public) }
let_it_be(:closed_issue) { create_issue(:closed) }
let(:context) { { current_user: user, issuable_reference_expansion_enabled: true } }
def create_link(text, data)
link_to(text, '', class: 'gfm has-tooltip', data: data)
ActionController::Base.helpers.link_to(text, '', class: 'gfm has-tooltip', data: data)
end
def create_issue(state)
create(:issue, state, project: project)
def create_issue(state, attributes = {})
create(:issue, state, attributes.merge(project: project))
end
def create_merge_request(state)
create(:merge_request, state,
source_project: project, target_project: project)
def create_merge_request(state, attributes = {})
create(:merge_request, state, attributes.merge(source_project: project, target_project: project))
end
it 'ignores non-GFM links' do
......@@ -139,6 +138,30 @@ RSpec.describe Banzai::Filter::IssuableStateFilter do
expect(doc.css('a').last.text).to eq("#{moved_issue.to_reference} (moved)")
end
it 'shows title for references with +' do
issue = create_issue(:opened, title: 'Some issue')
link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+')
doc = filter(link, context)
expect(doc.css('a').last.text).to eq("#{issue.title} (#{issue.to_reference})")
end
it 'truncates long title for references with +' do
issue = create_issue(:opened, title: 'Some issue ' * 10)
link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+')
doc = filter(link, context)
expect(doc.css('a').last.text).to eq("#{issue.title.truncate(50)} (#{issue.to_reference})")
end
it 'shows both title and state for closed references with +' do
issue = create_issue(:closed, title: 'Some issue')
link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue', reference_format: '+')
doc = filter(link, context)
expect(doc.css('a').last.text).to eq("#{issue.title} (#{issue.to_reference} - closed)")
end
end
context 'for merge request references' do
......@@ -197,5 +220,20 @@ RSpec.describe Banzai::Filter::IssuableStateFilter do
expect(doc.css('a').last.text).to eq("#{merge_request.to_reference} (merged)")
end
it 'shows title for references with +' do
merge_request = create_merge_request(:opened, title: 'Some merge request')
link = create_link(
merge_request.to_reference,
merge_request: merge_request.id,
reference_type: 'merge_request',
reference_format: '+'
)
doc = filter(link, context)
expect(doc.css('a').last.text).to eq("#{merge_request.title} (#{merge_request.to_reference})")
end
end
end
......@@ -116,6 +116,14 @@ RSpec.describe Banzai::Filter::References::IssueReferenceFilter do
expect(doc.children.first.attr('data-original')).to eq inner_html
end
it 'includes a data-reference-format attribute' do
doc = reference_filter("Issue #{reference}+")
link = doc.css('a').first
expect(link).to have_attribute('data-reference-format')
expect(link.attr('data-reference-format')).to eq('+')
end
it 'supports an :only_path context' do
doc = reference_filter("Issue #{reference}", only_path: true)
link = doc.css('a').first.attr('href')
......
......@@ -109,6 +109,14 @@ RSpec.describe Banzai::Filter::References::MergeRequestReferenceFilter do
expect(link.attr('data-merge-request')).to eq merge.id.to_s
end
it 'includes a data-reference-format attribute' do
doc = reference_filter("Merge #{reference}+")
link = doc.css('a').first
expect(link).to have_attribute('data-reference-format')
expect(link.attr('data-reference-format')).to eq('+')
end
it 'supports an :only_path context' do
doc = reference_filter("Merge #{reference}", only_path: true)
link = doc.css('a').first.attr('href')
......
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