diff --git a/Gemfile b/Gemfile index e8da99956..bd1615fa2 100644 --- a/Gemfile +++ b/Gemfile @@ -112,11 +112,15 @@ group :development do #gem 'ruby-dbus' if RUBY_PLATFORM =~ /linux/i # Error at deploy end +group :development, :test do + gem 'rspec-rails', '~> 2.14.1' +end + group :test do - gem 'rspec-rails', '~> 2.14.1', group: 'development' gem 'factory_girl_rails', '~> 4.4.1' gem 'rr', '~> 1.1.2' gem 'shoulda' + gem 'shoulda-matchers' gem 'mock_redis', '~> 0.11' gem 'rake' end diff --git a/Gemfile.lock b/Gemfile.lock index bb06d2df2..3d2d4071f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -66,7 +66,7 @@ GEM angular-i18n (0.1.2) angularjs-rails (1.2.14) arel (4.0.2) - atomic (1.1.15) + atomic (1.1.16) attr_encrypted (1.3.2) encryptor (>= 1.3.0) bcrypt (3.1.7) @@ -534,6 +534,7 @@ DEPENDENCIES schema_plus (~> 1.4.0) shotgun shoulda + shoulda-matchers skype soundmanager-rails state_machine (~> 1.2) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index b8015af79..459dcab5a 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -18,12 +18,12 @@ class UserMailer < ActionMailer::Base end end - def new_comment_notification(comment, user) - @user, @comment = user, comment + def new_comment_notification(comment, user_id) + @user, @comment = User.find(user_id), comment subject = @comment.issue_comment? ? subject_for_issue(@comment.commentable) : I18n.t('notifications.subjects.new_commit_comment_notification') mail( - to: email_with_name(user, user.email), + to: email_with_name(@user, @user.email), subject: subject, from: email_with_name(comment.user) ) do |format| diff --git a/app/models/build_list.rb b/app/models/build_list.rb index 67701645e..a1a1c4c82 100644 --- a/app/models/build_list.rb +++ b/app/models/build_list.rb @@ -144,7 +144,7 @@ class BuildList < ActiveRecord::Base serialize :extra_build_lists, Array serialize :extra_params, Hash - after_commit :place_build, on: :create + after_create :place_build after_destroy :remove_container state_machine :status, initial: :waiting_for_response do diff --git a/app/models/comment.rb b/app/models/comment.rb index 0f1f1ba06..9b1878195 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -7,7 +7,7 @@ class Comment < ActiveRecord::Base # #Num ISSUES_REGEX = /(?:[a-zA-Z0-9\-_]*\/)?(?:[a-zA-Z0-9\-_]*)?#[0-9]+/ - belongs_to :commentable, polymorphic: true, touch: true + belongs_to :commentable, polymorphic: true belongs_to :user belongs_to :project serialize :data @@ -17,6 +17,7 @@ class Comment < ActiveRecord::Base scope :for_commit, ->(c) { where(commentable_id: c.id.hex, commentable_type: c.class) } default_scope { order(:created_at) } + before_save :touch_commentable after_create :subscribe_on_reply, unless: ->(c) { c.commit_comment? } after_create :subscribe_users @@ -186,8 +187,12 @@ class Comment < ActiveRecord::Base protected + def touch_commentable + commentable.touch unless commit_comment? + end + def subscribe_on_reply - commentable.subscribes.create(user_id: user_id) if !commentable.subscribes.exists?(user_id: user_id) + commentable.subscribes.where(user_id: user_id).first_or_create end def subscribe_users @@ -195,7 +200,7 @@ class Comment < ActiveRecord::Base commentable.subscribes.create(user: user) if !commentable.subscribes.exists?(user_id: user.id) elsif commit_comment? recipients = project.admins - recipients << user << User.where(email: commentable.try(:committer).try(:email)).first # commentor and committer + recipients << user << User.find_by_email(commentable.try(:committer).try(:email)) # commentor and committer recipients.compact.uniq.each do |user| options = {project_id: project.id, subscribeable_id: commentable_id, subscribeable_type: commentable.class.name, user_id: user.id} Subscribe.subscribe_to_commit(options) if Subscribe.subscribed_to_commit?(project, user, commentable) diff --git a/app/models/concerns/feed/comment.rb b/app/models/concerns/feed/comment.rb index 46b9684cc..7a8a6e830 100644 --- a/app/models/concerns/feed/comment.rb +++ b/app/models/concerns/feed/comment.rb @@ -2,7 +2,7 @@ module Feed::Comment extend ActiveSupport::Concern included do - after_commit :new_comment_notifications, on: :create + after_create :new_comment_notifications # dont remove outdated issues link after_update -> { Comment.create_link_on_issues_from_item(self) } end @@ -13,49 +13,54 @@ module Feed::Comment return if automatic? 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) + if user_id != subscribe.user_id && can_notify_on_new_comment?(subscribe) + UserMailer.new_comment_notification(self, subscribe.user_id).deliver 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 - } + { + user_id: subscribe.user_id, + 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 + } + }, without_protection: true ) 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 + next if !subscribe.user_id || own_comment?(subscribe.user) + if subscribe.user.notifier.can_notify && + ( (subscribe.project.owner?(subscribe.user) && subscribe.user.notifier.new_comment_commit_repo_owner) || + (subscribe.user.commentor?(self.commentable) && subscribe.user.notifier.new_comment_commit_commentor) || (subscribe.user.committer?(self.commentable) && subscribe.user.notifier.new_comment_commit_owner) ) - UserMailer.new_comment_notification(self, subscribe.user).deliver + UserMailer.new_comment_notification(self, subscribe.user_id).deliver end 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} + { + user_id: subscribe.user_id, + 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 + } + }, without_protection: true ) end end @@ -63,6 +68,7 @@ module Feed::Comment end def can_notify_on_new_comment?(subscribe) - User.find(subscribe.user).notifier.new_comment && User.find(subscribe.user).notifier.can_notify + notifier = SettingsNotifier.find_by_user_id(subscribe.user_id) + notifier && notifier.new_comment && notifier.can_notify end end diff --git a/app/models/concerns/feed/issue.rb b/app/models/concerns/feed/issue.rb index 1f36a7837..bd5da6ace 100644 --- a/app/models/concerns/feed/issue.rb +++ b/app/models/concerns/feed/issue.rb @@ -2,13 +2,13 @@ module Feed::Issue extend ActiveSupport::Concern included do - after_commit :new_issue_notifications, on: :create + after_create :new_issue_notifications - after_commit :send_assign_notifications, on: :create, if: ->(i) { i.assignee } - after_commit -> { send_assign_notifications(:update) }, on: :update + after_create :send_assign_notifications, if: ->(i) { i.assignee } + after_update -> { send_assign_notifications(:update) } - after_commit :send_hooks, on: :create - after_commit -> { send_hooks(:update) }, on: :update, if: ->(i) { i.previous_changes['status'].present? } + after_create :send_hooks + after_update -> { send_hooks(:update) }, if: ->(i) { i.previous_changes['status'].present? } end private diff --git a/app/models/concerns/feed/user.rb b/app/models/concerns/feed/user.rb index 66b7ff111..a6cccb90a 100644 --- a/app/models/concerns/feed/user.rb +++ b/app/models/concerns/feed/user.rb @@ -2,7 +2,7 @@ module Feed::User extend ActiveSupport::Concern included do - after_commit :new_user_notification, on: :create + after_create :new_user_notification end private diff --git a/app/models/concerns/git.rb b/app/models/concerns/git.rb index 64ee9947d..c1da4c7ba 100644 --- a/app/models/concerns/git.rb +++ b/app/models/concerns/git.rb @@ -11,8 +11,8 @@ module Git validates_attachment_content_type :srpm, content_type: ['application/octet-stream', "application/x-rpm", "application/x-redhat-package-manager"], message: I18n.t('layout.invalid_content_type') after_create :create_git_repo - after_commit(on: :create) {|p| p.fork_git_repo unless p.is_root?} # later with resque - after_commit(on: :create) {|p| p.import_attached_srpm if p.srpm?} # later with resque # should be after create_git_repo + after_create {|p| p.fork_git_repo unless p.is_root?} # later with resque + after_create {|p| p.import_attached_srpm if p.srpm?} # later with resque # should be after create_git_repo after_destroy :destroy_git_repo # after_rollback -> { destroy_git_repo rescue true if new_record? } diff --git a/app/models/mass_build.rb b/app/models/mass_build.rb index 94bf85bd0..f979c5676 100644 --- a/app/models/mass_build.rb +++ b/app/models/mass_build.rb @@ -19,7 +19,7 @@ class MassBuild < ActiveRecord::Base validates :projects_list, length: {maximum: 500_000}, presence: true validates_inclusion_of :auto_publish, :increase_release_tag, in: [true, false] - after_commit :build_all, on: :create + after_create :build_all before_validation :set_data, on: :create COUNT_STATUSES = [ diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb index 791a2a4ab..06382bf59 100644 --- a/app/models/subscribe.rb +++ b/app/models/subscribe.rb @@ -3,6 +3,8 @@ class Subscribe < ActiveRecord::Base belongs_to :user belongs_to :project + attr_accessible :status + def commit_subscribe? subscribeable_type == 'Grit::Commit' end @@ -35,7 +37,7 @@ class Subscribe < ActiveRecord::Base if subscribe = Subscribe.where(options).first subscribe.update_attributes(status: status) else - Subscribe.create(options.merge(status: status)) + Subscribe.create(options.merge(status: status), without_protection: true) end end