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

Much more robust API (may help a lot of people) #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dstroot
Copy link

@dstroot dstroot commented Jan 25, 2013

Christophe: LOVE your tutorials! I have learned quite a bit! I wanted to give back by improving your wines api a bit -

  1. Returns proper HTML response codes
  2. Much faster! (because it returns proper response codes browser
    doesn't wait...)
  3. Can't crash the app by sending bad data (checks what you send it)
  4. Makes sure the browser won't cache the data (problem solved)
  5. If it can't update or delete an item you now will be notified
    properly
  6. res.send changed to res.json

Dan Stroot added 2 commits January 24, 2013 20:04
0) Returns proper HTML response codes
1) Much faster! (because it returns proper response codes browser
doesn't wait!)
2) Can't crash the app by sending bad data (checks what you send it)
3) Makes sure the browser won't cache the data (problem solved)
4) If it can't update or delete an item you now will be notified
properly
Christophe: LOVE your tutorials! I have learned quite a bit! I wanted to
give back by improving your wines api a bit -

0) Returns proper HTML response codes
1) Much faster! (because it returns proper response codes browser
doesn't wait...)
2) Can't crash the app by sending bad data (checks what you send it)
3) Makes sure the browser won't cache the data (problem solved)
4) If it can't update or delete an item you now will be notified
properly
5) res.send changed to res.json
// if (req.session.userAuthenticated) {

// We don't want the browser to cache the results
res.header('Cache-Control', 'no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just a watcher on this repo but wanted to comment that it's awesome that you've added no caching ✨

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Ryan!

On Thu, Jan 24, 2013 at 8:45 PM, Ryan Regalado [email protected]:

In routes/wines.js:

exports.findById = function(req, res) {
+

  • // At some point we may want to lock this api down!
  • // If this is on the Internet someone could easily
  • // steal, modify or delete your data! Need something like
  • // this on our api (assuming we have people authenticate):
  • // if (req.session.userAuthenticated) {
  • // We don't want the browser to cache the results
  • res.header('Cache-Control', 'no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0');

I'm just a watcher on this repo but wanted to comment that it's awesome
that you've added no caching [image: ✨]


Reply to this email directly or view it on GitHubhttps://github.com//pull/6/files#r2770226.

  • Dan Stroot

@dstroot
Copy link
Author

dstroot commented Jan 26, 2013

Christophe - sorry I put this in the master branch - kind of new to all this. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants