Feat/walt/purchase report sort frontend#1044
Conversation
…it into the purchase reports page
… the purchase reports page
…it into the purchase reports page
… the purchase reports page
Summary of ChangesHello @nakatashingo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、購入報告一覧ページのユーザビリティを大幅に向上させるための機能追加です。具体的には、立替者（局名および氏名）による絞り込み機能と、未清算金額および未封詰め金額の合計表示機能が導入されました。これにより、ユーザーはより効率的に必要な情報を探し、全体の財務状況を把握できるようになります。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying finansu with
|
| Latest commit: |
a57da05
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f1be018e.finansu.pages.dev |
| Branch Preview URL: | https://feat-walt-purchase-report-so.finansu.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
…l and PurchaseReportSummaryAmounts components
…haseReports component
TkymHrt
left a comment
TkymHrt
left a comment
There was a problem hiding this comment.
レビューしました！ コード全体はきれいにまとまっていて良さそうです。
1点、「立替者」の絞り込みについて追加で実装をお願いしたいです！
すでにシードデータで試したときに気づいているかもだけれど、現状だと一覧は paid_by （文字列）なのに、絞り込みは paid_by_user_id 基準だから、IDがないデータが検索に引っかからないんだよねー
API側では、paid_by での完全一致フィルタが使えるように対応済みだから、フロント側でも利用できるように調整してもらいたいです！
…support paidByUserId filtering
TkymHrt
left a comment
TkymHrt
left a comment
There was a problem hiding this comment.
封詰め・清算のステータス変更とかレコード削除の後も未清算金額・未封詰め金額のサマリが古い値のままかな。（一通り動作テストして、テスト項目書いといてくれると助かる！）
FinanSu/view/next-project/src/pages/purchase_report_list/index.tsx
Lines 207 to 209 in 566c3a9
たぶんこんな感じ
const {
data: buyReportsSummaryData,
isLoading: isBuyReportsSummaryLoading,
error: buyReportsSummaryError,
mutate: mutateBuyReportsSummary,
} = useGetBuyReportsSummary(...);
const onSuccess = useCallback(() => {
mutateBuyReportData();
mutateBuyReportsSummary();
}, [mutateBuyReportData, mutateBuyReportsSummary]);
…n in PurchaseReportPaidByFilterModal
…in useEffect for PurchaseReports component
…type guard useGetUsers() already returns User[] in generated types, making the complex unknown-cast filtering unreachable. Remove the isUser guard and replace the useMemo with a direct usersResponse?.data ?? [] access. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…x catch binding
- Remove selectedBureauId spread from API params: BUREAUS.id != financial_records.id
- Add NOTE comment explaining the mismatch until proper wiring is implemented
- Change catch {} to catch (e) {} to preserve thrown exception in error log
- Simplify PaidByFilterInput.paidBy type from string|null|undefined to string|null
- Change selectedPaidBy state initial value from undefined to null for consistency
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ll state onApply.paidBy and selectedPaidBy were typed as string|null|undefined while the parent state and normalizePaidBy return type are string|null, causing a TS2345 type error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…y ids - Add useGetFinancialRecords to map BUREAUS.id → financial_records.id by name, then pass the resolved financial_record_id to both buy-reports detail and summary API params so bureau filter actually narrows the results - Move useGetUsers after buyReports, pass ids of paidByUserId values found in the current reports instead of fetching all users unconditionally Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove unintended file_metas rows (id 4-7) added to the seed file on this branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…chaseReportSummaryAmounts for consistency
| const bureauToFinancialRecordId = useMemo(() => { | ||
| const records = financialRecordsData?.data?.financialRecords ?? []; | ||
| return new Map( | ||
| BUREAUS.flatMap((bureau) => { | ||
| const record = records.find((r) => r.name === bureau.name); | ||
| return record?.id != null ? ([[bureau.id, record.id]] as const) : []; | ||
| }), | ||
| ); | ||
| }, [financialRecordsData]); | ||
|
|
||
| const financialRecordId = | ||
| selectedBureauId != null ? (bureauToFinancialRecordId.get(selectedBureauId) ?? null) : null; | ||
|
|
||
| const hasSelectedBureauWithoutFinancialRecord = | ||
| selectedBureauId != null && financialRecordId == null; | ||
|
|
||
| const getBuyReportsDetailsParams: GetBuyReportsDetailsParams = { | ||
| year: selectedYear, | ||
| ...(financialRecordId != null | ||
| ? { financial_record_id: financialRecordId } | ||
| : hasSelectedBureauWithoutFinancialRecord | ||
| ? { financial_record_id: -1 } | ||
| : {}), | ||
| ...paidByFilterParams, |
| const nameOptions = useMemo( | ||
| (): NameOption[] => [ | ||
| { value: '', label: '絞り込みなし' }, | ||
| ...legacyPaidByOptions.map((name) => ({ value: `legacy:${name}`, label: name })), | ||
| ...filteredUsers.map((u) => { | ||
| const bureauName = bureauNameMap.get(u.bureauID); | ||
| const label = draftBureauId || !bureauName ? u.name : `${bureauName} ${u.name}`; | ||
| return { value: `user:${u.id}`, label }; | ||
| }), | ||
| ], | ||
| [filteredUsers, legacyPaidByOptions, bureauNameMap, draftBureauId], | ||
| ); | ||
|
|
||
| const nameSelectValue = | ||
| draftPaidByUserId != null | ||
| ? (nameOptions.find((o) => o.value === `user:${draftPaidByUserId}`) ?? nameOptions[0]) | ||
| : draftPaidBy != null | ||
| ? (nameOptions.find((o) => o.value === `legacy:${draftPaidBy}`) ?? nameOptions[0]) | ||
| : nameOptions[0]; |
TkymHrt
left a comment
TkymHrt
left a comment
There was a problem hiding this comment.
立替者の所属局と購入報告の申請局が混在してるけど、要件ってこれで良いんだっけ？
後方互換性まだ怪しそう。
現在のユーザー名と一致する paidBy を除外してるから、同名の過去データは選択肢にも出ないし、検索にも引っかからないんじゃないかな。バックエンド側で解消しないとか・・・
以下、Codexが見つけたやつ
-
ステータス更新後に余分な PUT が再発火し得ます。
view/next-project/src/pages/purchase_report_list/index.tsx:268のupdateStatusはuseEffectから呼ばれ、成功後にview/next-project/src/pages/purchase_report_list/index.tsx:278で一覧をmutateします。その再取得結果をview/next-project/src/pages/purchase_report_list/index.tsx:196のeffectがまたsealChecks/settlementChecksに流し込み、updateStatusの依存が変わって同じbuyReportIdで再度 PUT されます。更新処理はcheckbox/confirmのイベントハンドラ内で完結させるか、成功後にbuyReportIdをリセットしてください。 -
年度選択が再取得で最新年度に戻る可能性があります。
view/next-project/src/pages/purchase_report_list/index.tsx:53のeffectはyearPeriodsが変わるたびにselectedYearを最新年度へ上書きします。SWR の再検証やフォーカス復帰で、ユーザーが選んだ過去年が勝手に戻り得ます。初期化はprev === 0のときだけに限定してください。 -
差分ファイルに lint warning が残っています。
view/next-project/src/pages/purchase_report_list/index.tsx:125のlegacyPaidByOptionsは未使用です。加えてusersResponse?.data ?? [] / modalUsersResponse?.data ?? []の空配列生成が hook 依存警告を出しています。
TkymHrt
left a comment
TkymHrt
left a comment
There was a problem hiding this comment.
FinanSu/api/externals/repository/buy_report_repository.go
Lines 325 to 327 in a57da05
ここを下記みたいに変えればフロントのリファクタ無しで互換性問題は解決できそう。
ds = ds.Where(
goqu.Or(
goqu.I("buy_reports.paid_by_user_id").Eq(paidByUserID),
goqu.I("buy_reports.paid_by").Eq(
dialect.From("users").
Select(goqu.I("users.name")).
Where(goqu.I("users.id").Eq(paidByUserID)),
),
),
)
対応Issue
https://www.notion.so/nutfes-nutmeg/FinanSu-2b941f19206380518f1af6de573da465
https://nut-m-e-g.slack.com/archives/C020WQ3GY07/p1765010563203229
概要
画面スクリーンショット等
URLスクリーンショット
テスト項目
備考