SONAR-2607 Provide email notifications on review changes

Update ruby code to send notifications on changes in reviews

4 files changed, 42 insertions, 57 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java b/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java
index a89a5667acf..eeff3aa75a2 100644
--- a/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java
+++ b/sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java
@@ -41,25 +41,6 @@ public class ReviewsNotificationManager implements ServerComponent {
   }
 
   /**
-   * @param reviewId id of review, which was modified
-   * @param author author of change (username)
-   * @param creator author of review (username)
-   * @param assignee current assignee (username)
-   * @param oldComment old text of comment
-   * @param comment new text of comment
-   */
-  public void notifyCommentChanged(Long reviewId, String author, String creator, String assignee, String oldComment, String newComment) {
-    Notification notification = new Notification("review-changed")
-        .setFieldValue("reviewId", String.valueOf(reviewId))
-        .setFieldValue("author", author)
-        .setFieldValue("creator", creator)
-        .setFieldValue("assignee", assignee)
-        .setFieldValue("old.comment", oldComment)
-        .setFieldValue("new.comment", newComment);
-    notificationManager.scheduleForSending(notification);
-  }
-
-  /**
    * @param reviewId reviewId id of review, which was modified
    * @param author author of change (username)
    * @param oldValues map of old values
@@ -67,7 +48,7 @@ public class ReviewsNotificationManager implements ServerComponent {
    */
   public void notifyChanged(Long reviewId, String author, Map<String, String> oldValues, Map<String, String> newValues) {
     Notification notification = new Notification("review-changed")
-        .setFieldValue("reviewId", author)
+        .setFieldValue("reviewId", String.valueOf(reviewId))
         .setFieldValue("author", author)
         .setFieldValue("creator", newValues.get("creator"))
         .setFieldValue("assignee", newValues.get("assignee"));
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb
index b75d8a1048b..be8d1ddcfbd 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb
@@ -99,7 +99,7 @@ class Api::ReviewsController < Api::ApiController
             review.set_false_positive(true, :user => current_user, :text => comment, :violation_id => violation_id)
           elsif resolution == 'FIXED'
             review.create_comment(:user => current_user, :text => comment)
-            review.resolve
+            review.resolve(current_user)
           else
             raise "Incorrect resolution."
           end
@@ -176,7 +176,7 @@ class Api::ReviewsController < Api::ApiController
           user = find_user(assignee)
           raise "Assignee not found." unless user
         end
-        review.reassign(user)
+        review.reassign(current_user, user)
       end
       render_reviews([review], params[:output] == 'HTML')
     rescue ApiException => e
@@ -215,7 +215,7 @@ class Api::ReviewsController < Api::ApiController
           review.set_false_positive(true, :user => current_user, :text => comment)
         elsif resolution == 'FIXED'
           review.create_comment(:user => current_user, :text => comment) unless comment.blank?
-          review.resolve
+          review.resolve(current_user)
         else
           raise "Incorrect resolution."
         end
@@ -253,7 +253,7 @@ class Api::ReviewsController < Api::ApiController
           raise "Comment must be provided." unless comment && !comment.blank?
           review.set_false_positive(false, :user => current_user, :text => comment)
         else
-          review.reopen
+          review.reopen(current_user)
           review.create_comment(:user => current_user, :text => comment) unless comment.blank?
         end
       end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
index 92103a56846..46bcf2a83ab 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
@@ -73,8 +73,7 @@ class ReviewsController < ApplicationController
     end
 
     assignee = findUserByLogin(params[:assignee_login]) unless params[:assignee_login].blank?
-    @review.assignee = assignee
-    @review.save
+    @review.reassign(current_user, assignee)
 
     render :partial => 'reviews/view'
   end
@@ -97,7 +96,7 @@ class ReviewsController < ApplicationController
     end
 
     if params[:comment_id]
-      @review.edit_comment(params[:comment_id].to_i, params[:text])
+      @review.edit_comment(current_user, params[:comment_id].to_i, params[:text])
     else
       @review.create_comment(:user => current_user, :text => params[:text])
     end
@@ -134,7 +133,7 @@ class ReviewsController < ApplicationController
     end
     
     if @review
-      @review.delete_comment(params[:comment_id].to_i)
+      @review.delete_comment(current_user, params[:comment_id].to_i)
     end
     render :partial => "reviews/view"
   end
@@ -146,12 +145,12 @@ class ReviewsController < ApplicationController
       render :text => "<b>Cannot create the comment</b> : access denied."
       return
     end
-    
+
     if @review.isResolved?
-      @review.reopen
+      @review.reopen(current_user)
     else
       # for the moment, if a review is not open, it can only be "RESOLVED"
-      @review.resolve
+      @review.resolve(current_user)
     end
 
     render :partial => "reviews/view"
@@ -186,8 +185,7 @@ class ReviewsController < ApplicationController
 
     violation.build_review(:user_id => current_user.id)
     assignee = findUserByLogin(params[:assignee_login]) unless params[:assignee_login].blank?
-    violation.review.assignee = assignee
-    violation.review.save!
+    violation.review.reassign(current_user, assignee)
     violation.save
 
     render :partial => "resource/violation", :locals => { :violation => violation }
@@ -245,7 +243,7 @@ class ReviewsController < ApplicationController
     end
 
     if params[:comment_id]
-      violation.review.edit_comment(params[:comment_id].to_i, params[:text])
+      violation.review.edit_comment(current_user, params[:comment_id].to_i, params[:text])
     else
       violation.review.create_comment(:user => current_user, :text => params[:text])
     end
@@ -262,7 +260,7 @@ class ReviewsController < ApplicationController
     end
     sanitize_violation(violation)
     if violation.review
-      violation.review.delete_comment(params[:comment_id].to_i)
+      violation.review.delete_comment(current_user, params[:comment_id].to_i)
     end
     render :partial => "resource/violation", :locals => { :violation => violation }
   end
@@ -279,10 +277,10 @@ class ReviewsController < ApplicationController
     if violation.review
       review = violation.review
       if review.isResolved?
-        review.reopen
+        review.reopen(current_user)
       else
         # for the moment, if a review is not open, it can only be "RESOLVED"
-        review.resolve
+        review.resolve(current_user)
       end
     end    
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
index d4fe68c3808..e376c832739 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/review.rb
@@ -61,35 +61,38 @@ class Review < ActiveRecord::Base
   def create_comment(params={})
     comment = comments.create!(params)
     touch
-    # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, nil, comment.text.to_java)
+    notification_manager.notifyChanged(id.to_i, comment.user.login.to_java, to_java_map, to_java_map("comment" => comment.text))
   end
 
-  def edit_comment(comment_id, comment_text)
+  def edit_comment(current_user, comment_id, comment_text)
     comment=comments.find(comment_id)
     if comment
+      old_comment_text=comment.text
       comment.text=comment_text
       comment.save!
       touch
+      notification_manager.notifyChanged(id.to_i, current_user.login.to_java, to_java_map("comment" => old_comment_text), to_java_map("comment" => comment.text))
     end
   end
 
-  def edit_last_comment(comment_text)
+  # TODO Godin: seems that this method not used anymore
+  def edit_last_comment(current_user, comment_text)
     comment=comments.last
     old_comment_text=comment.text
     comment.text=comment_text
     comment.save!
     touch
-    # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, old_comment.to_java, comment.text.to_java)
+    notification_manager.notifyChanged(id.to_i, current_user.login.to_java, to_java_map("comment" => old_comment_text), to_java_map("comment" => comment.text))
   end
 
-  def delete_comment(comment_id)
+  def delete_comment(current_user, comment_id)
     comment=comments.find(comment_id)
     comments.pop
     if comment
       old_comment_text=comment.text
       comment.delete
       touch
-      # TODO notification_manager.notifyCommentChanged(id.to_i, current_user.login.to_java, self.user.login.to_java, self.assignee.login.to_java, old_comment_text.to_java, nil)
+      notification_manager.notifyChanged(id.to_i, current_user.login.to_java, to_java_map("comment" => old_comment_text), to_java_map)
     end
   end
 
@@ -97,37 +100,40 @@ class Review < ActiveRecord::Base
     Java::OrgSonarServerUi::JRubyFacade.getInstance().getReviewsNotificationManager()
   end
 
-  def to_java_map()
+  def to_java_map(params = {})
     map = java.util.HashMap.new({
-      :creator => user.login.to_java,
-      :assignee => assignee == nil ? nil : assignee.login.to_java,
-      :status => status.to_java,
-      :resolution => resolution.to_java
+      "creator" => user.login.to_java,
+      "assignee" => assignee == nil ? nil : assignee.login.to_java,
+      "status" => status.to_java,
+      "resolution" => resolution.to_java
     })
+    params.each_pair do |k,v|
+      map.put(k.to_java, v.to_java)
+    end
     map
   end
 
-  def reassign(user)
+  def reassign(current_user, assignee)
     old = self.to_java_map
-    self.assignee = user
+    self.assignee = assignee
     self.save!
-    # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
+    notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
   end
 
-  def reopen
+  def reopen(current_user)
     old = self.to_java_map
     self.status = STATUS_REOPENED
     self.resolution = nil
     self.save!
-    # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
+    notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
   end
 
-  def resolve
+  def resolve(current_user)
     old = self.to_java_map
     self.status = STATUS_RESOLVED
     self.resolution = 'FIXED'
     self.save!
-    # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
+    notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
   end
 
   # params are mandatory:
@@ -146,13 +152,13 @@ class Review < ActiveRecord::Base
           violation.save!
         end
       end
-      create_comment(:user => params[:user], :text => params[:text])
+      comment = comments.create!(:user => params[:user], :text => params[:text])
       old = self.to_java_map
       self.assignee = nil
       self.status = is_false_positive ? STATUS_RESOLVED : STATUS_REOPENED
       self.resolution = is_false_positive ? 'FALSE-POSITIVE' : nil
       self.save!
-      # TODO notification_manager.notifyChanged(id.to_i, current_user.login.to_java, old, to_java_map)
+      notification_manager.notifyChanged(id.to_i, comment.user.login.to_java, old, to_java_map("comment" => comment.text))
     end
   end