From 916391ef40630a18995b6fbc4f1ad73ed802004a Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Mon, 27 Mar 2017 22:03:16 +1000 Subject: [PATCH 01/20] Regular messages refactoring: extract notification require checker and message builder to service classes --- app/workers/telegram_sender_worker.rb | 62 ++++++----- lib/intouch.rb | 2 +- .../checker/private_message_required.rb | 2 + lib/intouch/message/live.rb | 6 - lib/intouch/message/regular.rb | 101 ----------------- lib/intouch/regular/checker/base.rb | 30 +++++ lib/intouch/regular/message/base.rb | 105 ++++++++++++++++++ lib/intouch/regular/message/private.rb | 52 +++++++++ test/unit/intouch/message/regular_test.rb | 38 ------- .../unit/intouch/regular/checker/base_test.rb | 85 ++++++++++++++ .../unit/intouch/regular/message/base_test.rb | 38 +++++++ 11 files changed, 349 insertions(+), 172 deletions(-) delete mode 100644 lib/intouch/message/live.rb delete mode 100644 lib/intouch/message/regular.rb create mode 100644 lib/intouch/regular/checker/base.rb create mode 100644 lib/intouch/regular/message/base.rb create mode 100644 lib/intouch/regular/message/private.rb delete mode 100644 test/unit/intouch/message/regular_test.rb create mode 100644 test/unit/intouch/regular/checker/base_test.rb create mode 100644 test/unit/intouch/regular/message/base_test.rb diff --git a/app/workers/telegram_sender_worker.rb b/app/workers/telegram_sender_worker.rb index 05a047f..ac2ec43 100644 --- a/app/workers/telegram_sender_worker.rb +++ b/app/workers/telegram_sender_worker.rb @@ -1,45 +1,55 @@ +require_relative '../../lib/intouch/regular/checker/base' +require_relative '../../lib/intouch/regular/message/private' + class TelegramSenderWorker include Sidekiq::Worker - TELEGRAM_SENDER_LOG = Logger.new(Rails.root.join('log/intouch', 'telegram-sender.log')) def perform(issue_id, state) + @issue = Issue.find issue_id + @state = state Intouch.set_locale - issue = Issue.find issue_id - - return unless issue.notificable_for_state? state - base_message = issue.telegram_message + return unless notificable? issue.intouch_recipients('telegram', state).each do |user| telegram_account = user.telegram_account next unless telegram_account.present? && telegram_account.active? - roles_in_issue = [] + message = message(user) + + TelegramMessageSender.perform_async(telegram_account.telegram_id, message) + end + rescue ActiveRecord::RecordNotFound => e + # ignore + end - roles_in_issue << 'assigned_to' if issue.assigned_to_id == user.id - roles_in_issue << 'watchers' if issue.watchers.pluck(:user_id).include? user.id - roles_in_issue << 'author' if issue.author_id == user.id + private - project = issue.project - settings = project.active_telegram_settings.try(:[], state) + attr_reader :issue, :state - if settings.present? - recipients = settings.select do |key, _value| - %w(author assigned_to watchers).include?(key) - end.keys + def notificable? + Intouch::Regular::Checker::Base.new( + issue: issue, + state: state, + project: project + ).required? + end - prefix = (roles_in_issue & recipients).map do |role| - I18n.t("intouch.telegram_message.recipient.#{role}") - end.join(', ') - else - prefix = nil - end + def message(user) + Intouch::Regular::Message::Private.new( + issue: issue, + user: user, + state: state, + project: project + ).message + end - message = prefix.present? ? "#{prefix}\n#{base_message}" : base_message + def project + @project ||= issue.project + end - TelegramMessageSender.perform_async(telegram_account.telegram_id, message) - end - rescue ActiveRecord::RecordNotFound => e - # ignore + def logger + @logger ||= Logger.new(Rails.root.join('log/intouch', 'telegram-sender.log')) end + end diff --git a/lib/intouch.rb b/lib/intouch.rb index 2834a24..5e9842b 100644 --- a/lib/intouch.rb +++ b/lib/intouch.rb @@ -57,7 +57,7 @@ def self.send_notifications(issues, state) if active && interval.present? && - Intouch::Message::Regular.new(issue).latest_action_on < interval.to_i.hours.ago && + Intouch::Regular::Message::Base.new(issue).latest_action_on < interval.to_i.hours.ago && (last_notification.nil? || last_notification < interval.to_i.hours.ago) if active_protocols.include? 'email' diff --git a/lib/intouch/checker/private_message_required.rb b/lib/intouch/checker/private_message_required.rb index a4beacf..9bdbfea 100644 --- a/lib/intouch/checker/private_message_required.rb +++ b/lib/intouch/checker/private_message_required.rb @@ -1,3 +1,5 @@ +# this is for live + module Intouch module Checker class PrivateMessageRequired diff --git a/lib/intouch/message/live.rb b/lib/intouch/message/live.rb deleted file mode 100644 index 5ce7ff4..0000000 --- a/lib/intouch/message/live.rb +++ /dev/null @@ -1,6 +0,0 @@ -module Intouch - module Message - class Live - end - end -end diff --git a/lib/intouch/message/regular.rb b/lib/intouch/message/regular.rb deleted file mode 100644 index c47e686..0000000 --- a/lib/intouch/message/regular.rb +++ /dev/null @@ -1,101 +0,0 @@ -module Intouch - module Message - class Regular - extend ServiceInitializer - - attr_reader :issue - - delegate :title, :assigned_to, :priority, :status, :link, - to: :formatter - - def initialize(issue) - @issue = issue - end - - def call - [ - unassigned_message, - overdue_message, - without_due_date_message, - inactive_message, - basic_message - ].compact.join("\n") - end - - def basic_message - <<~TEXT - `#{title}` - #{assigned_to} - #{priority} - #{status} - #{link} - TEXT - end - - def unassigned_message - I18n.t('intouch.telegram_message.issue.notice.unassigned') if issue.unassigned? || issue.assigned_to_group? - end - - def overdue_message - I18n.t('intouch.telegram_message.issue.notice.overdue') if issue.overdue? - end - - def without_due_date_message - I18n.t('intouch.telegram_message.issue.notice.without_due_date') if without_due_date? - end - - def without_due_date? - !issue.due_date.present? && issue.created_on < 1.day.ago - end - - def inactive_message - I18n.t('intouch.telegram_message.issue.inactive', hours: rounded_inactive_hours) if inactive? - end - - def rounded_inactive_hours - inactive_hours.round(1) - end - - def inactive? - return unless reminder_active? && reminder_interval.positive? - - inactive_hours >= reminder_interval - end - - def inactive_hours - @inactive_hours ||= ((Time.now - latest_action_on) / 3600) - end - - def reminder_interval - reminder_settings.try(:[], 'interval').to_i - end - - def reminder_active? - reminder_settings.try(:[], 'active') - end - - def reminder_settings - @reminder_settings ||= project.active_intouch_settings - .try(:[], 'reminder_settings') - .try(:[], issue.priority_id.to_s) - end - - def latest_action_on - latest_staff_action_on.present? ? latest_staff_action_on : issue.updated_on - end - - def latest_staff_action_on - @latest_assigner_update_on ||= - issue.journals.order(:id).where(user_id: project.assigner_ids).last.try(:created_on) - end - - def formatter - @formatter ||= Formatter.new(issue) - end - - def project - @project ||= @issue.project - end - end - end -end diff --git a/lib/intouch/regular/checker/base.rb b/lib/intouch/regular/checker/base.rb new file mode 100644 index 0000000..0103a17 --- /dev/null +++ b/lib/intouch/regular/checker/base.rb @@ -0,0 +1,30 @@ +module Intouch::Regular::Checker + class Base + def initialize(issue:, state:, project:) + @issue = issue + @state = state + @project = project + end + + attr_reader :issue, :state, :project + + def required? + notificable_for_state? && + (inactivity_notification? ? assigned_to_assigner? : true) + end + + private + + def notificable_for_state? + issue.notificable_for_state?(state) + end + + def inactivity_notification? + %w(feedback working).include?(state) + end + + def assigned_to_assigner? + project.assigner_ids.include?(issue.assigned_to_id) + end + end +end diff --git a/lib/intouch/regular/message/base.rb b/lib/intouch/regular/message/base.rb new file mode 100644 index 0000000..4898b1c --- /dev/null +++ b/lib/intouch/regular/message/base.rb @@ -0,0 +1,105 @@ +require_relative '../../service_initializer' + +module Intouch::Regular::Message + class Base + extend Intouch::ServiceInitializer + + attr_reader :issue + + delegate :title, :assigned_to, :priority, :status, :link, + to: :formatter + + def initialize(issue) + @issue = issue + end + + def call + base_message + end + + def base_message + @base_message ||= [ + unassigned_message, + overdue_message, + without_due_date_message, + inactive_message, + basic_message + ].compact.join("\n") + end + + def basic_message + <<~TEXT + `#{title}` + #{assigned_to} + #{priority} + #{status} + #{link} + TEXT + end + + def unassigned_message + I18n.t('intouch.telegram_message.issue.notice.unassigned') if issue.unassigned? || issue.assigned_to_group? + end + + def overdue_message + I18n.t('intouch.telegram_message.issue.notice.overdue') if issue.overdue? + end + + def without_due_date_message + I18n.t('intouch.telegram_message.issue.notice.without_due_date') if without_due_date? + end + + def without_due_date? + !issue.due_date.present? && issue.created_on < 1.day.ago + end + + def inactive_message + I18n.t('intouch.telegram_message.issue.inactive', hours: rounded_inactive_hours) if inactive? + end + + def rounded_inactive_hours + inactive_hours.round(1) + end + + def inactive? + return unless reminder_active? && reminder_interval.positive? + + inactive_hours >= reminder_interval + end + + def inactive_hours + @inactive_hours ||= ((Time.now - latest_action_on) / 3600) + end + + def reminder_interval + reminder_settings.try(:[], 'interval').to_i + end + + def reminder_active? + reminder_settings.try(:[], 'active') + end + + def reminder_settings + @reminder_settings ||= project.active_intouch_settings + .try(:[], 'reminder_settings') + .try(:[], issue.priority_id.to_s) + end + + def latest_action_on + latest_staff_action_on.present? ? latest_staff_action_on : issue.updated_on + end + + def latest_staff_action_on + @latest_assigner_update_on ||= + issue.journals.order(:id).where(user_id: project.assigner_ids).last.try(:created_on) + end + + def formatter + @formatter ||= Intouch::Message::Formatter.new(issue) + end + + def project + @project ||= issue.project + end + end +end diff --git a/lib/intouch/regular/message/private.rb b/lib/intouch/regular/message/private.rb new file mode 100644 index 0000000..96f1f6e --- /dev/null +++ b/lib/intouch/regular/message/private.rb @@ -0,0 +1,52 @@ +module Intouch::Regular::Message + class Private < Base + + def initialize(issue:, user:, state:, project:) + @issue = issue + @user = user + @state = state + @project = project + end + + attr_reader :issue, :user, :state, :project + + def call + message + end + + def message + prefix.present? ? "#{prefix}\n#{base_message}" : base_message + end + + def prefix + return nil unless settings.present? + + recipients_prefix + end + + def recipients_prefix + (roles_in_issue & recipients).map do |role| + I18n.t("intouch.telegram_message.recipient.#{role}") + end.join(', ') + end + + def roles_in_issue + roles_in_issue = [] + + roles_in_issue << 'assigned_to' if issue.assigned_to_id == user.id + roles_in_issue << 'watchers' if issue.watchers.pluck(:user_id).include? user.id + roles_in_issue << 'author' if issue.author_id == user.id + roles_in_issue + end + + def recipients + settings.select do |key, _value| + %w(author assigned_to watchers).include?(key) + end.keys + end + + def settings + @settings ||= project.active_telegram_settings.try(:[], state) + end + end +end diff --git a/test/unit/intouch/message/regular_test.rb b/test/unit/intouch/message/regular_test.rb deleted file mode 100644 index 339f093..0000000 --- a/test/unit/intouch/message/regular_test.rb +++ /dev/null @@ -1,38 +0,0 @@ -require File.expand_path('../../../../test_helper', __FILE__) - -class RegularMessageTest < ActiveSupport::TestCase - subject { Intouch::Message::Regular.new(issue) } - - let(:issue) { Object.new } - - describe 'inactive?' do - before do - Intouch::Message::Regular.any_instance.stubs(:reminder_active?).returns(true) - Intouch::Message::Regular.any_instance.stubs(:reminder_interval).returns(1) - end - - describe 'less than 1 hour ago' do - before do - Intouch::Message::Regular.any_instance.stubs(:latest_action_on).returns(59.minutes.ago) - end - - it { subject.inactive?.must_equal false } - end - - describe '1 hour ago' do - before do - Intouch::Message::Regular.any_instance.stubs(:latest_action_on).returns(60.minutes.ago) - end - - it { subject.inactive?.must_equal true } - end - - describe 'more than 1 hour ago' do - before do - Intouch::Message::Regular.any_instance.stubs(:latest_action_on).returns(61.minutes.ago) - end - - it { subject.inactive?.must_equal true } - end - end -end diff --git a/test/unit/intouch/regular/checker/base_test.rb b/test/unit/intouch/regular/checker/base_test.rb new file mode 100644 index 0000000..2a7f47d --- /dev/null +++ b/test/unit/intouch/regular/checker/base_test.rb @@ -0,0 +1,85 @@ +require_relative '../../../../test_helper' + +class Intouch::Regular::Checker::BaseTest < ActiveSupport::TestCase + subject do + Intouch::Regular::Checker::Base.new( + issue: issue, + state: state, + project: project + ).required? + end + + let(:assigner_id) { 1 } + let(:project) { OpenStruct.new(assigner_ids: [assigner_id]) } + + describe 'notificable for state' do + before { Issue.any_instance.stubs(:notificable_for_state?).returns(true) } + + describe 'issue assigned to client' do + let(:client_id) { 2 } + let(:issue) { Issue.new(assigned_to_id: client_id) } + + describe 'working' do + let(:state) { 'working' } + + it { subject.must_equal false } + end + + describe 'feedback' do + let(:state) { 'feedback' } + + it { subject.must_equal false } + end + + describe 'unassigned' do + let(:state) { 'unassigned' } + + it { subject.must_equal true } + end + + describe 'overdue' do + let(:state) { 'overdue' } + + it { subject.must_equal true } + end + end + + describe 'issue assigned to assigner' do + let(:issue) { Issue.new(assigned_to_id: assigner_id) } + + + describe 'working' do + let(:state) { 'working' } + + it { subject.must_equal true } + end + + describe 'feedback' do + let(:state) { 'feedback' } + + it { subject.must_equal true } + end + + describe 'unassigned' do + let(:state) { 'unassigned' } + + it { subject.must_equal true } + end + + describe 'overdue' do + let(:state) { 'overdue' } + + it { subject.must_equal true } + end + end + end + + describe 'not notificable' do + let(:state) { 'some_state' } + let(:issue) { Issue.new } + + before { Issue.any_instance.stubs(:notificable_for_state?).returns(false) } + + it { subject.must_equal false } + end +end \ No newline at end of file diff --git a/test/unit/intouch/regular/message/base_test.rb b/test/unit/intouch/regular/message/base_test.rb new file mode 100644 index 0000000..f095aaa --- /dev/null +++ b/test/unit/intouch/regular/message/base_test.rb @@ -0,0 +1,38 @@ +require_relative '../../../../test_helper' + +class Intouch::Regular::Message::BaseTest < ActiveSupport::TestCase + subject { Intouch::Regular::Message::Base.new(issue) } + + let(:issue) { Object.new } + + describe 'inactive?' do + before do + Intouch::Regular::Message::Base.any_instance.stubs(:reminder_active?).returns(true) + Intouch::Regular::Message::Base.any_instance.stubs(:reminder_interval).returns(1) + end + + describe 'less than 1 hour ago' do + before do + Intouch::Regular::Message::Base.any_instance.stubs(:latest_action_on).returns(59.minutes.ago) + end + + it { subject.inactive?.must_equal false } + end + + describe '1 hour ago' do + before do + Intouch::Regular::Message::Base.any_instance.stubs(:latest_action_on).returns(60.minutes.ago) + end + + it { subject.inactive?.must_equal true } + end + + describe 'more than 1 hour ago' do + before do + Intouch::Regular::Message::Base.any_instance.stubs(:latest_action_on).returns(61.minutes.ago) + end + + it { subject.inactive?.must_equal true } + end + end +end From d52cd7cff1ec8dfc6ad84f1bbb7f00e0e299fad6 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Mon, 27 Mar 2017 22:05:45 +1000 Subject: [PATCH 02/20] Cleanup --- app/workers/telegram_sender_worker.rb | 1 - lib/intouch/patches/issue_patch.rb | 48 +++++++++---------- lib/intouch/regular/message/base.rb | 2 +- lib/intouch/regular/message/private.rb | 1 - .../unit/intouch/regular/checker/base_test.rb | 3 +- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/app/workers/telegram_sender_worker.rb b/app/workers/telegram_sender_worker.rb index ac2ec43..d0bfeef 100644 --- a/app/workers/telegram_sender_worker.rb +++ b/app/workers/telegram_sender_worker.rb @@ -51,5 +51,4 @@ def project def logger @logger ||= Logger.new(Rails.root.join('log/intouch', 'telegram-sender.log')) end - end diff --git a/lib/intouch/patches/issue_patch.rb b/lib/intouch/patches/issue_patch.rb index 5729996..8084134 100644 --- a/lib/intouch/patches/issue_patch.rb +++ b/lib/intouch/patches/issue_patch.rb @@ -64,16 +64,16 @@ def notification_states def notificable_for_state?(state) case state - when 'unassigned' - notification_states.include?('unassigned') || notification_states.include?('assigned_to_group') - when 'overdue' - notification_states.include?('overdue') || notification_states.include?('without_due_date') - when 'working' - notification_states.include?('working') - when 'feedback' - notification_states.include?('feedback') - else - false + when 'unassigned' + notification_states.include?('unassigned') || notification_states.include?('assigned_to_group') + when 'overdue' + notification_states.include?('overdue') || notification_states.include?('without_due_date') + when 'working' + notification_states.include?('working') + when 'feedback' + notification_states.include?('feedback') + else + false end end @@ -81,14 +81,14 @@ def recipient_ids(protocol, state = notification_state) if project.send("active_#{protocol}_settings") && state && project.send("active_#{protocol}_settings")[state] project.send("active_#{protocol}_settings")[state].map do |key, value| case key - when 'author' - author.id - when 'assigned_to' - assigned_to_id if assigned_to.class == User - when 'watchers' - watchers.pluck(:user_id) - when 'user_groups' - Group.where(id: value).map(&:user_ids).flatten if value.present? + when 'author' + author.id + when 'assigned_to' + assigned_to_id if assigned_to.class == User + when 'watchers' + watchers.pluck(:user_id) + when 'user_groups' + Group.where(id: value).map(&:user_ids).flatten if value.present? end end.flatten.uniq + [assigner_id] end @@ -103,12 +103,12 @@ def live_recipient_ids(protocol) recipients.each_pair do |key, value| next unless value.try(:[], status_id.to_s).try(:include?, priority_id.to_s) case key - when 'author' - user_ids << author.id - when 'assigned_to' - user_ids << assigned_to_id if assigned_to.class == User - when 'watchers' - user_ids += watchers.pluck(:user_id) + when 'author' + user_ids << author.id + when 'assigned_to' + user_ids << assigned_to_id if assigned_to.class == User + when 'watchers' + user_ids += watchers.pluck(:user_id) end end user_ids.flatten.uniq + [assigner_id] - [updated_by.try(:id)] # Не отправляем сообщение тому, то обновил задачу diff --git a/lib/intouch/regular/message/base.rb b/lib/intouch/regular/message/base.rb index 4898b1c..4ce1552 100644 --- a/lib/intouch/regular/message/base.rb +++ b/lib/intouch/regular/message/base.rb @@ -7,7 +7,7 @@ class Base attr_reader :issue delegate :title, :assigned_to, :priority, :status, :link, - to: :formatter + to: :formatter def initialize(issue) @issue = issue diff --git a/lib/intouch/regular/message/private.rb b/lib/intouch/regular/message/private.rb index 96f1f6e..ef83273 100644 --- a/lib/intouch/regular/message/private.rb +++ b/lib/intouch/regular/message/private.rb @@ -1,6 +1,5 @@ module Intouch::Regular::Message class Private < Base - def initialize(issue:, user:, state:, project:) @issue = issue @user = user diff --git a/test/unit/intouch/regular/checker/base_test.rb b/test/unit/intouch/regular/checker/base_test.rb index 2a7f47d..5826bb3 100644 --- a/test/unit/intouch/regular/checker/base_test.rb +++ b/test/unit/intouch/regular/checker/base_test.rb @@ -47,7 +47,6 @@ class Intouch::Regular::Checker::BaseTest < ActiveSupport::TestCase describe 'issue assigned to assigner' do let(:issue) { Issue.new(assigned_to_id: assigner_id) } - describe 'working' do let(:state) { 'working' } @@ -82,4 +81,4 @@ class Intouch::Regular::Checker::BaseTest < ActiveSupport::TestCase it { subject.must_equal false } end -end \ No newline at end of file +end From 66a8a3f464ad96a6da03c87acbc7f80ddb74906e Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Mon, 27 Mar 2017 22:22:49 +1000 Subject: [PATCH 03/20] Fix require test_helper --- lib/intouch/regular/recipients_list.rb | 12 ++++++++++++ test/unit/intouch/regular/checker/base_test.rb | 2 +- test/unit/intouch/regular/message/base_test.rb | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 lib/intouch/regular/recipients_list.rb diff --git a/lib/intouch/regular/recipients_list.rb b/lib/intouch/regular/recipients_list.rb new file mode 100644 index 0000000..5155d7e --- /dev/null +++ b/lib/intouch/regular/recipients_list.rb @@ -0,0 +1,12 @@ +module Intouch::Regular + class RecipientsList + def initialize(issue:, protocol:, state:) + @issue = issue + @protocol = protocol + @state = state + end + + attr_reader :issue, :protocol, :state + + end +end \ No newline at end of file diff --git a/test/unit/intouch/regular/checker/base_test.rb b/test/unit/intouch/regular/checker/base_test.rb index 5826bb3..75bd7e1 100644 --- a/test/unit/intouch/regular/checker/base_test.rb +++ b/test/unit/intouch/regular/checker/base_test.rb @@ -1,4 +1,4 @@ -require_relative '../../../../test_helper' +require File.expand_path('../../../../../test_helper', __FILE__) class Intouch::Regular::Checker::BaseTest < ActiveSupport::TestCase subject do diff --git a/test/unit/intouch/regular/message/base_test.rb b/test/unit/intouch/regular/message/base_test.rb index f095aaa..61b3f35 100644 --- a/test/unit/intouch/regular/message/base_test.rb +++ b/test/unit/intouch/regular/message/base_test.rb @@ -1,4 +1,4 @@ -require_relative '../../../../test_helper' +require File.expand_path('../../../../../test_helper', __FILE__) class Intouch::Regular::Message::BaseTest < ActiveSupport::TestCase subject { Intouch::Regular::Message::Base.new(issue) } From d0f1bea6f5c8d2298f87024040571a680c4f70e1 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Mon, 27 Mar 2017 23:54:55 +1000 Subject: [PATCH 04/20] Extract recipient list to service class, live message refactoring --- app/workers/telegram_sender_worker.rb | 29 +++++------ lib/intouch/checker/notification_required.rb | 18 ------- .../checker/private_message_required.rb | 32 ------------ lib/intouch/live/checker/base.rb | 16 ++++++ lib/intouch/live/checker/private.rb | 28 +++++++++++ .../handler/new_issue.rb} | 14 +++--- .../handler/updated_issue.rb} | 14 ++++-- .../message/group.rb} | 8 ++- .../message/private.rb} | 12 ++--- lib/intouch/patches/issue_patch.rb | 31 +++++------- lib/intouch/patches/journal_patch.rb | 2 +- lib/intouch/regular/message/base.rb | 6 --- lib/intouch/regular/message/private.rb | 12 ++++- lib/intouch/regular/recipients_list.rb | 50 ++++++++++++++++++- .../checker/base_test.rb} | 6 +-- .../checker/private_test.rb} | 6 +-- .../intouch/live/handler/new_issue_test.rb | 50 +++++++++++++++++++ test/unit/intouch/new_issue_handler_test.rb | 50 ------------------- 18 files changed, 212 insertions(+), 172 deletions(-) delete mode 100644 lib/intouch/checker/notification_required.rb delete mode 100644 lib/intouch/checker/private_message_required.rb create mode 100644 lib/intouch/live/checker/base.rb create mode 100644 lib/intouch/live/checker/private.rb rename lib/intouch/{new_issue_handler.rb => live/handler/new_issue.rb} (52%) rename lib/intouch/{updated_issue_handler.rb => live/handler/updated_issue.rb} (62%) rename lib/intouch/{group_message_sender.rb => live/message/group.rb} (80%) rename lib/intouch/{private_message_sender.rb => live/message/private.rb} (83%) rename test/unit/intouch/{checker/notification_required_test.rb => live/checker/base_test.rb} (85%) rename test/unit/intouch/{checker/private_message_required_test.rb => live/checker/private_test.rb} (86%) create mode 100644 test/unit/intouch/live/handler/new_issue_test.rb delete mode 100644 test/unit/intouch/new_issue_handler_test.rb diff --git a/app/workers/telegram_sender_worker.rb b/app/workers/telegram_sender_worker.rb index d0bfeef..86ff82f 100644 --- a/app/workers/telegram_sender_worker.rb +++ b/app/workers/telegram_sender_worker.rb @@ -1,26 +1,19 @@ require_relative '../../lib/intouch/regular/checker/base' +require_relative '../../lib/intouch/regular/recipients_list' require_relative '../../lib/intouch/regular/message/private' class TelegramSenderWorker include Sidekiq::Worker def perform(issue_id, state) - @issue = Issue.find issue_id @state = state - Intouch.set_locale + @issue = Issue.find_by(id: issue_id) + return unless @issue.present? return unless notificable? + return unless users.present? - issue.intouch_recipients('telegram', state).each do |user| - telegram_account = user.telegram_account - next unless telegram_account.present? && telegram_account.active? - - message = message(user) - - TelegramMessageSender.perform_async(telegram_account.telegram_id, message) - end - rescue ActiveRecord::RecordNotFound => e - # ignore + users.each { |user| send_message(user) } end private @@ -35,13 +28,21 @@ def notificable? ).required? end - def message(user) + def users + @users ||= Intouch::Regular::RecipientsList.new( + issue: issue, + state: state, + protocol: 'telegram' + ).call + end + + def send_message(user) Intouch::Regular::Message::Private.new( issue: issue, user: user, state: state, project: project - ).message + ).send end def project diff --git a/lib/intouch/checker/notification_required.rb b/lib/intouch/checker/notification_required.rb deleted file mode 100644 index d015373..0000000 --- a/lib/intouch/checker/notification_required.rb +++ /dev/null @@ -1,18 +0,0 @@ -module Intouch - module Checker - class NotificationRequired - attr_reader :project, :issue - - def initialize(issue, project) - @issue = issue - @project = project - end - - def call - project.module_enabled?(:intouch) && - project.active? && - !issue.closed? - end - end - end -end diff --git a/lib/intouch/checker/private_message_required.rb b/lib/intouch/checker/private_message_required.rb deleted file mode 100644 index 9bdbfea..0000000 --- a/lib/intouch/checker/private_message_required.rb +++ /dev/null @@ -1,32 +0,0 @@ -# this is for live - -module Intouch - module Checker - class PrivateMessageRequired - attr_reader :project, :issue - - def initialize(issue, project) - @issue = issue - @project = project - end - - def call - issue.alarm? || Intouch.work_time? || notify_always? - end - - def required_recipients - @required_recipients ||= always_notify_settings.keys - end - - private - - def notify_always? - required_recipients.present? - end - - def always_notify_settings - project.active_intouch_settings['always_notify'] || {} - end - end - end -end diff --git a/lib/intouch/live/checker/base.rb b/lib/intouch/live/checker/base.rb new file mode 100644 index 0000000..4a6f5cb --- /dev/null +++ b/lib/intouch/live/checker/base.rb @@ -0,0 +1,16 @@ +module Intouch::Live::Checker + class Base + attr_reader :project, :issue + + def initialize(issue, project) + @issue = issue + @project = project + end + + def required? + project.module_enabled?(:intouch) && + project.active? && + !issue.closed? + end + end +end diff --git a/lib/intouch/live/checker/private.rb b/lib/intouch/live/checker/private.rb new file mode 100644 index 0000000..b2a215e --- /dev/null +++ b/lib/intouch/live/checker/private.rb @@ -0,0 +1,28 @@ +module Intouch::Live::Checker + class Private + attr_reader :project, :issue + + def initialize(issue, project) + @issue = issue + @project = project + end + + def required? + issue.alarm? || Intouch.work_time? || notify_always? + end + + def required_recipients + @required_recipients ||= always_notify_settings.keys + end + + private + + def notify_always? + required_recipients.present? + end + + def always_notify_settings + project.active_intouch_settings['always_notify'] || {} + end + end +end diff --git a/lib/intouch/new_issue_handler.rb b/lib/intouch/live/handler/new_issue.rb similarity index 52% rename from lib/intouch/new_issue_handler.rb rename to lib/intouch/live/handler/new_issue.rb index 9e0e8fe..7a310c9 100644 --- a/lib/intouch/new_issue_handler.rb +++ b/lib/intouch/live/handler/new_issue.rb @@ -1,7 +1,9 @@ -module Intouch - class NewIssueHandler - extend ServiceInitializer +require_relative '../checker/base' +require_relative '../message/private' +require_relative '../message/group' +module Intouch::Live::Handler + class NewIssue def initialize(issue) @issue = issue @project = @issue.project @@ -19,15 +21,15 @@ def call attr_reader :issue, :project def notification_required? - Checker::NotificationRequired.new(issue, project).call + Intouch::Live::Checker::Base.new(issue, project).required? end def send_private_messages - PrivateMessageSender.call(issue, project) + Intouch::Live::Message::Private.new(issue, project).send end def send_group_messages - GroupMessageSender.call(issue, project) + Intouch::Live::Message::Group.new(issue, project).send end end end diff --git a/lib/intouch/updated_issue_handler.rb b/lib/intouch/live/handler/updated_issue.rb similarity index 62% rename from lib/intouch/updated_issue_handler.rb rename to lib/intouch/live/handler/updated_issue.rb index edc2be2..c5b5d44 100644 --- a/lib/intouch/updated_issue_handler.rb +++ b/lib/intouch/live/handler/updated_issue.rb @@ -1,5 +1,9 @@ -module Intouch - class UpdatedIssueHandler +require_relative '../checker/base' +require_relative '../message/private' +require_relative '../message/group' + +module Intouch::Live::Handler + class UpdatedIssue def initialize(journal) @journal = journal @issue = @journal.issue @@ -18,17 +22,17 @@ def call attr_reader :issue, :project, :journal def notification_required? - Checker::NotificationRequired.new(issue, project).call + Intouch::Live::Checker::Base.new(issue, project).required? end def send_private_messages - PrivateMessageSender.new(issue, project).call + Intouch::Live::Message::Private.new(issue, project).send end def send_group_messages return unless need_group_message? - GroupMessageSender.new(issue, project).call + Intouch::Live::Message::Group.new(issue, project).send end def need_group_message? diff --git a/lib/intouch/group_message_sender.rb b/lib/intouch/live/message/group.rb similarity index 80% rename from lib/intouch/group_message_sender.rb rename to lib/intouch/live/message/group.rb index 6e2ab86..1507d3b 100644 --- a/lib/intouch/group_message_sender.rb +++ b/lib/intouch/live/message/group.rb @@ -1,7 +1,5 @@ -module Intouch - class GroupMessageSender - extend ServiceInitializer - +module Intouch::Live::Message + class Group attr_reader :project, :issue def initialize(issue, project) @@ -9,7 +7,7 @@ def initialize(issue, project) @project = project end - def call + def send return unless telegram_enabled? IntouchSender.send_live_telegram_group_message(issue.id) diff --git a/lib/intouch/private_message_sender.rb b/lib/intouch/live/message/private.rb similarity index 83% rename from lib/intouch/private_message_sender.rb rename to lib/intouch/live/message/private.rb index f229572..a5ea4a8 100644 --- a/lib/intouch/private_message_sender.rb +++ b/lib/intouch/live/message/private.rb @@ -1,7 +1,5 @@ -module Intouch - class PrivateMessageSender - extend ServiceInitializer - +module Intouch::Live::Message + class Private attr_reader :project, :issue def initialize(issue, project) @@ -9,7 +7,7 @@ def initialize(issue, project) @project = project end - def call + def send return unless need_private_message? send_telegram_private_messages @@ -19,7 +17,7 @@ def call private def need_private_message? - required_checker.call + required_checker.required? end def required_recipients @@ -47,7 +45,7 @@ def email_enabled? end def required_checker - @required_checker ||= Checker::PrivateMessageRequired.new(issue, project) + @required_checker ||= Intouch::Live::Checker::Private.new(issue, project) end end end diff --git a/lib/intouch/patches/issue_patch.rb b/lib/intouch/patches/issue_patch.rb index 8084134..7df6031 100644 --- a/lib/intouch/patches/issue_patch.rb +++ b/lib/intouch/patches/issue_patch.rb @@ -78,20 +78,11 @@ def notificable_for_state?(state) end def recipient_ids(protocol, state = notification_state) - if project.send("active_#{protocol}_settings") && state && project.send("active_#{protocol}_settings")[state] - project.send("active_#{protocol}_settings")[state].map do |key, value| - case key - when 'author' - author.id - when 'assigned_to' - assigned_to_id if assigned_to.class == User - when 'watchers' - watchers.pluck(:user_id) - when 'user_groups' - Group.where(id: value).map(&:user_ids).flatten if value.present? - end - end.flatten.uniq + [assigner_id] - end + Intouch::Regular::RecipientsList.new( + issue: self, + state: state, + protocol: protocol + ).recipient_ids end def live_recipient_ids(protocol) @@ -118,7 +109,11 @@ def live_recipient_ids(protocol) end def intouch_recipients(protocol, state = notification_state) - User.where(id: recipient_ids(protocol, state)) + Intouch::Regular::RecipientsList.new( + issue: self, + state: state, + protocol: protocol + ).call end def intouch_live_recipients(protocol) @@ -233,15 +228,13 @@ def telegram_live_message end def telegram_message - Intouch::Message::Regular.call(self) + Intouch::Regular::Message::Base.new(self).base_message end private - require_relative '../new_issue_handler' - def handle_new_issue - Intouch::NewIssueHandler.call(self) + Intouch::Live::Handler::NewIssue.new(self).call end end end diff --git a/lib/intouch/patches/journal_patch.rb b/lib/intouch/patches/journal_patch.rb index 252304b..f5172e0 100644 --- a/lib/intouch/patches/journal_patch.rb +++ b/lib/intouch/patches/journal_patch.rb @@ -10,7 +10,7 @@ def self.included(base) # :nodoc: private def handle_updated_issue - Intouch::UpdatedIssueHandler.new(self).call + Intouch::Live::Handler::UpdatedIssue.new(self).call end end end diff --git a/lib/intouch/regular/message/base.rb b/lib/intouch/regular/message/base.rb index 4ce1552..dc767fe 100644 --- a/lib/intouch/regular/message/base.rb +++ b/lib/intouch/regular/message/base.rb @@ -1,5 +1,3 @@ -require_relative '../../service_initializer' - module Intouch::Regular::Message class Base extend Intouch::ServiceInitializer @@ -13,10 +11,6 @@ def initialize(issue) @issue = issue end - def call - base_message - end - def base_message @base_message ||= [ unassigned_message, diff --git a/lib/intouch/regular/message/private.rb b/lib/intouch/regular/message/private.rb index ef83273..dedb453 100644 --- a/lib/intouch/regular/message/private.rb +++ b/lib/intouch/regular/message/private.rb @@ -9,11 +9,19 @@ def initialize(issue:, user:, state:, project:) attr_reader :issue, :user, :state, :project - def call - message + def send + return unless telegram_account.present? && telegram_account.active? + + TelegramMessageSender.perform_async(telegram_account.telegram_id, message) + end + + def telegram_account + @telegram_account ||= user.telegram_account end def message + Intouch.set_locale + prefix.present? ? "#{prefix}\n#{base_message}" : base_message end diff --git a/lib/intouch/regular/recipients_list.rb b/lib/intouch/regular/recipients_list.rb index 5155d7e..85d1a99 100644 --- a/lib/intouch/regular/recipients_list.rb +++ b/lib/intouch/regular/recipients_list.rb @@ -1,6 +1,6 @@ module Intouch::Regular class RecipientsList - def initialize(issue:, protocol:, state:) + def initialize(issue:, protocol:, state:) @issue = issue @protocol = protocol @state = state @@ -8,5 +8,53 @@ def initialize(issue:, protocol:, state:) attr_reader :issue, :protocol, :state + def call + return [] unless recipient_ids.present? + + User.where(id: recipient_ids) + end + + def recipient_ids + return @recipient_ids if defined?(@recipient_ids) + + return unless state_settings.present? + + @recipient_ids = potential_recipient_ids & assigner_ids + end + + def potential_recipient_ids + state_settings.map do |key, value| + case key + when 'author' + issue.author_id + when 'assigned_to' + issue.assigned_to_id if issue.assigned_to.class == User + when 'watchers' + issue.watchers.pluck(:user_id) + when 'user_groups' + Group.where(id: value).map(&:user_ids).flatten if value.present? + end + end.flatten.uniq + [issue.assigner_id] + end + + def assigner_ids + @assigner_ids ||= project.assigner_ids + end + + def state_settings + return @state_settings if defined?(@state_settings) + + return nil unless active_protocol_settings.present? + + @state_settings = active_protocol_settings[state] + end + + def active_protocol_settings + @active_protocol_settings ||= project.send("active_#{protocol}_settings") + end + + def project + @project ||= issue.project + end end end \ No newline at end of file diff --git a/test/unit/intouch/checker/notification_required_test.rb b/test/unit/intouch/live/checker/base_test.rb similarity index 85% rename from test/unit/intouch/checker/notification_required_test.rb rename to test/unit/intouch/live/checker/base_test.rb index 310f68d..4375cd9 100644 --- a/test/unit/intouch/checker/notification_required_test.rb +++ b/test/unit/intouch/live/checker/base_test.rb @@ -1,7 +1,7 @@ -require File.expand_path('../../../../test_helper', __FILE__) +require File.expand_path('../../../../../test_helper', __FILE__) -class NotificationRequiredCheckerTest < ActiveSupport::TestCase - subject { Intouch::Checker::NotificationRequired.new(issue, project).call } +class Intouch::Live::Checker::BaseTest < ActiveSupport::TestCase + subject { Intouch::Live::Checker::Base.new(issue, project).required? } let(:project) { Object.new } let(:issue) { Object.new } diff --git a/test/unit/intouch/checker/private_message_required_test.rb b/test/unit/intouch/live/checker/private_test.rb similarity index 86% rename from test/unit/intouch/checker/private_message_required_test.rb rename to test/unit/intouch/live/checker/private_test.rb index d8c163c..13d968f 100644 --- a/test/unit/intouch/checker/private_message_required_test.rb +++ b/test/unit/intouch/live/checker/private_test.rb @@ -1,7 +1,7 @@ -require File.expand_path('../../../../test_helper', __FILE__) +require File.expand_path('../../../../../test_helper', __FILE__) -class PrivateMessageRequiredTest < ActiveSupport::TestCase - subject { Intouch::Checker::PrivateMessageRequired.new(issue, project).call } +class Intouch::Live::Checker::PrivateTest < ActiveSupport::TestCase + subject { Intouch::Live::Checker::Private.new(issue, project).required? } let(:project) { Object.new } let(:issue) { Object.new } diff --git a/test/unit/intouch/live/handler/new_issue_test.rb b/test/unit/intouch/live/handler/new_issue_test.rb new file mode 100644 index 0000000..3fa8b06 --- /dev/null +++ b/test/unit/intouch/live/handler/new_issue_test.rb @@ -0,0 +1,50 @@ +require File.expand_path('../../../../../test_helper', __FILE__) + +class Intouch::Live::Handler::NewIssueTest < ActiveSupport::TestCase + fixtures :projects + + subject { Intouch::Live::Handler::NewIssue.new(issue).call } + + let(:project) { Project.first } + let(:issue) { Issue.new(project: project) } + + describe 'send' do + before do + Intouch::Live::Checker::Base.any_instance + .stubs(:required?) + .returns(true) + end + + it 'private message' do + Intouch::Live::Message::Private.any_instance.expects(:send) + + subject + end + + it 'group message' do + Intouch::Live::Message::Group.any_instance.expects(:send) + + subject + end + end + + describe 'not send' do + before do + Intouch::Live::Checker::Base.any_instance + .stubs(:required?) + .returns(false) + end + + it 'private message' do + Intouch::Live::Message::Private.expects(:new).never + + subject + end + + it 'group message' do + Intouch::Live::Message::Group.expects(:new).never + + subject + end + end +end diff --git a/test/unit/intouch/new_issue_handler_test.rb b/test/unit/intouch/new_issue_handler_test.rb deleted file mode 100644 index b6bdbd6..0000000 --- a/test/unit/intouch/new_issue_handler_test.rb +++ /dev/null @@ -1,50 +0,0 @@ -require File.expand_path('../../../test_helper', __FILE__) - -class NewIssueHandlerTest < ActiveSupport::TestCase - fixtures :projects - - subject { Intouch::NewIssueHandler.new(issue).call } - - let(:project) { Project.first } - let(:issue) { Issue.new(project: project) } - - describe 'send' do - before do - Intouch::Checker::NotificationRequired.any_instance - .stubs(:call) - .returns(true) - end - - it 'private message' do - Intouch::PrivateMessageSender.expects(:call).with(issue, project) - - subject - end - - it 'group message' do - Intouch::GroupMessageSender.expects(:call).with(issue, project) - - subject - end - end - - describe 'not send' do - before do - Intouch::Checker::NotificationRequired.any_instance - .stubs(:call) - .returns(false) - end - - it 'private message' do - Intouch::PrivateMessageSender.expects(:call).never - - subject - end - - it 'group message' do - Intouch::GroupMessageSender.expects(:call).never - - subject - end - end -end From 7e020a29bfde90ece809bc0abd0aa8a5b4145064 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Mon, 27 Mar 2017 23:56:06 +1000 Subject: [PATCH 05/20] Fix test --- test/unit/intouch/live/handler/new_issue_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/intouch/live/handler/new_issue_test.rb b/test/unit/intouch/live/handler/new_issue_test.rb index 3fa8b06..2fee2bc 100644 --- a/test/unit/intouch/live/handler/new_issue_test.rb +++ b/test/unit/intouch/live/handler/new_issue_test.rb @@ -36,13 +36,13 @@ class Intouch::Live::Handler::NewIssueTest < ActiveSupport::TestCase end it 'private message' do - Intouch::Live::Message::Private.expects(:new).never + Intouch::Live::Message::Private.any_instance.expects(:send).never subject end it 'group message' do - Intouch::Live::Message::Group.expects(:new).never + Intouch::Live::Message::Group.any_instance.expects(:send).never subject end From 46e7f73c0c307ed56f943aca2eb9ec378aa753be Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 00:39:05 +1000 Subject: [PATCH 06/20] Add journal to live checker, add logging to issue update --- lib/intouch/live/checker/base.rb | 23 ++++++++++++++++----- lib/intouch/live/handler/new_issue.rb | 5 ++++- lib/intouch/live/handler/updated_issue.rb | 15 +++++++++++++- test/unit/intouch/live/checker/base_test.rb | 7 ++++++- 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/lib/intouch/live/checker/base.rb b/lib/intouch/live/checker/base.rb index 4a6f5cb..022e483 100644 --- a/lib/intouch/live/checker/base.rb +++ b/lib/intouch/live/checker/base.rb @@ -1,16 +1,29 @@ module Intouch::Live::Checker class Base - attr_reader :project, :issue + attr_reader :project, :issue, :journal - def initialize(issue, project) + def initialize(issue:, project:, journal: nil) @issue = issue @project = project + @journal = journal end def required? - project.module_enabled?(:intouch) && - project.active? && - !issue.closed? + project_enabled? && issue_open? + end + + def project_enabled? + project.module_enabled?(:intouch) && project.active? + end + + def issue_open? + !issue.reload.closed? || journal_issue_state_open? + end + + def journal_issue_state_open? + return false unless journal.present? + + journal.new_status && !journal.new_status&.is_closed? end end end diff --git a/lib/intouch/live/handler/new_issue.rb b/lib/intouch/live/handler/new_issue.rb index 7a310c9..1d64018 100644 --- a/lib/intouch/live/handler/new_issue.rb +++ b/lib/intouch/live/handler/new_issue.rb @@ -21,7 +21,10 @@ def call attr_reader :issue, :project def notification_required? - Intouch::Live::Checker::Base.new(issue, project).required? + Intouch::Live::Checker::Base.new( + issue: issue, + project: project + ).required? end def send_private_messages diff --git a/lib/intouch/live/handler/updated_issue.rb b/lib/intouch/live/handler/updated_issue.rb index c5b5d44..bfd2d41 100644 --- a/lib/intouch/live/handler/updated_issue.rb +++ b/lib/intouch/live/handler/updated_issue.rb @@ -11,8 +11,13 @@ def initialize(journal) end def call + logger.debug journal.inspect + logger.debug issue.inspect + return unless notification_required? + logger.info 'notification required' + send_private_messages send_group_messages end @@ -22,7 +27,11 @@ def call attr_reader :issue, :project, :journal def notification_required? - Intouch::Live::Checker::Base.new(issue, project).required? + Intouch::Live::Checker::Base.new( + issue: issue, + project: project, + journal: journal + ).required? end def send_private_messages @@ -38,5 +47,9 @@ def send_group_messages def need_group_message? (journal.details.pluck(:prop_key) & %w(priority_id status_id)).present? end + + def logger + @logger ||= Logger.new(Rails.root.join('log/intouch', 'live-updated.log')) + end end end diff --git a/test/unit/intouch/live/checker/base_test.rb b/test/unit/intouch/live/checker/base_test.rb index 4375cd9..1ffb679 100644 --- a/test/unit/intouch/live/checker/base_test.rb +++ b/test/unit/intouch/live/checker/base_test.rb @@ -1,7 +1,12 @@ require File.expand_path('../../../../../test_helper', __FILE__) class Intouch::Live::Checker::BaseTest < ActiveSupport::TestCase - subject { Intouch::Live::Checker::Base.new(issue, project).required? } + subject do + Intouch::Live::Checker::Base.new( + issue: issue, + project: project + ).required? + end let(:project) { Object.new } let(:issue) { Object.new } From 84119135e6657db64032dda7bdea4056a3525621 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 00:54:19 +1000 Subject: [PATCH 07/20] Remove issue reload (because test fails with it) --- lib/intouch/live/checker/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/intouch/live/checker/base.rb b/lib/intouch/live/checker/base.rb index 022e483..43484a6 100644 --- a/lib/intouch/live/checker/base.rb +++ b/lib/intouch/live/checker/base.rb @@ -17,7 +17,7 @@ def project_enabled? end def issue_open? - !issue.reload.closed? || journal_issue_state_open? + !issue.closed? || journal_issue_state_open? end def journal_issue_state_open? From 1c1f9e554aebf0e9469776a5e27bc9dd2ae501c6 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 10:54:44 +1000 Subject: [PATCH 08/20] Add bold to inactive message --- lib/intouch/message/formatter.rb | 4 ++++ lib/intouch/regular/message/base.rb | 24 ++++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/intouch/message/formatter.rb b/lib/intouch/message/formatter.rb index b1183fb..22cd4e6 100644 --- a/lib/intouch/message/formatter.rb +++ b/lib/intouch/message/formatter.rb @@ -41,6 +41,10 @@ def performer def attention(text) "*!!! #{text} !!!*" end + + def bold(text) + "*#{text}*" + end end end end diff --git a/lib/intouch/regular/message/base.rb b/lib/intouch/regular/message/base.rb index dc767fe..5ad0a2d 100644 --- a/lib/intouch/regular/message/base.rb +++ b/lib/intouch/regular/message/base.rb @@ -4,7 +4,7 @@ class Base attr_reader :issue - delegate :title, :assigned_to, :priority, :status, :link, + delegate :title, :assigned_to, :priority, :status, :link, :bold, to: :formatter def initialize(issue) @@ -32,23 +32,31 @@ def basic_message end def unassigned_message - I18n.t('intouch.telegram_message.issue.notice.unassigned') if issue.unassigned? || issue.assigned_to_group? + return unless issue.unassigned? || issue.assigned_to_group? + + I18n.t('intouch.telegram_message.issue.notice.unassigned') end def overdue_message - I18n.t('intouch.telegram_message.issue.notice.overdue') if issue.overdue? + return unless issue.overdue? + + I18n.t('intouch.telegram_message.issue.notice.overdue') end def without_due_date_message - I18n.t('intouch.telegram_message.issue.notice.without_due_date') if without_due_date? - end + return unless without_due_date? - def without_due_date? - !issue.due_date.present? && issue.created_on < 1.day.ago + I18n.t('intouch.telegram_message.issue.notice.without_due_date') end def inactive_message - I18n.t('intouch.telegram_message.issue.inactive', hours: rounded_inactive_hours) if inactive? + return unless inactive? + + bold I18n.t('intouch.telegram_message.issue.inactive', hours: rounded_inactive_hours) + end + + def without_due_date? + !issue.due_date.present? && issue.created_on < 1.day.ago end def rounded_inactive_hours From 88bc22151f9ca841ccfffaed1d6713efaee60753 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 11:05:46 +1000 Subject: [PATCH 09/20] Update assigner group description --- .../settings/intouch/common/_assigner_groups.html.erb | 2 +- config/locales/en.yml | 4 +++- config/locales/ru.yml | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/views/projects/settings/intouch/common/_assigner_groups.html.erb b/app/views/projects/settings/intouch/common/_assigner_groups.html.erb index b2cc6c9..0dd06bb 100644 --- a/app/views/projects/settings/intouch/common/_assigner_groups.html.erb +++ b/app/views/projects/settings/intouch/common/_assigner_groups.html.erb @@ -1,6 +1,6 @@

- <%= raw t('intouch.project.settings.assigner_groups.description') %> + <%= t('intouch.project.settings.assigner_groups.description_html') %>

<% Group.find_each do |group| %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 12fafbe..ccde43c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -51,7 +51,9 @@ en: project: settings: assigner_groups: - description: Notifications addressed to Assigner, will be sent only if the Assigner is a part of one of the groups mentioned below. + description_html: | + Notifications addressed to Assigner, will be sent only if the Assigner is a part of one of the groups mentioned below.
+ Notifications about inactivity in issue will be sent only if the issue is assigned to the Assigner. common: copy: Copy copy_settings_from_tab: Copy the values from tab diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 33ce53b..97926c8 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -51,7 +51,9 @@ ru: project: settings: assigner_groups: - description: "Уведомления, адресованные Исполнителю, будут отправлены только, если Исполнитель входит в одну из отмеченных ниже групп." + description_html: | + Уведомления, адресованные Исполнителю, будут отправлены только, если Исполнитель входит в одну из отмеченных ниже групп.
+ Уведомления о бездействии в задачах будет отправлено только в случае, если задача назначена на Исполнителя. common: copy: "Скопировать" copy_settings_from_tab: "Скопировать значения со вкладки" From 2d3015fc8506df91999c95a1cce037e9f011097c Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 16:16:02 +1000 Subject: [PATCH 10/20] Reload issue and journal before check --- lib/intouch/live/checker/base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/intouch/live/checker/base.rb b/lib/intouch/live/checker/base.rb index 43484a6..e5e86ce 100644 --- a/lib/intouch/live/checker/base.rb +++ b/lib/intouch/live/checker/base.rb @@ -3,7 +3,7 @@ class Base attr_reader :project, :issue, :journal def initialize(issue:, project:, journal: nil) - @issue = issue + @issue = issue.reload @project = project @journal = journal end @@ -23,7 +23,7 @@ def issue_open? def journal_issue_state_open? return false unless journal.present? - journal.new_status && !journal.new_status&.is_closed? + journal.reload.new_status && !journal.new_status&.is_closed? end end end From 5d83034eadb3df679e01e962d645ab13ccf4c454 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 16:30:34 +1000 Subject: [PATCH 11/20] Add logger --- lib/intouch/live/checker/base.rb | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/intouch/live/checker/base.rb b/lib/intouch/live/checker/base.rb index e5e86ce..a312956 100644 --- a/lib/intouch/live/checker/base.rb +++ b/lib/intouch/live/checker/base.rb @@ -3,9 +3,14 @@ class Base attr_reader :project, :issue, :journal def initialize(issue:, project:, journal: nil) - @issue = issue.reload + @issue = issue @project = project @journal = journal + + logger.info 'Initialized with:' + logger.debug "Issue: #{issue.inspect}" + logger.debug "Project: #{project.inspect}" + logger.debug "Journal: #{journal.inspect}" end def required? @@ -13,17 +18,34 @@ def required? end def project_enabled? + logger.info 'Project enabled?' + + logger.debug "project.module_enabled?(:intouch) => #{project.module_enabled?(:intouch).inspect}" + logger.debug "project.active? => #{project.active?.inspect}" + project.module_enabled?(:intouch) && project.active? end def issue_open? + logger.debug "!issue.closed? => #{!issue.closed?.inspect}" + !issue.closed? || journal_issue_state_open? end def journal_issue_state_open? + logger.debug "journal.present? => #{journal.present?.inspect}" return false unless journal.present? - journal.reload.new_status && !journal.new_status&.is_closed? + logger.debug "journal.new_status => #{journal.new_status.inspect}" + logger.debug "!journal.new_status&.is_closed? => #{!journal.new_status&.is_closed?.inspect}" + + journal.new_status && !journal.new_status&.is_closed? + end + + private + + def logger + @logger ||= Logger.new(Rails.root.join('log/intouch', 'live-checker-base.log')) end end end From d49bb634554d5dde1289156f0280ef1e286e43e7 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 17:29:46 +1000 Subject: [PATCH 12/20] Replace after_create with after_commit callback --- lib/intouch/patches/issue_patch.rb | 2 +- lib/intouch/patches/journal_patch.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/intouch/patches/issue_patch.rb b/lib/intouch/patches/issue_patch.rb index 7df6031..183da5b 100644 --- a/lib/intouch/patches/issue_patch.rb +++ b/lib/intouch/patches/issue_patch.rb @@ -10,7 +10,7 @@ def self.included(base) # :nodoc: # noinspection RubyArgCount store :intouch_data, accessors: %w(last_notification) - after_create :handle_new_issue + after_commit :handle_new_issue, on: :create def self.alarms Issue.where(priority_id: IssuePriority.alarm_ids) diff --git a/lib/intouch/patches/journal_patch.rb b/lib/intouch/patches/journal_patch.rb index f5172e0..4469b24 100644 --- a/lib/intouch/patches/journal_patch.rb +++ b/lib/intouch/patches/journal_patch.rb @@ -5,7 +5,7 @@ def self.included(base) # :nodoc: base.class_eval do unloadable - after_create :handle_updated_issue + after_commit :handle_updated_issue, on: :create private From b29654b6a63f9e8489f59af3673feef5dd941c22 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 17:34:31 +1000 Subject: [PATCH 13/20] Add sleep --- lib/intouch/live/handler/updated_issue.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/intouch/live/handler/updated_issue.rb b/lib/intouch/live/handler/updated_issue.rb index bfd2d41..297f5ef 100644 --- a/lib/intouch/live/handler/updated_issue.rb +++ b/lib/intouch/live/handler/updated_issue.rb @@ -27,6 +27,7 @@ def call attr_reader :issue, :project, :journal def notification_required? + sleep 1 Intouch::Live::Checker::Base.new( issue: issue, project: project, From 382827ef3703b05e71b0c282c89cff6b2497e3ca Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Tue, 28 Mar 2017 17:43:41 +1000 Subject: [PATCH 14/20] Move live handler call to worker --- app/workers/live_handler_worker.rb | 8 ++++++++ lib/intouch/live/handler/updated_issue.rb | 1 - lib/intouch/patches/journal_patch.rb | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 app/workers/live_handler_worker.rb diff --git a/app/workers/live_handler_worker.rb b/app/workers/live_handler_worker.rb new file mode 100644 index 0000000..ac53491 --- /dev/null +++ b/app/workers/live_handler_worker.rb @@ -0,0 +1,8 @@ +class LiveHandlerWorker + include Sidekiq::Worker + + def perform(journal_id) + journal = Journal.find(journal_id) + Intouch::Live::Handler::UpdatedIssue.new(journal).call + end +end \ No newline at end of file diff --git a/lib/intouch/live/handler/updated_issue.rb b/lib/intouch/live/handler/updated_issue.rb index 297f5ef..bfd2d41 100644 --- a/lib/intouch/live/handler/updated_issue.rb +++ b/lib/intouch/live/handler/updated_issue.rb @@ -27,7 +27,6 @@ def call attr_reader :issue, :project, :journal def notification_required? - sleep 1 Intouch::Live::Checker::Base.new( issue: issue, project: project, diff --git a/lib/intouch/patches/journal_patch.rb b/lib/intouch/patches/journal_patch.rb index 4469b24..9a188e9 100644 --- a/lib/intouch/patches/journal_patch.rb +++ b/lib/intouch/patches/journal_patch.rb @@ -10,7 +10,7 @@ def self.included(base) # :nodoc: private def handle_updated_issue - Intouch::Live::Handler::UpdatedIssue.new(self).call + LiveHandlerWorker.perform_in(5.seconds, id) end end end From 5a3802af51472f874dc89d35a41b024458d4d8a5 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Wed, 29 Mar 2017 09:06:00 +1000 Subject: [PATCH 15/20] Fix notificable checker for telegram group sender worker --- app/workers/telegram_group_sender_worker.rb | 50 ++++++++++++++++----- app/workers/telegram_sender_worker.rb | 2 +- lib/intouch/regular/message/base.rb | 2 + lib/intouch/regular/message/private.rb | 4 +- 4 files changed, 44 insertions(+), 14 deletions(-) diff --git a/app/workers/telegram_group_sender_worker.rb b/app/workers/telegram_group_sender_worker.rb index 4642e8f..40348de 100644 --- a/app/workers/telegram_group_sender_worker.rb +++ b/app/workers/telegram_group_sender_worker.rb @@ -1,22 +1,50 @@ class TelegramGroupSenderWorker include Sidekiq::Worker - TELEGRAM_GROUP_SENDER_LOG = Logger.new(Rails.root.join('log/intouch', 'telegram-group-sender.log')) def perform(issue_id, group_ids, state) return unless group_ids.present? - Intouch.set_locale - issue = Issue.find issue_id + @issue = Issue.find_by(id: issue_id) + @group_ids = group_ids + @state = state - return unless issue.notificable_for_state? state + return unless @issue.present? + return unless notificable? + return unless groups.present? - message = issue.telegram_message + groups.each { |group| send_message(group) } + end + + private + + attr_reader :issue, :state, :group_ids + + def notificable? + Intouch::Regular::Checker::Base.new( + issue: issue, + state: state, + project: project + ).required? + end + + def groups + @groups ||= TelegramGroupChat.where(id: group_ids).uniq + end + + def send_message(group) + return unless group.tid.present? + TelegramMessageSender.perform_async(-group.tid, message) + end + + def message + @message ||= issue.telegram_message + end + + def project + @project ||= issue.project + end - TelegramGroupChat.where(id: group_ids).uniq.each do |group| - next unless group.tid.present? - TelegramMessageSender.perform_async(-group.tid, message) - end - rescue ActiveRecord::RecordNotFound => e - # ignore + def logger + @logger ||= Logger.new(Rails.root.join('log/intouch', 'telegram-group-sender.log')) end end diff --git a/app/workers/telegram_sender_worker.rb b/app/workers/telegram_sender_worker.rb index 86ff82f..a58cf89 100644 --- a/app/workers/telegram_sender_worker.rb +++ b/app/workers/telegram_sender_worker.rb @@ -6,8 +6,8 @@ class TelegramSenderWorker include Sidekiq::Worker def perform(issue_id, state) - @state = state @issue = Issue.find_by(id: issue_id) + @state = state return unless @issue.present? return unless notificable? diff --git a/lib/intouch/regular/message/base.rb b/lib/intouch/regular/message/base.rb index 5ad0a2d..30ee27c 100644 --- a/lib/intouch/regular/message/base.rb +++ b/lib/intouch/regular/message/base.rb @@ -9,6 +9,8 @@ class Base def initialize(issue) @issue = issue + + Intouch.set_locale end def base_message diff --git a/lib/intouch/regular/message/private.rb b/lib/intouch/regular/message/private.rb index dedb453..4212ba3 100644 --- a/lib/intouch/regular/message/private.rb +++ b/lib/intouch/regular/message/private.rb @@ -5,6 +5,8 @@ def initialize(issue:, user:, state:, project:) @user = user @state = state @project = project + + Intouch.set_locale end attr_reader :issue, :user, :state, :project @@ -20,8 +22,6 @@ def telegram_account end def message - Intouch.set_locale - prefix.present? ? "#{prefix}\n#{base_message}" : base_message end From 7dee1bbfbbeccce2c70b42a2f30da3a08dec09ca Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Thu, 30 Mar 2017 10:09:16 +1000 Subject: [PATCH 16/20] Add logger to private message --- lib/intouch/regular/message/private.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/intouch/regular/message/private.rb b/lib/intouch/regular/message/private.rb index 4212ba3..33206f6 100644 --- a/lib/intouch/regular/message/private.rb +++ b/lib/intouch/regular/message/private.rb @@ -14,6 +14,13 @@ def initialize(issue:, user:, state:, project:) def send return unless telegram_account.present? && telegram_account.active? + logger.info '=========================================' + logger.info "Notification for state: #{state}" + logger.info message + logger.debug issue.inspect + logger.debug user.inspect + logger.info '=========================================' + TelegramMessageSender.perform_async(telegram_account.telegram_id, message) end @@ -55,5 +62,9 @@ def recipients def settings @settings ||= project.active_telegram_settings.try(:[], state) end + + def logger + @logger ||= Logger.new(Rails.root.join('log/intouch', 'regular-message-private.log')) + end end end From de117283d26cb49a8ada24a563685ef49d057472 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Thu, 30 Mar 2017 10:18:21 +1000 Subject: [PATCH 17/20] Fix undefined telegram_group bug --- lib/intouch/patches/issue_patch.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/intouch/patches/issue_patch.rb b/lib/intouch/patches/issue_patch.rb index 183da5b..369a06e 100644 --- a/lib/intouch/patches/issue_patch.rb +++ b/lib/intouch/patches/issue_patch.rb @@ -220,7 +220,7 @@ def telegram_live_message message += "\n#{Intouch.issue_url(id)}" - if telegram_group.present? && telegram_group.shared_url.present? + if defined?(telegram_group) && telegram_group&.shared_url.present? message += ", [#{I18n.t('intouch.telegram_message.issue.telegram_link')}](#{telegram_group.shared_url})" end From ce67f0fca40fa06d38f9e20c440c3046b69edb96 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Thu, 30 Mar 2017 10:33:20 +1000 Subject: [PATCH 18/20] Increase limit for telegram group ids --- .../010_change_tid_limit_in_telegram_group_chats.rb | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 db/migrate/010_change_tid_limit_in_telegram_group_chats.rb diff --git a/db/migrate/010_change_tid_limit_in_telegram_group_chats.rb b/db/migrate/010_change_tid_limit_in_telegram_group_chats.rb new file mode 100644 index 0000000..51d896e --- /dev/null +++ b/db/migrate/010_change_tid_limit_in_telegram_group_chats.rb @@ -0,0 +1,9 @@ +class ChangeTidLimitInTelegramGroupChats < ActiveRecord::Migration + def up + change_column :telegram_group_chats, :tid, :integer, limit: 8 + end + + def down + change_column :telegram_group_chats, :tid, :integer, limit: 4 + end +end From 2b56703692d458bf4c51e0a283dd58e749852580 Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Thu, 30 Mar 2017 10:37:27 +1000 Subject: [PATCH 19/20] Update release info --- CHANGELOG.md | 5 +++++ init.rb | 16 +++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49af5e4..b2bf615 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.6.0 + +* Regular and live message refactoring and tuning +* Fix issues #33 and #43 + # 0.5.3 * Fix: Always send live message for required recipients for settings template diff --git a/init.rb b/init.rb index 27b6b18..8f83eba 100644 --- a/init.rb +++ b/init.rb @@ -13,17 +13,23 @@ name 'Redmine Intouch plugin' url 'https://github.com/centosadmin/redmine_intouch' description 'This is a plugin for Redmine which sends a reminder email and Telegram messages to the assignee workign on a task, whose status is not updated with-in allowed duration' - version '0.5.3' + version '0.6.0' author 'Centos-admin.ru' author_url 'https://centos-admin.ru' requires_redmine version_or_higher: '3.0' - settings(default: { 'active_protocols' => %w(email), 'work_day_from' => '10:00', 'work_day_to' => '18:00' }, - partial: 'settings/intouch') + settings( + default: { + 'active_protocols' => %w(email), + 'work_day_from' => '10:00', + 'work_day_to' => '18:00' + }, + partial: 'settings/intouch') project_module :intouch do - permission :manage_intouch_settings, projects: :settings, - intouch_settings: :save + permission :manage_intouch_settings, + projects: :settings, + intouch_settings: :save end end From d8d9be4cfe75b31a1d4b0da932a9e15da260b14d Mon Sep 17 00:00:00 2001 From: Artur Trofimov Date: Thu, 30 Mar 2017 10:39:24 +1000 Subject: [PATCH 20/20] Add links to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2bf615..4035d36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # 0.6.0 * Regular and live message refactoring and tuning -* Fix issues #33 and #43 +* Fix issues [#33](https://github.com/centosadmin/redmine_intouch/issues/33) and [#43](https://github.com/centosadmin/redmine_intouch/issues/43) # 0.5.3