inherited_resources icon indicating copy to clipboard operation
inherited_resources copied to clipboard

`Create` & `Update` should return `status code != 200` for failed requests

Open a-nickol opened this issue 2 years ago • 5 comments

To make inherited_resources work with the current version of rails out of the box, it is not allowed to not redirect if status code is 200 on a submitted form.

Since on a failure we want to render either :new or :edit, we are not allowed send status code 200, since this will result in the following error:

Error: Form responses must redirect to another location
    at _FormSubmission.requestSucceededWithResponse (application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1298:24)
    at FetchRequest.receive (application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1068:23)
    at FetchRequest.perform (application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1046:27)
formSubmissionErrored @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:2854
requestSucceededWithResponse @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1299
receive @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1068
perform @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1046
await in perform (async)
start @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1258
submitForm @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:2795
formSubmitted @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:3504
FormSubmitObserver.submitBubbled @ application-c20cb2d408231ec629db20f6bfe840b8b84f28b26acee36de46dd6b684b8aa43.js:1457

Workaround:

  def create
    create! do |success, failure|
      failure.html { render(:new, status: :unprocessable_entity) }
    end
  end

  def update
    update! do |success, failure|
      failure.html { render(:edit, status: :unprocessable_entity) }
    end
  end

a-nickol avatar Aug 31 '23 05:08 a-nickol

This change is probably unwanted, since of:

Inherited Resources is no longer actively maintained by the original author and has been transferred to the ActiveAdmin organization for maintenance. New feature requests are not encouraged.

a-nickol avatar Aug 31 '23 07:08 a-nickol

Oh fuck

andriytyurnikov avatar Oct 26 '23 14:10 andriytyurnikov

Is this really a feature request? Returning a 200 after what is essentially a failed request could be seen as a bug.

As ActiveAdmin is in a period of more active development, I would think that a solution that properly works with Turbo is important.

I used to add data-turbo: false all over the place. After some investigation and seeing the connected PR, it could be solved by an initializer with:

InheritedResources::Responder.error_status = :unprocessable_entity

So, if not actually wanted in InheritedResources, that line could be part of ActiveAdmin. It would still be a shame, no?

caifara avatar Mar 08 '24 09:03 caifara

In the past it was the expected behaviour to return status 200 and render :new or :edit templates. But yes with turbo this behaviour is not working anymore.

a-nickol avatar Mar 08 '24 09:03 a-nickol

Returning a 200 after what is essentially a failed request could be seen as a bug.

I do agree

I would think that a solution that properly works with Turbo is important.

This is still not clear for me. Since Active Admin has its own js file and does not support turbo, I don't know how turbo is being enabled in Active Admin. My guesses are A. Navigating from the application to the admin area without turbo: false? B. Application JS is being loaded inside the admin area? C. Other?

I used to add data-turbo: false all over the place.

This makes me think to Scenario A, but I'm not sure why it should be all over the place

About Scenario B, I don't know if it is a good practice to load application JS inside the admin Area

tagliala avatar Jun 01 '24 09:06 tagliala