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.

Sign up for GitHub

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

Jump to bottom

ft(buyer-cart): should allow a buyer to update their cart [#187364875] #45

Merged
merged 1 commit into from
May 22, 2024
Merged

ft(buyer-cart): should allow a buyer to update their cart [#187364875] #45

merged 1 commit into from
May 22, 2024

Conversation

Aldot-02
Copy link
Collaborator

@Aldot-02 Aldot-02 commented May 14, 2024 •
edited

ℹ️ Description

  • Should allow a buyer to update their cart given a post request to their cart using cart_id
  • The updated cart should be stored in the buyer's session
  • The product details (name, price, image) should be retrieved from the database for each item in the updated cart
  • The new cart total should be calculated by summing the prices of the items in the updated cart
  • An updated cart confirmation message should be sent to the frontend to be displayed to the buyer

Fixes #(issue)

🧪 How can this be tested?

This should be tested on the swagger UI using cart endpoints

📍 Type of change

Please check the options that are relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or Enhancement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

✅ Checklist:

  • My code follows the style guidelines of this project
  • I have created a loom video for the new feature or enhancement
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Aldot-02 Aldot-02 added Awaits another PR Use this on a PR which depends on another PR In progress Use this when you're still working on your task labels May 14, 2024
@Aldot-02 Aldot-02 self-assigned this May 14, 2024
@Aldot-02 Aldot-02 removed the Awaits another PR Use this on a PR which depends on another PR label May 20, 2024
@Aldot-02 Aldot-02 force-pushed the ft-buyer-should-be-able-to-update-their-cart-#187364875 branch 8 times, most recently from 8530aaa to ad7e629 Compare May 21, 2024 11:30
@Aldot-02 Aldot-02 added Needs peer reviews Use this when ready for peer reviews and removed In progress Use this when you're still working on your task labels May 21, 2024
tuyishimekyrie
tuyishimekyrie previously approved these changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@tuyishimekyrie tuyishimekyrie left a comment

Choose a reason for hiding this comment

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

LGTM

@Aldot-02 Aldot-02 force-pushed the ft-buyer-should-be-able-to-update-their-cart-#187364875 branch 2 times, most recently from f143293 to a8a7335 Compare May 21, 2024 12:44
Oliviier-dev
Oliviier-dev previously approved these changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@Oliviier-dev Oliviier-dev left a comment

Choose a reason for hiding this comment

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

LGTM

martinemahirwe
martinemahirwe previously approved these changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@martinemahirwe martinemahirwe left a comment

Choose a reason for hiding this comment

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

Good Work

@Aldot-02 Aldot-02 added Ready Use this when ready for merging and removed Needs peer reviews Use this when ready for peer reviews labels May 21, 2024
SergeInGithub
SergeInGithub requested changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@SergeInGithub SergeInGithub left a comment

Choose a reason for hiding this comment

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

Good work @Aldot-02 Work on the reviews below and you're good to go

src/controllers/cart.controller.ts Outdated
});
}
);
console.log(cart);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
console.log(cart);

src/routes/cart.route.ts Show resolved Hide resolved
@SergeInGithub SergeInGithub added Changes requested and removed Ready Use this when ready for merging labels May 21, 2024
tuyishimekyrie
tuyishimekyrie previously approved these changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@tuyishimekyrie tuyishimekyrie left a comment

Choose a reason for hiding this comment

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

LGTM

@Aldot-02 Aldot-02 added Ready Use this when ready for merging and removed Needs peer reviews Use this when ready for peer reviews labels May 21, 2024
Oliviier-dev
Oliviier-dev previously approved these changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@Oliviier-dev Oliviier-dev left a comment

Choose a reason for hiding this comment

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

LGTM

SergeInGithub
SergeInGithub reviewed May 21, 2024
View reviewed changes
Copy link
Collaborator

@SergeInGithub SergeInGithub left a comment

Choose a reason for hiding this comment

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

Look into this below @Aldot-02

src/routes/cart.route.ts Show resolved Hide resolved
@SergeInGithub SergeInGithub added Changes requested and removed Ready Use this when ready for merging labels May 21, 2024
@Aldot-02 Aldot-02 dismissed stale reviews from Oliviier-dev and tuyishimekyrie via 73d1872 May 21, 2024 19:19
@Aldot-02 Aldot-02 force-pushed the ft-buyer-should-be-able-to-update-their-cart-#187364875 branch from dc9461a to 73d1872 Compare May 21, 2024 19:19
@Aldot-02 Aldot-02 added Ready Use this when ready for merging and removed Changes requested labels May 21, 2024
Oliviier-dev
Oliviier-dev previously approved these changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@Oliviier-dev Oliviier-dev left a comment

Choose a reason for hiding this comment

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

LGTM

SergeInGithub
SergeInGithub reviewed May 22, 2024
View reviewed changes
Copy link
Collaborator

@SergeInGithub SergeInGithub left a comment

Choose a reason for hiding this comment

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

Minor suggestion @Aldot-02

src/validations/cart.validation.ts Outdated
@@ -1,17 +1,34 @@
import Joi from "joi";
import Joi from 'joi';
import { Request, Response, NextFunction } from 'express';

export const validateSchema = (schema: Joi.ObjectSchema) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good. One more thing. I just saw that you guys are repeating yourselves with this function. Suggestion, remove it from here and put it in like utils or helpers folder.... then we shall import it and reuse it in routes.

@SergeInGithub SergeInGithub added Changes requested Needs Rebase and removed Ready Use this when ready for merging labels May 22, 2024
ft(buyer-cart): Should allow a buyer to update their cart
75f5f20 
The updated cart should be stored in the buyer's session
The product details (name, price, image) should be retrieved from the database for each item in the updated cart
The new cart total should be calculated by summing the prices of the items in the updated cart
An updated cart confirmation message should be sent to the frontend to be displayed to the buyer
[Deliver #187364875]
@Aldot-02 Aldot-02 force-pushed the ft-buyer-should-be-able-to-update-their-cart-#187364875 branch from 73d1872 to 75f5f20 Compare May 22, 2024 07:36
@Aldot-02 Aldot-02 added Ready Use this when ready for merging and removed Needs Rebase Changes requested labels May 22, 2024
SergeInGithub
SergeInGithub approved these changes May 22, 2024
View reviewed changes
Copy link
Collaborator

@SergeInGithub SergeInGithub left a comment

Choose a reason for hiding this comment

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

LGTM

@SergeInGithub SergeInGithub merged commit be70914 into develop May 22, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SergeInGithub SergeInGithub SergeInGithub approved these changes

@martinemahirwe martinemahirwe Awaiting requested review from martinemahirwe

@Oliviier-dev Oliviier-dev Awaiting requested review from Oliviier-dev

@tuyishimekyrie tuyishimekyrie Awaiting requested review from tuyishimekyrie

Assignees

@Aldot-02 Aldot-02

Labels
Ready Use this when ready for merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Aldot-02 @tuyishimekyrie @SergeInGithub @Oliviier-dev @martinemahirwe