Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a new API endpoint to retrieve campus donation information categorized by building and floor for a specific year. The changes span the entire stack, including OpenAPI specifications, frontend hooks, and backend layers (handler, usecase, repository), supported by new test fixtures and integration tests. Feedback focuses on optimizing the repository's SQL query for better performance on large datasets and addressing potential data integrity risks when joining building records by name.
| donationTotalsByTeacher := dialect. | ||
| Select( | ||
| goqu.I("campus_donations.teacher_id").As("teacher_id"), | ||
| goqu.SUM(goqu.I("campus_donations.price")).As("total_price"), | ||
| ). | ||
| From(goqu.T("campus_donations")). | ||
| InnerJoin( | ||
| goqu.T("years"), | ||
| goqu.On(goqu.I("campus_donations.year_id").Eq(goqu.I("years.id"))), | ||
| ). | ||
| Where(goqu.I("years.year").Eq(year)). | ||
| GroupBy(goqu.I("campus_donations.teacher_id")) |
There was a problem hiding this comment.
The subquery donationTotalsByTeacher filters by year but does not filter by building or floor. While this is logically correct because it's joined later on teacher_id, it might be more efficient to include building/floor filters in the subquery if the campus_donations table is very large, to reduce the number of rows processed in the aggregation.
| InnerJoin( | ||
| goqu.T("buildings").As("selected_building"), | ||
| goqu.On( | ||
| goqu.I("buildings.name").Eq(goqu.I("selected_building.name")), |
There was a problem hiding this comment.
Joining on buildings.name to group units assumes that building names are unique identifiers for a group of units. If there's any risk of different buildings having the same name, this could lead to incorrect data aggregation. Consider if a more robust grouping mechanism (like a parent_building_id) is available or needed in the future.
Deploying finansu with
|
| Latest commit: |
d15d5b0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b4d8e536.finansu.pages.dev |
| Branch Preview URL: | https://feat-hikahana-campus-donatio.finansu.pages.dev |
| goqu.I("rooms.room_name").As("room_name"), | ||
| goqu.I("teachers.id").As("teacher_id"), | ||
| goqu.I("teachers.name").As("teacher_name"), | ||
| goqu.COALESCE(goqu.I("donation_totals.total_price"), 0).As("total_price"), |
There was a problem hiding this comment.
仕様書の
### 回収済み/未回収の判定
- 金額が登録済み（または募金レコードが存在）なら回収済み
- 未登録なら未回収
みたいに回収済みの0円なのか未回収なのかわかるようにしたいからnullのままはどう？
それともisDonatedとか追加する？
| fixtures, err = testfixtures.New( | ||
| testfixtures.Database(db), // You database connection | ||
| testfixtures.Dialect("mysql"), // Available: "postgresql", "timescaledb", "mysql", "mariadb", "sqlite" and "sqlserver" | ||
| testfixtures.Directory("fixtures"), // The directory containing the YAML files | ||
| ) | ||
| if err != nil { | ||
| fmt.Printf("Error creating fixtures: %v\n", err) | ||
| return |
There was a problem hiding this comment.
今回の編集範囲じゃないけどテスト実行中に気づいたこと書いておきます
テストのためにmake run-testをするところ、間違えてローカルホスト側で go test ./... を実行した際に ok と表示された。しかし、go test -v ./test で確認すると fixture 作成に失敗していた。（finansu_test_db作って、マイグレーション当てたらmake run-testでちゃんと動いた）
fixture 作成時にエラーが起きても、ここで return して m.Run() が呼ばれないため、テスト未実行のまま go test が成功扱いになる問題を見つけた。
2 participants
対応Issue
resolve #0
概要
GET /campus_donations/years/{year}/group_keys/{group_key}/floors?floor_number={floor_number}に変更しましたPOST /campus_donationsで学内募金を登録できるようにしましたPUT /campus_donations/{id}で学内募金を更新できるようにしましたuserId,teacherId,yearId,price,receivedAtにしていますgroup_keyはOpenAPI schemaのenumとして定義し、Go/フロントの生成コードから型安全に利用できるようにしましたtotalPrice: 0で返すようにしていますmake genでGo/フロントの生成コードを更新しました画面スクリーンショット等
テスト項目
go test ./internals/usecase/...go test ./...git diff --checkmake genGET /campus_donations/years/{year}/group_keys/{group_key}/floors?floor_number={floor_number}を実行し、指定したgroup_keyの棟だけ返ることfloor_numberなしで実行し、指定年度・指定棟グループの全フロアが返ることPOST /campus_donationsを実行し、登録結果が返ることPUT /campus_donations/{id}を実行し、更新結果が返ることtotalPriceが0で返ること備考
登録/更新API request例
{ "userId": 1, "teacherId": 1, "yearId": 1, "price": 2000, "receivedAt": "2026-04-28" }動作確認用リクエスト例
group_key
通常の棟グループは棟名の英語スネークケースで管理します。その他は
otherで管理します。