From 223df2ffa8a9f34dd23fc2a92a18ec6dee6d7726 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 4 Jul 2016 17:44:22 +0200 Subject: [PATCH 1/4] some cleanup of request for comments. Work in progress. Noticed a flaw when fetching the last submission, which is caused by timezone differences. First step to solve this. Existing Request for Comments still need to be updated with their current submissionId, the SQL to do that is not yet finished. --- app/assets/javascripts/editor.js.erb | 37 ++++++++++--------- .../request_for_comments_controller.rb | 2 +- app/models/request_for_comment.rb | 29 +++++++++++++++ app/models/submission.rb | 2 +- app/views/exercises/_editor_frame.html.slim | 4 +- app/views/request_for_comments/show.html.erb | 13 ++----- db/schema.rb | 3 +- 7 files changed, 57 insertions(+), 33 deletions(-) diff --git a/app/assets/javascripts/editor.js.erb b/app/assets/javascripts/editor.js.erb index 1e64195e..b706b437 100644 --- a/app/assets/javascripts/editor.js.erb +++ b/app/assets/javascripts/editor.js.erb @@ -1160,25 +1160,26 @@ $(function() { var file_id = $('.editor').data('id') var question = $('#question').val(); - $.ajax({ - method: 'POST', - url: '/request_for_comments', - data: { - request_for_comment: { - exercise_id: exercise_id, - file_id: file_id, - question: question, - "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, - "requested_at(4i)":17, - "requested_at(5i)":06 + var createRequestForComments = function(submission) { + console.log(submission); + $.ajax({ + method: 'POST', + url: '/request_for_comments', + data: { + request_for_comment: { + exercise_id: exercise_id, + file_id: file_id, + submission_id: submission.id, + question: question + } } - } - }).done(function() { - hideSpinner(); - $.flash.success({ text: $('#askForCommentsButton').data('message-success') }) - }).error(ajaxError); + }).done(function() { + hideSpinner(); + $.flash.success({ text: $('#askForCommentsButton').data('message-success') }); + }).error(ajaxError); + } + + createSubmission($('.requestCommentsButton'), null, createRequestForComments); $('#comment-modal').modal('hide'); var button = $('.requestCommentsButton'); diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index 72653ab2..37d8bef9 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/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 diff --git a/app/models/request_for_comment.rb b/app/models/request_for_comment.rb index 57b9a079..1343f5b3 100644 --- a/app/models/request_for_comment.rb +++ b/app/models/request_for_comment.rb @@ -25,6 +25,35 @@ class RequestForComment < ActiveRecord::Base limit 1").first end + def last_submission_before_creation + submission1 = 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 + submission2 = Submission.find_by_sql(" select * from submissions + where exercise_id = #{exercise_id} AND + user_id = #{user_id} AND + '#{created_at}' > created_at + order by created_at desc + limit 1").first + submission3 = Submission.find_by_sql(" select * from submissions + where exercise_id = #{exercise_id} AND + user_id = #{user_id} AND + '#{created_at.strftime('%Y-%m-%d %H:%M:%S.%N')}' > created_at + order by created_at desc + limit 1").first + submission4 = Submission.find_by_sql(" select * from submissions + where exercise_id = #{exercise_id} AND + user_id = #{user_id} AND + '#{created_at.localtime.strftime('%Y-%m-%d %H:%M:%S.%N')}' > created_at + order by created_at desc + limit 1").first + binding.pry + submission1 + end + def comments_count submission.files.map { |file| file.comments.size}.sum end diff --git a/app/models/submission.rb b/app/models/submission.rb index 323f1d58..28e98555 100644 --- a/app/models/submission.rb +++ b/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 diff --git a/app/views/exercises/_editor_frame.html.slim b/app/views/exercises/_editor_frame.html.slim index eacc62a9..01640fa8 100644 --- a/app/views/exercises/_editor_frame.html.slim +++ b/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' - i.fa.fa-comment-o + button.btn.btn-primary.requestCommentsButton type='button' id="requestComments" + i.fa.fa-comment = t('exercises.editor.requestComments') \ No newline at end of file diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index 6ef176bd..8e52150c 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -3,19 +3,12 @@

<% user = @request_for_comment.user - submission_id = ActiveRecord::Base.connection.execute("select id from submissions - 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) + submission = @request_for_comment.last_submission_before_creation %> - <%= user.displayname %> | <%= @request_for_comment.requested_at %> + <%= user.displayname %> | <%= @request_for_comment.requested_at.localtime %>

- <%= t('activerecord.attributes.exercise.instructions') %>: "<%= @request_for_comment.exercise.description %>" + <%= t('activerecord.attributes.exercise.description') %>: "<%= render_markdown(@request_for_comment.exercise.description) %>"
diff --git a/db/schema.rb b/db/schema.rb index 67b3b35c..287b5bd5 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: 20160624130951) do +ActiveRecord::Schema.define(version: 20160630154310) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -191,6 +191,7 @@ ActiveRecord::Schema.define(version: 20160624130951) do t.string "user_type" t.text "question" t.boolean "solved" + t.integer "submission_id" end create_table "submissions", force: true do |t| From 59536ab189102771eba68578cf56332e868f5e52 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 4 Jul 2016 17:46:37 +0200 Subject: [PATCH 2/4] the migrations to the changes in schema.rb. --- .../20160630154310_add_submission_to_request_for_comments.rb | 5 +++++ ...01092140_remove_requested_at_from_request_for_comments.rb | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 db/migrate/20160630154310_add_submission_to_request_for_comments.rb create mode 100644 db/migrate/20160701092140_remove_requested_at_from_request_for_comments.rb diff --git a/db/migrate/20160630154310_add_submission_to_request_for_comments.rb b/db/migrate/20160630154310_add_submission_to_request_for_comments.rb new file mode 100644 index 00000000..d7d13e67 --- /dev/null +++ b/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 diff --git a/db/migrate/20160701092140_remove_requested_at_from_request_for_comments.rb b/db/migrate/20160701092140_remove_requested_at_from_request_for_comments.rb new file mode 100644 index 00000000..bb5611f6 --- /dev/null +++ b/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 From f5868a4fa24db189eceb2db5defa04e99c11eedd Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Thu, 14 Jul 2016 13:22:24 +0200 Subject: [PATCH 3/4] merge master --- app/assets/javascripts/editor.js.erb | 59 +++++++++++- app/controllers/exercises_controller.rb | 8 +- app/controllers/teams_controller.rb | 51 ---------- app/models/exercise.rb | 1 - app/models/internal_user.rb | 2 - app/models/team.rb | 10 -- app/policies/exercise_policy.rb | 9 +- app/policies/team_policy.rb | 14 --- app/views/application/_navigation.html.slim | 2 +- app/views/exercises/_form.html.slim | 3 - .../external_users/statistics.html.slim | 4 +- app/views/exercises/show.html.slim | 1 - app/views/teams/_form.html.slim | 9 -- app/views/teams/edit.html.slim | 3 - app/views/teams/index.html.slim | 20 ---- app/views/teams/new.html.slim | 3 - app/views/teams/show.html.slim | 9 -- config/locales/de.yml | 8 -- config/locales/en.yml | 8 -- config/routes.rb | 1 - db/migrate/20160704143402_remove_teams.rb | 7 ++ db/schema.rb | 17 +--- db/seeds/development.rb | 3 - spec/controllers/teams_controller_spec.rb | 93 ------------------- spec/factories/team.rb | 6 -- spec/features/authorization_spec.rb | 6 +- spec/models/team_spec.rb | 9 -- spec/policies/exercise_policy_spec.rb | 22 +---- spec/policies/team_policy_spec.rb | 41 -------- 29 files changed, 78 insertions(+), 351 deletions(-) delete mode 100644 app/controllers/teams_controller.rb delete mode 100644 app/models/team.rb delete mode 100644 app/policies/team_policy.rb delete mode 100644 app/views/teams/_form.html.slim delete mode 100644 app/views/teams/edit.html.slim delete mode 100644 app/views/teams/index.html.slim delete mode 100644 app/views/teams/new.html.slim delete mode 100644 app/views/teams/show.html.slim create mode 100644 db/migrate/20160704143402_remove_teams.rb delete mode 100644 spec/controllers/teams_controller_spec.rb delete mode 100644 spec/factories/team.rb delete mode 100644 spec/models/team_spec.rb delete mode 100644 spec/policies/team_policy_spec.rb diff --git a/app/assets/javascripts/editor.js.erb b/app/assets/javascripts/editor.js.erb index b706b437..480f6580 100644 --- a/app/assets/javascripts/editor.js.erb +++ b/app/assets/javascripts/editor.js.erb @@ -19,6 +19,10 @@ $(function() { var SERVER_SEND_EVENT = 2; var editors = []; + var editor_for_file = new Map(); + var regex_for_language = new Map(); + var tracepositions_regex; + var active_file = undefined; var active_frame = undefined; var running = false; @@ -404,12 +408,18 @@ $(function() { editor.setTheme(THEME); editor.commands.bindKey("ctrl+alt+0", null); editors.push(editor); + editor_for_file.set($(element).parent().data('filename'), editor); var session = editor.getSession(); session.setMode($(element).data('mode')); session.setTabSize($(element).data('indent-size')); session.setUseSoftTabs(true); session.setUseWrapMode(true); + // set regex for parsing error traces based on the mode of the main file. + if( $(element).parent().data('role') == "main_file"){ + tracepositions_regex = regex_for_language.get($(element).data('mode')); + } + var file_id = $(element).data('id'); /* @@ -457,6 +467,12 @@ $(function() { $('#request-for-comments').on('click', requestComments); }; + + var initializeRegexes = function(){ + regex_for_language.set("ace/mode/python", /File "(.+?)", line (\d+)/g); + regex_for_language.set("ace/mode/java", /(.*\.java):(\d+):/g); + } + var initializeTooltips = function() { $('[data-tooltip]').tooltip(); }; @@ -587,7 +603,7 @@ $(function() { } else if (output.stdout) { //if (output_mode_is_streaming){ element.addClass('text-success').append(output.stdout); - flowrOutputBuffer += output.stdout; + // flowrOutputBuffer += output.stdout; //}else{ // element.addClass('text-success'); // element.data('content_buffer' , element.data('content_buffer') + output.stdout); @@ -876,7 +892,9 @@ $(function() { } var showWorkspaceTab = function(event) { - event.preventDefault(); + if(event){ + event.preventDefault(); + } showTab(0); }; @@ -1036,6 +1054,7 @@ $(function() { case 'exit': killWebsocketAndContainer(); handleStderrOutputForFlowr(); + augmentStacktraceInOutput(); break; case 'timeout': // just show the timeout message here. Another exit command is sent by the rails backend when the socket to the docker container closes. @@ -1047,6 +1066,41 @@ $(function() { } }; + + var jumpToSourceLine = function(event){ + var file = $(event.target).data('file'); + var line = $(event.target).data('line'); + + showWorkspaceTab(null); + // set active file ?!?! + + var frame = $('div.frame[data-filename="' + file + '"]'); + showFrame(frame); + + var editor = editor_for_file.get(file); + editor.gotoLine(line, 0); + + }; + + var augmentStacktraceInOutput = function() { + if(tracepositions_regex){ + var element = $('#output>pre'); + var text = element.text(); + element.on( "click", "a", jumpToSourceLine); + + var matches; + + while(matches = tracepositions_regex.exec(text)){ + var frame = $('div.frame[data-filename="' + matches[1] + '"]') + + if(frame.length > 0){ + element.html(text.replace(matches[0], "" + matches[0] + "")); + } + } + } + + }; + var renderWebsocketOutput = function(msg){ var element = findOrCreateRenderElement(0); element.append(msg.data); @@ -1208,6 +1262,7 @@ $(function() { if ($('#editor').isPresent()) { if (isBrowserSupported()) { + initializeRegexes(); initializeCodePilot(); $('.score, #development-environment').show(); configureEditors(); diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 0304abb4..45fd04d9 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -9,7 +9,6 @@ class ExercisesController < ApplicationController before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :implement, :run, :statistics, :submit, :reload] before_action :set_external_user, only: [:statistics] before_action :set_file_types, only: [:create, :edit, :new, :update] - before_action :set_teams, only: [:create, :edit, :new, :update] skip_before_filter :verify_authenticity_token, only: [:import_proforma_xml] skip_after_action :verify_authorized, only: [:import_proforma_xml] @@ -119,7 +118,7 @@ class ExercisesController < ApplicationController private :user_by_code_harbor_token def exercise_params - params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :team_id, :title, files_attributes: file_attributes).merge(user_id: current_user.id, user_type: current_user.class.name) + params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :title, files_attributes: file_attributes).merge(user_id: current_user.id, user_type: current_user.class.name) end private :exercise_params @@ -195,11 +194,6 @@ class ExercisesController < ApplicationController end private :set_file_types - def set_teams - @teams = Team.all.order(:name) - end - private :set_teams - def show end diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb deleted file mode 100644 index 1ca8f1fe..00000000 --- a/app/controllers/teams_controller.rb +++ /dev/null @@ -1,51 +0,0 @@ -class TeamsController < ApplicationController - include CommonBehavior - - before_action :set_team, only: MEMBER_ACTIONS - - def authorize! - authorize(@team || @teams) - end - private :authorize! - - def create - @team = Team.new(team_params) - authorize! - create_and_respond(object: @team) - end - - def destroy - destroy_and_respond(object: @team) - end - - def edit - end - - def index - @teams = Team.all.includes(:internal_users).order(:name).paginate(page: params[:page]) - authorize! - end - - def new - @team = Team.new - authorize! - end - - def set_team - @team = Team.find(params[:id]) - authorize! - end - private :set_team - - def show - end - - def team_params - params[:team].permit(:name, internal_user_ids: []) - end - private :team_params - - def update - update_and_respond(object: @team, params: team_params) - end -end diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 4a1e0486..5b7ff9b2 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -11,7 +11,6 @@ class Exercise < ActiveRecord::Base belongs_to :execution_environment has_many :submissions - belongs_to :team has_many :external_users, source: :user, source_type: ExternalUser, through: :submissions has_many :internal_users, source: :user, source_type: InternalUser, through: :submissions diff --git a/app/models/internal_user.rb b/app/models/internal_user.rb index e5cebde9..8f1bf04b 100644 --- a/app/models/internal_user.rb +++ b/app/models/internal_user.rb @@ -3,8 +3,6 @@ class InternalUser < ActiveRecord::Base authenticates_with_sorcery! - has_and_belongs_to_many :teams - validates :email, presence: true, uniqueness: true validates :password, confirmation: true, if: :password_void?, on: :update, presence: true validates :role, inclusion: {in: ROLES} diff --git a/app/models/team.rb b/app/models/team.rb deleted file mode 100644 index a0dcb8d6..00000000 --- a/app/models/team.rb +++ /dev/null @@ -1,10 +0,0 @@ -class Team < ActiveRecord::Base - has_and_belongs_to_many :internal_users - alias_method :members, :internal_users - - validates :name, presence: true - - def to_s - name - end -end diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index 29a1c570..55f7d16b 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -13,24 +13,19 @@ class ExercisePolicy < AdminOrAuthorPolicy end [:clone?, :destroy?, :edit?, :statistics?, :update?].each do |action| - define_method(action) { admin? || author? || team_member? } + define_method(action) { admin? || author?} end [:implement?, :submit?, :reload?].each do |action| define_method(action) { everyone } end - def team_member? - @record.team.try(:members, []).include?(@user) if @record.team - end - private :team_member? - class Scope < Scope def resolve if @user.admin? @scope.all elsif @user.internal_user? - @scope.where('user_id = ? OR public = TRUE OR (team_id IS NOT NULL AND team_id IN (SELECT t.id FROM teams t JOIN internal_users_teams iut ON t.id = iut.team_id WHERE iut.internal_user_id = ?))', @user.id, @user.id) + @scope.where('user_id = ? OR public = TRUE', @user.id) else @scope.none end diff --git a/app/policies/team_policy.rb b/app/policies/team_policy.rb deleted file mode 100644 index ff05c0c3..00000000 --- a/app/policies/team_policy.rb +++ /dev/null @@ -1,14 +0,0 @@ -class TeamPolicy < ApplicationPolicy - [:create?, :index?, :new?].each do |action| - define_method(action) { admin? } - end - - [:destroy?, :edit?, :show?, :update?].each do |action| - define_method(action) { admin? || member? } - end - - def member? - @record.members.include?(@user) - end - private :member? -end diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index 4ab39e30..3c260cb8 100644 --- a/app/views/application/_navigation.html.slim +++ b/app/views/application/_navigation.html.slim @@ -8,7 +8,7 @@ - if current_user.admin? li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path) li.divider - - models = [ExecutionEnvironment, Exercise, Consumer, CodeHarborLink, ExternalUser, FileType, InternalUser, Submission, Team].sort_by { |model| model.model_name.human(count: 2) } + - models = [ExecutionEnvironment, Exercise, Consumer, CodeHarborLink, ExternalUser, FileType, InternalUser, Submission].sort_by { |model| model.model_name.human(count: 2) } - models.each do |model| - if policy(model).index? li = link_to(model.model_name.human(count: 2), send(:"#{model.model_name.collection}_path")) diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index f0f69f7a..968540af 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -17,9 +17,6 @@ = f.label(:instructions) = f.hidden_field(:instructions) .form-control.markdown - /.form-group - = f.label(:team_id) - = f.collection_select(:team_id, @teams, :id, :name, {include_blank: true}, class: 'form-control') .checkbox label = f.check_box(:public) diff --git a/app/views/exercises/external_users/statistics.html.slim b/app/views/exercises/external_users/statistics.html.slim index 56bd614b..3c755be6 100644 --- a/app/views/exercises/external_users/statistics.html.slim +++ b/app/views/exercises/external_users/statistics.html.slim @@ -50,9 +50,9 @@ h1 = "#{@exercise} (external user #{@external_user})" td -submission.testruns.each do |run| - if run.passed - .unit-test-result.positive-result + .unit-test-result.positive-result title=run.output - else - .unit-test-result.negative-result + .unit-test-result.negative-result title=run.output td = Time.at(deltas[1..index].inject(:+)).utc.strftime("%H:%M:%S") if index > 0 -working_times_until.push((Time.at(deltas[1..index].inject(:+)).utc.strftime("%H:%M:%S") if index > 0)) p = t('.addendum') diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index b4b72932..5c554da8 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -12,7 +12,6 @@ h1 = row(label: 'exercise.description', value: render_markdown(@exercise.description)) = row(label: 'exercise.execution_environment', value: link_to_if(policy(@exercise.execution_environment).show?, @exercise.execution_environment, @exercise.execution_environment)) /= row(label: 'exercise.instructions', value: render_markdown(@exercise.instructions)) -= row(label: 'exercise.team', value: @exercise.team ? link_to(@exercise.team, @exercise.team) : nil) = row(label: 'exercise.maximum_score', value: @exercise.maximum_score) = row(label: 'exercise.public', value: @exercise.public?) = row(label: 'exercise.hide_file_tree', value: @exercise.hide_file_tree?) diff --git a/app/views/teams/_form.html.slim b/app/views/teams/_form.html.slim deleted file mode 100644 index 47f547dd..00000000 --- a/app/views/teams/_form.html.slim +++ /dev/null @@ -1,9 +0,0 @@ -= form_for(@team) do |f| - = render('shared/form_errors', object: @team) - .form-group - = f.label(:name) - = f.text_field(:name, class: 'form-control', required: true) - .form-group - = f.label(:internal_user_ids) - = f.collection_select(:internal_user_ids, InternalUser.all.order(:name), :id, :name, {}, {class: 'form-control', multiple: true}) - .actions = render('shared/submit_button', f: f, object: @team) diff --git a/app/views/teams/edit.html.slim b/app/views/teams/edit.html.slim deleted file mode 100644 index 0c1f4dac..00000000 --- a/app/views/teams/edit.html.slim +++ /dev/null @@ -1,3 +0,0 @@ -h1 = @hint - -= render('form') diff --git a/app/views/teams/index.html.slim b/app/views/teams/index.html.slim deleted file mode 100644 index 4eb6640b..00000000 --- a/app/views/teams/index.html.slim +++ /dev/null @@ -1,20 +0,0 @@ -h1 = Team.model_name.human(count: 2) - -.table-responsive - table.table - thead - tr - th = t('activerecord.attributes.team.name') - th = t('activerecord.attributes.team.internal_user_ids') - th colspan=3 = t('shared.actions') - tbody - - @teams.each do |team| - tr - td = team.name - td = team.members.count - td = link_to(t('shared.show'), team_path(team.id)) - td = link_to(t('shared.edit'), edit_team_path(team.id)) - td = link_to(t('shared.destroy'), team_path(team.id), data: {confirm: t('shared.confirm_destroy')}, method: :delete) - -= render('shared/pagination', collection: @teams) -p = render('shared/new_button', model: Team, path: new_team_path) diff --git a/app/views/teams/new.html.slim b/app/views/teams/new.html.slim deleted file mode 100644 index 8c8f2aab..00000000 --- a/app/views/teams/new.html.slim +++ /dev/null @@ -1,3 +0,0 @@ -h1 = t('shared.new_model', model: Team.model_name.human) - -= render('form') diff --git a/app/views/teams/show.html.slim b/app/views/teams/show.html.slim deleted file mode 100644 index 1b37d931..00000000 --- a/app/views/teams/show.html.slim +++ /dev/null @@ -1,9 +0,0 @@ -h1 - = @team - = render('shared/edit_button', object: @team, path: edit_team_path(@team.id)) - -= row(label: 'team.name', value: @team.name) -= row(label: 'team.internal_user_ids') do - ul.list-unstyled - - @team.members.order(:name).each do |internal_user| - li = link_to(internal_user, internal_user) diff --git a/config/locales/de.yml b/config/locales/de.yml index 53df1858..ce8ba98f 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -34,8 +34,6 @@ de: instructions: Anweisungen maximum_score: Erreichbare Punktzahl public: Öffentlich - team: Team - team_id: Team title: Titel user: Autor allow_file_creation: "Dateierstellung erlauben" @@ -91,9 +89,6 @@ de: files: Dateien score: Punktzahl user: Autor - team: - internal_user_ids: Mitglieder - name: Name models: code_harbor_link: one: CodeHarbor-Link @@ -128,9 +123,6 @@ de: submission: one: Abgabe other: Abgaben - team: - one: Team - other: Teams errors: messages: together: 'muss zusammen mit %{attribute} definiert werden' diff --git a/config/locales/en.yml b/config/locales/en.yml index 1e72cb34..86acbb15 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -34,8 +34,6 @@ en: instructions: Instructions maximum_score: Maximum Score public: Public - team: Team - team_id: Team title: Title user: Author allow_file_creation: "Allow file creation" @@ -91,9 +89,6 @@ en: files: Files score: Score user: Author - team: - internal_user_ids: Members - name: Name models: code_harbor_link: one: CodeHarbor Link @@ -128,9 +123,6 @@ en: submission: one: Submission other: Submissions - team: - one: Team - other: Teams errors: messages: together: 'has to be set along with %{attribute}' diff --git a/config/routes.rb b/config/routes.rb index a6ca545c..f13d1c9f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -99,5 +99,4 @@ Rails.application.routes.draw do end end - resources :teams end diff --git a/db/migrate/20160704143402_remove_teams.rb b/db/migrate/20160704143402_remove_teams.rb new file mode 100644 index 00000000..20b8a204 --- /dev/null +++ b/db/migrate/20160704143402_remove_teams.rb @@ -0,0 +1,7 @@ +class RemoveTeams < ActiveRecord::Migration + def change + remove_reference :exercises, :team + drop_table :teams + drop_table :internal_users_teams + end +end diff --git a/db/schema.rb b/db/schema.rb index 287b5bd5..5cad22c6 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: 20160630154310) do +ActiveRecord::Schema.define(version: 20160704143402) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -87,7 +87,6 @@ ActiveRecord::Schema.define(version: 20160630154310) do t.boolean "public" t.string "user_type" t.string "token" - t.integer "team_id" t.boolean "hide_file_tree" t.boolean "allow_file_creation" end @@ -173,14 +172,6 @@ ActiveRecord::Schema.define(version: 20160630154310) do add_index "internal_users", ["remember_me_token"], name: "index_internal_users_on_remember_me_token", using: :btree add_index "internal_users", ["reset_password_token"], name: "index_internal_users_on_reset_password_token", using: :btree - create_table "internal_users_teams", force: true do |t| - t.integer "internal_user_id" - t.integer "team_id" - end - - add_index "internal_users_teams", ["internal_user_id"], name: "index_internal_users_teams_on_internal_user_id", using: :btree - 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 "user_id", null: false t.integer "exercise_id", null: false @@ -204,12 +195,6 @@ ActiveRecord::Schema.define(version: 20160630154310) do t.string "user_type" end - create_table "teams", force: true do |t| - t.string "name" - t.datetime "created_at" - t.datetime "updated_at" - end - create_table "testruns", force: true do |t| t.boolean "passed" t.text "output" diff --git a/db/seeds/development.rb b/db/seeds/development.rb index 74783d39..6fc8e050 100644 --- a/db/seeds/development.rb +++ b/db/seeds/development.rb @@ -22,6 +22,3 @@ Hint.create_factories # submissions FactoryGirl.create(:submission, exercise: @exercises[:fibonacci]) - -# teams -FactoryGirl.create(:team, internal_users: InternalUser.limit(10)) diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb deleted file mode 100644 index 78dfc60d..00000000 --- a/spec/controllers/teams_controller_spec.rb +++ /dev/null @@ -1,93 +0,0 @@ -require 'rails_helper' - -describe TeamsController do - let(:team) { FactoryGirl.create(:team) } - let(:user) { FactoryGirl.create(:admin) } - before(:each) { allow(controller).to receive(:current_user).and_return(user) } - - describe 'POST #create' do - context 'with a valid team' do - let(:request) { proc { post :create, team: FactoryGirl.attributes_for(:team) } } - before(:each) { request.call } - - expect_assigns(team: Team) - - it 'creates the team' do - expect { request.call }.to change(Team, :count).by(1) - end - - expect_redirect(Team.last) - end - - context 'with an invalid team' do - before(:each) { post :create, team: {} } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:new) - end - end - - describe 'DELETE #destroy' do - before(:each) { delete :destroy, id: team.id } - - expect_assigns(team: Team) - - it 'destroys the team' do - team = FactoryGirl.create(:team) - expect { delete :destroy, id: team.id }.to change(Team, :count).by(-1) - end - - expect_redirect(:teams) - end - - describe 'GET #edit' do - before(:each) { get :edit, id: team.id } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:edit) - end - - describe 'GET #index' do - before(:all) { FactoryGirl.create_pair(:team) } - before(:each) { get :index } - - expect_assigns(teams: Team.all) - expect_status(200) - expect_template(:index) - end - - describe 'GET #new' do - before(:each) { get :new } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:new) - end - - describe 'GET #show' do - before(:each) { get :show, id: team.id } - - expect_assigns(team: :team) - expect_status(200) - expect_template(:show) - end - - describe 'PUT #update' do - context 'with a valid team' do - before(:each) { put :update, team: FactoryGirl.attributes_for(:team), id: team.id } - - expect_assigns(team: Team) - expect_redirect(:team) - end - - context 'with an invalid team' do - before(:each) { put :update, team: {name: ''}, id: team.id } - - expect_assigns(team: Team) - expect_status(200) - expect_template(:edit) - end - end -end diff --git a/spec/factories/team.rb b/spec/factories/team.rb deleted file mode 100644 index f34d323e..00000000 --- a/spec/factories/team.rb +++ /dev/null @@ -1,6 +0,0 @@ -FactoryGirl.define do - factory :team do - internal_users { build_pair :teacher } - name 'The A-Team' - end -end diff --git a/spec/features/authorization_spec.rb b/spec/features/authorization_spec.rb index 6a7eaad0..7f0ff04f 100644 --- a/spec/features/authorization_spec.rb +++ b/spec/features/authorization_spec.rb @@ -5,7 +5,7 @@ describe 'Authorization' do let(:user) { FactoryGirl.create(:admin) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } - [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser, Team].each do |model| + [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser].each do |model| expect_permitted_path(:"new_#{model.model_name.singular}_path") end end @@ -14,7 +14,7 @@ describe 'Authorization' do let(:user) { FactoryGirl.create(:external_user) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } - [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser, Team].each do |model| + [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser].each do |model| expect_forbidden_path(:"new_#{model.model_name.singular}_path") end end @@ -27,7 +27,7 @@ describe 'Authorization' do expect_forbidden_path(:"new_#{model.model_name.singular}_path") end - [ExecutionEnvironment, Exercise, FileType, Team].each do |model| + [ExecutionEnvironment, Exercise, FileType].each do |model| expect_permitted_path(:"new_#{model.model_name.singular}_path") end end diff --git a/spec/models/team_spec.rb b/spec/models/team_spec.rb deleted file mode 100644 index 777efd32..00000000 --- a/spec/models/team_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'rails_helper' - -describe Team do - let(:team) { described_class.create } - - it 'validates the presence of a name' do - expect(team.errors[:name]).to be_present - end -end diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index c9762f9e..7b5aeabf 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -3,8 +3,8 @@ require 'rails_helper' describe ExercisePolicy do subject { described_class } - let(:exercise) { FactoryGirl.build(:dummy, team: FactoryGirl.create(:team)) } - +let(:exercise) { FactoryGirl.build(:dummy) } + permissions :batch_update? do it 'grants access to admins only' do expect(subject).to permit(FactoryGirl.build(:admin), exercise) @@ -40,10 +40,6 @@ describe ExercisePolicy do expect(subject).to permit(exercise.author, exercise) end - it 'grants access to team members' do - expect(subject).to permit(exercise.team.members.first, exercise) - end - it 'does not grant access to all other users' do [:external_user, :teacher].each do |factory_name| expect(subject).not_to permit(FactoryGirl.build(factory_name), exercise) @@ -71,9 +67,7 @@ describe ExercisePolicy do [@admin, @teacher].each do |user| [true, false].each do |public| - [@team, nil].each do |team| - FactoryGirl.create(:dummy, public: public, team: team, user_id: user.id, user_type: InternalUser.class.name) - end + FactoryGirl.create(:dummy, public: public, user_id: user.id, user_type: InternalUser.class.name) end end end @@ -95,10 +89,6 @@ describe ExercisePolicy do end context 'for teachers' do - before(:each) do - @team = FactoryGirl.create(:team) - @team.members << @teacher - end let(:scope) { Pundit.policy_scope!(@teacher, Exercise) } @@ -110,12 +100,8 @@ describe ExercisePolicy do expect(scope.map(&:id)).to include(*Exercise.where(public: false, user_id: @teacher.id).map(&:id)) end - it "includes all of team members' non-public exercises" do - expect(scope.map(&:id)).to include(*Exercise.where(public: false, team_id: @teacher.teams.first.id).map(&:id)) - end - it "does not include other authors' non-public exercises" do - expect(scope.map(&:id)).not_to include(*Exercise.where(public: false).where("team_id <> #{@team.id} AND user_id <> #{@teacher.id}").map(&:id)) + expect(scope.map(&:id)).not_to include(*Exercise.where(public: false).where(user_id <> #{@teacher.id}").map(&:id)) end end end diff --git a/spec/policies/team_policy_spec.rb b/spec/policies/team_policy_spec.rb deleted file mode 100644 index aa3ba1d8..00000000 --- a/spec/policies/team_policy_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -require 'rails_helper' - -describe TeamPolicy do - subject { described_class } - - let(:team) { FactoryGirl.build(:team) } - - [:create?, :index?, :new?].each do |action| - permissions(action) do - it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), team) - end - - it 'grants access to teachers' do - expect(subject).to permit(FactoryGirl.build(:teacher), team) - end - - it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), team) - end - end - end - - [:destroy?, :edit?, :show?, :update?].each do |action| - permissions(action) do - it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), team) - end - - it 'grants access to members' do - expect(subject).to permit(team.members.last, team) - end - - it 'does not grant access to all other users' do - [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), team) - end - end - end - end -end From 8282820974620ad9fe3634632dacf816a07fcd5b Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Thu, 14 Jul 2016 16:22:22 +0200 Subject: [PATCH 4/4] removed requested_at partly using submission_id some cleanup --- app/assets/javascripts/editor.js.erb | 1 - app/models/request_for_comment.rb | 28 ++++--------------- .../request_for_comments/index.html.slim | 2 +- app/views/request_for_comments/show.html.erb | 4 +-- db/schema.rb | 7 ++--- 5 files changed, 12 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/editor.js.erb b/app/assets/javascripts/editor.js.erb index 480f6580..658ff3cd 100644 --- a/app/assets/javascripts/editor.js.erb +++ b/app/assets/javascripts/editor.js.erb @@ -1215,7 +1215,6 @@ $(function() { var question = $('#question').val(); var createRequestForComments = function(submission) { - console.log(submission); $.ajax({ method: 'POST', url: '/request_for_comments', diff --git a/app/models/request_for_comment.rb b/app/models/request_for_comment.rb index 1343f5b3..cd57c5c2 100644 --- a/app/models/request_for_comment.rb +++ b/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,33 +27,15 @@ 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 - submission1 = Submission.find_by_sql(" select * from submissions + 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 - submission2 = Submission.find_by_sql(" select * from submissions - where exercise_id = #{exercise_id} AND - user_id = #{user_id} AND - '#{created_at}' > created_at - order by created_at desc - limit 1").first - submission3 = Submission.find_by_sql(" select * from submissions - where exercise_id = #{exercise_id} AND - user_id = #{user_id} AND - '#{created_at.strftime('%Y-%m-%d %H:%M:%S.%N')}' > created_at - order by created_at desc - limit 1").first - submission4 = Submission.find_by_sql(" select * from submissions - where exercise_id = #{exercise_id} AND - user_id = #{user_id} AND - '#{created_at.localtime.strftime('%Y-%m-%d %H:%M:%S.%N')}' > created_at - order by created_at desc - limit 1").first - binding.pry - submission1 end def comments_count @@ -64,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 diff --git a/app/views/request_for_comments/index.html.slim b/app/views/request_for_comments/index.html.slim index 3bdbe6d0..b7ada0a2 100644 --- a/app/views/request_for_comments/index.html.slim +++ b/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) \ No newline at end of file diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index 8e52150c..c1d71672 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -1,11 +1,11 @@
-

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

+

<%= link_to(@request_for_comment.exercise.title, [:implement, @request_for_comment.exercise]) %>

<% user = @request_for_comment.user submission = @request_for_comment.last_submission_before_creation %> - <%= user.displayname %> | <%= @request_for_comment.requested_at.localtime %> + <%= user.displayname %> | <%= @request_for_comment.created_at.localtime %>

<%= t('activerecord.attributes.exercise.description') %>: "<%= render_markdown(@request_for_comment.exercise.description) %>" diff --git a/db/schema.rb b/db/schema.rb index 5cad22c6..79d13498 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -173,10 +173,9 @@ 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 "exercise_id", null: false - t.integer "file_id", null: false - t.datetime "requested_at" + t.integer "user_id", null: false + t.integer "exercise_id", null: false + t.integer "file_id", null: false t.datetime "created_at" t.datetime "updated_at" t.string "user_type"