Conversation
|
gm3組がレビュワーになってるw |
There was a problem hiding this comment.
Pull Request Overview
Limit teacher API responses to include room data and update related schemas, queries, and docs
- Swap
teacherschema forteacherWithRoomin OpenAPI, use JSON request body for POST - Remove
roomcolumn, adjust SQL, update use-case and repository to joinroom_teachers/rooms - Regenerate ER diagrams and HTML outputs to reflect zero rows for
sponsors
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| openapi/openapi.yaml | Switched to teacherWithRoom schema, updated POST body |
| mysql/db/33_teachers.sql | Dropped room column, adjusted INSERT columns |
| api/internals/usecase/teacher_usecase.go | Swapped domain.Teacher references to TeacherWithRoom, updated Scans |
| api/externals/repository/teacher_repository.go | Added selectTeacherWithRoomQuery, removed room select, refactored All/Find |
| ER files (HTML, DOT, XML) | Regenerated outputs, removed room rows, updated row counts |
Comments suppressed due to low confidence (3)
openapi/openapi.yaml:2414
- [nitpick] The PUT endpoint still uses query parameters for update payload, but POST uses a JSON
requestBody. For consistency and clearer API design, switch PUT to accept a similar JSON body.
put:
api/internals/usecase/teacher_usecase.go:17
- [nitpick] The return types now include
roomNameand omitisDeleted. Ensure you add or update unit tests to cover these new fields and validate the joined room data is correctly populated.
GetTeachers(context.Context) ([]TeacherWithRoom, error)
openapi/openapi.yaml:2455
-
Indentation under
application/jsonis off. It should be:application/json:
schema:
type: object
properties:
...
so that schema is nested correctly.
schema:
|
|
||
| func (t *teacherRepository) All(c context.Context) (*sql.Rows, error) { | ||
| query := "SELECT * FROM teachers WHERE is_deleted IS FALSE ORDER BY department_id ASC " | ||
| query, _, err := selectTeacherWithRoomQuery. |
There was a problem hiding this comment.
The All() query no longer filters out soft-deleted records (is_deleted). Add a Where(goqu.Ex{"teachers.is_deleted": false}) clause to the query builder so deleted teachers aren't returned.
| query, _, err := selectTeacherWithRoomQuery. | |
| query, _, err := selectTeacherWithRoomQuery. | |
| Where(goqu.Ex{"teachers.is_deleted": false}). |
| &teacher.DepartmentId, | ||
| &teacher.Id, |
There was a problem hiding this comment.
The Scan order doesn’t match the SELECT list. The first column is id, but you’re scanning it into DepartmentId. Reorder the arguments to Scan to match: e.g., &teacher.Id, &teacher.Name, &teacher.Position, &teacher.DepartmentId, etc.
| &teacher.DepartmentId, | |
| &teacher.Id, | |
| &teacher.Id, | |
| &teacher.DepartmentId, |
| query += "id = " + strconv.Itoa(id) | ||
|
|
||
| if(index != len(ids)-1){ | ||
| if index != len(ids)-1 { |
There was a problem hiding this comment.
Building the MultiDestroy query via string concatenation is error-prone and can be inefficient. Consider using a parameterized WHERE id IN (?) with Goqu for safety and performance.
対応Issue
resolve #0
概要
一覧と単体のgetは修正完了。
post,put,deleteは特に変更を加えていません
画面スクリーンショット等
URLスクリーンショット
テスト項目
備考