Commit 3a23639b authored by Ershad Kunnakkadan's avatar Ershad Kunnakkadan

Create directly_addressed Todos when mentioned in beginning of a line

parent 11d33873
......@@ -15,6 +15,7 @@ module TodosHelper
when Todo::MARKED then 'added a todo for'
when Todo::APPROVAL_REQUIRED then 'set you as an approver for'
when Todo::UNMERGEABLE then 'Could not merge'
when Todo::DIRECTLY_ADDRESSED then 'directly addressed you on'
end
end
......@@ -88,7 +89,8 @@ module TodosHelper
{ id: Todo::ASSIGNED, text: 'Assigned' },
{ id: Todo::MENTIONED, text: 'Mentioned' },
{ id: Todo::MARKED, text: 'Added' },
{ id: Todo::BUILD_FAILED, text: 'Pipelines' }
{ id: Todo::BUILD_FAILED, text: 'Pipelines' },
{ id: Todo::DIRECTLY_ADDRESSED, text: 'Directly addressed' }
]
end
......
......@@ -44,8 +44,15 @@ module Mentionable
end
def all_references(current_user = nil, extractor: nil)
extractor ||= Gitlab::ReferenceExtractor.
new(project, current_user)
# Use custom extractor if it's passed in the function parameters.
if extractor
@extractor = extractor
else
@extractor ||= Gitlab::ReferenceExtractor.
new(project, current_user)
@extractor.reset_memoized_values
end
self.class.mentionable_attrs.each do |attr, options|
text = __send__(attr)
......@@ -55,16 +62,20 @@ module Mentionable
skip_project_check: skip_project_check?
)
extractor.analyze(text, options)
@extractor.analyze(text, options)
end
extractor
@extractor
end
def mentioned_users(current_user = nil)
all_references(current_user).users
end
def directly_addressed_users(current_user = nil)
all_references(current_user).directly_addressed_users
end
# Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
def referenced_mentionables(current_user = self.author)
refs = all_references(current_user)
......
class DirectlyAddressedUser
class << self
def reference_pattern
User.reference_pattern
end
end
end
class Todo < ActiveRecord::Base
include Sortable
ASSIGNED = 1
MENTIONED = 2
BUILD_FAILED = 3
MARKED = 4
APPROVAL_REQUIRED = 5 # This is an EE-only feature
UNMERGEABLE = 6
ASSIGNED = 1
MENTIONED = 2
BUILD_FAILED = 3
MARKED = 4
APPROVAL_REQUIRED = 5 # This is an EE-only feature
UNMERGEABLE = 6
DIRECTLY_ADDRESSED = 7
ACTION_NAMES = {
ASSIGNED => :assigned,
......@@ -14,7 +15,8 @@ class Todo < ActiveRecord::Base
BUILD_FAILED => :build_failed,
MARKED => :marked,
APPROVAL_REQUIRED => :approval_required,
UNMERGEABLE => :unmergeable
UNMERGEABLE => :unmergeable,
DIRECTLY_ADDRESSED => :directly_addressed
}
belongs_to :author, class_name: "User"
......
......@@ -243,6 +243,12 @@ class TodoService
end
def create_mention_todos(project, target, author, note = nil)
# Create Todos for directly addressed users
directly_addressed_users = filter_directly_addressed_users(project, note || target, author)
attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note)
create_todos(directly_addressed_users, attributes)
# Create Todos for mentioned users
mentioned_users = filter_mentioned_users(project, note || target, author)
attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note)
create_todos(mentioned_users, attributes)
......@@ -282,10 +288,18 @@ class TodoService
)
end
def filter_todo_users(users, project, target)
reject_users_without_access(users, project, target).uniq
end
def filter_mentioned_users(project, target, author)
mentioned_users = target.mentioned_users(author)
mentioned_users = reject_users_without_access(mentioned_users, project, target)
mentioned_users.uniq
filter_todo_users(mentioned_users, project, target)
end
def filter_directly_addressed_users(project, target, author)
directly_addressed_users = target.directly_addressed_users(author)
filter_todo_users(directly_addressed_users, project, target)
end
def reject_users_without_access(users, project, target)
......
---
title: Added a feature to create a 'directly addressed' Todo when mentioned in the beginning of a line.
merge_request: 7926
author: Ershad Kunnakkadan
module Banzai
module Querying
module_function
# Searches a Nokogiri document using a CSS query, optionally optimizing it
# whenever possible.
#
# document - A document/element to search.
# query - The CSS query to use.
# document - A document/element to search.
# query - The CSS query to use.
# reference_options - A hash with nodes filter options
#
# Returns a Nokogiri::XML::NodeSet.
def self.css(document, query)
# Returns an array of Nokogiri::XML::Element objects if location is specified
# in reference_options. Otherwise it would a Nokogiri::XML::NodeSet.
def css(document, query, reference_options = {})
# When using "a.foo" Nokogiri compiles this to "//a[...]" but
# "descendant::a[...]" is quite a bit faster and achieves the same result.
xpath = Nokogiri::CSS.xpath_for(query)[0].gsub(%r{^//}, 'descendant::')
xpath = restrict_to_p_nodes_at_root(xpath) if filter_nodes_at_beginning?(reference_options)
nodes = document.xpath(xpath)
filter_nodes(nodes, reference_options)
end
def restrict_to_p_nodes_at_root(xpath)
xpath.gsub('descendant::', './p/')
end
def filter_nodes(nodes, reference_options)
if filter_nodes_at_beginning?(reference_options)
filter_nodes_at_beginning(nodes)
else
nodes
end
end
def filter_nodes_at_beginning?(reference_options)
reference_options && reference_options[:location] == :beginning
end
# Selects child nodes if they are present in the beginning among other siblings.
#
# nodes - A Nokogiri::XML::NodeSet.
#
# Returns an array of Nokogiri::XML::Element objects.
def filter_nodes_at_beginning(nodes)
parents_and_nodes = nodes.group_by(&:parent)
filtered_nodes = []
parents_and_nodes.each do |parent, nodes|
children = parent.children
nodes = nodes.to_a
children.each do |child|
next if child.text.blank?
node = nodes.shift
break unless node == child
filtered_nodes << node
end
end
document.xpath(xpath)
filtered_nodes
end
end
end
......@@ -16,6 +16,11 @@ module Banzai
processor.process(html_documents)
end
def reset_memoized_values
@html_documents = nil
@texts_and_contexts = []
end
private
def html_documents
......
......@@ -33,7 +33,7 @@ module Banzai
# they have access to.
class BaseParser
class << self
attr_accessor :reference_type
attr_accessor :reference_type, :reference_options
end
# Returns the attribute name containing the value for every object to be
......@@ -182,9 +182,10 @@ module Banzai
# the references.
def process(documents)
type = self.class.reference_type
reference_options = self.class.reference_options
nodes = documents.flat_map do |document|
Querying.css(document, "a[data-reference-type='#{type}'].gfm").to_a
Querying.css(document, "a[data-reference-type='#{type}'].gfm", reference_options).to_a
end
gather_references(nodes)
......
module Banzai
module ReferenceParser
class DirectlyAddressedUserParser < UserParser
self.reference_type = :user
self.reference_options = { location: :beginning }
end
end
end
module Gitlab
# Extract possible GFM references from an arbitrary String for further processing.
class ReferenceExtractor < Banzai::ReferenceExtractor
REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range)
REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range directly_addressed_user)
attr_accessor :project, :current_user, :author
def initialize(project, current_user = nil)
@project = project
@current_user = current_user
@references = {}
super()
......@@ -21,6 +20,11 @@ module Gitlab
super(type, project, current_user)
end
def reset_memoized_values
@references = {}
super()
end
REFERABLES.each do |type|
define_method("#{type}s") do
@references[type] ||= references(type)
......
......@@ -14,6 +14,10 @@ FactoryGirl.define do
action { Todo::MENTIONED }
end
trait :directly_addressed do
action { Todo::DIRECTLY_ADDRESSED }
end
trait :on_commit do
commit_id RepoHelpers.sample_commit.id
target_type "Commit"
......
......@@ -42,14 +42,85 @@ describe Gitlab::ReferenceExtractor, lib: true do
> @offteam
})
expect(subject.users).to match_array([])
end
describe 'directly addressed users' do
before do
@u_foo = create(:user, username: 'foo')
@u_foo2 = create(:user, username: 'foo2')
@u_foo3 = create(:user, username: 'foo3')
@u_foo4 = create(:user, username: 'foo4')
@u_foo5 = create(:user, username: 'foo5')
@u_bar = create(:user, username: 'bar')
@u_bar2 = create(:user, username: 'bar2')
@u_bar3 = create(:user, username: 'bar3')
@u_bar4 = create(:user, username: 'bar4')
@u_tom = create(:user, username: 'tom')
@u_tom2 = create(:user, username: 'tom2')
end
context 'when a user is directly addressed' do
it 'accesses the user object which is mentioned in the beginning of the line' do
subject.analyze('@foo What do you think? cc: @bar, @tom')
expect(subject.directly_addressed_users).to match_array([@u_foo])
end
it "doesn't access the user object if it's not mentioned in the beginning of the line" do
subject.analyze('What do you think? cc: @bar')
expect(subject.directly_addressed_users).to be_empty
end
end
context 'when multiple users are addressed' do
it 'accesses the user objects which are mentioned in the beginning of the line' do
subject.analyze('@foo @bar What do you think? cc: @tom')
expect(subject.directly_addressed_users).to match_array([@u_foo, @u_bar])
end
it "doesn't access the user objects if they are not mentioned in the beginning of the line" do
subject.analyze('What do you think? cc: @foo @bar @tom')
expect(subject.directly_addressed_users).to be_empty
end
end
context 'when multiple users are addressed in different paragraphs' do
it 'accesses user objects which are mentioned in the beginning of each paragraph' do
subject.analyze <<-NOTE.strip_heredoc
@foo What do you think? cc: @tom
- @bar can you please have a look?
>>>
@foo2 what do you think? cc: @bar2
>>>
@foo3 @foo4 thank you!
> @foo5 well done!
1. @bar3 Can you please check? cc: @tom2
2. @bar4 What do you this of this MR?
NOTE
expect(subject.directly_addressed_users).to match_array([@u_foo, @u_foo3, @u_foo4])
end
end
end
it 'accesses valid issue objects' do
@i0 = create(:issue, project: project)
@i1 = create(:issue, project: project)
subject.analyze("#{@i0.to_reference}, #{@i1.to_reference}, and #{Issue.reference_prefix}999.")
expect(subject.issues).to match_array([@i0, @i1])
end
......@@ -58,6 +129,7 @@ describe Gitlab::ReferenceExtractor, lib: true do
@m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'feature_conflict')
subject.analyze("!999, !#{@m1.iid}, and !#{@m0.iid}.")
expect(subject.merge_requests).to match_array([@m1, @m0])
end
......@@ -67,6 +139,7 @@ describe Gitlab::ReferenceExtractor, lib: true do
@l2 = create(:label)
subject.analyze("~#{@l0.id}, ~999, ~#{@l2.id}, ~#{@l1.id}")
expect(subject.labels).to match_array([@l0, @l1])
end
......@@ -76,6 +149,7 @@ describe Gitlab::ReferenceExtractor, lib: true do
@s2 = create(:project_snippet)
subject.analyze("$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}")
expect(subject.snippets).to match_array([@s0, @s1])
end
......@@ -127,6 +201,7 @@ describe Gitlab::ReferenceExtractor, lib: true do
it 'handles project issue references' do
subject.analyze("this refers issue #{issue.to_reference(project)}")
extracted = subject.issues
expect(extracted.size).to eq(1)
expect(extracted).to match_array([issue])
......
This diff is collapsed.
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