#345: fixed some specs, etc

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

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)

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|

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

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)

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,10 +13,11 @@ 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,
+            {
+              user_id:  subscribe.user_id,
               kind:     'new_comment_notification',
               data:     {
                 user_name:       user.name,
@@ -30,20 +31,22 @@ module Feed::Comment
                 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,
+          {
+            user_id:  subscribe.user_id,
             kind:     'new_comment_commit_notification',
             data:     {
               user_name:      user.name,
@@ -55,7 +58,9 @@ module Feed::Comment
               project_id:     project.id,
               comment_id:     id,
               project_name:   project.name,
-            project_owner:  project.owner.uname}
+              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

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

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

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? }
 

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 = [

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