diff --git a/app/assets/javascripts/editor.js b/app/assets/javascripts/editor.js index bec0c64b..e2598eb7 100644 --- a/app/assets/javascripts/editor.js +++ b/app/assets/javascripts/editor.js @@ -95,15 +95,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 + ": ", ""); @@ -118,8 +113,6 @@ $(function() { exercise_id: $('#editor').data('exercise-id'), files_attributes: (filter || _.identity)(collectFiles()) }, - source_submission_id: $('.ace_editor',$('#editor'))[0].dataset.id, - //annotations: annotations, annotations_arr: annotations_arr }, dataType: 'json', @@ -133,7 +126,38 @@ $(function() { }; var createSubmissionCallback = function(data){ - var id = $('.editor').data('id'); + // 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'); + + 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. + // 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 + 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 ); + } + } + } + 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(); + }; var destroyFile = function() { @@ -387,6 +411,9 @@ $(function() { var file_id = $(element).data('id'); setAnnotations(editor, file_id); + session.on('annotationRemoval', handleAnnotationRemoval); + session.on('annotationChange', handleAnnotationChange); + /* * Register event handlers */ @@ -401,8 +428,7 @@ $(function() { handleSidebarClick(e); }); */ - - + //session session.on('annotationRemoval', handleAnnotationRemoval); session.on('annotationChange', handleAnnotationChange); @@ -614,6 +640,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(); }; @@ -637,6 +667,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')); }; @@ -1028,6 +1062,7 @@ $(function() { $('#run').toggle(isActiveFileRunnable() && !running); $('#stop').toggle(isActiveFileStoppable()); $('#test').toggle(isActiveFileTestable()); + $('#request-for-comments').toggle(isActiveFileSubmission() && !isActiveFileBinary()); }; var requestComments = function(e) { @@ -1040,9 +1075,8 @@ $(function() { url: '/request_for_comments', data: { request_for_comment: { - requestorid: user_id, - exerciseid: exercise_id, - fileid: file_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, @@ -1056,6 +1090,8 @@ $(function() { }) showSpinner($('#request-for-comments')) + // hide button until next submission is created + $('#request-for-comments').toggle(false); } var initializeCodePilot = function() { diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 7076ca81..c1aab761 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -14,16 +14,16 @@ 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) 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 @@ -31,13 +31,24 @@ 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]) - #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 + + @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 + # alternative: # if the user is external, fetch the displayname from xikolo + # Xikolo::UserClient.get(comment.user_id.to_s)[:display_name] + end + } else @comments = Comment.all.limit(0) #we need an empty relation here end @@ -64,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 @@ -124,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 8c2379b4..9e0a5d49 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 } @@ -89,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(:exercise_id, :file_id, :requested_at).merge(requestor_user_id: current_user.id, user_type: current_user.class.name) end end diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 90460bd9..b77e4936 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 diff --git a/app/views/exercises/_editor_frame.html.slim b/app/views/exercises/_editor_frame.html.slim index 1db8ef76..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? 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 %>
- <%= f.label :requestorid %>
- <%= f.number_field :requestorid %> + <%= f.label :requestor_user_id %>
+ <%= f.number_field :requestor_user_id %>
- <%= f.label :exerciseid %>
- <%= f.number_field :exerciseid %> + <%= f.label :exercise_id %>
+ <%= f.number_field :exercise_id %>
- <%= f.label :fileid %>
- <%= f.number_field :fileid %> + <%= f.label :file_id %>
+ <%= f.number_field :file_id %>
<%= f.label :requested_at %>
<%= f.datetime_select :requested_at %>
+
+ <%= f.label :user_type %>
+ <%= f.text_field :user_type %> +
<%= f.submit %>
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 @@
<% @request_for_comments.each do |request_for_comment| %> -

<%= Exercise.find(request_for_comment.exerciseid) %>

+

<%= Exercise.find(request_for_comment.exercise_id) %>

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

<% 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 d9779583..179c8cd5 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -1,14 +1,22 @@
-

<%= Exercise.find(@request_for_comment.exerciseid) %>

+

<%= Exercise.find(@request_for_comment.exercise_id) %>

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

- -
- <%= CodeOcean::File.find(@request_for_comment.fileid).content %> + +
<%= CodeOcean::File.find(@request_for_comment.file_id).content %>