Feature db update #22
Conversation
|
Should fix #5 completely now |
|
wrong button |
| @@ -4,6 +4,9 @@ | |||
| * Author: Sandro Speth | |||
| * Author: Tobias Wältken | |||
| */ | |||
|
|
|||
| /* jslint esversion: 6 */ | |||
There was a problem hiding this comment.
Maybe create/decide on project wide jslint settings to enforce a uniform coding style
There was a problem hiding this comment.
this should be another issue
| break; | ||
| let stmt = db.prepare("SELECT price, stock FROM Beverages WHERE name = ?;"); | ||
| stmt.get(beverage, function(err, result) { | ||
| if (result == undefined) { |
There was a problem hiding this comment.
Check err first to allow for better error handling as well as catching database errors and other anomalies.
| console.log('[API] [FAIL] can\'t write /data/beverages.json'); | ||
| }); | ||
| break; | ||
| let stmt = db.prepare("SELECT price, stock FROM Beverages WHERE name = ?;"); |
There was a problem hiding this comment.
Name it stmt0 for consistency with stmt1, stmt2 and stmt3 ;-)
There was a problem hiding this comment.
should be new issue
src/api.js
Outdated
| stmt.run(uuidv4(), user, beverage, -cost, new Date().toUTCString()); | ||
| stmt.finalize(); | ||
|
|
||
| if (result.stock === 0) { |
There was a problem hiding this comment.
Do not check for Zero, although subzero stock makes no sense it is better to have records of all purchases than users beeing unable to purchase the beverage their holding because of some other error.
| @@ -19,7 +19,7 @@ var authData = [ | |||
|
|
|||
| db.serialize(function() { | |||
| db.run('DROP TABLE IF EXISTS History;'); | |||
| db.run("CREATE TABLE History (id VARCHAR(255), user VARCHAR(255) NOT NULL, reason VARCHAR(255), amount INTEGER NOT NULL DEFAULT 0, timestamp DATETIME NOT NULL DEFAULT (DATETIME('now', 'localtime')));"); | |||
| db.run("CREATE TABLE History (id VARCHAR(255), user VARCHAR(255) NOT NULL, reason VARCHAR(255), amount INTEGER NOT NULL DEFAULT 0, beverage VARCHAR(255) NOT NULL DEFAULT '', beverage_count INTEGER NOT NULL DEFAULT 0, timestamp DATETIME NOT NULL DEFAULT (DATETIME('now', 'localtime')));"); | |||
There was a problem hiding this comment.
change default value for beverage_count to 1 because it is the commonly used one
There was a problem hiding this comment.
For statements that don't change beverages (user balance changes only) you should be able to leave out the whole beverage part. If default is 1 in sql you would have nasty side effects with such statements.
|
|
||
| var stmt3 = db.prepare("INSERT INTO History(id, user, reason, amount, beverage, beverage_count) VALUES (?, ?, ?, ?, ?, ?);"); | ||
| stmt3.run(uuidv4(), user, beverage, -cost, beverage, 1); | ||
| }); |
There was a problem hiding this comment.
[important] missing the check if the given username exists!
[minor] missing error checking
There was a problem hiding this comment.
checking if user exists is needed in more than one method and should be as simple as a method call. The problem is, that sql statements work with callbacks. I don't have a good solution yet for this. => should be a new issue
| @@ -199,15 +200,16 @@ api.get('/orders', userAccess(function (req, res) { | |||
| api.get('/orders/:userId', userAccess(function (req, res) { | |||
| let userId = req.params.userId; | |||
| let limit = req.query.limit; | |||
| if (userId === undefined || userId === '' || !users.has(userId)) { | |||
| if (userId === undefined || userId === '') { | |||
There was a problem hiding this comment.
missing the check if the given username exists!
src/api.js
Outdated
| stmt.run(userId); | ||
| res.sendStatus(200); | ||
| // why return old user? | ||
| // res.status(200).send(JSON.stringify(user)); |
There was a problem hiding this comment.
remove comment we decided not to anymore
| @@ -361,15 +396,15 @@ api.patch('/users/:userId', adminAccess(function (req, res) { | |||
| let amount = req.query.amount; | |||
| let reason = req.query.reason; | |||
| if (userId != undefined && amount != undefined && reason != undefined | |||
| && userId != '' && reason != '' && amount != '' && users.has(userId)) { | |||
| && userId != '' && reason != '' && amount != '') { | |||
| stmt.run(uuidv4(), userId, reason, amount, new Date().toUTCString()); | ||
|
|
||
| var stmt = db.prepare("INSERT INTO History(id, user, reason, amount) VALUES (?, ?, ?, ?);"); | ||
| stmt.run(uuidv4(), userId, reason, amount); |
There was a problem hiding this comment.
rename stmt to stmt0 to stay consistent
Use the updated db for all api calls.
Fixes:
Needs testing and error handling.