From eeed2a49b75e11ca3f173e28be13f9da89d8b2ad Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 17:06:42 +0400 Subject: [PATCH 1/8] #200: moved observer of Issue to lib --- Gemfile | 1 + Gemfile.lock | 2 + app/models/activity_feed_observer.rb | 78 ++++++++++--------- app/models/issue.rb | 1 + lib/modules/observers/activity_feed/issue.rb | 59 ++++++++++++++ .../lib/observers/activity_feed/issue_spec.rb | 30 +++++++ spec/mailers/user_mailer_spec.rb | 1 - spec/models/activity_feed_observer_spec.rb | 6 -- 8 files changed, 134 insertions(+), 44 deletions(-) create mode 100644 lib/modules/observers/activity_feed/issue.rb create mode 100644 spec/lib/observers/activity_feed/issue_spec.rb delete mode 100644 spec/models/activity_feed_observer_spec.rb diff --git a/Gemfile b/Gemfile index c9423e923..e55eacfff 100644 --- a/Gemfile +++ b/Gemfile @@ -95,5 +95,6 @@ group :test do gem 'rr', '~> 1.0.4' gem 'shoulda' gem 'mock_redis', '0.6.2' + gem 'test_after_commit', '0.2.0' gem 'rake' end diff --git a/Gemfile.lock b/Gemfile.lock index dfe789d3c..b5c710783 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -363,6 +363,7 @@ GEM state_machine (1.1.2) stringex (1.4.0) systemu (2.5.2) + test_after_commit (0.2.0) therubyracer (0.10.2) libv8 (~> 3.3.10) thin (1.5.1) @@ -466,6 +467,7 @@ DEPENDENCIES shotgun shoulda state_machine + test_after_commit (= 0.2.0) therubyracer (~> 0.10.2) therubyrhino (~> 1.73.1) trinidad (~> 1.0.2) diff --git a/app/models/activity_feed_observer.rb b/app/models/activity_feed_observer.rb index 85ca47126..ae3c1ae05 100644 --- a/app/models/activity_feed_observer.rb +++ b/app/models/activity_feed_observer.rb @@ -1,6 +1,6 @@ # -*- encoding : utf-8 -*- class ActivityFeedObserver < ActiveRecord::Observer - observe :issue, :comment, :user, :build_list + observe :comment, :user, :build_list BUILD_LIST_STATUSES = [ BuildList::BUILD_PUBLISHED, @@ -10,7 +10,11 @@ class ActivityFeedObserver < ActiveRecord::Observer BuildList::TESTS_FAILED ].freeze - def after_create(record) + def after_commit(record) + # See: + # - http://rails-bestpractices.com/posts/695-use-after_commit + # - https://coderwall.com/p/f5-vlq + return unless record.send(:transaction_include_action?, :create) case record.class.to_s when 'User' ActivityFeed.create( @@ -19,29 +23,29 @@ class ActivityFeedObserver < ActiveRecord::Observer :data => {:user_name => record.user_appeal, :user_email => record.email} ) - when 'Issue' - record.collect_recipients.each do |recipient| - next if record.user_id == recipient.id - UserMailer.new_issue_notification(record, recipient).deliver if recipient.notifier.can_notify && recipient.notifier.new_issue - ActivityFeed.create( - :user => recipient, - :kind => 'new_issue_notification', - :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id,:issue_serial_id => record.serial_id, - :issue_title => record.title, :project_id => record.project.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} - ) - end + # when 'Issue' + # record.collect_recipients.each do |recipient| + # next if record.user_id == recipient.id + # UserMailer.new_issue_notification(record, recipient).deliver if recipient.notifier.can_notify && recipient.notifier.new_issue + # ActivityFeed.create( + # :user => recipient, + # :kind => 'new_issue_notification', + # :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id,:issue_serial_id => record.serial_id, + # :issue_title => record.title, :project_id => record.project.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} + # ) + # end - if record.assignee_id_changed? - UserMailer.new_issue_notification(record, record.assignee).deliver if record.assignee.notifier.issue_assign && record.assignee.notifier.can_notify - ActivityFeed.create( - :user => record.user, - :kind => 'issue_assign_notification', - :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id, :issue_serial_id => record.serial_id, - :project_id => record.project.id, :issue_title => record.title, :project_name => record.project.name, :project_owner => record.project.owner.uname} - ) - end - record.project.hooks.each{ |h| h.receive_issues(record, :create) } - Comment.create_link_on_issues_from_item(record) + # if record.assignee_id_changed? + # UserMailer.new_issue_notification(record, record.assignee).deliver if record.assignee.notifier.issue_assign && record.assignee.notifier.can_notify + # ActivityFeed.create( + # :user => record.user, + # :kind => 'issue_assign_notification', + # :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id, :issue_serial_id => record.serial_id, + # :project_id => record.project.id, :issue_title => record.title, :project_name => record.project.name, :project_owner => record.project.owner.uname} + # ) + # end + # record.project.hooks.each{ |h| h.receive_issues(record, :create) } + # Comment.create_link_on_issues_from_item(record) when 'Comment' return if record.automatic if record.issue_comment? @@ -138,19 +142,19 @@ class ActivityFeedObserver < ActiveRecord::Observer def after_update(record) case record.class.to_s - when 'Issue' - if record.assignee_id && record.assignee_id_changed? - UserMailer.issue_assign_notification(record, record.assignee).deliver if record.assignee.notifier.issue_assign && record.assignee.notifier.can_notify - ActivityFeed.create( - :user => record.assignee, - :kind => 'issue_assign_notification', - :data => {:user_name => record.assignee.name, :user_email => record.assignee.email, :issue_serial_id => record.serial_id, :issue_title => record.title, - :project_id => record.project.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} - ) - end - record.project.hooks.each{ |h| h.receive_issues(record, :update) } if record.status_changed? - # dont remove outdated issues link - Comment.create_link_on_issues_from_item(record) + # when 'Issue' + # if record.assignee_id && record.assignee_id_changed? + # UserMailer.issue_assign_notification(record, record.assignee).deliver if record.assignee.notifier.issue_assign && record.assignee.notifier.can_notify + # ActivityFeed.create( + # :user => record.assignee, + # :kind => 'issue_assign_notification', + # :data => {:user_name => record.assignee.name, :user_email => record.assignee.email, :issue_serial_id => record.serial_id, :issue_title => record.title, + # :project_id => record.project.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} + # ) + # end + # record.project.hooks.each{ |h| h.receive_issues(record, :update) } if record.status_changed? + # # dont remove outdated issues link + # Comment.create_link_on_issues_from_item(record) when 'BuildList' if record.mass_build.blank? && ( # Do not show mass build activity in activity feeds record.status_changed? && BUILD_LIST_STATUSES.include?(record.status) || diff --git a/app/models/issue.rb b/app/models/issue.rb index c9236a489..9e043eb0a 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1,5 +1,6 @@ # -*- encoding : utf-8 -*- class Issue < ActiveRecord::Base + include Modules::Observers::ActivityFeed::Issue STATUSES = ['open', 'closed'] belongs_to :project diff --git a/lib/modules/observers/activity_feed/issue.rb b/lib/modules/observers/activity_feed/issue.rb new file mode 100644 index 000000000..d2ed1d016 --- /dev/null +++ b/lib/modules/observers/activity_feed/issue.rb @@ -0,0 +1,59 @@ +# -*- encoding : utf-8 -*- +module Modules::Observers::ActivityFeed::Issue + extend ActiveSupport::Concern + + included do + after_commit :new_issue_notifications, :on => :create + after_update -> { send_assign_notifications(:update) } + end + + private + + def new_issue_notifications + collect_recipients.each do |recipient| + next if user_id == recipient.id + UserMailer.new_issue_notification(self, recipient).deliver if recipient.notifier.can_notify && recipient.notifier.new_issue + ActivityFeed.create( + :user => recipient, + :kind => 'new_issue_notification', + :data => { + :user_name => user.name, + :user_email => user.email, + :user_id => user_id, + :issue_serial_id => serial_id, + :issue_title => title, + :project_id => project.id, + :project_name => project.name, + :project_owner => project.owner.uname + } + ) + end + send_assign_notifications + end + + def send_assign_notifications(action = :create) + if assignee_id && assignee_id_changed? + if assignee.notifier.issue_assign && assignee.notifier.can_notify + user_mailer_action = action == :create ? :new_issue_notification : :issue_assign_notification + UserMailer.send(user_mailer_action, self, assignee).deliver + end + ActivityFeed.create( + :user => assignee, + :kind => 'issue_assign_notification', + :data => { + :user_name => assignee.name, + :user_email => assignee.email, + :issue_serial_id => serial_id, + :issue_title => title, + :project_id => project.id, + :project_name => project.name, + :project_owner => project.owner.uname + } + ) + end + project.hooks.each{ |h| h.receive_issues(self, action) } if action == :create || status_changed? + # dont remove outdated issues link + Comment.create_link_on_issues_from_item(self) + end + +end \ No newline at end of file diff --git a/spec/lib/observers/activity_feed/issue_spec.rb b/spec/lib/observers/activity_feed/issue_spec.rb new file mode 100644 index 000000000..37e9d2730 --- /dev/null +++ b/spec/lib/observers/activity_feed/issue_spec.rb @@ -0,0 +1,30 @@ +# -*- encoding : utf-8 -*- +require 'spec_helper' + +describe Modules::Observers::ActivityFeed::Issue do + before { stub_symlink_methods } + + it 'sends a notification email after create' do + issue = FactoryGirl.build(:issue, :assignee => nil) + mailer = mock!.deliver + mock(UserMailer).new_issue_notification(issue, issue.project.owner) { mailer } + issue.save + end + + it 'does not send a notification email after update' do + issue = FactoryGirl.create(:issue, :assignee => nil) + issue.title = 'new title' + dont_allow(UserMailer).new_issue_notification + issue.save + end + + it 'sends a notification email after a assignee of issue has been changed' do + user = FactoryGirl.create(:user) + issue = FactoryGirl.build(:issue, :assignee => nil) + issue.assignee = user + mailer = mock!.deliver + mock(UserMailer).issue_assign_notification(issue, user) { mailer } + issue.save + end + +end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 72c8674ec..17dacb5a8 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -2,7 +2,6 @@ require "spec_helper" describe UserMailer do - pending "add some examples to (or delete) #{__FILE__}" context 'On Issue create' do before(:each) do diff --git a/spec/models/activity_feed_observer_spec.rb b/spec/models/activity_feed_observer_spec.rb deleted file mode 100644 index f008468e9..000000000 --- a/spec/models/activity_feed_observer_spec.rb +++ /dev/null @@ -1,6 +0,0 @@ -# -*- encoding : utf-8 -*- -require 'spec_helper' - -describe ActivityFeedObserver do - pending "add some examples to (or delete) #{__FILE__}" -end From 8782129985e6d03628b10fa80482f7c433280498 Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 18:37:41 +0400 Subject: [PATCH 2/8] #200: moved User and BuildList observers --- app/models/activity_feed_observer.rb | 128 +----------------- app/models/build_list.rb | 1 + app/models/comment.rb | 6 +- app/models/user.rb | 1 + .../observers/activity_feed/build_list.rb | 46 +++++++ .../observers/activity_feed/comment.rb | 59 ++++++++ lib/modules/observers/activity_feed/user.rb | 19 +++ spec/factories/comments.rb | 1 + .../observers/activity_feed/comment_spec.rb | 14 ++ 9 files changed, 144 insertions(+), 131 deletions(-) create mode 100644 lib/modules/observers/activity_feed/build_list.rb create mode 100644 lib/modules/observers/activity_feed/comment.rb create mode 100644 lib/modules/observers/activity_feed/user.rb create mode 100644 spec/lib/observers/activity_feed/comment_spec.rb diff --git a/app/models/activity_feed_observer.rb b/app/models/activity_feed_observer.rb index ae3c1ae05..532b945dc 100644 --- a/app/models/activity_feed_observer.rb +++ b/app/models/activity_feed_observer.rb @@ -1,88 +1,8 @@ # -*- encoding : utf-8 -*- class ActivityFeedObserver < ActiveRecord::Observer - observe :comment, :user, :build_list - BUILD_LIST_STATUSES = [ - BuildList::BUILD_PUBLISHED, - BuildList::SUCCESS, - BuildList::BUILD_ERROR, - BuildList::FAILED_PUBLISH, - BuildList::TESTS_FAILED - ].freeze - - def after_commit(record) - # See: - # - http://rails-bestpractices.com/posts/695-use-after_commit - # - https://coderwall.com/p/f5-vlq - return unless record.send(:transaction_include_action?, :create) + def after_create(record) case record.class.to_s - when 'User' - ActivityFeed.create( - :user => record, - :kind => 'new_user_notification', - :data => {:user_name => record.user_appeal, :user_email => record.email} - ) - - # when 'Issue' - # record.collect_recipients.each do |recipient| - # next if record.user_id == recipient.id - # UserMailer.new_issue_notification(record, recipient).deliver if recipient.notifier.can_notify && recipient.notifier.new_issue - # ActivityFeed.create( - # :user => recipient, - # :kind => 'new_issue_notification', - # :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id,:issue_serial_id => record.serial_id, - # :issue_title => record.title, :project_id => record.project.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} - # ) - # end - - # if record.assignee_id_changed? - # UserMailer.new_issue_notification(record, record.assignee).deliver if record.assignee.notifier.issue_assign && record.assignee.notifier.can_notify - # ActivityFeed.create( - # :user => record.user, - # :kind => 'issue_assign_notification', - # :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id, :issue_serial_id => record.serial_id, - # :project_id => record.project.id, :issue_title => record.title, :project_name => record.project.name, :project_owner => record.project.owner.uname} - # ) - # end - # record.project.hooks.each{ |h| h.receive_issues(record, :create) } - # Comment.create_link_on_issues_from_item(record) - when 'Comment' - return if record.automatic - if record.issue_comment? - subscribes = record.commentable.subscribes - subscribes.each do |subscribe| - if record.user_id != subscribe.user_id - UserMailer.new_comment_notification(record, subscribe.user).deliver if record.can_notify_on_new_comment?(subscribe) - ActivityFeed.create( - :user => subscribe.user, - :kind => 'new_comment_notification', - :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id, :comment_body => record.body, - :issue_title => record.commentable.title, :issue_serial_id => record.commentable.serial_id, :project_id => record.commentable.project.id, - :comment_id => record.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} - ) - end - end - elsif record.commit_comment? - subscribes = Subscribe.comment_subscribes(record).where(:status => true) - subscribes.each do |subscribe| - next if record.own_comment?(subscribe.user) - if subscribe.user.notifier.can_notify and - ( (subscribe.project.owner?(subscribe.user) && subscribe.user.notifier.new_comment_commit_repo_owner) or - (subscribe.user.commentor?(record.commentable) && subscribe.user.notifier.new_comment_commit_commentor) or - (subscribe.user.committer?(record.commentable) && subscribe.user.notifier.new_comment_commit_owner) ) - UserMailer.new_comment_notification(record, subscribe.user).deliver - end - ActivityFeed.create( - :user => subscribe.user, - :kind => 'new_comment_commit_notification', - :data => {:user_name => record.user.name, :user_email => record.user.email, :user_id => record.user_id, :comment_body => record.body, - :commit_message => record.commentable.message, :commit_id => record.commentable.id, - :project_id => record.project.id, :comment_id => record.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} - ) - end - end - Comment.create_link_on_issues_from_item(record) - when 'GitHook' return unless record.project PullRequest.where("from_project_id = ? OR to_project_id = ?", record.project, record.project).needed_checking.each {|pull| pull.check} @@ -140,50 +60,4 @@ class ActivityFeedObserver < ActiveRecord::Observer end end - def after_update(record) - case record.class.to_s - # when 'Issue' - # if record.assignee_id && record.assignee_id_changed? - # UserMailer.issue_assign_notification(record, record.assignee).deliver if record.assignee.notifier.issue_assign && record.assignee.notifier.can_notify - # ActivityFeed.create( - # :user => record.assignee, - # :kind => 'issue_assign_notification', - # :data => {:user_name => record.assignee.name, :user_email => record.assignee.email, :issue_serial_id => record.serial_id, :issue_title => record.title, - # :project_id => record.project.id, :project_name => record.project.name, :project_owner => record.project.owner.uname} - # ) - # end - # record.project.hooks.each{ |h| h.receive_issues(record, :update) } if record.status_changed? - # # dont remove outdated issues link - # Comment.create_link_on_issues_from_item(record) - when 'BuildList' - if record.mass_build.blank? && ( # Do not show mass build activity in activity feeds - record.status_changed? && BUILD_LIST_STATUSES.include?(record.status) || - record.status == BuildList::BUILD_PENDING && record.bs_id_changed? - ) - - record.project.admins.each do |recipient| - user = record.publisher || record.user - ActivityFeed.create( - :user => recipient, - :kind => 'build_list_notification', - :data => { - :task_num => record.bs_id, - :build_list_id => record.id, - :status => record.status, - :updated_at => record.updated_at, - :project_id => record.project_id, - :project_name => record.project.name, - :project_owner => record.project.owner.uname, - :user_name => user.name, - :user_email => user.email, - :user_id => user.id - } - ) - end - end - when 'Comment' - # dont remove outdated issues link - Comment.create_link_on_issues_from_item(record) - end - end end diff --git a/app/models/build_list.rb b/app/models/build_list.rb index 7ab46c41c..adf967c07 100644 --- a/app/models/build_list.rb +++ b/app/models/build_list.rb @@ -3,6 +3,7 @@ class BuildList < ActiveRecord::Base include Modules::Models::CommitAndVersion include Modules::Models::FileStoreClean include AbfWorker::ModelHelper + include Modules::Observers::ActivityFeed::BuildList belongs_to :project belongs_to :arch diff --git a/app/models/comment.rb b/app/models/comment.rb index f78148b86..7272d6a9b 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,5 +1,7 @@ # -*- encoding : utf-8 -*- class Comment < ActiveRecord::Base + include Modules::Observers::ActivityFeed::Comment + # regexp take from http://code.google.com/p/concerto-platform/source/browse/v3/cms/lib/CodeMirror/mode/gfm/gfm.js?spec=svn861&r=861#71 # User/Project#Num # User#Num @@ -54,10 +56,6 @@ class Comment < ActiveRecord::Base user_id == user.id end - def can_notify_on_new_comment?(subscribe) - User.find(subscribe.user).notifier.new_comment && User.find(subscribe.user).notifier.can_notify - end - def actual_inline_comment?(diff = nil, force = false) unless force raise "This is not inline comment!" if data.blank? # for debug diff --git a/app/models/user.rb b/app/models/user.rb index a53f530e1..1c4322cc2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -60,6 +60,7 @@ class User < Avatar include Modules::Models::PersonalRepository include Modules::Models::ActsLikeMember + include Modules::Observers::ActivityFeed::User def admin? role == 'admin' diff --git a/lib/modules/observers/activity_feed/build_list.rb b/lib/modules/observers/activity_feed/build_list.rb new file mode 100644 index 000000000..ca904a788 --- /dev/null +++ b/lib/modules/observers/activity_feed/build_list.rb @@ -0,0 +1,46 @@ +# -*- encoding : utf-8 -*- +module Modules::Observers::ActivityFeed::BuildList + extend ActiveSupport::Concern + + included do + after_update :build_list_notifications + end + + private + + def build_list_notifications + if mass_build.blank? && ( # Do not show mass build activity in activity feeds + status_changed? && [ + BUILD_PUBLISHED, + SUCCESS, + BUILD_ERROR, + FAILED_PUBLISH, + TESTS_FAILED + ].include?(status) || + status == BUILD_PENDING && bs_id_changed? + ) + + updater = publisher || user + project.admins.each do |recipient| + ActivityFeed.create( + :user => recipient, + :kind => 'build_list_notification', + :data => { + :task_num => bs_id, + :build_list_id => id, + :status => status, + :updated_at => updated_at, + :project_id => project_id, + :project_name => project.name, + :project_owner => project.owner.uname, + :user_name => updater.name, + :user_email => updater.email, + :user_id => updater.id + } + ) + end + + end + end + +end \ No newline at end of file diff --git a/lib/modules/observers/activity_feed/comment.rb b/lib/modules/observers/activity_feed/comment.rb new file mode 100644 index 000000000..920169b4d --- /dev/null +++ b/lib/modules/observers/activity_feed/comment.rb @@ -0,0 +1,59 @@ +# -*- encoding : utf-8 -*- +module Modules::Observers::ActivityFeed::Comment + extend ActiveSupport::Concern + + included do + after_commit :new_comment_notifications, :on => :create, :unless => :automatic? + # dont remove outdated issues link + after_update -> { Comment.create_link_on_issues_from_item(self) } + end + + private + + def new_comment_notifications + if issue_comment? + commentable.subscribes.each do |subscribe| + if user_id != subscribe.user_id + UserMailer.new_comment_notification(self, subscribe.user).deliver if can_notify_on_new_comment?(subscribe) + send_new_comment_commit_notification(subscribe) + end + end + elsif commit_comment? + Subscribe.comment_subscribes(self).where(:status => true).each do |subscribe| + next if own_comment?(subscribe.user) + if subscribe.user.notifier.can_notify and + ( (subscribe.project.owner?(subscribe.user) && subscribe.user.notifier.new_comment_commit_repo_owner) or + (subscribe.user.commentor?(self.commentable) && subscribe.user.notifier.new_comment_commit_commentor) or + (subscribe.user.committer?(self.commentable) && subscribe.user.notifier.new_comment_commit_owner) ) + UserMailer.new_comment_notification(self, subscribe.user).deliver + end + send_new_comment_commit_notification(subscribe) + end + end + Comment.create_link_on_issues_from_item(self) + end + + def send_new_comment_commit_notification(subscribe) + ActivityFeed.create( + :user => subscribe.user, + :kind => 'new_comment_commit_notification', + :data => { + :user_name => user.name, + :user_email => user.email, + :user_id => user_id, + :comment_body => body, + :commit_message => commentable.message, + :commit_id => commentable.id, + :project_id => project.id, + :comment_id => id, + :project_name => project.name, + :project_owner => project.owner.uname + } + ) + end + + def can_notify_on_new_comment?(subscribe) + User.find(subscribe.user).notifier.new_comment && User.find(subscribe.user).notifier.can_notify + end + +end \ No newline at end of file diff --git a/lib/modules/observers/activity_feed/user.rb b/lib/modules/observers/activity_feed/user.rb new file mode 100644 index 000000000..971378b30 --- /dev/null +++ b/lib/modules/observers/activity_feed/user.rb @@ -0,0 +1,19 @@ +# -*- encoding : utf-8 -*- +module Modules::Observers::ActivityFeed::User + extend ActiveSupport::Concern + + included do + after_commit :new_user_notification, :on => :create + end + + private + + def new_user_notification + ActivityFeed.create( + :user => self, + :kind => 'new_user_notification', + :data => {:user_name => self.user_appeal, :user_email => self.email} + ) + end + +end \ No newline at end of file diff --git a/spec/factories/comments.rb b/spec/factories/comments.rb index 145b50ca6..76119494c 100644 --- a/spec/factories/comments.rb +++ b/spec/factories/comments.rb @@ -4,5 +4,6 @@ FactoryGirl.define do body { FactoryGirl.generate(:string) } association :user, :factory => :user association :commentable, :factory => :issue + project { |c| c.commentable.project } end end diff --git a/spec/lib/observers/activity_feed/comment_spec.rb b/spec/lib/observers/activity_feed/comment_spec.rb new file mode 100644 index 000000000..139e9e114 --- /dev/null +++ b/spec/lib/observers/activity_feed/comment_spec.rb @@ -0,0 +1,14 @@ +# -*- encoding : utf-8 -*- +require 'spec_helper' + +describe Modules::Observers::ActivityFeed::Comment do + before { stub_symlink_methods } + + it 'sends a notification email after create a issue comment' do + comment = FactoryGirl.build(:comment) + mailer = mock!.deliver + mock(UserMailer).new_comment_notification(comment, comment.commentable.assignee) { mailer } + comment.save + end + +end From 66fe5c11cfb2d567527dc4cd18ebca7b3282bb10 Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 18:40:19 +0400 Subject: [PATCH 3/8] #200: removed activity_feed_observer --- app/models/git_hook.rb | 2 +- config/application.rb | 2 +- lib/modules/models/git.rb | 2 +- .../modules/observers/activity_feed/git.rb | 8 +++++--- 4 files changed, 8 insertions(+), 6 deletions(-) rename app/models/activity_feed_observer.rb => lib/modules/observers/activity_feed/git.rb (96%) diff --git a/app/models/git_hook.rb b/app/models/git_hook.rb index 0aef7df8a..d13829002 100644 --- a/app/models/git_hook.rb +++ b/app/models/git_hook.rb @@ -65,7 +65,7 @@ class GitHook end def self.process(*args) - ActivityFeedObserver.instance.after_create(args.size > 1 ? GitHook.new(*args) : args.first) + Modules::Observers::ActivityFeed::Git.create_notifications(args.size > 1 ? GitHook.new(*args) : args.first) end def find_user(user) diff --git a/config/application.rb b/config/application.rb index 13f0a15c1..c51dde67b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -34,7 +34,7 @@ module Rosa # config.plugins = [ :exception_notification, :ssl_requirement, :all ] # Activate observers that should always be running. - config.active_record.observers = :event_log_observer, :activity_feed_observer, :build_list_observer + config.active_record.observers = :event_log_observer, :build_list_observer # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. diff --git a/lib/modules/models/git.rb b/lib/modules/models/git.rb index 43cc16965..6d96bd851 100644 --- a/lib/modules/models/git.rb +++ b/lib/modules/models/git.rb @@ -150,7 +150,7 @@ module Modules module ClassMethods def process_hook(owner_uname, repo, newrev, oldrev, ref, newrev_type, user = nil, message = nil) rec = GitHook.new(owner_uname, repo, newrev, oldrev, ref, newrev_type, user, message) - ActivityFeedObserver.instance.after_create rec + Modules::Observers::ActivityFeed::Git.create_notifications rec end end end diff --git a/app/models/activity_feed_observer.rb b/lib/modules/observers/activity_feed/git.rb similarity index 96% rename from app/models/activity_feed_observer.rb rename to lib/modules/observers/activity_feed/git.rb index 532b945dc..cf5b19ebd 100644 --- a/app/models/activity_feed_observer.rb +++ b/lib/modules/observers/activity_feed/git.rb @@ -1,7 +1,8 @@ # -*- encoding : utf-8 -*- -class ActivityFeedObserver < ActiveRecord::Observer +module Modules::Observers::ActivityFeed::Git + + def self.create_notifications(record) - def after_create(record) case record.class.to_s when 'GitHook' return unless record.project @@ -58,6 +59,7 @@ class ActivityFeedObserver < ActiveRecord::Observer ) end end + end -end +end \ No newline at end of file From 0bd7b042c43ce3f9f7f15d2b47f7d3b6f138ee1f Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 19:00:56 +0400 Subject: [PATCH 4/8] #200: fixed: uninitialized constant --- lib/modules/observers/activity_feed/build_list.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/modules/observers/activity_feed/build_list.rb b/lib/modules/observers/activity_feed/build_list.rb index ca904a788..1873f01e4 100644 --- a/lib/modules/observers/activity_feed/build_list.rb +++ b/lib/modules/observers/activity_feed/build_list.rb @@ -11,13 +11,13 @@ module Modules::Observers::ActivityFeed::BuildList def build_list_notifications if mass_build.blank? && ( # Do not show mass build activity in activity feeds status_changed? && [ - BUILD_PUBLISHED, - SUCCESS, - BUILD_ERROR, - FAILED_PUBLISH, - TESTS_FAILED + BuildList::BUILD_PUBLISHED, + BuildList::SUCCESS, + BuildList::BUILD_ERROR, + BuildList::FAILED_PUBLISH, + BuildList::TESTS_FAILED ].include?(status) || - status == BUILD_PENDING && bs_id_changed? + status == BuildList::BUILD_PENDING && bs_id_changed? ) updater = publisher || user From 48bd3455737e218b4fc6f62808501ad00046e6cf Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 20:18:53 +0400 Subject: [PATCH 5/8] #200: rollback some changes --- .../observers/activity_feed/comment.rb | 52 +++++++++++-------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/lib/modules/observers/activity_feed/comment.rb b/lib/modules/observers/activity_feed/comment.rb index 920169b4d..8a12429a5 100644 --- a/lib/modules/observers/activity_feed/comment.rb +++ b/lib/modules/observers/activity_feed/comment.rb @@ -15,7 +15,22 @@ module Modules::Observers::ActivityFeed::Comment commentable.subscribes.each do |subscribe| if user_id != subscribe.user_id UserMailer.new_comment_notification(self, subscribe.user).deliver if can_notify_on_new_comment?(subscribe) - send_new_comment_commit_notification(subscribe) + ActivityFeed.create( + :user => subscribe.user, + :kind => 'new_comment_notification', + :data => { + :user_name => user.name, + :user_email => user.email, + :user_id => user_id, + :comment_body => body, + :issue_title => commentable.title, + :issue_serial_id => commentable.serial_id, + :project_id => commentable.project.id, + :comment_id => id, + :project_name => project.name, + :project_owner => project.owner.uname + } + ) end end elsif commit_comment? @@ -27,31 +42,26 @@ module Modules::Observers::ActivityFeed::Comment (subscribe.user.committer?(self.commentable) && subscribe.user.notifier.new_comment_commit_owner) ) UserMailer.new_comment_notification(self, subscribe.user).deliver end - send_new_comment_commit_notification(subscribe) + ActivityFeed.create( + :user => subscribe.user, + :kind => 'new_comment_commit_notification', + :data => { + :user_name => user.name, + :user_email => user.email, + :user_id => user_id, + :comment_body => body, + :commit_message => commentable.message, + :commit_id => commentable.id, + :project_id => project.id, + :comment_id => id, + :project_name => project.name, + :project_owner => project.owner.uname} + ) end end Comment.create_link_on_issues_from_item(self) end - def send_new_comment_commit_notification(subscribe) - ActivityFeed.create( - :user => subscribe.user, - :kind => 'new_comment_commit_notification', - :data => { - :user_name => user.name, - :user_email => user.email, - :user_id => user_id, - :comment_body => body, - :commit_message => commentable.message, - :commit_id => commentable.id, - :project_id => project.id, - :comment_id => id, - :project_name => project.name, - :project_owner => project.owner.uname - } - ) - end - def can_notify_on_new_comment?(subscribe) User.find(subscribe.user).notifier.new_comment && User.find(subscribe.user).notifier.can_notify end From 8d3d3c36f9e2ecd1e47398c83bdd4eb88e1aea54 Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 20:53:14 +0400 Subject: [PATCH 6/8] #200: updated specs --- Gemfile | 1 - Gemfile.lock | 2 -- spec/lib/observers/activity_feed/comment_spec.rb | 2 ++ spec/lib/observers/activity_feed/issue_spec.rb | 2 ++ 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index e55eacfff..c9423e923 100644 --- a/Gemfile +++ b/Gemfile @@ -95,6 +95,5 @@ group :test do gem 'rr', '~> 1.0.4' gem 'shoulda' gem 'mock_redis', '0.6.2' - gem 'test_after_commit', '0.2.0' gem 'rake' end diff --git a/Gemfile.lock b/Gemfile.lock index b5c710783..dfe789d3c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -363,7 +363,6 @@ GEM state_machine (1.1.2) stringex (1.4.0) systemu (2.5.2) - test_after_commit (0.2.0) therubyracer (0.10.2) libv8 (~> 3.3.10) thin (1.5.1) @@ -467,7 +466,6 @@ DEPENDENCIES shotgun shoulda state_machine - test_after_commit (= 0.2.0) therubyracer (~> 0.10.2) therubyrhino (~> 1.73.1) trinidad (~> 1.0.2) diff --git a/spec/lib/observers/activity_feed/comment_spec.rb b/spec/lib/observers/activity_feed/comment_spec.rb index 139e9e114..1c027a3e3 100644 --- a/spec/lib/observers/activity_feed/comment_spec.rb +++ b/spec/lib/observers/activity_feed/comment_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Modules::Observers::ActivityFeed::Comment do + self.use_transactional_fixtures = false before { stub_symlink_methods } it 'sends a notification email after create a issue comment' do @@ -11,4 +12,5 @@ describe Modules::Observers::ActivityFeed::Comment do comment.save end + before(:all) { User.destroy_all } end diff --git a/spec/lib/observers/activity_feed/issue_spec.rb b/spec/lib/observers/activity_feed/issue_spec.rb index 37e9d2730..f67179655 100644 --- a/spec/lib/observers/activity_feed/issue_spec.rb +++ b/spec/lib/observers/activity_feed/issue_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Modules::Observers::ActivityFeed::Issue do + self.use_transactional_fixtures = false before { stub_symlink_methods } it 'sends a notification email after create' do @@ -27,4 +28,5 @@ describe Modules::Observers::ActivityFeed::Issue do issue.save end + before(:all) { User.destroy_all } end From 391d5af18d4ccf1c9ae0fe9485c68803d55126e2 Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 23:10:11 +0400 Subject: [PATCH 7/8] #200: fixed specs --- spec/lib/observers/activity_feed/comment_spec.rb | 2 -- spec/lib/observers/activity_feed/issue_spec.rb | 2 -- spec/models/comment_for_commit_spec.rb | 1 + spec/models/comment_spec.rb | 1 + spec/models/issue_spec.rb | 1 + spec/spec_helper.rb | 1 + 6 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/lib/observers/activity_feed/comment_spec.rb b/spec/lib/observers/activity_feed/comment_spec.rb index 1c027a3e3..84b5065d5 100644 --- a/spec/lib/observers/activity_feed/comment_spec.rb +++ b/spec/lib/observers/activity_feed/comment_spec.rb @@ -11,6 +11,4 @@ describe Modules::Observers::ActivityFeed::Comment do mock(UserMailer).new_comment_notification(comment, comment.commentable.assignee) { mailer } comment.save end - - before(:all) { User.destroy_all } end diff --git a/spec/lib/observers/activity_feed/issue_spec.rb b/spec/lib/observers/activity_feed/issue_spec.rb index f67179655..be287e73b 100644 --- a/spec/lib/observers/activity_feed/issue_spec.rb +++ b/spec/lib/observers/activity_feed/issue_spec.rb @@ -27,6 +27,4 @@ describe Modules::Observers::ActivityFeed::Issue do mock(UserMailer).issue_assign_notification(issue, user) { mailer } issue.save end - - before(:all) { User.destroy_all } end diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 9972a699a..1d779ef0d 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -37,6 +37,7 @@ def should_not_send_email(args={}) end describe Comment do + self.use_transactional_fixtures = false before { stub_symlink_methods } context 'for global admin user' do before(:each) do diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 2226a8c13..51b7eb5a9 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -19,6 +19,7 @@ def create_comment_in_issue issue, body end describe Comment do + self.use_transactional_fixtures = false before { stub_symlink_methods } context 'for global admin user' do before(:each) do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 1777fcc92..a8635a625 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -13,6 +13,7 @@ def create_issue issue_owner end describe Issue do + self.use_transactional_fixtures = false before do stub_symlink_methods any_instance_of(Project, :versions => ['v1.0', 'v2.0']) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c233dd750..f5c167bfa 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -30,6 +30,7 @@ RSpec.configure do |config| config.before(:all) { init_test_root } config.after(:all) { clear_test_root } + config.after(:all) { User.destroy_all } end def set_session_for(user=nil) From 8a83a4cefa773780f059f2b2749b39ef0c527938 Mon Sep 17 00:00:00 2001 From: Vokhmin Alexey V Date: Thu, 27 Jun 2013 23:43:44 +0400 Subject: [PATCH 8/8] #200: removed duplicates of specs --- .../observers/activity_feed/comment.rb | 3 +- spec/factories/comments.rb | 1 + spec/factories/issues.rb | 1 + spec/factories/users.rb | 1 + .../observers/activity_feed/comment_spec.rb | 14 --------- .../lib/observers/activity_feed/issue_spec.rb | 30 ------------------- spec/models/comment_for_commit_spec.rb | 1 - spec/models/comment_spec.rb | 1 - spec/models/issue_spec.rb | 1 - spec/spec_helper.rb | 1 - 10 files changed, 5 insertions(+), 49 deletions(-) delete mode 100644 spec/lib/observers/activity_feed/comment_spec.rb delete mode 100644 spec/lib/observers/activity_feed/issue_spec.rb diff --git a/lib/modules/observers/activity_feed/comment.rb b/lib/modules/observers/activity_feed/comment.rb index 8a12429a5..0327fcb12 100644 --- a/lib/modules/observers/activity_feed/comment.rb +++ b/lib/modules/observers/activity_feed/comment.rb @@ -3,7 +3,7 @@ module Modules::Observers::ActivityFeed::Comment extend ActiveSupport::Concern included do - after_commit :new_comment_notifications, :on => :create, :unless => :automatic? + after_commit :new_comment_notifications, :on => :create # dont remove outdated issues link after_update -> { Comment.create_link_on_issues_from_item(self) } end @@ -11,6 +11,7 @@ module Modules::Observers::ActivityFeed::Comment private def new_comment_notifications + return if automatic? if issue_comment? commentable.subscribes.each do |subscribe| if user_id != subscribe.user_id diff --git a/spec/factories/comments.rb b/spec/factories/comments.rb index 76119494c..fdfb51a9c 100644 --- a/spec/factories/comments.rb +++ b/spec/factories/comments.rb @@ -5,5 +5,6 @@ FactoryGirl.define do association :user, :factory => :user association :commentable, :factory => :issue project { |c| c.commentable.project } + after(:create) { |c| c.send(:new_comment_notifications) } end end diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index a76caea56..1bacc7763 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -7,5 +7,6 @@ FactoryGirl.define do association :user, :factory => :user association :assignee, :factory => :user status "open" + after(:create) { |i| i.send(:new_issue_notifications) } end end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 8f8d138a9..37dab7ef1 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -7,6 +7,7 @@ FactoryGirl.define do password '123456' password_confirmation {|u| u.password} confirmed_at { Time.now.utc } + after(:create) { |u| u.send(:new_user_notification) } end factory :admin, :parent => :user do diff --git a/spec/lib/observers/activity_feed/comment_spec.rb b/spec/lib/observers/activity_feed/comment_spec.rb deleted file mode 100644 index 84b5065d5..000000000 --- a/spec/lib/observers/activity_feed/comment_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# -*- encoding : utf-8 -*- -require 'spec_helper' - -describe Modules::Observers::ActivityFeed::Comment do - self.use_transactional_fixtures = false - before { stub_symlink_methods } - - it 'sends a notification email after create a issue comment' do - comment = FactoryGirl.build(:comment) - mailer = mock!.deliver - mock(UserMailer).new_comment_notification(comment, comment.commentable.assignee) { mailer } - comment.save - end -end diff --git a/spec/lib/observers/activity_feed/issue_spec.rb b/spec/lib/observers/activity_feed/issue_spec.rb deleted file mode 100644 index be287e73b..000000000 --- a/spec/lib/observers/activity_feed/issue_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -# -*- encoding : utf-8 -*- -require 'spec_helper' - -describe Modules::Observers::ActivityFeed::Issue do - self.use_transactional_fixtures = false - before { stub_symlink_methods } - - it 'sends a notification email after create' do - issue = FactoryGirl.build(:issue, :assignee => nil) - mailer = mock!.deliver - mock(UserMailer).new_issue_notification(issue, issue.project.owner) { mailer } - issue.save - end - - it 'does not send a notification email after update' do - issue = FactoryGirl.create(:issue, :assignee => nil) - issue.title = 'new title' - dont_allow(UserMailer).new_issue_notification - issue.save - end - - it 'sends a notification email after a assignee of issue has been changed' do - user = FactoryGirl.create(:user) - issue = FactoryGirl.build(:issue, :assignee => nil) - issue.assignee = user - mailer = mock!.deliver - mock(UserMailer).issue_assign_notification(issue, user) { mailer } - issue.save - end -end diff --git a/spec/models/comment_for_commit_spec.rb b/spec/models/comment_for_commit_spec.rb index 1d779ef0d..9972a699a 100644 --- a/spec/models/comment_for_commit_spec.rb +++ b/spec/models/comment_for_commit_spec.rb @@ -37,7 +37,6 @@ def should_not_send_email(args={}) end describe Comment do - self.use_transactional_fixtures = false before { stub_symlink_methods } context 'for global admin user' do before(:each) do diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 51b7eb5a9..2226a8c13 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -19,7 +19,6 @@ def create_comment_in_issue issue, body end describe Comment do - self.use_transactional_fixtures = false before { stub_symlink_methods } context 'for global admin user' do before(:each) do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index a8635a625..1777fcc92 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -13,7 +13,6 @@ def create_issue issue_owner end describe Issue do - self.use_transactional_fixtures = false before do stub_symlink_methods any_instance_of(Project, :versions => ['v1.0', 'v2.0']) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f5c167bfa..c233dd750 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -30,7 +30,6 @@ RSpec.configure do |config| config.before(:all) { init_test_root } config.after(:all) { clear_test_root } - config.after(:all) { User.destroy_all } end def set_session_for(user=nil)