0

I have a node.js API I am working on that has some parameter variables in the URL.

One of the tasks I need to achieve is that when a particular id is not valid (as in the id number does not exist in the sqlite database), is that it returns a 404

Typical url setup is localhost/api/employees/:id/timesheets

What I need to do is when that :id does not exist in the sqlite database, return a 404 however it is returning a 201 no matter what I do at the moment.

Please find my code below from the timesheets endpoint and let me know where I am going wrong, be much appreciated. In a nutshell, let's say employee 200 does not exist (so localhost/api/employees/200/timesheets - I want a 404 on that timesheets endpoint but currently it gives a 200 and is not 404'ing. Note that none of the methods are giving a 404 as required at the moment in the below code

Thanks

const express = require('express');
const timesheetsRouter = express.Router({mergeParams: true});

const sqlite3 = require('sqlite3');
const db = new sqlite3.Database(process.env.TEST_DATABASE || './database.sqlite');


timesheetsRouter.param('id', (req, res, next, id) => {
  req.id = id;
  console.log('doing name validations on ' + id);
  db.get('Select * from Timesheet where employee_id = $id', {$id: id}, (error, timesheet) => {
    if (error) {
      next(error);
    } else if (timesheet) {
      req.timesheet = timesheet;
      next();
    } else {
      res.sendStatus(404);
    }
  });
});

timesheetsRouter.get('/', (req, res, next) => {
  db.all(`select * from Timesheet where employee_id = $id`, {$id: req.params.id}, (error, rows) => {
      //console.log('executed sql');
      //console.log(rows);
    if (!rows) {
      console.log('triggered');
        //console.log('this is error ' + error);
        res.sendStatus(404);
      //next();
    } else {
      console.log(rows + ' This is rows');
      res.status(200).json({timesheets: rows});
    }
  });
});


const validateTimesheet = (req, res, next) => {
    //console.log('this is menu ' + req.body);
  if (!req.body.timesheet.hours || !req.body.timesheet.rate || !req.body.timesheet.date) {
    return res.sendStatus(400);
  }
  next();
}



  timesheetsRouter.post('/', validateTimesheet, (req, res, next) => {    
    //console.log('this is mmenu post body ' + req.body.menu.title);
    db.run(`INSERT INTO Timesheet(hours, rate, date, employee_id) VALUES ($hours, $rate, $date, $employee_id)`, 
    { $hours: req.body.timesheet.hours, $rate: req.body.timesheet.rate, $date: req.body.timesheet.date, $employee_id:req.params.id}, function (error) {
        if (error) {
            ('error with sql  ' + console.log(error));
            return res.sendStatus(500);
        }   

        db.get(`SELECT * FROM Timesheet WHERE id = ${this.lastID}`, (err, row) => {
      if (err) {
          //console.log(err);
        return res.sendStatus(500);
      } else {
       res.status(201).json({timesheet: row});
      }
  });
    })
})

timesheetsRouter.put('/:id', validateTimesheet, (req, res, next) => {

    const timesheetToUpdate = req.body.employee;
    //console.log(artistToUpdate);
    //console.log("this is params " + req.params.id);
    db.run(`UPDATE Timesheet SET hours=$hours, rate=$rate, date=$date where id=${req.params.id}`,
    {$hours:req.body.timesheet.hours, $rate: req.body.timesheet.rate, $date:req.body.timesheet.date}), function (error, row) {
        console.log(row);
        if (error) {
            console.log('this is error ' + error);
            return res.status(404).send();
        }
    }
        db.get(`SELECT * from Timesheet where id = $id`, {$id: req.params.id}, (error, row) => {
            if(!row) {
                return res.sendStatus(500);
            }
            //console.log(row);
            res.status(200).send({timesheet: row});
        })

    });




module.exports = timesheetsRouter;
Yangshun Tay
  • 33,183
  • 26
  • 102
  • 123
  • 1
    did you try logging what timesheets logs , i guess it returns some data from db so it is not going to your else – Rahul Singh Jan 07 '18 at 07:25
  • hi Rahul, yes i sure did and it didn't help too much, its basically just returning a empty object. Agree that its not getting to the 404 point but im not sure why! – Chocolol Jan 07 '18 at 07:31
  • check for then if not empty object. But in general if should work or you can use underscore js and try `_.isEmpty(object)` – Rahul Singh Jan 07 '18 at 07:36

2 Answers2

0

Run this in terminal npm install lodash --save. Lodash is a pretty nifty utility to use in any project

I have used _.isEmpty() to check whether you have received data and then proceed with it accordingly. You can check the details here https://lodash.com/docs/#isEmpty

const express = require('express');
const timesheetsRouter = express.Router({mergeParams: true});

const sqlite3 = require('sqlite3');
const _ = require('lodash');
const db = new sqlite3.Database(process.env.TEST_DATABASE || './database.sqlite');


timesheetsRouter.param('id', (req, res, next, id) => {
  req.id = id;
  console.log('doing name validations on ' + id);
  db.get('Select * from Timesheet where employee_id = $id', {$id: id}, (error, timesheet) => {
    if (error) {
      next(error);
    } else if (!_.isEmpty(timesheet)) {
      req.timesheet = timesheet;
      next();
    } else {
      res.sendStatus(404);
    }
  });
});

timesheetsRouter.get('/', (req, res, next) => {
  db.all(`select * from Timesheet where employee_id = $id`, {$id: req.params.id}, (error, rows) => {
      //console.log('executed sql');
      //console.log(rows);
    if (!rows) {
      console.log('triggered');
        //console.log('this is error ' + error);
        res.sendStatus(404);
      //next();
    } else {
      console.log(rows + ' This is rows');
      res.status(200).json({timesheets: rows});
    }
  });
});


const validateTimesheet = (req, res, next) => {
    //console.log('this is menu ' + req.body);
  if (!req.body.timesheet.hours || !req.body.timesheet.rate || !req.body.timesheet.date) {
    return res.sendStatus(400);
  }
  next();
}



  timesheetsRouter.post('/', validateTimesheet, (req, res, next) => {    
    //console.log('this is mmenu post body ' + req.body.menu.title);
    db.run(`INSERT INTO Timesheet(hours, rate, date, employee_id) VALUES ($hours, $rate, $date, $employee_id)`, 
    { $hours: req.body.timesheet.hours, $rate: req.body.timesheet.rate, $date: req.body.timesheet.date, $employee_id:req.params.id}, function (error) {
        if (error) {
            ('error with sql  ' + console.log(error));
            return res.sendStatus(500);
        }   

        db.get(`SELECT * FROM Timesheet WHERE id = ${this.lastID}`, (err, row) => {
      if (err) {
          //console.log(err);
        return res.sendStatus(500);
      } else {
       res.status(201).json({timesheet: row});
      }
  });
    })
})

timesheetsRouter.put('/:id', validateTimesheet, (req, res, next) => {

    const timesheetToUpdate = req.body.employee;
    //console.log(artistToUpdate);
    //console.log("this is params " + req.params.id);
    db.run(`UPDATE Timesheet SET hours=$hours, rate=$rate, date=$date where id=${req.params.id}`,
    {$hours:req.body.timesheet.hours, $rate: req.body.timesheet.rate, $date:req.body.timesheet.date}), function (error, row) {
        console.log(row);
        if (error) {
            console.log('this is error ' + error);
            return res.status(404).send();
        }
    }
        db.get(`SELECT * from Timesheet where id = $id`, {$id: req.params.id}, (error, row) => {
            if(!row) {
                return res.sendStatus(500);
            }
            //console.log(row);
            res.status(200).send({timesheet: row});
        })

    });




module.exports = timesheetsRouter;
Kushal
  • 1,282
  • 6
  • 15
0

If rows is an empty object when there is no such id, then !rows is !{} which is false. Hence it enters the else condition instead.

The negation of an empty object (!{}) is false as {} is not a falsy value. The only falsy values are false, 0, "", null, undefined, and NaN.

A simple way to check if an object is empty is Object.keys({}).length === 0. There's no need to install Lodash just to use _.isEmpty.

Simply change your code to

timesheetsRouter.get('/', (req, res, next) => {
  db.all(`select * from Timesheet where employee_id = $id`, {$id: req.params.id}, (error, rows) => {
    if (Object.keys(rows).length === 0) {
      res.sendStatus(404);
    } else {
      console.log(rows + ' This is rows');
      res.status(200).json({timesheets: rows});
    }
  });
});
Yangshun Tay
  • 33,183
  • 26
  • 102
  • 123
  • Thanks Yangshun that does help, but i need to return a empty array for valid id numbers and this just returns a 404 in that scenario :( – Chocolol Jan 07 '18 at 08:05
  • You should clarify that the callback values are in both cases. I'm not familiar with sqlite APIs – Yangshun Tay Jan 07 '18 at 08:07
  • Hi yangshun, if there is timesheet data a arrray is returned, if there is no data returned it is a empty array and this is actually a valid response. What i am trying to do is have it 404 when a prior variable in the route is not present in the database. So for example if employee 800 does not exist (url would be localhost/api/employees/800/timesheets - this should 404 but if 801 exists without a timesheet, a empty array is valid. – Chocolol Jan 07 '18 at 08:15
  • Then you should be doing a `select * from Employee where employee_id = $id` before selecting from `Timesheet`s. If selecting from `Employee`s give you empty response, return a 404. I don't think there's a way to do them in one query. – Yangshun Tay Jan 07 '18 at 16:56