AuburnACM / auacm

The Auburn ACM Website
Apache License 2.0
15 stars 3 forks source link

Add a 404 page #84

Closed BrandonLMorris closed 8 years ago

BrandonLMorris commented 8 years ago

Add a catch-all Angular route that will redirect to the 404 page.

I resisted adding a similar redirect to the API. The reason being that most of our interactions with the API are to get data, not actual pages. However, that does mean that unless the user attempts to go to an incorrect URL that starts with /#/, they won't see our fancy new page.

DaveShuckerow commented 8 years ago

image not bad

WilliamHester commented 8 years ago

You mentioned that the 404 page only works if the user tries to go to a URL that starts with /#. There's actually a way we can fix that. This article talks about enabling "HTML5 mode," which drops that part of the URL.

BrandonLMorris commented 8 years ago

That would require fixing all the links in the rest of the client, right? I like the idea, though it's perhaps out of the scope of this PR.

Now that I think about it, you can set Flask to capture 404s, and I can set that to redirect to #/404. I can also set a catch-all /api/<path:path> for bad API requests that will just send back a 404 (to keep the API client-agnostic). I'll do that when I get a chance

WilliamHester commented 8 years ago

Yeah, I guess it's kind of out of scope for this pull.

I like your idea about the redirects. Can you do that for this pull?

BrandonLMorris commented 8 years ago

Sure deal. I'll try to get it in tonight-ish. I know Dave is excited to get this live

BrandonLMorris commented 8 years ago

Ok I was thinking about it again and changed my mind about having Flask redirect to /#/404 because I felt that it was too closely coupling the server code and the API code. So here's the balance I struck: I moved the 404.html to be a Flask template.

Reasoning Like I mentioned, I really want to keep the server code away from the client code (point number 3). While this isn't perfect, I think it strikes a nice medium. The client handles its own bad routes with the full knowledge of Angular and the rest of the client app. The server side, agnostic to the client, does it's best to handle bad routes without depending on any implementation details of the client.

Probably the ideal solution would be to chop off the # prefix and let Angular handle all bad routes (though that may complicate things with our /api prefix), but this is what I came up with. Under this system, you could theoretically remove the client app entirely and replace it with something else, which is good, right?

tl;dr api design is hard.