Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions .env
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
POSTGRES_USER=postgres
POSTGRES_PASSWORD=123
POSTGRES_DB=postgres
POSTGRES_PORT=5432
44 changes: 44 additions & 0 deletions .github/workflows/test.yml-template
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: Test

on:
pull_request:
branches: [master]

jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [20.x]
services:
postgres:
image: postgres:latest
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: password
POSTGRES_DB: students
POSTGRES_PORT: 5432
POSTGRES_HOST: localhost
ports:
- 5432:5432
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
steps:
- uses: actions/checkout@v2
- name: Set up Node.js
uses: actions/setup-node@v1
with:
node-version: '20'
- name: Install dependencies
run: npm install
- name: Run tests
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: password
POSTGRES_DB: students
POSTGRES_HOST: localhost
POSTGRES_PORT: 5432
run: npm test
4 changes: 0 additions & 4 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,3 @@ node_modules

# MacOS
.DS_Store

# env files
*.env
.env*
9 changes: 5 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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.8.6",
"@mate-academy/scripts": "^2.1.3",
"axios": "^1.7.2",
"eslint": "^8.57.0",
"eslint-plugin-jest": "^28.6.0",
Expand Down
22 changes: 19 additions & 3 deletions src/createServer.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
'use strict';

const createServer = () => {
// your code goes here
};
const express = require('express');
const createExpensesRouter = require('./routes/expenses');
const createUsersRouter = require('./routes/users');
const createCategoriesRouter = require('./routes/categories');

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

// Use express to create a server
Comment on lines +10 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ці асоціації необхідно розкоментувати, щоб Sequelize знав про зв'язок між моделями Expense та Category. Це дозволить вам використовувати такі методи, як include, для отримання пов'язаних даних.

// Add a routes to the server
// Return the server (express app)
app.use(express.json());

app.use('/expenses', createExpensesRouter());
app.use('/users', createUsersRouter());
app.use('/categories', createCategoriesRouter());

return app;
}

module.exports = {
createServer,
Expand Down
16 changes: 16 additions & 0 deletions src/models/Category.model.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict';

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

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

const Category = sequelize.define('Category', {
name: {
type: DataTypes.STRING,
allowNull: false,
},
});

module.exports = {
Category,
};
36 changes: 34 additions & 2 deletions src/models/Expense.model.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,41 @@
'use strict';

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

const { DataTypes } = require('sequelize');
const Expense = sequelize.define(
// your code goes here
'Expense',
{
amount: {
type: DataTypes.INTEGER,
allowNull: false,
Comment on lines +9 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Будь ласка, розкоментуйте ці рядки, а також ті, що стосуються User. Визначення асоціацій є важливим, щоб Sequelize розумів зв'язки між вашими моделями Expense, User та Category. Це дозволить автоматично створити зовнішні ключі і використовувати корисні методи, як-от include.

},

title: {
type: DataTypes.STRING,
allowNull: false,
},

userId: {
type: DataTypes.INTEGER,
allowNull: false,
},

spentAt: {
type: DataTypes.DATE,
allowNull: false,
},

category: {
type: DataTypes.STRING,
allowNull: true,
},
Comment on lines +28 to +31
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With a dedicated Category model, it's a relational database best practice to link Expense and Category using a foreign key. Instead of storing the category name as a STRING, consider adding a categoryId field of type INTEGER that references the id of a category. This will make your data model more robust and efficient. You can then define the association between the models, for example using Expense.belongsTo(Category).

Comment on lines +28 to +31
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Щоб забезпечити цілісність даних, це поле слід замінити на зовнішній ключ до моделі Category. Будь ласка, замініть category: DataTypes.STRING на categoryId: DataTypes.INTEGER.

Comment on lines +28 to +31
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This field should be a foreign key referencing the Category model. Instead of storing the category name as a string, you should store the category's ID. Please change this to categoryId with a type of DataTypes.INTEGER as recommended in the previous review. This is crucial for establishing the correct relationship between expenses and categories.

note: {
type: DataTypes.STRING,
},
},
{
timestamps: false,
},
);

module.exports = {
Expand Down
11 changes: 8 additions & 3 deletions src/models/User.model.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
'use strict';

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

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

const User = sequelize.define(
// your code goes here
);
const User = sequelize.define('User', {
name: {
type: DataTypes.STRING,
Comment on lines +8 to +9
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To establish the relationship between the Expense and Category models within Sequelize, please uncomment these lines. This will allow you to perform queries that join these two tables and ensure the foreign key constraint works correctly.

allowNull: false,
},
});

module.exports = {
User,
Expand Down
8 changes: 8 additions & 0 deletions src/models/models.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@

const { User } = require('./User.model');
const { Expense } = require('./Expense.model');
const { Category } = require('./Category.model');

// Expense.belongsTo(User);
// User.hasMany(Expense);

// Expense.belongsTo(Category); якщо я це додам тести не проходитимуть
// Category.hasMany(Expense);
Comment on lines +7 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's a great practice to define the associations between your models. Uncommenting these lines would establish proper relationships (e.g., one user has many expenses, one category has many expenses). This allows Sequelize to automatically handle foreign keys and enables you to perform more powerful queries, like easily joining tables to get related data.

Comment on lines +10 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These lines must be uncommented to establish the required relationship between Expense and Category. This is a core requirement. After making this change and updating the Expense model, the next step is to update the tests to reflect the new data structure.


module.exports = {
models: {
User,
Expense,
Category,
},
};
60 changes: 60 additions & 0 deletions src/routes/categories.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
'use strict';

const express = require('express');
const { Category } = require('../models/Category.model');

function createCategoryRouter() {
const router = express.Router();

router.get('/', async (req, res) => {
res.send(await Category.findAll());
});

router.post('/', async (req, res) => {
const category = req.body;

if (!category.name) {
return res.status(400).send('Name is required');
}

res.status(201).send(await Category.create(category));
});

router.get('/:id', async (req, res) => {
const category = await Category.findByPk(Number(req.params.id));

if (!category) {
return res.status(404).send('Category not found');
}

res.send(category);
});

router.delete('/:id', async (req, res) => {
const category = await Category.findByPk(Number(req.params.id));

if (!category) {
return res.status(404).send('Category not found');
}

await category.destroy();

res.status(204).send('Deleted');
});

router.patch('/:id', async (req, res) => {
const category = await Category.findByPk(Number(req.params.id));

if (!category) {
return res.status(404).send('Category not found');
}

await category.update(req.body);

res.send(category);
});

return router;
}

module.exports = createCategoryRouter;
106 changes: 106 additions & 0 deletions src/routes/expenses.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
'use strict';

const express = require('express');
const { Expense } = require('../models/Expense.model');
const { User } = require('../models/User.model');
const { Op } = require('sequelize');

function createExpensesRoute() {
const router = express.Router();

router.get('/', async (req, res) => {
const whereClause = {};

if (req.query.userId) {
whereClause.userId = Number(req.query.userId);
}

if (req.query.categories) {
whereClause.category = req.query.categories;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After you update the Expense model to use categoryId instead of a string, this filtering logic will need to be changed to filter by categoryId as well.

}
Comment on lines +18 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This filter is still using whereClause.category. After you've updated the Expense model to use a categoryId, this logic should be changed to filter by the foreign key, categoryId.


if (req.query.from || req.query.to) {
whereClause.spentAt = {};

if (req.query.from) {
whereClause.spentAt[Op.gte] = req.query.from;
}

if (req.query.to) {
whereClause.spentAt[Op.lte] = req.query.to;
}
}

const filteredExpenses = await Expense.findAll({
where: whereClause,
});

res.send(filteredExpenses);
});

router.post('/', async (req, res) => {
const expense = req.body;

if (!expense.amount) {
return res.status(400).send('Amount is required');
}

if (!expense.title) {
return res.status(400).send('title is required');
}

if (!expense.userId) {
return res.status(400).send('userId is required');
}

const user = await User.findByPk(expense.userId);

if (!user) {
return res.status(400).send('Such user doesn`t exist');
}

if (!expense.spentAt) {
return res.status(400).send('spentAt is required');
}

res.status(201).send(await Expense.create(expense));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're validating the existence of a user before creating an expense, which is great. It would be a good practice to apply the same logic for the category. You should verify that the category provided in the request body exists in your Categories table. This prevents saving expenses with invalid or misspelled categories and ensures data integrity. This check becomes much more straightforward if you use a foreign key relationship as suggested in the review for the Expense.model.js file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a critical point for a required change. Before creating the expense, you must add logic to validate the category. If the request body contains a categoryId, you need to check if a category with that ID actually exists in the database. If it does not, you should return a 400 Bad Request error to prevent inconsistent data. You will need to import the Category model to perform this check.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is where the validation for the category should happen. As per the previous feedback, if expense.categoryId is provided, you must first verify that a category with that ID exists in the database. If it does not, you should return a 400 error. This will require you to import the Category model at the top of the file.

});

router.get('/:id', async (req, res) => {
const expense = await Expense.findByPk(Number(req.params.id));

if (!expense) {
return res.status(404).send('Expense not found');
}

res.send(expense);
});

router.delete('/:id', async (req, res) => {
const expense = await Expense.findByPk(Number(req.params.id));

if (!expense) {
return res.status(404).send('Expense not found');
}

await expense.destroy();

res.status(204).send('Deleted');
});

router.patch('/:id', async (req, res) => {
const expense = await Expense.findByPk(Number(req.params.id));

if (!expense) {
return res.status(404).send('Expense not found');
}

await expense.update(req.body);

res.send(expense);
});

return router;
}

module.exports = createExpensesRoute;
Loading
Loading