From e0c2c7b806eefbbd38b6395742605b1db6c05f87 Mon Sep 17 00:00:00 2001 From: Janis4411 Date: Mon, 8 Aug 2022 10:37:29 +0200 Subject: [PATCH] Hide score button if exercise has no tests We check for all teacher-defined assessments (linter and unit tests) to determine whether scoring should be possible --- app/controllers/exercises_controller.rb | 2 + app/controllers/submissions_controller.rb | 6 +-- app/models/exercise.rb | 4 ++ app/policies/exercise_policy.rb | 6 ++- spec/controllers/exercises_controller_spec.rb | 5 ++- spec/features/editor_spec.rb | 21 ++++++++++- spec/models/exercise_spec.rb | 37 +++++++++++++++++++ spec/policies/exercise_policy_spec.rb | 24 +++++++++++- 8 files changed, 97 insertions(+), 8 deletions(-) diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index d8c01ef8..b23cb460 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -324,6 +324,8 @@ class ExercisesController < ApplicationController end end + @embed_options[:disable_score] = true unless @exercise.teacher_defined_assessment? + @hide_rfc_button = @embed_options[:disable_rfc] @search = Search.new diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 680af5d7..cdc687a3 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -83,7 +83,7 @@ class SubmissionsController < ApplicationController client_socket = tubesock client_socket.onopen do |_event| - kill_client_socket(client_socket) if @embed_options[:disable_run] + return kill_client_socket(client_socket) if @embed_options[:disable_run] end client_socket.onclose do |_event| @@ -199,7 +199,7 @@ class SubmissionsController < ApplicationController hijack do |tubesock| tubesock.onopen do |_event| switch_locale do - kill_client_socket(tubesock) if @embed_options[:disable_score] + return kill_client_socket(tubesock) if @embed_options[:disable_score] || !@submission.exercise.teacher_defined_assessment? # The score is stored separately, we can forward it to the client immediately tubesock.send_data(JSON.dump(@submission.calculate_score)) @@ -226,7 +226,7 @@ class SubmissionsController < ApplicationController hijack do |tubesock| tubesock.onopen do |_event| switch_locale do - kill_client_socket(tubesock) if @embed_options[:disable_run] + return kill_client_socket(tubesock) if @embed_options[:disable_run] # The score is stored separately, we can forward it to the client immediately tubesock.send_data(JSON.dump(@submission.test(@file))) diff --git a/app/models/exercise.rb b/app/models/exercise.rb index 7711dbe7..ed7ad7d3 100644 --- a/app/models/exercise.rb +++ b/app/models/exercise.rb @@ -205,6 +205,10 @@ class Exercise < ApplicationRecord " end + def teacher_defined_assessment? + files.any?(&:teacher_defined_assessment?) + end + def get_working_times_for_study_group(study_group_id, user = nil) user_progress = [] additional_user_data = [] diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index 81fab186..497e6a42 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -29,10 +29,14 @@ class ExercisePolicy < AdminOrAuthorPolicy define_method(action) { (admin? || teacher_in_study_group? || author?) && @user.codeharbor_link } end - %i[implement? working_times? intervention? search? submit? reload?].each do |action| + %i[implement? working_times? intervention? search? reload?].each do |action| define_method(action) { everyone } end + def submit? + everyone && @record.teacher_defined_assessment? + end + class Scope < Scope def resolve if @user.admin? diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 57479b5c..ad245b64 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -6,7 +6,10 @@ describe ExercisesController do let(:exercise) { create(:dummy) } let(:user) { create(:admin) } - before { allow(controller).to receive(:current_user).and_return(user) } + before do + create(:test_file, context: exercise) + allow(controller).to receive(:current_user).and_return(user) + end describe 'PUT #batch_update' do let(:attributes) { {public: 'true'} } diff --git a/spec/features/editor_spec.rb b/spec/features/editor_spec.rb index 5892354b..193aad2d 100644 --- a/spec/features/editor_spec.rb +++ b/spec/features/editor_spec.rb @@ -23,6 +23,7 @@ describe 'Editor', js: true do }] end let(:user) { create(:teacher) } + let(:exercise_without_test) { create(:tdd) } before do visit(sign_in_path) @@ -93,12 +94,28 @@ describe 'Editor', js: true do end end + context 'when an exercise has one or more teacher-defined assessments' do + it 'displays the score button' do + visit(implement_exercise_path(exercise)) + expect(page).to have_content(exercise.title) + expect(page).to have_content(I18n.t('exercises.editor.score')) + end + end + + context 'when an exercise has no teacher-defined assessment' do + it 'disables the score button' do + visit(implement_exercise_path(exercise_without_test)) + expect(page).to have_content(exercise_without_test.title) + expect(page).not_to have_content(I18n.t('exercises.editor.score')) + end + end + it 'contains a button for submitting the exercise' do submission = build(:submission, user: user, exercise: exercise) allow(submission).to receive(:calculate_score).and_return(scoring_response) allow(Submission).to receive(:find).and_return(submission) click_button(I18n.t('exercises.editor.score')) - expect(page).not_to have_css('#submit_outdated') - expect(page).to have_css('#submit') + expect(page).not_to have_content(I18n.t('exercises.editor.tooltips.exercise_deadline_passed')) + expect(page).to have_content(I18n.t('exercises.editor.submit')) end end diff --git a/spec/models/exercise_spec.rb b/spec/models/exercise_spec.rb index 57bd66c9..8bbb7014 100644 --- a/spec/models/exercise_spec.rb +++ b/spec/models/exercise_spec.rb @@ -124,4 +124,41 @@ describe Exercise do expect(exercise.duplicate).to be_a(described_class) end end + + describe '#teacher_defined_assessment?' do + let(:exercise) { create(:dummy) } + + context 'when no assessment is defined' do + it 'returns false' do + expect(exercise).not_to be_teacher_defined_assessment + end + end + + context 'when unit tests are defined' do + before { create(:test_file, context: exercise) } + + it 'returns true' do + expect(exercise).to be_teacher_defined_assessment + end + end + + context 'when linter tests are defined' do + before { create(:test_file, context: exercise, role: 'teacher_defined_linter') } + + it 'returns true' do + expect(exercise).to be_teacher_defined_assessment + end + end + + context 'when unit and linter tests are defined' do + before do + create(:test_file, context: exercise) + create(:test_file, context: exercise, role: 'teacher_defined_linter') + end + + it 'returns true' do + expect(exercise).to be_teacher_defined_assessment + end + end + end end diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index 8fad12a6..d2b8aca3 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -112,7 +112,7 @@ describe ExercisePolicy do end end - %i[implement? submit?].each do |action| + %i[implement?].each do |action| permissions(action) do it 'grants access to anyone' do %i[admin external_user teacher].each do |factory_name| @@ -122,6 +122,28 @@ describe ExercisePolicy do end end + %i[submit?].each do |action| + permissions(action) do + context 'when teacher-defined assessments are available' do + before { create(:test_file, context: exercise) } + + it 'grants access to anyone' do + %i[admin external_user teacher].each do |factory_name| + expect(policy).to permit(build(factory_name), exercise) + end + end + end + + context 'when teacher-defined assessments are not available' do + it 'does not grant access to anyone' do + %i[admin external_user teacher].each do |factory_name| + expect(policy).not_to permit(build(factory_name), exercise) + end + end + end + end + end + describe ExercisePolicy::Scope do describe '#resolve' do let(:admin) { create(:admin) }