diff --git a/app/controllers/concerns/redirect_behavior.rb b/app/controllers/concerns/redirect_behavior.rb index 20cd6abc..00de2517 100644 --- a/app/controllers/concerns/redirect_behavior.rb +++ b/app/controllers/concerns/redirect_behavior.rb @@ -61,9 +61,9 @@ module RedirectBehavior def redirect_to_user_feedback uef = UserExerciseFeedback.find_by(exercise: @submission.exercise, user: current_user) url = if uef - edit_user_exercise_feedback_path(uef) + edit_exercise_user_exercise_feedback_path(uef, exercise_id: @submission.exercise) else - new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: @submission.exercise.id}) + new_exercise_user_exercise_feedback_path(exercise_id: @submission.exercise) end respond_to do |format| diff --git a/app/controllers/user_exercise_feedbacks_controller.rb b/app/controllers/user_exercise_feedbacks_controller.rb index 319d2b42..e5dc7c0e 100644 --- a/app/controllers/user_exercise_feedbacks_controller.rb +++ b/app/controllers/user_exercise_feedbacks_controller.rb @@ -3,6 +3,7 @@ class UserExerciseFeedbacksController < ApplicationController include CommonBehavior + before_action :set_exercise_and_authorize before_action :set_user_exercise_feedback, only: %i[edit update destroy] before_action :set_presets, only: %i[new edit create update] @@ -23,24 +24,15 @@ class UserExerciseFeedbacksController < ApplicationController end def new - exercise_id = if params[:user_exercise_feedback].nil? - params[:exercise_id] - else - params[:user_exercise_feedback][:exercise_id] - end - @exercise = Exercise.find(exercise_id) @uef = UserExerciseFeedback.find_or_initialize_by(user: current_user, exercise: @exercise) authorize! end - def edit - authorize! - end + def edit; end def create Sentry.set_extras(params: uef_params) - @exercise = Exercise.find(uef_params[:exercise_id]) rfc = RequestForComment.unsolved.where(exercise: @exercise, user: current_user).first submission = begin current_contributor.submissions.where(exercise: @exercise).order(created_at: :desc).first @@ -48,22 +40,20 @@ class UserExerciseFeedbacksController < ApplicationController nil end - if @exercise - @uef = UserExerciseFeedback.find_or_initialize_by(user: current_user, exercise: @exercise) - @uef.update(uef_params) - authorize! - if validate_inputs(uef_params) - path = - if rfc && submission && submission.normalized_score.to_d == BigDecimal('1.0') - request_for_comment_path(rfc) - else - implement_exercise_path(@exercise) - end - create_and_respond(object: @uef, path: proc { path }) - else - flash.now[:danger] = t('shared.message_failure') - redirect_back fallback_location: user_exercise_feedback_path(@uef) - end + @uef = UserExerciseFeedback.find_or_initialize_by(user: current_user, exercise: @exercise) + @uef.assign_attributes(uef_params) + authorize! + if validate_inputs(uef_params) + path = + if rfc && submission && submission.normalized_score.to_d == BigDecimal('1.0') + request_for_comment_path(rfc) + else + implement_exercise_path(@exercise) + end + create_and_respond(object: @uef, path: proc { path }) + else + flash.now[:danger] = t('shared.message_failure') + redirect_back fallback_location: exercise_user_exercise_feedback_path(@uef) end end @@ -75,7 +65,7 @@ class UserExerciseFeedbacksController < ApplicationController end rfc = RequestForComment.unsolved.where(exercise: @exercise, user: current_user).first authorize! - if @exercise && validate_inputs(uef_params) + if validate_inputs(uef_params) path = if rfc && submission && submission.normalized_score.to_d == BigDecimal('1.0') request_for_comment_path(rfc) @@ -85,7 +75,7 @@ class UserExerciseFeedbacksController < ApplicationController update_and_respond(object: @uef, params: uef_params, path:) else flash.now[:danger] = t('shared.message_failure') - redirect_back fallback_location: user_exercise_feedback_path(@uef) + redirect_back fallback_location: exercise_user_exercise_feedback_path(@uef) end end @@ -97,16 +87,19 @@ class UserExerciseFeedbacksController < ApplicationController private def authorize! - authorize(@uef || @uefs) + raise Pundit::NotAuthorizedError if @uef.present? && @uef.exercise != @exercise + + authorize(@uef) end - def to_s - name + def set_exercise_and_authorize + @exercise = Exercise.find(params[:exercise_id]) + authorize(@exercise, :implement?) end def set_user_exercise_feedback @uef = UserExerciseFeedback.find(params[:id]) - @exercise = @uef.exercise + authorize! end def set_presets diff --git a/app/models/user_exercise_feedback.rb b/app/models/user_exercise_feedback.rb index 1de5a100..6ca7e3f2 100644 --- a/app/models/user_exercise_feedback.rb +++ b/app/models/user_exercise_feedback.rb @@ -22,4 +22,8 @@ class UserExerciseFeedback < ApplicationRecord .order(created_at: :desc) .first end + + def self.parent_resource + Exercise + end end diff --git a/app/policies/user_exercise_feedback_policy.rb b/app/policies/user_exercise_feedback_policy.rb index 81861e92..fed5a2f2 100644 --- a/app/policies/user_exercise_feedback_policy.rb +++ b/app/policies/user_exercise_feedback_policy.rb @@ -8,4 +8,9 @@ class UserExerciseFeedbackPolicy < AdminOrAuthorPolicy def new? everyone end + + def show? + # We don't have a show action, so no one can show a UserExerciseFeedback directly. + no_one + end end diff --git a/app/views/user_exercise_feedbacks/_form.html.slim b/app/views/user_exercise_feedbacks/_form.html.slim index 8a7564d0..3acc3468 100644 --- a/app/views/user_exercise_feedbacks/_form.html.slim +++ b/app/views/user_exercise_feedbacks/_form.html.slim @@ -1,4 +1,4 @@ -= form_for(@uef) do |f| += form_for([@exercise, @uef]) do |f| div h1 id="exercise-headline" = t('activerecord.models.user_exercise_feedback.one') + " " + @exercise.title diff --git a/config/routes.rb b/config/routes.rb index d045b425..ad0a8699 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -89,7 +89,7 @@ Rails.application.routes.draw do get :working_times post :intervention get :statistics - get :feedback + get :feedback, path: 'feedbacks' get :reload get 'study_group_dashboard/:study_group_id', to: 'exercises#study_group_dashboard' post :export_external_check @@ -97,6 +97,7 @@ Rails.application.routes.draw do end resources :programming_groups + resources :user_exercise_feedbacks, except: %i[show index], path: 'feedbacks' end resources :programming_groups, except: %i[new create] @@ -118,8 +119,6 @@ Rails.application.routes.draw do resources :tips - resources :user_exercise_feedbacks, except: %i[show index] - resources :external_users, only: %i[index show], concerns: :statistics do resources :exercises do get :statistics, to: 'exercises#external_user_statistics', on: :member diff --git a/spec/controllers/submissions_controller_spec.rb b/spec/controllers/submissions_controller_spec.rb index 395b7293..52ce2d6f 100644 --- a/spec/controllers/submissions_controller_spec.rb +++ b/spec/controllers/submissions_controller_spec.rb @@ -144,13 +144,13 @@ RSpec.describe SubmissionsController do context 'without any previous feedback' do let(:uef) { nil } - expect_redirect { new_user_exercise_feedback_path(user_exercise_feedback: {exercise_id: submission.exercise.id}) } + expect_redirect { new_exercise_user_exercise_feedback_path(exercise_id: submission.exercise) } end context 'with a previous feedback for the same exercise' do let(:uef) { create(:user_exercise_feedback, exercise:, user: current_user) } - expect_redirect { edit_user_exercise_feedback_path(uef) } + expect_redirect { edit_exercise_user_exercise_feedback_path(uef, exercise_id: exercise) } end end