From 862fbb62b28a67c6fbc66a593cc296fd5f453ee1 Mon Sep 17 00:00:00 2001 From: Sytse Sijbrandij Date: Sun, 23 Jun 2013 21:54:24 +0200 Subject: [PATCH 1/5] Add animated screenshots. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 600fcff6d4..36afa435c7 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,8 @@ ![logo](https://raw.github.com/gitlabhq/gitlabhq/master/public/gitlab_logo.png) +![animated-screenshots](http://makeagif.com/media/6-23-2013/AN3Mo6.gif) + ### GitLab allows you to * keep your code secure on your own server * manage repositories, users and access permissions From 04e77826ba6dc496a045f49cc83f3cc1d0dda11b Mon Sep 17 00:00:00 2001 From: Sytse Sijbrandij Date: Mon, 24 Jun 2013 09:20:08 +0200 Subject: [PATCH 2/5] Notes about what PR's we would like to see. --- CONTRIBUTING.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9084d99db3..a0f9ce2d0d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,7 +36,7 @@ We welcome pull requests with fixes and improvements to GitLab code, tests, and/ ### Pull request guidelines -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: +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: 1. Fork the project on GitHub 1. Create a feature branch @@ -51,10 +51,11 @@ We will accept pull requests if: * The code has proper tests and all tests pass (or it is a test exposing a failure in existing code) * It can be merged without problems (if not please use: `git rebase master`) * It doesn't break any existing functionality -* It's quality code that conforms to the [Rails style guide](https://github.com/bbatsov/rails-style-guide) and best practices +* 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 * The description includes a motive for your change and the method you used to achieve it * It keeps the GitLab code base clean and well structured * We think other users will benefit from the same functionality * If it makes changes to the UI the pull request should include screenshots +* It is a single commit (please use git rebase -i to squash commits) 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). From cf35d19c2ef6afd04b7339e5425f2f4a015e6acf Mon Sep 17 00:00:00 2001 From: Javier Castro Date: Thu, 9 May 2013 20:37:47 -0300 Subject: [PATCH 3/5] Show participants on issues, including mentions, and notify email to all participants --- app/assets/stylesheets/sections/issues.scss | 4 +++ app/helpers/projects_helper.rb | 6 ++-- app/models/concerns/issuable.rb | 15 +++++++++ app/models/concerns/mentionable.rb | 37 +++++++++++++++++++++ app/models/note.rb | 2 ++ app/services/notification_service.rb | 15 ++++++--- app/views/issues/show.html.haml | 5 +++ spec/services/notification_service_spec.rb | 5 ++- 8 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 app/models/concerns/mentionable.rb diff --git a/app/assets/stylesheets/sections/issues.scss b/app/assets/stylesheets/sections/issues.scss index 5a1b476fe2..ed7902fec3 100644 --- a/app/assets/stylesheets/sections/issues.scss +++ b/app/assets/stylesheets/sections/issues.scss @@ -106,3 +106,7 @@ input.check_all_issues { #update_status { width: 100px; } + +.participants { + margin-bottom: 10px; +} diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 878b091376..39290ec05b 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -17,7 +17,7 @@ module ProjectsHelper end def link_to_member(project, author, opts = {}) - default_opts = { avatar: true } + default_opts = { avatar: true, name: true, size: 16 } opts = default_opts.merge(opts) return "(deleted)" unless author @@ -25,10 +25,10 @@ module ProjectsHelper author_html = "" # Build avatar image tag - author_html << image_tag(gravatar_icon(author.try(:email)), width: 16, class: "avatar avatar-inline s16") if opts[:avatar] + 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] # Build name span tag - author_html << content_tag(:span, sanitize(author.name), class: 'author') + author_html << content_tag(:span, sanitize(author.name), class: 'author') if opts[:name] author_html = author_html.html_safe diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 11e3d8eed1..3844085906 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -6,6 +6,7 @@ # module Issuable extend ActiveSupport::Concern + include Mentionable included do belongs_to :project @@ -97,4 +98,18 @@ module Issuable def votes_count upvotes + downvotes end + + # Return all users participating on the discussion + def participants + users = [] + users << author + users << assignee if is_assigned? + mentions = [] + mentions << self.mentioned_users + notes.each do |note| + users << note.author + mentions << note.mentioned_users + end + users.concat(mentions.reduce([], :|)).uniq + end end diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb new file mode 100644 index 0000000000..f22070f850 --- /dev/null +++ b/app/models/concerns/mentionable.rb @@ -0,0 +1,37 @@ +# == Mentionable concern +# +# Contains common functionality shared between Issues and Notes +# +# Used by Issue, Note +# +module Mentionable + extend ActiveSupport::Concern + + def mentioned_users + users = [] + return users if mentionable_text.blank? + has_project = self.respond_to? :project + matches = mentionable_text.scan(/@[a-zA-Z][a-zA-Z0-9_\-\.]*/) + matches.each do |match| + identifier = match.delete "@" + if has_project + id = project.users_projects.joins(:user).where(users: { username: identifier }).pluck(:user_id).first + else + id = User.where(username: identifier).pluck(:id).first + end + users << User.find(id) unless id.blank? + end + users.uniq + end + + def mentionable_text + if self.class == Issue + description + elsif self.class == Note + note + else + nil + end + end + +end diff --git a/app/models/note.rb b/app/models/note.rb index 9a3481faaa..56a8749e47 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -19,6 +19,8 @@ require 'carrierwave/orm/activerecord' require 'file_size_validator' class Note < ActiveRecord::Base + include Mentionable + attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, :attachment, :line_code, :commit_id diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 379d2c5462..b0243481b3 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -110,9 +110,11 @@ class NotificationService else opts.merge!(noteable_id: note.noteable_id) target = note.noteable - recipients = [] - recipients << target.assignee if target.respond_to?(:assignee) - recipients << target.author if target.respond_to?(:author) + if target.respond_to?(:participants) + recipients = target.participants + else + recipients = [] + end end # Get users who left comment in thread @@ -181,7 +183,12 @@ class NotificationService end def new_resource_email(target, method) - recipients = reject_muted_users([target.assignee], target.project) + if target.respond_to?(:participants) + recipients = target.participants + else + recipients = [] + end + recipients = reject_muted_users(recipients, target.project) recipients = recipients.concat(project_watchers(target.project)).uniq recipients.delete(target.author) diff --git a/app/views/issues/show.html.haml b/app/views/issues/show.html.haml index 2e204b8240..891a01f14c 100644 --- a/app/views/issues/show.html.haml +++ b/app/views/issues/show.html.haml @@ -65,4 +65,9 @@ - else = 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" +.participants + %cite.cgray #{@issue.participants.count} participants + - @issue.participants.each do |participant| + = link_to_member(@project, participant, name: false, size: 24) + .voting_notes#notes= render "notes/notes_with_form" diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 21e4202f4e..7650148230 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -19,7 +19,7 @@ describe NotificationService do describe 'Notes' do context 'issue note' do let(:issue) { create(:issue, assignee: create(:user)) } - let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id) } + let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced') } before do build_team(note.project) @@ -30,6 +30,7 @@ describe NotificationService do should_email(@u_watcher.id) should_email(note.noteable.author_id) should_email(note.noteable.assignee_id) + should_email(@u_mentioned.id) should_not_email(note.author_id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) @@ -235,9 +236,11 @@ describe NotificationService do @u_watcher = create(:user, notification_level: Notification::N_WATCH) @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING) @u_disabled = create(:user, notification_level: Notification::N_DISABLED) + @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_WATCH) project.team << [@u_watcher, :master] project.team << [@u_participating, :master] project.team << [@u_disabled, :master] + project.team << [@u_mentioned, :master] end end From f3a924023756ce99ee29d9c8d6cbb4c3b3e9ac26 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 24 Jun 2013 17:40:23 +0200 Subject: [PATCH 4/5] we should not use `console.log` in production code... --- app/assets/javascripts/dispatcher.js.coffee | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index fb149b7f67..130db5bfdb 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -1,6 +1,6 @@ $ -> new Dispatcher() - + class Dispatcher constructor: () -> @initSearch() @@ -10,8 +10,6 @@ class Dispatcher page = $('body').attr('data-page') project_id = $('body').attr('data-project-id') - console.log(page) - unless page return false From 32e029d2618dd657d4f8063325a87ec5e4713f15 Mon Sep 17 00:00:00 2001 From: Sytse Sijbrandij Date: Mon, 24 Jun 2013 18:54:08 +0200 Subject: [PATCH 5/5] Detail gem update policy. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 600fcff6d4..a1d0b54c5a 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ * [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.png)](https://codeclimate.com/github/gitlabhq/gitlabhq) -* [![Dependency Status](https://gemnasium.com/gitlabhq/gitlabhq.png)](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) +* [![Dependency Status](https://gemnasium.com/gitlabhq/gitlabhq.png)](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. * [![Coverage Status](https://coveralls.io/repos/gitlabhq/gitlabhq/badge.png?branch=master)](https://coveralls.io/r/gitlabhq/gitlabhq)