Skip to content

Conversation

@pakeku
Copy link

@pakeku pakeku commented Jun 10, 2025

No description provided.

pakeku added 30 commits May 11, 2025 00:05
…erver and MongoDB connection using stopDatabase(). This improves stability in dev and production environments.
@pakeku
Copy link
Author

pakeku commented Jun 10, 2025

@johnny-rice forgot to make my PR some weeks ago.

@johnny-rice
Copy link
Member

@johnny-rice forgot to make my PR some weeks ago.
@pakeku checking!

@@ -0,0 +1,36 @@
# 📦 .env.sample
Copy link
Member

Choose a reason for hiding this comment

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

Nice job adding an env sample

# Dependency directories
node_modules/
jspm_packages/
package-lock.json
Copy link
Member

Choose a reason for hiding this comment

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

Sometimes good to have package lock committed so the CI can use it with the exact packages it was committed with. Recomend removing it from the ignore.

@@ -0,0 +1,7 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Nice

- Week 10: <https://github.com/SummerOfCode2020/week-10-store-manager-api>

- Week 12: <https://github.com/SummerOfCode2020/week-12-store-manager-api>
# Node.js and Express Backend
Copy link
Member

Choose a reason for hiding this comment

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

Wow - nicely improved version.

@@ -0,0 +1,19 @@
/** @type {import('ts-jest').JestConfigWithTsJest} */
Copy link
Member

Choose a reason for hiding this comment

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

Love seeing jest

{
"name": "week-10",
"version": "1.0.0",
"name": "backend-api",
Copy link
Member

Choose a reason for hiding this comment

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

So long week 10!

return;
}

const hashedPassword = await bcrypt.hash(password, 10);
Copy link
Member

Choose a reason for hiding this comment

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

This is great. 12 rounds could be a good way to go here.

[![code Style: Prettier](https://img.shields.io/badge/code_style-prettier-ff69b4.svg?style=flat&logo=prettier)](https://prettier.io/)
[![ESLint](https://img.shields.io/badge/linting-eslint-blue.svg?style=flat&logo=eslint)](https://eslint.org/)
[![TypeScript](https://img.shields.io/badge/language-typescript-blue.svg?style=flat&logo=typescript)](https://www.typescriptlang.org/)
[![Swagger UI](https://img.shields.io/badge/docs-Swagger_UI-blue?logo=swagger)](http://localhost:3000/api-docs)
Copy link
Member

Choose a reason for hiding this comment

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

Could be good to specify local here since we have localhost hardcoded here.

Copy link
Member

@johnny-rice johnny-rice left a comment

Choose a reason for hiding this comment

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

Great PR @pakeku !! This is a huge improvement to the aged project. Very nice modernization of the repo. These changes make it much more production ready than the old original.

✅ TypeScript conversion is clean and well structured
✅ JWT auth works well with solid tests
✅ Middleware and error handling are much more robust
✅ TDD setup with Jest + Supertest + in-memory DB is excellent
✅ Swagger docs + improved README are very helpful
✅ CI workflow is a great addition

Minor notes:

  • In stores.ts, getStores() strips metadata from response. Just checking if this is intentional, since metadata is used in the update test.
  • Store model vs HTTP test fields (store_profile, shipping_address) have some mismatch and could standardize.

Great work. Ready to approve once you confirm the small field consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants