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

solution #259

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6,403 changes: 4,293 additions & 2,110 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
},
"devDependencies": {
"@mate-academy/eslint-config": "latest",
"@mate-academy/scripts": "^1.7.3",
"@mate-academy/scripts": "^1.8.1",
"axios": "^1.6.7",
"eslint": "^7.32.0",
"eslint-plugin-jest": "^22.4.1",
Expand Down
101 changes: 101 additions & 0 deletions src/controllers/expenseController.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
const expenseService = require('../services/expenseService');
const userService = require('../services/userService');

module.exports = {
async getAll(req, res) {
const { userId, categories, from, to } = req.query;

Choose a reason for hiding this comment

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

Consider creating a middleware folder and placing middleware here for checking and validating the user input from the request before passing it to the controller. The user data from the request should be sanitized. Also, you will be able to send 400 errors with additional information about errors.

const numberUserId = parseInt(userId);

const expenses = await expenseService.getAllFiltered({
userId: numberUserId,
categories,
from,
to,
});

res.status(200).send(expenses.map(expenseService.format));

Choose a reason for hiding this comment

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

Also, it would be better to wrap this bunch of code inside the try-catch block to handle errors related to the DB connection, etc. In this case, we should provide a client with a proper error code and avoid throwing an error object directly to the client.

Choose a reason for hiding this comment

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

Suggested change
const expenses = await expenseService.getAllFiltered({
userId: numberUserId,
categories,
from,
to,
});
res.status(200).send(expenses.map(expenseService.format));
try {
const expenses = await expenseService.getAllFiltered({
userId: numberUserId,
categories,
from,
to,
});
res.status(200).send(expenses.map(expenseService.format));
} catch(err) {
res.statusCode = 500;
res.send('Something went wrong);
}

},
async getOne(req, res) {
const id = parseInt(req.params.id);

const expense = await expenseService.getById(id);

if (!expense) {
res.sendStatus(404);

return;
}

res.status(200).send(expenseService.format(expense));
},
async create(req, res) {
const { userId, spentAt, title, amount, category, note } = req.body;

const foundUser = await userService.getById(userId);

if (!userId || !spentAt || !title || !amount || !category || !foundUser) {
res.sendStatus(400);

return;
}

const expense = await expenseService.create({
userId,
spentAt,
title,
amount,
category,
note,
});

res.status(201).send(expenseService.format(expense));
},
async update(req, res) {
const currentId = parseInt(req.params.id);
const { id, userId, spentAt, title, amount, category, note } = req.body;

const foundExpense = await expenseService.getById(currentId);

if (!foundExpense) {
res.sendStatus(404);

return;
}

if (!id && !userId && !spentAt && !title && !amount && !category && !note) {
res.sendStatus(400);

return;
}

await expenseService.update({
currentId,
id,
userId,
spentAt,
title,
amount,
category,
note,
});

const updatedExpense = await expenseService.getById(id ?? currentId);

res.status(200).send(expenseService.format(updatedExpense));
},
async remove(req, res) {
const id = parseInt(req.params.id);

const foundExpense = await expenseService.getById(id);

if (!foundExpense) {
res.sendStatus(404);

return;
}

await expenseService.remove(id);

res.sendStatus(204);
},
};
75 changes: 75 additions & 0 deletions src/controllers/userController.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
const userService = require('../services/userService');

module.exports = {
async getAll(_req, res) {
const users = await userService.getAll();

res.status(200).send(users);
},
async getOne(req, res) {
const id = parseInt(req.params.id);

const user = await userService.getById(id);

if (!user) {
res.sendStatus(404);

return;
}

res.status(200).send(user);
},
async create(req, res) {
const { name } = req.body;

if (!name) {
res.sendStatus(400);

return;
}

const user = await userService.create({ name });

res.status(201).send(user);
},
async update(req, res) {
const currentId = parseInt(req.params.id);

const { name, id } = req.body;

if (!name && !id) {
res.sendStatus(400);

return;
}

const foundUser = await userService.getById(currentId);

if (!foundUser) {
res.sendStatus(404);

return;
}

await userService.update({ currentId, id, name });

const updatedUser = await userService.getById(id ?? currentId);

res.status(200).send(updatedUser);
},
async remove(req, res) {
const id = parseInt(req.params.id);

const foundUser = await userService.getById(id);

if (!foundUser) {
res.sendStatus(404);

return;
}

await userService.remove(id);

res.sendStatus(204);
},
};
17 changes: 14 additions & 3 deletions src/createServer.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
'use strict';

const createServer = () => {
// your code goes here
};
const express = require('express');

const { userRoute } = require('./routes/userRoute');
const { expenseRouter } = require('./routes/expenseRoute');

function createServer() {
const app = express();

app.use(express.json());
app.use('/users', userRoute);
app.use('/expenses', expenseRouter);

return app;
}

module.exports = {
createServer,
Expand Down
39 changes: 38 additions & 1 deletion src/models/Expense.model.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,46 @@
'use strict';

const { DataTypes } = require('sequelize');

const { sequelize } = require('../db.js');
const { User } = require('./User.model.js');

const Expense = sequelize.define(
// your code goes here
'Expense',
{
id: {
type: DataTypes.INTEGER,
primaryKey: true,
autoIncrement: true,
},
title: {
type: DataTypes.STRING,
allowNull: false,
},
amount: {
type: DataTypes.INTEGER,
allowNull: false,
},
category: {
type: DataTypes.STRING,
allowNull: false,
},
userId: {
field: 'user_id',
type: DataTypes.INTEGER,
references: {
model: User,
key: 'id',
},
},
note: DataTypes.STRING,
},
{
tableName: 'expense',
timestamps: true,
createdAt: 'spentAt',
updatedAt: true,
},
);

module.exports = {
Expand Down
19 changes: 18 additions & 1 deletion src/models/User.model.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
'use strict';

const { DataTypes } = require('sequelize');

const { sequelize } = require('../db.js');

const User = sequelize.define(
// your code goes here
'User',
{
id: {
type: DataTypes.INTEGER,
primaryKey: true,
autoIncrement: true,
},
name: {
type: DataTypes.STRING,
allowNull: false,
},
},
{
timestamps: false,
tableName: 'user',
},
);

module.exports = {
Expand Down
12 changes: 12 additions & 0 deletions src/routes/expenseRoute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const { Router } = require('express');
const expenseController = require('../controllers/expenseController');

const router = Router();

router.get('/', expenseController.getAll);
router.get('/:id', expenseController.getOne);
router.post('/', expenseController.create);

Choose a reason for hiding this comment

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

Suggested change
router.post('/', expenseController.create);
router.post('/', yourMiddlewareToCheckAndSanitizeTheRequest, expenseController.create);

Middlewares should be placed here

router.patch('/:id', expenseController.update);
router.delete('/:id', expenseController.remove);

module.exports.expenseRouter = router;
13 changes: 13 additions & 0 deletions src/routes/userRoute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const { Router } = require('express');

const userController = require('../controllers/userController');

const router = Router();

router.get('/', userController.getAll);
router.get('/:id', userController.getOne);
router.post('/', userController.create);
router.patch('/:id', userController.update);
router.delete('/:id', userController.remove);

module.exports.userRoute = router;
82 changes: 82 additions & 0 deletions src/services/expenseService.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
const { Op } = require('sequelize');
const { Expense } = require('../models/Expense.model');

module.exports = {
getAllFiltered(query) {
const where = this.configureWhere(query);

return Expense.findAll({
where,
});
},
getById(id) {
return Expense.findByPk(id);
},
create({ userId, spentAt, title, amount, category, note }) {
return Expense.create({
userId,
spentAt,
title,
amount,
category,
note,
});
},
update({ currentId, id, userId, spentAt, title, amount, category, note }) {
return Expense.update(
{
id,
userId,
spentAt,
title,
amount,
category,
note,
},
{ where: { id: currentId } },
);
},
async remove(id) {
await Expense.destroy({ where: { id } });
},
format({ id, userId, spentAt, title, amount, category, note }) {
return {
id,
userId,
spentAt,
title,
amount,
category,
note,
};
},
configureWhere({ userId, categories, from, to }) {
const where = {};

if (userId) {
where.userId = userId;
}

if (categories) {
where.category = {
[Op.in]: [categories].flat(),
};
}

if (from) {
if (!where.spentAt) {
where.spentAt = {};
}
where.spentAt[Op.gte] = from;
}

if (to) {
if (!where.spentAt) {
where.spentAt = {};
}
where.spentAt[Op.lte] = to;
}

return where;
},
};
Loading
Loading