Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions internal/handler/converter.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package handler

import (
"strings"
"time"

api "github.com/fun-dotto/academic-api/generated"
Expand Down Expand Up @@ -273,7 +274,9 @@ func timetableSlotToAPI(slot domain.TimetableSlot) api.DottoFoundationV1Timetabl

func timetableItemToAPI(d domain.TimetableItem) api.TimetableItem {
var slot *api.DottoFoundationV1TimetableSlot
if d.Slot != nil {
if d.Slot != nil &&
strings.TrimSpace(string(d.Slot.DayOfWeek)) != "" &&
strings.TrimSpace(string(d.Slot.Period)) != "" {
s := timetableSlotToAPI(*d.Slot)
slot = &s
Comment on lines +277 to 281
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

PR説明では「曜日と時限が空の場合(両方空)」と読めますが、ここは DayOfWeek または Period のどちらか一方でも空(/空白)なら Slot を nil にしています。意図が「両方空のときだけ nil」なら条件を調整してください。意図が「片方欠けている Slot は無効」なら、レスポンス側で黙って落とすのではなく、どこかの層でバリデーションしてエラーにする等の方がクライアントの不具合に気づきやすいです。

Copilot uses AI. Check for mistakes.
}
Comment on lines +277 to 282
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

空白チェックに TrimSpace を使っていますが、生成する Slot はトリム前の値(d.Slot.DayOfWeek / d.Slot.Period)をそのまま返しています。例えば " Monday " のように前後に空白がある場合、チェックは通る一方で空白付きの値がAPIに出てしまうので、ここでもトリム済みの値で組み立てるか、そもそもトリムせず厳密に比較するか、どちらかに揃えるのが安全です。

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -304,9 +307,13 @@ func toDomainTimetableItemFromRequest(req api.TimetableItemRequest) domain.Timet
Subject: domain.Subject{ID: req.SubjectId},
}
if req.Slot != nil {
item.Slot = &domain.TimetableSlot{
DayOfWeek: domain.DayOfWeek(req.Slot.DayOfWeek),
Period: domain.Period(req.Slot.Period),
dow := strings.TrimSpace(string(req.Slot.DayOfWeek))
per := strings.TrimSpace(string(req.Slot.Period))
if dow != "" && per != "" {
item.Slot = &domain.TimetableSlot{
DayOfWeek: domain.DayOfWeek(dow),
Period: domain.Period(per),
}
}
Comment on lines 309 to 317
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

ここも PR説明(曜日と時限が空の場合)より広い条件で、DayOfWeek または Period のどちらかが空(/空白)だと Slot を丸ごと無視して作成します。部分的な入力が来たときに黙って値が落ちるため、API仕様として「片方だけは不正」としたいなら 400 を返す等の明示的な扱いを検討してください(少なくとも意図をコメントやテストで固定すると安全です)。

Copilot uses AI. Check for mistakes.
}
rooms := make([]domain.Room, len(req.RoomIds))
Expand Down
161 changes: 161 additions & 0 deletions internal/handler/converter_timetable_item_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package handler

import (
"testing"

api "github.com/fun-dotto/academic-api/generated"
"github.com/fun-dotto/academic-api/internal/domain"
)

func TestTimetableItemToAPI_Slot(t *testing.T) {
baseSubject := domain.Subject{ID: "subject-1", Name: "Test"}

tests := []struct {
name string
slot *domain.TimetableSlot
wantNil bool
wantDow api.DottoFoundationV1DayOfWeek
wantPer api.DottoFoundationV1Period
}{
{
name: "slot is nil",
slot: nil,
wantNil: true,
},
{
name: "both fields empty string",
slot: &domain.TimetableSlot{
DayOfWeek: "",
Period: "",
},
wantNil: true,
},
{
name: "dayOfWeek empty",
slot: &domain.TimetableSlot{
DayOfWeek: "",
Period: domain.PeriodPeriod1,
},
wantNil: true,
},
{
name: "period empty",
slot: &domain.TimetableSlot{
DayOfWeek: domain.DayOfWeekMonday,
Period: "",
},
wantNil: true,
},
{
name: "whitespace only",
slot: &domain.TimetableSlot{
DayOfWeek: " \t",
Period: " ",
},
wantNil: true,
},
{
name: "valid slot",
slot: &domain.TimetableSlot{
DayOfWeek: domain.DayOfWeekMonday,
Period: domain.PeriodPeriod1,
},
wantNil: false,
wantDow: api.Monday,
wantPer: api.Period1,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := timetableItemToAPI(domain.TimetableItem{
ID: "item-1",
Subject: baseSubject,
Slot: tt.slot,
Rooms: nil,
})

if tt.wantNil {
if got.Slot != nil {
t.Fatalf("Slot: got non-nil %+v, want nil", got.Slot)
}
return
}
if got.Slot == nil {
t.Fatal("Slot: got nil, want non-nil")
}
if got.Slot.DayOfWeek != tt.wantDow {
t.Errorf("DayOfWeek: got %q, want %q", got.Slot.DayOfWeek, tt.wantDow)
}
if got.Slot.Period != tt.wantPer {
t.Errorf("Period: got %q, want %q", got.Slot.Period, tt.wantPer)
}
})
}
}

func TestToDomainTimetableItemFromRequest_Slot(t *testing.T) {
subjectID := "subject-1"

tests := []struct {
name string
slot *api.DottoFoundationV1TimetableSlot
wantNil bool
}{
{
name: "request slot is nil",
slot: nil,
wantNil: true,
},
{
name: "both empty",
slot: &api.DottoFoundationV1TimetableSlot{
DayOfWeek: "",
Period: "",
},
wantNil: true,
},
{
name: "whitespace only",
slot: &api.DottoFoundationV1TimetableSlot{
DayOfWeek: " ",
Period: "\t",
},
wantNil: true,
},
{
name: "valid",
slot: &api.DottoFoundationV1TimetableSlot{
DayOfWeek: api.Tuesday,
Period: api.Period2,
},
wantNil: false,
},
}
Comment on lines +110 to +134
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

toDomainTimetableItemFromRequest 側は「DayOfWeekだけ空」「Periodだけ空」の分岐が入っていますが、このテストではその2ケースがカバーされていません(両方空/空白のみ/validのみ)。この挙動を仕様として固定するために、片方だけ空のケースも追加しておくと回帰を防げます。

Copilot uses AI. Check for mistakes.

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := toDomainTimetableItemFromRequest(api.TimetableItemRequest{
SubjectId: subjectID,
Slot: tt.slot,
RoomIds: []string{},
})

if tt.wantNil {
if got.Slot != nil {
t.Fatalf("Slot: got %+v, want nil", got.Slot)
}
return
}
if got.Slot == nil {
t.Fatal("Slot: got nil, want non-nil")
}
if got.Slot.DayOfWeek != domain.DayOfWeekTuesday {
t.Errorf("DayOfWeek: got %q", got.Slot.DayOfWeek)
}
if got.Slot.Period != domain.PeriodPeriod2 {
t.Errorf("Period: got %q", got.Slot.Period)
}
})
}
}
Loading