From 6229b5de8d77070793502c90e1bc143eedcc18b2 Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Wed, 24 Jun 2015 20:21:02 +0200
Subject: [PATCH 01/12] re-activate copying of comments, and also show comments
 of original file author in show method

---
 app/controllers/comments_controller.rb    | 7 +++++--
 app/controllers/submissions_controller.rb | 7 ++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index 7076ca81..4da2bf5c 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -14,7 +14,7 @@ class CommentsController < ApplicationController
   def index
     #@comments = Comment.all
     #if admin, show all comments.
-    #check whether user is the author of the passed file_id, if so, show all comments. otherwise, only show comments of auther and own comments
+    #check whether user is the author of the passed file_id, if so, show all comments. otherwise, only show comments of the file-author and own comments
     file = CodeOcean::File.find(params[:file_id])
     #there might be no submission yet, so dont use find
     submission = Submission.find_by(id: file.context_id)
@@ -31,7 +31,10 @@ class CommentsController < ApplicationController
         # fetch all comments for this file
         @comments = Comment.where(file_id: params[:file_id])
       else
-        @comments = Comment.where(file_id: params[:file_id], user_id: user_id)
+        # fetch comments of the current user
+        #@comments = Comment.where(file_id: params[:file_id], user_id: user_id)
+        # fetch comments of file-author and the current user
+        @comments = Comment.where(file_id: params[:file_id], user_id: [user_id, submission.user_id])
       end
 
       #@comments = Comment.where(file_id: params[:file_id])
diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb
index 59a05ab6..87cf825f 100644
--- a/app/controllers/submissions_controller.rb
+++ b/app/controllers/submissions_controller.rb
@@ -20,7 +20,7 @@ class SubmissionsController < ApplicationController
   def create
     @submission = Submission.new(submission_params)
     authorize!
-    #copy_comments
+    copy_comments
     create_and_respond(object: @submission)
   end
 
@@ -28,13 +28,14 @@ class SubmissionsController < ApplicationController
     # copy each annotation and set the target_file.id
     unless(params[:annotations_arr].nil?)
       params[:annotations_arr].each do | annotation |
+        #comment = Comment.new(annotation[1].permit(:user_id, :file_id, :user_type, :row, :column, :text, :created_at, :updated_at))
         comment = Comment.new(:user_id => annotation[1][:user_id], :file_id => annotation[1][:file_id], :user_type => current_user.class.name, :row => annotation[1][:row], :column => annotation[1][:column], :text => annotation[1][:text])
         source_file = CodeOcean::File.find(annotation[1][:file_id])
 
-        #comment = Comment.new(annotation[1].permit(:user_id, :file_id, :user_type, :row, :column, :text, :created_at, :updated_at))
+        # retrieve target file
         target_file = @submission.files.detect do |file|
           # file_id has to be that of a the former iteration OR of the initial file (if this is the first run)
-          file.file_id == source_file.file_id || file.file_id == source_file.id #seems to be needed here: (check this): || file.file_id == source_file.id
+          file.file_id == source_file.file_id || file.file_id == source_file.id #seems to be needed here: (check this): || file.file_id == source_file.id ; yes this is needed, for comments on templates as well as comments on files added by users.
         end
 
         #save to assign an id

From 3e04222367871ed6139f67c0518a5bdb38571b0a Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Wed, 24 Jun 2015 20:22:53 +0200
Subject: [PATCH 02/12] setting correct ids for the files in the editor after
 creating a new submission. Still a bit clumsy concerning the passed json
 parameters, but does the job.

---
 app/assets/javascripts/editor.js         | 40 ++++++++++++++++++++++--
 app/views/submissions/show.json.jbuilder |  2 +-
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index 971c16ff..e92c5862 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -117,8 +117,11 @@ $(function() {
           exercise_id: $('#editor').data('exercise-id'),
           files_attributes: (filter || _.identity)(collectFiles())
         },
-        source_submission_id: $('.ace_editor',$('#editor'))[0].dataset.id,
-        //annotations: annotations,
+        // fixed: not used any longer
+        // todo : get source_submission_id of each editor, since comments have to be copied for each file
+        //
+        // source_submission_id: $('.ace_editor',$('#editor'))[0].dataset.id,
+        // annotations: annotations,
         annotations_arr: annotations_arr
       },
       dataType: 'json',
@@ -132,7 +135,37 @@ $(function() {
   };
 
     var createSubmissionCallback = function(data){
-      var id =  $('.editor').data('id');
+
+        // update the ids of the editors and reload the annotations
+        for (var i = 0; i < editors.length; i++) {
+            var file_id_old = $(editors[i].container).data('file-id');
+
+            // if we have an file_id set (the file is a copy of a teacher supplied given file)
+            if (file_id_old != null){
+                // if we find file_id_old (this is the reference to the base file) in the submission, this is the match
+                for(var j = 0; j< data.files.length; j++){
+                    if(data.files[j].file_id == file_id_old){
+                        //$(editors[i].container).data('id') = data.files[j].id;
+                        $(editors[i].container).data('id', data.files[j].id );
+                    }
+                }
+            } else {
+                // the old file was created from scratch. set the id of the editor to the matching new one
+                var old_id =  $(editors[i].container).data('id');
+                for(var j = 0; j< data.files.length; j++){
+                    if(data.files[j].file_id == old_id){
+                        $(editors[i].container).data('id', data.files[j].id);
+                    }
+                }
+            }
+            setAnnotations(editors[i], $(editors[i].container).data('id'));
+        }
+
+        /*
+        $('.editor').each(function(index, element) {
+            var file_id_old =  $(element).data('id');
+
+        }); */
     };
 
   var destroyFile = function() {
@@ -377,6 +410,7 @@ $(function() {
         commentModal.find('#addCommentButton').on('click', function(e){
           var user_id = $(element).data('user-id');
           var commenttext = commentModal.find('textarea').val();
+          var file_id =  $(element).data('id');
 
           if (commenttext !== "") {
             createComment(user_id, file_id, row, editor, commenttext);
diff --git a/app/views/submissions/show.json.jbuilder b/app/views/submissions/show.json.jbuilder
index fc984edf..f137f874 100644
--- a/app/views/submissions/show.json.jbuilder
+++ b/app/views/submissions/show.json.jbuilder
@@ -1 +1 @@
-json.extract! @submission, :download_url, :id, :score_url, :render_url, :run_url, :stop_url, :test_url
+json.extract! @submission, :download_url, :id, :score_url, :render_url, :run_url, :stop_url, :test_url, :files

From 04a22392b6fd7d2e481ad6e615ac9b1d4ee7dea9 Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Fri, 26 Jun 2015 10:59:12 +0200
Subject: [PATCH 03/12] some cleanup

---
 app/assets/javascripts/editor.js | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index e92c5862..a2bde5f5 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -94,15 +94,10 @@ $(function() {
   var createSubmission = function(initiator, filter, callback) {
     showSpinner(initiator);
 
-      var annotations = {};
       var annotations_arr = [];
-      var file_ids = [];
-
 
       $('.editor').each(function(index, element) {
-          var file_id =  $(element).data('id');
           var editor = ace.edit(element);
-          //annotations[file_id] = editor.getSession().getAnnotations();
           var cleaned_annotations = editor.getSession().getAnnotations();
           for(var i = cleaned_annotations.length-1; i>=0; --i){
               cleaned_annotations[i].text = cleaned_annotations[i].text.replace(cleaned_annotations[i].username + ": ", "");
@@ -117,11 +112,6 @@ $(function() {
           exercise_id: $('#editor').data('exercise-id'),
           files_attributes: (filter || _.identity)(collectFiles())
         },
-        // fixed: not used any longer
-        // todo : get source_submission_id of each editor, since comments have to be copied for each file
-        //
-        // source_submission_id: $('.ace_editor',$('#editor'))[0].dataset.id,
-        // annotations: annotations,
         annotations_arr: annotations_arr
       },
       dataType: 'json',
@@ -140,6 +130,8 @@ $(function() {
         for (var i = 0; i < editors.length; i++) {
             var file_id_old = $(editors[i].container).data('file-id');
 
+            // file_id_old is always set. Either it is a reference to a teacher supplied given file, or it is the actual id of a new user created file.
+            // I am not sure why the latter happens, but therefore the else part is not needed any longer...
             // if we have an file_id set (the file is a copy of a teacher supplied given file)
             if (file_id_old != null){
                 // if we find file_id_old (this is the reference to the base file) in the submission, this is the match
@@ -149,23 +141,10 @@ $(function() {
                         $(editors[i].container).data('id', data.files[j].id );
                     }
                 }
-            } else {
-                // the old file was created from scratch. set the id of the editor to the matching new one
-                var old_id =  $(editors[i].container).data('id');
-                for(var j = 0; j< data.files.length; j++){
-                    if(data.files[j].file_id == old_id){
-                        $(editors[i].container).data('id', data.files[j].id);
-                    }
-                }
             }
             setAnnotations(editors[i], $(editors[i].container).data('id'));
         }
 
-        /*
-        $('.editor').each(function(index, element) {
-            var file_id_old =  $(element).data('id');
-
-        }); */
     };
 
   var destroyFile = function() {
@@ -383,6 +362,7 @@ $(function() {
       session.on('annotationChange', handleAnnotationChange);
 
       // TODO refactor here
+      // TODO: only show modal dialog when the file is part of a submission (and not the template)
       // Code for clicks on gutter / sidepanel
       editor.on("guttermousedown", function(e){
         var target  = e.domEvent.target;

From a49ffc978be04c61b2319ad7201eb8a304ada64c Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Fri, 26 Jun 2015 18:25:03 +0200
Subject: [PATCH 04/12] only allow comments on submissions, not on templates

---
 app/assets/javascripts/editor.js            | 15 +++++++++++----
 app/views/exercises/_editor_frame.html.slim |  2 +-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index d86b9011..93267501 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -125,13 +125,19 @@ $(function() {
   };
 
     var createSubmissionCallback = function(data){
-
         // update the ids of the editors and reload the annotations
         for (var i = 0; i < editors.length; i++) {
+
+            // set the data attribute to submission
+            $(editors[i].container).data('context-type', 'Submission');
+
+
             var file_id_old = $(editors[i].container).data('file-id');
 
             // file_id_old is always set. Either it is a reference to a teacher supplied given file, or it is the actual id of a new user created file.
-            // I am not sure why the latter happens, but therefore the else part is not needed any longer...
+            // This is the case, since it is set via a call to ancestor_id on the model, which returns either file_id if set, or id if it is not set.
+            // therefore the else part is not needed any longer...
+
             // if we have an file_id set (the file is a copy of a teacher supplied given file)
             if (file_id_old != null){
                 // if we find file_id_old (this is the reference to the base file) in the submission, this is the match
@@ -361,12 +367,13 @@ $(function() {
       session.on('annotationRemoval', handleAnnotationRemoval);
       session.on('annotationChange', handleAnnotationChange);
 
-      // TODO refactor here
-      // TODO: only show modal dialog when the file is part of a submission (and not the template)
+      // TODO refactor here, put this in its own function
       // Code for clicks on gutter / sidepanel
       editor.on("guttermousedown", function(e){
         var target  = e.domEvent.target;
 
+        // only allow comments on submissions, not on the template
+        if ($(editor.container).data('context-type') != 'Submission') return;
         if (target.className.indexOf("ace_gutter-cell") == -1) return;
         if (!editor.isFocused()) return;
         if (e.clientX > 25 + target.getBoundingClientRect().left) return;
diff --git a/app/views/exercises/_editor_frame.html.slim b/app/views/exercises/_editor_frame.html.slim
index 1db8ef76..e1596c42 100644
--- a/app/views/exercises/_editor_frame.html.slim
+++ b/app/views/exercises/_editor_frame.html.slim
@@ -12,4 +12,4 @@
         = link_to(file.native_file.file.name_with_extension, file.native_file.url)
   - else
     .editor-content.hidden data-file-id=file.ancestor_id = file.content
-    .editor data-file-id=file.ancestor_id data-indent-size=file.file_type.indent_size data-mode=file.file_type.editor_mode data-read-only=file.read_only data-id=file.id
+    .editor data-file-id=file.ancestor_id data-indent-size=file.file_type.indent_size data-mode=file.file_type.editor_mode data-read-only=file.read_only data-id=file.id data-context-type=file.context_type

From 81d2d69d111e84f56720aaff1a9c39c56f69cc4d Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Thu, 9 Jul 2015 12:03:03 +0200
Subject: [PATCH 05/12] Display request for comments button if we are working
 on a non binary submission. Introduced needed data attributes for that and
 assigned them to the frame (instead of the editor). Data Attributes get
 updated by the CreateSubmissionCallback. Adjusted comment checks to also use
 the attributes of the frame.

---
 app/assets/javascripts/editor.js              | 22 ++++++++++++++++---
 .../exercises/_editor_file_tree.html.slim     |  2 +-
 app/views/exercises/_editor_frame.html.slim   |  4 ++--
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index 93267501..045e03ae 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -125,12 +125,16 @@ $(function() {
   };
 
     var createSubmissionCallback = function(data){
+        // set all frames context types to submission
+        $('.frame').each(function(index, element) {
+            $(element).data('context-type', 'Submission');
+        });
+
         // update the ids of the editors and reload the annotations
         for (var i = 0; i < editors.length; i++) {
 
             // set the data attribute to submission
-            $(editors[i].container).data('context-type', 'Submission');
-
+            //$(editors[i].container).data('context-type', 'Submission');
 
             var file_id_old = $(editors[i].container).data('file-id');
 
@@ -150,6 +154,8 @@ $(function() {
             }
             setAnnotations(editors[i], $(editors[i].container).data('id'));
         }
+        // toggle button states (it might be the case that the request for comments button has to be enabled
+        toggleButtonStates();
 
     };
 
@@ -373,7 +379,8 @@ $(function() {
         var target  = e.domEvent.target;
 
         // only allow comments on submissions, not on the template
-        if ($(editor.container).data('context-type') != 'Submission') return;
+        if(active_frame.data('context-type') != 'Submission') return;
+        //if ($(editor.container).data('context-type') != 'Submission') return;
         if (target.className.indexOf("ace_gutter-cell") == -1) return;
         if (!editor.isFocused()) return;
         if (e.clientX > 25 + target.getBoundingClientRect().left) return;
@@ -573,6 +580,10 @@ $(function() {
     $('#start-over').on('click', confirmReset);
   };
 
+  var isActiveFileBinary = function() {
+      return 'binary' in active_frame.data();
+  };
+
   var isActiveFileExecutable = function() {
     return 'executable' in active_frame.data();
   };
@@ -596,6 +607,10 @@ $(function() {
     return isActiveFileRunnable() && running;
   };
 
+  var isActiveFileSubmission = function() {
+      return ['Submission'].includes(active_frame.data('contextType'));
+  };
+
   var isActiveFileTestable = function() {
     return isActiveFileExecutable() && ['teacher_defined_test', 'user_defined_test'].includes(active_frame.data('role'));
   };
@@ -960,6 +975,7 @@ $(function() {
     $('#run').toggle(isActiveFileRunnable() && !running);
     $('#stop').toggle(isActiveFileStoppable());
     $('#test').toggle(isActiveFileTestable());
+    $('#request-for-comments').toggle(isActiveFileSubmission() && !isActiveFileBinary());
   };
 
   var requestComments = function(e) {
diff --git a/app/views/exercises/_editor_file_tree.html.slim b/app/views/exercises/_editor_file_tree.html.slim
index 627d5e02..ac8c91d0 100644
--- a/app/views/exercises/_editor_file_tree.html.slim
+++ b/app/views/exercises/_editor_file_tree.html.slim
@@ -5,6 +5,6 @@ hr
 = render('editor_button', classes: 'btn-block btn-primary btn-xs', data: {:'data-cause' => 'file'}, icon: 'fa fa-plus', id: 'create-file', label: t('exercises.editor.create_file'))
 = render('editor_button', classes: 'btn-block btn-warning btn-xs', data: {:'data-cause' => 'file', :'data-message-confirm' => t('shared.confirm_destroy')}, icon: 'fa fa-times', id: 'destroy-file', label: t('exercises.editor.destroy_file'))
 = render('editor_button', classes: 'btn-block btn-primary btn-xs', icon: 'fa fa-download', id: 'download', label: t('exercises.editor.download'))
-/= render('editor_button', classes: 'btn-block btn-primary btn-xs', icon: 'fa fa-bullhorn', id: 'request-for-comments', label: 'Request comments')
+= render('editor_button', classes: 'btn-block btn-primary btn-xs', icon: 'fa fa-bullhorn', id: 'request-for-comments', label: 'Request comments')
 
 = render('shared/modal', id: 'modal-file', template: 'code_ocean/files/_form', title: t('exercises.editor.create_file'))
diff --git a/app/views/exercises/_editor_frame.html.slim b/app/views/exercises/_editor_frame.html.slim
index e1596c42..dc077e02 100644
--- a/app/views/exercises/_editor_frame.html.slim
+++ b/app/views/exercises/_editor_frame.html.slim
@@ -1,4 +1,4 @@
-.frame data-executable=file.file_type.executable? data-filename=file.name_with_extension data-renderable=file.file_type.renderable? data-role=file.role
+.frame data-executable=file.file_type.executable? data-filename=file.name_with_extension data-renderable=file.file_type.renderable? data-role=file.role data-binary=file.file_type.binary? data-context-type=file.context_type
   - if file.file_type.binary?
     .binary-file data-file-id=file.ancestor_id
       - if file.file_type.renderable?
@@ -12,4 +12,4 @@
         = link_to(file.native_file.file.name_with_extension, file.native_file.url)
   - else
     .editor-content.hidden data-file-id=file.ancestor_id = file.content
-    .editor data-file-id=file.ancestor_id data-indent-size=file.file_type.indent_size data-mode=file.file_type.editor_mode data-read-only=file.read_only data-id=file.id data-context-type=file.context_type
+    .editor data-file-id=file.ancestor_id data-indent-size=file.file_type.indent_size data-mode=file.file_type.editor_mode data-read-only=file.read_only data-id=file.id

From 06131e6e636f4b7adcaf60fa2858afe7bbf93bba Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Thu, 9 Jul 2015 16:08:26 +0200
Subject: [PATCH 06/12] Remove logic to find newest submission for an exercise
 and user. This is no longer necessary, since we update the file ids in
 editor.js

---
 .../request_for_comments_controller.rb        | 23 -------------------
 1 file changed, 23 deletions(-)

diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb
index 8c2379b4..fbeb21b7 100644
--- a/app/controllers/request_for_comments_controller.rb
+++ b/app/controllers/request_for_comments_controller.rb
@@ -35,30 +35,7 @@ class RequestForCommentsController < ApplicationController
   # POST /request_for_comments
   # POST /request_for_comments.json
   def create
-
-    file = CodeOcean::File.find(request_for_comment_params[:fileid])
-
-    # get newest version of the file. this method is only called if there is at least one submission (prevented in frontend otherwise)
-    # find newest submission for that exercise and user, use the file with the same filename for that.
-    # this is necessary because the passed params are not up to date since the data attributes are not updated upon submission creation.
-
-    # if we stat from the template, the context type is exercise. we find the newest submission based on the context_id and the current_user.id
-      if(file.context_type =='Exercise')
-        newest_submission = Submission.where(exercise_id: file.context_id, user_id: current_user.id).order('created_at DESC').first
-      else
-        # else we start from a submission. we find it it by the given context_id and retrieve the newest submission with the info of the known submission.
-        submission = Submission.find(file.context_id)
-        newest_submission = Submission.where(exercise_id: submission.exercise_id, user_id: submission.user_id).order('created_at DESC').first
-      end
-      newest_file = CodeOcean::File.where(context_id: newest_submission.id, name: file.name).first
-
-    #finally, correct the fileid and create the request for comment
-    request_for_comment_params[:fileid]=newest_file.id
-
     @request_for_comment = RequestForComment.new(request_for_comment_params)
-
-
-
     respond_to do |format|
       if @request_for_comment.save
         format.json { render :show, status: :created, location: @request_for_comment }

From eec5832a655e7be4d2289e3544fd4e60b68b9659 Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Tue, 4 Aug 2015 15:16:06 +0200
Subject: [PATCH 07/12] allow requesting comments only once per submission

---
 app/assets/javascripts/editor.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index 11678979..ddbbc36e 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -1004,6 +1004,8 @@ $(function() {
     })
 
     showSpinner($('#request-for-comments'))
+    // hide button until next submission is created
+    $('#request-for-comments').toggle(false);
   }
 
     var initializeCodePilot = function() {

From 512e90ebd7c4c1f40de12329aa0c0ccda7d853db Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Tue, 18 Aug 2015 16:32:56 +0200
Subject: [PATCH 08/12] Show correct usernames on comments, have correct
 linenumbers on request_for_comments

---
 app/controllers/comments_controller.rb       | 12 +++++++++++-
 app/views/request_for_comments/show.html.erb |  8 ++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index 4da2bf5c..91e31068 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -40,7 +40,17 @@ class CommentsController < ApplicationController
       #@comments = Comment.where(file_id: params[:file_id])
 
       #add names to comments
-      @comments.map{|comment| comment.username = Xikolo::UserClient.get(comment.user_id.to_s)[:display_name]}
+      # if the user is internal, set the name
+      # todo:
+      # if the user is external, fetch the displayname from xikolo
+      @comments.map{|comment|
+        if(comment.user_type == 'InternalUser')
+          comment.username = InternalUser.find(comment.user_id).name
+        elsif(comment.user_type == 'ExternalUser')
+          comment.username = ExternalUser.find(comment.user_id).name
+              #alternativ: Xikolo::UserClient.get(comment.user_id.to_s)[:display_name]
+        end
+      }
     else
       @comments = Comment.all.limit(0) #we need an empty relation here
     end
diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb
index d9779583..1b8e7495 100644
--- a/app/views/request_for_comments/show.html.erb
+++ b/app/views/request_for_comments/show.html.erb
@@ -4,11 +4,11 @@
     <%= InternalUser.find(@request_for_comment.requestorid) %> | <%= @request_for_comment.requested_at %>
   </p>
 </div>
-<!--
--->
 
-<div id='commentitor' class='editor' data-read-only='true' data-file-id='<%=@request_for_comment.fileid%>'>
-  <%= CodeOcean::File.find(@request_for_comment.fileid).content %>
+<!--
+do not put a carriage return in the line below. it will be present in the presentation of the source code, otherwise.
+-->
+<div id='commentitor' class='editor' data-read-only='true' data-file-id='<%=@request_for_comment.fileid%>'><%= CodeOcean::File.find(@request_for_comment.fileid).content %>
 </div>
 
 <script type="text/javascript">

From e5209143859298769d3a4a3793d0e8a1929c6acc Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Tue, 18 Aug 2015 17:44:31 +0200
Subject: [PATCH 09/12] Some correction of column names. Support for internal
 as well as external users. Added column user_type and used it where necessary

---
 app/assets/javascripts/editor.js                 |  6 +++---
 app/controllers/comments_controller.rb           | 16 +++++++---------
 .../request_for_comments_controller.rb           |  2 +-
 app/views/request_for_comments/_form.html.erb    | 16 ++++++++++------
 app/views/request_for_comments/index.html.erb    | 13 +++++++++++--
 .../request_for_comments/index.json.jbuilder     |  2 +-
 app/views/request_for_comments/show.html.erb     | 14 +++++++++++---
 .../request_for_comments/show.json.jbuilder      |  2 +-
 ...1554_add_user_type_to_request_for_comments.rb |  5 +++++
 .../20150818142251_correct_column_names.rb       |  7 +++++++
 db/schema.rb                                     |  9 +++++----
 11 files changed, 62 insertions(+), 30 deletions(-)
 create mode 100644 db/migrate/20150818141554_add_user_type_to_request_for_comments.rb
 create mode 100644 db/migrate/20150818142251_correct_column_names.rb

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index b9ca3017..93e3539b 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -1014,9 +1014,9 @@ $(function() {
       url: '/request_for_comments',
       data: {
         request_for_comment: {
-          requestorid: user_id,
-          exerciseid: exercise_id,
-          fileid: file_id,
+          requestor_user_id: user_id,
+          exercise_id: exercise_id,
+          file_id: file_id,
           "requested_at(1i)": 2015, // these are the timestamp values that the request handler demands
           "requested_at(2i)":3, // they could be random here, because the timestamp is updated on serverside anyway
           "requested_at(3i)":27,
diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index 91e31068..434aa67b 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -20,10 +20,10 @@ class CommentsController < ApplicationController
     submission = Submission.find_by(id: file.context_id)
     if submission
       is_admin = false
-      if current_user.respond_to? :external_id
-        user_id = current_user.external_id
-      else
-        user_id = current_user.id
+      user_id = current_user.id
+
+      # if we have an internal user, check whether he is an admin
+      if not current_user.respond_to? :external_id
         is_admin = current_user.role == 'admin'
       end
 
@@ -37,18 +37,16 @@ class CommentsController < ApplicationController
         @comments = Comment.where(file_id: params[:file_id], user_id: [user_id, submission.user_id])
       end
 
-      #@comments = Comment.where(file_id: params[:file_id])
-
       #add names to comments
       # if the user is internal, set the name
-      # todo:
-      # if the user is external, fetch the displayname from xikolo
+
       @comments.map{|comment|
         if(comment.user_type == 'InternalUser')
           comment.username = InternalUser.find(comment.user_id).name
         elsif(comment.user_type == 'ExternalUser')
           comment.username = ExternalUser.find(comment.user_id).name
-              #alternativ: Xikolo::UserClient.get(comment.user_id.to_s)[:display_name]
+          # alternative: # if the user is external, fetch the displayname from xikolo
+          # Xikolo::UserClient.get(comment.user_id.to_s)[:display_name]
         end
       }
     else
diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb
index fbeb21b7..2a68619b 100644
--- a/app/controllers/request_for_comments_controller.rb
+++ b/app/controllers/request_for_comments_controller.rb
@@ -66,6 +66,6 @@ class RequestForCommentsController < ApplicationController
 
     # Never trust parameters from the scary internet, only allow the white list through.
     def request_for_comment_params
-      params.require(:request_for_comment).permit(:requestorid, :exerciseid, :fileid, :requested_at)
+      params.require(:request_for_comment).permit(:requestor_user_id, :exercise_id, :file_id, :requested_at)
     end
 end
diff --git a/app/views/request_for_comments/_form.html.erb b/app/views/request_for_comments/_form.html.erb
index a8d037e9..493e2445 100644
--- a/app/views/request_for_comments/_form.html.erb
+++ b/app/views/request_for_comments/_form.html.erb
@@ -12,21 +12,25 @@
   <% end %>
 
   <div class="field">
-    <%= f.label :requestorid %><br>
-    <%= f.number_field :requestorid %>
+    <%= f.label :requestor_user_id %><br>
+    <%= f.number_field :requestor_user_id %>
   </div>
   <div class="field">
-    <%= f.label :exerciseid %><br>
-    <%= f.number_field :exerciseid %>
+    <%= f.label :exercise_id %><br>
+    <%= f.number_field :exercise_id %>
   </div>
   <div class="field">
-    <%= f.label :fileid %><br>
-    <%= f.number_field :fileid %>
+    <%= f.label :file_id %><br>
+    <%= f.number_field :file_id %>
   </div>
   <div class="field">
     <%= f.label :requested_at %><br>
     <%= f.datetime_select :requested_at %>
   </div>
+  <div class="field">
+    <%= f.label :user_type %><br>
+    <%= f.text_field :user_type %>
+  </div>
   <div class="actions">
     <%= f.submit %>
   </div>
diff --git a/app/views/request_for_comments/index.html.erb b/app/views/request_for_comments/index.html.erb
index 311be4b5..8a234e6f 100644
--- a/app/views/request_for_comments/index.html.erb
+++ b/app/views/request_for_comments/index.html.erb
@@ -3,9 +3,18 @@
 <div class="list-group">
   <% @request_for_comments.each do |request_for_comment| %>
     <a href="<%= request_for_comment_path(request_for_comment) %>" class="list-group-item">
-      <h4 class="list-group-item-heading"><%= Exercise.find(request_for_comment.exerciseid) %></h4>
+      <h4 class="list-group-item-heading"><%= Exercise.find(request_for_comment.exercise_id) %></h4>
       <p class="list-group-item-text">
-        <%= InternalUser.find(request_for_comment.requestorid) %> | <%= request_for_comment.requested_at %>
+        <%
+           user = nil
+           if (request_for_comment.user_type == 'InternalUser')
+             user = InternalUser.find(request_for_comment.requestor_user_id)
+           else
+             user = ExternalUser.find(request_for_comment.requestor_user_id)
+           end
+        %>
+
+          <%= user %> | <%= request_for_comment.requested_at %>
       </p>
     </a>
   <% end %>
diff --git a/app/views/request_for_comments/index.json.jbuilder b/app/views/request_for_comments/index.json.jbuilder
index 26b37b67..72204e6e 100644
--- a/app/views/request_for_comments/index.json.jbuilder
+++ b/app/views/request_for_comments/index.json.jbuilder
@@ -1,4 +1,4 @@
 json.array!(@request_for_comments) do |request_for_comment|
-  json.extract! request_for_comment, :id, :requestorid, :exerciseid, :fileid, :requested_at
+  json.extract! request_for_comment, :id, :requestor_user_id, :exercise_id, :file_id, :requested_at, :user_type
   json.url request_for_comment_url(request_for_comment, format: :json)
 end
diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb
index 1b8e7495..179c8cd5 100644
--- a/app/views/request_for_comments/show.html.erb
+++ b/app/views/request_for_comments/show.html.erb
@@ -1,14 +1,22 @@
 <div class="list-group">
-  <h4 class="list-group-item-heading"><%= Exercise.find(@request_for_comment.exerciseid) %></h4>
+  <h4 class="list-group-item-heading"><%= Exercise.find(@request_for_comment.exercise_id) %></h4>
   <p class="list-group-item-text">
-    <%= InternalUser.find(@request_for_comment.requestorid) %> | <%= @request_for_comment.requested_at %>
+      <%
+         user = nil
+         if (@request_for_comment.user_type == 'InternalUser')
+           user = InternalUser.find(@request_for_comment.requestor_user_id)
+         else
+           user = ExternalUser.find(@request_for_comment.requestor_user_id)
+         end
+      %>
+    <%= user %> | <%= @request_for_comment.requested_at %>
   </p>
 </div>
 
 <!--
 do not put a carriage return in the line below. it will be present in the presentation of the source code, otherwise.
 -->
-<div id='commentitor' class='editor' data-read-only='true' data-file-id='<%=@request_for_comment.fileid%>'><%= CodeOcean::File.find(@request_for_comment.fileid).content %>
+<div id='commentitor' class='editor' data-read-only='true' data-file-id='<%=@request_for_comment.file_id%>'><%= CodeOcean::File.find(@request_for_comment.file_id).content %>
 </div>
 
 <script type="text/javascript">
diff --git a/app/views/request_for_comments/show.json.jbuilder b/app/views/request_for_comments/show.json.jbuilder
index 27897168..b3962aad 100644
--- a/app/views/request_for_comments/show.json.jbuilder
+++ b/app/views/request_for_comments/show.json.jbuilder
@@ -1 +1 @@
-json.extract! @request_for_comment, :id, :requestorid, :exerciseid, :fileid, :requested_at, :created_at, :updated_at
+json.extract! @request_for_comment, :id, :requestor_user_id, :exercise_id, :file_id, :requested_at, :created_at, :updated_at, :user_type
diff --git a/db/migrate/20150818141554_add_user_type_to_request_for_comments.rb b/db/migrate/20150818141554_add_user_type_to_request_for_comments.rb
new file mode 100644
index 00000000..f1450bff
--- /dev/null
+++ b/db/migrate/20150818141554_add_user_type_to_request_for_comments.rb
@@ -0,0 +1,5 @@
+class AddUserTypeToRequestForComments < ActiveRecord::Migration
+  def change
+    add_column :request_for_comments, :user_type, :string
+  end
+end
diff --git a/db/migrate/20150818142251_correct_column_names.rb b/db/migrate/20150818142251_correct_column_names.rb
new file mode 100644
index 00000000..dd73814a
--- /dev/null
+++ b/db/migrate/20150818142251_correct_column_names.rb
@@ -0,0 +1,7 @@
+class CorrectColumnNames < ActiveRecord::Migration
+  def change
+	rename_column :request_for_comments, :requestorid, :requestor_user_id
+  	rename_column :request_for_comments, :exerciseid, :exercise_id
+  	rename_column :request_for_comments, :fileid, :file_id
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 6685f86c..30fb419f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150421074734) do
+ActiveRecord::Schema.define(version: 20150818142251) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -171,12 +171,13 @@ ActiveRecord::Schema.define(version: 20150421074734) do
   add_index "internal_users_teams", ["team_id"], name: "index_internal_users_teams_on_team_id", using: :btree
 
   create_table "request_for_comments", force: true do |t|
-    t.integer  "requestorid",  null: false
-    t.integer  "exerciseid",   null: false
-    t.integer  "fileid",       null: false
+    t.integer  "requestor_user_id", null: false
+    t.integer  "exercise_id",       null: false
+    t.integer  "file_id",           null: false
     t.datetime "requested_at"
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.string   "user_type"
   end
 
   create_table "submissions", force: true do |t|

From dc0b3a22941c918fd772692c68c6211d5208b230 Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Wed, 19 Aug 2015 15:35:25 +0200
Subject: [PATCH 10/12] cleanup of passed parameters, user_id and user_type are
 extracted from current user and not used from the passed POST values..

---
 app/assets/javascripts/editor.js                   | 2 --
 app/controllers/comments_controller.rb             | 4 ++--
 app/controllers/request_for_comments_controller.rb | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index 93e3539b..50c51179 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -510,7 +510,6 @@ $(function() {
       var jqxhr = $.ajax({
         data: {
           comment: {
-            user_id: user_id,
             file_id: file_id,
             row: row,
             column: 0,
@@ -1014,7 +1013,6 @@ $(function() {
       url: '/request_for_comments',
       data: {
         request_for_comment: {
-          requestor_user_id: user_id,
           exercise_id: exercise_id,
           file_id: file_id,
           "requested_at(1i)": 2015, // these are the timestamp values that the request handler demands
diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
index 434aa67b..c1aab761 100644
--- a/app/controllers/comments_controller.rb
+++ b/app/controllers/comments_controller.rb
@@ -75,7 +75,7 @@ class CommentsController < ApplicationController
   # POST /comments
   # POST /comments.json
   def create
-    @comment = Comment.new(comment_params.merge(user_type: current_user.class.name))
+    @comment = Comment.new(comment_params)
 
     respond_to do |format|
       if @comment.save
@@ -135,6 +135,6 @@ class CommentsController < ApplicationController
     def comment_params
       #params.require(:comment).permit(:user_id, :file_id, :row, :column, :text)
       # fuer production mode, damit böse menschen keine falsche user_id uebergeben:
-      params.require(:comment).permit(:file_id, :row, :column, :text).merge(user_id: current_user.id)
+      params.require(:comment).permit(:file_id, :row, :column, :text).merge(user_id: current_user.id, user_type: current_user.class.name)
     end
 end
diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb
index 2a68619b..9e0a5d49 100644
--- a/app/controllers/request_for_comments_controller.rb
+++ b/app/controllers/request_for_comments_controller.rb
@@ -66,6 +66,6 @@ class RequestForCommentsController < ApplicationController
 
     # Never trust parameters from the scary internet, only allow the white list through.
     def request_for_comment_params
-      params.require(:request_for_comment).permit(:requestor_user_id, :exercise_id, :file_id, :requested_at)
+      params.require(:request_for_comment).permit(:exercise_id, :file_id, :requested_at).merge(requestor_user_id: current_user.id, user_type: current_user.class.name)
     end
 end

From 43d2428ca109a7ccba411f474581e3c48fc2e7a5 Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Mon, 31 Aug 2015 19:19:19 +0200
Subject: [PATCH 11/12] some bugfixes concerning comments, add first code for
 learning analytics

---
 app/assets/javascripts/editor.js | 77 ++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index cb4ecfb1..0f70f6a4 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -256,17 +256,20 @@ $(function() {
   };
 
   var handlePasteEvent = function (pasteObject) {
-    console.log("Handling paste event. this is ", this );
-    console.log("Text: " + pasteObject.text);
+    //console.log("Handling paste event. this is ", this );
+    //console.log("Text: " + pasteObject.text);
 
     var same = (lastCopyText === pasteObject.text)
-    console.log("Text is the same: " + same);
+    //console.log("Text is the same: " + same);
 
     // if the text is not copied from within the editor (from any file), send an event to lanalytics
-    if(!same){
-      //publishCodeOceanEvent("editor-paste", {
-      //  text: pasteObject.text
-      //});
+    //if(!same){
+    //  publishCodeOceanEvent("codeocean_editor_paste", {
+    //    text: pasteObject.text,
+    //    exercise: $('#editor').data('exercise-id'),
+    //    file_id: "1"
+    //
+    //  });
     }
   };
 
@@ -445,24 +448,22 @@ $(function() {
   var setAnnotationsCallback = function (response, session) {
       var annotations = response;
 
+      // add classname and the username in front of each comment
       $.each(annotations, function(index, comment){
           comment.className = "code-ocean_comment";
           comment.text = comment.username + ": " + comment.text;
-        //  comment.text = comment.user_id + ": " + comment.text;
       });
 
       session.setAnnotations(annotations);
   }
 
-  var deleteComment = function (user_id, file_id, row, editor) {
+  var deleteComment = function (file_id, row, editor) {
       var jqxhr = $.ajax({
           type: 'DELETE',
           url: "/comments",
           data: {
             row: row,
-            file_id: file_id,
-            user_id: user_id
-          }
+            file_id: file_id          }
       });
       jqxhr.done(function (response) {
           setAnnotations(editor, file_id);
@@ -470,11 +471,10 @@ $(function() {
       jqxhr.fail(ajaxError);
   }
 
-  var createComment = function (user_id, file_id, row, editor, commenttext){
+  var createComment = function (file_id, row, editor, commenttext){
       var jqxhr = $.ajax({
         data: {
           comment: {
-            user_id: user_id,
             file_id: file_id,
             row: row,
             column: 0,
@@ -523,9 +523,10 @@ $(function() {
   // Code for clicks on gutter / sidepanel
   var handleSidebarClick = function(e) {
       var target  = e.domEvent.target;
+      var editor =  e.editor;
 
       if (target.className.indexOf("ace_gutter-cell") == -1) return;
-      if (!e.editor.isFocused()) return;
+      if (!editor.isFocused()) return;
       if (e.clientX > 25 + target.getBoundingClientRect().left) return;
 
       var row = e.getDocumentPosition().row;
@@ -545,18 +546,20 @@ $(function() {
       commentModal.find('#removeAllButton').off('click');
 
       commentModal.find('#addCommentButton').on('click', function(e){
-        var user_id = $(element).data('user-id');
         var commenttext = commentModal.find('textarea').val();
+        // attention: use id of data attribute here, not file-id (file-id is the original file)
+        var file_id = $(editor.container).data('id');
 
         if (commenttext !== "") {
-          createComment(user_id, file_id, row, editor, commenttext);
+          createComment(file_id, row, editor, commenttext);
           commentModal.modal('hide');
         }
       });
 
       commentModal.find('#removeAllButton').on('click', function(e){
-        var user_id = $(element).data('user-id');
-        deleteComment(user_id,file_id,row,editor);
+        // attention: use id of data attribute here, not file-id (file-id is the original file)
+        var file_id = $(editor.container).data('id');
+        deleteComment(file_id,row, editor);
         commentModal.modal('hide');
       });
 
@@ -734,23 +737,29 @@ $(function() {
   };
 
     // Publishing events for other (JS) components to react to codeocean events
- var publishCodeOceanEvent = function (eventName, contextData) {
-  /*$(this.$baseElement).trigger(eventName, {
-    resource: this.$baseElement.data("lanalytics-resource"),
-    inContext: contextData
-  });
-  */
+   var publishCodeOceanEvent = function (eventName, contextData) {
 
-   $.ajax("/lanalytics/log", {
-     type: 'POST',
-     cache: false,
-     dataType: 'JSON',
-     data: experienceStatement.params() ,
-     success: (response_data, text_status, jqXHR),
-     error: (jqXHR, textStatus, errorThrown)
-    })
+     var payload = {
+       user: {
+         resource_uuid: $('#editor').data('user-id')
+       },
+       verb: eventName,
+       resource: {},
+       timestamp: new Date().toISOString(),
+       with_result: {},
+       in_context: contextData
+     };
 
-};
+     $.ajax("https://open.hpi.de/lanalytics/log", {
+       type: 'POST',
+       cache: false,
+       dataType: 'JSON',
+       data: payload,
+       success: {},
+       error: {}
+      })
+
+  };
 
   var renderCode = function(event) {
     event.preventDefault();

From 1dab25b6369d324f3c7ec0b0e1c68095a793ae39 Mon Sep 17 00:00:00 2001
From: Ralf Teusner <ralf.teusner@hpi.de>
Date: Mon, 31 Aug 2015 19:43:16 +0200
Subject: [PATCH 12/12] fixed broken merge...

---
 app/assets/javascripts/editor.js | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js
index 7e8c82ff..e2598eb7 100644
--- a/app/assets/javascripts/editor.js
+++ b/app/assets/javascripts/editor.js
@@ -414,21 +414,6 @@ $(function() {
       session.on('annotationRemoval', handleAnnotationRemoval);
       session.on('annotationChange', handleAnnotationChange);
 
-      // TODO refactor here, put this in its own function
-      // Code for clicks on gutter / sidepanel
-      editor.on("guttermousedown", function(e){
-        var target  = e.domEvent.target;
-
-        // only allow comments on submissions, not on the template
-        if(active_frame.data('context-type') != 'Submission') return;
-        //if ($(editor.container).data('context-type') != 'Submission') return;
-        if (target.className.indexOf("ace_gutter-cell") == -1) return;
-        if (!editor.isFocused()) return;
-        if (e.clientX > 25 + target.getBoundingClientRect().left) return;
-
-        var row = e.getDocumentPosition().row;
-        e.stop();
-
       /*
        * Register event handlers
        */