Merge pull request #122 from openHPI/clean-comments

Cleaner RFC comments

app/assets/stylesheets/request-for-comments.css.scss

@@ -17,3 +17,50 @@
   width: 100%;
   height: 200px;
 }
+
+.ace_tooltip {
+  display: none !important;
+}
+
+p.comment {
+  width: 400px;
+}
+
+.popover-header {
+  width: 100%;
+  overflow: hidden;
+  padding-bottom: 10px;
+  margin: auto;
+}
+
+.popover-username {
+  font-weight: bold;
+  width: 60%;
+  float: left;
+}
+
+.popover-date {
+  text-align: right;
+  color: #008cba;
+  margin-left: 60%;
+  font-size: x-small;
+}
+
+.popover-updated {
+  text-align: right;
+  margin-left: 60%;
+  font-size: x-small;
+}
+
+.popover-comment {
+  word-wrap: break-word;
+  margin-bottom: 10px;
+}
+
+.popover-divider {
+  width: 100%;
+  height: 1px;
+  background-color: #008cba;
+  overflow: hidden;
+  margin-bottom: 10px;
+}

app/controllers/comments_controller.rb

@@ -19,6 +19,8 @@ class CommentsController < ApplicationController
       @comments = Comment.where(file_id: params[:file_id])
       @comments.map{|comment|
         comment.username = comment.user.displayname
+        comment.date = comment.created_at.strftime('%d.%m.%Y %k:%M')
+        comment.updated = (comment.created_at != comment.updated_at)
       }
     else
       @comments = []

app/models/comment.rb

@@ -1,7 +1,7 @@
 class Comment < ActiveRecord::Base
   # inherit the creation module: encapsulates that this is a polymorphic user, offers some aliases and makes sure that all necessary attributes are set.
   include Creation
-  attr_accessor :username
+  attr_accessor :username, :date, :updated
 
   belongs_to :file, class_name: 'CodeOcean::File'
   belongs_to :user, polymorphic: true

app/policies/comment_policy.rb

@@ -12,7 +12,7 @@ class CommentPolicy < ApplicationPolicy
     everyone
   end
 
-  [:new?, :destroy?].each do |action|
+  [:new?, :destroy?, :update?].each do |action|
     define_method(action) { admin? || author? }
   end
 

app/views/comments/index.json.jbuilder

@@ -1,4 +1,4 @@
 json.array!(@comments) do |comment|
-  json.extract! comment, :id, :user_id, :file_id, :row, :column, :text, :username
+  json.extract! comment, :id, :user_id, :file_id, :row, :column, :text, :username, :date, :updated
   json.url comment_url(comment, format: :json)
 end

app/views/request_for_comments/show.html.erb

@@ -64,6 +64,8 @@
   </h5>
 </div>
 <hr>
+
+<div class="hidden sanitizer"></div>
 <!--
 do not put a carriage return in the line below. it will be present in the presentation of the source code, otherwise.
 also, all settings from the rails model needed for the editor configuration in the JavaScript are attached to the editor as data attributes here.
@@ -140,6 +142,18 @@ also, all settings from the rails model needed for the editor configuration in t
     currentEditor.on("guttermousedown", handleSidebarClick);
   });
 
+  function cleanupPopovers() {
+    // remove all possible popovers
+    $('.editor > .ace_gutter > .ace_gutter-layer > .ace_gutter-cell').popover('destroy');
+  }
+
+  function preprocess(commentText) {
+      // sanitize comments to deal with XSS attacks:
+      commentText = $('div.sanitizer').text(commentText).html();
+      // display original line breaks:
+      return commentText.replace(/\n/g, '<br>');
+  }
+
   function setAnnotations(editor, fileid) {
     var session = editor.getSession();
 
@@ -153,18 +167,65 @@ also, all settings from the rails model needed for the editor configuration in t
     });
 
     jqrequest.done(function(response){
-      $.each(response, function(index, comment) {
+      // comments need to be sorted to cluster them per line
-        comment.className = "code-ocean_comment";
+      var comments = response.slice().sort(function (a, b) {
+          return a.row - b.row;
+      });
+      while (comments.length > 0) {
+        // new cluster of comments
+        var cluster = [];
+        var clusterRow = comments[0].row;
+        // now collect all comments on this line
+        while (comments.length > 0 && comments[0].row === clusterRow) {
+          cluster.push(comments.shift());
+        }
+        // sort the comments by creation date
+        cluster = cluster.sort(function (a, b) {
+            return a.id - b.id;
+        });
 
+        // build the markup for the current line's popover
+        var popupContent = '';
+        cluster.forEach(function(comment, index) {
+            if (index !== 0) {
+                popupContent += '<div class="popover-divider"></div>'
+            }
+            popupContent += '<p class="comment">';
+            popupContent += '<div class="popover-header">' +
+                '<div class="popover-username">' + preprocess(comment.username) + '</div>' +
+                '<div class="popover-date">' + comment.date + '</div>';
+            if (comment.updated) {
+                popupContent += '<div class="popover-updated">' +
+                    '<i class="fa fa-pencil" aria-hidden="true"></i>' +
+                    '<%= t('request_for_comments.comment_edited') %>' +
+                    '</div>'
+            }
+            popupContent += '</div>';
+            popupContent += '<div class="popover-comment">' + preprocess(comment.text) + '</div>';
+            popupContent += '</p>';
+        });
+
+        // attach the popover to the ace sidebar (where the comment icon is displayed)
+        var icon = $('*[data-file-id="' + fileid + '"]')          // the editor for this file
+                .find('.ace_gutter > .ace_gutter-layer')          // the sidebar
+                .find('div:nth-child(' + (clusterRow + 1) + ')'); // the correct line
+        icon.popover({
+          content: popupContent,
+          html: true, // necessary to style comments. XSS is not possible due to comment pre-processing (sanitizing)
+          trigger: 'hover',
+          container: 'body'
+        });
+      }
+
+      $.each(response, function(index, comment) {
+        comment.className = 'code-ocean_comment';
         // if we have tabs or carriage returns in the comment, just add the name and leave it as it is. otherwise: format!
         if(comment.text.includes("\n") || comment.text.includes("\t")){
           comment.text = comment.username + ": " + comment.text;
         } else {
           comment.text = comment.username + ": " + stringDivider(comment.text, 80, "\n");
         }
-
       });
-
       session.setAnnotations(response);
     })
   }
@@ -182,6 +243,7 @@ also, all settings from the rails model needed for the editor configuration in t
   }
 
   function deleteComment(file_id, row, editor) {
+    cleanupPopovers();
     var jqxhr = $.ajax({
       type: 'DELETE',
       url: "/comments",
@@ -196,6 +258,7 @@ also, all settings from the rails model needed for the editor configuration in t
   }
 
   function createComment(file_id, row, editor, commenttext){
+    cleanupPopovers();
     var jqxhr = $.ajax({
       data: {
         comment: {

config/locales/de.yml

@@ -466,6 +466,7 @@ de:
     write_a_thank_you_node: "Wenn Sie möchten, können Sie sich bei allen Mitstudenten, die Ihnen bei der Beantwortung Ihrer Frage geholfen haben, bedanken:"
     send_thank_you_note: "Senden"
     cancel_thank_you_note: "Nichts senden"
+    comment_edited: "bearbeitet"
   sessions:
     create:
       failure: Fehlerhafte E-Mail oder Passwort.

config/locales/en.yml

@@ -487,6 +487,7 @@ en:
     write_a_thank_you_node: "If you want, you can write a thank you note to all your commenters:"
     send_thank_you_note: "Send"
     cancel_thank_you_note: "Don't send"
+    comment_edited: "edited"
   sessions:
     create:
       failure: Invalid email or password.