Chrome error: ajax (PUT) update in tutorial. #79

Closed
willvincent opened this Issue Feb 18, 2012 · 14 comments

Comments

Projects
None yet
5 participants

In the tutorial, the update method redirects before ending the connection, this causes that socket to be forcefully closed which Chrome apparently isn't happy about.

Redirecting back to the same page also seems a bit unnecessary on an ajax call.

Replacing the self.redirect() line with resp.end(); resolves the issue with Chrome, and still plays nicely with other browsers.

Contributor

mde commented Feb 19, 2012

Nice catch. The redirect is for normal form-submissions, but not correct for an Ajax update. Shouldn't it be self.respond, and send back a JSON representation of the instance? @Techwraith, what do you think? (Ultimately might be nice to detect if it's an Ajax call or normal post, and respond accordingly, but that's probably overkill starting off.)

Contributor

Techwraith commented Feb 19, 2012

Yeah, lets do self.respond and send back the instance as JSON. Now that I think about it, browsers can't really do put requests normally anyways, right?

Contributor

mde commented Feb 19, 2012

Now that I think about it too, the create call could be done with Ajax as well -- in that case, you'd want to respond with a 201/created. I don't think there's a way to specify the response-code yet, so I guess we'll need to add that as well.

Contributor

polotek commented Feb 20, 2012

I haven't looked at this in detail, but it feels to me like the behavior of the response should depend on content negotiation. If it's application/json then return the response. If it's text/html or whatever then redirect.

Contributor

mde commented Feb 20, 2012

Content-negotiation only determines the format of the response. But yes, now that I think about it we can easily do the right thing here -- calling self.transfer after a successful update will hand off transparently to the "show" action, which will respond correctly with the required 200 (" If an existing resource is modified, either the 200 (OK) or 204 (No Content) response codes SHOULD be sent to indicate successful completion of the request."), with the data formatted correctly via the usual content-negotiation.

On a related topic, what is the correct way to get validation error messages back to the view? I see that when you have validation methods that set a message those messages are added to the object.

In the demo on error for the add form it's doing a self.redirect() which doesn't seem to allow you to pass the current state of the object back to the view, ie: whatever values were submit and any applicable error messages. I've been playing with this off and on for the past couple of days and it seems to work ok with a self.respond() that passes the object to the view as a param, but that requires essentially duplicating the view for both the add and create controller methods.

I saw another issue (I think) about flash messages, seems like that would be a better way of handling display of error messages. What I see as the best option here would be for validation methods that return error messages to instead (or maybe in addition) set those messages in the session, which then could be processed in the layout template, or something similar, so that any error, warning, success, etc messages could be easily passed back to the user in a uniform way.

This seems like something that would be worth having as part of geddy core. I'd be happy to work on implementing it, but being fairly new to geddy I'm not sure where to start with something like that. :)

Contributor

Techwraith commented Feb 21, 2012

@willvincent Flash messages are on the roadmap, for now you'll have to use our (poorly documented) session support.

Contributor

mde commented Feb 22, 2012

Ah, looks like there's a bit of a mixup in the tutorial page with respect to edit/update/show. @Techwraith, let's take a look at this tomorrow, and sort it out. The old Geddy v0.1 scaffold code dealt with this in a pretty straightforward way.

I wanted to contribute to this, albeit being slightly off topic (only slightly) Chrome didn't like the redirect via ajax, so I am doing respond() instead, which seems to invoke the templates, and it's crying about an "update" template. I'll be creating one, but for a PUT request on //:id a template shouldn't be required.

Contributor

Techwraith commented Mar 22, 2012

@bellingboe Totally. We've fixed this in the example apps in the repo, but we haven't fixed up the tutorial yet. I'll be taking care of that in a couple of weeks. We're waiting for a couple of articles to run on http://net.tutsplus.com first.

Awesome. That's actually where I first heard of this. It's great. A few minor things, (socket.io anyone?=] ) but I'm really liking this a lot.

Contributor

Techwraith commented Mar 22, 2012

Socket.io is doable if you put it in the init.js file. I'm working on creating a socket.io-messageRouter so you can have socket.io controllers if you'd like. Start another issue if you want to help plan it :)

Contributor

mde commented Apr 28, 2012

Can this be closed?

@mde mde closed this Apr 28, 2012

@mde mde reopened this Apr 28, 2012

Contributor

Techwraith commented Apr 30, 2012

I'm going to fix up the tutorial on the site as soon as the new Mongo stuff lands in master. I'll have a tutorial for the in-mem store, but I'll also have a tutorial for Mongo in there somewhere. Both of them should "Do the Right Thing".

@Techwraith Techwraith closed this Oct 7, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment