仮デプロイ - #51
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 882affe86f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| occurred_at: debtEntry.occurredAt, | ||
| deleted_at: debtEntry.deletedAt?.toLocaleDateString('ja-JP', { timeZone: 'Asia/Tokyo' }) || null, | ||
| deleted_by_id: debtEntry.deletedBy, |
There was a problem hiding this comment.
Return deleted_at in the schema’s YYYY-MM-DD format
The new deleted_at field is formatted with toLocaleDateString('ja-JP'), which yields strings like YYYY/MM/DD. However, the API contract in products/validator/src/response/group.ts requires deleted_at to match ^\d{4}-\d{2}-\d{2}$ (hyphen-separated). Any client that validates or parses dates using the documented format will reject these responses or mis-handle them. Consider formatting deleted_at to match the same YYYY-MM-DD pattern as occurred_at, or relax the schema if slashes are intended.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR implements soft delete cancellation functionality for group debts, allowing users to undo debt deletions. It also refactors the warning message into a reusable component and improves toast notification messages.
Changes:
- Added soft delete cancellation endpoint (
/api/group/debt/cancel) and corresponding frontend UI - Modified debt history to display both active and deleted debts with toggle functionality
- Refactored warning message into a reusable
WarningMessagecomponent - Improved user-facing messages in Profile and other components for better clarity
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| products/validator/src/response/group.ts | Added deleted_at, deleted_by_id, deleted_by_name fields to debt history response schema |
| products/validator/src/request/group.ts | Added cancelGroupDebtRequestSchema for the cancel endpoint |
| products/validator/src/index.ts | Exported new cancel request schema |
| products/frontend/src/share/WarningMessage/index.tsx | New reusable component for displaying development warning |
| products/frontend/src/share/WarningMessage/index.module.css | Styles for the warning message component |
| products/frontend/src/share/index.ts | Exported WarningMessage component |
| products/frontend/src/routes/Root/index.tsx | Replaced inline Error component with WarningMessage |
| products/frontend/src/routes/Login/index.tsx | Replaced inline Error component with WarningMessage |
| products/frontend/src/routes/Profile/index.tsx | Improved toast message specificity (prefixed with "プロフィール情報") |
| products/frontend/src/routes/GroupDetail/index.tsx | Added cancel debt mutation and handler logic |
| products/frontend/src/routes/GroupDetail/components/History/index.tsx | Added toggle for showing completed debts and UI for deleted debt entries |
| products/frontend/src/routes/GroupDetail/components/History/index.module.css | Added styles for completed debt display and toggle button |
| products/frontend/src/routes/GroupDetail/components/Member/index.module.css | Added width: fit-content for better layout |
| products/frontend/src/index.css | Changed disabled color from dark gray to lighter gray for better visibility |
| products/frontend/src/api/openapi.d.ts | Added type definitions for new fields and cancel endpoint |
| products/frontend/openapi.json | Added OpenAPI schema for new fields and cancel endpoint |
| products/backend/src/presentation/routes/info.ts | Added UTC timezone comment for clarity |
| products/backend/src/presentation/routes/group.ts | Modified debt history query to return all debts (including deleted), added cancel endpoint implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| /*** 履歴一覧 (未完済) ***/ | ||
| /* これが基準のデザインj*/ |
There was a problem hiding this comment.
There's a typo in the Japanese comment. It should be "これが基準のデザイン" (this is the standard design) instead of "これが基準のデザインj" (with an extra "j" at the end).
| /* これが基準のデザインj*/ | |
| /* これが基準のデザイン */ |
| amount: debtEntry.amount, | ||
| description: debtEntry.description === null ? '' : debtEntry.description, | ||
| occurred_at: debtEntry.occurredAt, | ||
| deleted_at: debtEntry.deletedAt?.toLocaleDateString('ja-JP', { timeZone: 'Asia/Tokyo' }) || null, |
There was a problem hiding this comment.
There's an inconsistency in date formatting. The occurred_at field is passed through as-is from the database (line 453), while deleted_at is formatted using toLocaleDateString with Japanese locale (line 454). Both fields should use the same format pattern (YYYY-MM-DD as defined in the schema regex /^\d{4}-\d{2}-\d{2}$/). The toLocaleDateString method may produce a date string in the format "YYYY/MM/DD" which doesn't match the expected format.
| debtData.push({ | ||
| debt_id: debtEntry.id, | ||
| debtor_id: debtEntry.debtorId, | ||
| debtor_name: | ||
| DebtorNameInfo[0].displayName !== null && DebtorNameInfo[0].displayName.length > 0 | ||
| ? DebtorNameInfo[0].displayName | ||
| : DebtorNameInfo[0].name, | ||
| debtorNameInfo[0].displayName !== null && debtorNameInfo[0].displayName.length > 0 | ||
| ? debtorNameInfo[0].displayName | ||
| : debtorNameInfo[0].name, | ||
| creditor_id: debtEntry.creditorId, | ||
| creditor_name: | ||
| CreditorNameInfo[0].displayName !== null && CreditorNameInfo[0].displayName.length > 0 | ||
| ? CreditorNameInfo[0].displayName | ||
| : CreditorNameInfo[0].name, | ||
| creditorNameInfo[0].displayName !== null && creditorNameInfo[0].displayName.length > 0 | ||
| ? creditorNameInfo[0].displayName | ||
| : creditorNameInfo[0].name, | ||
| amount: debtEntry.amount, | ||
| description: debtEntry.description === null ? '' : debtEntry.description, | ||
| occurred_at: debtEntry.occurredAt, | ||
| deleted_at: debtEntry.deletedAt?.toLocaleDateString('ja-JP', { timeZone: 'Asia/Tokyo' }) || null, | ||
| deleted_by_id: debtEntry.deletedBy, | ||
| deleted_by_name: | ||
| debtEntry.deletedBy && deletedByNameInfo | ||
| ? deletedByNameInfo[0].displayName !== null && deletedByNameInfo[0].displayName.length > 0 | ||
| ? deletedByNameInfo[0].displayName | ||
| : deletedByNameInfo[0].name | ||
| : null, | ||
| }); |
There was a problem hiding this comment.
The OpenAPI specification defines an isDeleted boolean field as required in the response schema, but the backend code doesn't return this field. The backend only returns deleted_at, deleted_by_id, and deleted_by_name. Either add the isDeleted field to the response (e.g., isDeleted: debtEntry.deletedAt !== null) or remove it from the OpenAPI schema.
| {props.debtHistoryResult.debts.map((debt, index) => | ||
| debt.deleted_at === null ? ( | ||
| <li className={styles.li} key={index}> | ||
| <div className={styles.info}> | ||
| {/* section 1 */} | ||
| <p className={styles.summary}> | ||
| {debt.debtor_name} さんが {debt.creditor_name} さんに {debt.amount} 円を借りています。 | ||
| </p> | ||
| {/* section 2 */} | ||
| <p> | ||
| <span className={styles.label}>[発生日] </span> | ||
| <span> {debt.occurred_at}</span> | ||
| </p> | ||
| {/* section 3 */} | ||
| {!detail.has(debt.debt_id) && ( | ||
| <button className={styles.button} type="button" onClick={() => detailExpandHandler(debt.debt_id)}> | ||
| さらに表示 | ||
| </button> | ||
| )} | ||
| {detail.has(debt.debt_id) && ( | ||
| <> | ||
| <p className={styles.detail}>{debt.description || '詳細情報は、未記入のようです。'}</p> | ||
| <button | ||
| className={styles.button} | ||
| type="button" | ||
| onClick={() => detailShrinkHandler(debt.debt_id)} | ||
| > | ||
| 閉じる | ||
| </button> | ||
| </> | ||
| )} | ||
| </div> | ||
| <FullPaymentButton | ||
| onClick={() => props.deleteGroupDebtHandler(debt.debt_id)} | ||
| disabled={props.fullPaymentButtonDisabled} | ||
| /> | ||
| </li> | ||
| ) : ( | ||
| showCompleted && ( | ||
| <li className={styles.completedLi} key={index}> |
There was a problem hiding this comment.
Using array index as the key prop in React is not recommended, especially when items can be reordered, filtered, or removed. Since each debt entry has a unique debt_id, use that instead. Change key={index} to key={debt.debt_id} on both list items.
No description provided.