-
Notifications
You must be signed in to change notification settings - Fork 2
#629 마일리지 기능 추가 #634
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
base: dev
Are you sure you want to change the base?
#629 마일리지 기능 추가 #634
Conversation
kmc7468
left a comment
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.
고생하셨습니다! 일단 1차 리뷰 남겨드립니다... ㅎㅎㅎ
Main branch update from Dev branch
Main branch update from Dev branch
Main branch update from Dev branch
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.
Pull request overview
This PR adds a mileage feature to reward users based on their taxi-sharing activity. The feature tracks transactions tied to room participation, calculates mileage based on forecasted taxi fares, and provides APIs for viewing summaries, transaction history, and leaderboards.
Key changes:
- Created mileage transaction system with pending/confirmed/voided statuses
- Integrated mileage calculation into room lifecycle (create, join, abort, settlement)
- Added scheduled task to fetch and cache taxi fare data from Naver Maps API
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/mongo.d.ts | Export Mileage type |
| src/services/rooms.ts | Integrate mileage transactions into room handlers |
| src/mileage/services/transaction.ts | Core transaction CRUD logic |
| src/mileage/services/summary.ts | User mileage summary with tier calculation |
| src/mileage/services/leaderboard.ts | Leaderboard aggregation with ranking |
| src/mileage/schedules/getTaxiFare.ts | Scheduled taxi fare fetching routine |
| src/mileage/routes/*.ts | Route definitions for mileage endpoints |
| src/mileage/routes/docs/*.js | Swagger documentation |
| src/mileage/modules/mongo.ts | Mileage schema definition |
| src/mileage/modules/forecastTaxiFare.ts | Taxi fare prediction logic |
| src/mileage/index.ts | Main mileage router |
| src/loadenv.ts | Naver Maps API configuration |
| src/index.ts | Mount mileage router |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { summaryHandler } from "../services/summary"; | ||
|
|
||
| const router = express.Router(); | ||
|
|
Copilot
AI
Nov 25, 2025
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.
Missing authentication middleware. The summary endpoint accesses user-specific data (req.userOid) but lacks authMiddleware protection. Add router.use(authMiddleware) to protect this route, following the pattern in other authenticated routes.
| import { summaryHandler } from "../services/summary"; | |
| const router = express.Router(); | |
| import { summaryHandler } from "../services/summary"; | |
| import { authMiddleware } from "../middleware/auth"; | |
| const router = express.Router(); | |
| router.use(authMiddleware); |
| import express from "express"; | ||
| import { leaderboardHandler } from "../services/leaderboard"; | ||
| import { mileageZod } from "./docs/schemas/mileageSchema"; | ||
| import { validateQuery } from "@/middlewares"; | ||
|
|
||
| const router = express.Router(); | ||
|
|
||
| router.get( | ||
| "/", | ||
| validateQuery(mileageZod.leaderboardHandler), | ||
| leaderboardHandler | ||
| ); |
Copilot
AI
Nov 25, 2025
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.
[nitpick] Missing input validation. The leaderboardHandler validates limit as a positive integer in the handler itself (line 14), but the validation should occur at the middleware level using validateQuery. However, if leaderboard is meant to be public, consider documenting this design decision.
| }); | ||
|
|
||
| const parameter = ordinaryLeastSquares( | ||
| changedRecord.filter((record) => record != 0) |
Copilot
AI
Nov 25, 2025
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.
Use strict equality (record !== 0) instead of loose equality (record != 0) for type-safe comparisons.
| changedRecord.filter((record) => record != 0) | |
| changedRecord.filter((record) => record !== 0) |
src/services/rooms.ts
Outdated
| await updateTransaction(updates); | ||
| } | ||
| } catch (err) { | ||
| // room 로직에 영향 없게 조용히 처리 |
Copilot
AI
Nov 25, 2025
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.
Silent error handling without logging. This catch block in joinHandler silently ignores errors when updating mileage transactions. Add logger.error(err) for debugging, consistent with the error handling at line 489 in abortHandler.
| // room 로직에 영향 없게 조용히 처리 | |
| // room 로직에 영향 없게 조용히 처리 | |
| logger.error(err); |
| example: "earn", | ||
| }, | ||
| amount: { type: "integer", example: 50 }, | ||
| createdAt: { |
Copilot
AI
Nov 25, 2025
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.
Documentation uses 'createdAt' but the actual implementation uses 'createAt'. Once the schema field is corrected to 'createdAt', this documentation will be accurate.
|
|
||
| export const mileageZod = { | ||
| transactionViewHandler: z.object({ | ||
| type: z.string(), |
Copilot
AI
Nov 25, 2025
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.
The 'type' field lacks validation constraints. According to the documentation (mileage.js:48), valid values are ["earn", "use", "event", "attendance"], but the schema accepts any string. Add .enum() validation or document that the field is optional for filtering.
| type: z.string(), | |
| type: z.enum(["earn", "use", "event", "attendance"]), |
…aist/taxi-back into 629-add-mileage-feature
Summary
It closes #629