]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-2607 Provide email notifications on review changes
authorEvgeny Mandrikov <mandrikov@gmail.com>
Mon, 25 Jul 2011 21:08:01 +0000 (01:08 +0400)
committerEvgeny Mandrikov <mandrikov@gmail.com>
Mon, 25 Jul 2011 22:57:19 +0000 (02:57 +0400)
Update ruby code to send notifications on changes in reviews

sonar-server/src/main/java/org/sonar/server/notifications/reviews/ReviewsNotificationManager.java
sonar-server/src/main/webapp/WEB-INF/app/controllers/api/reviews_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/controllers/reviews_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/models/review.rb

index a89a5667acfed6f318a86cef007f700bc928f2d5..eeff3aa75a2a64c155b7411e2eab29f51847ed9d 100644 (file)
@@ -40,25 +40,6 @@ public class ReviewsNotificationManager implements ServerComponent {
     this.notificationManager = notificationManager;
   }
 
-  /**
-   * @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)
@@ -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"));
index b75d8a1048bf97b4b98ff26084f23e7f686c879c..be8d1ddcfbd0f0219b968d6352aac3b094497ac5 100644 (file)
@@ -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
index 92103a568463213049e3eeea6c4bdd613af7e54f..46bcf2a83abb21506f822270627f0a520c924911 100644 (file)
@@ -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    
 
index d4fe68c3808ed3183cb1a9373441d00115f45d8c..e376c832739f26270a1fa6cd0809190ede451879 100644 (file)
@@ -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