Skip to content

Conversation

@hikahana
Copy link
Collaborator

@hikahana hikahana commented May 5, 2025

対応Issue

resolve #0

概要

一覧と単体のgetは修正完了。
post,put,deleteは特に変更を加えていません

画面スクリーンショット等

  • URL
    スクリーンショット

テスト項目

備考

@github-actions github-actions bot added the bug Something isn't working label May 5, 2025
@hikahana hikahana requested review from harata-t and izuizu0424 May 6, 2025 11:06
@Kubosaka
Copy link
Collaborator

gm3組がレビュワーになってるw

Base automatically changed from fix/hikahana/er to develop May 11, 2025 21:05
@hikahana hikahana requested review from Kubosaka and Copilot and removed request for harata-t and izuizu0424 May 11, 2025 21:05
@hikahana hikahana marked this pull request as ready for review May 17, 2025 06:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Limit teacher API responses to include room data and update related schemas, queries, and docs

  • Swap teacher schema for teacherWithRoom in OpenAPI, use JSON request body for POST
  • Remove room column, adjust SQL, update use-case and repository to join room_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 roomName and omit isDeleted. 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/json is 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.
Copy link

Copilot AI May 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
query, _, err := selectTeacherWithRoomQuery.
query, _, err := selectTeacherWithRoomQuery.
Where(goqu.Ex{"teachers.is_deleted": false}).

Copilot uses AI. Check for mistakes.
Comment on lines +49 to +50
&teacher.DepartmentId,
&teacher.Id,
Copy link

Copilot AI May 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
&teacher.DepartmentId,
&teacher.Id,
&teacher.Id,
&teacher.DepartmentId,

Copilot uses AI. Check for mistakes.
query += "id = " + strconv.Itoa(id)

if(index != len(ids)-1){
if index != len(ids)-1 {
Copy link

Copilot AI May 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants