Merge branch 'stuartnelson3/gitlab-ce-stn/issue-due-email' into 'master'

Email notification on issue due date

Closes #27500

See merge request gitlab-org/gitlab-ce!17985

app/mailers/emails/issues.rb

@@ -6,6 +6,12 @@ module Emails
       mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id, reason))
     end
 
+    def issue_due_email(recipient_id, issue_id, reason = nil)
+      setup_issue_mail(issue_id, recipient_id)
+
+      mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id, reason))
+    end
+
     def new_mention_in_issue_email(recipient_id, issue_id, updated_by_user_id, reason = nil)
       setup_issue_mail(issue_id, recipient_id)
       mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason))

app/models/issue.rb

@@ -49,6 +49,7 @@ class Issue < ActiveRecord::Base
   scope :without_due_date, -> { where(due_date: nil) }
   scope :due_before, ->(date) { where('issues.due_date < ?', date) }
   scope :due_between, ->(from_date, to_date) { where('issues.due_date >= ?', from_date).where('issues.due_date <= ?', to_date) }
+  scope :due_tomorrow, -> { where(due_date: Date.tomorrow) }
 
   scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') }
   scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') }

app/models/notification_setting.rb

@@ -47,7 +47,8 @@ class NotificationSetting < ActiveRecord::Base
   ].freeze
 
   EXCLUDED_WATCHER_EVENTS = [
-    :push_to_merge_request
+    :push_to_merge_request,
+    :issue_due
   ].push(*EXCLUDED_PARTICIPATING_EVENTS).freeze
 
   def self.find_or_create_for(source)

app/services/notification_recipient_service.rb

@@ -203,10 +203,11 @@ module NotificationRecipientService
       attr_reader :action
       attr_reader :previous_assignee
       attr_reader :skip_current_user
-      def initialize(target, current_user, action:, previous_assignee: nil, skip_current_user: true)
+      def initialize(target, current_user, action:, custom_action: nil, previous_assignee: nil, skip_current_user: true)
         @target = target
         @current_user = current_user
         @action = action
+        @custom_action = custom_action
         @previous_assignee = previous_assignee
         @skip_current_user = skip_current_user
       end
@@ -236,7 +237,13 @@ module NotificationRecipientService
           add_mentions(current_user, target: target)
 
           # Add the assigned users, if any
-          assignees = custom_action == :new_issue ? target.assignees : target.assignee
+          assignees = case custom_action
+                      when :new_issue
+                        target.assignees
+                      else
+                        target.assignee
+                      end
+
           # We use the `:participating` notification level in order to match existing legacy behavior as captured
           # in existing specs (notification_service_spec.rb ~ line 507)
           add_recipients(assignees, :participating, NotificationReason::ASSIGNED) if assignees

app/services/notification_service.rb

@@ -373,6 +373,20 @@ class NotificationService
     end
   end
 
+  def issue_due(issue)
+    recipients = NotificationRecipientService.build_recipients(
+      issue,
+      issue.author,
+      action: 'due',
+      custom_action: :issue_due,
+      skip_current_user: false
+    )
+
+    recipients.each do |recipient|
+      mailer.send(:issue_due_email, recipient.user.id, issue.id, recipient.reason).deliver_later
+    end
+  end
+
   protected
 
   def new_resource_email(target, method)

app/views/notify/issue_due_email.html.haml

@@ -0,0 +1,12 @@
+%p.details
+  #{link_to @issue.author_name, user_url(@issue.author)}'s issue is due soon.
+
+- if @issue.assignees.any?
+  %p
+    Assignee: #{@issue.assignee_list}
+%p
+  This issue is due on: #{@issue.due_date.to_s(:medium)}
+
+- if @issue.description
+  %div
+  = markdown(@issue.description, pipeline: :email, author: @issue.author)

app/views/notify/issue_due_email.text.erb

@@ -0,0 +1,7 @@
+The following issue is due on <%= @issue.due_date %>:
+
+Issue <%= @issue.iid %>: <%= url_for(project_issue_url(@issue.project, @issue)) %>
+Author:    <%= @issue.author_name %>
+Assignee:  <%= @issue.assignee_list %>
+
+<%= @issue.description %>

app/workers/all_queues.yml

@@ -18,6 +18,7 @@
 - cronjob:stuck_import_jobs
 - cronjob:stuck_merge_jobs
 - cronjob:trending_projects
+- cronjob:issue_due_scheduler
 
 - gcp_cluster:cluster_install_app
 - gcp_cluster:cluster_provision
@@ -39,6 +40,8 @@
 - github_importer:github_import_stage_import_pull_requests
 - github_importer:github_import_stage_import_repository
 
+- mail_scheduler:mail_scheduler_issue_due
+
 - object_storage_upload
 - object_storage:object_storage_background_move
 - object_storage:object_storage_migrate_uploads

app/workers/concerns/mail_scheduler_queue.rb

@@ -0,0 +1,7 @@
+module MailSchedulerQueue
+  extend ActiveSupport::Concern
+
+  included do
+    queue_namespace :mail_scheduler
+  end
+end

app/workers/issue_due_scheduler_worker.rb

@@ -0,0 +1,10 @@
+class IssueDueSchedulerWorker
+  include ApplicationWorker
+  include CronjobQueue
+
+  def perform
+    project_ids = Issue.opened.due_tomorrow.group(:project_id).pluck(:project_id).map { |id| [id] }
+
+    MailScheduler::IssueDueWorker.bulk_perform_async(project_ids)
+  end
+end

app/workers/mail_scheduler/issue_due_worker.rb

@@ -0,0 +1,14 @@
+module MailScheduler
+  class IssueDueWorker
+    include ApplicationWorker
+    include MailSchedulerQueue
+
+    def perform(project_id)
+      notification_service = NotificationService.new
+
+      Issue.opened.due_tomorrow.in_projects(project_id).preload(:project).find_each do |issue|
+        notification_service.issue_due(issue)
+      end
+    end
+  end
+end

changelogs/unreleased/16957-issue-due-email.yml

@@ -0,0 +1,5 @@
+---
+title: Add cron job to email users on issue due date
+merge_request: 17985
+author: Stuart Nelson
+type: added

config/initializers/1_settings.rb

@@ -455,6 +455,10 @@ Settings.cron_jobs['pages_domain_verification_cron_worker'] ||= Settingslogic.ne
 Settings.cron_jobs['pages_domain_verification_cron_worker']['cron'] ||= '*/15 * * * *'
 Settings.cron_jobs['pages_domain_verification_cron_worker']['job_class'] = 'PagesDomainVerificationCronWorker'
 
+Settings.cron_jobs['issue_due_scheduler_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['issue_due_scheduler_worker']['cron'] ||= '50 00 * * *'
+Settings.cron_jobs['issue_due_scheduler_worker']['job_class'] = 'IssueDueSchedulerWorker'
+
 #
 # Sidekiq
 #

config/sidekiq_queues.yml

@@ -34,6 +34,7 @@
   - [email_receiver, 2]
   - [emails_on_push, 2]
   - [mailers, 2]
+  - [mail_scheduler, 2]
   - [invalid_gpg_signature_update, 2]
   - [create_gpg_signature, 2]
   - [rebase, 2]

db/migrate/20180330121048_add_issue_due_to_notification_settings.rb

@@ -0,0 +1,9 @@
+class AddIssueDueToNotificationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :notification_settings, :issue_due, :boolean
+  end
+end

db/schema.rb

@@ -1325,6 +1325,7 @@ ActiveRecord::Schema.define(version: 20180405142733) do
     t.boolean "failed_pipeline"
     t.boolean "success_pipeline"
     t.boolean "push_to_merge_request"
+    t.boolean "issue_due"
   end
 
   add_index "notification_settings", ["source_id", "source_type"], name: "index_notification_settings_on_source_id_and_source_type", using: :btree

doc/api/notification_settings.md

@@ -23,6 +23,7 @@ new_issue
 reopen_issue
 close_issue
 reassign_issue
+issue_due
 new_merge_request
 push_to_merge_request
 reopen_merge_request
@@ -75,6 +76,7 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab
 | `reopen_issue` | boolean | no | Enable/disable this notification |
 | `close_issue` | boolean | no | Enable/disable this notification |
 | `reassign_issue` | boolean | no | Enable/disable this notification |
+| `issue_due` | boolean | no | Enable/disable this notification |
 | `new_merge_request` | boolean | no | Enable/disable this notification |
 | `push_to_merge_request` | boolean | no | Enable/disable this notification |
 | `reopen_merge_request` | boolean | no | Enable/disable this notification |
@@ -142,6 +144,7 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab
 | `reopen_issue` | boolean | no | Enable/disable this notification |
 | `close_issue` | boolean | no | Enable/disable this notification |
 | `reassign_issue` | boolean | no | Enable/disable this notification |
+| `issue_due` | boolean | no | Enable/disable this notification |
 | `new_merge_request` | boolean | no | Enable/disable this notification |
 | `push_to_merge_request` | boolean | no | Enable/disable this notification |
 | `reopen_merge_request` | boolean | no | Enable/disable this notification |
@@ -166,6 +169,7 @@ Example responses:
     "reopen_issue": false,
     "close_issue": false,
     "reassign_issue": false,
+    "issue_due": false,
     "new_merge_request": false,
     "push_to_merge_request": false,
     "reopen_merge_request": false,

doc/user/project/issues/due_dates.md

@@ -35,5 +35,9 @@ Due dates also appear in your [todos list](../../../workflow/todos.md).
 
 ![Issues with due dates in the todos](img/due_dates_todos.png)
 
+The day before an open issue is due, an email will be sent to all participants
+of the issue. Both the due date and the day before are calculated using the
+server's timezone.
+
 [ce-3614]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3614
 [permissions]: ../../permissions.md#project

doc/workflow/notifications.md

@@ -86,6 +86,7 @@ In most of the below cases, the notification will be sent to:
 | Close issue            | |
 | Reassign issue         | The above, plus the old assignee |
 | Reopen issue           | |
+| Due issue              | Participants and Custom notification level with this event selected |
 | New merge request      | |
 | Push to merge request  | Participants and Custom notification level with this event selected |
 | Reassign merge request | The above, plus the old assignee |
@@ -96,15 +97,14 @@ In most of the below cases, the notification will be sent to:
 | Failed pipeline        | The author of the pipeline |
 | Successful pipeline    | The author of the pipeline, if they have the custom notification setting for successful pipelines set |
 
-
 In addition, if the title or description of an Issue or Merge Request is
 changed, notifications will be sent to any **new** mentions by `@username` as
 if they had been mentioned in the original text.
 
-You won't receive notifications for Issues, Merge Requests or Milestones
-created by yourself. You will only receive automatic notifications when
-somebody else comments or adds changes to the ones that you've created or
-mentions you.
+You won't receive notifications for Issues, Merge Requests or Milestones created
+by yourself (except when an issue is due). You will only receive automatic
+notifications when somebody else comments or adds changes to the ones that
+you've created or mentions you.
 
 ### Email Headers
 
@@ -122,7 +122,7 @@ Notification emails include headers that provide extra content about the notific
 | X-GitLab-NotificationReason | The reason for being notified. "mentioned", "assigned", etc             |
 
 #### X-GitLab-NotificationReason
-This header holds the reason for the notification to have been sent out, 
+This header holds the reason for the notification to have been sent out,
 where reason can be `mentioned`, `assigned`, `own_activity`, etc.
 Only one reason is sent out according to its priority:
 - `own_activity`
@@ -130,7 +130,7 @@ Only one reason is sent out according to its priority:
 - `mentioned`
 
 The reason in this header will also be shown in the footer of the notification email.  For example an email with the
-reason `assigned` will have this sentence in the footer: 
+reason `assigned` will have this sentence in the footer:
 `"You are receiving this email because you have been assigned an item on {configured GitLab hostname}"`
 
 **Note: Only reasons listed above have been implemented so far**

spec/services/notification_service_spec.rb

@@ -933,6 +933,46 @@ describe NotificationService, :mailer do
         let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) }
       end
     end
+
+    describe '#issue_due' do
+      before do
+        issue.update!(due_date: Date.today)
+
+        update_custom_notification(:issue_due, @u_guest_custom, resource: project)
+        update_custom_notification(:issue_due, @u_custom_global)
+      end
+
+      it 'sends email to issue notification recipients, excluding watchers' do
+        notification.issue_due(issue)
+
+        should_email(issue.assignees.first)
+        should_email(issue.author)
+        should_email(@u_guest_custom)
+        should_email(@u_custom_global)
+        should_email(@u_participant_mentioned)
+        should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
+        should_not_email(@u_watcher)
+        should_not_email(@u_guest_watcher)
+        should_not_email(@unsubscriber)
+        should_not_email(@u_participating)
+        should_not_email(@u_disabled)
+        should_not_email(@u_lazy_participant)
+      end
+
+      it 'sends the email from the author' do
+        notification.issue_due(issue)
+        email = find_email_for(@subscriber)
+
+        expect(email.header[:from].display_names).to eq([issue.author.name])
+      end
+
+      it_behaves_like 'participating notifications' do
+        let(:participant) { create(:user, username: 'user-participant') }
+        let(:issuable) { issue }
+        let(:notification_trigger) { notification.issue_due(issue) }
+      end
+    end
   end
 
   describe 'Merge Requests' do

spec/workers/issue_due_scheduler_worker_spec.rb

@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe IssueDueSchedulerWorker do
+  describe '#perform' do
+    it 'schedules one MailScheduler::IssueDueWorker per project with open issues due tomorrow' do
+      project1 = create(:project)
+      project2 = create(:project)
+      project_closed_issue = create(:project)
+      project_issue_due_another_day = create(:project)
+
+      create(:issue, :opened, project: project1, due_date: Date.tomorrow)
+      create(:issue, :opened, project: project1, due_date: Date.tomorrow)
+      create(:issue, :opened, project: project2, due_date: Date.tomorrow)
+      create(:issue, :closed, project: project_closed_issue, due_date: Date.tomorrow)
+      create(:issue, :opened, project: project_issue_due_another_day, due_date: Date.today)
+
+      expect(MailScheduler::IssueDueWorker).to receive(:bulk_perform_async).with([[project1.id], [project2.id]])
+
+      described_class.new.perform
+    end
+  end
+end

spec/workers/mail_scheduler/issue_due_worker_spec.rb

@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+describe MailScheduler::IssueDueWorker do
+  describe '#perform' do
+    let(:worker) { described_class.new }
+    let(:project) { create(:project) }
+
+    it 'sends emails for open issues due tomorrow in the project specified' do
+      issue1 = create(:issue, :opened, project: project, due_date: Date.tomorrow)
+      issue2 = create(:issue, :opened, project: project, due_date: Date.tomorrow)
+      create(:issue, :closed, project: project, due_date: Date.tomorrow) # closed
+      create(:issue, :opened, project: project, due_date: 2.days.from_now) # due on another day
+      create(:issue, :opened, due_date: Date.tomorrow) # different project
+
+      expect_any_instance_of(NotificationService).to receive(:issue_due).with(issue1)
+      expect_any_instance_of(NotificationService).to receive(:issue_due).with(issue2)
+
+      worker.perform(project.id)
+    end
+  end
+end