diff options
author | Sean McGivern <sean@gitlab.com> | 2018-03-30 16:05:20 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-03-30 16:55:05 +0300 |
commit | 2db218f8bf186c509c927ce3e9d0502fee4f8349 (patch) | |
tree | 8290216f729e4d5d376986720b5f730eb16e4141 | |
parent | 5ab75649f3ea00b64cb63e7e5283100c6b70cfb5 (diff) |
Send emails for issues due tomorrow
Also, refactor the mail sending slightly: instead of one worker sending all
emails, create a worker per project with issues due, which will send all emails
for that project.
-rw-r--r-- | app/models/issue.rb | 1 | ||||
-rw-r--r-- | app/workers/all_queues.yml | 4 | ||||
-rw-r--r-- | app/workers/concerns/mail_scheduler_queue.rb | 7 | ||||
-rw-r--r-- | app/workers/issue_due_scheduler_worker.rb | 10 | ||||
-rw-r--r-- | app/workers/issue_due_worker.rb | 10 | ||||
-rw-r--r-- | app/workers/mail_scheduler/issue_due_worker.rb | 14 | ||||
-rw-r--r-- | changelogs/unreleased/16957-issue-due-email.yml | 2 | ||||
-rw-r--r-- | config/initializers/1_settings.rb | 6 | ||||
-rw-r--r-- | config/sidekiq_queues.yml | 1 | ||||
-rw-r--r-- | doc/user/project/issues/due_dates.md | 3 | ||||
-rw-r--r-- | spec/workers/issue_due_scheduler_worker_spec.rb | 23 | ||||
-rw-r--r-- | spec/workers/issue_due_worker_spec.rb | 17 | ||||
-rw-r--r-- | spec/workers/mail_scheduler/issue_due_worker_spec.rb | 21 |
13 files changed, 87 insertions, 32 deletions
diff --git a/app/models/issue.rb b/app/models/issue.rb index 7bfc45c1f43..f65cd8bf896 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -48,6 +48,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') } diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index ef445055f6e..9aea3bad27b 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -18,7 +18,7 @@ - cronjob:stuck_import_jobs - cronjob:stuck_merge_jobs - cronjob:trending_projects -- cronjob:issue_due +- cronjob:issue_due_scheduler - gcp_cluster:cluster_install_app - gcp_cluster:cluster_provision @@ -40,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 diff --git a/app/workers/concerns/mail_scheduler_queue.rb b/app/workers/concerns/mail_scheduler_queue.rb new file mode 100644 index 00000000000..9df55ad9522 --- /dev/null +++ b/app/workers/concerns/mail_scheduler_queue.rb @@ -0,0 +1,7 @@ +module MailSchedulerQueue + extend ActiveSupport::Concern + + included do + queue_namespace :mail_scheduler + end +end diff --git a/app/workers/issue_due_scheduler_worker.rb b/app/workers/issue_due_scheduler_worker.rb new file mode 100644 index 00000000000..9c06304bff6 --- /dev/null +++ b/app/workers/issue_due_scheduler_worker.rb @@ -0,0 +1,10 @@ +class IssueDueSchedulerWorker + include ApplicationWorker + include CronjobQueue + + def perform + Issue.opened.due_tomorrow.group(:project_id).pluck(:project_id).each do |project_id| + MailScheduler::IssueDueWorker.perform_async(project_id) + end + end +end diff --git a/app/workers/issue_due_worker.rb b/app/workers/issue_due_worker.rb deleted file mode 100644 index 90d2cc09f1b..00000000000 --- a/app/workers/issue_due_worker.rb +++ /dev/null @@ -1,10 +0,0 @@ -class IssueDueWorker - include ApplicationWorker - include CronjobQueue - - def perform - Issue.where(due_date: Date.today).find_each do |issue| - NotificationService.new.issue_due_email(issue) - end - end -end diff --git a/app/workers/mail_scheduler/issue_due_worker.rb b/app/workers/mail_scheduler/issue_due_worker.rb new file mode 100644 index 00000000000..b06079d68ca --- /dev/null +++ b/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 diff --git a/changelogs/unreleased/16957-issue-due-email.yml b/changelogs/unreleased/16957-issue-due-email.yml index 308f72e6a67..83944ca4f73 100644 --- a/changelogs/unreleased/16957-issue-due-email.yml +++ b/changelogs/unreleased/16957-issue-due-email.yml @@ -1,5 +1,5 @@ --- title: Add cron job to email users on issue due date -merge_request: 16957 +merge_request: 17985 author: Stuart Nelson type: added diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 1ddf380c56d..c8a4c1af50f 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -453,9 +453,9 @@ 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_worker'] ||= Settingslogic.new({}) -Settings.cron_jobs['issue_due_worker']['cron'] ||= '50 00 * * *' -Settings.cron_jobs['issue_due_worker']['job_class'] = 'IssueDueWorker' +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' # # GitLab Shell diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index c811034b29d..47fbbed44cf 100644 --- a/config/sidekiq_queues.yml +++ b/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] diff --git a/doc/user/project/issues/due_dates.md b/doc/user/project/issues/due_dates.md index e0c405353ce..400f581e513 100644 --- a/doc/user/project/issues/due_dates.md +++ b/doc/user/project/issues/due_dates.md @@ -35,5 +35,8 @@ 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. + [ce-3614]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3614 [permissions]: ../../permissions.md#project diff --git a/spec/workers/issue_due_scheduler_worker_spec.rb b/spec/workers/issue_due_scheduler_worker_spec.rb new file mode 100644 index 00000000000..eff5855834c --- /dev/null +++ b/spec/workers/issue_due_scheduler_worker_spec.rb @@ -0,0 +1,23 @@ +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(:perform_async).with(project1.id) + expect(MailScheduler::IssueDueWorker).to receive(:perform_async).with(project2.id) + + described_class.new.perform + end + end +end diff --git a/spec/workers/issue_due_worker_spec.rb b/spec/workers/issue_due_worker_spec.rb deleted file mode 100644 index 9004edbee3a..00000000000 --- a/spec/workers/issue_due_worker_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -require 'spec_helper' - -describe IssueDueWorker do - describe 'perform' do - let(:worker) { described_class.new } - - it 'finds issues due on the day run' do - issue1 = create(:issue, :opened, due_date: Date.today) - create(:issue, :opened, due_date: 3.days.from_now) - create(:issue, :opened, due_date: 4.days.from_now) - - expect_any_instance_of(NotificationService).to receive(:issue_due_email).with(issue1) - - worker.perform - end - end -end diff --git a/spec/workers/mail_scheduler/issue_due_worker_spec.rb b/spec/workers/mail_scheduler/issue_due_worker_spec.rb new file mode 100644 index 00000000000..48ac1b8a1a4 --- /dev/null +++ b/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 |