-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
Add Event Booking APIs #378
Add Event Booking APIs #378
Conversation
@github-actions[bot] is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request involve updates to the Changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
backend/.env.example
Outdated
PORT=3000 | ||
EMAIL_PASS=your_16_digit_pass | ||
JWT_SECRET=secret | ||
GOOGLE_CLIENT_ID=your_google_client_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you delete @haseebzaki-07
const mongoose = require("mongoose"); | ||
const { string } = require("zod"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@haseebzaki-07 is required or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
backend/models/customer.model.js (1)
29-32
: LGTM! Consider adding an index for performance.The implementation of the
bookedEvents
field is correct and aligns with the PR objectives. It effectively establishes a many-to-many relationship between customers and events.Consider the following improvements:
- Add an index to the
bookedEvents
field to enhance query performance, especially as the dataset grows:bookedEvents: [{ type: Schema.Types.ObjectId, ref: "Event", index: true }],- Implement a custom validator to ensure unique event bookings per customer:
Then, define thevalidate: [arrayLimit, '{PATH} exceeds the limit of unique events']arrayLimit
function:function arrayLimit(val) { return new Set(val).size === val.length; }These suggestions can improve performance and data integrity but are not critical for the current implementation.
backend/models/events.model.js (2)
39-42
: LGTM! Consider adding validation for consistency.The addition of the
bookedCustomers
field is well-implemented and aligns with the PR objectives. It correctly establishes a many-to-many relationship between events and customers.For consistency with other fields in the schema, consider adding a
required
property set tofalse
(assuming it's optional) and adefault
value of an empty array. This would make the field's behavior more explicit:bookedCustomers: [{ type: mongoose.Schema.Types.ObjectId, ref: 'Customer' // Reference to the Customer model - }] + }], + required: false, + default: [] }
Line range hint
15-22
: Consider using Date type for improved data handlingWhile the current implementation of
date
andtime
as strings is functional, consider using native Date types for these fields. This change could provide better data integrity and enable easier querying and sorting of events.Here's a suggested improvement:
date: { - type: String, + type: Date, required: true, }, time: { - type: String, + type: Date, required: true, },If you decide to implement this change, remember to update any related controllers or services that interact with these fields to handle Date objects instead of strings.
backend/routes/eventRouter.js (1)
1-2
: Consider using a proper logging mechanism instead of console.log.While console.log can be useful for debugging during development, it's generally not recommended for production code. Consider using the existing logger (imported from "../config/logger") for consistency and better control over log levels.
You could replace the console.log with:
-console.log("eventRouter loaded"); +logger.info("eventRouter loaded");backend/controller/event.controller.js (1)
74-74
: Standardize parameter retrieval for consistencyIn
bookEvent
,customerId
andeventId
are retrieved fromreq.body
, whereas ingetBookedEvents
,customerId
is retrieved fromreq.params
. For consistent API design, consider using the same method of parameter retrieval in both functions.If you choose to retrieve IDs from
req.params
in both functions, adjust the code and routes accordingly.Also applies to: 103-103
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- README.md (0 hunks)
- backend/.env.example (0 hunks)
- backend/controller/event.controller.js (2 hunks)
- backend/index.js (1 hunks)
- backend/models/customer.model.js (1 hunks)
- backend/models/events.model.js (1 hunks)
- backend/routes/eventRouter.js (3 hunks)
💤 Files with no reviewable changes (2)
- README.md
- backend/.env.example
🧰 Additional context used
🔇 Additional comments (6)
backend/routes/eventRouter.js (2)
10-11
: LGTM: New controller methods imported correctly.The new methods
bookEvent
andgetBookedEvents
have been properly imported from the event controller. This aligns with the PR objectives of implementing event booking functionality.
36-36
: LGTM: Delete route corrected and improved.The delete route has been correctly updated to use the DELETE HTTP method and now includes an ID parameter. This change allows for deleting specific events and aligns with the PR objectives.
backend/index.js (3)
6-6
: Clarify the purpose of the added comment.The comment "// Corrected typo" has been added, but it's not clear what typo was corrected. The import statement for 'passport' on the next line appears unchanged. Could you please clarify the purpose of this comment or remove it if it's no longer relevant?
Line range hint
1-70
: Overall structure and configuration look good.Apart from the issues mentioned above, the rest of the file is well-structured and properly configured for an Express.js application. It includes necessary imports, middleware setup, database connection, route definitions, and error handling. The changes made don't affect the overall functionality of the application.
9-9
:⚠️ Potential issueFix typo in errorMiddleware import path.
The import statement for errorMiddleware has been changed, but it appears to introduce a typo rather than correct one. The path now includes an extra 'r' in "error".
Please apply this correction:
-const errorMiddleware = require("./middlewares/errrorMiddleware"); +const errorMiddleware = require("./middlewares/errorMiddleware");Also, consider moving the "Corrected typo" comment to this line if it was intended for this change.
To ensure the correct file exists, please run:
backend/controller/event.controller.js (1)
2-2
: LGTMThe import of the
Customer
model is correctly added to support the new booking functionality.
backend/routes/eventRouter.js
Outdated
bookEvents : "/event/book-event", | ||
GetBookedEvents : "/event/get-booked-events/:customerId", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve changes, but correct the booking events endpoint.
The new endpoints for booking events and getting booked events have been correctly added to the API information. However, there's a minor inconsistency in the booking events endpoint.
Please update the booking events endpoint to match the actual route path:
- bookEvents : "/event/book-event",
+ bookEvents : "/event/book-events",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
bookEvents : "/event/book-event", | |
GetBookedEvents : "/event/get-booked-events/:customerId", | |
bookEvents : "/event/book-events", | |
GetBookedEvents : "/event/get-booked-events/:customerId", |
hey @RamakrushnaBiswal made the necessary changes and brought back the .env.example |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
b8aaa36
into
RamakrushnaBiswal:main
fixes #363
This PR adds necessary routes and controllers for a user to book any event, get his all event bookings.
Updated the schema of user and events so that a user can be associated with multiple events also the events can be associated by multiple users.
Upadated the method of delete route from "get" to "delete" because it was a bug also associated "/:id" for the specific event to be deleted.
route to book event:
route to get all bookings of a user:
route to create an event:
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Bug Fixes