Commit 52feefb8e196cae0774292cce274e068356aec21
Exists in
master
and in
4 other branches
Merge remote-tracking branch 'upstream/master'
Showing
11 changed files
with
88 additions
and
14 deletions
Show diff stats
CONTRIBUTING.md
@@ -36,7 +36,7 @@ We welcome pull requests with fixes and improvements to GitLab code, tests, and/ | @@ -36,7 +36,7 @@ We welcome pull requests with fixes and improvements to GitLab code, tests, and/ | ||
36 | 36 | ||
37 | ### Pull request guidelines | 37 | ### Pull request guidelines |
38 | 38 | ||
39 | -If you can, please submit a pull request with the fix or improvements including tests. If you don't know how to fix the issue but can write a test that exposes the issue we will accept that as well. The workflow to make a pull request is as follows: | 39 | +If you can, please submit a pull request with the fix or improvements including tests. If you don't know how to fix the issue but can write a test that exposes the issue we will accept that as well. In general bug fixes that include a regression test are merged quickly while new features without proper tests are least likely to receive timely feedback. The workflow to make a pull request is as follows: |
40 | 40 | ||
41 | 1. Fork the project on GitHub | 41 | 1. Fork the project on GitHub |
42 | 1. Create a feature branch | 42 | 1. Create a feature branch |
@@ -51,10 +51,11 @@ We will accept pull requests if: | @@ -51,10 +51,11 @@ We will accept pull requests if: | ||
51 | * The code has proper tests and all tests pass (or it is a test exposing a failure in existing code) | 51 | * The code has proper tests and all tests pass (or it is a test exposing a failure in existing code) |
52 | * It can be merged without problems (if not please use: `git rebase master`) | 52 | * It can be merged without problems (if not please use: `git rebase master`) |
53 | * It doesn't break any existing functionality | 53 | * It doesn't break any existing functionality |
54 | -* It's quality code that conforms to the [Rails style guide](https://github.com/bbatsov/rails-style-guide) and best practices | 54 | +* It's quality code that conforms to the [Ruby](https://github.com/bbatsov/ruby-style-guide) and [Rails](https://github.com/bbatsov/rails-style-guide) style guides and best practices |
55 | * The description includes a motive for your change and the method you used to achieve it | 55 | * The description includes a motive for your change and the method you used to achieve it |
56 | * It keeps the GitLab code base clean and well structured | 56 | * It keeps the GitLab code base clean and well structured |
57 | * We think other users will benefit from the same functionality | 57 | * We think other users will benefit from the same functionality |
58 | * If it makes changes to the UI the pull request should include screenshots | 58 | * If it makes changes to the UI the pull request should include screenshots |
59 | +* It is a single commit (please use git rebase -i to squash commits) | ||
59 | 60 | ||
60 | For examples of feedback on pull requests please look at already [closed pull requests](https://github.com/gitlabhq/gitlabhq/pulls?direction=desc&page=1&sort=created&state=closed). | 61 | For examples of feedback on pull requests please look at already [closed pull requests](https://github.com/gitlabhq/gitlabhq/pulls?direction=desc&page=1&sort=created&state=closed). |
README.md
@@ -2,6 +2,8 @@ | @@ -2,6 +2,8 @@ | ||
2 | 2 | ||
3 |  | 3 |  |
4 | 4 | ||
5 | + | ||
6 | + | ||
5 | ### GitLab allows you to | 7 | ### GitLab allows you to |
6 | * keep your code secure on your own server | 8 | * keep your code secure on your own server |
7 | * manage repositories, users and access permissions | 9 | * manage repositories, users and access permissions |
@@ -22,7 +24,7 @@ | @@ -22,7 +24,7 @@ | ||
22 | 24 | ||
23 | * [](https://codeclimate.com/github/gitlabhq/gitlabhq) | 25 | * [](https://codeclimate.com/github/gitlabhq/gitlabhq) |
24 | 26 | ||
25 | -* [](https://gemnasium.com/gitlabhq/gitlabhq) this button can be yellow (small updates are available) but must not be red (a security fix or an important update is available) | 27 | +* [](https://gemnasium.com/gitlabhq/gitlabhq) this button can be yellow (small updates are available) but must not be red (a security fix or an important update is available), gems are updated in major releases of GitLab. |
26 | 28 | ||
27 | * [](https://coveralls.io/r/gitlabhq/gitlabhq) | 29 | * [](https://coveralls.io/r/gitlabhq/gitlabhq) |
28 | 30 |
app/assets/javascripts/dispatcher.js.coffee
1 | $ -> | 1 | $ -> |
2 | new Dispatcher() | 2 | new Dispatcher() |
3 | - | 3 | + |
4 | class Dispatcher | 4 | class Dispatcher |
5 | constructor: () -> | 5 | constructor: () -> |
6 | @initSearch() | 6 | @initSearch() |
@@ -10,8 +10,6 @@ class Dispatcher | @@ -10,8 +10,6 @@ class Dispatcher | ||
10 | page = $('body').attr('data-page') | 10 | page = $('body').attr('data-page') |
11 | project_id = $('body').attr('data-project-id') | 11 | project_id = $('body').attr('data-project-id') |
12 | 12 | ||
13 | - console.log(page) | ||
14 | - | ||
15 | unless page | 13 | unless page |
16 | return false | 14 | return false |
17 | 15 |
app/assets/stylesheets/sections/issues.scss
app/helpers/projects_helper.rb
@@ -17,7 +17,7 @@ module ProjectsHelper | @@ -17,7 +17,7 @@ module ProjectsHelper | ||
17 | end | 17 | end |
18 | 18 | ||
19 | def link_to_member(project, author, opts = {}) | 19 | def link_to_member(project, author, opts = {}) |
20 | - default_opts = { avatar: true } | 20 | + default_opts = { avatar: true, name: true, size: 16 } |
21 | opts = default_opts.merge(opts) | 21 | opts = default_opts.merge(opts) |
22 | 22 | ||
23 | return "(deleted)" unless author | 23 | return "(deleted)" unless author |
@@ -25,10 +25,10 @@ module ProjectsHelper | @@ -25,10 +25,10 @@ module ProjectsHelper | ||
25 | author_html = "" | 25 | author_html = "" |
26 | 26 | ||
27 | # Build avatar image tag | 27 | # Build avatar image tag |
28 | - author_html << image_tag(gravatar_icon(author.try(:email)), width: 16, class: "avatar avatar-inline s16") if opts[:avatar] | 28 | + author_html << image_tag(gravatar_icon(author.try(:email), opts[:size]), width: opts[:size], class: "avatar avatar-inline #{"s#{opts[:size]}" if opts[:size]}") if opts[:avatar] |
29 | 29 | ||
30 | # Build name span tag | 30 | # Build name span tag |
31 | - author_html << content_tag(:span, sanitize(author.name), class: 'author') | 31 | + author_html << content_tag(:span, sanitize(author.name), class: 'author') if opts[:name] |
32 | 32 | ||
33 | author_html = author_html.html_safe | 33 | author_html = author_html.html_safe |
34 | 34 |
app/models/concerns/issuable.rb
@@ -6,6 +6,7 @@ | @@ -6,6 +6,7 @@ | ||
6 | # | 6 | # |
7 | module Issuable | 7 | module Issuable |
8 | extend ActiveSupport::Concern | 8 | extend ActiveSupport::Concern |
9 | + include Mentionable | ||
9 | 10 | ||
10 | included do | 11 | included do |
11 | belongs_to :project | 12 | belongs_to :project |
@@ -97,4 +98,18 @@ module Issuable | @@ -97,4 +98,18 @@ module Issuable | ||
97 | def votes_count | 98 | def votes_count |
98 | upvotes + downvotes | 99 | upvotes + downvotes |
99 | end | 100 | end |
101 | + | ||
102 | + # Return all users participating on the discussion | ||
103 | + def participants | ||
104 | + users = [] | ||
105 | + users << author | ||
106 | + users << assignee if is_assigned? | ||
107 | + mentions = [] | ||
108 | + mentions << self.mentioned_users | ||
109 | + notes.each do |note| | ||
110 | + users << note.author | ||
111 | + mentions << note.mentioned_users | ||
112 | + end | ||
113 | + users.concat(mentions.reduce([], :|)).uniq | ||
114 | + end | ||
100 | end | 115 | end |
@@ -0,0 +1,37 @@ | @@ -0,0 +1,37 @@ | ||
1 | +# == Mentionable concern | ||
2 | +# | ||
3 | +# Contains common functionality shared between Issues and Notes | ||
4 | +# | ||
5 | +# Used by Issue, Note | ||
6 | +# | ||
7 | +module Mentionable | ||
8 | + extend ActiveSupport::Concern | ||
9 | + | ||
10 | + def mentioned_users | ||
11 | + users = [] | ||
12 | + return users if mentionable_text.blank? | ||
13 | + has_project = self.respond_to? :project | ||
14 | + matches = mentionable_text.scan(/@[a-zA-Z][a-zA-Z0-9_\-\.]*/) | ||
15 | + matches.each do |match| | ||
16 | + identifier = match.delete "@" | ||
17 | + if has_project | ||
18 | + id = project.users_projects.joins(:user).where(users: { username: identifier }).pluck(:user_id).first | ||
19 | + else | ||
20 | + id = User.where(username: identifier).pluck(:id).first | ||
21 | + end | ||
22 | + users << User.find(id) unless id.blank? | ||
23 | + end | ||
24 | + users.uniq | ||
25 | + end | ||
26 | + | ||
27 | + def mentionable_text | ||
28 | + if self.class == Issue | ||
29 | + description | ||
30 | + elsif self.class == Note | ||
31 | + note | ||
32 | + else | ||
33 | + nil | ||
34 | + end | ||
35 | + end | ||
36 | + | ||
37 | +end |
app/models/note.rb
@@ -19,6 +19,8 @@ require 'carrierwave/orm/activerecord' | @@ -19,6 +19,8 @@ require 'carrierwave/orm/activerecord' | ||
19 | require 'file_size_validator' | 19 | require 'file_size_validator' |
20 | 20 | ||
21 | class Note < ActiveRecord::Base | 21 | class Note < ActiveRecord::Base |
22 | + include Mentionable | ||
23 | + | ||
22 | attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, | 24 | attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, |
23 | :attachment, :line_code, :commit_id | 25 | :attachment, :line_code, :commit_id |
24 | 26 |
app/services/notification_service.rb
@@ -110,9 +110,11 @@ class NotificationService | @@ -110,9 +110,11 @@ class NotificationService | ||
110 | else | 110 | else |
111 | opts.merge!(noteable_id: note.noteable_id) | 111 | opts.merge!(noteable_id: note.noteable_id) |
112 | target = note.noteable | 112 | target = note.noteable |
113 | - recipients = [] | ||
114 | - recipients << target.assignee if target.respond_to?(:assignee) | ||
115 | - recipients << target.author if target.respond_to?(:author) | 113 | + if target.respond_to?(:participants) |
114 | + recipients = target.participants | ||
115 | + else | ||
116 | + recipients = [] | ||
117 | + end | ||
116 | end | 118 | end |
117 | 119 | ||
118 | # Get users who left comment in thread | 120 | # Get users who left comment in thread |
@@ -181,7 +183,12 @@ class NotificationService | @@ -181,7 +183,12 @@ class NotificationService | ||
181 | end | 183 | end |
182 | 184 | ||
183 | def new_resource_email(target, method) | 185 | def new_resource_email(target, method) |
184 | - recipients = reject_muted_users([target.assignee], target.project) | 186 | + if target.respond_to?(:participants) |
187 | + recipients = target.participants | ||
188 | + else | ||
189 | + recipients = [] | ||
190 | + end | ||
191 | + recipients = reject_muted_users(recipients, target.project) | ||
185 | recipients = recipients.concat(project_watchers(target.project)).uniq | 192 | recipients = recipients.concat(project_watchers(target.project)).uniq |
186 | recipients.delete(target.author) | 193 | recipients.delete(target.author) |
187 | 194 |
app/views/issues/show.html.haml
@@ -65,4 +65,9 @@ | @@ -65,4 +65,9 @@ | ||
65 | - else | 65 | - else |
66 | = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue" | 66 | = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue" |
67 | 67 | ||
68 | +.participants | ||
69 | + %cite.cgray #{@issue.participants.count} participants | ||
70 | + - @issue.participants.each do |participant| | ||
71 | + = link_to_member(@project, participant, name: false, size: 24) | ||
72 | + | ||
68 | .voting_notes#notes= render "notes/notes_with_form" | 73 | .voting_notes#notes= render "notes/notes_with_form" |
spec/services/notification_service_spec.rb
@@ -19,7 +19,7 @@ describe NotificationService do | @@ -19,7 +19,7 @@ describe NotificationService do | ||
19 | describe 'Notes' do | 19 | describe 'Notes' do |
20 | context 'issue note' do | 20 | context 'issue note' do |
21 | let(:issue) { create(:issue, assignee: create(:user)) } | 21 | let(:issue) { create(:issue, assignee: create(:user)) } |
22 | - let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id) } | 22 | + let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced') } |
23 | 23 | ||
24 | before do | 24 | before do |
25 | build_team(note.project) | 25 | build_team(note.project) |
@@ -30,6 +30,7 @@ describe NotificationService do | @@ -30,6 +30,7 @@ describe NotificationService do | ||
30 | should_email(@u_watcher.id) | 30 | should_email(@u_watcher.id) |
31 | should_email(note.noteable.author_id) | 31 | should_email(note.noteable.author_id) |
32 | should_email(note.noteable.assignee_id) | 32 | should_email(note.noteable.assignee_id) |
33 | + should_email(@u_mentioned.id) | ||
33 | should_not_email(note.author_id) | 34 | should_not_email(note.author_id) |
34 | should_not_email(@u_participating.id) | 35 | should_not_email(@u_participating.id) |
35 | should_not_email(@u_disabled.id) | 36 | should_not_email(@u_disabled.id) |
@@ -235,9 +236,11 @@ describe NotificationService do | @@ -235,9 +236,11 @@ describe NotificationService do | ||
235 | @u_watcher = create(:user, notification_level: Notification::N_WATCH) | 236 | @u_watcher = create(:user, notification_level: Notification::N_WATCH) |
236 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) | 237 | @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) |
237 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) | 238 | @u_disabled = create(:user, notification_level: Notification::N_DISABLED) |
239 | + @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_WATCH) | ||
238 | 240 | ||
239 | project.team << [@u_watcher, :master] | 241 | project.team << [@u_watcher, :master] |
240 | project.team << [@u_participating, :master] | 242 | project.team << [@u_participating, :master] |
241 | project.team << [@u_disabled, :master] | 243 | project.team << [@u_disabled, :master] |
244 | + project.team << [@u_mentioned, :master] | ||
242 | end | 245 | end |
243 | end | 246 | end |