From d2a089d057706f888d814c4ac2d66a50900f48dc Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Mon, 3 Jul 2017 10:09:48 +0200 Subject: [PATCH 01/69] Add structured errors to schema --- app/models/error_template.rb | 3 ++ app/models/error_template_attribute.rb | 3 ++ app/models/structured_error.rb | 4 +++ app/models/structured_error_attribute.rb | 4 +++ .../20170703075832_create_error_templates.rb | 11 +++++++ ...075959_create_error_template_attributes.rb | 11 +++++++ ...20170703080205_create_structured_errors.rb | 10 ++++++ ...0355_create_structured_error_attributes.rb | 11 +++++++ db/schema.rb | 33 ++++++++++++++++++- test/factories/error_template_attributes.rb | 7 ++++ test/factories/error_templates.rb | 7 ++++ test/factories/structured_error_attributes.rb | 7 ++++ test/factories/structured_errors.rb | 6 ++++ test/models/error_template_attribute_test.rb | 7 ++++ test/models/error_template_test.rb | 7 ++++ .../models/structured_error_attribute_test.rb | 7 ++++ test/models/structured_error_test.rb | 7 ++++ 17 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 app/models/error_template.rb create mode 100644 app/models/error_template_attribute.rb create mode 100644 app/models/structured_error.rb create mode 100644 app/models/structured_error_attribute.rb create mode 100644 db/migrate/20170703075832_create_error_templates.rb create mode 100644 db/migrate/20170703075959_create_error_template_attributes.rb create mode 100644 db/migrate/20170703080205_create_structured_errors.rb create mode 100644 db/migrate/20170703080355_create_structured_error_attributes.rb create mode 100644 test/factories/error_template_attributes.rb create mode 100644 test/factories/error_templates.rb create mode 100644 test/factories/structured_error_attributes.rb create mode 100644 test/factories/structured_errors.rb create mode 100644 test/models/error_template_attribute_test.rb create mode 100644 test/models/error_template_test.rb create mode 100644 test/models/structured_error_attribute_test.rb create mode 100644 test/models/structured_error_test.rb diff --git a/app/models/error_template.rb b/app/models/error_template.rb new file mode 100644 index 00000000..9b441d51 --- /dev/null +++ b/app/models/error_template.rb @@ -0,0 +1,3 @@ +class ErrorTemplate < ActiveRecord::Base + belongs_to :execution_environment +end diff --git a/app/models/error_template_attribute.rb b/app/models/error_template_attribute.rb new file mode 100644 index 00000000..361b727b --- /dev/null +++ b/app/models/error_template_attribute.rb @@ -0,0 +1,3 @@ +class ErrorTemplateAttribute < ActiveRecord::Base + belongs_to :error_template +end diff --git a/app/models/structured_error.rb b/app/models/structured_error.rb new file mode 100644 index 00000000..46f40423 --- /dev/null +++ b/app/models/structured_error.rb @@ -0,0 +1,4 @@ +class StructuredError < ActiveRecord::Base + belongs_to :error_template + belongs_to :file, class_name: 'CodeOcean::File' +end diff --git a/app/models/structured_error_attribute.rb b/app/models/structured_error_attribute.rb new file mode 100644 index 00000000..335a5901 --- /dev/null +++ b/app/models/structured_error_attribute.rb @@ -0,0 +1,4 @@ +class StructuredErrorAttribute < ActiveRecord::Base + belongs_to :structured_error + belongs_to :error_template_attribute +end diff --git a/db/migrate/20170703075832_create_error_templates.rb b/db/migrate/20170703075832_create_error_templates.rb new file mode 100644 index 00000000..6f442842 --- /dev/null +++ b/db/migrate/20170703075832_create_error_templates.rb @@ -0,0 +1,11 @@ +class CreateErrorTemplates < ActiveRecord::Migration + def change + create_table :error_templates do |t| + t.belongs_to :execution_environment + t.string :name + t.string :signature + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20170703075959_create_error_template_attributes.rb b/db/migrate/20170703075959_create_error_template_attributes.rb new file mode 100644 index 00000000..3503fcac --- /dev/null +++ b/db/migrate/20170703075959_create_error_template_attributes.rb @@ -0,0 +1,11 @@ +class CreateErrorTemplateAttributes < ActiveRecord::Migration + def change + create_table :error_template_attributes do |t| + t.belongs_to :error_template + t.string :key + t.string :regex + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20170703080205_create_structured_errors.rb b/db/migrate/20170703080205_create_structured_errors.rb new file mode 100644 index 00000000..560649b4 --- /dev/null +++ b/db/migrate/20170703080205_create_structured_errors.rb @@ -0,0 +1,10 @@ +class CreateStructuredErrors < ActiveRecord::Migration + def change + create_table :structured_errors do |t| + t.references :error_template + t.belongs_to :file + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20170703080355_create_structured_error_attributes.rb b/db/migrate/20170703080355_create_structured_error_attributes.rb new file mode 100644 index 00000000..aa9ee04e --- /dev/null +++ b/db/migrate/20170703080355_create_structured_error_attributes.rb @@ -0,0 +1,11 @@ +class CreateStructuredErrorAttributes < ActiveRecord::Migration + def change + create_table :structured_error_attributes do |t| + t.belongs_to :structured_error + t.references :error_template_attribute + t.string :value + + t.timestamps null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 81693c1d..6e12f864 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: 20170608141612) do +ActiveRecord::Schema.define(version: 20170703080355) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -47,6 +47,22 @@ ActiveRecord::Schema.define(version: 20170608141612) do t.string "oauth_secret", limit: 255 end + create_table "error_template_attributes", force: :cascade do |t| + t.integer "error_template_id" + t.string "key" + t.string "regex" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + create_table "error_templates", force: :cascade do |t| + t.integer "execution_environment_id" + t.string "name" + t.string "signature" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "errors", force: :cascade do |t| t.integer "execution_environment_id" t.text "message" @@ -268,6 +284,21 @@ ActiveRecord::Schema.define(version: 20170608141612) do t.datetime "updated_at" end + create_table "structured_error_attributes", force: :cascade do |t| + t.integer "structured_error_id" + t.integer "error_template_attribute_id" + t.string "value" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + create_table "structured_errors", force: :cascade do |t| + t.integer "error_template_id" + t.integer "file_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "submissions", force: :cascade do |t| t.integer "exercise_id" t.float "score" diff --git a/test/factories/error_template_attributes.rb b/test/factories/error_template_attributes.rb new file mode 100644 index 00000000..24adb856 --- /dev/null +++ b/test/factories/error_template_attributes.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :error_template_attribute do + error_template nil + key "MyString" + regex "MyString" + end +end diff --git a/test/factories/error_templates.rb b/test/factories/error_templates.rb new file mode 100644 index 00000000..2abf68c9 --- /dev/null +++ b/test/factories/error_templates.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :error_template do + execution_environment nil + name "MyString" + signature "MyString" + end +end diff --git a/test/factories/structured_error_attributes.rb b/test/factories/structured_error_attributes.rb new file mode 100644 index 00000000..7485967c --- /dev/null +++ b/test/factories/structured_error_attributes.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :structured_error_attribute do + structured_error nil + error_template_attribute nil + value "MyString" + end +end diff --git a/test/factories/structured_errors.rb b/test/factories/structured_errors.rb new file mode 100644 index 00000000..4a87cec1 --- /dev/null +++ b/test/factories/structured_errors.rb @@ -0,0 +1,6 @@ +FactoryGirl.define do + factory :structured_error do + error_template nil + file nil + end +end diff --git a/test/models/error_template_attribute_test.rb b/test/models/error_template_attribute_test.rb new file mode 100644 index 00000000..187ae1b7 --- /dev/null +++ b/test/models/error_template_attribute_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class ErrorTemplateAttributeTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/models/error_template_test.rb b/test/models/error_template_test.rb new file mode 100644 index 00000000..538dc19a --- /dev/null +++ b/test/models/error_template_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class ErrorTemplateTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/models/structured_error_attribute_test.rb b/test/models/structured_error_attribute_test.rb new file mode 100644 index 00000000..1dcb316b --- /dev/null +++ b/test/models/structured_error_attribute_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class StructuredErrorAttributeTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/models/structured_error_test.rb b/test/models/structured_error_test.rb new file mode 100644 index 00000000..28b03689 --- /dev/null +++ b/test/models/structured_error_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class StructuredErrorTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end From 872611bff64e624c5105fb526d16248297f0cd92 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Tue, 11 Jul 2017 19:22:12 +0200 Subject: [PATCH 02/69] Update schema to reflect re-usable error_template_attributes, descriptions, hints, and sorting --- app/models/error_template.rb | 1 + app/models/error_template_attribute.rb | 2 +- app/models/execution_environment.rb | 1 + ...add_description_and_hint_to_error_template.rb | 9 +++++++++ ..._template_attribute_relationship_to_n_to_m.rb | 6 ++++++ db/schema.rb | 16 ++++++++++++---- 6 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20170711170456_add_description_and_hint_to_error_template.rb create mode 100644 db/migrate/20170711170928_change_error_template_attribute_relationship_to_n_to_m.rb diff --git a/app/models/error_template.rb b/app/models/error_template.rb index 9b441d51..dc497d48 100644 --- a/app/models/error_template.rb +++ b/app/models/error_template.rb @@ -1,3 +1,4 @@ class ErrorTemplate < ActiveRecord::Base belongs_to :execution_environment + has_and_belongs_to_many :error_template_attributes end diff --git a/app/models/error_template_attribute.rb b/app/models/error_template_attribute.rb index 361b727b..15229b33 100644 --- a/app/models/error_template_attribute.rb +++ b/app/models/error_template_attribute.rb @@ -1,3 +1,3 @@ class ErrorTemplateAttribute < ActiveRecord::Base - belongs_to :error_template + has_and_belongs_to_many :error_template end diff --git a/app/models/execution_environment.rb b/app/models/execution_environment.rb index 3a4efdde..74177173 100644 --- a/app/models/execution_environment.rb +++ b/app/models/execution_environment.rb @@ -11,6 +11,7 @@ class ExecutionEnvironment < ActiveRecord::Base has_many :exercises belongs_to :file_type has_many :hints + has_many :error_templates scope :with_exercises, -> { where('id IN (SELECT execution_environment_id FROM exercises)') } diff --git a/db/migrate/20170711170456_add_description_and_hint_to_error_template.rb b/db/migrate/20170711170456_add_description_and_hint_to_error_template.rb new file mode 100644 index 00000000..62cbee95 --- /dev/null +++ b/db/migrate/20170711170456_add_description_and_hint_to_error_template.rb @@ -0,0 +1,9 @@ +class AddDescriptionAndHintToErrorTemplate < ActiveRecord::Migration + def change + add_column :error_templates, :description, :text + add_column :error_templates, :hint, :text + + add_column :error_template_attributes, :description, :text + add_column :error_template_attributes, :important, :boolean + end +end diff --git a/db/migrate/20170711170928_change_error_template_attribute_relationship_to_n_to_m.rb b/db/migrate/20170711170928_change_error_template_attribute_relationship_to_n_to_m.rb new file mode 100644 index 00000000..d10fbda2 --- /dev/null +++ b/db/migrate/20170711170928_change_error_template_attribute_relationship_to_n_to_m.rb @@ -0,0 +1,6 @@ +class ChangeErrorTemplateAttributeRelationshipToNToM < ActiveRecord::Migration + def change + remove_belongs_to :error_template_attributes, :error_template + create_join_table :error_templates, :error_template_attributes + end +end diff --git a/db/schema.rb b/db/schema.rb index 6e12f864..137d251d 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: 20170703080355) do +ActiveRecord::Schema.define(version: 20170711170928) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -48,11 +48,17 @@ ActiveRecord::Schema.define(version: 20170703080355) do end create_table "error_template_attributes", force: :cascade do |t| - t.integer "error_template_id" t.string "key" t.string "regex" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.text "description" + t.boolean "important" + end + + create_table "error_template_attributes_templates", id: false, force: :cascade do |t| + t.integer "error_template_id", null: false + t.integer "error_template_attribute_id", null: false end create_table "error_templates", force: :cascade do |t| @@ -61,6 +67,8 @@ ActiveRecord::Schema.define(version: 20170703080355) do t.string "signature" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.text "description" + t.text "hint" end create_table "errors", force: :cascade do |t| From 4d684a7a05e24911abbab4ca5468b97be96738ba Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 09:52:33 +0200 Subject: [PATCH 03/69] Extract structured errors on run and submit --- app/controllers/concerns/submission_scoring.rb | 8 ++++++++ app/controllers/submissions_controller.rb | 16 +++++++++++++++- app/models/structured_error.rb | 8 ++++++++ app/models/structured_error_attribute.rb | 5 +++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/submission_scoring.rb b/app/controllers/concerns/submission_scoring.rb index 16f1f061..9c577aca 100644 --- a/app/controllers/concerns/submission_scoring.rb +++ b/app/controllers/concerns/submission_scoring.rb @@ -9,6 +9,14 @@ module SubmissionScoring assessment = assessor.assess(output) passed = ((assessment[:passed] == assessment[:count]) and (assessment[:score] > 0)) testrun_output = passed ? nil : output[:stderr] + if !testrun_output.blank? + submission.exercise.execution_environment.error_templates.each do |template| + pattern = Regexp.new(template.signature).freeze + if pattern.match(testrun_output) + StructuredError.create_from_template(template, testrun_output) + end + end + end Testrun.new(submission: submission, file: file, passed: passed, output: testrun_output).save output.merge!(assessment) output.merge!(filename: file.name_with_extension, message: feedback_message(file, output[:score]), weight: file.weight) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 9b37fde0..ae111823 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -6,7 +6,7 @@ class SubmissionsController < ApplicationController include SubmissionScoring include Tubesock::Hijack - before_action :set_submission, only: [:download, :download_file, :render_file, :run, :score, :show, :statistics, :stop, :test] + before_action :set_submission, only: [:download, :download_file, :render_file, :run, :score, :extract_errors, :show, :statistics, :stop, :test] before_action :set_docker_client, only: [:run, :test] before_action :set_files, only: [:download, :download_file, :render_file, :show] before_action :set_file, only: [:download_file, :render_file] @@ -187,6 +187,9 @@ class SubmissionsController < ApplicationController end def kill_socket(tubesock) + # search for errors and save them as StructuredError (for scoring runs see submission_scoring.rb) + extract_errors + # save the output of this "run" as a "testrun" (scoring runs are saved in submission_scoring.rb) save_run_output @@ -195,6 +198,17 @@ class SubmissionsController < ApplicationController tubesock.close end + def extract_errors + if !@message_buffer.blank? + @submission.exercise.execution_environment.error_templates.each do |template| + pattern = Regexp.new(template.signature).freeze + if pattern.match?(@message_buffer) + StructuredError.create_from_template(template, @message_buffer) + end + end + end + end + def handle_message(message, tubesock, container) @message_buffer ||= "" # Handle special commands first diff --git a/app/models/structured_error.rb b/app/models/structured_error.rb index 46f40423..2f03ec54 100644 --- a/app/models/structured_error.rb +++ b/app/models/structured_error.rb @@ -1,4 +1,12 @@ class StructuredError < ActiveRecord::Base belongs_to :error_template belongs_to :file, class_name: 'CodeOcean::File' + + def self.create_from_template(template, message_buffer) + instance = self.create(error_template: template) + template.error_template_attributes.each do |attribute| + StructuredErrorAttribute.create_from_template(attribute, instance, message_buffer) + end + instance + end end diff --git a/app/models/structured_error_attribute.rb b/app/models/structured_error_attribute.rb index 335a5901..b9967719 100644 --- a/app/models/structured_error_attribute.rb +++ b/app/models/structured_error_attribute.rb @@ -1,4 +1,9 @@ class StructuredErrorAttribute < ActiveRecord::Base belongs_to :structured_error belongs_to :error_template_attribute + + def self.create_from_template(attribute, structured_error, message_buffer) + value = message_buffer.match(attribute.regex).captures[0] + self.create(structured_error: structured_error, error_template_attribute: attribute, value: value) + end end From 4d38195c99bbc292a9513d0e58ef5b610dfa0d73 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 10:11:52 +0200 Subject: [PATCH 04/69] Scaffold error_templates UI --- .../stylesheets/error_templates.css.scss | 3 + app/assets/stylesheets/scaffolds.css.scss | 69 +++++++++++++++ app/controllers/error_templates_controller.rb | 86 +++++++++++++++++++ app/helpers/error_templates_helper.rb | 2 + app/policies/error_template_policy.rb | 3 + app/views/error_templates/_form.html.erb | 17 ++++ app/views/error_templates/edit.html.erb | 6 ++ app/views/error_templates/index.html.erb | 25 ++++++ app/views/error_templates/index.json.jbuilder | 4 + app/views/error_templates/new.html.erb | 5 ++ app/views/error_templates/show.html.erb | 4 + app/views/error_templates/show.json.jbuilder | 1 + config/routes.rb | 1 + .../error_templates_controller_test.rb | 49 +++++++++++ 14 files changed, 275 insertions(+) create mode 100644 app/assets/stylesheets/error_templates.css.scss create mode 100644 app/assets/stylesheets/scaffolds.css.scss create mode 100644 app/controllers/error_templates_controller.rb create mode 100644 app/helpers/error_templates_helper.rb create mode 100644 app/policies/error_template_policy.rb create mode 100644 app/views/error_templates/_form.html.erb create mode 100644 app/views/error_templates/edit.html.erb create mode 100644 app/views/error_templates/index.html.erb create mode 100644 app/views/error_templates/index.json.jbuilder create mode 100644 app/views/error_templates/new.html.erb create mode 100644 app/views/error_templates/show.html.erb create mode 100644 app/views/error_templates/show.json.jbuilder create mode 100644 test/controllers/error_templates_controller_test.rb diff --git a/app/assets/stylesheets/error_templates.css.scss b/app/assets/stylesheets/error_templates.css.scss new file mode 100644 index 00000000..bbd8b05e --- /dev/null +++ b/app/assets/stylesheets/error_templates.css.scss @@ -0,0 +1,3 @@ +// Place all the styles related to the error_templates controller here. +// They will automatically be included in application.css. +// You can use Sass (SCSS) here: http://sass-lang.com/ diff --git a/app/assets/stylesheets/scaffolds.css.scss b/app/assets/stylesheets/scaffolds.css.scss new file mode 100644 index 00000000..6ec6a8ff --- /dev/null +++ b/app/assets/stylesheets/scaffolds.css.scss @@ -0,0 +1,69 @@ +body { + background-color: #fff; + color: #333; + font-family: verdana, arial, helvetica, sans-serif; + font-size: 13px; + line-height: 18px; +} + +p, ol, ul, td { + font-family: verdana, arial, helvetica, sans-serif; + font-size: 13px; + line-height: 18px; +} + +pre { + background-color: #eee; + padding: 10px; + font-size: 11px; +} + +a { + color: #000; + &:visited { + color: #666; + } + &:hover { + color: #fff; + background-color: #000; + } +} + +div { + &.field, &.actions { + margin-bottom: 10px; + } +} + +#notice { + color: green; +} + +.field_with_errors { + padding: 2px; + background-color: red; + display: table; +} + +#error_explanation { + width: 450px; + border: 2px solid red; + padding: 7px; + padding-bottom: 0; + margin-bottom: 20px; + background-color: #f0f0f0; + h2 { + text-align: left; + font-weight: bold; + padding: 5px 5px 5px 15px; + font-size: 12px; + margin: -7px; + margin-bottom: 0px; + background-color: #c00; + color: #fff; + } + ul li { + font-size: 12px; + list-style: square; + } +} diff --git a/app/controllers/error_templates_controller.rb b/app/controllers/error_templates_controller.rb new file mode 100644 index 00000000..1007eeaf --- /dev/null +++ b/app/controllers/error_templates_controller.rb @@ -0,0 +1,86 @@ +class ErrorTemplatesController < ApplicationController + before_action :set_error_template, only: [:show, :edit, :update, :destroy] + + def authorize! + authorize(@error_templates || @error_template) + end + private :authorize! + + # GET /error_templates + # GET /error_templates.json + def index + @error_templates = ErrorTemplate.all + authorize! + end + + # GET /error_templates/1 + # GET /error_templates/1.json + def show + authorize! + end + + # GET /error_templates/new + def new + @error_template = ErrorTemplate.new + authorize! + end + + # GET /error_templates/1/edit + def edit + authorize! + end + + # POST /error_templates + # POST /error_templates.json + def create + authorize! + @error_template = ErrorTemplate.new(error_template_params) + + respond_to do |format| + if @error_template.save + format.html { redirect_to @error_template, notice: 'Error template was successfully created.' } + format.json { render :show, status: :created, location: @error_template } + else + format.html { render :new } + format.json { render json: @error_template.errors, status: :unprocessable_entity } + end + end + end + + # PATCH/PUT /error_templates/1 + # PATCH/PUT /error_templates/1.json + def update + authorize! + respond_to do |format| + if @error_template.update(error_template_params) + format.html { redirect_to @error_template, notice: 'Error template was successfully updated.' } + format.json { render :show, status: :ok, location: @error_template } + else + format.html { render :edit } + format.json { render json: @error_template.errors, status: :unprocessable_entity } + end + end + end + + # DELETE /error_templates/1 + # DELETE /error_templates/1.json + def destroy + authorize! + @error_template.destroy + respond_to do |format| + format.html { redirect_to error_templates_url, notice: 'Error template was successfully destroyed.' } + format.json { head :no_content } + end + end + + private + # Use callbacks to share common setup or constraints between actions. + def set_error_template + @error_template = ErrorTemplate.find(params[:id]) + end + + # Never trust parameters from the scary internet, only allow the white list through. + def error_template_params + params.fetch(:error_template, {}) + end +end diff --git a/app/helpers/error_templates_helper.rb b/app/helpers/error_templates_helper.rb new file mode 100644 index 00000000..f42cf302 --- /dev/null +++ b/app/helpers/error_templates_helper.rb @@ -0,0 +1,2 @@ +module ErrorTemplatesHelper +end diff --git a/app/policies/error_template_policy.rb b/app/policies/error_template_policy.rb new file mode 100644 index 00000000..30bdbd24 --- /dev/null +++ b/app/policies/error_template_policy.rb @@ -0,0 +1,3 @@ +class ErrorTemplatePolicy < AdminOnlyPolicy + +end diff --git a/app/views/error_templates/_form.html.erb b/app/views/error_templates/_form.html.erb new file mode 100644 index 00000000..17371914 --- /dev/null +++ b/app/views/error_templates/_form.html.erb @@ -0,0 +1,17 @@ +<%= form_for(@error_template) do |f| %> + <% if @error_template.errors.any? %> +
+

<%= pluralize(@error_template.errors.count, "error") %> prohibited this error_template from being saved:

+ +
    + <% @error_template.errors.full_messages.each do |message| %> +
  • <%= message %>
  • + <% end %> +
+
+ <% end %> + +
+ <%= f.submit %> +
+<% end %> diff --git a/app/views/error_templates/edit.html.erb b/app/views/error_templates/edit.html.erb new file mode 100644 index 00000000..0d2919e5 --- /dev/null +++ b/app/views/error_templates/edit.html.erb @@ -0,0 +1,6 @@ +

Editing Error Template

+ +<%= render 'form' %> + +<%= link_to 'Show', @error_template %> | +<%= link_to 'Back', error_templates_path %> diff --git a/app/views/error_templates/index.html.erb b/app/views/error_templates/index.html.erb new file mode 100644 index 00000000..65829aeb --- /dev/null +++ b/app/views/error_templates/index.html.erb @@ -0,0 +1,25 @@ +

<%= notice %>

+ +

Listing Error Templates

+ + + + + + + + + + <% @error_templates.each do |error_template| %> + + + + + + <% end %> + +
<%= link_to 'Show', error_template %><%= link_to 'Edit', edit_error_template_path(error_template) %><%= link_to 'Destroy', error_template, method: :delete, data: { confirm: 'Are you sure?' } %>
+ +
+ +<%= link_to 'New Error template', new_error_template_path %> diff --git a/app/views/error_templates/index.json.jbuilder b/app/views/error_templates/index.json.jbuilder new file mode 100644 index 00000000..ce85b2be --- /dev/null +++ b/app/views/error_templates/index.json.jbuilder @@ -0,0 +1,4 @@ +json.array!(@error_templates) do |error_template| + json.extract! error_template, :id + json.url error_template_url(error_template, format: :json) +end diff --git a/app/views/error_templates/new.html.erb b/app/views/error_templates/new.html.erb new file mode 100644 index 00000000..6b4acd06 --- /dev/null +++ b/app/views/error_templates/new.html.erb @@ -0,0 +1,5 @@ +

New Error Template

+ +<%= render 'form' %> + +<%= link_to 'Back', error_templates_path %> diff --git a/app/views/error_templates/show.html.erb b/app/views/error_templates/show.html.erb new file mode 100644 index 00000000..9debad9b --- /dev/null +++ b/app/views/error_templates/show.html.erb @@ -0,0 +1,4 @@ +

<%= notice %>

+ +<%= link_to 'Edit', edit_error_template_path(@error_template) %> | +<%= link_to 'Back', error_templates_path %> diff --git a/app/views/error_templates/show.json.jbuilder b/app/views/error_templates/show.json.jbuilder new file mode 100644 index 00000000..1cbaff55 --- /dev/null +++ b/app/views/error_templates/show.json.jbuilder @@ -0,0 +1 @@ +json.extract! @error_template, :id, :created_at, :updated_at diff --git a/config/routes.rb b/config/routes.rb index d340a204..ce5be3ae 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,7 @@ FILENAME_REGEXP = /[\w\.]+/ unless Kernel.const_defined?(:FILENAME_REGEXP) Rails.application.routes.draw do + resources :error_templates resources :file_templates do collection do get 'by_file_type/:file_type_id', as: :by_file_type, action: :by_file_type diff --git a/test/controllers/error_templates_controller_test.rb b/test/controllers/error_templates_controller_test.rb new file mode 100644 index 00000000..452ad134 --- /dev/null +++ b/test/controllers/error_templates_controller_test.rb @@ -0,0 +1,49 @@ +require 'test_helper' + +class ErrorTemplatesControllerTest < ActionController::TestCase + setup do + @error_template = error_templates(:one) + end + + test "should get index" do + get :index + assert_response :success + assert_not_nil assigns(:error_templates) + end + + test "should get new" do + get :new + assert_response :success + end + + test "should create error_template" do + assert_difference('ErrorTemplate.count') do + post :create, error_template: { } + end + + assert_redirected_to error_template_path(assigns(:error_template)) + end + + test "should show error_template" do + get :show, id: @error_template + assert_response :success + end + + test "should get edit" do + get :edit, id: @error_template + assert_response :success + end + + test "should update error_template" do + patch :update, id: @error_template, error_template: { } + assert_redirected_to error_template_path(assigns(:error_template)) + end + + test "should destroy error_template" do + assert_difference('ErrorTemplate.count', -1) do + delete :destroy, id: @error_template + end + + assert_redirected_to error_templates_path + end +end From 9eadc3a4db3b68175f207c1dc86301c5e3ab8205 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 11:04:29 +0200 Subject: [PATCH 05/69] Make error template UI usable --- .../stylesheets/error_templates.css.scss | 3 --- app/controllers/error_templates_controller.rb | 6 ++--- app/views/error_templates/_form.html.erb | 17 ------------- app/views/error_templates/_form.html.slim | 19 ++++++++++++++ app/views/error_templates/edit.html.erb | 6 ----- app/views/error_templates/edit.html.slim | 3 +++ app/views/error_templates/index.html.erb | 25 ------------------- app/views/error_templates/index.html.slim | 22 ++++++++++++++++ app/views/error_templates/index.json.jbuilder | 4 --- app/views/error_templates/new.html.erb | 5 ---- app/views/error_templates/new.html.slim | 3 +++ app/views/error_templates/show.html.erb | 4 --- app/views/error_templates/show.html.slim | 10 ++++++++ app/views/error_templates/show.json.jbuilder | 1 - config/locales/de.yml | 9 ++++++- config/locales/en.yml | 8 ++++++ 16 files changed, 76 insertions(+), 69 deletions(-) delete mode 100644 app/assets/stylesheets/error_templates.css.scss delete mode 100644 app/views/error_templates/_form.html.erb create mode 100644 app/views/error_templates/_form.html.slim delete mode 100644 app/views/error_templates/edit.html.erb create mode 100644 app/views/error_templates/edit.html.slim delete mode 100644 app/views/error_templates/index.html.erb create mode 100644 app/views/error_templates/index.html.slim delete mode 100644 app/views/error_templates/index.json.jbuilder delete mode 100644 app/views/error_templates/new.html.erb create mode 100644 app/views/error_templates/new.html.slim delete mode 100644 app/views/error_templates/show.html.erb create mode 100644 app/views/error_templates/show.html.slim delete mode 100644 app/views/error_templates/show.json.jbuilder diff --git a/app/assets/stylesheets/error_templates.css.scss b/app/assets/stylesheets/error_templates.css.scss deleted file mode 100644 index bbd8b05e..00000000 --- a/app/assets/stylesheets/error_templates.css.scss +++ /dev/null @@ -1,3 +0,0 @@ -// Place all the styles related to the error_templates controller here. -// They will automatically be included in application.css. -// You can use Sass (SCSS) here: http://sass-lang.com/ diff --git a/app/controllers/error_templates_controller.rb b/app/controllers/error_templates_controller.rb index 1007eeaf..6bf9a4f2 100644 --- a/app/controllers/error_templates_controller.rb +++ b/app/controllers/error_templates_controller.rb @@ -9,7 +9,7 @@ class ErrorTemplatesController < ApplicationController # GET /error_templates # GET /error_templates.json def index - @error_templates = ErrorTemplate.all + @error_templates = ErrorTemplate.all.order(:execution_environment_id, :name).paginate(page: params[:page]) authorize! end @@ -33,8 +33,8 @@ class ErrorTemplatesController < ApplicationController # POST /error_templates # POST /error_templates.json def create - authorize! @error_template = ErrorTemplate.new(error_template_params) + authorize! respond_to do |format| if @error_template.save @@ -81,6 +81,6 @@ class ErrorTemplatesController < ApplicationController # Never trust parameters from the scary internet, only allow the white list through. def error_template_params - params.fetch(:error_template, {}) + params[:error_template].permit(:name, :execution_environment_id, :signature, :description, :hint) end end diff --git a/app/views/error_templates/_form.html.erb b/app/views/error_templates/_form.html.erb deleted file mode 100644 index 17371914..00000000 --- a/app/views/error_templates/_form.html.erb +++ /dev/null @@ -1,17 +0,0 @@ -<%= form_for(@error_template) do |f| %> - <% if @error_template.errors.any? %> -
-

<%= pluralize(@error_template.errors.count, "error") %> prohibited this error_template from being saved:

- -
    - <% @error_template.errors.full_messages.each do |message| %> -
  • <%= message %>
  • - <% end %> -
-
- <% end %> - -
- <%= f.submit %> -
-<% end %> diff --git a/app/views/error_templates/_form.html.slim b/app/views/error_templates/_form.html.slim new file mode 100644 index 00000000..f9363155 --- /dev/null +++ b/app/views/error_templates/_form.html.slim @@ -0,0 +1,19 @@ += form_for(@error_template) do |f| + = render('shared/form_errors', object: @error_template) + .form-group + = f.label(:name) + = f.text_field(:name, class: 'form-control', required: true) + .form-group + = f.label(:execution_environment_id) + = f.collection_select(:execution_environment_id, ExecutionEnvironment.all.order(:name), :id, :name, {include_blank: false}, class: 'form-control') + .form-group + = f.label(:signature) + = f.text_field(:signature, class: 'form-control') + .help-block == t('error_templates.hints.signature') + .form-group + = f.label(:description) + = f.text_field(:description, class: 'form-control') + .form-group + = f.label(:hint) + = f.text_field(:hint, class: 'form-control') + .actions = render('shared/submit_button', f: f, object: @error_template) diff --git a/app/views/error_templates/edit.html.erb b/app/views/error_templates/edit.html.erb deleted file mode 100644 index 0d2919e5..00000000 --- a/app/views/error_templates/edit.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -

Editing Error Template

- -<%= render 'form' %> - -<%= link_to 'Show', @error_template %> | -<%= link_to 'Back', error_templates_path %> diff --git a/app/views/error_templates/edit.html.slim b/app/views/error_templates/edit.html.slim new file mode 100644 index 00000000..2d12b363 --- /dev/null +++ b/app/views/error_templates/edit.html.slim @@ -0,0 +1,3 @@ +h1 = @error_template + += render('form') diff --git a/app/views/error_templates/index.html.erb b/app/views/error_templates/index.html.erb deleted file mode 100644 index 65829aeb..00000000 --- a/app/views/error_templates/index.html.erb +++ /dev/null @@ -1,25 +0,0 @@ -

<%= notice %>

- -

Listing Error Templates

- - - - - - - - - - <% @error_templates.each do |error_template| %> - - - - - - <% end %> - -
<%= link_to 'Show', error_template %><%= link_to 'Edit', edit_error_template_path(error_template) %><%= link_to 'Destroy', error_template, method: :delete, data: { confirm: 'Are you sure?' } %>
- -
- -<%= link_to 'New Error template', new_error_template_path %> diff --git a/app/views/error_templates/index.html.slim b/app/views/error_templates/index.html.slim new file mode 100644 index 00000000..1b028614 --- /dev/null +++ b/app/views/error_templates/index.html.slim @@ -0,0 +1,22 @@ +h1 = ErrorTemplate.model_name.human(count: 2) + +.table-responsive + table.sortable.table + thead + tr + th = t('activerecord.attributes.error_template.name') + th = t('activerecord.attributes.error_template.description') + th = t('activerecord.attributes.exercise.execution_environment') + th colspan=5 = t('shared.actions') + tbody + - @error_templates.each do |error_template| + tr + td = error_template.name + td = error_template.description + td = link_to(error_template.execution_environment) + td = link_to(t('shared.show'), error_template) + td = link_to(t('shared.edit'), edit_error_template_path(error_template)) + td = link_to(t('shared.destroy'), error_template, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + += render('shared/pagination', collection: @error_templates) +p = render('shared/new_button', model: ErrorTemplate) diff --git a/app/views/error_templates/index.json.jbuilder b/app/views/error_templates/index.json.jbuilder deleted file mode 100644 index ce85b2be..00000000 --- a/app/views/error_templates/index.json.jbuilder +++ /dev/null @@ -1,4 +0,0 @@ -json.array!(@error_templates) do |error_template| - json.extract! error_template, :id - json.url error_template_url(error_template, format: :json) -end diff --git a/app/views/error_templates/new.html.erb b/app/views/error_templates/new.html.erb deleted file mode 100644 index 6b4acd06..00000000 --- a/app/views/error_templates/new.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -

New Error Template

- -<%= render 'form' %> - -<%= link_to 'Back', error_templates_path %> diff --git a/app/views/error_templates/new.html.slim b/app/views/error_templates/new.html.slim new file mode 100644 index 00000000..eeecadc8 --- /dev/null +++ b/app/views/error_templates/new.html.slim @@ -0,0 +1,3 @@ +h1 = t('shared.new_model', model: ErrorTemplate.model_name.human) + += render('form') diff --git a/app/views/error_templates/show.html.erb b/app/views/error_templates/show.html.erb deleted file mode 100644 index 9debad9b..00000000 --- a/app/views/error_templates/show.html.erb +++ /dev/null @@ -1,4 +0,0 @@ -

<%= notice %>

- -<%= link_to 'Edit', edit_error_template_path(@error_template) %> | -<%= link_to 'Back', error_templates_path %> diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim new file mode 100644 index 00000000..21843b14 --- /dev/null +++ b/app/views/error_templates/show.html.slim @@ -0,0 +1,10 @@ +h1 + = @error_template + = render('shared/edit_button', object: @error_template) + += row(label: 'error_template.name', value: @error_template.name) += row(label: 'exercise.execution_environment', value: link_to(@error_template.execution_environment)) +- [:signature, :description, :hint].each do |attribute| + = row(label: "error_template.#{attribute}", value: @error_template.send(attribute)) + +//todo: list attributes \ No newline at end of file diff --git a/app/views/error_templates/show.json.jbuilder b/app/views/error_templates/show.json.jbuilder deleted file mode 100644 index 1cbaff55..00000000 --- a/app/views/error_templates/show.json.jbuilder +++ /dev/null @@ -1 +0,0 @@ -json.extract! @error_template, :id, :created_at, :updated_at diff --git a/config/locales/de.yml b/config/locales/de.yml index d0aef588..d37992c9 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -109,6 +109,11 @@ de: name: "Name" file_type: "Dateityp" content: "Code" + error_template: + name: Name + signature: Regulärer Ausdruck + description: Beschreibung + hint: Hinweis models: code_harbor_link: one: CodeHarbor-Link @@ -580,4 +585,6 @@ de: estimated_time_20_to_30: "zwischen 20 und 30 Minuten" estimated_time_more_30: "mehr als 30 Minuten" working_time: "Geschätze Bearbeitungszeit für diese Aufgabe:" - + error_templates: + hints: + signature: "Ein regulärer Ausdruck in Ruby-Syntax und ohne führende und schließende \"/\"" diff --git a/config/locales/en.yml b/config/locales/en.yml index 9a334697..17427a5f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -130,6 +130,11 @@ en: name: "Name" file_type: "File Type" content: "Content" + error_template: + name: Name + signature: Signature Regular Expression + description: Description + hint: Hint models: code_harbor_link: one: CodeHarbor Link @@ -601,3 +606,6 @@ en: estimated_time_20_to_30: "between 20 and 30 minutes" estimated_time_more_30: "more than 30 minutes" working_time: "Estimated time working on this exercise:" + error_templates: + hints: + signature: "A regular expression in Ruby syntax without leading and trailing \"/\"" From fdee3c3efc9629a80f590a7a6f63f721c230e34f Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 11:11:32 +0200 Subject: [PATCH 06/69] Fix string in heading and breadcrumbs --- app/models/error_template.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/error_template.rb b/app/models/error_template.rb index dc497d48..be4a3279 100644 --- a/app/models/error_template.rb +++ b/app/models/error_template.rb @@ -1,4 +1,8 @@ class ErrorTemplate < ActiveRecord::Base belongs_to :execution_environment has_and_belongs_to_many :error_template_attributes + + def to_s + "#{id} [#{name}]" + end end From 2f759c03a9dd2b466fc37b0cd0880593f5e65e3a Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 11:23:51 +0200 Subject: [PATCH 07/69] List error template attributes --- app/views/error_templates/show.html.slim | 23 ++++++++++++++++++++++- config/locales/de.yml | 6 ++++++ config/locales/en.yml | 6 ++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index 21843b14..373aa615 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -7,4 +7,25 @@ h1 - [:signature, :description, :hint].each do |attribute| = row(label: "error_template.#{attribute}", value: @error_template.send(attribute)) -//todo: list attributes \ No newline at end of file +h3 + = t 'error_templates.attributes' + +.table-responsive + table.sortable.table + thead + tr + th + th = t('activerecord.attributes.error_template_attributes.key') + th = t('activerecord.attributes.error_template_attributes.description') + th = t('activerecord.attributes.error_template_attributes.regex') + tbody + - @error_template.error_template_attributes.order(:important).each do |attribute| + tr + td + - if attribute.important + span class="fa fa-star" aria-hidden="true" + - else + span class="fa fa-star-o" aria-hidden="true" + td = attribute.key + td = attribute.description + td = attribute.regex diff --git a/config/locales/de.yml b/config/locales/de.yml index d37992c9..ab4f835d 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -114,6 +114,11 @@ de: signature: Regulärer Ausdruck description: Beschreibung hint: Hinweis + error_template_attributes: + important: "Wichtig" + key: "Name" + description: "Beschreibung" + regex: "Regulärer Ausdruck" models: code_harbor_link: one: CodeHarbor-Link @@ -588,3 +593,4 @@ de: error_templates: hints: signature: "Ein regulärer Ausdruck in Ruby-Syntax und ohne führende und schließende \"/\"" + attributes: "Attribute" diff --git a/config/locales/en.yml b/config/locales/en.yml index 17427a5f..cd61a066 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -135,6 +135,11 @@ en: signature: Signature Regular Expression description: Description hint: Hint + error_template_attributes: + important: "Important" + key: "Identifier" + description: "Description" + regex: "Regular Expression" models: code_harbor_link: one: CodeHarbor Link @@ -609,3 +614,4 @@ en: error_templates: hints: signature: "A regular expression in Ruby syntax without leading and trailing \"/\"" + attributes: "Attributes" From 4b87d960baaf359b8d8806b8eca32b9c2e5578d8 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 11:47:47 +0200 Subject: [PATCH 08/69] Remove scaffold.css --- app/assets/stylesheets/scaffolds.css.scss | 69 ----------------------- 1 file changed, 69 deletions(-) delete mode 100644 app/assets/stylesheets/scaffolds.css.scss diff --git a/app/assets/stylesheets/scaffolds.css.scss b/app/assets/stylesheets/scaffolds.css.scss deleted file mode 100644 index 6ec6a8ff..00000000 --- a/app/assets/stylesheets/scaffolds.css.scss +++ /dev/null @@ -1,69 +0,0 @@ -body { - background-color: #fff; - color: #333; - font-family: verdana, arial, helvetica, sans-serif; - font-size: 13px; - line-height: 18px; -} - -p, ol, ul, td { - font-family: verdana, arial, helvetica, sans-serif; - font-size: 13px; - line-height: 18px; -} - -pre { - background-color: #eee; - padding: 10px; - font-size: 11px; -} - -a { - color: #000; - &:visited { - color: #666; - } - &:hover { - color: #fff; - background-color: #000; - } -} - -div { - &.field, &.actions { - margin-bottom: 10px; - } -} - -#notice { - color: green; -} - -.field_with_errors { - padding: 2px; - background-color: red; - display: table; -} - -#error_explanation { - width: 450px; - border: 2px solid red; - padding: 7px; - padding-bottom: 0; - margin-bottom: 20px; - background-color: #f0f0f0; - h2 { - text-align: left; - font-weight: bold; - padding: 5px 5px 5px 15px; - font-size: 12px; - margin: -7px; - margin-bottom: 0px; - background-color: #c00; - color: #fff; - } - ul li { - font-size: 12px; - list-style: square; - } -} From 0c8c8562f57b410457cac1449e8cca2516bccc0b Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 11:58:18 +0200 Subject: [PATCH 09/69] Scaffold error template attributes --- .../error_template_attributes_controller.rb | 86 +++++++++++++++++++ .../error_template_attributes_helper.rb | 2 + app/models/error_template_attribute.rb | 4 + .../error_template_attribute_policy.rb | 3 + .../error_template_attributes/_form.html.slim | 16 ++++ .../error_template_attributes/edit.html.slim | 3 + .../error_template_attributes/index.html.slim | 28 ++++++ .../error_template_attributes/new.html.slim | 3 + .../error_template_attributes/show.html.slim | 8 ++ app/views/error_templates/show.html.slim | 6 +- config/locales/de.yml | 2 +- config/locales/en.yml | 2 +- config/routes.rb | 1 + ...ror_template_attributes_controller_test.rb | 49 +++++++++++ 14 files changed, 208 insertions(+), 5 deletions(-) create mode 100644 app/controllers/error_template_attributes_controller.rb create mode 100644 app/helpers/error_template_attributes_helper.rb create mode 100644 app/policies/error_template_attribute_policy.rb create mode 100644 app/views/error_template_attributes/_form.html.slim create mode 100644 app/views/error_template_attributes/edit.html.slim create mode 100644 app/views/error_template_attributes/index.html.slim create mode 100644 app/views/error_template_attributes/new.html.slim create mode 100644 app/views/error_template_attributes/show.html.slim create mode 100644 test/controllers/error_template_attributes_controller_test.rb diff --git a/app/controllers/error_template_attributes_controller.rb b/app/controllers/error_template_attributes_controller.rb new file mode 100644 index 00000000..dc2da30e --- /dev/null +++ b/app/controllers/error_template_attributes_controller.rb @@ -0,0 +1,86 @@ +class ErrorTemplateAttributesController < ApplicationController + before_action :set_error_template_attribute, only: [:show, :edit, :update, :destroy] + + def authorize! + authorize(@error_template_attributes || @error_template_attribute) + end + private :authorize! + + # GET /error_template_attributes + # GET /error_template_attributes.json + def index + @error_template_attributes = ErrorTemplateAttribute.all.order(:id).paginate(page: params[:page]) + authorize! + end + + # GET /error_template_attributes/1 + # GET /error_template_attributes/1.json + def show + authorize! + end + + # GET /error_template_attributes/new + def new + @error_template_attribute = ErrorTemplateAttribute.new + authorize! + end + + # GET /error_template_attributes/1/edit + def edit + authorize! + end + + # POST /error_template_attributes + # POST /error_template_attributes.json + def create + @error_template_attribute = ErrorTemplateAttribute.new(error_template_attribute_params) + authorize! + + respond_to do |format| + if @error_template_attribute.save + format.html { redirect_to @error_template_attribute, notice: 'Error template attribute was successfully created.' } + format.json { render :show, status: :created, location: @error_template_attribute } + else + format.html { render :new } + format.json { render json: @error_template_attribute.errors, status: :unprocessable_entity } + end + end + end + + # PATCH/PUT /error_template_attributes/1 + # PATCH/PUT /error_template_attributes/1.json + def update + authorize! + respond_to do |format| + if @error_template_attribute.update(error_template_attribute_params) + format.html { redirect_to @error_template_attribute, notice: 'Error template attribute was successfully updated.' } + format.json { render :show, status: :ok, location: @error_template_attribute } + else + format.html { render :edit } + format.json { render json: @error_template_attribute.errors, status: :unprocessable_entity } + end + end + end + + # DELETE /error_template_attributes/1 + # DELETE /error_template_attributes/1.json + def destroy + authorize! + @error_template_attribute.destroy + respond_to do |format| + format.html { redirect_to error_template_attributes_url, notice: 'Error template attribute was successfully destroyed.' } + format.json { head :no_content } + end + end + + private + # Use callbacks to share common setup or constraints between actions. + def set_error_template_attribute + @error_template_attribute = ErrorTemplateAttribute.find(params[:id]) + end + + # Never trust parameters from the scary internet, only allow the white list through. + def error_template_attribute_params + params.fetch(:error_template_attribute, {}) + end +end diff --git a/app/helpers/error_template_attributes_helper.rb b/app/helpers/error_template_attributes_helper.rb new file mode 100644 index 00000000..9046f407 --- /dev/null +++ b/app/helpers/error_template_attributes_helper.rb @@ -0,0 +1,2 @@ +module ErrorTemplateAttributesHelper +end diff --git a/app/models/error_template_attribute.rb b/app/models/error_template_attribute.rb index 15229b33..844c1019 100644 --- a/app/models/error_template_attribute.rb +++ b/app/models/error_template_attribute.rb @@ -1,3 +1,7 @@ class ErrorTemplateAttribute < ActiveRecord::Base has_and_belongs_to_many :error_template + + def to_s + "#{id} [#{key}]" + end end diff --git a/app/policies/error_template_attribute_policy.rb b/app/policies/error_template_attribute_policy.rb new file mode 100644 index 00000000..eed0896d --- /dev/null +++ b/app/policies/error_template_attribute_policy.rb @@ -0,0 +1,3 @@ +class ErrorTemplateAttributePolicy < AdminOnlyPolicy + +end diff --git a/app/views/error_template_attributes/_form.html.slim b/app/views/error_template_attributes/_form.html.slim new file mode 100644 index 00000000..4fc28b02 --- /dev/null +++ b/app/views/error_template_attributes/_form.html.slim @@ -0,0 +1,16 @@ += form_for(@error_template_attribute) do |f| + = render('shared/form_errors', object: @error_template_attribute) + .form-group + = f.label(:key) + = f.text_field(:key, class: 'form-control', required: true) + .form-group + = f.label(:description) + = f.text_field(:description, class: 'form-control') + .form-group + = f.label(:regex) + = f.text_field(:regex, class: 'form-control', required: true) + .help-block == t('error_templates.hints.signature') + .form-group + = f.check_box(:important) + = t('activerecord.attributes.error_template_attribute.important') + .actions = render('shared/submit_button', f: f, object: @error_template_attribute) diff --git a/app/views/error_template_attributes/edit.html.slim b/app/views/error_template_attributes/edit.html.slim new file mode 100644 index 00000000..f3279442 --- /dev/null +++ b/app/views/error_template_attributes/edit.html.slim @@ -0,0 +1,3 @@ +h1 = @error_template_attribute + += render('form') diff --git a/app/views/error_template_attributes/index.html.slim b/app/views/error_template_attributes/index.html.slim new file mode 100644 index 00000000..81d5cac9 --- /dev/null +++ b/app/views/error_template_attributes/index.html.slim @@ -0,0 +1,28 @@ +h1 = ErrorTemplateAttribute.model_name.human(count: 2) + +.table-responsive + table.sortable.table + thead + tr + th + th = t('activerecord.attributes.error_template_attribute.key') + th = t('activerecord.attributes.error_template_attribute.description') + th = t('activerecord.attributes.error_template_attribute.regex') + th colspan=5 = t('shared.actions') + tbody + - @error_template_attributes.each do |error_template_attribute| + tr + td + - if error_template_attribute.important + span class="fa fa-star" aria-hidden="true" + - else + span class="fa fa-star-o" aria-hidden="true" + td = error_template_attribute.key + td = error_template_attribute.description + td = error_template_attribute.regex + td = link_to(t('shared.show'), error_template_attribute) + td = link_to(t('shared.edit'), edit_error_template_attribute_path(error_template_attribute)) + td = link_to(t('shared.destroy'), error_template_attribute, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + += render('shared/pagination', collection: @error_template_attributes) +p = render('shared/new_button', model: ErrorTemplateAttribute) diff --git a/app/views/error_template_attributes/new.html.slim b/app/views/error_template_attributes/new.html.slim new file mode 100644 index 00000000..12c719f9 --- /dev/null +++ b/app/views/error_template_attributes/new.html.slim @@ -0,0 +1,3 @@ +h1 = t('shared.new_model', model: ErrorTemplateAttribute.model_name.human) + += render('form') diff --git a/app/views/error_template_attributes/show.html.slim b/app/views/error_template_attributes/show.html.slim new file mode 100644 index 00000000..2bdd01ca --- /dev/null +++ b/app/views/error_template_attributes/show.html.slim @@ -0,0 +1,8 @@ +h1 + = @error_template_attribute + = render('shared/edit_button', object: @error_template_attribute) + +- [:key, :description, :regex, :important].each do |attribute| + = row(label: "error_template_attribute.#{attribute}", value: @error_template_attribute.send(attribute)) + +// todo: used by diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index 373aa615..885c3788 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -15,9 +15,9 @@ h3 thead tr th - th = t('activerecord.attributes.error_template_attributes.key') - th = t('activerecord.attributes.error_template_attributes.description') - th = t('activerecord.attributes.error_template_attributes.regex') + th = t('activerecord.attributes.error_template_attribute.key') + th = t('activerecord.attributes.error_template_attribute.description') + th = t('activerecord.attributes.error_template_attribute.regex') tbody - @error_template.error_template_attributes.order(:important).each do |attribute| tr diff --git a/config/locales/de.yml b/config/locales/de.yml index ab4f835d..6aca7d64 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -114,7 +114,7 @@ de: signature: Regulärer Ausdruck description: Beschreibung hint: Hinweis - error_template_attributes: + error_template_attribute: important: "Wichtig" key: "Name" description: "Beschreibung" diff --git a/config/locales/en.yml b/config/locales/en.yml index cd61a066..34d8894f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -135,7 +135,7 @@ en: signature: Signature Regular Expression description: Description hint: Hint - error_template_attributes: + error_template_attribute: important: "Important" key: "Identifier" description: "Description" diff --git a/config/routes.rb b/config/routes.rb index ce5be3ae..c614a50c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,7 @@ FILENAME_REGEXP = /[\w\.]+/ unless Kernel.const_defined?(:FILENAME_REGEXP) Rails.application.routes.draw do + resources :error_template_attributes resources :error_templates resources :file_templates do collection do diff --git a/test/controllers/error_template_attributes_controller_test.rb b/test/controllers/error_template_attributes_controller_test.rb new file mode 100644 index 00000000..7dd008ec --- /dev/null +++ b/test/controllers/error_template_attributes_controller_test.rb @@ -0,0 +1,49 @@ +require 'test_helper' + +class ErrorTemplateAttributesControllerTest < ActionController::TestCase + setup do + @error_template_attribute = error_template_attributes(:one) + end + + test "should get index" do + get :index + assert_response :success + assert_not_nil assigns(:error_template_attributes) + end + + test "should get new" do + get :new + assert_response :success + end + + test "should create error_template_attribute" do + assert_difference('ErrorTemplateAttribute.count') do + post :create, error_template_attribute: { } + end + + assert_redirected_to error_template_attribute_path(assigns(:error_template_attribute)) + end + + test "should show error_template_attribute" do + get :show, id: @error_template_attribute + assert_response :success + end + + test "should get edit" do + get :edit, id: @error_template_attribute + assert_response :success + end + + test "should update error_template_attribute" do + patch :update, id: @error_template_attribute, error_template_attribute: { } + assert_redirected_to error_template_attribute_path(assigns(:error_template_attribute)) + end + + test "should destroy error_template_attribute" do + assert_difference('ErrorTemplateAttribute.count', -1) do + delete :destroy, id: @error_template_attribute + end + + assert_redirected_to error_template_attributes_path + end +end From c0160ae4515e3314e442746b0c5087b59311eea8 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 12:06:19 +0200 Subject: [PATCH 10/69] Make table more concise --- app/views/error_templates/index.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/error_templates/index.html.slim b/app/views/error_templates/index.html.slim index 1b028614..f44b3f67 100644 --- a/app/views/error_templates/index.html.slim +++ b/app/views/error_templates/index.html.slim @@ -7,7 +7,7 @@ h1 = ErrorTemplate.model_name.human(count: 2) th = t('activerecord.attributes.error_template.name') th = t('activerecord.attributes.error_template.description') th = t('activerecord.attributes.exercise.execution_environment') - th colspan=5 = t('shared.actions') + th colspan=3 = t('shared.actions') tbody - @error_templates.each do |error_template| tr From 2cc1a7478ccb78244a597086b7344cd31a39212e Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 12:06:40 +0200 Subject: [PATCH 11/69] Add actions to attribute list --- app/views/error_templates/show.html.slim | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index 885c3788..cc262171 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -18,6 +18,7 @@ h3 th = t('activerecord.attributes.error_template_attribute.key') th = t('activerecord.attributes.error_template_attribute.description') th = t('activerecord.attributes.error_template_attribute.regex') + th colspan=3 = t('shared.actions') tbody - @error_template.error_template_attributes.order(:important).each do |attribute| tr @@ -29,3 +30,5 @@ h3 td = attribute.key td = attribute.description td = attribute.regex + td = link_to(t('shared.show'), attribute) + td = "Remove" From 24fd142d3c723cc4f4ef46012a02c00bebd1c127 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 15:15:19 +0200 Subject: [PATCH 12/69] Add localization and plural forms for error templates and error template attributes --- config/locales/de.yml | 6 ++++++ config/locales/en.yml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/config/locales/de.yml b/config/locales/de.yml index 6aca7d64..edb5610f 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -129,6 +129,12 @@ de: error: one: Fehler other: Fehler + error_template: + one: Fehlertemplate + other: Fehlertemplates + error_template_attribute: + one: Fehlertemplatettribut + other: Fehlertemplatettribute execution_environment: one: Ausführungsumgebung other: Ausführungsumgebungen diff --git a/config/locales/en.yml b/config/locales/en.yml index 34d8894f..7cfe4c48 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -150,6 +150,12 @@ en: error: one: Error other: Errors + error_template: + one: Error Template + other: Error Templates + error_template_attribute: + one: Error Template Attribute + other: Error Template Attributes execution_environment: one: Execution Environment other: Execution Environments From 28605fbe9b0f7c9f6772213cb5fb6492af0ebf3b Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 21:25:18 +0200 Subject: [PATCH 13/69] Support adding and removing attributes of error templates --- app/assets/javascripts/error_templates.js | 18 +++++++++++++++++ app/assets/stylesheets/error_templates.scss | 9 +++++++++ app/controllers/error_templates_controller.rb | 20 ++++++++++++++++++- app/policies/error_template_policy.rb | 6 ++++++ app/views/error_templates/show.html.slim | 8 +++++++- config/locales/de.yml | 1 + config/locales/en.yml | 1 + config/routes.rb | 7 ++++++- 8 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 app/assets/javascripts/error_templates.js create mode 100644 app/assets/stylesheets/error_templates.scss diff --git a/app/assets/javascripts/error_templates.js b/app/assets/javascripts/error_templates.js new file mode 100644 index 00000000..1b9b5234 --- /dev/null +++ b/app/assets/javascripts/error_templates.js @@ -0,0 +1,18 @@ +$(function() { + if ($.isController('error_templates')) { + $('#add-attribute').find('button').on('click', function () { + $.ajax(location + '/attribute.json', { + method: 'POST', + data: { + _method: 'PUT', + dataType: 'json', + error_template_attribute_id: $('#add-attribute').find('select').val() + } + }).success(function () { + location.reload(); + }).error(function (error) { + $.flash.danger({text: error.statusText}); + }); + }); + } +}); \ No newline at end of file diff --git a/app/assets/stylesheets/error_templates.scss b/app/assets/stylesheets/error_templates.scss new file mode 100644 index 00000000..29b2f96c --- /dev/null +++ b/app/assets/stylesheets/error_templates.scss @@ -0,0 +1,9 @@ +#add-attribute { + display: flex; + max-width: 400px; + margin-top: 30px; + + button { + margin-left: 10px; + } +} \ No newline at end of file diff --git a/app/controllers/error_templates_controller.rb b/app/controllers/error_templates_controller.rb index 6bf9a4f2..2632abf9 100644 --- a/app/controllers/error_templates_controller.rb +++ b/app/controllers/error_templates_controller.rb @@ -1,5 +1,5 @@ class ErrorTemplatesController < ApplicationController - before_action :set_error_template, only: [:show, :edit, :update, :destroy] + before_action :set_error_template, only: [:show, :edit, :update, :destroy, :add_attribute, :remove_attribute] def authorize! authorize(@error_templates || @error_template) @@ -73,6 +73,24 @@ class ErrorTemplatesController < ApplicationController end end + def add_attribute + authorize! + @error_template.error_template_attributes << ErrorTemplateAttribute.find(params['error_template_attribute_id']) + respond_to do |format| + format.html { redirect_to @error_template } + format.json { head :no_content } + end + end + + def remove_attribute + authorize! + @error_template.error_template_attributes.delete(ErrorTemplateAttribute.find(params['error_template_attribute_id'])) + respond_to do |format| + format.html { redirect_to @error_template } + format.json { head :no_content } + end + end + private # Use callbacks to share common setup or constraints between actions. def set_error_template diff --git a/app/policies/error_template_policy.rb b/app/policies/error_template_policy.rb index 30bdbd24..908be08e 100644 --- a/app/policies/error_template_policy.rb +++ b/app/policies/error_template_policy.rb @@ -1,3 +1,9 @@ class ErrorTemplatePolicy < AdminOnlyPolicy + def add_attribute? + admin? + end + def remove_attribute? + admin? + end end diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index cc262171..5b513e6c 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -31,4 +31,10 @@ h3 td = attribute.description td = attribute.regex td = link_to(t('shared.show'), attribute) - td = "Remove" + td = link_to(t('shared.destroy'), attribute_error_template_url(:error_template_attribute_id => attribute.id), :method => :delete) + +#add-attribute + = collection_select({}, :error_template_attribute_id, + ErrorTemplateAttribute.where.not(id: @error_template.error_template_attributes.select(:id).to_a).order(:important, :key), + :id, :key, {include_blank: false}, class: '') + button.btn.btn-default = t('error_templates.add_attribute') diff --git a/config/locales/de.yml b/config/locales/de.yml index edb5610f..c2414a16 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -600,3 +600,4 @@ de: hints: signature: "Ein regulärer Ausdruck in Ruby-Syntax und ohne führende und schließende \"/\"" attributes: "Attribute" + add_attribute: "Attribut hinzufügen" diff --git a/config/locales/en.yml b/config/locales/en.yml index 7cfe4c48..d76774d2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -621,3 +621,4 @@ en: hints: signature: "A regular expression in Ruby syntax without leading and trailing \"/\"" attributes: "Attributes" + add_attribute: "Add attribute" diff --git a/config/routes.rb b/config/routes.rb index c614a50c..24a9671d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,7 +2,12 @@ FILENAME_REGEXP = /[\w\.]+/ unless Kernel.const_defined?(:FILENAME_REGEXP) Rails.application.routes.draw do resources :error_template_attributes - resources :error_templates + resources :error_templates do + member do + put 'attribute', to: 'error_templates#add_attribute' + delete 'attribute', to: 'error_templates#remove_attribute' + end + end resources :file_templates do collection do get 'by_file_type/:file_type_id', as: :by_file_type, action: :by_file_type From 45614c5a8cf4631a18d5aaa02e52c877c04b4bbf Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 21:37:43 +0200 Subject: [PATCH 14/69] Add error template views to navigation --- app/views/application/_navigation.html.slim | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index b9663d3f..7b9e0a33 100644 --- a/app/views/application/_navigation.html.slim +++ b/app/views/application/_navigation.html.slim @@ -8,7 +8,8 @@ - if current_user.admin? li = link_to(t('breadcrumbs.dashboard.show'), admin_dashboard_path) li.divider - - models = [ExecutionEnvironment, Exercise, Consumer, CodeHarborLink, ExternalUser, FileType, FileTemplate, InternalUser].sort_by { |model| model.model_name.human(count: 2) } + - models = [ExecutionEnvironment, Exercise, Consumer, CodeHarborLink, ExternalUser, FileType, FileTemplate, + ErrorTemplate, ErrorTemplateAttribute, InternalUser].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")) From a9068ef1b7c7a28a46063d8b25fcd46190b8745c Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 21:37:57 +0200 Subject: [PATCH 15/69] Fix error template attribute creation --- app/controllers/error_template_attributes_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/error_template_attributes_controller.rb b/app/controllers/error_template_attributes_controller.rb index dc2da30e..e5e0d486 100644 --- a/app/controllers/error_template_attributes_controller.rb +++ b/app/controllers/error_template_attributes_controller.rb @@ -81,6 +81,6 @@ class ErrorTemplateAttributesController < ApplicationController # Never trust parameters from the scary internet, only allow the white list through. def error_template_attribute_params - params.fetch(:error_template_attribute, {}) + params[:error_template_attribute].permit(:key, :description, :regex, :important) end end From 0fabed4c1eb4490081f4cd66c18177ac570134c5 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 21:43:49 +0200 Subject: [PATCH 16/69] Fix sorting by important attributes --- app/views/error_templates/show.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index 5b513e6c..574e3d88 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -35,6 +35,6 @@ h3 #add-attribute = collection_select({}, :error_template_attribute_id, - ErrorTemplateAttribute.where.not(id: @error_template.error_template_attributes.select(:id).to_a).order(:important, :key), + ErrorTemplateAttribute.where.not(id: @error_template.error_template_attributes.select(:id).to_a).order('important DESC', :key), :id, :key, {include_blank: false}, class: '') button.btn.btn-default = t('error_templates.add_attribute') From 7ba245920c6c1b5c72a66a3dc7c6ae16492c13f5 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 12 Jul 2017 21:46:39 +0200 Subject: [PATCH 17/69] Use the same sorting everywhere --- app/controllers/error_template_attributes_controller.rb | 2 +- app/views/error_templates/show.html.slim | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/error_template_attributes_controller.rb b/app/controllers/error_template_attributes_controller.rb index e5e0d486..05de2772 100644 --- a/app/controllers/error_template_attributes_controller.rb +++ b/app/controllers/error_template_attributes_controller.rb @@ -9,7 +9,7 @@ class ErrorTemplateAttributesController < ApplicationController # GET /error_template_attributes # GET /error_template_attributes.json def index - @error_template_attributes = ErrorTemplateAttribute.all.order(:id).paginate(page: params[:page]) + @error_template_attributes = ErrorTemplateAttribute.all.order('important DESC', :key, :id).paginate(page: params[:page]) authorize! end diff --git a/app/views/error_templates/show.html.slim b/app/views/error_templates/show.html.slim index 574e3d88..9936ef7f 100644 --- a/app/views/error_templates/show.html.slim +++ b/app/views/error_templates/show.html.slim @@ -20,7 +20,7 @@ h3 th = t('activerecord.attributes.error_template_attribute.regex') th colspan=3 = t('shared.actions') tbody - - @error_template.error_template_attributes.order(:important).each do |attribute| + - @error_template.error_template_attributes.order('important DESC', :key).each do |attribute| tr td - if attribute.important From 5d6158f95a2d63062fcdf89e20eec619316fd457 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 19 Jul 2017 15:46:19 +0200 Subject: [PATCH 18/69] Fix structured error creation if attributes don't match; write match status to database --- app/models/structured_error_attribute.rb | 12 ++++++++++-- ...133351_add_match_to_structured_error_attribute.rb | 5 +++++ db/schema.rb | 3 ++- 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20170719133351_add_match_to_structured_error_attribute.rb diff --git a/app/models/structured_error_attribute.rb b/app/models/structured_error_attribute.rb index b9967719..6eb17fc4 100644 --- a/app/models/structured_error_attribute.rb +++ b/app/models/structured_error_attribute.rb @@ -3,7 +3,15 @@ class StructuredErrorAttribute < ActiveRecord::Base belongs_to :error_template_attribute def self.create_from_template(attribute, structured_error, message_buffer) - value = message_buffer.match(attribute.regex).captures[0] - self.create(structured_error: structured_error, error_template_attribute: attribute, value: value) + match = false + value = nil + result = message_buffer.match(attribute.regex) + if result != nil + match = true + if result.captures.size > 0 + value = result.captures[0] + end + end + self.create(structured_error: structured_error, error_template_attribute: attribute, value: value, match: match) end end diff --git a/db/migrate/20170719133351_add_match_to_structured_error_attribute.rb b/db/migrate/20170719133351_add_match_to_structured_error_attribute.rb new file mode 100644 index 00000000..7ec3ccc0 --- /dev/null +++ b/db/migrate/20170719133351_add_match_to_structured_error_attribute.rb @@ -0,0 +1,5 @@ +class AddMatchToStructuredErrorAttribute < ActiveRecord::Migration + def change + add_column :structured_error_attributes, :match, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 137d251d..62a776f9 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: 20170711170928) do +ActiveRecord::Schema.define(version: 20170719133351) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -298,6 +298,7 @@ ActiveRecord::Schema.define(version: 20170711170928) do t.string "value" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "match" end create_table "structured_errors", force: :cascade do |t| From 280b4dbe0c00bd0716b6eafc60537bb299a80dd1 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 23 Aug 2017 15:37:48 +0200 Subject: [PATCH 19/69] Fix question mark bug --- app/controllers/submissions_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index ae111823..5a256b17 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -202,7 +202,7 @@ class SubmissionsController < ApplicationController if !@message_buffer.blank? @submission.exercise.execution_environment.error_templates.each do |template| pattern = Regexp.new(template.signature).freeze - if pattern.match?(@message_buffer) + if pattern.match(@message_buffer) StructuredError.create_from_template(template, @message_buffer) end end From 44a3cabe988df187755a69da3e285b4a6c85be12 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 27 Sep 2017 16:08:56 +0200 Subject: [PATCH 20/69] Scaffold exercise collection routes --- .../exercise_collections_controller.rb | 24 +++++++++++++++++++ app/policies/exercise_collection_policy.rb | 3 +++ .../exercise_collections/index.html.slim | 1 + app/views/exercise_collections/show.html.slim | 1 + config/routes.rb | 2 ++ .../exercise_collections_controller_test.rb | 14 +++++++++++ 6 files changed, 45 insertions(+) create mode 100644 app/controllers/exercise_collections_controller.rb create mode 100644 app/policies/exercise_collection_policy.rb create mode 100644 app/views/exercise_collections/index.html.slim create mode 100644 app/views/exercise_collections/show.html.slim create mode 100644 test/controllers/exercise_collections_controller_test.rb diff --git a/app/controllers/exercise_collections_controller.rb b/app/controllers/exercise_collections_controller.rb new file mode 100644 index 00000000..359ed7b8 --- /dev/null +++ b/app/controllers/exercise_collections_controller.rb @@ -0,0 +1,24 @@ +class ExerciseCollectionsController < ApplicationController + + before_action :set_exercise_collection, only: [:show] + + def index + @exercise_collections = ExerciseCollection.all + authorize! + end + + def show + end + + + private + + def set_exercise_collection + @exercise_collection = ExerciseCollection.find(params[:id]) + authorize! + end + + def authorize! + authorize(@exercise_collection || @exercise_collections) + end +end diff --git a/app/policies/exercise_collection_policy.rb b/app/policies/exercise_collection_policy.rb new file mode 100644 index 00000000..ff150290 --- /dev/null +++ b/app/policies/exercise_collection_policy.rb @@ -0,0 +1,3 @@ +class ExerciseCollectionPolicy < AdminOnlyPolicy + +end diff --git a/app/views/exercise_collections/index.html.slim b/app/views/exercise_collections/index.html.slim new file mode 100644 index 00000000..03f4829c --- /dev/null +++ b/app/views/exercise_collections/index.html.slim @@ -0,0 +1 @@ +h1 = 'ExerciseCollections#index' diff --git a/app/views/exercise_collections/show.html.slim b/app/views/exercise_collections/show.html.slim new file mode 100644 index 00000000..6ed2b5f7 --- /dev/null +++ b/app/views/exercise_collections/show.html.slim @@ -0,0 +1 @@ +h1 = 'ExerciseCollection#show' diff --git a/config/routes.rb b/config/routes.rb index eb243464..7f254f1d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -74,6 +74,8 @@ Rails.application.routes.draw do end end + resources :exercise_collections + resources :proxy_exercises do member do post :clone diff --git a/test/controllers/exercise_collections_controller_test.rb b/test/controllers/exercise_collections_controller_test.rb new file mode 100644 index 00000000..699c9271 --- /dev/null +++ b/test/controllers/exercise_collections_controller_test.rb @@ -0,0 +1,14 @@ +require 'test_helper' + +class ExerciseCollectionsControllerTest < ActionController::TestCase + test "should get index" do + get :index + assert_response :success + end + + test "should get show" do + get :show + assert_response :success + end + +end From 93797a665de07cfd9b8fd70a6a4375a599218798 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 27 Sep 2017 16:37:52 +0200 Subject: [PATCH 21/69] Implement index page --- .../exercise_collections_controller.rb | 2 +- .../exercise_collections/index.html.slim | 25 ++++++++++++++++++- config/locales/de.yml | 8 ++++++ config/locales/en.yml | 8 ++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/app/controllers/exercise_collections_controller.rb b/app/controllers/exercise_collections_controller.rb index 359ed7b8..fb4e9ba2 100644 --- a/app/controllers/exercise_collections_controller.rb +++ b/app/controllers/exercise_collections_controller.rb @@ -3,7 +3,7 @@ class ExerciseCollectionsController < ApplicationController before_action :set_exercise_collection, only: [:show] def index - @exercise_collections = ExerciseCollection.all + @exercise_collections = ExerciseCollection.all.paginate(:page => params[:page]) authorize! end diff --git a/app/views/exercise_collections/index.html.slim b/app/views/exercise_collections/index.html.slim index 03f4829c..98de8a20 100644 --- a/app/views/exercise_collections/index.html.slim +++ b/app/views/exercise_collections/index.html.slim @@ -1 +1,24 @@ -h1 = 'ExerciseCollections#index' +h1 = ExerciseCollection.model_name.human(count: 2) + +.table-responsive + table.table + thead + tr + th = t('activerecord.attributes.exercise_collections.id') + th = t('activerecord.attributes.exercise_collections.name') + th = t('activerecord.attributes.exercise_collections.updated_at') + th = t('activerecord.attributes.exercise_collections.exercises') + th colspan=3 = t('shared.actions') + tbody + - @exercise_collections.each do |collection| + tr + td = collection.id + td = link_to(collection.name, collection) + td = collection.updated_at + td = collection.exercises.size + td = link_to(t('shared.show'), collection) + td = link_to(t('shared.edit'), edit_exercise_collection_path(collection)) + td = link_to(t('shared.destroy'), collection, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + += render('shared/pagination', collection: @exercise_collections) +p = render('shared/new_button', model: ExerciseCollection) \ No newline at end of file diff --git a/config/locales/de.yml b/config/locales/de.yml index 2ab23322..23232f19 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -111,6 +111,11 @@ de: name: "Name" file_type: "Dateityp" content: "Code" + exercise_collections: + id: "ID" + name: "Name" + updated_at: "Letzte Änderung" + exercises: "Aufgaben" models: code_harbor_link: one: CodeHarbor-Link @@ -127,6 +132,9 @@ de: exercise: one: Aufgabe other: Aufgaben + exercise_collection: + one: Aufgabesammlung + other: Aufgabensammlungen proxy_exercise: one: Proxy Aufgabe other: Proxy Aufgaben diff --git a/config/locales/en.yml b/config/locales/en.yml index e2cba090..543d9cd0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -111,6 +111,11 @@ en: name: "Name" file_type: "File Type" content: "Content" + exercise_collections: + id: "ID" + name: "Name" + updated_at: "Last Update" + exercises: "Exercises" models: code_harbor_link: one: CodeHarbor Link @@ -127,6 +132,9 @@ en: exercise: one: Exercise other: Exercises + exercise_collection: + one: Exercise Collection + other: Exercise Collections proxy_exercise: one: Proxy Exercise other: Proxy Exercises From f0c0621b3114d17ffd7d54f959fb53d866c6d628 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 27 Sep 2017 16:52:42 +0200 Subject: [PATCH 22/69] Implement show route --- app/models/exercise_collection.rb | 4 ++++ app/views/exercise_collections/show.html.slim | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/exercise_collection.rb b/app/models/exercise_collection.rb index 2dca0e9d..f672554d 100644 --- a/app/models/exercise_collection.rb +++ b/app/models/exercise_collection.rb @@ -2,4 +2,8 @@ class ExerciseCollection < ActiveRecord::Base has_and_belongs_to_many :exercises + def to_s + "#{I18n.t('activerecord.models.exercise_collection.one')}: #{name} (#{id})" + end + end \ No newline at end of file diff --git a/app/views/exercise_collections/show.html.slim b/app/views/exercise_collections/show.html.slim index 6ed2b5f7..89081efe 100644 --- a/app/views/exercise_collections/show.html.slim +++ b/app/views/exercise_collections/show.html.slim @@ -1 +1,11 @@ -h1 = 'ExerciseCollection#show' +h1 + = @exercise_collection + = render('shared/edit_button', object: @exercise_collection) + += row(label: 'exercise_collections.name', value: @exercise_collection.name) += row(label: 'exercise_collections.updated_at', value: @exercise_collection.updated_at) + +h4 = t('activerecord.attributes.exercise_collections.exercises') +ul.list-unstyled + - @exercise_collection.exercises.sort_by{|c| c.title}.each do |exercise| + li = exercise \ No newline at end of file From bce58071d13b6a85e00f6823bef78c40b023b4b9 Mon Sep 17 00:00:00 2001 From: Thomas Hille Date: Tue, 3 Oct 2017 18:46:39 +0200 Subject: [PATCH 23/69] removed expected working time from show/create/index and database since we do not use it --- app/controllers/exercises_controller.rb | 3 +- app/views/exercises/_form.html.slim | 3 -- app/views/exercises/index.html.slim | 2 - app/views/exercises/show.html.slim | 1 - config/locales/de.yml | 1 - config/locales/en.yml | 1 - ...1002131135_remove_expected_working_time.rb | 5 ++ db/schema.rb | 50 ++++++++++++++++--- 8 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 db/migrate/20171002131135_remove_expected_working_time.rb diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 141b47ee..4734b2c1 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -143,8 +143,7 @@ class ExercisesController < ApplicationController private :user_by_code_harbor_token def exercise_params - params[:exercise][:expected_worktime_seconds] = params[:exercise][:expected_worktime_minutes].to_i * 60 - params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :allow_auto_completion, :title, :expected_difficulty, :expected_worktime_seconds, files_attributes: file_attributes, :tag_ids => []).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, :allow_auto_completion, :title, :expected_difficulty, files_attributes: file_attributes, :tag_ids => []).merge(user_id: current_user.id, user_type: current_user.class.name) end private :exercise_params diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index bb4aa851..646c359a 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -35,9 +35,6 @@ .form-group = f.label(t('activerecord.attributes.exercise.difficulty')) = f.number_field :expected_difficulty, in: 1..10, step: 1 - .form-group - = f.label(t('activerecord.attributes.exercise.worktime')) - = f.number_field "expected_worktime_minutes", value: @exercise.expected_worktime_seconds / 60, in: 1..1000, step: 1 h2 = t('exercises.form.tags') ul.list-unstyled.panel-group diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index bd8fe880..8b78af65 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -18,7 +18,6 @@ h1 = Exercise.model_name.human(count: 2) th = t('activerecord.attributes.exercise.maximum_score') th = t('activerecord.attributes.exercise.tags') th = t('activerecord.attributes.exercise.difficulty') - th = t('activerecord.attributes.exercise.worktime') th = t('activerecord.attributes.exercise.public') - if policy(Exercise).batch_update? @@ -34,7 +33,6 @@ h1 = Exercise.model_name.human(count: 2) td = exercise.maximum_score td = exercise.exercise_tags.count td = exercise.expected_difficulty - td = (exercise.expected_worktime_seconds / 60).ceil td.public data-value=exercise.public? = symbol_for(exercise.public?) td = link_to(t('shared.edit'), edit_exercise_path(exercise)) if policy(exercise).edit? td = link_to(t('.implement'), implement_exercise_path(exercise)) if policy(exercise).implement? diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index 1efbd612..e3dfc7d5 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -20,7 +20,6 @@ h1 = row(label: 'exercise.embedding_parameters') do = content_tag(:input, nil, class: 'form-control', readonly: true, value: embedding_parameters(@exercise)) = row(label: 'exercise.difficulty', value: @exercise.expected_difficulty) -= row(label: 'exercise.worktime', value: "#{@exercise.expected_worktime_seconds/60} min") = row(label: 'exercise.tags', value: @exercise.exercise_tags.map{|et| "#{et.tag.name} (#{et.factor})"}.sort.join(", ")) h2 = t('activerecord.attributes.exercise.files') diff --git a/config/locales/de.yml b/config/locales/de.yml index a97023a4..27aba44f 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -41,7 +41,6 @@ de: allow_auto_completion: "Autovervollständigung aktivieren" allow_file_creation: "Dateierstellung erlauben" difficulty: Schwierigkeitsgrad - worktime: "vermutete Arbeitszeit in Minuten" token: "Aufgaben-Token" proxy_exercise: title: Title diff --git a/config/locales/en.yml b/config/locales/en.yml index 3d6e6b9c..380e5d7e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -41,7 +41,6 @@ en: allow_auto_completion: "Allow auto completion" allow_file_creation: "Allow file creation" difficulty: Difficulty - worktime: "Expected worktime in minutes" token: "Exercise Token" proxy_exercise: title: Title diff --git a/db/migrate/20171002131135_remove_expected_working_time.rb b/db/migrate/20171002131135_remove_expected_working_time.rb new file mode 100644 index 00000000..eb4bbcb4 --- /dev/null +++ b/db/migrate/20171002131135_remove_expected_working_time.rb @@ -0,0 +1,5 @@ +class RemoveExpectedWorkingTime < ActiveRecord::Migration + def change + remove_column :exercises, :expected_worktime_seconds + end +end diff --git a/db/schema.rb b/db/schema.rb index 988cd447..c8eb631b 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: 20170920145852) do +ActiveRecord::Schema.define(version: 20171002131135) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -47,6 +47,13 @@ ActiveRecord::Schema.define(version: 20170920145852) do t.string "oauth_secret", limit: 255 end + create_table "copy_paste_events", id: false, force: :cascade do |t| + t.integer "exercise_id" + t.string "text" + t.integer "user_id" + t.datetime "created_at" + end + create_table "errors", force: :cascade do |t| t.integer "execution_environment_id" t.text "message" @@ -99,19 +106,18 @@ ActiveRecord::Schema.define(version: 20170920145852) do create_table "exercises", force: :cascade do |t| t.text "description" t.integer "execution_environment_id" - t.string "title", limit: 255 + t.string "title", limit: 255 t.datetime "created_at" t.datetime "updated_at" t.integer "user_id" t.text "instructions" t.boolean "public" - t.string "user_type", limit: 255 - t.string "token", limit: 255 + t.string "user_type", limit: 255 + t.string "token", limit: 255 t.boolean "hide_file_tree" t.boolean "allow_file_creation" - t.boolean "allow_auto_completion", default: false - t.integer "expected_worktime_seconds", default: 60 - t.integer "expected_difficulty", default: 1 + t.boolean "allow_auto_completion", default: false + t.integer "expected_difficulty", default: 1 end create_table "exercises_proxy_exercises", id: false, force: :cascade do |t| @@ -124,6 +130,11 @@ ActiveRecord::Schema.define(version: 20170920145852) do add_index "exercises_proxy_exercises", ["exercise_id"], name: "index_exercises_proxy_exercises_on_exercise_id", using: :btree add_index "exercises_proxy_exercises", ["proxy_exercise_id"], name: "index_exercises_proxy_exercises_on_proxy_exercise_id", using: :btree + create_table "external_user_skill_level", id: false, force: :cascade do |t| + t.string "external_user_id" + t.integer "skill_level" + end + create_table "external_users", force: :cascade do |t| t.integer "consumer_id" t.string "email", limit: 255 @@ -342,4 +353,29 @@ ActiveRecord::Schema.define(version: 20170920145852) do add_index "user_proxy_exercise_exercises", ["proxy_exercise_id"], name: "index_user_proxy_exercise_exercises_on_proxy_exercise_id", using: :btree add_index "user_proxy_exercise_exercises", ["user_type", "user_id"], name: "index_user_proxy_exercise_exercises_on_user_type_and_user_id", using: :btree + create_table "user_proxy_exercise_exercises_copy", id: false, force: :cascade do |t| + t.integer "id" + t.integer "user_id" + t.string "user_type" + t.integer "proxy_exercise_id" + t.integer "exercise_id" + t.datetime "created_at" + t.datetime "updated_at" + t.json "reason" + t.json "reason_json" + end + + create_table "wk_with_wk_until_rfc", id: false, force: :cascade do |t| + t.string "external_user_id", limit: 255 + t.integer "user_id" + t.integer "exercise_id" + t.float "max_score" + t.float "max_reachable_points" + t.string "working_time" + t.string "working_time_until_rfc" + t.string "working_time_until_rfc_reply" + t.time "percentile75" + t.time "percentile90" + end + end From 7cb7146e7ef190f4dcc3fec2c74a615e034e77b3 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 11:35:51 +0200 Subject: [PATCH 24/69] Implement update --- app/controllers/exercise_collections_controller.rb | 13 ++++++++++++- app/views/exercise_collections/_form.html.slim | 11 +++++++++++ app/views/exercise_collections/edit.html.slim | 1 + 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 app/views/exercise_collections/_form.html.slim create mode 100644 app/views/exercise_collections/edit.html.slim diff --git a/app/controllers/exercise_collections_controller.rb b/app/controllers/exercise_collections_controller.rb index fb4e9ba2..a00f48fd 100644 --- a/app/controllers/exercise_collections_controller.rb +++ b/app/controllers/exercise_collections_controller.rb @@ -1,6 +1,7 @@ class ExerciseCollectionsController < ApplicationController + include CommonBehavior - before_action :set_exercise_collection, only: [:show] + before_action :set_exercise_collection, only: [:show, :edit, :update] def index @exercise_collections = ExerciseCollection.all.paginate(:page => params[:page]) @@ -10,6 +11,12 @@ class ExerciseCollectionsController < ApplicationController def show end + def edit + end + + def update + update_and_respond(object: @exercise_collection, params: exercise_collection_params) + end private @@ -21,4 +28,8 @@ class ExerciseCollectionsController < ApplicationController def authorize! authorize(@exercise_collection || @exercise_collections) end + + def exercise_collection_params + params[:exercise_collection].permit(:name, :exercise_ids) + end end diff --git a/app/views/exercise_collections/_form.html.slim b/app/views/exercise_collections/_form.html.slim new file mode 100644 index 00000000..a8ebf0b2 --- /dev/null +++ b/app/views/exercise_collections/_form.html.slim @@ -0,0 +1,11 @@ +- exercises = Exercise.all + += form_for(@exercise_collection, data: {exercises: exercises}, multipart: true) do |f| + = render('shared/form_errors', object: @exercise_collection) + .form-group + = f.label(:name) + = f.text_field(:name, class: 'form-control', required: true) + .form-group + = f.label(:exercises) + = f.collection_select(:exercise_ids, exercises, :id, :title, {}, {class: 'form-control', multiple: true}) + .actions = render('shared/submit_button', f: f, object: @exercise_collection) \ No newline at end of file diff --git a/app/views/exercise_collections/edit.html.slim b/app/views/exercise_collections/edit.html.slim new file mode 100644 index 00000000..4e864ab1 --- /dev/null +++ b/app/views/exercise_collections/edit.html.slim @@ -0,0 +1 @@ += render('form') \ No newline at end of file From 8e4a6946900b99485d66392271b5c58137db3db2 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 11:37:21 +0200 Subject: [PATCH 25/69] Add title --- app/views/exercise_collections/edit.html.slim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/exercise_collections/edit.html.slim b/app/views/exercise_collections/edit.html.slim index 4e864ab1..a3694a25 100644 --- a/app/views/exercise_collections/edit.html.slim +++ b/app/views/exercise_collections/edit.html.slim @@ -1 +1,3 @@ +h1 = @exercise_collection + = render('form') \ No newline at end of file From 95a849e9f551e369d47f5743e2f643d0d0f6ce40 Mon Sep 17 00:00:00 2001 From: Thomas Hille Date: Tue, 3 Oct 2017 18:46:39 +0200 Subject: [PATCH 26/69] removed expected working time from show/create/index and database since we do not use it --- app/controllers/exercises_controller.rb | 3 +- app/views/exercises/_form.html.slim | 3 -- app/views/exercises/index.html.slim | 2 - app/views/exercises/show.html.slim | 1 - config/locales/de.yml | 1 - config/locales/en.yml | 1 - ...1002131135_remove_expected_working_time.rb | 5 ++ db/schema.rb | 50 ++++++++++++++++--- 8 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 db/migrate/20171002131135_remove_expected_working_time.rb diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 141b47ee..4734b2c1 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -143,8 +143,7 @@ class ExercisesController < ApplicationController private :user_by_code_harbor_token def exercise_params - params[:exercise][:expected_worktime_seconds] = params[:exercise][:expected_worktime_minutes].to_i * 60 - params[:exercise].permit(:description, :execution_environment_id, :file_id, :instructions, :public, :hide_file_tree, :allow_file_creation, :allow_auto_completion, :title, :expected_difficulty, :expected_worktime_seconds, files_attributes: file_attributes, :tag_ids => []).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, :allow_auto_completion, :title, :expected_difficulty, files_attributes: file_attributes, :tag_ids => []).merge(user_id: current_user.id, user_type: current_user.class.name) end private :exercise_params diff --git a/app/views/exercises/_form.html.slim b/app/views/exercises/_form.html.slim index bb4aa851..646c359a 100644 --- a/app/views/exercises/_form.html.slim +++ b/app/views/exercises/_form.html.slim @@ -35,9 +35,6 @@ .form-group = f.label(t('activerecord.attributes.exercise.difficulty')) = f.number_field :expected_difficulty, in: 1..10, step: 1 - .form-group - = f.label(t('activerecord.attributes.exercise.worktime')) - = f.number_field "expected_worktime_minutes", value: @exercise.expected_worktime_seconds / 60, in: 1..1000, step: 1 h2 = t('exercises.form.tags') ul.list-unstyled.panel-group diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index bd8fe880..8b78af65 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -18,7 +18,6 @@ h1 = Exercise.model_name.human(count: 2) th = t('activerecord.attributes.exercise.maximum_score') th = t('activerecord.attributes.exercise.tags') th = t('activerecord.attributes.exercise.difficulty') - th = t('activerecord.attributes.exercise.worktime') th = t('activerecord.attributes.exercise.public') - if policy(Exercise).batch_update? @@ -34,7 +33,6 @@ h1 = Exercise.model_name.human(count: 2) td = exercise.maximum_score td = exercise.exercise_tags.count td = exercise.expected_difficulty - td = (exercise.expected_worktime_seconds / 60).ceil td.public data-value=exercise.public? = symbol_for(exercise.public?) td = link_to(t('shared.edit'), edit_exercise_path(exercise)) if policy(exercise).edit? td = link_to(t('.implement'), implement_exercise_path(exercise)) if policy(exercise).implement? diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index 1efbd612..e3dfc7d5 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -20,7 +20,6 @@ h1 = row(label: 'exercise.embedding_parameters') do = content_tag(:input, nil, class: 'form-control', readonly: true, value: embedding_parameters(@exercise)) = row(label: 'exercise.difficulty', value: @exercise.expected_difficulty) -= row(label: 'exercise.worktime', value: "#{@exercise.expected_worktime_seconds/60} min") = row(label: 'exercise.tags', value: @exercise.exercise_tags.map{|et| "#{et.tag.name} (#{et.factor})"}.sort.join(", ")) h2 = t('activerecord.attributes.exercise.files') diff --git a/config/locales/de.yml b/config/locales/de.yml index a97023a4..27aba44f 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -41,7 +41,6 @@ de: allow_auto_completion: "Autovervollständigung aktivieren" allow_file_creation: "Dateierstellung erlauben" difficulty: Schwierigkeitsgrad - worktime: "vermutete Arbeitszeit in Minuten" token: "Aufgaben-Token" proxy_exercise: title: Title diff --git a/config/locales/en.yml b/config/locales/en.yml index 3d6e6b9c..380e5d7e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -41,7 +41,6 @@ en: allow_auto_completion: "Allow auto completion" allow_file_creation: "Allow file creation" difficulty: Difficulty - worktime: "Expected worktime in minutes" token: "Exercise Token" proxy_exercise: title: Title diff --git a/db/migrate/20171002131135_remove_expected_working_time.rb b/db/migrate/20171002131135_remove_expected_working_time.rb new file mode 100644 index 00000000..eb4bbcb4 --- /dev/null +++ b/db/migrate/20171002131135_remove_expected_working_time.rb @@ -0,0 +1,5 @@ +class RemoveExpectedWorkingTime < ActiveRecord::Migration + def change + remove_column :exercises, :expected_worktime_seconds + end +end diff --git a/db/schema.rb b/db/schema.rb index 988cd447..c8eb631b 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: 20170920145852) do +ActiveRecord::Schema.define(version: 20171002131135) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -47,6 +47,13 @@ ActiveRecord::Schema.define(version: 20170920145852) do t.string "oauth_secret", limit: 255 end + create_table "copy_paste_events", id: false, force: :cascade do |t| + t.integer "exercise_id" + t.string "text" + t.integer "user_id" + t.datetime "created_at" + end + create_table "errors", force: :cascade do |t| t.integer "execution_environment_id" t.text "message" @@ -99,19 +106,18 @@ ActiveRecord::Schema.define(version: 20170920145852) do create_table "exercises", force: :cascade do |t| t.text "description" t.integer "execution_environment_id" - t.string "title", limit: 255 + t.string "title", limit: 255 t.datetime "created_at" t.datetime "updated_at" t.integer "user_id" t.text "instructions" t.boolean "public" - t.string "user_type", limit: 255 - t.string "token", limit: 255 + t.string "user_type", limit: 255 + t.string "token", limit: 255 t.boolean "hide_file_tree" t.boolean "allow_file_creation" - t.boolean "allow_auto_completion", default: false - t.integer "expected_worktime_seconds", default: 60 - t.integer "expected_difficulty", default: 1 + t.boolean "allow_auto_completion", default: false + t.integer "expected_difficulty", default: 1 end create_table "exercises_proxy_exercises", id: false, force: :cascade do |t| @@ -124,6 +130,11 @@ ActiveRecord::Schema.define(version: 20170920145852) do add_index "exercises_proxy_exercises", ["exercise_id"], name: "index_exercises_proxy_exercises_on_exercise_id", using: :btree add_index "exercises_proxy_exercises", ["proxy_exercise_id"], name: "index_exercises_proxy_exercises_on_proxy_exercise_id", using: :btree + create_table "external_user_skill_level", id: false, force: :cascade do |t| + t.string "external_user_id" + t.integer "skill_level" + end + create_table "external_users", force: :cascade do |t| t.integer "consumer_id" t.string "email", limit: 255 @@ -342,4 +353,29 @@ ActiveRecord::Schema.define(version: 20170920145852) do add_index "user_proxy_exercise_exercises", ["proxy_exercise_id"], name: "index_user_proxy_exercise_exercises_on_proxy_exercise_id", using: :btree add_index "user_proxy_exercise_exercises", ["user_type", "user_id"], name: "index_user_proxy_exercise_exercises_on_user_type_and_user_id", using: :btree + create_table "user_proxy_exercise_exercises_copy", id: false, force: :cascade do |t| + t.integer "id" + t.integer "user_id" + t.string "user_type" + t.integer "proxy_exercise_id" + t.integer "exercise_id" + t.datetime "created_at" + t.datetime "updated_at" + t.json "reason" + t.json "reason_json" + end + + create_table "wk_with_wk_until_rfc", id: false, force: :cascade do |t| + t.string "external_user_id", limit: 255 + t.integer "user_id" + t.integer "exercise_id" + t.float "max_score" + t.float "max_reachable_points" + t.string "working_time" + t.string "working_time_until_rfc" + t.string "working_time_until_rfc_reply" + t.time "percentile75" + t.time "percentile90" + end + end From 0e7decd7fcef13814a5bfdb2c4c8f1fcbb0b910c Mon Sep 17 00:00:00 2001 From: Thomas Hille Date: Wed, 4 Oct 2017 11:41:45 +0200 Subject: [PATCH 27/69] removed clutter in schema.rb --- db/schema.rb | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index c8eb631b..35042622 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -47,13 +47,6 @@ ActiveRecord::Schema.define(version: 20171002131135) do t.string "oauth_secret", limit: 255 end - create_table "copy_paste_events", id: false, force: :cascade do |t| - t.integer "exercise_id" - t.string "text" - t.integer "user_id" - t.datetime "created_at" - end - create_table "errors", force: :cascade do |t| t.integer "execution_environment_id" t.text "message" @@ -130,11 +123,6 @@ ActiveRecord::Schema.define(version: 20171002131135) do add_index "exercises_proxy_exercises", ["exercise_id"], name: "index_exercises_proxy_exercises_on_exercise_id", using: :btree add_index "exercises_proxy_exercises", ["proxy_exercise_id"], name: "index_exercises_proxy_exercises_on_proxy_exercise_id", using: :btree - create_table "external_user_skill_level", id: false, force: :cascade do |t| - t.string "external_user_id" - t.integer "skill_level" - end - create_table "external_users", force: :cascade do |t| t.integer "consumer_id" t.string "email", limit: 255 @@ -353,29 +341,4 @@ ActiveRecord::Schema.define(version: 20171002131135) do add_index "user_proxy_exercise_exercises", ["proxy_exercise_id"], name: "index_user_proxy_exercise_exercises_on_proxy_exercise_id", using: :btree add_index "user_proxy_exercise_exercises", ["user_type", "user_id"], name: "index_user_proxy_exercise_exercises_on_user_type_and_user_id", using: :btree - create_table "user_proxy_exercise_exercises_copy", id: false, force: :cascade do |t| - t.integer "id" - t.integer "user_id" - t.string "user_type" - t.integer "proxy_exercise_id" - t.integer "exercise_id" - t.datetime "created_at" - t.datetime "updated_at" - t.json "reason" - t.json "reason_json" - end - - create_table "wk_with_wk_until_rfc", id: false, force: :cascade do |t| - t.string "external_user_id", limit: 255 - t.integer "user_id" - t.integer "exercise_id" - t.float "max_score" - t.float "max_reachable_points" - t.string "working_time" - t.string "working_time_until_rfc" - t.string "working_time_until_rfc_reply" - t.time "percentile75" - t.time "percentile90" - end - end From a0ebd5bec519936fd7b6d868dd87f545e6ac9b66 Mon Sep 17 00:00:00 2001 From: ThommyH Date: Wed, 4 Oct 2017 11:50:57 +0200 Subject: [PATCH 28/69] Update schema.rb --- db/schema.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 35042622..896ffbd3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -99,14 +99,14 @@ ActiveRecord::Schema.define(version: 20171002131135) do create_table "exercises", force: :cascade do |t| t.text "description" t.integer "execution_environment_id" - t.string "title", limit: 255 + t.string "title", limit: 255 t.datetime "created_at" t.datetime "updated_at" t.integer "user_id" t.text "instructions" t.boolean "public" - t.string "user_type", limit: 255 - t.string "token", limit: 255 + t.string "user_type", limit: 255 + t.string "token", limit: 255 t.boolean "hide_file_tree" t.boolean "allow_file_creation" t.boolean "allow_auto_completion", default: false From e74c25746cb791b3ae8c4ca4dcf260b80fea0049 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 12:40:37 +0200 Subject: [PATCH 29/69] Implement new action --- .../exercise_collections_controller.rb | 20 +++++++++++++++++-- app/views/exercise_collections/new.html.slim | 3 +++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 app/views/exercise_collections/new.html.slim diff --git a/app/controllers/exercise_collections_controller.rb b/app/controllers/exercise_collections_controller.rb index a00f48fd..4861a062 100644 --- a/app/controllers/exercise_collections_controller.rb +++ b/app/controllers/exercise_collections_controller.rb @@ -1,7 +1,7 @@ class ExerciseCollectionsController < ApplicationController include CommonBehavior - before_action :set_exercise_collection, only: [:show, :edit, :update] + before_action :set_exercise_collection, only: [:show, :edit, :update, :destroy] def index @exercise_collections = ExerciseCollection.all.paginate(:page => params[:page]) @@ -11,6 +11,22 @@ class ExerciseCollectionsController < ApplicationController def show end + def new + @exercise_collection = ExerciseCollection.new + authorize! + end + + def create + @exercise_collection = ExerciseCollection.new(exercise_collection_params) + authorize! + create_and_respond(object: @exercise_collection) + end + + def destroy + authorize! + destroy_and_respond(object: @exercise_collection) + end + def edit end @@ -30,6 +46,6 @@ class ExerciseCollectionsController < ApplicationController end def exercise_collection_params - params[:exercise_collection].permit(:name, :exercise_ids) + params[:exercise_collection].permit(:name, :exercise_ids => []) end end diff --git a/app/views/exercise_collections/new.html.slim b/app/views/exercise_collections/new.html.slim new file mode 100644 index 00000000..21a299f2 --- /dev/null +++ b/app/views/exercise_collections/new.html.slim @@ -0,0 +1,3 @@ +h1 = t('shared.new_model', model: ExerciseCollection.model_name.human) + += render('form') \ No newline at end of file From 70eb0d19b89049e16e44b69b51d5624f2f8a83a3 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 12:47:59 +0200 Subject: [PATCH 30/69] Link to exercises contained in collections --- app/views/exercise_collections/show.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/exercise_collections/show.html.slim b/app/views/exercise_collections/show.html.slim index 89081efe..ac2d0a37 100644 --- a/app/views/exercise_collections/show.html.slim +++ b/app/views/exercise_collections/show.html.slim @@ -8,4 +8,4 @@ h1 h4 = t('activerecord.attributes.exercise_collections.exercises') ul.list-unstyled - @exercise_collection.exercises.sort_by{|c| c.title}.each do |exercise| - li = exercise \ No newline at end of file + li = link_to(exercise, exercise) \ No newline at end of file From 06e99059d494a265711b29662d9a7d7974439a07 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 12:53:09 +0200 Subject: [PATCH 31/69] Add new lines at eof --- app/models/exercise_collection.rb | 2 +- app/views/exercise_collections/_form.html.slim | 2 +- app/views/exercise_collections/edit.html.slim | 2 +- app/views/exercise_collections/index.html.slim | 2 +- app/views/exercise_collections/new.html.slim | 2 +- app/views/exercise_collections/show.html.slim | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/exercise_collection.rb b/app/models/exercise_collection.rb index f672554d..10946962 100644 --- a/app/models/exercise_collection.rb +++ b/app/models/exercise_collection.rb @@ -6,4 +6,4 @@ class ExerciseCollection < ActiveRecord::Base "#{I18n.t('activerecord.models.exercise_collection.one')}: #{name} (#{id})" end -end \ No newline at end of file +end diff --git a/app/views/exercise_collections/_form.html.slim b/app/views/exercise_collections/_form.html.slim index a8ebf0b2..5acfd466 100644 --- a/app/views/exercise_collections/_form.html.slim +++ b/app/views/exercise_collections/_form.html.slim @@ -8,4 +8,4 @@ .form-group = f.label(:exercises) = f.collection_select(:exercise_ids, exercises, :id, :title, {}, {class: 'form-control', multiple: true}) - .actions = render('shared/submit_button', f: f, object: @exercise_collection) \ No newline at end of file + .actions = render('shared/submit_button', f: f, object: @exercise_collection) diff --git a/app/views/exercise_collections/edit.html.slim b/app/views/exercise_collections/edit.html.slim index a3694a25..187f31d2 100644 --- a/app/views/exercise_collections/edit.html.slim +++ b/app/views/exercise_collections/edit.html.slim @@ -1,3 +1,3 @@ h1 = @exercise_collection -= render('form') \ No newline at end of file += render('form') diff --git a/app/views/exercise_collections/index.html.slim b/app/views/exercise_collections/index.html.slim index 98de8a20..75a9d011 100644 --- a/app/views/exercise_collections/index.html.slim +++ b/app/views/exercise_collections/index.html.slim @@ -21,4 +21,4 @@ h1 = ExerciseCollection.model_name.human(count: 2) td = link_to(t('shared.destroy'), collection, data: {confirm: t('shared.confirm_destroy')}, method: :delete) = render('shared/pagination', collection: @exercise_collections) -p = render('shared/new_button', model: ExerciseCollection) \ No newline at end of file +p = render('shared/new_button', model: ExerciseCollection) diff --git a/app/views/exercise_collections/new.html.slim b/app/views/exercise_collections/new.html.slim index 21a299f2..0e48ccd9 100644 --- a/app/views/exercise_collections/new.html.slim +++ b/app/views/exercise_collections/new.html.slim @@ -1,3 +1,3 @@ h1 = t('shared.new_model', model: ExerciseCollection.model_name.human) -= render('form') \ No newline at end of file += render('form') diff --git a/app/views/exercise_collections/show.html.slim b/app/views/exercise_collections/show.html.slim index ac2d0a37..65c28283 100644 --- a/app/views/exercise_collections/show.html.slim +++ b/app/views/exercise_collections/show.html.slim @@ -8,4 +8,4 @@ h1 h4 = t('activerecord.attributes.exercise_collections.exercises') ul.list-unstyled - @exercise_collection.exercises.sort_by{|c| c.title}.each do |exercise| - li = link_to(exercise, exercise) \ No newline at end of file + li = link_to(exercise, exercise) From e7adb00dc1e1b68a4090c2b342d8b34b675e7b42 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 13:22:47 +0200 Subject: [PATCH 32/69] Fix empty questions in new RFC UI --- app/views/request_for_comments/show.html.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index eb88caf1..d3751d0b 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -29,7 +29,8 @@ <%= t('activerecord.attributes.request_for_comments.question')%>
- <%= @request_for_comment.question or t('request_for_comments.no_question')%> + <% question = @request_for_comment.question %> + <%= question.nil? or question.empty? ? t('request_for_comments.no_question') : question %>
From 2582f55b1eca96f729e716e24ec0bd4053ce2fbf Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 14:12:54 +0200 Subject: [PATCH 33/69] Add navigation link to exercise collections --- app/views/application/_navigation.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index fbeca1c3..19f10a85 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, ProxyExercise, Tag, Consumer, CodeHarborLink, ExternalUser, FileType, FileTemplate, InternalUser].sort_by { |model| model.model_name.human(count: 2) } + - models = [ExecutionEnvironment, Exercise, ExerciseCollection, ProxyExercise, Tag, Consumer, CodeHarborLink, ExternalUser, FileType, FileTemplate, InternalUser].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")) From ee5825c71b7c5e376e53ccd68112ffc42d08618a Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 4 Oct 2017 15:42:33 +0200 Subject: [PATCH 34/69] Order exercises in form --- app/views/exercise_collections/_form.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/exercise_collections/_form.html.slim b/app/views/exercise_collections/_form.html.slim index 5acfd466..3c336a62 100644 --- a/app/views/exercise_collections/_form.html.slim +++ b/app/views/exercise_collections/_form.html.slim @@ -1,4 +1,4 @@ -- exercises = Exercise.all +- exercises = Exercise.order(:title) = form_for(@exercise_collection, data: {exercises: exercises}, multipart: true) do |f| = render('shared/form_errors', object: @exercise_collection) From b078cddc68e556f59c01ce5b4f6b8ee474fa532f Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 4 Oct 2017 16:18:24 +0200 Subject: [PATCH 35/69] enhance testrun output --- app/controllers/concerns/submission_scoring.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/concerns/submission_scoring.rb b/app/controllers/concerns/submission_scoring.rb index 41f9ff26..f845ed0a 100644 --- a/app/controllers/concerns/submission_scoring.rb +++ b/app/controllers/concerns/submission_scoring.rb @@ -8,7 +8,7 @@ module SubmissionScoring output = execute_test_file(file, submission) assessment = assessor.assess(output) passed = ((assessment[:passed] == assessment[:count]) and (assessment[:score] > 0)) - testrun_output = passed ? nil : output[:stderr] + testrun_output = passed ? nil : 'message: ' + output[:message].to_s + '\n stdout: ' + output[:stdout].to_s + '\n stderr: ' + output[:stderr].to_s Testrun.new(submission: submission, cause: 'assess', file: file, passed: passed, output: testrun_output).save output.merge!(assessment) output.merge!(filename: file.name_with_extension, message: feedback_message(file, output[:score]), weight: file.weight) From e52c9213a1515eed2258dfe9b381267fb9ed1d83 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 4 Oct 2017 16:42:42 +0200 Subject: [PATCH 36/69] try to activate interpretation of carriage return --- app/controllers/concerns/submission_scoring.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/concerns/submission_scoring.rb b/app/controllers/concerns/submission_scoring.rb index f845ed0a..8891332a 100644 --- a/app/controllers/concerns/submission_scoring.rb +++ b/app/controllers/concerns/submission_scoring.rb @@ -8,7 +8,7 @@ module SubmissionScoring output = execute_test_file(file, submission) assessment = assessor.assess(output) passed = ((assessment[:passed] == assessment[:count]) and (assessment[:score] > 0)) - testrun_output = passed ? nil : 'message: ' + output[:message].to_s + '\n stdout: ' + output[:stdout].to_s + '\n stderr: ' + output[:stderr].to_s + testrun_output = passed ? nil : 'message: ' + output[:message].to_s + "\n stdout: " + output[:stdout].to_s + "\n stderr: " + output[:stderr].to_s Testrun.new(submission: submission, cause: 'assess', file: file, passed: passed, output: testrun_output).save output.merge!(assessment) output.merge!(filename: file.name_with_extension, message: feedback_message(file, output[:score]), weight: file.weight) From d353dbaf5ba7efe9d6beb44631d4ab3b50c73bac Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Sun, 15 Oct 2017 18:23:58 +0200 Subject: [PATCH 37/69] Implement index action. Repair destroy --- .../user_exercise_feedbacks_controller.rb | 37 +++++++++++++------ app/policies/user_exercise_feedback_policy.rb | 10 +---- .../user_exercise_feedbacks/index.html.slim | 19 ++++++++++ config/locales/de.yml | 3 ++ config/locales/en.yml | 3 ++ 5 files changed, 52 insertions(+), 20 deletions(-) create mode 100644 app/views/user_exercise_feedbacks/index.html.slim diff --git a/app/controllers/user_exercise_feedbacks_controller.rb b/app/controllers/user_exercise_feedbacks_controller.rb index 0d1e1925..6ce02da7 100644 --- a/app/controllers/user_exercise_feedbacks_controller.rb +++ b/app/controllers/user_exercise_feedbacks_controller.rb @@ -1,7 +1,8 @@ class UserExerciseFeedbacksController < ApplicationController include CommonBehavior - before_action :set_user_exercise_feedback, only: [:edit, :update] + before_action :set_user_exercise_feedback, only: [:show, :edit, :update] + before_action :set_user_exercise_feedback_by_id, only: [:destroy] def comment_presets [[0,t('user_exercise_feedback.difficulty_easy')], @@ -19,10 +20,14 @@ class UserExerciseFeedbacksController < ApplicationController [4,t('user_exercise_feedback.estimated_time_more_30')]] end - def authorize! - authorize(@uef) + def index + @uefs = UserExerciseFeedback.order(:id).paginate(page: params[:page]) + authorize! + end + + def show + authorize! end - private :authorize! def create @exercise = Exercise.find(uef_params[:exercise_id]) @@ -49,7 +54,8 @@ class UserExerciseFeedbacksController < ApplicationController end def destroy - destroy_and_respond(object: @tag) + authorize! + destroy_and_respond(object: @uef) end def edit @@ -58,11 +64,6 @@ class UserExerciseFeedbacksController < ApplicationController authorize! end - def uef_params - params[:user_exercise_feedback].permit(:feedback_text, :difficulty, :exercise_id, :user_estimated_worktime).merge(user_id: current_user.id, user_type: current_user.class.name) - end - private :uef_params - def new @texts = comment_presets.to_a @times = time_presets.to_a @@ -89,6 +90,12 @@ class UserExerciseFeedbacksController < ApplicationController end end + private + + def authorize! + authorize(@uef || @uefs) + end + def to_s name end @@ -98,6 +105,14 @@ class UserExerciseFeedbacksController < ApplicationController @uef = UserExerciseFeedback.find_by(exercise_id: params[:user_exercise_feedback][:exercise_id], user: current_user) end + def set_user_exercise_feedback_by_id + @uef = UserExerciseFeedback.find(params[:id]) + end + + def uef_params + params[:user_exercise_feedback].permit(:feedback_text, :difficulty, :exercise_id, :user_estimated_worktime).merge(user_id: current_user.id, user_type: current_user.class.name) + end + def validate_inputs(uef_params) begin if uef_params[:difficulty].to_i < 0 || uef_params[:difficulty].to_i >= comment_presets.size @@ -112,4 +127,4 @@ class UserExerciseFeedbacksController < ApplicationController end end -end \ No newline at end of file +end diff --git a/app/policies/user_exercise_feedback_policy.rb b/app/policies/user_exercise_feedback_policy.rb index 20a89a6e..a570a28c 100644 --- a/app/policies/user_exercise_feedback_policy.rb +++ b/app/policies/user_exercise_feedback_policy.rb @@ -1,8 +1,4 @@ -class UserExerciseFeedbackPolicy < ApplicationPolicy - def author? - @user == @record.author - end - private :author? +class UserExerciseFeedbackPolicy < AdminOrAuthorPolicy def create? everyone @@ -12,8 +8,4 @@ class UserExerciseFeedbackPolicy < ApplicationPolicy everyone end - [:show? ,:destroy?, :edit?, :update?].each do |action| - define_method(action) { admin? || author?} - end - end diff --git a/app/views/user_exercise_feedbacks/index.html.slim b/app/views/user_exercise_feedbacks/index.html.slim new file mode 100644 index 00000000..84f2a15f --- /dev/null +++ b/app/views/user_exercise_feedbacks/index.html.slim @@ -0,0 +1,19 @@ +h1 = UserExerciseFeedback.model_name.human(count: 2) + +.table-responsive + table.table + thead + tr + th colspan=2 = t('activerecord.attributes.user_exercise_feedback.user') + th = t('activerecord.attributes.user_exercise_feedback.exercise') + th colspan=2 = t('shared.actions') + tbody + - @uefs.each do |uef| + tr + td = uef.user.id + td = uef.user.name + td = link_to(uef.exercise.title, uef.exercise) + td = link_to(t('shared.show'), uef) + td = link_to(t('shared.destroy'), uef, data: {confirm: t('shared.confirm_destroy')}, method: :delete) + += render('shared/pagination', collection: @uefs) diff --git a/config/locales/de.yml b/config/locales/de.yml index 1e2b2337..523259e6 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -116,6 +116,9 @@ de: name: "Name" updated_at: "Letzte Änderung" exercises: "Aufgaben" + user_exercise_feedback: + user: "Autor" + exercise: "Aufgabe" models: code_harbor_link: one: CodeHarbor-Link diff --git a/config/locales/en.yml b/config/locales/en.yml index c55c90b0..4f4e0768 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -116,6 +116,9 @@ en: name: "Name" updated_at: "Last Update" exercises: "Exercises" + user_exercise_feedback: + user: "Author" + exercise: "Exercise" models: code_harbor_link: one: CodeHarbor Link From 12b9365e6eb8b4e1865fa443f9d98bead9fa72e7 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Sun, 15 Oct 2017 18:55:33 +0200 Subject: [PATCH 38/69] Implement show action --- app/controllers/user_exercise_feedbacks_controller.rb | 4 ++-- app/views/user_exercise_feedbacks/show.html.slim | 7 +++++++ config/locales/de.yml | 1 + config/locales/en.yml | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 app/views/user_exercise_feedbacks/show.html.slim diff --git a/app/controllers/user_exercise_feedbacks_controller.rb b/app/controllers/user_exercise_feedbacks_controller.rb index 6ce02da7..29b718e9 100644 --- a/app/controllers/user_exercise_feedbacks_controller.rb +++ b/app/controllers/user_exercise_feedbacks_controller.rb @@ -1,8 +1,8 @@ class UserExerciseFeedbacksController < ApplicationController include CommonBehavior - before_action :set_user_exercise_feedback, only: [:show, :edit, :update] - before_action :set_user_exercise_feedback_by_id, only: [:destroy] + before_action :set_user_exercise_feedback, only: [:edit, :update] + before_action :set_user_exercise_feedback_by_id, only: [:show, :destroy] def comment_presets [[0,t('user_exercise_feedback.difficulty_easy')], diff --git a/app/views/user_exercise_feedbacks/show.html.slim b/app/views/user_exercise_feedbacks/show.html.slim new file mode 100644 index 00000000..2c53e47f --- /dev/null +++ b/app/views/user_exercise_feedbacks/show.html.slim @@ -0,0 +1,7 @@ +h2 = @uef + += row(label: 'activerecord.attributes.user_exercise_feedback.exercise', value: link_to(@uef.exercise.title, @uef.exercise)) += row(label: 'user_exercise_feedback.user', value: @uef.user) += row(label: 'activerecord.attributes.user_exercise_feedback.feedback_text', value: @uef.feedback_text) += row(label: 'user_exercise_feedback.difficulty', value: @uef.difficulty) += row(label: 'user_exercise_feedback.working_time', value: @uef.user_estimated_worktime) diff --git a/config/locales/de.yml b/config/locales/de.yml index 523259e6..5e111be0 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -119,6 +119,7 @@ de: user_exercise_feedback: user: "Autor" exercise: "Aufgabe" + feedback_text: "Feedback Text" models: code_harbor_link: one: CodeHarbor-Link diff --git a/config/locales/en.yml b/config/locales/en.yml index 4f4e0768..e77242dc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -119,6 +119,7 @@ en: user_exercise_feedback: user: "Author" exercise: "Exercise" + feedback_text: "Feedback Text" models: code_harbor_link: one: CodeHarbor Link From 74a4313949438856124853e4a25ed8d3b12846ce Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Mon, 16 Oct 2017 11:34:57 +0200 Subject: [PATCH 39/69] Add feedback to navigation --- app/views/application/_navigation.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index 19f10a85..b65ead89 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, ExerciseCollection, ProxyExercise, Tag, Consumer, CodeHarborLink, ExternalUser, FileType, FileTemplate, InternalUser].sort_by { |model| model.model_name.human(count: 2) } + - models = [ExecutionEnvironment, Exercise, ExerciseCollection, ProxyExercise, Tag, Consumer, CodeHarborLink, ExternalUser, FileType, FileTemplate, InternalUser, UserExerciseFeedback].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")) From d162f78b67a78ea8340148835751480eccb7ff33 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Mon, 16 Oct 2017 11:45:23 +0200 Subject: [PATCH 40/69] Add filters to index page --- app/controllers/user_exercise_feedbacks_controller.rb | 3 ++- app/models/user_exercise_feedback.rb | 3 ++- app/views/user_exercise_feedbacks/index.html.slim | 8 ++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/controllers/user_exercise_feedbacks_controller.rb b/app/controllers/user_exercise_feedbacks_controller.rb index 29b718e9..8abffd66 100644 --- a/app/controllers/user_exercise_feedbacks_controller.rb +++ b/app/controllers/user_exercise_feedbacks_controller.rb @@ -21,7 +21,8 @@ class UserExerciseFeedbacksController < ApplicationController end def index - @uefs = UserExerciseFeedback.order(:id).paginate(page: params[:page]) + @search = UserExerciseFeedback.all.search params[:q] + @uefs = @search.result.includes(:execution_environment).order(:id).paginate(page: params[:page]) authorize! end diff --git a/app/models/user_exercise_feedback.rb b/app/models/user_exercise_feedback.rb index 78d84972..5694573b 100644 --- a/app/models/user_exercise_feedback.rb +++ b/app/models/user_exercise_feedback.rb @@ -2,10 +2,11 @@ class UserExerciseFeedback < ActiveRecord::Base include Creation belongs_to :exercise + has_one :execution_environment, through: :exercise validates :user_id, uniqueness: { scope: [:exercise_id, :user_type] } def to_s "User Exercise Feedback" end -end \ No newline at end of file +end diff --git a/app/views/user_exercise_feedbacks/index.html.slim b/app/views/user_exercise_feedbacks/index.html.slim index 84f2a15f..52249229 100644 --- a/app/views/user_exercise_feedbacks/index.html.slim +++ b/app/views/user_exercise_feedbacks/index.html.slim @@ -1,5 +1,13 @@ h1 = UserExerciseFeedback.model_name.human(count: 2) += render(layout: 'shared/form_filters') do |f| + .form-group + = f.label(:execution_environment_id_eq, t('activerecord.attributes.exercise.execution_environment'), class: 'sr-only') + = f.collection_select(:execution_environment_id_eq, ExecutionEnvironment.with_exercises, :id, :name, class: 'form-control', prompt: t('activerecord.attributes.exercise.execution_environment')) + .form-group + = f.label(:exercise_title_cont, t('activerecord.attributes.request_for_comments.exercise'), class: 'sr-only') + = f.search_field(:exercise_title_cont, class: 'form-control', placeholder: t('activerecord.attributes.request_for_comments.exercise')) + .table-responsive table.table thead From ddeab8c34feb9c1333959ffad40f7258bd98b995 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Mon, 16 Oct 2017 13:12:46 +0200 Subject: [PATCH 41/69] Remove auto-generated tests --- .../exercise_collections_controller_test.rb | 14 -------------- test/controllers/subscription_controller_test.rb | 7 ------- test/factories/subscriptions.rb | 7 ------- test/models/subscription_test.rb | 7 ------- 4 files changed, 35 deletions(-) delete mode 100644 test/controllers/exercise_collections_controller_test.rb delete mode 100644 test/controllers/subscription_controller_test.rb delete mode 100644 test/factories/subscriptions.rb delete mode 100644 test/models/subscription_test.rb diff --git a/test/controllers/exercise_collections_controller_test.rb b/test/controllers/exercise_collections_controller_test.rb deleted file mode 100644 index 699c9271..00000000 --- a/test/controllers/exercise_collections_controller_test.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'test_helper' - -class ExerciseCollectionsControllerTest < ActionController::TestCase - test "should get index" do - get :index - assert_response :success - end - - test "should get show" do - get :show - assert_response :success - end - -end diff --git a/test/controllers/subscription_controller_test.rb b/test/controllers/subscription_controller_test.rb deleted file mode 100644 index 8dde0e19..00000000 --- a/test/controllers/subscription_controller_test.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'test_helper' - -class SubscriptionControllerTest < ActionController::TestCase - # test "the truth" do - # assert true - # end -end diff --git a/test/factories/subscriptions.rb b/test/factories/subscriptions.rb deleted file mode 100644 index 11c5a67a..00000000 --- a/test/factories/subscriptions.rb +++ /dev/null @@ -1,7 +0,0 @@ -FactoryGirl.define do - factory :subscription do - user nil - request_for_comments nil - type "" - end -end diff --git a/test/models/subscription_test.rb b/test/models/subscription_test.rb deleted file mode 100644 index a045d1ea..00000000 --- a/test/models/subscription_test.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'test_helper' - -class SubscriptionTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end -end From 686d56bbd6e73ca907d9c981de11cd061723196f Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Mon, 16 Oct 2017 13:20:40 +0200 Subject: [PATCH 42/69] Add rspec persistence file to config to allow for re-running only failed tests locally --- spec/spec_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0ea8706a..1fa8ecda 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -82,4 +82,6 @@ RSpec.configure do |config| # a real object. This is generally recommended. mocks.verify_partial_doubles = true end + + config.example_status_persistence_file_path = 'tmp/rspec_persistence_file.txt' end From a00adbce256ce89c494d56074290ffab9b0b5d9e Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Mon, 16 Oct 2017 14:02:40 +0200 Subject: [PATCH 43/69] Move *_url spec to controller, because the subscription model does not handle urls anymore --- .../submissions_controller_spec.rb | 21 +++++++++++++++++++ spec/models/submission_spec.rb | 15 ------------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 3670645b..d1e489ff 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -183,6 +183,27 @@ describe SubmissionsController do expect_template(:show) end + describe 'GET #show.json' do + before(:each) { get :show, id: submission.id, format: :json } + expect_assigns(submission: :submission) + expect_status(200) + + [:render, :run, :test].each do |action| + describe "##{action}_url" do + let(:url) { response.body.send(:"#{action}_url") } + + it "starts like the #{action} path" do + filename = File.basename(__FILE__) + expect(url).to start_with(Rails.application.routes.url_helpers.send(:"#{action}_submission_path", submission, filename).sub(filename, '')) + end + + it 'ends with a placeholder' do + expect(url).to end_with(Submission::FILENAME_URL_PLACEHOLDER) + end + end + end + end + describe 'GET #score' do let(:request) { proc { get :score, id: submission.id } } before(:each) { request.call } diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 3c297ca4..75f0cf2a 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -16,21 +16,6 @@ describe Submission do expect(described_class.create.errors[:user_type]).to be_present end - [:render, :run, :test].each do |action| - describe "##{action}_url" do - let(:url) { submission.send(:"#{action}_url") } - - it "starts like the #{action} path" do - filename = File.basename(__FILE__) - expect(url).to start_with(Rails.application.routes.url_helpers.send(:"#{action}_submission_path", submission, filename).sub(filename, '')) - end - - it 'ends with a placeholder' do - expect(url).to end_with(Submission::FILENAME_URL_PLACEHOLDER) - end - end - end - describe '#main_file' do let(:submission) { FactoryGirl.create(:submission) } From ffe4f65628b3cf0b5c4e913898173488e806e4c9 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 18 Oct 2017 22:05:07 +0200 Subject: [PATCH 44/69] Adapt lti_spec to current functionality --- spec/concerns/lti_spec.rb | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index c03ef9a5..a7028224 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -25,31 +25,23 @@ describe Lti do describe '#external_user_name' do let(:first_name) { 'Jane' } - let(:full_name) { 'John Doe' } let(:last_name) { 'Doe' } + let(:full_name) { 'John Doe' } let(:provider) { double } + let(:provider_full) { double(:lis_person_name_full => full_name) } context 'when a full name is provided' do it 'returns the full name' do - expect(provider).to receive(:lis_person_name_full).twice.and_return(full_name) - expect(controller.send(:external_user_name, provider)).to eq(full_name) - end - end - - context 'when first and last name are provided' do - it 'returns the concatenated names' do - expect(provider).to receive(:lis_person_name_full) - expect(provider).to receive(:lis_person_name_given).twice.and_return(first_name) - expect(provider).to receive(:lis_person_name_family).twice.and_return(last_name) - expect(controller.send(:external_user_name, provider)).to eq("#{first_name} #{last_name}") + expect(provider_full).to receive(:lis_person_name_full).twice.and_return(full_name) + expect(controller.send(:external_user_name, provider_full)).to eq(full_name) end end context 'when only partial information is provided' do it 'returns the first available name' do expect(provider).to receive(:lis_person_name_full) - expect(provider).to receive(:lis_person_name_given).twice.and_return(first_name) - expect(provider).to receive(:lis_person_name_family) + expect(provider).to receive(:lis_person_name_given).and_return(first_name) + expect(provider).not_to receive(:lis_person_name_family) expect(controller.send(:external_user_name, provider)).to eq(first_name) end end @@ -122,6 +114,7 @@ describe Lti do context 'when grading is not supported' do it 'returns a corresponding status' do + skip('todo') expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(false) expect(controller.send(:send_score, submission.exercise_id, score, submission.user_id)[:status]).to eq('unsupported') end @@ -140,10 +133,12 @@ describe Lti do end it 'sends the score' do + skip('todo') controller.send(:send_score, submission.exercise_id, score, submission.user_id) end it 'returns code, message, and status' do + skip('todo') result = controller.send(:send_score, submission.exercise_id, score, submission.user_id) expect(result[:code]).to eq(response.response_code) expect(result[:message]).to eq(response.body) From 6d28f427d89eb124f2ee65bcb79fd4da2c21e880 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 25 Oct 2017 10:12:29 +0200 Subject: [PATCH 45/69] Add Ralf's comment to skipped tests --- spec/concerns/lti_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index a7028224..32d740aa 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -114,7 +114,7 @@ describe Lti do context 'when grading is not supported' do it 'returns a corresponding status' do - skip('todo') + skip('ralf: this does not work, since send_score pulls data from the database, which then returns an empty array. On this is called .first, which returns nil and lets the test fail. Before Toms changes, this was taken from the session, which could be mocked') expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:outcome_service?).and_return(false) expect(controller.send(:send_score, submission.exercise_id, score, submission.user_id)[:status]).to eq('unsupported') end @@ -133,12 +133,12 @@ describe Lti do end it 'sends the score' do - skip('todo') + skip('ralf: this does not work, since send_score pulls data from the database, which then returns an empty array. On this is called .first, which returns nil and lets the test fail. Before Toms changes, this was taken from the session, which could be mocked') controller.send(:send_score, submission.exercise_id, score, submission.user_id) end it 'returns code, message, and status' do - skip('todo') + skip('ralf: this does not work, since send_score pulls data from the database, which then returns an empty array. On this is called .first, which returns nil and lets the test fail. Before Toms changes, this was taken from the session, which could be mocked') result = controller.send(:send_score, submission.exercise_id, score, submission.user_id) expect(result[:code]).to eq(response.response_code) expect(result[:message]).to eq(response.body) From 539b2931c31d1597a41e500677175db2822b24df Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 25 Oct 2017 10:38:27 +0200 Subject: [PATCH 46/69] changed behaviour of user exercise feedback (max 20 is collected), added some constant for the integers used. --- app/controllers/exercises_controller.rb | 31 ++++++++++++++++--------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 141b47ee..77b7bd7e 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -24,6 +24,9 @@ class ExercisesController < ApplicationController 3 end + MAX_EXERCISE_FEEDBACKS = 20 + MAX_COMMENTS_PER_REQUEST = 5 + def java_course_token "702cbd2a-c84c-4b37-923a-692d7d1532d0" @@ -382,7 +385,7 @@ class ExercisesController < ApplicationController # otherwise an internal user could be shown a false rfc here, since current_user.id is polymorphic, but only makes sense for external users when used with rfcs.) # redirect 10 percent pseudorandomly to the feedback page if current_user.respond_to? :external_id - if ((current_user.id + @submission.exercise.created_at.to_i) % 10 == 1) + if (((current_user.id + @submission.exercise.created_at.to_i) % 10 == 1) && @exercise.user_exercise_feedbacks.size <= MAX_EXERCISE_FEEDBACKS) redirect_to_user_feedback return elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise, user_id: current_user.id).first @@ -397,22 +400,28 @@ class ExercisesController < ApplicationController return # else: show open rfc for same exercise if available - elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise).where.not(question: nil).order("RANDOM()").find { | rfc_element |(rfc_element.comments_count < 5) } - # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated. - flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc') - flash.keep(:notice) + elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise).where.not(question: nil).order("RANDOM()").find { | rfc_element |(rfc_element.comments_count < MAX_COMMENTS_PER_REQUEST) } + if(not rfc.nil?) + # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated. + flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc') + flash.keep(:notice) - respond_to do |format| - format.html { redirect_to(rfc) } - format.json { render(json: {redirect: url_for(rfc)}) } + respond_to do |format| + format.html { redirect_to(rfc) } + format.json { render(json: {redirect: url_for(rfc)}) } + end + return end - return end end else # redirect to feedback page if score is less than 100 percent - redirect_to_user_feedback - return + if @exercise.user_exercise_feedbacks.size <= 50 + redirect_to_user_feedback + else + redirect_to_lti_return_path + end + return end redirect_to_lti_return_path end From 01aad0a4a69ddbf9915213c08ca64a75f894a5be Mon Sep 17 00:00:00 2001 From: Thomas Hille Date: Wed, 25 Oct 2017 11:28:27 +0200 Subject: [PATCH 47/69] fixed problem in ProxyExercise that caused: ActiveRecord::AssociationTypeMismatch (Exercise(#51937940) expected, got TrueClass(#7943420)): --- app/models/proxy_exercise.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/proxy_exercise.rb b/app/models/proxy_exercise.rb index 5922e062..82fad35f 100644 --- a/app/models/proxy_exercise.rb +++ b/app/models/proxy_exercise.rb @@ -36,8 +36,8 @@ class ProxyExercise < ActiveRecord::Base Rails.logger.debug("retrieved assigned exercise for user #{user.id}: Exercise #{assigned_user_proxy_exercise.exercise}" ) assigned_user_proxy_exercise.exercise else + Rails.logger.debug("find new matching exercise for user #{user.id}" ) matching_exercise = - Rails.logger.debug("find new matching exercise for user #{user.id}" ) begin find_matching_exercise(user) rescue => e #fallback From 87f280089d76efaf79132fdf91daff572dc43220 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 25 Oct 2017 12:07:16 +0200 Subject: [PATCH 48/69] Fix return value of logger being assigned to exercise --- app/models/proxy_exercise.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/proxy_exercise.rb b/app/models/proxy_exercise.rb index 5922e062..5bdff20d 100644 --- a/app/models/proxy_exercise.rb +++ b/app/models/proxy_exercise.rb @@ -37,8 +37,8 @@ class ProxyExercise < ActiveRecord::Base assigned_user_proxy_exercise.exercise else matching_exercise = - Rails.logger.debug("find new matching exercise for user #{user.id}" ) begin + Rails.logger.debug("find new matching exercise for user #{user.id}" ) find_matching_exercise(user) rescue => e #fallback Rails.logger.error("finding matching exercise failed. Fall back to random exercise! Error: #{$!}" ) @@ -85,8 +85,7 @@ class ProxyExercise < ActiveRecord::Base @reason[:reason] = "easiest exercise in pool. empty potential exercises" select_easiest_exercise(exercises) else - recommended_exercise = select_best_matching_exercise(user, exercises_user_has_accessed, potential_recommended_exercises) - recommended_exercise + select_best_matching_exercise(user, exercises_user_has_accessed, potential_recommended_exercises) end end @@ -238,4 +237,4 @@ class ProxyExercise < ActiveRecord::Base exercises.order(:expected_difficulty).first end -end \ No newline at end of file +end From 0bade2c2e7376eb3f8987ece69e1cf683039276d Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 25 Oct 2017 14:05:10 +0200 Subject: [PATCH 49/69] Fix recommending too difficult questions if user has too low level --- app/models/proxy_exercise.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/proxy_exercise.rb b/app/models/proxy_exercise.rb index 5bdff20d..612fbc59 100644 --- a/app/models/proxy_exercise.rb +++ b/app/models/proxy_exercise.rb @@ -72,7 +72,7 @@ class ProxyExercise < ActiveRecord::Base # find exercises potential_recommended_exercises = [] - exercises.where("expected_difficulty > 1").each do |ex| + exercises.where("expected_difficulty >= 1").each do |ex| ## find exercises which have only tags the user has already seen if (ex.tags - tags_user_has_seen).empty? potential_recommended_exercises << ex From 13e33bf977f073578515fce648899ebbac5f0895 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Thu, 26 Oct 2017 15:42:06 +0200 Subject: [PATCH 50/69] Add optional logging for tests --- config/environments/test.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/config/environments/test.rb b/config/environments/test.rb index 1c19f08b..a5c78bf9 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -39,4 +39,8 @@ Rails.application.configure do # Raises error for missing translations # config.action_view.raise_on_missing_translations = true + + #config.logger = Logger.new(STDOUT) + # Set log level + #config.log_level = :DEBUG end From 14a135a0c9e7e05326c2af564ae508772470d5b3 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Thu, 26 Oct 2017 15:42:20 +0200 Subject: [PATCH 51/69] Add explanatory comment to config --- spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1fa8ecda..ca9f9a63 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -83,5 +83,6 @@ RSpec.configure do |config| mocks.verify_partial_doubles = true end + # Save test results to persistence file to enable usage of --next-failure flag in local testing/debugging config.example_status_persistence_file_path = 'tmp/rspec_persistence_file.txt' end From 34e96e40beedfbd2d531ad973d2c66052b59b1c6 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Thu, 26 Oct 2017 15:43:14 +0200 Subject: [PATCH 52/69] Fix submissions controller test json response --- spec/controllers/submissions_controller_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index d1e489ff..d9ee5316 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -184,13 +184,17 @@ describe SubmissionsController do end describe 'GET #show.json' do + # Render views requested in controller tests in order to get json responses + # https://github.com/rails/jbuilder/issues/32 + render_views + before(:each) { get :show, id: submission.id, format: :json } expect_assigns(submission: :submission) expect_status(200) [:render, :run, :test].each do |action| describe "##{action}_url" do - let(:url) { response.body.send(:"#{action}_url") } + let(:url) { JSON.parse(response.body).with_indifferent_access.fetch("#{action}_url") } it "starts like the #{action} path" do filename = File.basename(__FILE__) From 0fd993c1cd94371ac054df0475eadbe5863a7e06 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Thu, 26 Oct 2017 15:48:21 +0200 Subject: [PATCH 53/69] Move submission url attributes to controller test --- spec/controllers/submissions_controller_spec.rb | 10 ++++++++++ spec/models/submission_spec.rb | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index d9ee5316..98f36cb2 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -206,6 +206,16 @@ describe SubmissionsController do end end end + + [:score, :stop].each do |action| + describe "##{action}_url" do + let(:url) { JSON.parse(response.body).with_indifferent_access.fetch("#{action}_url") } + + it "corresponds to the #{action} path" do + expect(url).to eq(Rails.application.routes.url_helpers.send(:"#{action}_submission_path", submission)) + end + end + end end describe 'GET #score' do diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 75f0cf2a..64f4e49e 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -63,16 +63,6 @@ describe Submission do end end - [:score, :stop].each do |action| - describe "##{action}_url" do - let(:url) { submission.send(:"#{action}_url") } - - it "corresponds to the #{action} path" do - expect(url).to eq(Rails.application.routes.url_helpers.send(:"#{action}_submission_path", submission)) - end - end - end - describe '#siblings' do let(:siblings) { described_class.find_by(user: user).siblings } let(:user) { FactoryGirl.create(:external_user) } From 04baf6c5d55612b3d95864ce7fc98b410db1a97c Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Thu, 26 Oct 2017 16:14:40 +0200 Subject: [PATCH 54/69] Make paths explicit to fix tests --- app/views/exercises/_editor.html.slim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/exercises/_editor.html.slim b/app/views/exercises/_editor.html.slim index 4291028f..e692b748 100644 --- a/app/views/exercises/_editor.html.slim +++ b/app/views/exercises/_editor.html.slim @@ -3,7 +3,7 @@ - consumer_id = @current_user.respond_to?(:external_id) ? @current_user.consumer_id : '' #'tests' #(@current_user.uuid.present? ? @current_user.uuid : '') - show_break_interventions = @show_break_interventions || "false" - show_rfc_interventions = @show_rfc_interventions || "false" -#editor.row data-exercise-id=exercise.id data-message-depleted=t('exercises.editor.depleted') data-message-timeout=t('exercises.editor.timeout', permitted_execution_time: @exercise.execution_environment.permitted_execution_time) data-errors-url=execution_environment_errors_path(exercise.execution_environment) data-submissions-url=submissions_path data-user-id=@current_user.id data-user-external-id=external_user_external_id data-working-times-url=working_times_exercise_path data-intervention-save-url=intervention_exercise_path data-rfc-interventions=show_rfc_interventions data-break-interventions=show_break_interventions data-course_token=@course_token data-search-save-url=search_exercise_path +#editor.row data-exercise-id=@exercise.id data-message-depleted=t('exercises.editor.depleted') data-message-timeout=t('exercises.editor.timeout', permitted_execution_time: @exercise.execution_environment.permitted_execution_time) data-errors-url=execution_environment_errors_path(exercise.execution_environment) data-submissions-url=submissions_path data-user-id=@current_user.id data-user-external-id=external_user_external_id data-working-times-url=working_times_exercise_path(@exercise) data-intervention-save-url=intervention_exercise_path(@exercise) data-rfc-interventions=show_rfc_interventions data-break-interventions=show_break_interventions data-course_token=@course_token data-search-save-url=search_exercise_path(@exercise) div id="sidebar" class=(@exercise.hide_file_tree ? 'sidebar-col-collapsed' : 'sidebar-col') = render('editor_file_tree', exercise: @exercise, files: @files) div id='output_sidebar' class='output-col-collapsed' = render('exercises/editor_output', external_user_id: external_user_id, consumer_id: consumer_id ) div id='frames' class='editor-col' @@ -24,4 +24,4 @@ = render('shared/modal', id: 'comment-modal', title: t('exercises.implement.comment.request'), template: 'exercises/_request_comment_dialogcontent') -= render('shared/modal', id: 'break-intervention-modal', title: t('exercises.implement.break_intervention.title'), template: 'interventions/_break_intervention_modal') \ No newline at end of file += render('shared/modal', id: 'break-intervention-modal', title: t('exercises.implement.break_intervention.title'), template: 'interventions/_break_intervention_modal') From 08c715470806b19ac0ddfe3b1141cf132b23df8e Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 1 Nov 2017 10:14:03 +0100 Subject: [PATCH 55/69] removed duplicate logging statement --- app/models/proxy_exercise.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/proxy_exercise.rb b/app/models/proxy_exercise.rb index 3e5d740c..9c84e3ad 100644 --- a/app/models/proxy_exercise.rb +++ b/app/models/proxy_exercise.rb @@ -39,7 +39,6 @@ class ProxyExercise < ActiveRecord::Base Rails.logger.debug("find new matching exercise for user #{user.id}" ) matching_exercise = begin - Rails.logger.debug("find new matching exercise for user #{user.id}" ) find_matching_exercise(user) rescue => e #fallback Rails.logger.error("finding matching exercise failed. Fall back to random exercise! Error: #{$!}" ) From 97d840955756cc8b8baf05353c8d5d5bbad584bc Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 1 Nov 2017 10:14:52 +0100 Subject: [PATCH 56/69] add association between exercise and user_exercise_feedback --- app/models/exercise.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 58d328ae..b7487399 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -20,6 +20,7 @@ class Exercise < ActiveRecord::Base has_many :exercise_tags has_many :tags, through: :exercise_tags accepts_nested_attributes_for :exercise_tags + has_many :user_exercise_feedbacks has_many :external_users, source: :user, source_type: ExternalUser, through: :submissions has_many :internal_users, source: :user, source_type: InternalUser, through: :submissions From ac14e2d0ca2e41eb4a03ed5add5f75e43a78b054 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 1 Nov 2017 11:57:56 +0100 Subject: [PATCH 57/69] first steps towards testing the redirect with regards to user_feedbacks --- spec/controllers/exercises_controller_spec.rb | 26 +++++++++++++++++++ spec/factories/exercise.rb | 18 +++++++++++++ spec/factories/user_exercise_feedback.rb | 7 +++++ 3 files changed, 51 insertions(+) create mode 100644 spec/factories/user_exercise_feedback.rb diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 43dada43..dca3103b 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -313,4 +313,30 @@ describe ExercisesController do expect_template(:edit) end end + + describe 'redirect after submit' do + + let!(:submission_with_created_at_to_receive_feedback) { FactoryGirl.create(:submission, exercise_id: exercise.id, user_id: user.id, user_type: user.class.name, created_at: (10 - (user.id % 10))) } + + let(:exercise_with_user_feedbacks) { FactoryGirl.build(:dummy_with_user_feedbacks, submission: submission_with_created_at_to_receive_feedback) } + let(:exercise_with_21_user_feedbacks) { FactoryGirl.build(:dummy_with_user_feedbacks, user_exercise_feedback_count: 21) } + + + + it 'with less than maximal user feedbacks' do + exercise_with_user_feedbacks.send(:redirect_after_submit) + expect_redirect(new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: @exercise.id})) + end + + it 'with more than maximal user feedbacks' do + exercise_with_user_feedbacks.send(:redirect_after_submit) + expect_redirect(new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: @exercise.id})) + end + + + + expect_redirect(Exercise.last) + + end + end diff --git a/spec/factories/exercise.rb b/spec/factories/exercise.rb index ea588faf..9d8ac5b1 100644 --- a/spec/factories/exercise.rb +++ b/spec/factories/exercise.rb @@ -38,6 +38,24 @@ FactoryGirl.define do association :execution_environment, factory: :ruby instructions title 'Dummy' + + factory :dummy_with_user_feedbacks do + # user_feedbacks_count is declared as a transient attribute and available in + # attributes on the factory, as well as the callback via the evaluator + transient do + user_feedbacks_count 5 + end + + # the after(:create) yields two values; the exercise instance itself and the + # evaluator, which stores all values from the factory, including transient + # attributes; `create_list`'s second argument is the number of records + # to create and we make sure the user_exercise_feedback is associated properly to the exercise + after(:create) do |exercise, evaluator| + create_list(:user_exercise_feedback, evaluator.user_feedbacks_count, exercise: exercise) + end + + end + end factory :even_odd, class: Exercise do diff --git a/spec/factories/user_exercise_feedback.rb b/spec/factories/user_exercise_feedback.rb new file mode 100644 index 00000000..b5a376cd --- /dev/null +++ b/spec/factories/user_exercise_feedback.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :user_exercise_feedback, class: UserExerciseFeedback do + created_by_external_user + feedback_text 'Most suitable exercise ever' + end + +end \ No newline at end of file From 3608712706c8ef0c503c09c1deee4967506e1f4b Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 6 Nov 2017 15:38:50 +0100 Subject: [PATCH 58/69] forgot some minor changes in spec.. (still work in progress) --- spec/controllers/exercises_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index dca3103b..b509a33f 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -316,10 +316,10 @@ describe ExercisesController do describe 'redirect after submit' do - let!(:submission_with_created_at_to_receive_feedback) { FactoryGirl.create(:submission, exercise_id: exercise.id, user_id: user.id, user_type: user.class.name, created_at: (10 - (user.id % 10))) } + let!(:submission_with_created_at_to_receive_feedback) { FactoryGirl.create(:submission, exercise_id: exercise.id, user_id: user.id, user_type: user.class.name, created_at: (11 - (user.id % 10))) } let(:exercise_with_user_feedbacks) { FactoryGirl.build(:dummy_with_user_feedbacks, submission: submission_with_created_at_to_receive_feedback) } - let(:exercise_with_21_user_feedbacks) { FactoryGirl.build(:dummy_with_user_feedbacks, user_exercise_feedback_count: 21) } + let(:exercise_with_21_user_feedbacks) { FactoryGirl.build(:dummy_with_user_feedbacks, user_exercise_feedback_count: 21, submission: submission_with_created_at_to_receive_feedback)) } From c4cf11f29944074ab260be2d913ad7bac2e391ff Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 8 Nov 2017 15:39:04 +0100 Subject: [PATCH 59/69] Refactor user redirect after submission --- app/controllers/exercises_controller.rb | 33 ++++++++++++------------- app/models/exercise.rb | 6 +++++ app/models/submission.rb | 14 +++++++++++ 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 77b7bd7e..148f0178 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -24,10 +24,6 @@ class ExercisesController < ApplicationController 3 end - MAX_EXERCISE_FEEDBACKS = 20 - MAX_COMMENTS_PER_REQUEST = 5 - - def java_course_token "702cbd2a-c84c-4b37-923a-692d7d1532d0" end @@ -380,15 +376,18 @@ class ExercisesController < ApplicationController def redirect_after_submit Rails.logger.debug('Redirecting user with score:s ' + @submission.normalized_score.to_s) - if @submission.normalized_score == 1.0 + if submission.normalized_score == 1.0 # if user is external and has an own rfc, redirect to it and message him to clean up and accept the answer. (we need to check that the user is external, # otherwise an internal user could be shown a false rfc here, since current_user.id is polymorphic, but only makes sense for external users when used with rfcs.) # redirect 10 percent pseudorandomly to the feedback page if current_user.respond_to? :external_id - if (((current_user.id + @submission.exercise.created_at.to_i) % 10 == 1) && @exercise.user_exercise_feedbacks.size <= MAX_EXERCISE_FEEDBACKS) + if @submission.redirect_to_feedback? redirect_to_user_feedback return - elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise, user_id: current_user.id).first + end + + rfc = @submission.own_unsolved_rfc + if rfc # set a message that informs the user that his own RFC should be closed. flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_own_rfc') flash.keep(:notice) @@ -398,20 +397,20 @@ class ExercisesController < ApplicationController format.json { render(json: {redirect: url_for(rfc)}) } end return + end # else: show open rfc for same exercise if available - elsif rfc = RequestForComment.unsolved.where(exercise_id: @submission.exercise).where.not(question: nil).order("RANDOM()").find { | rfc_element |(rfc_element.comments_count < MAX_COMMENTS_PER_REQUEST) } - if(not rfc.nil?) - # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated. - flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc') - flash.keep(:notice) + rfc = @submission.unsolved_rfc + unless rfc.nil? + # set a message that informs the user that his score was perfect and help in RFC is greatly appreciated. + flash[:notice] = I18n.t('exercises.submit.full_score_redirect_to_rfc') + flash.keep(:notice) - respond_to do |format| - format.html { redirect_to(rfc) } - format.json { render(json: {redirect: url_for(rfc)}) } - end - return + respond_to do |format| + format.html {redirect_to(rfc)} + format.json {render(json: {redirect: url_for(rfc)})} end + return end end else diff --git a/app/models/exercise.rb b/app/models/exercise.rb index b7487399..06ed391f 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -37,6 +37,8 @@ class Exercise < ActiveRecord::Base @working_time_statistics = nil + MAX_EXERCISE_FEEDBACKS = 20 + def average_percentage if average_score and maximum_score != 0.0 and submissions.exists?(cause: 'submit') @@ -362,4 +364,8 @@ class Exercise < ActiveRecord::Base end private :valid_main_file? + def needs_more_feedback + user_exercise_feedbacks.size <= MAX_EXERCISE_FEEDBACKS + end + end diff --git a/app/models/submission.rb b/app/models/submission.rb index 69a48307..38ea16d6 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -18,6 +18,8 @@ class Submission < ActiveRecord::Base validates :cause, inclusion: {in: CAUSES} validates :exercise_id, presence: true + MAX_COMMENTS_ON_RECOMMENDED_RFC = 5 + def build_files_hash(files, attribute) files.map(&attribute.to_proc).zip(files).to_h end @@ -53,4 +55,16 @@ class Submission < ActiveRecord::Base def to_s Submission.model_name.human end + + def redirect_to_feedback? + ((user_id + exercise.created_at.to_i) % 10 == 1) && exercise.needs_more_feedback + end + + def own_unsolved_rfc + RequestForComment.unsolved.where(exercise_id: exercise, user_id: user_id).first + end + + def unsolved_rfc + RequestForComment.unsolved.where(exercise_id: exercise).where.not(question: nil).order("RANDOM()").find { | rfc_element |(rfc_element.comments_count < MAX_COMMENTS_ON_RECOMMENDED_RFC) } + end end From 03141409e8bbeb6716248ebc5ae7275665a0163d Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 8 Nov 2017 15:41:48 +0100 Subject: [PATCH 60/69] Fix tests for user_exercise_feedback --- spec/controllers/exercises_controller_spec.rb | 26 ---------- spec/models/submission_spec.rb | 49 +++++++++++++++++++ 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index b509a33f..43dada43 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -313,30 +313,4 @@ describe ExercisesController do expect_template(:edit) end end - - describe 'redirect after submit' do - - let!(:submission_with_created_at_to_receive_feedback) { FactoryGirl.create(:submission, exercise_id: exercise.id, user_id: user.id, user_type: user.class.name, created_at: (11 - (user.id % 10))) } - - let(:exercise_with_user_feedbacks) { FactoryGirl.build(:dummy_with_user_feedbacks, submission: submission_with_created_at_to_receive_feedback) } - let(:exercise_with_21_user_feedbacks) { FactoryGirl.build(:dummy_with_user_feedbacks, user_exercise_feedback_count: 21, submission: submission_with_created_at_to_receive_feedback)) } - - - - it 'with less than maximal user feedbacks' do - exercise_with_user_feedbacks.send(:redirect_after_submit) - expect_redirect(new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: @exercise.id})) - end - - it 'with more than maximal user feedbacks' do - exercise_with_user_feedbacks.send(:redirect_after_submit) - expect_redirect(new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: @exercise.id})) - end - - - - expect_redirect(Exercise.last) - - end - end diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 64f4e49e..227c6b99 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -85,4 +85,53 @@ describe Submission do expect(submission.to_s).to eq(described_class.model_name.human) end end + + describe '#redirect_to_feedback?' do + + context 'with no exercise feedback' do + let(:exercise) {FactoryGirl.create(:dummy)} + let(:user) {FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) % 10)} + let(:submission) {FactoryGirl.build(:submission, exercise: exercise, user: user)} + + it 'sends 10% of users to feedback page' do + expect(submission.send(:redirect_to_feedback?)).to be_truthy + end + + it 'does not redirect other users' do + 9.times do |i| + submission = FactoryGirl.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) + expect(submission.send(:redirect_to_feedback?)).to be_falsey + end + end + end + + context 'with little exercise feedback' do + let(:exercise) {FactoryGirl.create(:dummy_with_user_feedbacks)} + let(:user) {FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) % 10)} + let(:submission) {FactoryGirl.build(:submission, exercise: exercise, user: user)} + + it 'sends 10% of users to feedback page' do + expect(submission.send(:redirect_to_feedback?)).to be_truthy + end + + it 'does not redirect other users' do + 9.times do |i| + submission = FactoryGirl.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) + expect(submission.send(:redirect_to_feedback?)).to be_falsey + end + end + end + + context 'with enough exercise feedback' do + let(:exercise) {FactoryGirl.create(:dummy_with_user_feedbacks, user_feedbacks_count: 42)} + let(:user) {FactoryGirl.create(:external_user)} + + it 'sends nobody to feedback page' do + 30.times do |i| + submission = FactoryGirl.create(:submission, exercise: exercise, user: FactoryGirl.create(:external_user)) + expect(submission.send(:redirect_to_feedback?)).to be_falsey + end + end + end + end end From 2118e610f91e510edf7749943684ffe30d6a1fd1 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 8 Nov 2017 17:06:19 +0100 Subject: [PATCH 61/69] fixed broken migration --- db/migrate/20160204111716_add_user_to_code_harbor_link.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20160204111716_add_user_to_code_harbor_link.rb b/db/migrate/20160204111716_add_user_to_code_harbor_link.rb index 8fa36ed1..fe509666 100644 --- a/db/migrate/20160204111716_add_user_to_code_harbor_link.rb +++ b/db/migrate/20160204111716_add_user_to_code_harbor_link.rb @@ -1,5 +1,5 @@ class AddUserToCodeHarborLink < ActiveRecord::Migration def change - add_reference :code_harbor_links, :user, index: true, foreign_key: true + add_reference :code_harbor_links, :user, polymorphic: true, index: true end end From 69250901a1f57ddf654f1bc43e031dd6b6ef36f4 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 8 Nov 2017 21:24:46 +0100 Subject: [PATCH 62/69] Fix typo --- app/controllers/exercises_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 148f0178..92bc5b97 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -376,7 +376,7 @@ class ExercisesController < ApplicationController def redirect_after_submit Rails.logger.debug('Redirecting user with score:s ' + @submission.normalized_score.to_s) - if submission.normalized_score == 1.0 + if @submission.normalized_score == 1.0 # if user is external and has an own rfc, redirect to it and message him to clean up and accept the answer. (we need to check that the user is external, # otherwise an internal user could be shown a false rfc here, since current_user.id is polymorphic, but only makes sense for external users when used with rfcs.) # redirect 10 percent pseudorandomly to the feedback page From c5c42ca3bc60c933364b5d24d116e226b303d62e Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 8 Nov 2017 22:31:01 +0100 Subject: [PATCH 63/69] Show list of feedback messages for specific exercises --- app/assets/stylesheets/exercises.css.scss | 14 +++++++++++++- app/controllers/exercises_controller.rb | 7 ++++++- app/models/exercise.rb | 1 + app/policies/exercise_policy.rb | 2 +- app/views/exercises/feedback.html.slim | 15 +++++++++++++++ app/views/exercises/index.html.slim | 1 + config/locales/de.yml | 1 + config/locales/en.yml | 1 + config/routes.rb | 1 + 9 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 app/views/exercises/feedback.html.slim diff --git a/app/assets/stylesheets/exercises.css.scss b/app/assets/stylesheets/exercises.css.scss index 73c0b26e..e46d59af 100644 --- a/app/assets/stylesheets/exercises.css.scss +++ b/app/assets/stylesheets/exercises.css.scss @@ -93,4 +93,16 @@ a.file-heading { margin: -1px 0 0 0; top: 100%; left: 0; -} \ No newline at end of file +} + +.feedback { + .text { + margin-bottom: 10px; + } + .difficulty { + font-weight: bold; + } + .worktime { + font-weight: bold; + } +} diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 141b47ee..969e5e4f 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -6,7 +6,7 @@ class ExercisesController < ApplicationController before_action :handle_file_uploads, only: [:create, :update] before_action :set_execution_environments, only: [:create, :edit, :new, :update] - before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :implement, :working_times, :intervention, :search, :run, :statistics, :submit, :reload] + before_action :set_exercise, only: MEMBER_ACTIONS + [:clone, :implement, :working_times, :intervention, :search, :run, :statistics, :submit, :reload, :feedback] before_action :set_external_user, only: [:statistics] before_action :set_file_types, only: [:create, :edit, :new, :update] before_action :set_course_token, only: [:implement] @@ -90,6 +90,11 @@ class ExercisesController < ApplicationController collect_set_and_unset_exercise_tags end + def feedback + authorize! + @feedbacks = @exercise.user_exercise_feedbacks.paginate(page: params[:page]) + end + def import_proforma_xml begin user = user_for_oauth2_request() diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 58d328ae..b7487399 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -20,6 +20,7 @@ class Exercise < ActiveRecord::Base has_many :exercise_tags has_many :tags, through: :exercise_tags accepts_nested_attributes_for :exercise_tags + has_many :user_exercise_feedbacks 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/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index 54d22b87..e4563832 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -12,7 +12,7 @@ class ExercisePolicy < AdminOrAuthorPolicy @user.internal_user? end - [:clone?, :destroy?, :edit?, :statistics?, :update?].each do |action| + [:clone?, :destroy?, :edit?, :statistics?, :update?, :feedback?].each do |action| define_method(action) { admin? || author?} end diff --git a/app/views/exercises/feedback.html.slim b/app/views/exercises/feedback.html.slim new file mode 100644 index 00000000..07b88167 --- /dev/null +++ b/app/views/exercises/feedback.html.slim @@ -0,0 +1,15 @@ +h1 = @exercise + +ul.list-unstyled.panel-group#files + - @feedbacks.each do |feedback| + li.panel.panel-default + .panel-heading role="tab" id="heading" + div.clearfix + span = feedback.user.name + .panel-collapse role="tabpanel" + .panel-body.feedback + .text = feedback.feedback_text + .difficulty = "#{t('user_exercise_feedback.difficulty')} #{feedback.difficulty}" if feedback.difficulty + .worktime = "#{t('user_exercise_feedback.working_time')} #{feedback.user_estimated_worktime}" if feedback.user_estimated_worktime + += render('shared/pagination', collection: @feedbacks) diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index bd8fe880..18c2cd98 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -47,6 +47,7 @@ h1 = Exercise.model_name.human(count: 2) span.sr-only Toggle Dropdown ul.dropdown-menu.pull-right role="menu" li = link_to(t('shared.show'), exercise) if policy(exercise).show? + li = link_to(t('activerecord.models.user_exercise_feedback.other'), feedback_exercise_path(exercise)) if policy(exercise).feedback? li = link_to(t('shared.destroy'), exercise, data: {confirm: t('shared.confirm_destroy')}, method: :delete) if policy(exercise).destroy? li = link_to(t('.clone'), clone_exercise_path(exercise), data: {confirm: t('shared.confirm_destroy')}, method: :post) if policy(exercise).clone? diff --git a/config/locales/de.yml b/config/locales/de.yml index afeea353..a7a2b436 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -328,6 +328,7 @@ de: clone: Duplizieren implement: Implementieren test_files: Test-Dateien + feedback: Feedback statistics: average_score: Durchschnittliche Punktzahl final_submissions: Finale Abgaben diff --git a/config/locales/en.yml b/config/locales/en.yml index 6c1d91fd..7eba1527 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -328,6 +328,7 @@ en: clone: Duplicate implement: Implement test_files: Test Files + feedback: Feedback statistics: average_score: Average Score final_submissions: Final Submissions diff --git a/config/routes.rb b/config/routes.rb index b6fef404..264e7471 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -76,6 +76,7 @@ Rails.application.routes.draw do post :intervention post :search get :statistics + get :feedback get :reload post :submit end From 6161d6caaf1aba8a5c908bf17540e9697c1a0162 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 15 Nov 2017 14:17:55 +0100 Subject: [PATCH 64/69] clean up code, use method instead of magic constant. --- app/controllers/exercises_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 92bc5b97..85ef3dbf 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -415,7 +415,7 @@ class ExercisesController < ApplicationController end else # redirect to feedback page if score is less than 100 percent - if @exercise.user_exercise_feedbacks.size <= 50 + if @exercise.needs_more_feedback? redirect_to_user_feedback else redirect_to_lti_return_path From 0e26ab98c8cfcea8c6450147e3e579b05e2218ef Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 15 Nov 2017 15:18:07 +0100 Subject: [PATCH 65/69] rename factory_girl_(rails) to factory_bot_(rails) --- db/seeds.rb | 4 +-- db/seeds/development.rb | 10 +++--- db/seeds/production.rb | 4 +-- spec/concerns/lti_spec.rb | 20 +++++------ spec/concerns/submission_scoring_spec.rb | 4 +-- .../admin/dashboard_controller_spec.rb | 2 +- .../application_controller_spec.rb | 4 +-- .../code_ocean/files_controller_spec.rb | 10 +++--- spec/controllers/consumers_controller_spec.rb | 12 +++---- .../execution_environments_controller_spec.rb | 12 +++---- spec/controllers/exercises_controller_spec.rb | 28 +++++++-------- .../external_users_controller_spec.rb | 4 +-- .../controllers/file_types_controller_spec.rb | 12 +++---- spec/controllers/hints_controller_spec.rb | 14 ++++---- .../internal_users_controller_spec.rb | 12 +++---- spec/controllers/sessions_controller_spec.rb | 18 +++++----- .../submissions_controller_spec.rb | 14 ++++---- spec/factories/code_ocean/file.rb | 2 +- spec/factories/consumer.rb | 2 +- spec/factories/error.rb | 2 +- spec/factories/execution_environment.rb | 2 +- spec/factories/exercise.rb | 4 +-- spec/factories/external_user.rb | 2 +- spec/factories/file_type.rb | 2 +- spec/factories/hint.rb | 2 +- spec/factories/internal_user.rb | 2 +- spec/factories/lti_parameter.rb | 4 +-- spec/factories/proxy_exercise.rb | 2 +- spec/factories/shared_traits.rb | 2 +- spec/factories/submission.rb | 2 +- spec/factories/user_exercise_feedback.rb | 4 +-- spec/features/authentication_spec.rb | 4 +-- spec/features/authorization_spec.rb | 6 ++-- spec/features/editor_spec.rb | 6 ++-- spec/features/factories_spec.rb | 2 +- spec/helpers/admin/dashboard_helper_spec.rb | 2 +- spec/helpers/exercise_helper_spec.rb | 2 +- spec/lib/assessor_spec.rb | 4 +-- spec/lib/docker_client_spec.rb | 8 ++--- spec/lib/docker_container_pool_spec.rb | 2 +- spec/lib/file_tree_spec.rb | 18 +++++----- spec/lib/whistleblower_spec.rb | 2 +- spec/mailers/user_mailer_spec.rb | 2 +- spec/models/consumer_spec.rb | 2 +- spec/models/execution_environment_spec.rb | 10 +++--- spec/models/exercise_spec.rb | 14 ++++---- spec/models/external_user_spec.rb | 4 +-- spec/models/internal_user_spec.rb | 16 ++++----- spec/models/submission_spec.rb | 34 +++++++++---------- spec/policies/admin/dashboard_policy_spec.rb | 6 ++-- spec/policies/code_ocean/file_policy_spec.rb | 16 ++++----- spec/policies/consumer_policy_spec.rb | 4 +-- spec/policies/error_policy_spec.rb | 12 +++---- .../execution_environment_policy_spec.rb | 16 ++++----- spec/policies/exercise_policy_spec.rb | 28 +++++++-------- spec/policies/external_user_policy_spec.rb | 4 +-- spec/policies/file_type_policy_spec.rb | 12 +++---- spec/policies/hint_policy_spec.rb | 12 +++---- spec/policies/internal_user_policy_spec.rb | 10 +++--- spec/policies/submission_policy_spec.rb | 12 +++---- spec/support/docker.rb | 2 +- spec/uploaders/file_uploader_spec.rb | 2 +- .../shell.html.slim_spec.rb | 2 +- .../exercises/implement.html.slim_spec.rb | 4 +-- test/factories/error_template_attributes.rb | 2 +- test/factories/error_templates.rb | 2 +- test/factories/structured_error_attributes.rb | 2 +- test/factories/structured_errors.rb | 2 +- 68 files changed, 253 insertions(+), 253 deletions(-) diff --git a/db/seeds.rb b/db/seeds.rb index 823a6358..28d543ef 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,5 +1,5 @@ def find_factories_by_class(klass) - FactoryGirl.factories.select do |factory| + FactoryBot.factories.select do |factory| factory.instance_variable_get(:@class_name) == klass || factory.instance_variable_get(:@name) == klass.model_name.singular.to_sym end end @@ -9,7 +9,7 @@ module ActiveRecord [:build, :create].each do |strategy| define_singleton_method("#{strategy}_factories") do |attributes = {}| find_factories_by_class(self).map(&:name).map do |factory_name| - FactoryGirl.send(strategy, factory_name, attributes) + FactoryBot.send(strategy, factory_name, attributes) end end end diff --git a/db/seeds/development.rb b/db/seeds/development.rb index 6fc8e050..b0c550c0 100644 --- a/db/seeds/development.rb +++ b/db/seeds/development.rb @@ -1,9 +1,9 @@ # consumers -FactoryGirl.create(:consumer) -FactoryGirl.create(:consumer, name: 'openSAP') +FactoryBot.create(:consumer) +FactoryBot.create(:consumer, name: 'openSAP') # users -[:admin, :external_user, :teacher].each { |factory_name| FactoryGirl.create(factory_name) } +[:admin, :external_user, :teacher].each { |factory_name| FactoryBot.create(factory_name) } # execution environments ExecutionEnvironment.create_factories @@ -12,7 +12,7 @@ ExecutionEnvironment.create_factories Error.create_factories # exercises -@exercises = find_factories_by_class(Exercise).map(&:name).map { |factory_name| [factory_name, FactoryGirl.create(factory_name)] }.to_h +@exercises = find_factories_by_class(Exercise).map(&:name).map { |factory_name| [factory_name, FactoryBot.create(factory_name)] }.to_h # file types FileType.create_factories @@ -21,4 +21,4 @@ FileType.create_factories Hint.create_factories # submissions -FactoryGirl.create(:submission, exercise: @exercises[:fibonacci]) +FactoryBot.create(:submission, exercise: @exercises[:fibonacci]) diff --git a/db/seeds/production.rb b/db/seeds/production.rb index 301becfd..5b63f57b 100644 --- a/db/seeds/production.rb +++ b/db/seeds/production.rb @@ -1,7 +1,7 @@ require 'highline/import' # consumers -FactoryGirl.create(:consumer) +FactoryBot.create(:consumer) # users email = ask('Enter admin email: ') @@ -11,7 +11,7 @@ passwords = ['password', 'password confirmation'].map do |attribute| end if passwords.uniq.length == 1 - FactoryGirl.create(:admin, email: email, name: 'Administrator', password: passwords.first) + FactoryBot.create(:admin, email: email, name: 'Administrator', password: passwords.first) else abort('Passwords do not match!') end diff --git a/spec/concerns/lti_spec.rb b/spec/concerns/lti_spec.rb index 32d740aa..a7b1dfcc 100644 --- a/spec/concerns/lti_spec.rb +++ b/spec/concerns/lti_spec.rb @@ -11,7 +11,7 @@ describe Lti do describe '#build_tool_provider' do it 'instantiates a tool provider' do expect(IMS::LTI::ToolProvider).to receive(:new) - controller.send(:build_tool_provider, consumer: FactoryGirl.build(:consumer), parameters: {}) + controller.send(:build_tool_provider, consumer: FactoryBot.build(:consumer), parameters: {}) end end @@ -95,10 +95,10 @@ describe Lti do end describe '#send_score' do - let(:consumer) { FactoryGirl.create(:consumer) } + let(:consumer) { FactoryBot.create(:consumer) } let(:score) { 0.5 } - let(:submission) { FactoryGirl.create(:submission) } - let!(:lti_parameter) { FactoryGirl.create(:lti_parameter)} + let(:submission) { FactoryBot.create(:submission) } + let!(:lti_parameter) { FactoryBot.create(:lti_parameter)} context 'with an invalid score' do it 'raises an exception' do @@ -159,18 +159,18 @@ describe Lti do let(:parameters) { {} } it 'stores data in the session' do - controller.instance_variable_set(:@current_user, FactoryGirl.create(:external_user)) - controller.instance_variable_set(:@exercise, FactoryGirl.create(:fibonacci)) + controller.instance_variable_set(:@current_user, FactoryBot.create(:external_user)) + controller.instance_variable_set(:@exercise, FactoryBot.create(:fibonacci)) expect(controller.session).to receive(:[]=).with(:consumer_id, anything) expect(controller.session).to receive(:[]=).with(:external_user_id, anything) - controller.send(:store_lti_session_data, consumer: FactoryGirl.build(:consumer), parameters: parameters) + controller.send(:store_lti_session_data, consumer: FactoryBot.build(:consumer), parameters: parameters) end it 'it creates an LtiParameter Object' do before_count = LtiParameter.count - controller.instance_variable_set(:@current_user, FactoryGirl.create(:external_user)) - controller.instance_variable_set(:@exercise, FactoryGirl.create(:fibonacci)) - controller.send(:store_lti_session_data, consumer: FactoryGirl.build(:consumer), parameters: parameters) + controller.instance_variable_set(:@current_user, FactoryBot.create(:external_user)) + controller.instance_variable_set(:@exercise, FactoryBot.create(:fibonacci)) + controller.send(:store_lti_session_data, consumer: FactoryBot.build(:consumer), parameters: parameters) expect(LtiParameter.count).to eq(before_count + 1) end end diff --git a/spec/concerns/submission_scoring_spec.rb b/spec/concerns/submission_scoring_spec.rb index 1b42a947..c32fe9c3 100644 --- a/spec/concerns/submission_scoring_spec.rb +++ b/spec/concerns/submission_scoring_spec.rb @@ -6,8 +6,8 @@ end describe SubmissionScoring do let(:controller) { Controller.new } - before(:all) { @submission = FactoryGirl.create(:submission, cause: 'submit') } - before(:each) { controller.instance_variable_set(:@current_user, FactoryGirl.create(:external_user)) } + before(:all) { @submission = FactoryBot.create(:submission, cause: 'submit') } + before(:each) { controller.instance_variable_set(:@current_user, FactoryBot.create(:external_user)) } describe '#collect_test_results' do after(:each) { controller.send(:collect_test_results, @submission) } diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb index d62bb0ee..20265e16 100644 --- a/spec/controllers/admin/dashboard_controller_spec.rb +++ b/spec/controllers/admin/dashboard_controller_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe Admin::DashboardController do - before(:each) { allow(controller).to receive(:current_user).and_return(FactoryGirl.build(:admin)) } + before(:each) { allow(controller).to receive(:current_user).and_return(FactoryBot.build(:admin)) } describe 'GET #show' do describe 'with format HTML' do diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 0ad4b20d..eec0abbe 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe ApplicationController do describe '#current_user' do context 'with an external user' do - let(:external_user) { FactoryGirl.create(:external_user) } + let(:external_user) { FactoryBot.create(:external_user) } before(:each) { session[:external_user_id] = external_user.id } it 'returns the external user' do @@ -12,7 +12,7 @@ describe ApplicationController do end context 'without an external user' do - let(:internal_user) { FactoryGirl.create(:teacher) } + let(:internal_user) { FactoryBot.create(:teacher) } before(:each) { login_user(internal_user) } it 'returns the internal user' do diff --git a/spec/controllers/code_ocean/files_controller_spec.rb b/spec/controllers/code_ocean/files_controller_spec.rb index 08062acb..9bed884b 100644 --- a/spec/controllers/code_ocean/files_controller_spec.rb +++ b/spec/controllers/code_ocean/files_controller_spec.rb @@ -1,14 +1,14 @@ require 'rails_helper' describe CodeOcean::FilesController do - let(:user) { FactoryGirl.create(:admin) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'POST #create' do - let(:submission) { FactoryGirl.create(:submission, user: user) } + let(:submission) { FactoryBot.create(:submission, user: user) } context 'with a valid file' do - let(:request) { proc { post :create, code_ocean_file: FactoryGirl.build(:file, context: submission).attributes, format: :json } } + let(:request) { proc { post :create, code_ocean_file: FactoryBot.build(:file, context: submission).attributes, format: :json } } before(:each) { request.call } expect_assigns(file: CodeOcean::File) @@ -31,14 +31,14 @@ describe CodeOcean::FilesController do end describe 'DELETE #destroy' do - let(:exercise) { FactoryGirl.create(:fibonacci) } + let(:exercise) { FactoryBot.create(:fibonacci) } let(:request) { proc { delete :destroy, id: exercise.files.first.id } } before(:each) { request.call } expect_assigns(file: CodeOcean::File) it 'destroys the file' do - FactoryGirl.create(:fibonacci) + FactoryBot.create(:fibonacci) expect { request.call }.to change(CodeOcean::File, :count).by(-1) end diff --git a/spec/controllers/consumers_controller_spec.rb b/spec/controllers/consumers_controller_spec.rb index 1060c845..448f13dc 100644 --- a/spec/controllers/consumers_controller_spec.rb +++ b/spec/controllers/consumers_controller_spec.rb @@ -1,13 +1,13 @@ require 'rails_helper' describe ConsumersController do - let(:consumer) { FactoryGirl.create(:consumer) } - let(:user) { FactoryGirl.create(:admin) } + let(:consumer) { FactoryBot.create(:consumer) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'POST #create' do context 'with a valid consumer' do - let(:request) { proc { post :create, consumer: FactoryGirl.attributes_for(:consumer) } } + let(:request) { proc { post :create, consumer: FactoryBot.attributes_for(:consumer) } } before(:each) { request.call } expect_assigns(consumer: Consumer) @@ -34,7 +34,7 @@ describe ConsumersController do expect_assigns(consumer: Consumer) it 'destroys the consumer' do - consumer = FactoryGirl.create(:consumer) + consumer = FactoryBot.create(:consumer) expect { delete :destroy, id: consumer.id }.to change(Consumer, :count).by(-1) end @@ -50,7 +50,7 @@ describe ConsumersController do end describe 'GET #index' do - let!(:consumers) { FactoryGirl.create_pair(:consumer) } + let!(:consumers) { FactoryBot.create_pair(:consumer) } before(:each) { get :index } expect_assigns(consumers: Consumer.all) @@ -76,7 +76,7 @@ describe ConsumersController do describe 'PUT #update' do context 'with a valid consumer' do - before(:each) { put :update, consumer: FactoryGirl.attributes_for(:consumer), id: consumer.id } + before(:each) { put :update, consumer: FactoryBot.attributes_for(:consumer), id: consumer.id } expect_assigns(consumer: Consumer) expect_redirect(:consumer) diff --git a/spec/controllers/execution_environments_controller_spec.rb b/spec/controllers/execution_environments_controller_spec.rb index 2abf59ef..ee9ba3d5 100644 --- a/spec/controllers/execution_environments_controller_spec.rb +++ b/spec/controllers/execution_environments_controller_spec.rb @@ -1,15 +1,15 @@ require 'rails_helper' describe ExecutionEnvironmentsController do - let(:execution_environment) { FactoryGirl.create(:ruby) } - let(:user) { FactoryGirl.create(:admin) } + let(:execution_environment) { FactoryBot.create(:ruby) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'POST #create' do before(:each) { expect(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) } context 'with a valid execution environment' do - let(:request) { proc { post :create, execution_environment: FactoryGirl.attributes_for(:ruby) } } + let(:request) { proc { post :create, execution_environment: FactoryBot.attributes_for(:ruby) } } before(:each) { request.call } expect_assigns(docker_images: Array) @@ -37,7 +37,7 @@ describe ExecutionEnvironmentsController do expect_assigns(execution_environment: :execution_environment) it 'destroys the execution environment' do - execution_environment = FactoryGirl.create(:ruby) + execution_environment = FactoryBot.create(:ruby) expect { delete :destroy, id: execution_environment.id }.to change(ExecutionEnvironment, :count).by(-1) end @@ -72,7 +72,7 @@ describe ExecutionEnvironmentsController do end describe 'GET #index' do - before(:all) { FactoryGirl.create_pair(:ruby) } + before(:all) { FactoryBot.create_pair(:ruby) } before(:each) { get :index } expect_assigns(execution_environments: ExecutionEnvironment.all) @@ -150,7 +150,7 @@ describe ExecutionEnvironmentsController do context 'with a valid execution environment' do before(:each) do expect(DockerClient).to receive(:image_tags).at_least(:once).and_return([]) - put :update, execution_environment: FactoryGirl.attributes_for(:ruby), id: execution_environment.id + put :update, execution_environment: FactoryBot.attributes_for(:ruby), id: execution_environment.id end expect_assigns(docker_images: Array) diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 43dada43..f1d5db96 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -1,8 +1,8 @@ require 'rails_helper' describe ExercisesController do - let(:exercise) { FactoryGirl.create(:dummy) } - let(:user) { FactoryGirl.create(:admin) } + let(:exercise) { FactoryBot.create(:dummy) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'PUT #batch_update' do @@ -52,7 +52,7 @@ describe ExercisesController do end describe 'POST #create' do - let(:exercise_attributes) { FactoryGirl.build(:dummy).attributes } + let(:exercise_attributes) { FactoryBot.build(:dummy).attributes } context 'with a valid exercise' do let(:request) { proc { post :create, exercise: exercise_attributes } } @@ -71,7 +71,7 @@ describe ExercisesController do let(:request) { proc { post :create, exercise: exercise_attributes.merge(files_attributes: files_attributes) } } context 'when specifying the file content within the form' do - let(:files_attributes) { {'0' => FactoryGirl.build(:file).attributes} } + let(:files_attributes) { {'0' => FactoryBot.build(:file).attributes} } it 'creates the file' do expect { request.call }.to change(CodeOcean::File, :count) @@ -79,11 +79,11 @@ describe ExercisesController do end context 'when uploading a file' do - let(:files_attributes) { {'0' => FactoryGirl.build(:file, file_type: file_type).attributes.merge(content: uploaded_file)} } + let(:files_attributes) { {'0' => FactoryBot.build(:file, file_type: file_type).attributes.merge(content: uploaded_file)} } context 'when uploading a binary file' do let(:file_path) { Rails.root.join('db', 'seeds', 'audio_video', 'devstories.mp4') } - let(:file_type) { FactoryGirl.create(:dot_mp4) } + let(:file_type) { FactoryBot.create(:dot_mp4) } let(:uploaded_file) { Rack::Test::UploadedFile.new(file_path, 'video/mp4', true) } it 'creates the file' do @@ -98,7 +98,7 @@ describe ExercisesController do context 'when uploading a non-binary file' do let(:file_path) { Rails.root.join('db', 'seeds', 'fibonacci', 'exercise.rb') } - let(:file_type) { FactoryGirl.create(:dot_rb) } + let(:file_type) { FactoryBot.create(:dot_rb) } let(:uploaded_file) { Rack::Test::UploadedFile.new(file_path, 'text/x-ruby', false) } it 'creates the file' do @@ -128,7 +128,7 @@ describe ExercisesController do expect_assigns(exercise: :exercise) it 'destroys the exercise' do - exercise = FactoryGirl.create(:dummy) + exercise = FactoryBot.create(:dummy) expect { delete :destroy, id: exercise.id }.to change(Exercise, :count).by(-1) end @@ -147,13 +147,13 @@ describe ExercisesController do let(:request) { proc { get :implement, id: exercise.id } } context 'with an exercise with visible files' do - let(:exercise) { FactoryGirl.create(:fibonacci) } + let(:exercise) { FactoryBot.create(:fibonacci) } before(:each) { request.call } expect_assigns(exercise: :exercise) context 'with an existing submission' do - let!(:submission) { FactoryGirl.create(:submission, exercise_id: exercise.id, user_id: user.id, user_type: user.class.name) } + let!(:submission) { FactoryBot.create(:submission, exercise_id: exercise.id, user_id: user.id, user_type: user.class.name) } it "populates the editors with the submission's files' content" do request.call @@ -182,7 +182,7 @@ describe ExercisesController do describe 'GET #index' do let(:scope) { Pundit.policy_scope!(user, Exercise) } - before(:all) { FactoryGirl.create_pair(:dummy) } + before(:all) { FactoryBot.create_pair(:dummy) } before(:each) { get :index } expect_assigns(exercises: :scope) @@ -230,8 +230,8 @@ describe ExercisesController do describe 'POST #submit' do let(:output) { {} } let(:request) { post :submit, format: :json, id: exercise.id, submission: {cause: 'submit', exercise_id: exercise.id} } - let!(:external_user) { FactoryGirl.create(:external_user) } - let!(:lti_parameter) { FactoryGirl.create(:lti_parameter, external_user: external_user, exercise: exercise) } + let!(:external_user) { FactoryBot.create(:external_user) } + let!(:lti_parameter) { FactoryBot.create(:lti_parameter, external_user: external_user, exercise: exercise) } before(:each) do allow_any_instance_of(Submission).to receive(:normalized_score).and_return(1) @@ -298,7 +298,7 @@ describe ExercisesController do describe 'PUT #update' do context 'with a valid exercise' do - let(:exercise_attributes) { FactoryGirl.build(:dummy).attributes } + let(:exercise_attributes) { FactoryBot.build(:dummy).attributes } before(:each) { put :update, exercise: exercise_attributes, id: exercise.id } expect_assigns(exercise: Exercise) diff --git a/spec/controllers/external_users_controller_spec.rb b/spec/controllers/external_users_controller_spec.rb index b3029e6e..c8df8da7 100644 --- a/spec/controllers/external_users_controller_spec.rb +++ b/spec/controllers/external_users_controller_spec.rb @@ -1,8 +1,8 @@ require 'rails_helper' describe ExternalUsersController do - let(:user) { FactoryGirl.build(:admin) } - let!(:users) { FactoryGirl.create_pair(:external_user) } + let(:user) { FactoryBot.build(:admin) } + let!(:users) { FactoryBot.create_pair(:external_user) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'GET #index' do diff --git a/spec/controllers/file_types_controller_spec.rb b/spec/controllers/file_types_controller_spec.rb index 037e5ae5..83e8396b 100644 --- a/spec/controllers/file_types_controller_spec.rb +++ b/spec/controllers/file_types_controller_spec.rb @@ -1,13 +1,13 @@ require 'rails_helper' describe FileTypesController do - let(:file_type) { FactoryGirl.create(:dot_rb) } - let(:user) { FactoryGirl.create(:admin) } + let(:file_type) { FactoryBot.create(:dot_rb) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'POST #create' do context 'with a valid file type' do - let(:request) { proc { post :create, file_type: FactoryGirl.attributes_for(:dot_rb) } } + let(:request) { proc { post :create, file_type: FactoryBot.attributes_for(:dot_rb) } } before(:each) { request.call } expect_assigns(editor_modes: Array) @@ -36,7 +36,7 @@ describe FileTypesController do expect_assigns(file_type: FileType) it 'destroys the file type' do - file_type = FactoryGirl.create(:dot_rb) + file_type = FactoryBot.create(:dot_rb) expect { delete :destroy, id: file_type.id }.to change(FileType, :count).by(-1) end @@ -53,7 +53,7 @@ describe FileTypesController do end describe 'GET #index' do - before(:all) { FactoryGirl.create_pair(:dot_rb) } + before(:all) { FactoryBot.create_pair(:dot_rb) } before(:each) { get :index } expect_assigns(file_types: FileType.all) @@ -80,7 +80,7 @@ describe FileTypesController do describe 'PUT #update' do context 'with a valid file type' do - before(:each) { put :update, file_type: FactoryGirl.attributes_for(:dot_rb), id: file_type.id } + before(:each) { put :update, file_type: FactoryBot.attributes_for(:dot_rb), id: file_type.id } expect_assigns(editor_modes: Array) expect_assigns(file_type: FileType) diff --git a/spec/controllers/hints_controller_spec.rb b/spec/controllers/hints_controller_spec.rb index 916e5150..f54d09ad 100644 --- a/spec/controllers/hints_controller_spec.rb +++ b/spec/controllers/hints_controller_spec.rb @@ -1,14 +1,14 @@ require 'rails_helper' describe HintsController do - let(:execution_environment) { FactoryGirl.create(:ruby) } - let(:hint) { FactoryGirl.create(:ruby_syntax_error) } - let(:user) { FactoryGirl.create(:admin) } + let(:execution_environment) { FactoryBot.create(:ruby) } + let(:hint) { FactoryBot.create(:ruby_syntax_error) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'POST #create' do context 'with a valid hint' do - let(:request) { proc { post :create, execution_environment_id: execution_environment.id, hint: FactoryGirl.attributes_for(:ruby_syntax_error) } } + let(:request) { proc { post :create, execution_environment_id: execution_environment.id, hint: FactoryBot.attributes_for(:ruby_syntax_error) } } before(:each) { request.call } expect_assigns(execution_environment: :execution_environment) @@ -38,7 +38,7 @@ describe HintsController do expect_assigns(hint: Hint) it 'destroys the hint' do - hint = FactoryGirl.create(:ruby_syntax_error) + hint = FactoryBot.create(:ruby_syntax_error) expect { delete :destroy, execution_environment_id: execution_environment.id, id: hint.id }.to change(Hint, :count).by(-1) end @@ -55,7 +55,7 @@ describe HintsController do end describe 'GET #index' do - before(:all) { FactoryGirl.create_pair(:ruby_syntax_error) } + before(:all) { FactoryBot.create_pair(:ruby_syntax_error) } before(:each) { get :index, execution_environment_id: execution_environment.id } expect_assigns(execution_environment: :execution_environment) @@ -84,7 +84,7 @@ describe HintsController do describe 'PUT #update' do context 'with a valid hint' do - before(:each) { put :update, execution_environment_id: execution_environment.id, hint: FactoryGirl.attributes_for(:ruby_syntax_error), id: hint.id } + before(:each) { put :update, execution_environment_id: execution_environment.id, hint: FactoryBot.attributes_for(:ruby_syntax_error), id: hint.id } expect_assigns(execution_environment: :execution_environment) expect_assigns(hint: Hint) diff --git a/spec/controllers/internal_users_controller_spec.rb b/spec/controllers/internal_users_controller_spec.rb index e0efb339..936852af 100644 --- a/spec/controllers/internal_users_controller_spec.rb +++ b/spec/controllers/internal_users_controller_spec.rb @@ -1,11 +1,11 @@ require 'rails_helper' describe InternalUsersController do - let(:user) { FactoryGirl.build(:admin) } - let!(:users) { FactoryGirl.create_pair(:teacher) } + let(:user) { FactoryBot.build(:admin) } + let!(:users) { FactoryBot.create_pair(:teacher) } describe 'GET #activate' do - let(:user) { InternalUser.create(FactoryGirl.attributes_for(:teacher)) } + let(:user) { InternalUser.create(FactoryBot.attributes_for(:teacher)) } before(:each) do user.send(:setup_activation) @@ -37,7 +37,7 @@ describe InternalUsersController do end describe 'PUT #activate' do - let(:user) { InternalUser.create(FactoryGirl.attributes_for(:teacher)) } + let(:user) { InternalUser.create(FactoryBot.attributes_for(:teacher)) } let(:password) { SecureRandom.hex } before(:each) do @@ -103,7 +103,7 @@ describe InternalUsersController do before(:each) { allow(controller).to receive(:current_user).and_return(user) } context 'with a valid internal user' do - let(:request) { proc { post :create, internal_user: FactoryGirl.attributes_for(:teacher) } } + let(:request) { proc { post :create, internal_user: FactoryBot.attributes_for(:teacher) } } before(:each) { request.call } expect_assigns(user: InternalUser) @@ -303,7 +303,7 @@ describe InternalUsersController do before(:each) { allow(controller).to receive(:current_user).and_return(user) } context 'with a valid internal user' do - before(:each) { put :update, internal_user: FactoryGirl.attributes_for(:teacher), id: users.first.id } + before(:each) { put :update, internal_user: FactoryBot.attributes_for(:teacher), id: users.first.id } expect_assigns(user: InternalUser) expect_redirect { user } diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 50fef8d6..1d7fa877 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -1,12 +1,12 @@ require 'rails_helper' describe SessionsController do - let(:consumer) { FactoryGirl.create(:consumer) } + let(:consumer) { FactoryBot.create(:consumer) } describe 'POST #create' do let(:password) { user_attributes[:password] } let(:user) { InternalUser.create(user_attributes) } - let(:user_attributes) { FactoryGirl.attributes_for(:teacher) } + let(:user_attributes) { FactoryBot.attributes_for(:teacher) } context 'with valid credentials' do before(:each) do @@ -27,8 +27,8 @@ describe SessionsController do end describe 'POST #create_through_lti' do - let(:exercise) { FactoryGirl.create(:dummy) } - let(:exercise2) { FactoryGirl.create(:dummy) } + let(:exercise) { FactoryBot.create(:dummy) } + let(:exercise2) { FactoryBot.create(:dummy) } let(:nonce) { SecureRandom.hex } before(:each) { I18n.locale = I18n.default_locale } @@ -73,7 +73,7 @@ describe SessionsController do context 'with valid launch parameters' do let(:locale) { :de } let(:request) { post :create_through_lti, custom_locale: locale, custom_token: exercise.token, oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, user_id: user.external_id } - let(:user) { FactoryGirl.create(:external_user, consumer_id: consumer.id) } + let(:user) { FactoryBot.create(:external_user, consumer_id: consumer.id) } before(:each) { expect_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) } it 'assigns the current user' do @@ -132,14 +132,14 @@ describe SessionsController do end it 'redirects to recommended exercise if requested token of proxy exercise' do - FactoryGirl.create(:proxy_exercise, exercises: [exercise]) + FactoryBot.create(:proxy_exercise, exercises: [exercise]) post :create_through_lti, custom_locale: locale, custom_token: ProxyExercise.first.token, oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, user_id: user.external_id expect(controller).to redirect_to(implement_exercise_path(exercise.id)) end it 'recommends only exercises who are 1 degree more complicated than what user has seen' do # dummy user has no exercises finished, therefore his highest difficulty is 0 - FactoryGirl.create(:proxy_exercise, exercises: [exercise, exercise2]) + FactoryBot.create(:proxy_exercise, exercises: [exercise, exercise2]) exercise.expected_difficulty = 3 exercise.save exercise2.expected_difficulty = 1 @@ -191,7 +191,7 @@ describe SessionsController do describe 'GET #destroy_through_lti' do let(:request) { proc { get :destroy_through_lti, consumer_id: consumer.id, submission_id: submission.id } } - let(:submission) { FactoryGirl.create(:submission, exercise: FactoryGirl.create(:dummy)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:dummy)) } before(:each) do #Todo replace session with lti_parameter @@ -225,7 +225,7 @@ describe SessionsController do context 'when a user is already logged in' do before(:each) do - expect(controller).to receive(:current_user).and_return(FactoryGirl.build(:teacher)) + expect(controller).to receive(:current_user).and_return(FactoryBot.build(:teacher)) get :new end diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 98f36cb2..d2e92a48 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -1,8 +1,8 @@ require 'rails_helper' describe SubmissionsController do - let(:submission) { FactoryGirl.create(:submission) } - let(:user) { FactoryGirl.create(:admin) } + let(:submission) { FactoryBot.create(:submission) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow(controller).to receive(:current_user).and_return(user) } describe 'POST #create' do @@ -11,8 +11,8 @@ describe SubmissionsController do end context 'with a valid submission' do - let(:exercise) { FactoryGirl.create(:hello_world) } - let(:request) { proc { post :create, format: :json, submission: FactoryGirl.attributes_for(:submission, exercise_id: exercise.id) } } + let(:exercise) { FactoryBot.create(:hello_world) } + let(:request) { proc { post :create, format: :json, submission: FactoryBot.attributes_for(:submission, exercise_id: exercise.id) } } before(:each) { request.call } expect_assigns(submission: Submission) @@ -42,7 +42,7 @@ describe SubmissionsController do end context 'with a valid filename' do - let(:submission) { FactoryGirl.create(:submission, exercise: FactoryGirl.create(:audio_video)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:audio_video)) } before(:each) { get :download_file, filename: file.name_with_extension, id: submission.id } context 'for a binary file' do @@ -74,7 +74,7 @@ describe SubmissionsController do end describe 'GET #index' do - before(:all) { FactoryGirl.create_pair(:submission) } + before(:all) { FactoryBot.create_pair(:submission) } before(:each) { get :index } expect_assigns(submissions: Submission.all) @@ -92,7 +92,7 @@ describe SubmissionsController do end context 'with a valid filename' do - let(:submission) { FactoryGirl.create(:submission, exercise: FactoryGirl.create(:audio_video)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:audio_video)) } before(:each) { get :render_file, filename: file.name_with_extension, id: submission.id } context 'for a binary file' do diff --git a/spec/factories/code_ocean/file.rb b/spec/factories/code_ocean/file.rb index 663dc33f..bc672045 100644 --- a/spec/factories/code_ocean/file.rb +++ b/spec/factories/code_ocean/file.rb @@ -1,7 +1,7 @@ require 'seeds_helper' module CodeOcean - FactoryGirl.define do + FactoryBot.define do factory :file, class: CodeOcean::File do content '' association :context, factory: :submission diff --git a/spec/factories/consumer.rb b/spec/factories/consumer.rb index 58bfbadc..a4f32ca8 100644 --- a/spec/factories/consumer.rb +++ b/spec/factories/consumer.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :consumer do name 'openHPI' oauth_key { SecureRandom.hex } diff --git a/spec/factories/error.rb b/spec/factories/error.rb index 235a90c0..c10d47bf 100644 --- a/spec/factories/error.rb +++ b/spec/factories/error.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :error, class: Error do association :execution_environment, factory: :ruby message "exercise.rb:4:in `
': undefined local variable or method `foo' for main:Object (NameError)" diff --git a/spec/factories/execution_environment.rb b/spec/factories/execution_environment.rb index 051d9de6..013a041b 100644 --- a/spec/factories/execution_environment.rb +++ b/spec/factories/execution_environment.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :coffee_script, class: ExecutionEnvironment do created_by_teacher default_memory_limit diff --git a/spec/factories/exercise.rb b/spec/factories/exercise.rb index 9d8ac5b1..2fcd7e07 100644 --- a/spec/factories/exercise.rb +++ b/spec/factories/exercise.rb @@ -2,7 +2,7 @@ require 'seeds_helper' def create_seed_file(exercise, path, file_attributes = {}) file_extension = File.extname(path) - file_type = FactoryGirl.create(file_attributes[:file_type] || :"dot_#{file_extension.gsub('.', '')}") + file_type = FactoryBot.create(file_attributes[:file_type] || :"dot_#{file_extension.gsub('.', '')}") name = File.basename(path).gsub(file_extension, '') file_attributes.merge!(file_type: file_type, name: name, path: path.split('/')[1..-2].join('/'), role: file_attributes[:role] || 'regular_file') if file_type.binary? @@ -13,7 +13,7 @@ def create_seed_file(exercise, path, file_attributes = {}) exercise.add_file!(file_attributes) end -FactoryGirl.define do +FactoryBot.define do factory :audio_video, class: Exercise do created_by_teacher description "Try HTML's audio and video capabilities." diff --git a/spec/factories/external_user.rb b/spec/factories/external_user.rb index 0d30796a..41a9c3e8 100644 --- a/spec/factories/external_user.rb +++ b/spec/factories/external_user.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :external_user do association :consumer generated_email diff --git a/spec/factories/file_type.rb b/spec/factories/file_type.rb index 14b8e14e..eb7b8c90 100644 --- a/spec/factories/file_type.rb +++ b/spec/factories/file_type.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :dot_coffee, class: FileType do created_by_admin editor_mode 'ace/mode/coffee' diff --git a/spec/factories/hint.rb b/spec/factories/hint.rb index 3f06babf..fa1335a9 100644 --- a/spec/factories/hint.rb +++ b/spec/factories/hint.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :node_js_invalid_assignment, class: Hint do association :execution_environment, factory: :node_js english diff --git a/spec/factories/internal_user.rb b/spec/factories/internal_user.rb index f7311e4c..0e1c8682 100644 --- a/spec/factories/internal_user.rb +++ b/spec/factories/internal_user.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :admin, class: InternalUser do activated_user email 'admin@example.org' diff --git a/spec/factories/lti_parameter.rb b/spec/factories/lti_parameter.rb index 9cd0f367..0d882acc 100644 --- a/spec/factories/lti_parameter.rb +++ b/spec/factories/lti_parameter.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do LTI_PARAMETERS = { lis_result_sourcedid: "c2db0c7c-4411-4b27-a52b-ddfc3dc32065", @@ -17,4 +17,4 @@ FactoryGirl.define do lti_parameters LTI_PARAMETERS.except(:lis_outcome_service_url) end end -end \ No newline at end of file +end diff --git a/spec/factories/proxy_exercise.rb b/spec/factories/proxy_exercise.rb index 9c9974d6..303839a6 100644 --- a/spec/factories/proxy_exercise.rb +++ b/spec/factories/proxy_exercise.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :proxy_exercise, class: ProxyExercise do token 'dummytoken' title 'Dummy' diff --git a/spec/factories/shared_traits.rb b/spec/factories/shared_traits.rb index ea39e926..43847e8c 100644 --- a/spec/factories/shared_traits.rb +++ b/spec/factories/shared_traits.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do [:admin, :external_user, :teacher].each do |factory_name| trait :"created_by_#{factory_name}" do association :user, factory: factory_name diff --git a/spec/factories/submission.rb b/spec/factories/submission.rb index cd4e34e1..f40a2a1f 100644 --- a/spec/factories/submission.rb +++ b/spec/factories/submission.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :submission do cause 'save' created_by_external_user diff --git a/spec/factories/user_exercise_feedback.rb b/spec/factories/user_exercise_feedback.rb index b5a376cd..42c5d803 100644 --- a/spec/factories/user_exercise_feedback.rb +++ b/spec/factories/user_exercise_feedback.rb @@ -1,7 +1,7 @@ -FactoryGirl.define do +FactoryBot.define do factory :user_exercise_feedback, class: UserExerciseFeedback do created_by_external_user feedback_text 'Most suitable exercise ever' end -end \ No newline at end of file +end diff --git a/spec/features/authentication_spec.rb b/spec/features/authentication_spec.rb index 8b85a64b..2a777dab 100644 --- a/spec/features/authentication_spec.rb +++ b/spec/features/authentication_spec.rb @@ -1,8 +1,8 @@ require 'rails_helper' describe 'Authentication' do - let(:user) { FactoryGirl.create(:admin) } - let(:password) { FactoryGirl.attributes_for(:admin)[:password] } + let(:user) { FactoryBot.create(:admin) } + let(:password) { FactoryBot.attributes_for(:admin)[:password] } context 'when signed out' do before(:each) { visit(root_path) } diff --git a/spec/features/authorization_spec.rb b/spec/features/authorization_spec.rb index 7f0ff04f..7535634f 100644 --- a/spec/features/authorization_spec.rb +++ b/spec/features/authorization_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' describe 'Authorization' do context 'as an admin' do - let(:user) { FactoryGirl.create(:admin) } + let(:user) { FactoryBot.create(:admin) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser].each do |model| @@ -11,7 +11,7 @@ describe 'Authorization' do end context 'as an external user' do - let(:user) { FactoryGirl.create(:external_user) } + let(:user) { FactoryBot.create(:external_user) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } [Consumer, ExecutionEnvironment, Exercise, FileType, InternalUser].each do |model| @@ -20,7 +20,7 @@ describe 'Authorization' do end context 'as a teacher' do - let(:user) { FactoryGirl.create(:teacher) } + let(:user) { FactoryBot.create(:teacher) } before(:each) { allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) } [Consumer, InternalUser].each do |model| diff --git a/spec/features/editor_spec.rb b/spec/features/editor_spec.rb index bd1c46a2..88e480d4 100644 --- a/spec/features/editor_spec.rb +++ b/spec/features/editor_spec.rb @@ -1,13 +1,13 @@ require 'rails_helper' describe 'Editor', js: true do - let(:exercise) { FactoryGirl.create(:audio_video, instructions: Forgery(:lorem_ipsum).sentence) } - let(:user) { FactoryGirl.create(:teacher) } + let(:exercise) { FactoryBot.create(:audio_video, instructions: Forgery(:lorem_ipsum).sentence) } + let(:user) { FactoryBot.create(:teacher) } before(:each) do visit(sign_in_path) fill_in('email', with: user.email) - fill_in('password', with: FactoryGirl.attributes_for(:teacher)[:password]) + fill_in('password', with: FactoryBot.attributes_for(:teacher)[:password]) click_button(I18n.t('sessions.new.link')) visit(implement_exercise_path(exercise)) end diff --git a/spec/features/factories_spec.rb b/spec/features/factories_spec.rb index 5e002f91..cba0d735 100644 --- a/spec/features/factories_spec.rb +++ b/spec/features/factories_spec.rb @@ -2,6 +2,6 @@ require 'rails_helper' describe 'Factories' do it 'are all valid', docker: true, permitted_execution_time: 30 do - expect { FactoryGirl.lint }.not_to raise_error + expect { FactoryBot.lint }.not_to raise_error end end diff --git a/spec/helpers/admin/dashboard_helper_spec.rb b/spec/helpers/admin/dashboard_helper_spec.rb index a063c756..9dadaa54 100644 --- a/spec/helpers/admin/dashboard_helper_spec.rb +++ b/spec/helpers/admin/dashboard_helper_spec.rb @@ -8,7 +8,7 @@ describe Admin::DashboardHelper do end describe '#docker_data' do - before(:each) { FactoryGirl.create(:ruby) } + before(:each) { FactoryBot.create(:ruby) } it 'contains an entry for every execution environment' do expect(docker_data.length).to eq(ExecutionEnvironment.count) diff --git a/spec/helpers/exercise_helper_spec.rb b/spec/helpers/exercise_helper_spec.rb index 2fa9b331..7feb10ca 100644 --- a/spec/helpers/exercise_helper_spec.rb +++ b/spec/helpers/exercise_helper_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' describe ExerciseHelper do describe '#embedding_parameters' do - let(:exercise) { FactoryGirl.build(:dummy) } + let(:exercise) { FactoryBot.build(:dummy) } it 'contains the locale' do expect(embedding_parameters(exercise)).to start_with("locale=#{I18n.locale}") diff --git a/spec/lib/assessor_spec.rb b/spec/lib/assessor_spec.rb index 38edf8d5..ac726c3f 100644 --- a/spec/lib/assessor_spec.rb +++ b/spec/lib/assessor_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe Assessor do - let(:assessor) { described_class.new(execution_environment: FactoryGirl.build(:ruby)) } + let(:assessor) { described_class.new(execution_environment: FactoryBot.build(:ruby)) } describe '#assess' do let(:assess) { assessor.assess(stdout: stdout) } @@ -53,7 +53,7 @@ describe Assessor do context 'with an execution environment without a testing framework adapter' do it 'raises an error' do - expect { described_class.new(execution_environment: FactoryGirl.build(:ruby, testing_framework: nil)) }.to raise_error(Assessor::Error) + expect { described_class.new(execution_environment: FactoryBot.build(:ruby, testing_framework: nil)) }.to raise_error(Assessor::Error) end end end diff --git a/spec/lib/docker_client_spec.rb b/spec/lib/docker_client_spec.rb index feb457f6..6f6674d7 100644 --- a/spec/lib/docker_client_spec.rb +++ b/spec/lib/docker_client_spec.rb @@ -3,10 +3,10 @@ require 'seeds_helper' describe DockerClient, docker: true do let(:command) { 'whoami' } - let(:docker_client) { described_class.new(execution_environment: FactoryGirl.build(:java), user: FactoryGirl.build(:admin)) } - let(:execution_environment) { FactoryGirl.build(:java) } + let(:docker_client) { described_class.new(execution_environment: FactoryBot.build(:java), user: FactoryGirl.build(:admin)) } + let(:execution_environment) { FactoryBot.build(:java) } let(:image) { double } - let(:submission) { FactoryGirl.create(:submission) } + let(:submission) { FactoryBot.create(:submission) } let(:workspace_path) { '/tmp' } describe '.check_availability!' do @@ -146,7 +146,7 @@ describe DockerClient, docker: true do end describe '#create_workspace_file' do - let(:file) { FactoryGirl.build(:file, content: 'puts 42') } + let(:file) { FactoryBot.build(:file, content: 'puts 42') } let(:file_path) { File.join(workspace_path, file.name_with_extension) } after(:each) { File.delete(file_path) } diff --git a/spec/lib/docker_container_pool_spec.rb b/spec/lib/docker_container_pool_spec.rb index d419dd5a..1ff39657 100644 --- a/spec/lib/docker_container_pool_spec.rb +++ b/spec/lib/docker_container_pool_spec.rb @@ -9,7 +9,7 @@ describe DockerContainerPool do private :reload_class before(:each) do - @execution_environment = FactoryGirl.create(:ruby) + @execution_environment = FactoryBot.create(:ruby) reload_class end diff --git a/spec/lib/file_tree_spec.rb b/spec/lib/file_tree_spec.rb index 64995936..0d6ac12a 100644 --- a/spec/lib/file_tree_spec.rb +++ b/spec/lib/file_tree_spec.rb @@ -8,7 +8,7 @@ describe FileTree do context 'for a media file' do context 'for an audio file' do - let(:file) { FactoryGirl.build(:file, file_type: FactoryGirl.build(:dot_mp3)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_mp3)) } it 'is an audio file icon' do expect(file_icon).to include('fa-file-audio-o') @@ -16,7 +16,7 @@ describe FileTree do end context 'for an image file' do - let(:file) { FactoryGirl.build(:file, file_type: FactoryGirl.build(:dot_jpg)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_jpg)) } it 'is an image file icon' do expect(file_icon).to include('fa-file-image-o') @@ -24,7 +24,7 @@ describe FileTree do end context 'for a video file' do - let(:file) { FactoryGirl.build(:file, file_type: FactoryGirl.build(:dot_mp4)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_mp4)) } it 'is a video file icon' do expect(file_icon).to include('fa-file-video-o') @@ -34,7 +34,7 @@ describe FileTree do context 'for other files' do context 'for a read-only file' do - let(:file) { FactoryGirl.build(:file, read_only: true) } + let(:file) { FactoryBot.build(:file, read_only: true) } it 'is a lock icon' do expect(file_icon).to include('fa-lock') @@ -42,7 +42,7 @@ describe FileTree do end context 'for an executable file' do - let(:file) { FactoryGirl.build(:file, file_type: FactoryGirl.build(:dot_py)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_py)) } it 'is a code file icon' do expect(file_icon).to include('fa-file-code-o') @@ -50,7 +50,7 @@ describe FileTree do end context 'for a renderable file' do - let(:file) { FactoryGirl.build(:file, file_type: FactoryGirl.build(:dot_svg)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_svg)) } it 'is a text file icon' do expect(file_icon).to include('fa-file-text-o') @@ -58,7 +58,7 @@ describe FileTree do end context 'for all other files' do - let(:file) { FactoryGirl.build(:file, file_type: FactoryGirl.build(:dot_md)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_md)) } it 'is a generic file icon' do expect(file_icon).to include('fa-file-o') @@ -75,7 +75,7 @@ describe FileTree do describe '#initialize' do let(:file_tree) { described_class.new(files) } - let(:files) { FactoryGirl.build_list(:file, 10, context: nil, path: 'foo/bar/baz') } + let(:files) { FactoryBot.build_list(:file, 10, context: nil, path: 'foo/bar/baz') } it 'creates a root node' do expect_any_instance_of(Tree::TreeNode).to receive(:initialize).with(file_tree.send(:root_label)) @@ -92,7 +92,7 @@ describe FileTree do end describe '#map_to_js_tree' do - let(:file) { FactoryGirl.build(:file) } + let(:file) { FactoryBot.build(:file) } let(:js_tree) { file_tree.send(:map_to_js_tree, node) } let!(:leaf) { root.add(Tree::TreeNode.new('', file)) } let(:root) { Tree::TreeNode.new('', file) } diff --git a/spec/lib/whistleblower_spec.rb b/spec/lib/whistleblower_spec.rb index a7a7c9c6..e0a8a8d9 100644 --- a/spec/lib/whistleblower_spec.rb +++ b/spec/lib/whistleblower_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe Whistleblower do - let(:hint) { FactoryGirl.create(:ruby_no_method_error) } + let(:hint) { FactoryBot.create(:ruby_no_method_error) } let(:stderr) { "undefined method `foo' for main:Object (NoMethodError)" } let(:whistleblower) { described_class.new(execution_environment: hint.execution_environment) } diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index b40b0728..d89b646c 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe UserMailer do - let(:user) { InternalUser.create(FactoryGirl.attributes_for(:teacher)) } + let(:user) { InternalUser.create(FactoryBot.attributes_for(:teacher)) } describe '#activation_needed_email' do let(:mail) { described_class.activation_needed_email(user) } diff --git a/spec/models/consumer_spec.rb b/spec/models/consumer_spec.rb index ed5e0fbe..0c977955 100644 --- a/spec/models/consumer_spec.rb +++ b/spec/models/consumer_spec.rb @@ -12,7 +12,7 @@ describe Consumer do end it 'validates the uniqueness of the OAuth key' do - consumer.update(oauth_key: FactoryGirl.create(:consumer).oauth_key) + consumer.update(oauth_key: FactoryBot.create(:consumer).oauth_key) expect(consumer.errors[:oauth_key]).to be_present end diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index a55eda25..2dcee496 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -6,7 +6,7 @@ describe ExecutionEnvironment do it 'validates that the Docker image works', docker: true do expect(execution_environment).to receive(:validate_docker_image?).and_return(true) expect(execution_environment).to receive(:working_docker_image?) - execution_environment.update(docker_image: FactoryGirl.attributes_for(:ruby)[:docker_image]) + execution_environment.update(docker_image: FactoryBot.attributes_for(:ruby)[:docker_image]) end it 'validates the presence of a Docker image name' do @@ -69,7 +69,7 @@ describe ExecutionEnvironment do describe '#valid_test_setup?' do context 'with a test command and a testing framework' do - before(:each) { execution_environment.update(test_command: FactoryGirl.attributes_for(:ruby)[:test_command], testing_framework: FactoryGirl.attributes_for(:ruby)[:testing_framework]) } + before(:each) { execution_environment.update(test_command: FactoryBot.attributes_for(:ruby)[:test_command], testing_framework: FactoryGirl.attributes_for(:ruby)[:testing_framework]) } it 'is valid' do expect(execution_environment.errors[:test_command]).to be_blank @@ -77,7 +77,7 @@ describe ExecutionEnvironment do end context 'with a test command but no testing framework' do - before(:each) { execution_environment.update(test_command: FactoryGirl.attributes_for(:ruby)[:test_command], testing_framework: nil) } + before(:each) { execution_environment.update(test_command: FactoryBot.attributes_for(:ruby)[:test_command], testing_framework: nil) } it 'is invalid' do expect(execution_environment.errors[:test_command]).to be_present @@ -85,7 +85,7 @@ describe ExecutionEnvironment do end context 'with no test command but a testing framework' do - before(:each) { execution_environment.update(test_command: nil, testing_framework: FactoryGirl.attributes_for(:ruby)[:testing_framework]) } + before(:each) { execution_environment.update(test_command: nil, testing_framework: FactoryBot.attributes_for(:ruby)[:testing_framework]) } it 'is invalid' do expect(execution_environment.errors[:test_command]).to be_present @@ -113,7 +113,7 @@ describe ExecutionEnvironment do end it 'is true otherwise' do - execution_environment.docker_image = FactoryGirl.attributes_for(:ruby)[:docker_image] + execution_environment.docker_image = FactoryBot.attributes_for(:ruby)[:docker_image] allow(Rails.env).to receive(:test?).and_return(false) expect(execution_environment.send(:validate_docker_image?)).to be true end diff --git a/spec/models/exercise_spec.rb b/spec/models/exercise_spec.rb index f91ed4fa..640efd92 100644 --- a/spec/models/exercise_spec.rb +++ b/spec/models/exercise_spec.rb @@ -2,17 +2,17 @@ require 'rails_helper' describe Exercise do let(:exercise) { described_class.create.tap { |exercise| exercise.update(public: nil, token: nil) } } - let(:users) { FactoryGirl.create_list(:external_user, 10) } + let(:users) { FactoryBot.create_list(:external_user, 10) } def create_submissions 10.times do - FactoryGirl.create(:submission, cause: 'submit', exercise: exercise, score: Forgery(:basic).number, user: users.sample) + FactoryBot.create(:submission, cause: 'submit', exercise: exercise, score: Forgery(:basic).number, user: users.sample) end end it 'validates the number of main files' do - exercise = FactoryGirl.create(:dummy) - exercise.files += FactoryGirl.create_pair(:file) + exercise = FactoryBot.create(:dummy) + exercise.files += FactoryBot.create_pair(:file) expect(exercise).to receive(:valid_main_file?).and_call_original exercise.save expect(exercise.errors[:files]).to be_present @@ -46,7 +46,7 @@ describe Exercise do end describe '#average_percentage' do - let(:exercise) { FactoryGirl.create(:fibonacci) } + let(:exercise) { FactoryBot.create(:fibonacci) } context 'without submissions' do it 'returns nil' do @@ -65,7 +65,7 @@ describe Exercise do end describe '#average_score' do - let(:exercise) { FactoryGirl.create(:fibonacci) } + let(:exercise) { FactoryBot.create(:fibonacci) } context 'without submissions' do it 'returns nil' do @@ -84,7 +84,7 @@ describe Exercise do end describe '#duplicate' do - let(:exercise) { FactoryGirl.create(:fibonacci) } + let(:exercise) { FactoryBot.create(:fibonacci) } after(:each) { exercise.duplicate } it 'duplicates the exercise' do diff --git a/spec/models/external_user_spec.rb b/spec/models/external_user_spec.rb index a47f4aca..5d1d0aae 100644 --- a/spec/models/external_user_spec.rb +++ b/spec/models/external_user_spec.rb @@ -13,7 +13,7 @@ describe ExternalUser do describe '#admin?' do it 'is false' do - expect(FactoryGirl.build(:external_user).admin?).to be false + expect(FactoryBot.build(:external_user).admin?).to be false end end @@ -31,7 +31,7 @@ describe ExternalUser do describe '#teacher?' do it 'is false' do - expect(FactoryGirl.build(:external_user).teacher?).to be false + expect(FactoryBot.build(:external_user).teacher?).to be false end end end diff --git a/spec/models/internal_user_spec.rb b/spec/models/internal_user_spec.rb index 9a41d7b6..25f7e172 100644 --- a/spec/models/internal_user_spec.rb +++ b/spec/models/internal_user_spec.rb @@ -9,12 +9,12 @@ describe InternalUser do end it 'validates the uniqueness of the email address' do - user.update(email: FactoryGirl.create(:admin).email) + user.update(email: FactoryBot.create(:admin).email) expect(user.errors[:email]).to be_present end context 'when not activated' do - let(:user) { FactoryGirl.create(:teacher) } + let(:user) { FactoryBot.create(:teacher) } before(:each) do user.send(:setup_activation) @@ -33,7 +33,7 @@ describe InternalUser do end context 'with a pending password reset' do - let(:user) { FactoryGirl.create(:teacher) } + let(:user) { FactoryBot.create(:teacher) } before(:each) { user.deliver_reset_password_instructions! } it 'validates the confirmation of the password' do @@ -48,7 +48,7 @@ describe InternalUser do end context 'when complete' do - let(:user) { FactoryGirl.create(:teacher, activation_state: 'active') } + let(:user) { FactoryBot.create(:teacher, activation_state: 'active') } it 'does not validate the confirmation of the password' do user.update(password: password, password_confirmation: '') @@ -71,8 +71,8 @@ describe InternalUser do describe '#admin?' do it 'is only true for admins' do - expect(FactoryGirl.build(:admin).admin?).to be true - expect(FactoryGirl.build(:teacher).admin?).to be false + expect(FactoryBot.build(:admin).admin?).to be true + expect(FactoryBot.build(:teacher).admin?).to be false end end @@ -90,8 +90,8 @@ describe InternalUser do describe '#teacher?' do it 'is only true for teachers' do - expect(FactoryGirl.build(:admin).teacher?).to be false - expect(FactoryGirl.build(:teacher).teacher?).to be true + expect(FactoryBot.build(:admin).teacher?).to be false + expect(FactoryBot.build(:teacher).teacher?).to be true end end end diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 227c6b99..0d48eedf 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe Submission do - let(:submission) { FactoryGirl.create(:submission, exercise: FactoryGirl.create(:dummy)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:dummy)) } it 'validates the presence of a cause' do expect(described_class.create.errors[:cause]).to be_present @@ -17,7 +17,7 @@ describe Submission do end describe '#main_file' do - let(:submission) { FactoryGirl.create(:submission) } + let(:submission) { FactoryBot.create(:submission) } it "returns the submission's main file" do expect(submission.main_file).to be_a(CodeOcean::File) @@ -27,7 +27,7 @@ describe Submission do describe '#normalized_score' do context 'with a score' do - let(:submission) { FactoryGirl.create(:submission) } + let(:submission) { FactoryBot.create(:submission) } before(:each) { submission.score = submission.exercise.maximum_score / 2 } it 'returns the score as a value between 0 and 1' do @@ -46,7 +46,7 @@ describe Submission do describe '#percentage' do context 'with a score' do - let(:submission) { FactoryGirl.create(:submission) } + let(:submission) { FactoryBot.create(:submission) } before(:each) { submission.score = submission.exercise.maximum_score / 2 } it 'returns the score expressed as a percentage' do @@ -65,11 +65,11 @@ describe Submission do describe '#siblings' do let(:siblings) { described_class.find_by(user: user).siblings } - let(:user) { FactoryGirl.create(:external_user) } + let(:user) { FactoryBot.create(:external_user) } before(:each) do 10.times.each_with_index do |_, index| - FactoryGirl.create(:submission, exercise: submission.exercise, user: (index.even? ? user : FactoryGirl.create(:external_user))) + FactoryBot.create(:submission, exercise: submission.exercise, user: (index.even? ? user : FactoryGirl.create(:external_user))) end end @@ -89,9 +89,9 @@ describe Submission do describe '#redirect_to_feedback?' do context 'with no exercise feedback' do - let(:exercise) {FactoryGirl.create(:dummy)} - let(:user) {FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) % 10)} - let(:submission) {FactoryGirl.build(:submission, exercise: exercise, user: user)} + let(:exercise) {FactoryBot.create(:dummy)} + let(:user) {FactoryBot.build(:external_user, id: (11 - exercise.created_at.to_i % 10) % 10)} + let(:submission) {FactoryBot.build(:submission, exercise: exercise, user: user)} it 'sends 10% of users to feedback page' do expect(submission.send(:redirect_to_feedback?)).to be_truthy @@ -99,16 +99,16 @@ describe Submission do it 'does not redirect other users' do 9.times do |i| - submission = FactoryGirl.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) + submission = FactoryBot.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) expect(submission.send(:redirect_to_feedback?)).to be_falsey end end end context 'with little exercise feedback' do - let(:exercise) {FactoryGirl.create(:dummy_with_user_feedbacks)} - let(:user) {FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) % 10)} - let(:submission) {FactoryGirl.build(:submission, exercise: exercise, user: user)} + let(:exercise) {FactoryBot.create(:dummy_with_user_feedbacks)} + let(:user) {FactoryBot.build(:external_user, id: (11 - exercise.created_at.to_i % 10) % 10)} + let(:submission) {FactoryBot.build(:submission, exercise: exercise, user: user)} it 'sends 10% of users to feedback page' do expect(submission.send(:redirect_to_feedback?)).to be_truthy @@ -116,19 +116,19 @@ describe Submission do it 'does not redirect other users' do 9.times do |i| - submission = FactoryGirl.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) + submission = FactoryBot.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) expect(submission.send(:redirect_to_feedback?)).to be_falsey end end end context 'with enough exercise feedback' do - let(:exercise) {FactoryGirl.create(:dummy_with_user_feedbacks, user_feedbacks_count: 42)} - let(:user) {FactoryGirl.create(:external_user)} + let(:exercise) {FactoryBot.create(:dummy_with_user_feedbacks, user_feedbacks_count: 42)} + let(:user) {FactoryBot.create(:external_user)} it 'sends nobody to feedback page' do 30.times do |i| - submission = FactoryGirl.create(:submission, exercise: exercise, user: FactoryGirl.create(:external_user)) + submission = FactoryBot.create(:submission, exercise: exercise, user: FactoryGirl.create(:external_user)) expect(submission.send(:redirect_to_feedback?)).to be_falsey end end diff --git a/spec/policies/admin/dashboard_policy_spec.rb b/spec/policies/admin/dashboard_policy_spec.rb index 0592843a..71a7adcd 100644 --- a/spec/policies/admin/dashboard_policy_spec.rb +++ b/spec/policies/admin/dashboard_policy_spec.rb @@ -5,15 +5,15 @@ describe Admin::DashboardPolicy do permissions :show? do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), :dashboard) + expect(subject).to permit(FactoryBot.build(:admin), :dashboard) end it 'does not grant access to teachers' do - expect(subject).not_to permit(FactoryGirl.build(:teacher), :dashboard) + expect(subject).not_to permit(FactoryBot.build(:teacher), :dashboard) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), :dashboard) + expect(subject).not_to permit(FactoryBot.build(:external_user), :dashboard) end end end diff --git a/spec/policies/code_ocean/file_policy_spec.rb b/spec/policies/code_ocean/file_policy_spec.rb index a41f0a0b..ac928cb3 100644 --- a/spec/policies/code_ocean/file_policy_spec.rb +++ b/spec/policies/code_ocean/file_policy_spec.rb @@ -3,15 +3,15 @@ require 'rails_helper' describe CodeOcean::FilePolicy do subject { described_class } - let(:exercise) { FactoryGirl.create(:fibonacci) } - let(:submission) { FactoryGirl.create(:submission) } + let(:exercise) { FactoryBot.create(:fibonacci) } + let(:submission) { FactoryBot.create(:submission) } permissions :create? do context 'as part of an exercise' do let(:file) { exercise.files.first } it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), file) + expect(subject).to permit(FactoryBot.build(:admin), file) end it 'grants access to authors' do @@ -20,7 +20,7 @@ describe CodeOcean::FilePolicy do 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), file) + expect(subject).not_to permit(FactoryBot.build(factory_name), file) end end end @@ -34,7 +34,7 @@ describe CodeOcean::FilePolicy do it 'does not grant access to all other users' do [:admin, :external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), file) + expect(subject).not_to permit(FactoryBot.build(factory_name), file) end end end @@ -45,7 +45,7 @@ describe CodeOcean::FilePolicy do let(:file) { exercise.files.first } it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), file) + expect(subject).to permit(FactoryBot.build(:admin), file) end it 'grants access to authors' do @@ -54,7 +54,7 @@ describe CodeOcean::FilePolicy do 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), file) + expect(subject).not_to permit(FactoryBot.build(factory_name), file) end end end @@ -64,7 +64,7 @@ describe CodeOcean::FilePolicy do it 'does not grant access to anyone' do [:admin, :external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), file) + expect(subject).not_to permit(FactoryBot.build(factory_name), file) end end end diff --git a/spec/policies/consumer_policy_spec.rb b/spec/policies/consumer_policy_spec.rb index 01b74dd3..36f96288 100644 --- a/spec/policies/consumer_policy_spec.rb +++ b/spec/policies/consumer_policy_spec.rb @@ -6,9 +6,9 @@ describe ConsumerPolicy do [:create?, :destroy?, :edit?, :index?, :new?, :show?, :update?].each do |action| permissions(action) do it 'grants access to admins only' do - expect(subject).to permit(FactoryGirl.build(:admin), Consumer.new) + expect(subject).to permit(FactoryBot.build(:admin), Consumer.new) [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), Consumer.new) + expect(subject).not_to permit(FactoryBot.build(factory_name), Consumer.new) end end end diff --git a/spec/policies/error_policy_spec.rb b/spec/policies/error_policy_spec.rb index 185eeeae..2fff2e7e 100644 --- a/spec/policies/error_policy_spec.rb +++ b/spec/policies/error_policy_spec.rb @@ -3,20 +3,20 @@ require 'rails_helper' describe ErrorPolicy do subject { described_class } - let(:error) { FactoryGirl.build(:error) } + let(:error) { FactoryBot.build(:error) } [:create?, :index?, :new?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), error) + expect(subject).to permit(FactoryBot.build(:admin), error) end it 'grants access to teachers' do - expect(subject).to permit(FactoryGirl.build(:teacher), error) + expect(subject).to permit(FactoryBot.build(:teacher), error) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), error) + expect(subject).not_to permit(FactoryBot.build(:external_user), error) end end end @@ -24,7 +24,7 @@ describe ErrorPolicy do [:destroy?, :edit?, :show?, :update?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), error) + expect(subject).to permit(FactoryBot.build(:admin), error) end it 'grants access to authors' do @@ -33,7 +33,7 @@ describe ErrorPolicy do 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), error) + expect(subject).not_to permit(FactoryBot.build(factory_name), error) end end end diff --git a/spec/policies/execution_environment_policy_spec.rb b/spec/policies/execution_environment_policy_spec.rb index 6f41f7e4..ef5787b9 100644 --- a/spec/policies/execution_environment_policy_spec.rb +++ b/spec/policies/execution_environment_policy_spec.rb @@ -3,20 +3,20 @@ require 'rails_helper' describe ExecutionEnvironmentPolicy do subject { described_class } - let(:execution_environment) { FactoryGirl.build(:ruby) } + let(:execution_environment) { FactoryBot.build(:ruby) } [:create?, :index?, :new?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), execution_environment) + expect(subject).to permit(FactoryBot.build(:admin), execution_environment) end it 'grants access to teachers' do - expect(subject).to permit(FactoryGirl.build(:teacher), execution_environment) + expect(subject).to permit(FactoryBot.build(:teacher), execution_environment) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), execution_environment) + expect(subject).not_to permit(FactoryBot.build(:external_user), execution_environment) end end end @@ -24,7 +24,7 @@ describe ExecutionEnvironmentPolicy do [:execute_command?, :shell?, :statistics?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), execution_environment) + expect(subject).to permit(FactoryBot.build(:admin), execution_environment) end it 'grants access to authors' do @@ -33,7 +33,7 @@ describe ExecutionEnvironmentPolicy do 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), execution_environment) + expect(subject).not_to permit(FactoryBot.build(factory_name), execution_environment) end end end @@ -42,7 +42,7 @@ describe ExecutionEnvironmentPolicy do [:destroy?, :edit?, :show?, :update?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), execution_environment) + expect(subject).to permit(FactoryBot.build(:admin), execution_environment) end it 'does not grant access to authors' do @@ -51,7 +51,7 @@ describe ExecutionEnvironmentPolicy do 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), execution_environment) + expect(subject).not_to permit(FactoryBot.build(factory_name), execution_environment) end end end diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index 2799123f..f0024fa8 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -3,13 +3,13 @@ require 'rails_helper' describe ExercisePolicy do subject { described_class } -let(:exercise) { FactoryGirl.build(:dummy) } +let(:exercise) { FactoryBot.build(:dummy) } permissions :batch_update? do it 'grants access to admins only' do - expect(subject).to permit(FactoryGirl.build(:admin), exercise) + expect(subject).to permit(FactoryBot.build(:admin), exercise) [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), exercise) + expect(subject).not_to permit(FactoryBot.build(factory_name), exercise) end end end @@ -17,15 +17,15 @@ let(:exercise) { FactoryGirl.build(:dummy) } [:create?, :index?, :new?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), exercise) + expect(subject).to permit(FactoryBot.build(:admin), exercise) end it 'grants access to teachers' do - expect(subject).to permit(FactoryGirl.build(:teacher), exercise) + expect(subject).to permit(FactoryBot.build(:teacher), exercise) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), exercise) + expect(subject).not_to permit(FactoryBot.build(:external_user), exercise) end end end @@ -33,7 +33,7 @@ let(:exercise) { FactoryGirl.build(:dummy) } [:clone?, :destroy?, :edit?, :statistics?, :update?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), exercise) + expect(subject).to permit(FactoryBot.build(:admin), exercise) end it 'grants access to authors' do @@ -42,7 +42,7 @@ let(:exercise) { FactoryGirl.build(:dummy) } 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) + expect(subject).not_to permit(FactoryBot.build(factory_name), exercise) end end end @@ -51,7 +51,7 @@ let(:exercise) { FactoryGirl.build(:dummy) } [:show?].each do |action| permissions(action) do it 'not grants access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), exercise) + expect(subject).not_to permit(FactoryBot.build(:external_user), exercise) end end end @@ -60,7 +60,7 @@ let(:exercise) { FactoryGirl.build(:dummy) } permissions(action) do it 'grants access to anyone' do [:admin, :external_user, :teacher].each do |factory_name| - expect(subject).to permit(FactoryGirl.build(factory_name), Exercise.new) + expect(subject).to permit(FactoryBot.build(factory_name), Exercise.new) end end end @@ -69,13 +69,13 @@ let(:exercise) { FactoryGirl.build(:dummy) } describe ExercisePolicy::Scope do describe '#resolve' do before(:all) do - @admin = FactoryGirl.create(:admin) - @external_user = FactoryGirl.create(:external_user) - @teacher = FactoryGirl.create(:teacher) + @admin = FactoryBot.create(:admin) + @external_user = FactoryBot.create(:external_user) + @teacher = FactoryBot.create(:teacher) [@admin, @teacher].each do |user| [true, false].each do |public| - FactoryGirl.create(:dummy, public: public, user_id: user.id, user_type: InternalUser.class.name) + FactoryBot.create(:dummy, public: public, user_id: user.id, user_type: InternalUser.class.name) end end end diff --git a/spec/policies/external_user_policy_spec.rb b/spec/policies/external_user_policy_spec.rb index be506c7f..d668c305 100644 --- a/spec/policies/external_user_policy_spec.rb +++ b/spec/policies/external_user_policy_spec.rb @@ -6,9 +6,9 @@ describe ExternalUserPolicy do [:create?, :destroy?, :edit?, :index?, :new?, :show?, :update?].each do |action| permissions(action) do it 'grants access to admins only' do - expect(subject).to permit(FactoryGirl.build(:admin), ExternalUser.new) + expect(subject).to permit(FactoryBot.build(:admin), ExternalUser.new) [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), ExternalUser.new) + expect(subject).not_to permit(FactoryBot.build(factory_name), ExternalUser.new) end end end diff --git a/spec/policies/file_type_policy_spec.rb b/spec/policies/file_type_policy_spec.rb index 042e4393..55ec4120 100644 --- a/spec/policies/file_type_policy_spec.rb +++ b/spec/policies/file_type_policy_spec.rb @@ -3,20 +3,20 @@ require 'rails_helper' describe FileTypePolicy do subject { described_class } - let(:file_type) { FactoryGirl.build(:dot_rb) } + let(:file_type) { FactoryBot.build(:dot_rb) } [:create?, :index?, :new?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), file_type) + expect(subject).to permit(FactoryBot.build(:admin), file_type) end it 'grants access to teachers' do - expect(subject).to permit(FactoryGirl.build(:teacher), file_type) + expect(subject).to permit(FactoryBot.build(:teacher), file_type) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), file_type) + expect(subject).not_to permit(FactoryBot.build(:external_user), file_type) end end end @@ -24,7 +24,7 @@ describe FileTypePolicy do [:destroy?, :edit?, :show?, :update?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), file_type) + expect(subject).to permit(FactoryBot.build(:admin), file_type) end it 'grants access to authors' do @@ -33,7 +33,7 @@ describe FileTypePolicy do 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), file_type) + expect(subject).not_to permit(FactoryBot.build(factory_name), file_type) end end end diff --git a/spec/policies/hint_policy_spec.rb b/spec/policies/hint_policy_spec.rb index ccde798f..2aa9d9e5 100644 --- a/spec/policies/hint_policy_spec.rb +++ b/spec/policies/hint_policy_spec.rb @@ -3,20 +3,20 @@ require 'rails_helper' describe HintPolicy do subject { described_class } - let(:hint) { FactoryGirl.build(:ruby_no_method_error) } + let(:hint) { FactoryBot.build(:ruby_no_method_error) } [:create?, :index?, :new?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), hint) + expect(subject).to permit(FactoryBot.build(:admin), hint) end it 'grants access to teachers' do - expect(subject).to permit(FactoryGirl.build(:teacher), hint) + expect(subject).to permit(FactoryBot.build(:teacher), hint) end it 'does not grant access to external users' do - expect(subject).not_to permit(FactoryGirl.build(:external_user), hint) + expect(subject).not_to permit(FactoryBot.build(:external_user), hint) end end end @@ -24,7 +24,7 @@ describe HintPolicy do [:destroy?, :edit?, :show?, :update?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), hint) + expect(subject).to permit(FactoryBot.build(:admin), hint) end it 'grants access to authors' do @@ -33,7 +33,7 @@ describe HintPolicy do 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), hint) + expect(subject).not_to permit(FactoryBot.build(factory_name), hint) end end end diff --git a/spec/policies/internal_user_policy_spec.rb b/spec/policies/internal_user_policy_spec.rb index 3ad6e7d9..8a50988f 100644 --- a/spec/policies/internal_user_policy_spec.rb +++ b/spec/policies/internal_user_policy_spec.rb @@ -6,9 +6,9 @@ describe InternalUserPolicy do [:create?, :edit?, :index?, :new?, :show?, :update?].each do |action| permissions(action) do it 'grants access to admins only' do - expect(subject).to permit(FactoryGirl.build(:admin), InternalUser.new) + expect(subject).to permit(FactoryBot.build(:admin), InternalUser.new) [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), InternalUser.new) + expect(subject).not_to permit(FactoryBot.build(factory_name), InternalUser.new) end end end @@ -18,16 +18,16 @@ describe InternalUserPolicy do context 'with an admin user' do it 'grants access to no one' do [:admin, :external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), FactoryGirl.build(:admin)) + expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryGirl.build(:admin)) end end end context 'with a non-admin user' do it 'grants access to admins only' do - expect(subject).to permit(FactoryGirl.build(:admin), InternalUser.new) + expect(subject).to permit(FactoryBot.build(:admin), InternalUser.new) [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), FactoryGirl.build(:teacher)) + expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryGirl.build(:teacher)) end end end diff --git a/spec/policies/submission_policy_spec.rb b/spec/policies/submission_policy_spec.rb index f844b984..9d79b4e1 100644 --- a/spec/policies/submission_policy_spec.rb +++ b/spec/policies/submission_policy_spec.rb @@ -6,7 +6,7 @@ describe SubmissionPolicy do permissions :create? do it 'grants access to anyone' do [:admin, :external_user, :teacher].each do |factory_name| - expect(subject).to permit(FactoryGirl.build(factory_name), Submission.new) + expect(subject).to permit(FactoryBot.build(factory_name), Submission.new) end end end @@ -14,21 +14,21 @@ describe SubmissionPolicy do [:download_file?, :render_file?, :run?, :score?, :show?, :statistics?, :stop?, :test?].each do |action| permissions(action) do it 'grants access to admins' do - expect(subject).to permit(FactoryGirl.build(:admin), Submission.new) + expect(subject).to permit(FactoryBot.build(:admin), Submission.new) end it 'grants access to authors' do - user = FactoryGirl.create(:external_user) - expect(subject).to permit(user, FactoryGirl.build(:submission, exercise: Exercise.new, user_id: user.id, user_type: user.class.name)) + user = FactoryBot.create(:external_user) + expect(subject).to permit(user, FactoryBot.build(:submission, exercise: Exercise.new, user_id: user.id, user_type: user.class.name)) end end end permissions :index? do it 'grants access to admins only' do - expect(subject).to permit(FactoryGirl.build(:admin), Submission.new) + expect(subject).to permit(FactoryBot.build(:admin), Submission.new) [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryGirl.build(factory_name), Submission.new) + expect(subject).not_to permit(FactoryBot.build(factory_name), Submission.new) end end end diff --git a/spec/support/docker.rb b/spec/support/docker.rb index 7cb16ed4..e2be05f9 100644 --- a/spec/support/docker.rb +++ b/spec/support/docker.rb @@ -1,5 +1,5 @@ CONTAINER = Docker::Container.send(:new, Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex) -IMAGE = Docker::Image.new(Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex, 'RepoTags' => [FactoryGirl.attributes_for(:ruby)[:docker_image]]) +IMAGE = Docker::Image.new(Docker::Connection.new('http://example.org', {}), 'id' => SecureRandom.hex, 'RepoTags' => [FactoryBot.attributes_for(:ruby)[:docker_image]]) RSpec.configure do |config| config.before(:each) do |example| diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index eabc37d1..3d335fa4 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' describe FileUploader do let(:file_path) { Rails.root.join('db', 'seeds', 'fibonacci', 'exercise.rb') } - let(:uploader) { described_class.new(FactoryGirl.create(:file)) } + let(:uploader) { described_class.new(FactoryBot.create(:file)) } before(:each) { uploader.store!(File.open(file_path, 'r')) } after(:each) { uploader.remove! } diff --git a/spec/views/execution_environments/shell.html.slim_spec.rb b/spec/views/execution_environments/shell.html.slim_spec.rb index 791804e3..84adb038 100644 --- a/spec/views/execution_environments/shell.html.slim_spec.rb +++ b/spec/views/execution_environments/shell.html.slim_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe 'execution_environments/shell.html.slim' do - let(:execution_environment) { FactoryGirl.create(:ruby) } + let(:execution_environment) { FactoryBot.create(:ruby) } before(:each) do assign(:execution_environment, execution_environment) diff --git a/spec/views/exercises/implement.html.slim_spec.rb b/spec/views/exercises/implement.html.slim_spec.rb index 84a65fa2..7c5e641a 100644 --- a/spec/views/exercises/implement.html.slim_spec.rb +++ b/spec/views/exercises/implement.html.slim_spec.rb @@ -1,12 +1,12 @@ require 'rails_helper' describe 'exercises/implement.html.slim' do - let(:exercise) { FactoryGirl.create(:fibonacci) } + let(:exercise) { FactoryBot.create(:fibonacci) } let(:files) { exercise.files.visible } let(:non_binary_files) { files.reject { |file| file.file_type.binary? } } before(:each) do - assign(:current_user, FactoryGirl.create(:admin)) + assign(:current_user, FactoryBot.create(:admin)) assign(:exercise, exercise) assign(:files, files) assign(:paths, []) diff --git a/test/factories/error_template_attributes.rb b/test/factories/error_template_attributes.rb index 24adb856..84e65757 100644 --- a/test/factories/error_template_attributes.rb +++ b/test/factories/error_template_attributes.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :error_template_attribute do error_template nil key "MyString" diff --git a/test/factories/error_templates.rb b/test/factories/error_templates.rb index 2abf68c9..3a2637ca 100644 --- a/test/factories/error_templates.rb +++ b/test/factories/error_templates.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :error_template do execution_environment nil name "MyString" diff --git a/test/factories/structured_error_attributes.rb b/test/factories/structured_error_attributes.rb index 7485967c..0b720b23 100644 --- a/test/factories/structured_error_attributes.rb +++ b/test/factories/structured_error_attributes.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :structured_error_attribute do structured_error nil error_template_attribute nil diff --git a/test/factories/structured_errors.rb b/test/factories/structured_errors.rb index 4a87cec1..041b4bbd 100644 --- a/test/factories/structured_errors.rb +++ b/test/factories/structured_errors.rb @@ -1,4 +1,4 @@ -FactoryGirl.define do +FactoryBot.define do factory :structured_error do error_template nil file nil From 204902ef0bd190579278d0ba53ac96b98ea8117a Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 15 Nov 2017 15:19:55 +0100 Subject: [PATCH 66/69] updated gemfile to use factory_bot --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index afd0aeff..8595fdf9 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,7 @@ gem 'coffee-rails', '~> 4.0.0' gem 'concurrent-ruby', '~> 1.0.1' gem 'concurrent-ruby-ext', '~> 1.0.1', platform: :ruby gem 'docker-api','~> 1.25.0', require: 'docker' -gem 'factory_girl_rails', '~> 4.0' +gem 'factory_bot_rails', '~> 4.8.2' gem 'forgery' gem 'highline' gem 'jbuilder', '~> 2.0' From 960025cbf8a9891a09889da0219c22bd799994d2 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 15 Nov 2017 15:24:36 +0100 Subject: [PATCH 67/69] pushed Gemfile.lock --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 67860f20..901aa706 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -125,10 +125,10 @@ GEM eventmachine (1.0.9.1-java) excon (0.54.0) execjs (2.6.0) - factory_girl (4.5.0) + factory_bot (4.8.2) activesupport (>= 3.0.0) - factory_girl_rails (4.6.0) - factory_girl (~> 4.5.0) + factory_bot_rails (4.8.2) + factory_bot (~> 4.8.2) railties (>= 3.0.0) faraday (0.9.2) multipart-post (>= 1.2, < 3) @@ -401,7 +401,7 @@ DEPENDENCIES d3-rails database_cleaner docker-api (~> 1.25.0) - factory_girl_rails (~> 4.0) + factory_bot_rails (~> 4.8.2) faye-websocket forgery highline From 451d4536188e9c7d40aebdc3bd8ce9cfb1bcf043 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 15 Nov 2017 15:29:42 +0100 Subject: [PATCH 68/69] updated gitignore --- .gitignore | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 46060552..af16e87b 100644 --- a/.gitignore +++ b/.gitignore @@ -4,9 +4,11 @@ /config/secrets.yml /config/sendmail.yml /config/smtp.yml -/config/docker.yml.erb +/config/docker.yml*.erb /config/*.production.yml /config/*.staging.yml +/config/*.staging-epic.yml +/config/deploy/staging-epic.rb /coverage /log /public/assets @@ -14,6 +16,7 @@ /rubocop.html /tmp /vagrant/ +/.capistrano /.vagrant *.sublime-* /.idea From fc873cb0538adcd5f94517174139a6245e821d16 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 15 Nov 2017 15:37:42 +0100 Subject: [PATCH 69/69] second run of replacement regex from https://github.com/thoughtbot/factory_bot/blob/4-9-0-stable/UPGRADE_FROM_FACTORY_GIRL.md ... Nested calls seem to be fun.. --- spec/controllers/sessions_controller_spec.rb | 2 +- spec/controllers/submissions_controller_spec.rb | 4 ++-- spec/lib/docker_client_spec.rb | 2 +- spec/lib/file_tree_spec.rb | 12 ++++++------ spec/models/execution_environment_spec.rb | 2 +- spec/models/submission_spec.rb | 10 +++++----- spec/policies/internal_user_policy_spec.rb | 4 ++-- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 1d7fa877..480530da 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -191,7 +191,7 @@ describe SessionsController do describe 'GET #destroy_through_lti' do let(:request) { proc { get :destroy_through_lti, consumer_id: consumer.id, submission_id: submission.id } } - let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:dummy)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryBot.create(:dummy)) } before(:each) do #Todo replace session with lti_parameter diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index d2e92a48..2a3593c3 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -42,7 +42,7 @@ describe SubmissionsController do end context 'with a valid filename' do - let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:audio_video)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryBot.create(:audio_video)) } before(:each) { get :download_file, filename: file.name_with_extension, id: submission.id } context 'for a binary file' do @@ -92,7 +92,7 @@ describe SubmissionsController do end context 'with a valid filename' do - let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:audio_video)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryBot.create(:audio_video)) } before(:each) { get :render_file, filename: file.name_with_extension, id: submission.id } context 'for a binary file' do diff --git a/spec/lib/docker_client_spec.rb b/spec/lib/docker_client_spec.rb index 6f6674d7..5901c130 100644 --- a/spec/lib/docker_client_spec.rb +++ b/spec/lib/docker_client_spec.rb @@ -3,7 +3,7 @@ require 'seeds_helper' describe DockerClient, docker: true do let(:command) { 'whoami' } - let(:docker_client) { described_class.new(execution_environment: FactoryBot.build(:java), user: FactoryGirl.build(:admin)) } + let(:docker_client) { described_class.new(execution_environment: FactoryBot.build(:java), user: FactoryBot.build(:admin)) } let(:execution_environment) { FactoryBot.build(:java) } let(:image) { double } let(:submission) { FactoryBot.create(:submission) } diff --git a/spec/lib/file_tree_spec.rb b/spec/lib/file_tree_spec.rb index 0d6ac12a..92a4deb0 100644 --- a/spec/lib/file_tree_spec.rb +++ b/spec/lib/file_tree_spec.rb @@ -8,7 +8,7 @@ describe FileTree do context 'for a media file' do context 'for an audio file' do - let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_mp3)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_mp3)) } it 'is an audio file icon' do expect(file_icon).to include('fa-file-audio-o') @@ -16,7 +16,7 @@ describe FileTree do end context 'for an image file' do - let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_jpg)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_jpg)) } it 'is an image file icon' do expect(file_icon).to include('fa-file-image-o') @@ -24,7 +24,7 @@ describe FileTree do end context 'for a video file' do - let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_mp4)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_mp4)) } it 'is a video file icon' do expect(file_icon).to include('fa-file-video-o') @@ -42,7 +42,7 @@ describe FileTree do end context 'for an executable file' do - let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_py)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_py)) } it 'is a code file icon' do expect(file_icon).to include('fa-file-code-o') @@ -50,7 +50,7 @@ describe FileTree do end context 'for a renderable file' do - let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_svg)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_svg)) } it 'is a text file icon' do expect(file_icon).to include('fa-file-text-o') @@ -58,7 +58,7 @@ describe FileTree do end context 'for all other files' do - let(:file) { FactoryBot.build(:file, file_type: FactoryGirl.build(:dot_md)) } + let(:file) { FactoryBot.build(:file, file_type: FactoryBot.build(:dot_md)) } it 'is a generic file icon' do expect(file_icon).to include('fa-file-o') diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index 2dcee496..fc119b38 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -69,7 +69,7 @@ describe ExecutionEnvironment do describe '#valid_test_setup?' do context 'with a test command and a testing framework' do - before(:each) { execution_environment.update(test_command: FactoryBot.attributes_for(:ruby)[:test_command], testing_framework: FactoryGirl.attributes_for(:ruby)[:testing_framework]) } + before(:each) { execution_environment.update(test_command: FactoryBot.attributes_for(:ruby)[:test_command], testing_framework: FactoryBot.attributes_for(:ruby)[:testing_framework]) } it 'is valid' do expect(execution_environment.errors[:test_command]).to be_blank diff --git a/spec/models/submission_spec.rb b/spec/models/submission_spec.rb index 0d48eedf..50fcf285 100644 --- a/spec/models/submission_spec.rb +++ b/spec/models/submission_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe Submission do - let(:submission) { FactoryBot.create(:submission, exercise: FactoryGirl.create(:dummy)) } + let(:submission) { FactoryBot.create(:submission, exercise: FactoryBot.create(:dummy)) } it 'validates the presence of a cause' do expect(described_class.create.errors[:cause]).to be_present @@ -69,7 +69,7 @@ describe Submission do before(:each) do 10.times.each_with_index do |_, index| - FactoryBot.create(:submission, exercise: submission.exercise, user: (index.even? ? user : FactoryGirl.create(:external_user))) + FactoryBot.create(:submission, exercise: submission.exercise, user: (index.even? ? user : FactoryBot.create(:external_user))) end end @@ -99,7 +99,7 @@ describe Submission do it 'does not redirect other users' do 9.times do |i| - submission = FactoryBot.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) + submission = FactoryBot.build(:submission, exercise: exercise, user: FactoryBot.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) expect(submission.send(:redirect_to_feedback?)).to be_falsey end end @@ -116,7 +116,7 @@ describe Submission do it 'does not redirect other users' do 9.times do |i| - submission = FactoryBot.build(:submission, exercise: exercise, user: FactoryGirl.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) + submission = FactoryBot.build(:submission, exercise: exercise, user: FactoryBot.build(:external_user, id: (11 - exercise.created_at.to_i % 10) - i - 1)) expect(submission.send(:redirect_to_feedback?)).to be_falsey end end @@ -128,7 +128,7 @@ describe Submission do it 'sends nobody to feedback page' do 30.times do |i| - submission = FactoryBot.create(:submission, exercise: exercise, user: FactoryGirl.create(:external_user)) + submission = FactoryBot.create(:submission, exercise: exercise, user: FactoryBot.create(:external_user)) expect(submission.send(:redirect_to_feedback?)).to be_falsey end end diff --git a/spec/policies/internal_user_policy_spec.rb b/spec/policies/internal_user_policy_spec.rb index 8a50988f..0644e358 100644 --- a/spec/policies/internal_user_policy_spec.rb +++ b/spec/policies/internal_user_policy_spec.rb @@ -18,7 +18,7 @@ describe InternalUserPolicy do context 'with an admin user' do it 'grants access to no one' do [:admin, :external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryGirl.build(:admin)) + expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryBot.build(:admin)) end end end @@ -27,7 +27,7 @@ describe InternalUserPolicy do it 'grants access to admins only' do expect(subject).to permit(FactoryBot.build(:admin), InternalUser.new) [:external_user, :teacher].each do |factory_name| - expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryGirl.build(:teacher)) + expect(subject).not_to permit(FactoryBot.build(factory_name), FactoryBot.build(:teacher)) end end end