I am writing some knex queries to check if a supplied code otherwise known as a voucher
is legitimate under 4 constraints.
- The voucher exists
- The voucher has not expired
- The voucher is for the correct event
- There are enough vouchers left
My current code (despite not being finished) is the following:
export default () => (async (req, res) => { const { eventId, orderId, voucher, left } = req.params;
const [{ code }, { eventId }, { expiryDate }, {quantity}] = await Promise.all([ knex('vouchers') .where({ code: voucher }) .first(), knex('vouchers') .where({ event_id }) .first(), knex('mv_vouchers') .where({ voucher_id: code, // 'left', > , 0, }) .first(), ]); if (code && eventId && expiryDate && quantity) {
await knex.insert([{order_id: orderId}, {voucher_id: voucher}], 'id').into('order_vouchers'); res.status(202).end(); } else{ res.status(404).end(); } });
The section I am worried about is the sectioned code. I saw someone use this online somewhere but can’t for the life of me find it on stackoverflow anymore. Under my assumption the constants code
, eventId
etc should all return true or false and then be able to be used in my if logic. Is this along the right lines or completely wrong?
Advertisement
Answer
Firstly, code
and eventId
should only return a bool if the data type of that column is a bool. To get a bool, you’d have to manipulate the response from each query e.g. by checking for the existence of a property in the response or add case statements to your query (would get a bit messy IMO).
Secondly, I’d suggest moving away from using 4 queries that are combined with Promise.All()
and moving to a single query along the lines of the following (assuming that you don’t need to know explicitly why the query responds with no results):
export default () => (async (req, res) => { const { eventId, voucher } = req.params; const response = await knex('vouchers') .join('mv_vouchers', 'vouchers.id', 'mv_vouchers.voucherId') .where('vouchers.code', '=', voucher) .where('vouchers.event_id', '=', eventId) .first()