From fa9004c78cec9c7b3410caf1308c0cd8c4bc266f Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 30 Jul 2015 15:29:43 +0200 Subject: Send notification to all participants when MR is merged. --- CHANGELOG | 1 + app/services/notification_service.rb | 49 ++++++------------------------ spec/services/notification_service_spec.rb | 7 ++++- 3 files changed, 16 insertions(+), 41 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index c03e5053d17..9e0d96c5b7d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,7 @@ v 7.14.0 (unreleased) - Add fetch command to the MR page - Fix bug causing Bitbucket importer to crash when OAuth application had been removed. - Add fetch command to the MR page. + - Send notification to all participants when MR is merged. v 7.13.2 - Fix randomly failed spec diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 312b56eb87b..3735a136365 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -70,12 +70,6 @@ class NotificationService reassign_resource_email(merge_request, merge_request.target_project, current_user, 'reassigned_merge_request_email') end - # When we close a merge request we should send next emails: - # - # * merge_request author if their notification level is not Disabled - # * merge_request assignee if their notification level is not Disabled - # * project team members with notification level higher then Participating - # def close_mr(merge_request, current_user) close_resource_email(merge_request, merge_request.target_project, current_user, 'closed_merge_request_email') end @@ -84,26 +78,8 @@ class NotificationService reopen_resource_email(issue, issue.project, current_user, 'issue_status_changed_email', 'reopened') end - # When we merge a merge request we should send next emails: - # - # * merge_request author if their notification level is not Disabled - # * merge_request assignee if their notification level is not Disabled - # * project team members with notification level higher then Participating - # def merge_mr(merge_request, current_user) - recipients = [merge_request.author, merge_request.assignee] - - recipients = add_project_watchers(recipients, merge_request.target_project) - recipients = reject_muted_users(recipients, merge_request.target_project) - - recipients = add_subscribed_users(recipients, merge_request) - recipients = reject_unsubscribed_users(recipients, merge_request) - - recipients.delete(current_user) - - recipients.each do |recipient| - mailer.merged_merge_request_email(recipient.id, merge_request.id, current_user.id) - end + close_resource_email(merge_request, merge_request.target_project, current_user, 'merged_merge_request_email') end def reopen_mr(merge_request, current_user) @@ -364,8 +340,7 @@ class NotificationService end def new_resource_email(target, project, method) - recipients = build_recipients(target, project) - recipients.delete(target.author) + recipients = build_recipients(target, project, target.author) recipients.each do |recipient| mailer.send(method, recipient.id, target.id) @@ -373,8 +348,7 @@ class NotificationService end def close_resource_email(target, project, current_user, method) - recipients = build_recipients(target, project) - recipients.delete(current_user) + recipients = build_recipients(target, project, current_user) recipients.each do |recipient| mailer.send(method, recipient.id, target.id, current_user.id) @@ -383,8 +357,7 @@ class NotificationService def reassign_resource_email(target, project, current_user, method) assignee_id_was = previous_record(target, "assignee_id") - recipients = build_recipients(target, project) - recipients.delete(current_user) + recipients = build_recipients(target, project, current_user) recipients.each do |recipient| mailer.send(method, recipient.id, target.id, assignee_id_was, current_user.id) @@ -392,21 +365,15 @@ class NotificationService end def reopen_resource_email(target, project, current_user, method, status) - recipients = build_recipients(target, project) - recipients.delete(current_user) + recipients = build_recipients(target, project, current_user) recipients.each do |recipient| mailer.send(method, recipient.id, target.id, status, current_user.id) end end - def build_recipients(target, project) - recipients = - if target.respond_to?(:participants) - target.participants - else - [target.author, target.assignee] - end + def build_recipients(target, project, current_user) + recipients = target.participants(current_user) recipients = add_project_watchers(recipients, project) recipients = reject_mention_users(recipients, project) @@ -415,6 +382,8 @@ class NotificationService recipients = add_subscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target) + recipients.delete(current_user) + recipients end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 253e5823499..9da6c9dc949 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -300,7 +300,7 @@ describe NotificationService do describe 'Merge Requests' do let(:project) { create(:project, :public) } - let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user) } + let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' } before do build_team(merge_request.target_project) @@ -311,6 +311,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) notification.new_merge_request(merge_request, @u_disabled) @@ -329,6 +330,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) @@ -349,6 +351,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) @@ -369,6 +372,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) @@ -389,6 +393,7 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@u_participant_mentioned.id) should_email(@subscriber.id) should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) -- cgit v1.2.1 From c6799b0e1ecd1b7373f0ea23195e18c7ad5c199d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 6 Aug 2015 12:16:05 +0200 Subject: Update notification docs --- doc/workflow/notifications.md | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md index 2b5f06dd1fa..025992deece 100644 --- a/doc/workflow/notifications.md +++ b/doc/workflow/notifications.md @@ -52,18 +52,35 @@ Below is the table of events users can be notified of: | New SSH key added | User | Security email, always sent. | | New email added | User | Security email, always sent. | | New user created | User | Sent on user creation, except for omniauth (LDAP)| -| New issue created | Issue assignee [1], project members [2] | [1] not disabled, [2] higher than participating | | User added to project | User | Sent when user is added to project | | Project access level changed | User | Sent when user project access level is changed | | User added to group | User | Sent when user is added to group | -| Project moved | Project members [1] | [1] not disabled | | Group access level changed | User | Sent when user group access level is changed | -| Close issue | Issue author [1], issue assignee [2], project members [3] | [1] [2] not disabled, [3] higher than participating | -| Reassign issue | New issue assignee [1], old issue assignee [2] | [1] [2] not disabled | -| Reopen issue | Project members [1] | [1] higher than participating | -| New merge request | MR assignee [1] | [1] not disabled | -| Reassign merge request | New MR assignee [1], old MR assignee [2] | [1] [2] not disabled | -| Close merge request | MR author [1], MR assignee [2], project members [3] | [1] [2] not disabled, [3] higher than participating | -| Reopen merge request | Project members [1] | [1] higher than participating | -| Merge merge request | MR author [1], MR assignee [2], project members [3] | [1] [2] not disabled, [3] higher than participating | -| New comment | Mentioned users [1], users participating [2], project members [3] | [1] [2] not disabled, [3] higher than participating | +| Project moved | Project members [1] | [1] not disabled | + +### Issue / Merge Request events + +In all of the below cases, the notification will be sent to: +- Participants: + - the author and assignee of the issue/merge request + - authors of comments on the issue/merge request + - anyone mentioned by `@username` in the issue/merge request description + - anyone mentioned by `@username` in any of the comments on the issue/merge request + + ...with notification level "Participating" or higher + +- Watchers: project members with notification level "Watch" +- Subscribers: anyone who manually subscribed to the issue/merge request + +| Event | Sent to | +|------------------------|---------| +| New issue | | +| Close issue | | +| Reassign issue | The above, plus the old assignee | +| Reopen issue | | +| New merge request | | +| Reassign merge request | The above, plus the old assignee | +| Close merge request | | +| Reopen merge request | | +| Merge merge request | | +| New comment | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher | -- cgit v1.2.1 From c015f96ba44e80df5d89c6311c7804b439425280 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 6 Aug 2015 13:15:33 +0200 Subject: Cache all events --- CHANGELOG | 1 + app/views/events/_event.html.haml | 10 ++++------ app/views/events/event/_created_project.html.haml | 4 ++-- app/views/events/event/_push.html.haml | 11 ++++++----- app/views/layouts/_head.html.haml | 4 +++- app/views/layouts/_user_styles.html.haml | 16 ++++++++++++++++ 6 files changed, 32 insertions(+), 14 deletions(-) create mode 100644 app/views/layouts/_user_styles.html.haml diff --git a/CHANGELOG b/CHANGELOG index 439534c37d9..4473f0637fd 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -37,6 +37,7 @@ v 7.14.0 (unreleased) - Fetch code from forks to refs/merge-requests/:id/head when merge request created - Remove satellites - Remove comments and email addresses when publicly exposing ssh keys (Zeger-Jan van de Weg) + - Cache all events v 7.13.2 - Fix randomly failed spec diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml index 5ab5ffc238c..0377760a9b8 100644 --- a/app/views/events/_event.html.haml +++ b/app/views/events/_event.html.haml @@ -3,13 +3,11 @@ .event-item-timestamp #{time_ago_with_tooltip(event.created_at)} - - if event.created_project? - = cache [event, current_user] do - = image_tag avatar_icon(event.author_email, 24), class: "avatar s24", alt:'' - = render "events/event/created_project", event: event - - else + = cache event, "v1" do = image_tag avatar_icon(event.author_email, 24), class: "avatar s24", alt:'' - - if event.push? + - if event.created_project? + = render "events/event/created_project", event: event + - elsif event.push? = render "events/event/push", event: event - elsif event.commented? = render "events/event/note", event: event diff --git a/app/views/events/event/_created_project.html.haml b/app/views/events/event/_created_project.html.haml index c2577a24982..8cf36c711b4 100644 --- a/app/views/events/event/_created_project.html.haml +++ b/app/views/events/event/_created_project.html.haml @@ -8,8 +8,8 @@ - else = event.project_name -- if current_user == event.author && !event.project.private? && twitter_sharing_enabled? - .event-body +- if !event.project.private? && twitter_sharing_enabled? + .event-body{"data-user-is" => event.author_id} .event-note .md %p diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 34a7c00dc43..4de3e66962b 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -17,7 +17,7 @@ - few_commits.each do |commit| = render "events/commit", commit: commit, project: project - - create_mr = current_user == event.author && event.new_ref? && create_mr_button?(event.project.default_branch, event.ref_name, event.project) + - create_mr = event.new_ref? && create_mr_button?(event.project.default_branch, event.ref_name, event.project) - if event.commits_count > 1 %li.commits-stat - if event.commits_count > 2 @@ -34,10 +34,11 @@ Compare #{from_label}...#{truncate_sha(event.commit_to)} - if create_mr - or - = link_to create_mr_path(event.project.default_branch, event.ref_name, event.project) do - create a merge request + %span{"data-user-is" => event.author_id} + or + = link_to create_mr_path(event.project.default_branch, event.ref_name, event.project) do + create a merge request - elsif create_mr - %li.commits-stat + %li.commits-stat{"data-user-is" => event.author_id} = link_to create_mr_path(event.project.default_branch, event.ref_name, event.project) do Create Merge Request diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index 54cddc30b74..397649dacf8 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -27,7 +27,7 @@ = favicon_link_tag 'touch-icon-ipad-retina.png', rel: 'apple-touch-icon', sizes: '152x152' -# Windows 8 pinned site tile - %meta{name: 'msapplication-TileImage', content: image_url('msapplication-tile.png')} + %meta{name: 'msapplication-TileImage', content: image_path('msapplication-tile.png')} %meta{name: 'msapplication-TileColor', content: '#30353E'} = yield :meta_tags @@ -35,3 +35,5 @@ = render 'layouts/google_analytics' if extra_config.has_key?('google_analytics_id') = render 'layouts/piwik' if extra_config.has_key?('piwik_url') && extra_config.has_key?('piwik_site_id') = render 'layouts/bootlint' if Rails.env.development? + + = render 'layouts/user_styles' diff --git a/app/views/layouts/_user_styles.html.haml b/app/views/layouts/_user_styles.html.haml new file mode 100644 index 00000000000..b7f0f316724 --- /dev/null +++ b/app/views/layouts/_user_styles.html.haml @@ -0,0 +1,16 @@ +:css + [data-user-is] { + display: none !important; + } + + [data-user-is="#{current_user.try(:id)}"] { + display: block !important; + } + + [data-user-is-not] { + display: block !important; + } + + [data-user-is-not="#{current_user.try(:id)}"] { + display: none !important; + } -- cgit v1.2.1 From 9bd074d4cf6b81328921c03c93a385b703da71c9 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 6 Aug 2015 14:55:03 +0200 Subject: Use display: inline where appropriate --- app/views/events/event/_push.html.haml | 2 +- app/views/layouts/_user_styles.html.haml | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 4de3e66962b..8bed5cdb9cc 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -34,7 +34,7 @@ Compare #{from_label}...#{truncate_sha(event.commit_to)} - if create_mr - %span{"data-user-is" => event.author_id} + %span{"data-user-is" => event.author_id, "data-display" => "inline"} or = link_to create_mr_path(event.project.default_branch, event.ref_name, event.project) do create a merge request diff --git a/app/views/layouts/_user_styles.html.haml b/app/views/layouts/_user_styles.html.haml index b7f0f316724..b76b3cb5510 100644 --- a/app/views/layouts/_user_styles.html.haml +++ b/app/views/layouts/_user_styles.html.haml @@ -7,10 +7,18 @@ display: block !important; } + [data-user-is="#{current_user.try(:id)}"][data-display="inline"] { + display: inline !important; + } + [data-user-is-not] { display: block !important; } + [data-user-is-not][data-display="inline"] { + display: inline !important; + } + [data-user-is-not="#{current_user.try(:id)}"] { display: none !important; } -- cgit v1.2.1 From 8ab6bd3eb3b49c6ca557b3053105de81af31ca51 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Fri, 7 Aug 2015 22:13:18 +0300 Subject: Do a git checkout on Gemfile.lock and schema.rb prior to update. Fixes #2227 --- doc/update/patch_versions.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/update/patch_versions.md b/doc/update/patch_versions.md index e29ee2a7b3d..22b9be059d6 100644 --- a/doc/update/patch_versions.md +++ b/doc/update/patch_versions.md @@ -22,6 +22,7 @@ sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production ```bash cd /home/git/gitlab sudo -u git -H git fetch --all +sudo -u git -H git checkout -- Gemfile.lock db/schema.rb sudo -u git -H git checkout LATEST_TAG ``` -- cgit v1.2.1 From 3f045b25d7e290953fb668100928b882e00fb7bd Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Sat, 8 Aug 2015 22:02:40 -0400 Subject: Fix transparency for msapplication-tile image [ci skip] --- app/assets/images/msapplication-tile.png | Bin 6102 -> 5798 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/app/assets/images/msapplication-tile.png b/app/assets/images/msapplication-tile.png index f8c5c8b28b4..58bbf2b20cb 100644 Binary files a/app/assets/images/msapplication-tile.png and b/app/assets/images/msapplication-tile.png differ -- cgit v1.2.1