Conversation
SasakiTakatsugu
requested review from
KooriyamaHiroki,
bigdragon0610,
Copilot and
silent-bayline
There was a problem hiding this comment.
Pull Request Overview
This PR adds billing plan configuration functionality, introducing new API endpoints to retrieve pricing plans, tax rates, and manage tenant plan information.
- Added endpoints to fetch pricing plans and tax rates
- Added tenant plan management endpoints for retrieving and updating plan configurations
- Implemented proper authentication and authorization checks for billing operations
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/routes/billingRoutes.ts
Outdated
|
|
||
| const userInfo = req.userInfo; | ||
| if (!userInfo) { | ||
| return res.status(401).json({ error: "Internal server error" }); |
There was a problem hiding this comment.
The error message 'Internal server error' is misleading for a 401 status code. This should be an authentication error message like 'Unauthorized' or 'Authentication required'.
| return res.status(401).json({ error: "Internal server error" }); | |
| return res.status(401).json({ error: "Unauthorized" }); |
src/routes/billingRoutes.ts
Outdated
|
|
||
| const userInfo = req.userInfo; | ||
| if (!userInfo) { | ||
| return res.status(401).json({ error: "Internal server error" }); |
There was a problem hiding this comment.
The error message 'Internal server error' is misleading for a 401 status code. This should be an authentication error message like 'Unauthorized' or 'Authentication required'.
| return res.status(401).json({ error: "Internal server error" }); | |
| return res.status(401).json({ error: "Unauthorized" }); |
src/routes/billingRoutes.ts
Outdated
| } | ||
|
|
||
| // レスポンスを構築 | ||
| const response: any = { |
There was a problem hiding this comment.
Using 'any' type reduces type safety. Consider defining a proper interface for the response object to improve maintainability and catch potential type errors.
src/routes/billingRoutes.ts
Outdated
| const authCli = new AuthClient(); | ||
|
|
||
| // テナントプランを更新 | ||
| const updateTenantPlanParam: any = { |
There was a problem hiding this comment.
Using 'any' type reduces type safety. Consider defining a proper interface for the updateTenantPlanParam object to improve maintainability and catch potential type errors.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (!tenantId) { | ||
| return res.status(400).json({ error: "tenant_id is required" }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing input validation for required field 'next_plan_id'. The field should be validated before proceeding with the request body destructuring to ensure it exists and is a non-empty string.
| // Validate next_plan_id before destructuring | |
| if ( | |
| !req.body.next_plan_id || | |
| typeof req.body.next_plan_id !== "string" || | |
| req.body.next_plan_id.trim() === "" | |
| ) { | |
| return res.status(400).json({ error: "next_plan_id is required and must be a non-empty string" }); | |
| } |
|
|
||
| // テナントプランを更新 | ||
| const updateTenantPlanParam: UpdateTenantPlanParam = { | ||
| next_plan_id: next_plan_id, |
There was a problem hiding this comment.
[nitpick] Redundant property assignment. Use shorthand property notation 'next_plan_id' instead of 'next_plan_id: next_plan_id' for cleaner code.
| next_plan_id: next_plan_id, | |
| next_plan_id, |
| }; | ||
|
|
||
| // 税率IDが指定されている場合のみ設定 | ||
| if (tax_rate_id && tax_rate_id !== "") { |
There was a problem hiding this comment.
[nitpick] Inconsistent checking pattern. The condition 'tax_rate_id && tax_rate_id !== ""' can be simplified to just 'tax_rate_id' since the truthiness check already handles empty strings in JavaScript/TypeScript.
| if (tax_rate_id && tax_rate_id !== "") { | |
| if (tax_rate_id) { |
2 participants
No description provided.