62 get chats match endpoints#63
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements a GET endpoint at /private/match/:offset/:limit to retrieve matches between users along with their associated chat IDs. The implementation includes database queries to find reciprocal likes (matches) and retrieve the chat ID for matched users.
Key changes:
- Added new
/private/matchendpoint that returns matched users with their profile information and chat IDs - Improved profile picture deletion logic to better handle index updates
- Cleaned up debugging console.log statements throughout the codebase
- Updated test fixtures to use longer bio text that meets validation requirements
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| fastify/assets/srcs/routes/private/match/index.ts | New route definition for GET /match endpoint with offset/limit parameters |
| fastify/assets/srcs/controllers/private/match/index.ts | New controller handler to fetch matches for authenticated user |
| fastify/assets/srcs/models/Like/index.ts | Added Match interface and getMatches() method with SQL query for reciprocal likes |
| fastify/assets/srcs/services/UserService.ts | Added getMatches() service method, updated profile picture deletion logic, removed debug logging |
| fastify/assets/@types/fastify.d.ts | Added getMatches type definition to UserService interface |
| fastify/assets/srcs/routes/private/index.ts | Registered new match routes with /match prefix |
| fastify/assets/srcs/routes/private/user/me/profilePicture.ts | Changed index parameter schema from integer to string with int32 format |
| fastify/assets/srcs/controllers/private/me/profilePictures.ts | Updated to parse index as string and convert to number |
| fastify/assets/srcs/services/BrowsingService.ts | Removed debug console.log statements |
| fastify/assets/srcs/plugins/oAuth/facebook.ts | Removed debug console.log statements |
| fastify/assets/srcs/models/User/index.ts | Removed debug console.log statement |
| postgresql/init.sql | Refactored notifications_details view with clearer alias naming |
| fastify/assets/test/integration/ws/like.test.ts | Added test case for GET /match endpoint |
| fastify/assets/test/integration/ws/ws.test.ts | Updated test bio/tags to meet validation requirements |
| fastify/assets/test/integration/ws/view.test.ts | Removed unused imports |
| fastify/assets/test/integration/ws/browsing.test.ts | Updated test bio/tags to meet validation requirements |
| fastify/assets/test/integration/private/me/profilePicture.test.ts | Updated test bio/tags to meet validation requirements |
| fastify/assets/test/integration/private/me/profile.test.ts | Updated test data and removed obsolete test assertions |
| fastify/assets/test/integration/fixtures/auth.fixtures.ts | Updated quickUser bio template and added error logging |
| fastify/assets/test/integration/auth/auth.test.ts | Updated test bio/tags to meet validation requirements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { offset, limit } = request.params as { offset: string, limit: string }; | ||
| if (!userId) | ||
| throw new UnauthorizedError(); | ||
| const matches = await request.server.userService.getMatches(userId, Number(offset), Number(limit)); |
There was a problem hiding this comment.
No validation for negative or invalid offset/limit values before passing them to the database query. The Number() conversion could result in NaN for non-numeric strings, which would cause SQL query issues. Add validation to ensure offset and limit are non-negative integers before using them.
| const matches = await request.server.userService.getMatches(userId, Number(offset), Number(limit)); | |
| const offsetNum = Number(offset); | |
| const limitNum = Number(limit); | |
| if (!Number.isInteger(offsetNum) || offsetNum < 0 || !Number.isInteger(limitNum) || limitNum < 0) { | |
| return reply.status(400).send({ error: 'Invalid offset or limit' }); | |
| } | |
| const matches = await request.server.userService.getMatches(userId, offsetNum, limitNum); |
| profilePicture: row.profile_picture, | ||
| chatId: row.chat_id | ||
| }))); | ||
| } catch { |
There was a problem hiding this comment.
The catch block swallows all error information without logging. This makes debugging difficult when database queries fail. Consider logging the error before throwing InternalServerError, similar to how it's done in other methods in this class (e.g., getAllByLikerId and getAllByLikedId).
| } catch { | |
| } catch (error) { | |
| this.fastify.log.error((error as Error).message || undefined); |
| user.profilePictureIndex = user.profilePictures.length > 0 ? user.profilePictureIndex - 1 : undefined; | ||
| } else if (pictureIndex === user.profilePictureIndex) { |
There was a problem hiding this comment.
The logic for updating profilePictureIndex when removing a picture at an index less than the current index appears incorrect. When you decrement the index, you're checking if profilePictures.length > 0, but this check happens after the picture has already been removed from the array. If there are still pictures remaining after removal, the index should always be decremented by 1, not conditionally set to undefined. The current logic could incorrectly set the index to undefined when there are still pictures in the array.
| user.profilePictureIndex = user.profilePictures.length > 0 ? user.profilePictureIndex - 1 : undefined; | |
| } else if (pictureIndex === user.profilePictureIndex) { | |
| // A picture before the current profile picture was removed: | |
| // shift the profile picture index left by 1. | |
| user.profilePictureIndex = user.profilePictureIndex - 1; | |
| } else if (pictureIndex === user.profilePictureIndex) { | |
| // The current profile picture was removed: reset to first picture if any remain. |
| age: { type: 'integer' }, | ||
| commonInterests: { type: 'integer' }, | ||
| distance: { type: 'number' }, |
There was a problem hiding this comment.
The response schema defines fields (age, commonInterests, distance) that are not returned by the Match interface or getMatches method implementation. The Match interface only includes id, firstName, profilePicture, and chatId fields. Either remove these unused fields from the schema or update the implementation to return them.
| age: { type: 'integer' }, | |
| commonInterests: { type: 'integer' }, | |
| distance: { type: 'number' }, |
| // matches: { type: 'array', items: { | ||
| // type: 'object', | ||
| // properties: { | ||
| // id: { type: 'integer' }, | ||
| // firstName: { type: 'string' }, | ||
| // profilePicture: { type: 'string', nullable: true }, | ||
| // age: { type: 'integer' }, | ||
| // commonInterests: { type: 'integer' }, | ||
| // distance: { type: 'number' }, | ||
| // chatId: { type: 'integer', nullable: true } | ||
| // } | ||
| // }} | ||
|
|
There was a problem hiding this comment.
There is a commented-out code block showing the expected schema structure, but it doesn't match the actual implementation. The actual Match interface only returns id, firstName, profilePicture, and chatId, but this comment suggests age, commonInterests, and distance should also be returned. Either remove this misleading comment or update the implementation to match it.
| // matches: { type: 'array', items: { | |
| // type: 'object', | |
| // properties: { | |
| // id: { type: 'integer' }, | |
| // firstName: { type: 'string' }, | |
| // profilePicture: { type: 'string', nullable: true }, | |
| // age: { type: 'integer' }, | |
| // commonInterests: { type: 'integer' }, | |
| // distance: { type: 'number' }, | |
| // chatId: { type: 'integer', nullable: true } | |
| // } | |
| // }} |
| 'Cookie': `jwt=${tokenA}` | ||
| } | ||
| }); | ||
| console.log('get matches response:', res.body); |
There was a problem hiding this comment.
Console.log statement left in test code. This debug statement should be removed before merging to production.
| console.log('get matches response:', res.body); |
| headers: { | ||
| 'Cookie': `jwt=${token}` | ||
| } | ||
| } |
There was a problem hiding this comment.
Trailing whitespace added at the end of the line. This should be removed for code cleanliness.
| } | |
| } |
| if (!user) | ||
| throw new NotFoundError(); | ||
| if (user.profilePictures.length <= 1) | ||
| if (user.profilePictures.length < 1) |
There was a problem hiding this comment.
The validation logic check has been changed from <= 1 to < 1, which means users can now delete their last profile picture when they have exactly 1 picture. This creates an inconsistency - the error message says "Cannot delete the last profile picture" but the condition now allows deletion when there's only 1 picture left. The original <= 1 condition was correct to prevent deletion of the last picture.
| if (user.profilePictures.length < 1) | |
| if (user.profilePictures.length <= 1) |
| gender: userData.gender, | ||
| orientation: userData.orientation, | ||
| bornAt: new Date(userData.bornAt) | ||
| } |
There was a problem hiding this comment.
Avoid automated semicolon insertion (95% of all statements in the enclosing function have an explicit semicolon).
GET /private/match return
[ { id: user id, firstName: user first name, profilePicture, user picture, chatId: chat id beetween both users } ]