Commit 4ff345c4 authored by Paco Guzman's avatar Paco Guzman

Simplify Mentionable concern instance methods

We remove some arguments that are rarely used or 
used just to simplify setups on specs.

Modified Mentionable#create_new_cross_references method 
we don’t need to calculate previous references to avoid the 
duplication because we do that at database level when 
creating references extracted from the current entity state.

MergeRequests won’t create cross_references for commits that are included so we change a spec to use a different merge request to make references to commits to other branches
parent ccd89ec9
...@@ -11,6 +11,7 @@ v 8.13.0 (unreleased) ...@@ -11,6 +11,7 @@ v 8.13.0 (unreleased)
- Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller) - Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller)
- Add more tests for calendar contribution (ClemMakesApps) - Add more tests for calendar contribution (ClemMakesApps)
- Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references
- Simplify Mentionable concern instance methods
- Fix permission for setting an issue's due date - Fix permission for setting an issue's due date
- Expose expires_at field when sharing project on API - Expose expires_at field when sharing project on API
- Fix issue with page scrolling to top when closing or pinning sidebar (lukehowell) - Fix issue with page scrolling to top when closing or pinning sidebar (lukehowell)
......
...@@ -43,19 +43,15 @@ module Mentionable ...@@ -43,19 +43,15 @@ module Mentionable
self self
end end
def all_references(current_user = nil, text = nil, extractor: nil) def all_references(current_user = nil, extractor: nil)
extractor ||= Gitlab::ReferenceExtractor. extractor ||= Gitlab::ReferenceExtractor.
new(project, current_user) new(project, current_user)
if text self.class.mentionable_attrs.each do |attr, options|
extractor.analyze(text, author: author) text = __send__(attr)
else options = options.merge(cache_key: [self, attr], author: author)
self.class.mentionable_attrs.each do |attr, options|
text = __send__(attr)
options = options.merge(cache_key: [self, attr], author: author)
extractor.analyze(text, options) extractor.analyze(text, options)
end
end end
extractor extractor
...@@ -66,8 +62,8 @@ module Mentionable ...@@ -66,8 +62,8 @@ module Mentionable
end end
# Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference. # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
def referenced_mentionables(current_user = self.author, text = nil) def referenced_mentionables(current_user = self.author)
refs = all_references(current_user, text) refs = all_references(current_user)
refs = (refs.issues + refs.merge_requests + refs.commits) refs = (refs.issues + refs.merge_requests + refs.commits)
# We're using this method instead of Array diffing because that requires # We're using this method instead of Array diffing because that requires
...@@ -77,8 +73,8 @@ module Mentionable ...@@ -77,8 +73,8 @@ module Mentionable
end end
# Create a cross-reference Note for each GFM reference to another Mentionable found in the +mentionable_attrs+. # Create a cross-reference Note for each GFM reference to another Mentionable found in the +mentionable_attrs+.
def create_cross_references!(author = self.author, without = [], text = nil) def create_cross_references!(author = self.author, without = [])
refs = referenced_mentionables(author, text) refs = referenced_mentionables(author)
# We're using this method instead of Array diffing because that requires # We're using this method instead of Array diffing because that requires
# both of the object's `hash` values to be the same, which may not be the # both of the object's `hash` values to be the same, which may not be the
...@@ -97,10 +93,7 @@ module Mentionable ...@@ -97,10 +93,7 @@ module Mentionable
return if changes.empty? return if changes.empty?
original_text = changes.collect { |_, vals| vals.first }.join(' ') create_cross_references!(author)
preexisting = referenced_mentionables(author, original_text)
create_cross_references!(author, preexisting)
end end
private private
......
require 'spec_helper' require 'spec_helper'
describe Mentionable do describe Mentionable do
include Mentionable class Example
include Mentionable
def author attr_accessor :project, :message
nil attr_mentionable :message
def author
nil
end
end end
describe 'references' do describe 'references' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:mentionable) { Example.new }
it 'excludes JIRA references' do it 'excludes JIRA references' do
allow(project).to receive_messages(jira_tracker?: true) allow(project).to receive_messages(jira_tracker?: true)
expect(referenced_mentionables(project, 'JIRA-123')).to be_empty
mentionable.project = project
mentionable.message = 'JIRA-123'
expect(mentionable.referenced_mentionables).to be_empty
end end
end end
end end
...@@ -39,9 +48,8 @@ describe Issue, "Mentionable" do ...@@ -39,9 +48,8 @@ describe Issue, "Mentionable" do
let(:user) { create(:user) } let(:user) { create(:user) }
def referenced_issues(current_user) def referenced_issues(current_user)
text = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" issue.title = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}"
issue.referenced_mentionables(current_user)
issue.referenced_mentionables(current_user, text)
end end
context 'when the current user can see the issue' do context 'when the current user can see the issue' do
......
...@@ -522,7 +522,7 @@ describe MergeRequest, models: true do ...@@ -522,7 +522,7 @@ describe MergeRequest, models: true do
end end
it_behaves_like 'an editable mentionable' do it_behaves_like 'an editable mentionable' do
subject { create(:merge_request) } subject { create(:merge_request, :simple) }
let(:backref_text) { "merge request #{subject.to_reference}" } let(:backref_text) { "merge request #{subject.to_reference}" }
let(:set_mentionable_text) { ->(txt){ subject.description = txt } } let(:set_mentionable_text) { ->(txt){ subject.description = txt } }
......
...@@ -9,7 +9,7 @@ shared_context 'mentionable context' do ...@@ -9,7 +9,7 @@ shared_context 'mentionable context' do
let(:author) { subject.author } let(:author) { subject.author }
let(:mentioned_issue) { create(:issue, project: project) } let(:mentioned_issue) { create(:issue, project: project) }
let!(:mentioned_mr) { create(:merge_request, :simple, source_project: project) } let!(:mentioned_mr) { create(:merge_request, source_project: project) }
let(:mentioned_commit) { project.commit("HEAD~1") } let(:mentioned_commit) { project.commit("HEAD~1") }
let(:ext_proj) { create(:project, :public) } let(:ext_proj) { create(:project, :public) }
...@@ -100,6 +100,7 @@ shared_examples 'an editable mentionable' do ...@@ -100,6 +100,7 @@ shared_examples 'an editable mentionable' do
it 'creates new cross-reference notes when the mentionable text is edited' do it 'creates new cross-reference notes when the mentionable text is edited' do
subject.save subject.save
subject.create_cross_references!
new_text = <<-MSG.strip_heredoc new_text = <<-MSG.strip_heredoc
These references already existed: These references already existed:
...@@ -131,6 +132,7 @@ shared_examples 'an editable mentionable' do ...@@ -131,6 +132,7 @@ shared_examples 'an editable mentionable' do
end end
# These two issues are new and should receive reference notes # These two issues are new and should receive reference notes
# In the case of MergeRequests remember that cannot mention commits included in the MergeRequest
new_issues.each do |newref| new_issues.each do |newref|
expect(SystemNoteService).to receive(:cross_reference). expect(SystemNoteService).to receive(:cross_reference).
with(newref, subject.local_reference, author) with(newref, subject.local_reference, author)
......
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