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 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' 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 diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb index 2d145e34..f6ff6887 100644 --- a/app/assets/javascripts/editor/editor.js.erb +++ b/app/assets/javascripts/editor/editor.js.erb @@ -580,7 +580,7 @@ configureEditors: function () { * */ initializeInterventionTimer: function() { - if ($('#editor').data('rfc-interventions') == true || $('#editor').data('break-interventions') == true) { // split in break or rfc intervention + if ($('#editor').data('rfc-interventions') || $('#editor').data('break-interventions')) { // split in break or rfc intervention window.onblur = function() { window.blurred = true; }; window.onfocus = function() { window.blurred = false; }; diff --git a/app/assets/stylesheets/exercises.css.scss b/app/assets/stylesheets/exercises.css.scss index e46d59af..2ef110d7 100644 --- a/app/assets/stylesheets/exercises.css.scss +++ b/app/assets/stylesheets/exercises.css.scss @@ -95,14 +95,43 @@ a.file-heading { left: 0; } -.feedback { - .text { +.feedback-page { + .header { + font-weight: bold; margin-bottom: 10px; } - .difficulty { - font-weight: bold; + + .value { + border: 1px solid grey; + padding: 10px; + margin-bottom: 10px; } - .worktime { + + .no-feedback { font-weight: bold; + margin-top: 50px; + } + + .feedback-header { + display: flex; + + .username { + flex-grow: 1; + font-weight: bold; + } + + .date {} + } + + .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 969e5e4f..fa48363e 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -24,6 +24,9 @@ class ExercisesController < ApplicationController 3 end + def max_intervention_count_per_exercise + 1 + end def java_course_token "702cbd2a-c84c-4b37-923a-692d7d1532d0" @@ -148,8 +151,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 @@ -171,17 +173,19 @@ class ExercisesController < ApplicationController def implement redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists? user_solved_exercise = @exercise.has_user_solved(current_user) - user_got_enough_interventions = UserExerciseIntervention.where(user: current_user).where("created_at >= ?", Time.zone.now.beginning_of_day).count >= max_intervention_count_per_day - is_java_course = @course_token && @course_token.eql?(java_course_token) + count_interventions_today = UserExerciseIntervention.where(user: current_user).where("created_at >= ?", Time.zone.now.beginning_of_day).count + user_got_intervention_in_exercise = UserExerciseIntervention.where(user: current_user, exercise: @exercise).size >= max_intervention_count_per_exercise + user_got_enough_interventions = count_interventions_today >= max_intervention_count_per_day or user_got_intervention_in_exercise + is_java_course = @course_token and @course_token.eql?(java_course_token) user_intervention_group = UserGroupSeparator.getInterventionGroup(current_user) case user_intervention_group when :no_intervention when :break_intervention - @show_break_interventions = (user_solved_exercise || !is_java_course || user_got_enough_interventions) ? "false" : "true" + @show_break_interventions = (not user_solved_exercise and is_java_course and not user_got_enough_interventions) ? "true" : "false" when :rfc_intervention - @show_rfc_interventions = (user_solved_exercise || !is_java_course || user_got_enough_interventions) ? "false" : "true" + @show_rfc_interventions = (not user_solved_exercise and is_java_course and not user_got_enough_interventions) ? "true" : "false" end @search = Search.new @@ -387,10 +391,13 @@ 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 @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) @@ -400,24 +407,30 @@ 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 < 5) } + 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)}) } + format.html {redirect_to(rfc)} + format.json {render(json: {redirect: url_for(rfc)})} end return end end else # redirect to feedback page if score is less than 100 percent - redirect_to_user_feedback - return + if @exercise.needs_more_feedback? + redirect_to_user_feedback + else + redirect_to_lti_return_path + end + return end redirect_to_lti_return_path end diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 8e7a8e39..73db8c25 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -390,7 +390,7 @@ class SubmissionsController < ApplicationController content += "#{request.base_url}/evaluate\n" @submission.files.each do |file| file_path = file.path.to_s == '' ? file.name_with_extension : File.join(file.path, file.name_with_extension) - content += "#{file_path}=#{file.id.to_s}\n" + content += "#{file_path}=#{file.file_id.to_s}\n" end File.open(path, "w+") do |f| f.write(content) diff --git a/app/controllers/user_exercise_feedbacks_controller.rb b/app/controllers/user_exercise_feedbacks_controller.rb index 0d1e1925..8abffd66 100644 --- a/app/controllers/user_exercise_feedbacks_controller.rb +++ b/app/controllers/user_exercise_feedbacks_controller.rb @@ -2,6 +2,7 @@ class UserExerciseFeedbacksController < ApplicationController include CommonBehavior 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')], @@ -19,10 +20,15 @@ class UserExerciseFeedbacksController < ApplicationController [4,t('user_exercise_feedback.estimated_time_more_30')]] end - def authorize! - authorize(@uef) + def index + @search = UserExerciseFeedback.all.search params[:q] + @uefs = @search.result.includes(:execution_environment).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 +55,8 @@ class UserExerciseFeedbacksController < ApplicationController end def destroy - destroy_and_respond(object: @tag) + authorize! + destroy_and_respond(object: @uef) end def edit @@ -58,11 +65,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 +91,12 @@ class UserExerciseFeedbacksController < ApplicationController end end + private + + def authorize! + authorize(@uef || @uefs) + end + def to_s name end @@ -98,6 +106,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 +128,4 @@ class UserExerciseFeedbacksController < ApplicationController end end -end \ No newline at end of file +end diff --git a/app/models/exercise.rb b/app/models/exercise.rb index b7487399..f03d6641 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..d75d6019 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 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/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/application/_navigation.html.slim b/app/views/application/_navigation.html.slim index 668ccab4..a2604c7a 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, + - models = [ExecutionEnvironment, Exercise, ExerciseCollection, ProxyExercise, Tag, Consumer, CodeHarborLink, UserExerciseFeedback, ErrorTemplate, ErrorTemplateAttribute, ExternalUser, FileType, FileTemplate, InternalUser].sort_by {|model| model.model_name.human(count: 2) } - models.each do |model| - if policy(model).index? 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/external_users/statistics.html.slim b/app/views/exercises/external_users/statistics.html.slim index 8b5c3b30..06132754 100644 --- a/app/views/exercises/external_users/statistics.html.slim +++ b/app/views/exercises/external_users/statistics.html.slim @@ -54,8 +54,6 @@ h1 = "#{@exercise} (external user #{@external_user})" -submission_or_intervention.testruns.each do |run| - if run.passed .unit-test-result.positive-result title=run.output - - elsif run.failed - .unit-test-result.negative-result title=run.output - else .unit-test-result.unknown-result title=run.output td = Time.at(deltas[1..index].inject(:+)).utc.strftime("%H:%M:%S") if index > 0 diff --git a/app/views/exercises/feedback.html.slim b/app/views/exercises/feedback.html.slim index 07b88167..3cdbc30f 100644 --- a/app/views/exercises/feedback.html.slim +++ b/app/views/exercises/feedback.html.slim @@ -1,15 +1,24 @@ -h1 = @exercise +h1 = link_to(@exercise, exercise_path(@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 +.feedback-page + .header = t('activerecord.attributes.exercise.description') + .value = render_markdown(@exercise.description) -= render('shared/pagination', collection: @feedbacks) + .header = t('activerecord.models.user_exercise_feedback.other') + - if @feedbacks.nil? or @feedbacks.size == 0 + .no-feedback = t('user_exercise_feedback.no_feedback') + + ul.list-unstyled.panel-group + - @feedbacks.each do |feedback| + li.panel.panel-default + .panel-heading role="tab" id="heading" + div.clearfix.feedback-header + span.username = link_to(feedback.user.name, statistics_external_user_exercise_path(id: @exercise.id, external_user_id: feedback.user.id)) + span.date = feedback.created_at + .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 18c2cd98..47198fbb 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/app/views/user_exercise_feedbacks/index.html.slim b/app/views/user_exercise_feedbacks/index.html.slim new file mode 100644 index 00000000..52249229 --- /dev/null +++ b/app/views/user_exercise_feedbacks/index.html.slim @@ -0,0 +1,27 @@ +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 + 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/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 9754caac..1e7d1d00 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 @@ -128,6 +127,10 @@ de: use_anomaly_detection: "Abweichungen in der Arbeitszeit erkennen" updated_at: "Letzte Änderung" exercises: "Aufgaben" + user_exercise_feedback: + user: "Autor" + exercise: "Aufgabe" + feedback_text: "Feedback Text" models: code_harbor_link: one: CodeHarbor-Link @@ -692,6 +695,7 @@ 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:" + no_feedback: "Es wurde noch kein Feedback zu dieser Aufgabe gegeben." 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 777ee9b7..d5e8cd4d 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 @@ -128,6 +127,10 @@ en: use_anomaly_detection: "Enable Worktime Anomaly Detection" updated_at: "Last Update" exercises: "Exercises" + user_exercise_feedback: + user: "Author" + exercise: "Exercise" + feedback_text: "Feedback Text" models: code_harbor_link: one: CodeHarbor Link @@ -692,6 +695,7 @@ 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:" + no_feedback: "There is no feedback for this exercise yet." error_templates: hints: signature: "A regular expression in Ruby syntax without leading and trailing \"/\"" 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/migrate/20171120153705_add_timestamps_to_user_exercise_feedbacks.rb b/db/migrate/20171120153705_add_timestamps_to_user_exercise_feedbacks.rb new file mode 100644 index 00000000..4529d527 --- /dev/null +++ b/db/migrate/20171120153705_add_timestamps_to_user_exercise_feedbacks.rb @@ -0,0 +1,6 @@ +class AddTimestampsToUserExerciseFeedbacks < ActiveRecord::Migration + def up + add_column :user_exercise_feedbacks, :created_at, :datetime, null: false, default: Time.now + add_column :user_exercise_feedbacks, :updated_at, :datetime, null: false, default: Time.now + end +end diff --git a/db/schema.rb b/db/schema.rb index 474aa6f1..828e8e41 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -128,19 +128,18 @@ ActiveRecord::Schema.define(version: 20171210172208) 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 add_index "exercises", ["id"], name: "index_exercises_on_id", using: :btree @@ -355,13 +354,15 @@ ActiveRecord::Schema.define(version: 20171210172208) do end create_table "user_exercise_feedbacks", force: :cascade do |t| - t.integer "exercise_id", null: false - t.integer "user_id", null: false - t.string "user_type", null: false - t.integer "difficulty" - t.integer "working_time_seconds" - t.string "feedback_text" - t.integer "user_estimated_worktime" + t.integer "exercise_id", null: false + t.integer "user_id", null: false + t.string "user_type", null: false + t.integer "difficulty" + t.integer "working_time_seconds" + t.string "feedback_text" + t.integer "user_estimated_worktime" + t.datetime "created_at", default: '2017-11-20 18:20:25', null: false + t.datetime "updated_at", default: '2017-11-20 18:20:25', null: false end create_table "user_exercise_interventions", force: :cascade do |t| 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/lib/xikolo/client.rb b/lib/xikolo/client.rb deleted file mode 100644 index 4e6f22b2..00000000 --- a/lib/xikolo/client.rb +++ /dev/null @@ -1,59 +0,0 @@ -class Xikolo::Client - def self.get_user(user_id) - params = {:user_id => user_id} - response = get_request(user_profile_url(user_id), params) - if response - return JSON.parse(response) - else - return nil - end - end - - def self.user_profile_url(user_id) - return url + 'v2/users/' + user_id - end - - def self.post_request(url, params) - begin - return RestClient.post url, params, http_header - rescue - return nil - end - end - - def self.get_request(url, params) - begin - return RestClient.get url, {:params => params}.merge(http_header) - rescue - return nil - end - end - - def self.http_header - return {:accept => accept, :authorization => token} - end - - def self.url - @url ||= CodeOcean::Config.new(:code_ocean).read.fetch(:xikolo_api_url, 'http://localhost:3000/api/') #caches this with ||=, second value of fetch is default value - end - - def self.accept - 'application/vnd.xikolo.v1, application/vnd.api+json, application/json' - end - - def self.token - 'Token token='+Rails.application.secrets.openhpi_api_token#+'"' - end - - private - - def authenticate_with_user - params = {:email => "admin@openhpi.de", :password => "admin"} - response = post_request(authentication_url, params) - @token = 'Token token="'+JSON.parse(response)['token']+'"' - end - - def self.authentication_url - return @url + 'authenticate' - end -end \ No newline at end of file diff --git a/lib/xikolo/user_client.rb b/lib/xikolo/user_client.rb deleted file mode 100644 index c681c999..00000000 --- a/lib/xikolo/user_client.rb +++ /dev/null @@ -1,15 +0,0 @@ -class Xikolo::UserClient - def self.get(user_id) - user = Xikolo::Client.get_user(user_id) - - # return default values if user is not found or if there is a server issue: - if user - name = user.dig('data', 'attributes', 'name') || "User " + user_id - user_visual = user.dig('data', 'attributes', 'avatar_url') || ActionController::Base.helpers.image_path('default.png') - language = user.dig('data', 'attributes', 'language') || "DE" - return {display_name: name, user_visual: user_visual, language: language} - else - return {display_name: "User " + user_id, user_visual: ActionController::Base.helpers.image_path('default.png'), language: "DE"} - end - end -end \ No newline at end of file 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..480530da 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: FactoryBot.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..2a3593c3 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: FactoryBot.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: 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/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 ea588faf..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." @@ -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/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 new file mode 100644 index 00000000..42c5d803 --- /dev/null +++ b/spec/factories/user_exercise_feedback.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :user_exercise_feedback, class: UserExerciseFeedback do + created_by_external_user + feedback_text 'Most suitable exercise ever' + end + +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..5901c130 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: FactoryBot.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..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) { FactoryGirl.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) { FactoryGirl.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) { FactoryGirl.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') @@ -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: 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) { FactoryGirl.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) { FactoryGirl.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') @@ -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..fc119b38 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: FactoryBot.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 64f4e49e..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) { FactoryGirl.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 @@ -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 : FactoryBot.create(:external_user))) end end @@ -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) {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 + end + + it 'does not redirect other users' do + 9.times do |i| + 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 + end + + context 'with little exercise feedback' do + 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 + end + + it 'does not redirect other users' do + 9.times do |i| + 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 + end + + context 'with enough exercise feedback' do + 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 = FactoryBot.create(:submission, exercise: exercise, user: FactoryBot.create(:external_user)) + expect(submission.send(:redirect_to_feedback?)).to be_falsey + end + end + end + 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..0644e358 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), FactoryBot.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), FactoryBot.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