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

Dev/ananya/point shop rework #273

Merged
merged 17 commits into from
Feb 2, 2025
Merged

Dev/ananya/point shop rework #273

merged 17 commits into from
Feb 2, 2025

Conversation

ananyaanand4
Copy link
Contributor

point shop rework

src/services/shop/shop-router.ts Outdated Show resolved Hide resolved
src/services/shop/shop-router.ts Outdated Show resolved Hide resolved
src/services/shop/shop-router.ts Show resolved Hide resolved
src/services/shop/shop-router.ts Show resolved Hide resolved
src/services/shop/shop-router.ts Outdated Show resolved Hide resolved
Copy link
Member

@Timothy-Gonzalez Timothy-Gonzalez left a comment

Choose a reason for hiding this comment

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

This is a large review - please make sure you don't miss some of the more important comments buried in it since GitHub likes to hide the middle ones. I've summarized below, but make sure you resolve each comment as you fix them. Thanks!
image
^ Make sure to expand ^

Please make sure you're not deleting existing endpoints - all of the /shop/item endpoints are gone. That's also why the diff is so bad - you're replacing the item endpoints. Instead, just add your new ones.

Two main issues:

  • We need to verify the order is still valid every time it is gotten - as quanities change over time. If the quantities in the shop aren't matching anymore, decrease the items they have. So, if they have 15 item1 in their cart, and the quantity of item1 has dropped to 10, they now have 10 item1 in their cart.
  • To do so, we should be using singular operations. Calling findOne in a loop is almost always incorrect. Call find with a set of ids you want, then loop over the result. You should also do this for updates. This will reduce the db calls for a large order substantionally, improve latency, and reduce race conditions.

Finally, there's also a bit of minor code convention stuff which should be pretty easy to cleanup.

Feel free to message me if you have specific questions.

src/common/config.ts Show resolved Hide resolved
src/services/shop/shop-schemas.ts Outdated Show resolved Hide resolved
src/services/shop/shop-schemas.ts Outdated Show resolved Hide resolved
src/services/shop/shop-schemas.ts Outdated Show resolved Hide resolved
src/services/shop/shop-schemas.ts Outdated Show resolved Hide resolved
src/services/shop/shop-router.test.ts Outdated Show resolved Hide resolved
src/services/shop/shop-router.test.ts Outdated Show resolved Hide resolved
src/services/shop/shop-router.test.ts Show resolved Hide resolved
src/services/shop/shop-router.test.ts Outdated Show resolved Hide resolved
src/services/shop/shop-router.test.ts Outdated Show resolved Hide resolved
@aletya aletya merged commit 3194654 into main Feb 2, 2025
4 checks passed
@aletya aletya deleted the dev/ananya/point-shop-rework branch February 2, 2025 19:46
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.

3 participants