Skip to content

Commit 3b0d2b7

Browse files
committed
Ensure HTTP status See Other (303) for form submissions
Turbo requires us to use the HTTP status code 303 for successful form submissions after stateful server changes. See https://turbo.hotwired.dev/handbook/drive#redirecting-after-a-form-submission
1 parent 7edfe85 commit 3b0d2b7

24 files changed

+73
-73
lines changed

app/controllers/application_controller.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,15 +142,15 @@ def render_error(message, status)
142142
format.any do
143143
# Prevent redirect loop
144144
if request.url == request.referer || request.referer&.match?(sign_in_path)
145-
redirect_to :root, alert: message
145+
redirect_to :root, alert: message, status: :see_other
146146
# Redirect to main domain if the request originated from our render_host
147147
elsif request.path == '/' && request.host == RENDER_HOST
148-
redirect_to Rails.application.config.action_mailer.default_url_options, allow_other_host: true
148+
redirect_to Rails.application.config.action_mailer.default_url_options, allow_other_host: true, status: :see_other
149149
elsif current_user.nil? && status == :unauthorized
150150
session[:return_to_url] = request.fullpath if current_user.nil?
151-
redirect_to sign_in_path, alert: t('application.not_signed_in')
151+
redirect_to sign_in_path, alert: t('application.not_signed_in'), status: :see_other
152152
else
153-
redirect_back fallback_location: :root, allow_other_host: false, alert: message
153+
redirect_back fallback_location: :root, allow_other_host: false, alert: message, status: :see_other
154154
end
155155
end
156156
format.json { render json: {error: message}, status: }

app/controllers/code_ocean/files_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ def create_and_respond(options = {})
6767
filename = "#{@object.path || ''}/#{@object.name || ''}#{@object.file_type.try(:file_extension) || ''}"
6868
format.html do
6969
flash[:danger] = t('code_ocean/files.error.filename', name: filename)
70-
redirect_to(options[:path])
70+
redirect_to options[:path], status: :see_other
7171
end
7272
format.json { render(json: @object.errors, status: :unprocessable_content) }
7373
end

app/controllers/concerns/common_behavior.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def destroy_and_respond(options = {})
2424
@object.destroy
2525
respond_to do |format|
2626
path = options[:path] || send(:"#{@object.class.model_name.collection}_path")
27-
format.html { redirect_to(path, notice: t('shared.object_destroyed', model: @object.class.model_name.human)) }
27+
format.html { redirect_to path, notice: t('shared.object_destroyed', model: @object.class.model_name.human), status: :see_other }
2828
format.json { head(:no_content) }
2929
end
3030
end
@@ -36,7 +36,7 @@ def respond_with_invalid_object(format, options = {})
3636
end
3737

3838
def respond_with_valid_object(format, options = {})
39-
format.html { redirect_to(options[:path], notice: options[:notice]) }
39+
format.html { redirect_to options[:path], notice: options[:notice], status: :see_other }
4040
format.json { render(:show, location: @object, status: options[:status]) }
4141
end
4242
private :respond_with_valid_object

app/controllers/concerns/lti.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ def return_to_consumer(options = {})
110110
# The `lti_msg` is displayed to the user as an information.
111111
return_url = consumer_return_url(@provider, options)
112112
if return_url && URI.parse(return_url).absolute?
113-
redirect_to(return_url, allow_other_host: true)
113+
redirect_to return_url, allow_other_host: true, status: :see_other
114114
else
115115
flash[:danger] = options[:lti_errormsg]
116116
flash[:info] = options[:lti_msg]
117-
redirect_to(:root)
117+
redirect_to :root, status: :see_other
118118
end
119119
end
120120

app/controllers/concerns/redirect_behavior.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def redirect_after_submit
2727
def redirect_to_community_solution
2828
url = edit_community_solution_path(@community_solution, lock_id: @community_solution_lock.id)
2929
respond_to do |format|
30-
format.html { redirect_to(url) }
30+
format.html { redirect_to url, status: :see_other }
3131
format.json { render(json: {redirect: url}) }
3232
end
3333
end
@@ -67,7 +67,7 @@ def redirect_to_user_feedback
6767
end
6868

6969
respond_to do |format|
70-
format.html { redirect_to(url) }
70+
format.html { redirect_to url, status: :see_other }
7171
format.json { render(json: {redirect: url}) }
7272
end
7373
end
@@ -81,7 +81,7 @@ def redirect_to_unsolved_rfc(own: false)
8181
@rfc.increment!(:times_featured) unless own # rubocop:disable Rails/SkipsModelValidations
8282

8383
respond_to do |format|
84-
format.html { redirect_to(@rfc) }
84+
format.html { redirect_to @rfc, status: :see_other }
8585
format.json { render(json: {redirect: url_for(@rfc)}) }
8686
end
8787
end
@@ -111,7 +111,7 @@ def redirect_to_lti_return_path
111111

112112
path = lti_return_path(submission_id: @submission.id)
113113
respond_to do |format|
114-
format.html { redirect_to(path) }
114+
format.html { redirect_to path, status: :see_other }
115115
format.json { render(json: {redirect: path}) }
116116
end
117117
end

app/controllers/concerns/webauthn/authentication.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def authenticate(user)
3939
_sign_in_as(user)
4040
return _finalize_login(user) if user.fully_authenticated?
4141

42-
redirect_to new_webauthn_credential_authentication_path
42+
redirect_to new_webauthn_credential_authentication_path, status: :see_other
4343
user
4444
end
4545

@@ -92,7 +92,7 @@ def destroy_webauthn_cookie(_user = nil)
9292

9393
# Redirect to the WebAuthn authentication page if the user has not completed the WebAuthn authentication.
9494
def _require_webauthn_credential_authentication(user = current_user)
95-
redirect_to new_webauthn_credential_authentication_path unless _webauthn_credential_authentication_completed?(user)
95+
redirect_to(new_webauthn_credential_authentication_path, status: :see_other) unless _webauthn_credential_authentication_completed?(user)
9696
end
9797

9898
# Finish the login process and redirect the user to the return_to_url.

app/controllers/error_template_attributes_controller.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def create
4242
respond_to do |format|
4343
if @error_template_attribute.save
4444
format.html do
45-
redirect_to @error_template_attribute, notice: t('shared.object_created', model: @error_template_attribute.class.model_name.human)
45+
redirect_to @error_template_attribute, notice: t('shared.object_created', model: @error_template_attribute.class.model_name.human), status: :see_other
4646
end
4747
format.json { render :show, status: :created, location: @error_template_attribute }
4848
else
@@ -59,7 +59,7 @@ def update
5959
respond_to do |format|
6060
if @error_template_attribute.update(error_template_attribute_params)
6161
format.html do
62-
redirect_to @error_template_attribute, notice: t('shared.object_updated', model: @error_template_attribute.class.model_name.human)
62+
redirect_to @error_template_attribute, notice: t('shared.object_updated', model: @error_template_attribute.class.model_name.human), status: :see_other
6363
end
6464
format.json { render :show, status: :ok, location: @error_template_attribute }
6565
else
@@ -76,7 +76,7 @@ def destroy
7676
@error_template_attribute.destroy
7777
respond_to do |format|
7878
format.html do
79-
redirect_to ErrorTemplateAttribute, notice: t('shared.object_destroyed', model: @error_template_attribute.class.model_name.human)
79+
redirect_to ErrorTemplateAttribute, notice: t('shared.object_destroyed', model: @error_template_attribute.class.model_name.human), status: :see_other
8080
end
8181
format.json { head :no_content }
8282
end

app/controllers/error_templates_controller.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def create
4040

4141
respond_to do |format|
4242
if @error_template.save
43-
format.html { redirect_to @error_template, notice: t('shared.object_created', model: @error_template.class.model_name.human) }
43+
format.html { redirect_to @error_template, notice: t('shared.object_created', model: @error_template.class.model_name.human), status: :see_other }
4444
format.json { render :show, status: :created, location: @error_template }
4545
else
4646
format.html { render :new, status: :unprocessable_content }
@@ -55,7 +55,7 @@ def update
5555
authorize!
5656
respond_to do |format|
5757
if @error_template.update(error_template_params)
58-
format.html { redirect_to @error_template, notice: t('shared.object_updated', model: @error_template.class.model_name.human) }
58+
format.html { redirect_to @error_template, notice: t('shared.object_updated', model: @error_template.class.model_name.human), status: :see_other }
5959
format.json { render :show, status: :ok, location: @error_template }
6060
else
6161
format.html { render :edit, status: :unprocessable_content }
@@ -70,7 +70,7 @@ def destroy
7070
authorize!
7171
@error_template.destroy
7272
respond_to do |format|
73-
format.html { redirect_to ErrorTemplate, notice: t('shared.object_destroyed', model: @error_template.class.model_name.human) }
73+
format.html { redirect_to ErrorTemplate, notice: t('shared.object_destroyed', model: @error_template.class.model_name.human), status: :see_other }
7474
format.json { head :no_content }
7575
end
7676
end
@@ -79,7 +79,7 @@ def add_attribute
7979
authorize!
8080
@error_template.error_template_attributes << ErrorTemplateAttribute.find(params[:error_template_attribute_id])
8181
respond_to do |format|
82-
format.html { redirect_to @error_template }
82+
format.html { redirect_to @error_template, status: :see_other }
8383
format.json { head :no_content }
8484
end
8585
end
@@ -88,7 +88,7 @@ def remove_attribute
8888
authorize!
8989
@error_template.error_template_attributes.delete(ErrorTemplateAttribute.find(params[:error_template_attribute_id]))
9090
respond_to do |format|
91-
format.html { redirect_to @error_template }
91+
format.html { redirect_to @error_template, status: :see_other }
9292
format.json { head :no_content }
9393
end
9494
end

app/controllers/execution_environments_controller.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ def sync_to_runner_management
194194
Runner.strategy_class.sync_environment(@execution_environment)
195195
rescue Runner::Error => e
196196
Rails.logger.warn { "Runner error while synchronizing execution environment with id #{@execution_environment.id}: #{e.message}" }
197-
redirect_to @execution_environment, alert: t('execution_environments.index.synchronize.failure', error: ERB::Util.html_escape(e.message))
197+
redirect_to @execution_environment, alert: t('execution_environments.index.synchronize.failure', error: ERB::Util.html_escape(e.message)), status: :see_other
198198
else
199-
redirect_to @execution_environment, notice: t('execution_environments.index.synchronize.success')
199+
redirect_to @execution_environment, notice: t('execution_environments.index.synchronize.success'), status: :see_other
200200
end
201201
end
202202

@@ -239,9 +239,9 @@ def sync_all_to_runner_management
239239
end
240240

241241
if success.all?
242-
redirect_to ExecutionEnvironment, notice: t('execution_environments.index.synchronize_all.success')
242+
redirect_to ExecutionEnvironment, notice: t('execution_environments.index.synchronize_all.success'), status: :see_other
243243
else
244-
redirect_to ExecutionEnvironment, alert: t('execution_environments.index.synchronize_all.failure')
244+
redirect_to ExecutionEnvironment, alert: t('execution_environments.index.synchronize_all.failure'), status: :see_other
245245
end
246246
end
247247

app/controllers/exercises_controller.rb

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ def clone
5959
exercise = @exercise.duplicate(public: false, token: nil, user: current_user, uuid: nil)
6060
exercise.send(:generate_token)
6161
if exercise.save
62-
redirect_to(exercise_path(exercise), notice: t('shared.object_cloned', model: Exercise.model_name.human))
62+
redirect_to exercise_path(exercise), notice: t('shared.object_cloned', model: Exercise.model_name.human), status: :see_other
6363
else
6464
flash[:danger] = t('shared.message_failure')
65-
redirect_to(@exercise)
65+
redirect_to @exercise, status: :see_other
6666
end
6767
end
6868

@@ -238,7 +238,7 @@ def download_proforma
238238
zip_file = ProformaService::ExportTask.call(exercise: @exercise)
239239
send_data(zip_file.string, type: 'application/zip', filename: "exercise_#{@exercise.id}.zip", disposition: 'attachment')
240240
rescue ProformaXML::PostGenerateValidationError => e
241-
redirect_to :root, danger: JSON.parse(e.message).join('<br>')
241+
redirect_to :root, danger: JSON.parse(e.message).join('<br>'), status: :see_other
242242
end
243243

244244
def user_from_api_key
@@ -331,7 +331,7 @@ def handle_exercise_tips(tips_params)
331331
ExerciseTip.destroy(previous_exercise_tips - remaining_exercise_tips)
332332
rescue JSON::ParserError => e
333333
flash[:danger] = "JSON error: #{e.message}"
334-
redirect_to(edit_exercise_path(@exercise))
334+
redirect_to edit_exercise_path(@exercise), status: :see_other
335335
end
336336
end
337337

@@ -349,7 +349,7 @@ def update_exercise_tips(exercise_tips, parent_exercise_tip_id, rank)
349349
rank += 1
350350
unless current_exercise_tip.save
351351
flash[:danger] = current_exercise_tip.errors.full_messages.join('. ')
352-
redirect_to(edit_exercise_path(@exercise)) and break
352+
redirect_to(edit_exercise_path(@exercise), status: :see_other) and break
353353
end
354354

355355
children = update_exercise_tips exercise_tip[:children], current_exercise_tip.id, rank
@@ -371,17 +371,16 @@ def implement # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedC
371371
session.delete(:pair_programming)
372372
@current_contributor = current_user
373373
else
374-
return redirect_back_or_to(
375-
implement_exercise_path(current_contributor.exercise),
376-
alert: t('exercises.implement.existing_programming_group', exercise: current_contributor.exercise.title)
377-
)
374+
return redirect_back_or_to implement_exercise_path(current_contributor.exercise),
375+
alert: t('exercises.implement.existing_programming_group', exercise: current_contributor.exercise.title),
376+
status: :see_other
378377
end
379378
elsif session[:pg_id].blank? && (pg = current_user.programming_groups.find_by(exercise: @exercise)) && pg.submissions.where(study_group_id: current_user.current_study_group_id).any?
380379
# we are just acting on behalf of a single user who has already worked on this exercise as part of a programming group **in the context of the current study group**
381380
session[:pg_id] = pg.id
382381
@current_contributor = pg
383382
elsif session[:pg_id].blank? && session[:pair_programming] == 'mandatory'
384-
return redirect_back_or_to(new_exercise_programming_group_path(@exercise))
383+
return redirect_back_or_to new_exercise_programming_group_path(@exercise), status: :see_other
385384
elsif session[:pg_id].blank? && session[:pair_programming] == 'optional' && current_user.submissions.where(study_group_id: current_user.current_study_group_id, exercise: @exercise).none?
386385
Event.find_or_create_by(category: 'pp_work_alone', user: current_user, exercise: @exercise, data: nil, file_id: nil)
387386
current_user.pair_programming_waiting_users&.find_by(exercise: @exercise)&.update(status: :worked_alone)
@@ -492,9 +491,9 @@ def not_authorized_for_exercise(_exception)
492491
return render_not_authorized unless %w[implement working_times intervention reload].include?(action_name)
493492

494493
if current_user.admin? || current_user.teacher?
495-
redirect_to(@exercise, alert: t('exercises.implement.unpublished')) if @exercise.unpublished?
496-
redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists?
497-
redirect_to(@exercise, alert: t('exercises.implement.no_execution_environment')) if @exercise.execution_environment.blank?
494+
redirect_to(@exercise, alert: t('exercises.implement.unpublished'), status: :see_other) if @exercise.unpublished?
495+
redirect_to(@exercise, alert: t('exercises.implement.no_files'), status: :see_other) unless @exercise.files.visible.exists?
496+
redirect_to(@exercise, alert: t('exercises.implement.no_execution_environment'), status: :see_other) if @exercise.execution_environment.blank?
498497
else
499498
render_not_authorized
500499
end

0 commit comments

Comments
 (0)