Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling form errors with Rails automatically? #85

Closed
excid3 opened this Issue Apr 30, 2016 · 37 comments

Comments

Projects
None yet
@excid3
Copy link

commented Apr 30, 2016

When you submit a form that has errors, Rails normally renders the new action again with the form and the errors. With Turbolinks as it is setup right now, the redirects are handled nicely, but this piece requires you to build your own create.js.erb response to render the errors.

Would it make sense to add something into Turbolinks / turbolinks-rails that if you submitted a non-GET request and it returned an HTML response that it would take the URL of the request and navigate to that but render the HTML that was returned from the server?

@kevinhughes27

This comment has been minimized.

Copy link

commented Apr 30, 2016

I solved this by modifying $.rails.ajax like so:

$.rails.ajax = (options) ->
  op = $.ajax(options)

  op.done (response) ->
    unless response.substring(0, 10) == 'Turbolinks'
      Turbolinks.clearCache()
      document.documentElement.innerHTML = response
      Turbolinks.dispatch("turbolinks:load")
      window.scroll(0,0)
@fernandes

This comment has been minimized.

Copy link

commented May 2, 2016

@excid3

Just improving (and providing an alternative to @kevinhughes27 's answer), this is how I handle form errors in TL

I add a data-behavior="turbolinks-form" on forms I want to be handled using my JS, and then add this file under app/assets/javascripts/support/form_bindings.js

formBindings = function() {
  $('[data-behavior="turbolinks-form"').on("ajax:success", function(e, data, status, xhr) {
    return;
  }).on("ajax:error", function(e, xhr, status, error) {
    form_id = "#" + this.id;
    $(form_id).html($(form_id, xhr.responseText).html());
    return;
  });
};

$(document).on('turbolinks:load', formBindings);

the main idea is once its return a HTML rendering a form with the same id that posted, just replaces the HTML. the concept behind was to:

  1. use data behaviors to separate code / data
  2. bind only specific form
  3. replace using id lookups and html function

so, here is my suggestion 😉

ps: after posting I re read your question, not sure if it answers yours, so I'm gonna leave it here mostly as an answer to Handling form errors with Rails automatically? then to your proper question heheh

@gregblass

This comment has been minimized.

Copy link

commented May 4, 2016

I think where I lost @sstephenson is here:

"In response to an XHR submit on the server, return JavaScript that performs a Turbolinks.visit to be evaluated by the browser."

An example would help me there, in the case where the record is saved and in the case where there are form validation errors?

EDIT: This is what I'm doing so far for the success using Turbolinks and Materialize toasts:

In my controller:

format.js {
  flash[:toast] = 'Created record successfully.'
  render :create
}

In create.js.erb:

Turbolinks.visit("<%= success_path %>");

In my _flash.html.erb partial:

<% flash.each do |type, message| %>
  <% if type == "toast" %>
    <script id="toast">
      $(function() {
        Materialize.toast('<%= message %>', 3000);
      });
    </script>
  <% elsif type == "success" %>
    ...
  <% elsif type == "alert" %>
    ... 
  <% end %>
<% end %>

EDIT 2: For validation failures, I was able to just make a new.js.erb that replaces the main part of the page with the form rendered after the object has errors on it. Not too hard and it works!

@packagethief

This comment has been minimized.

Copy link
Member

commented May 17, 2016

Would it make sense to add something into Turbolinks / turbolinks-rails that if you submitted a non-GET request and it returned an HTML response that it would take the URL of the request and navigate to that but render the HTML that was returned from the server?

We intend to explore expanding Turbolinks to support XHR form submission in the future, so I think this is something we can consider at that time. Until then, I don't think Turbolinks should make (more) assumptions about how you might be responding to non-Turbolinks XHR requests.

Assuming you're using jQuery via Rails' UJS adapter (assumptions!), the following pattern should work well for validation errors. In create.js.erb:

<% if @message.valid? %>
  Turbolinks.clearCache()
  Turbolinks.visit("<%= message_path(@message) %>", { action: "replace" })
<% else %>
  $("form").replaceWith("<%=j render "form" %>")
<% end %>

Ensure you exclude such form-containing pages from Turbolinks' preview cache to prevent any validation errors from being displayed during a preview on the next visit.

@nerdcave

This comment has been minimized.

Copy link

commented May 24, 2016

Is there a way to just pass an HTML page string to Turbolinks and have it do the processing? I think that would really help in cases like this. I dug through the source and couldn't find a way that worked.
For example:

ajax.done (response) ->
  Turbolinks.clearCache()
  Turbolinks.render(response) #something like this exist?

Similar to @kevinhughes27's suggestion, but I think it would make more sense to pass off the DOM processing to Turbolinks to ensure some consistency in app behavior (events get dispatched properly, etc). Any way to do this?

@kevinhughes27

This comment has been minimized.

Copy link

commented May 24, 2016

Turbolinks3 used to have this as Turbolinks.replace - maybe its possible to bring back this method but not the partial replace aspects of it?

@gregblass

This comment has been minimized.

Copy link

commented Jun 20, 2016

I have no affiliation with GoRails or @excid3, and I'm not trying to cross any boundaries here on that front, but the screencast he did on this is very helpful and explains the code @kevinhughes27 posted in more detail. IMO its a great solution - I have implemented it myself in production.

@FloLinn

This comment has been minimized.

Copy link

commented Jul 13, 2016

@kevinhughes27 @acapro
you can also avoid the blinking by only replacing a part of the page.
In my project, I only replace the form object
$("form").html($("form", response).html())

@gregblass

This comment has been minimized.

Copy link

commented Aug 9, 2016

One issue I've found with @kevinhughes27 's modifying $rails.ajax is that then it messes up any other ajax request (like when I just want to load a partial via ajax with a standard format.js / action.js.erb file).

So it looks like I may be handling each form on a case by case basis because of that. Unless I can figure out a way to do it the other way around - that is, include something in my one off ajax requests to not fire that code off.

@gregblass

This comment has been minimized.

Copy link

commented Sep 9, 2016

Here's my current design pattern I've been using in my production apps:

In the controller, on a form error:

render partial: "form", status: 422

Then I use this app wide:

$(document).on("ajax:error", "form", function(e, data, status, xhr) {
  $(this).replaceWith(data.responseText);
  Turbolinks.dispatch("turbolinks:load");
  $(".field_with_errors input:first").focus();
});

And of course make all forms remote. This works great for me.

@woahdae

This comment has been minimized.

Copy link

commented Feb 21, 2017

@sstephenson why was this closed without comment? I was quite surprised that turbolinks 5 gets you almost out the gate with 100% CRUD coverage, except this one case you have to figure out yourself. It should still be open for discussion, no? Or (wrongly, IMO) declare "wontfix"?

@woahdae

This comment has been minimized.

Copy link

commented Feb 21, 2017

@gregblass Really nice solution, consider the following a "yes and."

While not good enough yet as a patch to turbolinks (I'm sure the project has its own way of parsing the body already), I think the following is more idiomatic. The whole point of turbolinks is to just code "normal" Rails, turn on turbolinks, and have everything "just work" but faster. So:

In your controller, nothing changes except currently the 422 status (Turbolinks could do that for you, like they do with the custom redirect_to):

render "edit", status: 422

Then the javascript becomes something very similar to what Turbolinks does. Again, I'm sure the project authors would do this better, but I don't feel like digging in since it's not clear there'd be any interest.

$(document).on("ajax:error", 'body', function(e, data, status, xhr) {
  $(this).html(
    data.responseText.match(
      /<body[^>]*>((.|[\n\r])*)<\/body>/im
    )[0]
  );
  Turbolinks.dispatch("turbolinks:load");

  // This is application-dependent, but a nice touch.
  $(".has-error input:first").focus();
});

Two reasons I like this approach:

  1. I had no idea Turbolinks had this issue, and with NO other changes to my code (just the status: 422 bit and the JS) everything works. Personally, I use the 'form' template for just the form fields, and let 'new' and 'edit' pass in their own form objects and draw their own buttons. I could pull these in to 'form,' but that'd be coupling the organization of your views to a technical gotcha with Turbolinks.
  2. It's not just the form that matters, for example I have flash message handling in the layout. With the proposed technique, the flash messages still work just fine. Sure, I could change the flash messages to be handled with JS too, but same point as before. It's supposed to "just work."

Why does this issue exist, and (again) why was it closed? Is there some edge-case reason why this must break convention over configuration?

@jondavidchristopher

This comment has been minimized.

Copy link

commented Apr 26, 2017

@woahdae nice solution above. I am using it currently. I hit an interesting edge case where errors are only displayed via a flash message (think devise login error "Invalid Email or Password"). When on a mobile device, activating the keyboard causes the page to scroll down. When submitting a form with errors the flash message is displayed but it is above the fold where the user cannot see it. It is standard "Turbolinks" practice to scroll back to the top of the page after loading it. Implemented a simple fix for this below and thought I would share:

// This is application-dependent
if ($("#flash_message").length > 0) {
    scrollTo(0,0);
} else {
    $(".has-error input, .has-error select").first().focus();
}
@nerdcave

This comment has been minimized.

Copy link

commented May 1, 2017

I think a lot of the confusion here is because the Turbolinks gem hijacks the redirect_to call in Rails but not the render call. This does seem incomplete to me, but I can also see why they tread lightly.

If you wanted to override render yourself and keep the solution server-side, you could do something like this:

# maybe the Turbolinks gem could do something like this
def render(*args)
  options = args.extract_options!
  return super unless options[:turbolinks]

  html = render_to_string args[0]
  script = <<-SCRIPT
    Turbolinks.clearCache();
    var parser = new DOMParser();
    doc = parser.parseFromString("#{ActionController::Base.helpers.j(html)}", "text/html");
    document.documentElement.replaceChild(doc.body, document.body);
    Turbolinks.dispatch("turbolinks:load");
    window.scroll(0, 0);
  SCRIPT
  self.status = 200
  self.response_body = script
  response.content_type = "text/javascript"
end

# in your controller:
def update
  if @obj.update(obj_params)
    redirect_to @obj
  else
    # render entire page, not just the form
    render :edit, turbolinks: true
end
@grosser

This comment has been minimized.

Copy link

commented May 5, 2017

can we reopen this and get a proper solution ?
... we know the request was xhr ... and that it came from turbolinks ... so do whatever necessary serverside and fix turbolinks js to handle that ...

@grosser

This comment has been minimized.

Copy link

commented May 6, 2017

FYI simpler body replacement:

$("body").html(response.match(/<body[^>]*>([\s\S]*?)<\/body>/i)[1]);

... my complete script to handle get forms and regular forms is now:

$(document).on('ajax:before', 'form[method=get][data-remote=true]', function(e) {
  e.preventDefault(); // do not perform regular sumbit
  e.stopPropagation(); // do not let regular remote handler see this
  var form = $(this);
  Turbolinks.visit(form.attr("action") + form.serialize());
});

(function(){
  var remoteForm = 'form[method!=get][data-remote=true]';
  $(document).on('submit', remoteForm, function(e, response) {
    Turbolinks.controller.history.push(window.location.href);
  });

  $(document).on('ajax:success', remoteForm, function(e, response) {
    if(response.substring(0, 10) == 'Turbolinks'){ return; }
    Turbolinks.clearCache();
    $("body").html(response.match(/<body[^>]*>([\s\S]*?)<\/body>/i)[1]);
    Turbolinks.dispatch("turbolinks:load");
    window.scroll(0, 0);
  });
})();
@danieldraper

This comment has been minimized.

Copy link

commented May 6, 2017

Rails 5 introduced form_with which by default treats all forms as AJAX unless you pass the :local option. The solution for Turbolinks should probably be applied to Rails to work with form_with also. See https://github.com/rails/rails/blob/master/guides/source/working_with_javascript_in_rails.md

@minimul

This comment has been minimized.

Copy link

commented May 10, 2017

@nerdcave Just what I needed when doing an upgrade to TL5.

Submitted as a PR as well.

turbolinks/turbolinks-rails#20

Thanks.

@jondavidchristopher

This comment has been minimized.

Copy link

commented Jun 1, 2017

Another issue I just ran into with wholesale replacing the body on an error is that inline page scripts are not executed. Below is a simple snippet of CoffeeScript that handles this via Turbolinks.

    renderer = new Turbolinks.Renderer
    for inline_script in document.body.getElementsByTagName "script"
      element = renderer.createScriptElement inline_script
      inline_script.parentNode.replaceChild element, inline_script
@codyrobbins

This comment has been minimized.

Copy link

commented Jun 2, 2017

I’m just going to pile on and say that this seems like a glaring hole in Turbolinks and it makes no sense to me that you have to roll your own form submissions instead of having them “just work” out of the box. It seems inconsistent with the way Turbolinks handles responses to GET requests and redirects and the documentation doesn’t make it clear that Turbolinks leaves it all up to you in this respect. It took me quite a while to figure out that I wasn’t doing something wrong but rather that Turbolinks just drops the ball entirely here.

@tomrossi7

This comment has been minimized.

Copy link

commented Jun 13, 2017

@woahdae I really like your approach. I'm not sure if it is related to jquery-rails 4.3.1 and rails 5.1.1, but I'm unable to get it to work. The "ajax:error" event is triggered, but I can't access the data. Logging appeared to show that of the variables passed (e, data, status, xhr), only e was being passed from ujs? Am I missing something obvious?

I got it to work, by accessing it through the event (e), but that looks crazy:

$(document).on("ajax:error", 'body', function(e, data, status) {
    responseText = e.detail[2].responseText
    $(this).html(
      responseText.match(
        /<body[^>]*>((.|[\n\r])*)<\/body>/im
      )[0]
    );
    Turbolinks.dispatch("turbolinks:load");

    // This is application-dependent, but a nice touch.
    $(".has-error input:first").focus();
  });

@johnclittle

This comment has been minimized.

Copy link

commented Jun 14, 2017

Attempting to respect the data-turbolinks-permanent designations. Does this look appropriate?

	$(document).on "ajax:error", "body", (e, data, status, xhr) ->
		newBody = document.createElement("body")
		newBody.innerHTML = data.responseText.match(/<body[^>]*>((.|[\n\r])*)<\/body>/im)[0]
		existingBodyAttrs = document.getElementsByTagName("body")[0].attributes

		i = 0
		while(i < existingBodyAttrs.length)
			newBody.setAttribute(existingBodyAttrs[i].name,existingBodyAttrs[i].value) 
			i++

		for replaceableElement in newBody.querySelectorAll("[id][data-turbolinks-permanent]")
			if element = document.body.querySelector("##{replaceableElement.id}[data-turbolinks-permanent]")
				replaceableElement.parentNode.replaceChild(element, replaceableElement)

		document.body = newBody

		Turbolinks.dispatch("turbolinks:load")

		$(".has-error input:first").focus()
@hansondr

This comment has been minimized.

Copy link

commented Jun 21, 2017

@tomrossi7

There were a number of changes that came with rails-ujs. The current callback format is:

$(document).on("ajax:error", 'body', function(event) {
  var data = event.detail[0];
  var status = event.detail[1];
  var xhr = event.detail[2];
  var responseText = xhr.responseText;
  // ....
}
@stefatkins

This comment has been minimized.

Copy link

commented Aug 14, 2017

Is there a solution in ES6 ? I dropped jQuery in my project.

@mvanduijker

This comment has been minimized.

Copy link

commented Aug 14, 2017

I dropped jquery as well. I am doing something like.

document.body.addEventListener("ajax:error", function (e) {
  if (e.detail[2].status !== 422) {
    return
  }
  document.body = e.detail[0].body
  Turbolinks.dispatch("turbolinks:load")
  scrollTo(0, 0)
})
@stefatkins

This comment has been minimized.

Copy link

commented Aug 14, 2017

Thanks mvanduijker I add a little change to your code. WIthout it will only catch ajax error the first time.

document.addEventListener("turbolinks:load", () => {
  document.body.addEventListener("ajax:error", (e) => {
    if (e.detail[2].status !== 422) {
      return
    }
    document.body = e.detail[0].body
    Turbolinks.dispatch("turbolinks:load")
    scrollTo(0, 0)
  })
})
@dustMason

This comment has been minimized.

Copy link

commented Aug 18, 2017

@stefan694 and @mvanduijker You can also create a persistent event listener without re-attaching every time Turbolinks loads the page:

document.addEventListener("ajax:error", (e) => {
  if (e.detail[2].status !== 422) { return }
  document.body = e.detail[0].body
  Turbolinks.dispatch("turbolinks:load")
  scrollTo(0, 0)
})
@mvanduijker

This comment has been minimized.

Copy link

commented Aug 20, 2017

Nice catch @dustMason!

@davidscolgan

This comment has been minimized.

Copy link

commented Sep 10, 2017

While spelunking in the sourcecode for changelog.com (not Rails but uses Turbolinks), I found them using this technique:

// submit all other forms with Turbolinks                                                   
u(document).on("submit", "form:not(.js-cm)", function(event) {                              
  event.preventDefault();                                                                   
                                                                                            
  const form = u(this);                                                                     
  const action = form.attr("action");                                                       
  const method = form.attr("method");                                                       
  const data = form.serialize();                                                            
  const referrer = location.href;                                                           
                                                                                            
  if (method == "get") {                                                                    
    return Turbolinks.visit(`${action}?${data}`);                                           
  }                                                                                         
                                                                                            
  const options = {method: method, body: data, headers: {"Turbolinks-Referrer": referrer}}; 
  const andThen = function(err, resp, req) {                                                
    if (req.getResponseHeader("content-type").match(/javascript/)) {                        
      eval(resp);                                                                           
    } else {                                                                                
      const snapshot = Turbolinks.Snapshot.wrap(resp);                                      
      Turbolinks.controller.cache.put(referrer, snapshot);                                  
      Turbolinks.visit(referrer, {action: "restore"});                                      
    }                                                                                       
  }                                                                                         
                                                                                            
  ajax(action, options, andThen);                                                           
});                                                                                         

from https://github.com/thechangelog/changelog.com/blob/master/assets/app/app.js#L121

In summary, for forms marked to be submitted with Turbolinks, submit the request, and check if the response has content-type Javascript. If it does eval it, otherwise take the response (which would be the html for the whole page of an invalid form), manually add it to the Turbolinks cache, and then visit that newly added page.

I don't think adding pages manually to the cache is documented anywhere and I'm not using Rails so there may be other gotchas, but I was able to implement this technique in Django and it does appear to work pretty well for me. Plus you don't have to mess with anything server-side other than not returning a JS response if there is a problem.

@stefatkins

This comment has been minimized.

Copy link

commented Sep 19, 2017

I had some difficulties getting the given solutions with basecamps trix editor. If someone is looking for a fix here is what I did to reinitialize trix after sending the html through json.

html : 
<%= f.hidden_field :body, id: "body", data: { behavior: "trix-editor" } %>

js :
document.addEventListener("turbolinks:load", () => {
  const trixField = document.body.querySelector("[data-behavior='trix-
  editor']")
  if (document.body.contains(trixField)) {
    const fieldId = trixField.attributes["id"].value
    const trixEditor = document.createElement("trix-editor")
    trixEditor.setAttribute("input", fieldId)
    const parentDiv = trixField.parentNode
    parentDiv.insertBefore(trixEditor, trixField)
   }
});

I'm open to better fixes :)

@hsgubert

This comment has been minimized.

Copy link

commented Sep 26, 2017

I solved this problem in my application by capturing the rendered HTML after a form submit, and replicating the behavior of Turbolinks, namely:

  1. The body of the page is replaced by the new body
  2. Inline <script> tags on the new body are run
  3. The correct turbolinks events are triggered (e.g. request-start, request-end, before-render, render, load)

I've packaged my solution to a gem turbolinks-form and using it is as simple as adding an option to your form_tag/form_for and rendering something with an error code in your controller.

I'm glad to receive any comments

@domchristie

This comment has been minimized.

Copy link
Collaborator

commented Oct 23, 2017

The changelog.com code that @davidscolgan shared is great, since it uses Turbolinks to render the response, run inline scripts, handle scrolling, and fire the load event, all with just a few built-in methods. Clever.

For those using Rails, the form serialization code is not necessary as it is built-in to jquery/rails-ujs. To combine some of the other solutions with the changelog code:

$.rails.ajax = function(options) {
  return $.ajax(options).done(function(response, status, $xhr) {
    if (response && ($xhr.getResponseHeader('Content-Type') || '').substring(0, 9) === 'text/html') {
      var referrer = window.location.href
      Turbolinks.controller.cache.put(referrer, Turbolinks.Snapshot.wrap(response))
      Turbolinks.visit(referrer, { action: 'restore' })
    }
  });
};

or for rails-ujs (i.e. without jQuery):

document.addEventListener('ajax:complete',function (event) {
  var xhr = event.detail[0]
  if ((xhr.getResponseHeader('Content-Type') || '').substring(0, 9) === 'text/html') {
    var referrer = window.location.href
    var snapshot = Turbolinks.Snapshot.wrap(xhr.response)
    Turbolinks.controller.cache.put(referrer, snapshot)
    Turbolinks.visit(referrer, { action: 'restore' })
  }
}, false)
@kmmbvnr

This comment has been minimized.

Copy link

commented Dec 7, 2017

I extended a bit @domchristie solution

After a successful form submission, the location should be changed to the redirected.

performPostRequest() {
    // disable form buttons
    this.root_.querySelectorAll('button').forEach(
      (button) => button.disabled=true
    );

    let xhr = new XMLHttpRequest();
    xhr.open('POST', window.location.search, true);
    xhr.setRequestHeader('Turbolinks-Referrer', window.location);

    xhr.onload = (event) => {
      let location = xhr.getResponseHeader('turbolinks-location');
      let snapshot = window.Turbolinks.Snapshot.wrap(xhr.response);

      if (!location) {
        location = window.location.href;
      }

      window.Turbolinks.controller.adapter.hideProgressBar();
      window.Turbolinks.controller.cache.put(location, snapshot);
      window.Turbolinks.visit(location, {action: 'restore'});

      if (xhr.status > 299) {
        Turbolinks.controller.disable();
      }
    };

    window.Turbolinks.controller.adapter.showProgressBarAfterDelay();
    xhr.send(new FormData(this.root_));
  }
@pioz

This comment has been minimized.

Copy link

commented Feb 1, 2018

My solution is this:

$.rails.ajax = function(options) {
  return $.ajax(options).done(function(response, status, xhr) {
    if (response && (xhr.getResponseHeader('Content-Type') || '').substring(0, 9) === 'text/html') {
      var referrer = window.location.href;
      Turbolinks.controller.cache.put(referrer, Turbolinks.Snapshot.wrap(response));
      Turbolinks.visit(referrer, { action: 'restore' });
    }
  });
};
@acapro

This comment has been minimized.

Copy link

commented Apr 10, 2018

i've modified @dustMason's code so that only the forms get replaced instead of the whole body. This way the rest of the JS stuff on the page still works fine.

document.addEventListener("ajax:error", (e) => {
  if (e.detail[2].status !== 422) { return }

  for(let i=0; i < document.forms.length; i++) {
    document.forms[i].innerHTML = e.detail[0].forms[i].innerHTML
  }

  Turbolinks.dispatch("turbolinks:load")
  scrollTo(0, 0)
})
@chatnuere

This comment has been minimized.

Copy link

commented Jun 9, 2018

as my form are validated by parsley.js I just wanted to display some flash message...

so based on @gregblass response i just append my flash divs that are handled by the turbolinks:load event :

$(document).on("ajax:error", "form", function(e, data, status, xhr) {
  if(data.status !== 422)return;
  $('body').append($(data.responseText).find('.flash_msg'));
  Turbolinks.dispatch("turbolinks:load");
});
@jorgemanrubia

This comment has been minimized.

Copy link

commented Jun 18, 2018

I published a gem to support handling render with Turbolinks. It is based on the idea shared by @nerdcave in this comment:

  • By default, it handles render with Turbolinks for Ajax requests that are not GET (and that are not rendering json).
  • There is a global config flag to change the default behavior
  • It supports passing a turbolinks option to render to enable/disable turbolinks on a per-request basis.

I think the situation right now is inconsistent and can be really confusing:

  • Rails 5 adds Turbolinks by default
  • Forms are remote by default
  • render won't work with the defaults mentioned above.

The fact that forms in scaffolds are generated with local: true in Rails 5.2 looks like an smell to me. The vanilla code generated by Rails won't use its own defaults!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.