50 suggested profiles#51
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a user browsing feature with filtering and sorting capabilities, allowing users to discover compatible profiles based on various criteria including location, age, tags, and fame rating.
- Adds a new
BrowsingServicewith configurable filters (age, location, fame rate, tags) and sorting options (distance, age, fame rate, tags, or a weighted default algorithm) - Implements HTTP endpoints for browsing users with URL-based parameter passing
- Updates the quickUser test fixture to use 'bisexual' orientation for better test coverage with the new browsing logic
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 52 comments.
Show a summary per file
| File | Description |
|---|---|
fastify/assets/test/integration/ws/browsing.test.ts |
New integration tests for basic browsing functionality and match scoring |
fastify/assets/test/integration/ws/browsing.filtersort.test.ts |
Comprehensive tests for filtering and sorting capabilities |
fastify/assets/test/integration/utils/browsing.ts |
Test utilities for setting user profile attributes and calculating age differences |
fastify/assets/test/integration/fixtures/auth.fixtures.ts |
Changed default orientation from 'heterosexual' to 'bisexual' |
fastify/assets/srcs/services/BrowsingService.ts |
Core service implementing browsing logic with SQL queries and sorting algorithms |
fastify/assets/srcs/routes/private/user/me/profile.ts |
Removed required constraint on location latitude/longitude fields |
fastify/assets/srcs/routes/private/index.ts |
Registered browsing routes under /browsing prefix |
fastify/assets/srcs/routes/private/browsing/index.ts |
Defined browsing endpoint with route schema validation |
fastify/assets/srcs/controllers/private/me/profile.ts |
Refactored profile update handler with explicit type checking |
fastify/assets/srcs/controllers/private/browsing/index.ts |
Handler for processing browsing requests and returning filtered/sorted users |
fastify/assets/srcs/app.ts |
Registered BrowsingService plugin |
fastify/assets/@types/fastify.d.ts |
Added type declarations for browsingService |
Comments suppressed due to low confidence (2)
fastify/assets/test/integration/ws/browsing.test.ts:10
- Unused imports getAgeDifference, likeUser, viewUser.
import { setTags, setLocalisation, setBirthDate, likeUser, viewUser, getAgeDifference } from '../utils/browsing';
fastify/assets/test/integration/ws/browsing.test.ts:14
- Unused import create.
import { create } from 'node:domain';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const rows = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { tags: ['music'] }); | ||
|
|
||
| for (let i = 0; i < rows.length - 1; i++) { |
There was a problem hiding this comment.
Off-by-one error in test: The loop condition i < rows.length - 1 means the last element in the array is never tested. This should be i < rows.length to ensure all users match the 'music' tag filter.
| for (let i = 0; i < rows.length - 1; i++) { | |
| for (let i = 0; i < rows.length; i++) { |
| for (let i = 0; i < rows.length - 1; i++) { | ||
| expect(rows[i].fameRate).to.be.at.least(300); | ||
| expect(rows[i].fameRate).to.be.at.most(800); | ||
| } | ||
|
|
||
| const rows1 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 600, max: 1000 } }); | ||
| for (let i = 0; i < rows1.length - 1; i++) { | ||
| expect(rows1[i].fameRate).to.be.at.least(600); | ||
| expect(rows1[i].fameRate).to.be.at.most(1000); | ||
| } | ||
|
|
||
| const rows2 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 0, max: 0 } }); | ||
| for (let i = 0; i < rows2.length - 1; i++) { |
There was a problem hiding this comment.
Off-by-one error in test: The loop condition i < rows1.length - 1 means the last element in the array is never tested for the fame rate filter. This should be i < rows1.length to ensure all users match the fame rate range.
| for (let i = 0; i < rows.length - 1; i++) { | |
| expect(rows[i].fameRate).to.be.at.least(300); | |
| expect(rows[i].fameRate).to.be.at.most(800); | |
| } | |
| const rows1 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 600, max: 1000 } }); | |
| for (let i = 0; i < rows1.length - 1; i++) { | |
| expect(rows1[i].fameRate).to.be.at.least(600); | |
| expect(rows1[i].fameRate).to.be.at.most(1000); | |
| } | |
| const rows2 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 0, max: 0 } }); | |
| for (let i = 0; i < rows2.length - 1; i++) { | |
| for (let i = 0; i < rows.length; i++) { | |
| expect(rows[i].fameRate).to.be.at.least(300); | |
| expect(rows[i].fameRate).to.be.at.most(800); | |
| } | |
| const rows1 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 600, max: 1000 } }); | |
| for (let i = 0; i < rows1.length; i++) { | |
| expect(rows1[i].fameRate).to.be.at.least(600); | |
| expect(rows1[i].fameRate).to.be.at.most(1000); | |
| } | |
| const rows2 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 0, max: 0 } }); | |
| for (let i = 0; i < rows2.length; i++) { |
| for (let i = 0; i < rows.length - 1; i++) { | ||
| expect(rows[i].fameRate).to.be.at.least(300); | ||
| expect(rows[i].fameRate).to.be.at.most(800); | ||
| } | ||
|
|
||
| const rows1 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 600, max: 1000 } }); | ||
| for (let i = 0; i < rows1.length - 1; i++) { | ||
| expect(rows1[i].fameRate).to.be.at.least(600); | ||
| expect(rows1[i].fameRate).to.be.at.most(1000); | ||
| } | ||
|
|
||
| const rows2 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 0, max: 0 } }); | ||
| for (let i = 0; i < rows2.length - 1; i++) { |
There was a problem hiding this comment.
Off-by-one error in test: The loop condition i < rows2.length - 1 means the last element in the array is never tested for the fame rate filter. This should be i < rows2.length to ensure all users match the fame rate range.
| for (let i = 0; i < rows.length - 1; i++) { | |
| expect(rows[i].fameRate).to.be.at.least(300); | |
| expect(rows[i].fameRate).to.be.at.most(800); | |
| } | |
| const rows1 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 600, max: 1000 } }); | |
| for (let i = 0; i < rows1.length - 1; i++) { | |
| expect(rows1[i].fameRate).to.be.at.least(600); | |
| expect(rows1[i].fameRate).to.be.at.most(1000); | |
| } | |
| const rows2 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 0, max: 0 } }); | |
| for (let i = 0; i < rows2.length - 1; i++) { | |
| for (let i = 0; i < rows.length; i++) { | |
| expect(rows[i].fameRate).to.be.at.least(300); | |
| expect(rows[i].fameRate).to.be.at.most(800); | |
| } | |
| const rows1 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 600, max: 1000 } }); | |
| for (let i = 0; i < rows1.length; i++) { | |
| expect(rows1[i].fameRate).to.be.at.least(600); | |
| expect(rows1[i].fameRate).to.be.at.most(1000); | |
| } | |
| const rows2 = await app.browsingService.browseUsers(data1.id as number, 10, 0, 50, { fameRate: { min: 0, max: 0 } }); | |
| for (let i = 0; i < rows2.length; i++) { |
| ${filters?.age ? `AND (CURRENT_DATE - u.born_at) / 365 | ||
| BETWEEN ${filters.age.min} AND ${filters.age.max} | ||
| ` : ''} | ||
| ${filters?.fameRate ? `AND u.fame_rate BETWEEN ${filters.fameRate.min} AND ${filters.fameRate.max}` : ''} |
There was a problem hiding this comment.
SQL injection vulnerability: Fame rate filter values are directly concatenated into the SQL query without parameterization. This creates a SQL injection risk. Use parameterized queries instead by adding these values to the parameters array.
| private fastify: FastifyInstance; | ||
| private userModel: UserModel; | ||
| private likeModel: LikeModel; | ||
| private viewModel: ViewModel |
There was a problem hiding this comment.
Missing semicolon: Line 44 is missing a semicolon at the end. While JavaScript allows this, it's inconsistent with the rest of the codebase which uses semicolons.
| private viewModel: ViewModel | |
| private viewModel: ViewModel; |
| this.timeout(5000); | ||
|
|
||
| const { userData: data1, token: token1 } = await quickUser(app); | ||
| const { userData: data2, token: token2 } = await quickUser(app); |
There was a problem hiding this comment.
Unused variable data2.
| const { userData: data2, token: token2 } = await quickUser(app); |
|
|
||
| const { userData: data1, token: token1 } = await quickUser(app); | ||
| const { userData: data2, token: token2 } = await quickUser(app); | ||
| const { userData: data3, token: token3 } = await quickUser(app); |
There was a problem hiding this comment.
Unused variable data3.
| const { userData: data3, token: token3 } = await quickUser(app); | |
| const { token: token3 } = await quickUser(app); |
| const { userData: data1, token: token1 } = await quickUser(app); | ||
| const { userData: data2, token: token2 } = await quickUser(app); | ||
| const { userData: data3, token: token3 } = await quickUser(app); | ||
| const { userData: data4, token: token4 } = await quickUser(app); |
There was a problem hiding this comment.
Unused variable data4.
| const { userData: data4, token: token4 } = await quickUser(app); | |
| const { token: token4 } = await quickUser(app); |
| const { userData: data2, token: token2 } = await quickUser(app); | ||
| const { userData: data3, token: token3 } = await quickUser(app); | ||
| const { userData: data4, token: token4 } = await quickUser(app); | ||
| const { userData: data5, token: token5 } = await quickUser(app); |
There was a problem hiding this comment.
Unused variable data5.
| const { userData: data5, token: token5 } = await quickUser(app); | |
| const { token: token5 } = await quickUser(app); |
| const totalScore = (ageScore * ageWeight) + (tagsScore * tagsWeight) + (fameRateScore * fameRateWeight) + (distanceScore * distanceWeight); | ||
| // console.log(`User ${user.id} - Age Score: ${ageScore.toFixed(2)}, Tags Score: ${tagsScore.toFixed(2)}, Fame Rate Score: ${fameRateScore.toFixed(2)}, Total Score: ${totalScore.toFixed(2)}, Distance Score: ${distanceScore.toFixed(2)}`); | ||
| scoreMap.set(user.id, totalScore); | ||
| }) |
There was a problem hiding this comment.
Avoid automated semicolon insertion (90% of all statements in the enclosing function have an explicit semicolon).
| }) | |
| }); |
No description provided.