when pay_later flag is passed, we do not need to generate order_ids#204
when pay_later flag is passed, we do not need to generate order_ids#204vendz wants to merge 1 commit into
Conversation
Summary of ChangesHello @vendz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial enhancement to the booking process by ensuring that payment order IDs are only generated when an immediate payment is required. This change streamlines the 'pay later' booking flow, preventing the creation of unnecessary payment orders and improving the efficiency of the system. Additionally, several code formatting adjustments have been made to enhance readability and maintainability across key controller and helper files. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly implements the pay_later flag to conditionally skip order ID generation. However, I've identified a critical issue where an await keyword was removed from a promise-based call, which will break order creation. Additionally, a country-specific check was removed from the payment logic in two controller files, which could lead to unintended behavior for international users. There is also a minor issue of an unused import. Please review the detailed comments.
| var order; | ||
| if (['prod', 'qa'].includes(process.env.NODE_ENV) && amount > 0) { | ||
| order = await razorpay.orders.create(options); | ||
| order = razorpay.orders.create(options); |
There was a problem hiding this comment.
The await keyword has been removed from the razorpay.orders.create(options) call. Since this function returns a Promise, removing await will cause the order variable to be assigned the Promise object itself, not the resolved order data. Consequently, any subsequent code attempting to access properties like order.id will fail, breaking the order creation process.
| order = razorpay.orders.create(options); | |
| order = await razorpay.orders.create(options); |
| if (req.user.country == 'India' && amount > 0) { | ||
| const payLater = req.body.pay_later === true; | ||
|
|
||
| if (amount > 0 && !payLater) { |
There was a problem hiding this comment.
The condition to generate an order ID has been changed, removing the req.user.country == 'India' check. This will now trigger order ID generation for non-Indian users. As Razorpay is the payment provider, this could cause issues if it's not configured for international payments. Was this change intentional? If so, consider adding a comment to clarify why this check is no longer necessary.
| if (req.user.country == 'India' && amount > 0) { | ||
| const payLater = req.body.pay_later === true; | ||
|
|
||
| if (amount > 0 && !payLater) { |
There was a problem hiding this comment.
Similar to the change in guestBooking.controller.js, the check for req.user.country == 'India' has been removed. This will cause order IDs to be generated for non-Indian users, which might lead to payment failures if the Razorpay integration doesn't support international transactions. Please verify if this is the intended behavior.
| import database from '../../config/database.js'; | ||
| import ApiError from '../../utils/ApiError.js'; | ||
| import moment from 'moment'; | ||
| import logger from '../../config/logger.js'; |
No description provided.