CoachingLogistics / team-manager

Helps manage team logistical information
1 stars 4 forks source link

bootstrapped it up #17

Closed loganwatanabe closed 10 years ago

loganwatanabe commented 10 years ago

I put in bootstraps and made a layout on the index page. So see that if you want to know how to style your views. To go along with that, I also had to put in parameters with all the render views, namely putting in user: req.user to see if the person is logged in or not.

Hopefully this is a good look for now.

loganwatanabe commented 10 years ago

Also completed test coverage for users and families, and applied the "layout" styling to all the views.

Also have the team roster filling form functional, the last thing it needs is roster spot to be there, and also the mailing function (i commented spots for it in its team controller method).

AEgan commented 10 years ago

changing 'edit' and 'show' for the players controller to redirect to 404 instead of render the page and send a 404 response breaks 2 of my tests. Is there a reason why we should redirect to 404 instead of rendering it? Just wondering if I should change my tests or change the controller action.

loganwatanabe commented 10 years ago

Well, the layout has a ejs call to user, in order to have the current user's info in the top bar, so you can render the page in right there but it needs to include the {user: req.user} key/value.

I'm not sure about sending the 404 response, can you do res.send(404).redirect("/404")? Or maybe res.status(404).redirect... or something. I'm not that familiar with the different response sendings.

Either way, I feel like being consistent with how we handle errors is important so that our app doesn't break.

loganwatanabe commented 10 years ago

Actually, none of those methods work, since the redirect sends a 302.

So I guess we can set up this instead to get the status and the page easily:

http://stackoverflow.com/questions/6528876/how-to-redirect-404-errors-to-a-page-in-expressjs

AEgan commented 10 years ago

Could we render the 404 and add the {user: req.user}?

res.status(404).render('404', {user: req.user});

if not I'll change it to keep it consistent, not that big of a deal.

loganwatanabe commented 10 years ago

I made a default error page view, app.get("*", home.err), in routes which will send any unspecified paths to the 404 page. But yeah, I'll change it back to what you have above.