-
Notifications
You must be signed in to change notification settings - Fork 2
収支管理csvダウンロード機能追加 #1022
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
収支管理csvダウンロード機能追加 #1022
Conversation
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
収支管理ページに CSV ダウンロード機能を追加するプルリクだよ!バックエンドで CSV 出力の API エンドポイント実装して、フロントエンドにダウンロードボタン追加してるね✨
- バックエンドに
/fund_informations/csv/downloadエンドポイント追加 - CSV は Shift_JIS エンコーディングで出力(Excel で開きやすいね!)
- フロントエンドに CSV ダウンロードボタンを実装
Reviewed changes
Copilot reviewed 4 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openapi/openapi.yaml | CSV ダウンロード用の新しい API エンドポイント定義を追加 |
| api/generated/openapi_gen.go | OpenAPI 定義から自動生成されたコード(パラメータ型とインターフェース) |
| api/router/router.go | CSV ダウンロードエンドポイントをルーティングに登録 |
| api/externals/controller/income_expenditure_management_controller.go | CSV ダウンロード機能の実装(データ取得、CSV 生成、Shift_JIS 変換) |
| view/next-project/src/generated/model/index.ts | TypeScript 型定義のエクスポート追加( Found と Fund のタイポあり) |
| view/next-project/src/generated/model/getFundInformationsCsvDownloadParams.ts | CSV ダウンロード API 用のパラメータ型定義(自動生成) |
| view/next-project/src/generated/model/getFoundInformationsCsvDownloadParams.ts | タイポのあるファイル(Fund が Found になってる) |
| view/next-project/src/generated/hooks.ts | CSV ダウンロード API を呼び出すための React フック追加 |
| view/next-project/src/components/fund_information/FundInformationHeader.tsx | CSV ダウンロードボタンを UI に追加 |
| <AddButton onClick={handleCreateClick}>収入報告</AddButton> | ||
| <div className='flex w-full flex-col gap-2 md:w-auto md:flex-row md:items-center'> | ||
| <OutlinePrimaryButton className='w-full md:w-fit'> | ||
| <a href={`${process.env.CSR_API_URI}/fund_informations/csv/download`} download> |
Copilot
AI
Nov 23, 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.
year パラメータが URL に含まれてないっぽいね💦 年度フィルタリング機能あるのに、ダウンロード時に year が渡されないから、意図した年度のデータが取れないかも😱 router.query とか使って year パラメータを URL に含めるようにした方がいいと思うよ!例えば ${process.env.CSR_API_URI}/fund_informations/csv/download?year=${router.query.year} みたいな感じで✨
| <a href={`${process.env.CSR_API_URI}/fund_informations/csv/download`} download> | |
| <a | |
| href={ | |
| router.query.year | |
| ? `${process.env.CSR_API_URI}/fund_informations/csv/download?year=${router.query.year}` | |
| : `${process.env.CSR_API_URI}/fund_informations/csv/download` | |
| } | |
| download | |
| > |
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.
[must] これ対応してください
リンクにyearのクエリパラメータが入っていないので
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.
これどうやって年度取得したらいいかよくわからなかったので教えてほしいです!
| } | ||
|
|
||
| w := c.Response().Writer | ||
| fileName := fmt.Sprintf("fund_informations_%s.csv", year) |
Copilot
AI
Nov 23, 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.
年度パラメータが空文字列の時のファイル名がちょっと変になっちゃうかも💦 fund_informations_.csv みたいになっちゃうから、year が空の場合は "all" とか現在の年度をデフォルト値として使った方がいいんじゃない?🤔 例えば year が空なら "all" とか設定するとか!
Kubosaka
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.
少しコメント!
| <AddButton onClick={handleCreateClick}>収入報告</AddButton> | ||
| <div className='flex w-full flex-col gap-2 md:w-auto md:flex-row md:items-center'> | ||
| <OutlinePrimaryButton className='w-full md:w-fit'> | ||
| <a href={`${process.env.CSR_API_URI}/fund_informations/csv/download`} download> |
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.
[must] これ対応してください
リンクにyearのクエリパラメータが入っていないので
openapi/openapi.yaml
Outdated
| schema: | ||
| type: string | ||
| format: binary | ||
| /fund_informations/csv/download: |
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.
[imo] endpointをincome_expenditure_managementにしてほしいです、収支管理はAPIはこっちに揃えているので
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.
修正しました!
|
収支管理では年度を指定する機能がないのでいったん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.
Pull request overview
Copilot reviewed 4 out of 10 changed files in this pull request and generated 3 comments.
| http.Error(writer, "CSVの書き込み中にエラーが発生しました", http.StatusInternalServerError) | ||
| return err | ||
| } | ||
| } | ||
| csvWriter.Flush() | ||
| if err := csvWriter.Error(); err != nil { | ||
| http.Error(writer, "CSVのフラッシュ中にエラーが発生しました", http.StatusInternalServerError) |
Copilot
AI
Nov 30, 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.
エラーメッセージがちょい微妙かも!🤔 "CSVの書き込み中にエラーが発生しました"って出てるけど、http.Errorを呼んだ後にreturn errしてるから、エラーメッセージが二重に出力されちゃう可能性あるよ!
http.Error呼んだらすぐreturnするか、エラーハンドリングの方法を統一した方がいいかもね💡
if err := csvWriter.Write(record); err != nil {
return err
}こんな感じでシンプルにして、エラーハンドリングはEchoのミドルウェアに任せる方がぱねぇかも✨
| http.Error(writer, "CSVの書き込み中にエラーが発生しました", http.StatusInternalServerError) | |
| return err | |
| } | |
| } | |
| csvWriter.Flush() | |
| if err := csvWriter.Error(); err != nil { | |
| http.Error(writer, "CSVのフラッシュ中にエラーが発生しました", http.StatusInternalServerError) | |
| return err | |
| } | |
| } | |
| csvWriter.Flush() | |
| if err := csvWriter.Error(); err != nil { |
| csvWriter.Flush() | ||
| if err := csvWriter.Error(); err != nil { | ||
| http.Error(writer, "CSVのフラッシュ中にエラーが発生しました", http.StatusInternalServerError) | ||
| return err |
Copilot
AI
Nov 30, 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.
こっちも同じパターン!"CSVのフラッシュ中にエラーが発生しました"って出した後にreturn errしてるから、エラーが二重に処理されちゃうかも🥺
line 113-115と同じで、エラーハンドリングを統一した方がしっかりしてるよ〜💪
| return err | |
| return nil |
| <div className='flex w-full flex-col gap-2 md:w-auto md:flex-row md:items-center'> | ||
| <OutlinePrimaryButton className='w-full md:w-fit'> | ||
| <a | ||
| href={`${process.env.CSR_API_URI}/income_expenditure_management/csv/download?year=2025`} |
Copilot
AI
Nov 30, 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.
年度の値がハードコードされてるけど、これやばたにえん🥺 2025年決め打ちになってるから、年度が変わったら動かなくなっちゃうよ!
useFundInformations.tsにあるuseCurrentYearId()みたいに、現在の年度を動的に取得する実装がおすすめだよ〜✨
const { data: yearData } = useGetYears();
const currentYear = yearData?.data?.[0]?.year;こんな感じで取得して、URLに埋め込むようにしよ!💪
|
締め切りも近いのでこれで一旦マージします。yearの渡し方は年度指定のやり方等が定まってから修正しましょう。 |
対応Issue
概要
画面スクリーンショット等
テスト項目
備考