Merge pull request #72 from openHPI/rfc_cleanup

Rfc cleanup

app/assets/javascripts/editor.js.erb

@@ -1214,25 +1214,25 @@ $(function() {
     var file_id = $('.editor').data('id')
     var question = $('#question').val();
 
-    $.ajax({
+    var createRequestForComments = function(submission) {
-      method: 'POST',
+      $.ajax({
-      url: '/request_for_comments',
+        method: 'POST',
-      data: {
+        url: '/request_for_comments',
-        request_for_comment: {
+        data: {
-          exercise_id: exercise_id,
+          request_for_comment: {
-          file_id: file_id,
+            exercise_id: exercise_id,
-          question: question,
+            file_id: file_id,
-          "requested_at(1i)": 2015, // these are the timestamp values that the request handler demands
+            submission_id: submission.id,
-          "requested_at(2i)":3, // they could be random here, because the timestamp is updated on serverside anyway
+            question: question
-          "requested_at(3i)":27,
+          }
-          "requested_at(4i)":17,
-          "requested_at(5i)":06
         }
-      }
+      }).done(function() {
-    }).done(function() {
+        hideSpinner();
-      hideSpinner();
+        $.flash.success({ text: $('#askForCommentsButton').data('message-success') });
-      $.flash.success({ text: $('#askForCommentsButton').data('message-success') })
+      }).error(ajaxError);
-    }).error(ajaxError);
+    }
+
+    createSubmission($('.requestCommentsButton'), null, createRequestForComments);
 
     $('#comment-modal').modal('hide');
     var button = $('.requestCommentsButton');

app/controllers/request_for_comments_controller.rb

@@ -82,6 +82,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(:exercise_id, :file_id, :question, :requested_at, :solved).merge(user_id: current_user.id, user_type: current_user.class.name)
+      params.require(:request_for_comment).permit(:exercise_id, :file_id, :question, :requested_at, :solved, :submission_id).merge(user_id: current_user.id, user_type: current_user.class.name)
     end
 end

app/models/request_for_comment.rb

@@ -17,6 +17,8 @@ class RequestForComment < ActiveRecord::Base
     Submission.find(file.context_id)
   end
 
+  # not used right now, finds the last submission for the respective user and exercise.
+  # might be helpful to check whether the exercise has been solved in the meantime.
   def last_submission
     Submission.find_by_sql(" select * from submissions
             where exercise_id = #{exercise_id} AND
@@ -25,6 +27,17 @@ class RequestForComment < ActiveRecord::Base
             limit 1").first
   end
 
+  # not used any longer, since we directly saved the submission_id now.
+  # Was used before that to determine the submission belonging to the request_for_comment.
+  def last_submission_before_creation
+    Submission.find_by_sql(" select * from submissions
+            where exercise_id = #{exercise_id} AND
+            user_id =  #{user_id} AND
+            '#{created_at.localtime}' > created_at
+            order by created_at desc
+            limit 1").first
+  end
+
   def comments_count
     submission.files.map { |file| file.comments.size}.sum
   end
@@ -35,6 +48,6 @@ class RequestForComment < ActiveRecord::Base
 
     private
     def self.row_number_user_sql
-      select("id, user_id, exercise_id, file_id, question, requested_at, created_at, updated_at, user_type, solved, row_number() OVER (PARTITION BY user_id ORDER BY created_at DESC) as row_number").to_sql
+      select("id, user_id, exercise_id, file_id, question, created_at, updated_at, user_type, solved, submission_id, row_number() OVER (PARTITION BY user_id ORDER BY created_at DESC) as row_number").to_sql
     end
 end

app/models/submission.rb

@@ -2,7 +2,7 @@ class Submission < ActiveRecord::Base
   include Context
   include Creation
 
-  CAUSES = %w(assess download file render run save submit test autosave)
+  CAUSES = %w(assess download file render run save submit test autosave requestComments)
   FILENAME_URL_PLACEHOLDER = '{filename}'
 
   belongs_to :exercise

app/views/exercises/_editor_frame.html.slim

@@ -14,6 +14,6 @@
     .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
 
-  button.btn.btn-primary.requestCommentsButton type='button'
+  button.btn.btn-primary.requestCommentsButton type='button' id="requestComments"
-    i.fa.fa-comment-o
+    i.fa.fa-comment
     = t('exercises.editor.requestComments')

app/views/request_for_comments/index.html.slim

@@ -27,6 +27,6 @@ h1 = RequestForComment.model_name.human(count: 2)
             td = '-'
           td = request_for_comment.comments_count
           td = request_for_comment.user.displayname
-          td = t('shared.time.before', time: distance_of_time_in_words_to_now(request_for_comment.requested_at))
+          td = t('shared.time.before', time: distance_of_time_in_words_to_now(request_for_comment.created_at))
 
 = render('shared/pagination', collection: @request_for_comments)

app/views/request_for_comments/show.html.erb

@@ -1,21 +1,14 @@
 <div class="list-group">
-  <h4 id ="exercise_caption" class="list-group-item-heading" data-rfc-id = "<%= @request_for_comment.id %>" ><%= Exercise.find(@request_for_comment.exercise_id) %></h4>
+  <h4 id ="exercise_caption" class="list-group-item-heading" data-rfc-id = "<%= @request_for_comment.id %>" ><%= link_to(@request_for_comment.exercise.title, [:implement, @request_for_comment.exercise]) %></h4>
   <p class="list-group-item-text">
       <%
         user = @request_for_comment.user
-        submission_id =  ActiveRecord::Base.connection.execute("select id from submissions
+        submission =  @request_for_comment.last_submission_before_creation
-          where exercise_id =
-          #{@request_for_comment.exercise_id} AND
-          user_id =  #{@request_for_comment.user_id} AND
-          '#{@request_for_comment.created_at}' > created_at
-          order by created_at desc
-          limit 1").first['id'].to_i
-        submission = Submission.find(submission_id)
       %>
-    <%= user.displayname %> | <%= @request_for_comment.requested_at %>
+    <%= user.displayname %> | <%= @request_for_comment.created_at.localtime %>
   </p>
   <h5>
-    <u><%= t('activerecord.attributes.exercise.instructions') %>:</u> "<%= @request_for_comment.exercise.description %>"
+    <u><%= t('activerecord.attributes.exercise.description') %>:</u> "<%= render_markdown(@request_for_comment.exercise.description) %>"
   </h5>
 
   <h5>

db/migrate/20160630154310_add_submission_to_request_for_comments.rb

@@ -0,0 +1,5 @@
+class AddSubmissionToRequestForComments < ActiveRecord::Migration
+  def change
+    add_reference :request_for_comments, :submission
+  end
+end

db/migrate/20160701092140_remove_requested_at_from_request_for_comments.rb

@@ -0,0 +1,5 @@
+class RemoveRequestedAtFromRequestForComments < ActiveRecord::Migration
+  def change
+    remove_column :request_for_comments, :requested_at
+  end
+end

db/schema.rb

@@ -173,15 +173,15 @@ ActiveRecord::Schema.define(version: 20160704143402) do
   add_index "internal_users", ["reset_password_token"], name: "index_internal_users_on_reset_password_token", using: :btree
 
   create_table "request_for_comments", force: true do |t|
-    t.integer  "user_id",      null: false
+    t.integer  "user_id",       null: false
-    t.integer  "exercise_id",  null: false
+    t.integer  "exercise_id",   null: false
-    t.integer  "file_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"
     t.text     "question"
     t.boolean  "solved"
+    t.integer  "submission_id"
   end
 
   create_table "submissions", force: true do |t|