Conversation
There was a problem hiding this comment.
Pull request overview
日付配列（dates）から年度（Year）と学期（Semesters）を推定し、時間割取得クエリのフィルタに反映することで、個人カレンダー生成時の取得範囲を適切に絞り込むPRです。
Changes:
datesからYear/Semestersを算出してTimetableItemListFilterに設定- 年度・学期推定のための
determineSemestersFromDatesヘルパーを追加
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _, date := range dates { | ||
| yearMap[date.Year()] = struct{}{} | ||
|
|
||
| month := date.Month() | ||
| if month >= 4 && month <= 9 { |
There was a problem hiding this comment.
yearMap[date.Year()] だと暦年ベースになり、1〜3月の date では日本の大学の「年度（4月始まり）」とズレます（例: 2026-01 は本来 2025 年度の H2 になりそう）。ここでは date.Month() < time.April の場合に year を 1 減らした「年度」を使って yearMap に入れるなど、年度計算を入れた方が subjects.year フィルタと整合します。
| var year *int | ||
| if len(yearMap) == 1 { | ||
| for y := range yearMap { | ||
| year = &y | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
len(yearMap) != 1 のときに year=nil になるため、timetableItemRepo.List 側で subjects.year が一切絞られず、該当ユーザーの履修登録とは無関係な年度の時間割までまとめて取得する挙動になります。dates が年跨ぎ（例: 月表示の週が12月末〜1月頭）になるケースがあるなら、年度ごとにクエリを分けて結果をマージする／フィルタ側を Years []int のように拡張する等で、常に年度で絞れるようにした方がスケールしやすいです。
|
前期はこれで大丈夫だと思うので後期の挙動は一旦考慮しない |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for _, date := range dates { | ||
| yearMap[date.Year()] = struct{}{} | ||
|
|
||
| month := date.Month() | ||
| if month >= 4 && month <= 9 { | ||
| semesterMap[domain.CourseSemesterH1] = struct{}{} | ||
| semesterMap[domain.CourseSemesterQ1] = struct{}{} | ||
| semesterMap[domain.CourseSemesterQ2] = struct{}{} | ||
| semesterMap[domain.CourseSemesterAllYear] = struct{}{} | ||
| } else { | ||
| semesterMap[domain.CourseSemesterH2] = struct{}{} | ||
| semesterMap[domain.CourseSemesterQ3] = struct{}{} | ||
| semesterMap[domain.CourseSemesterQ4] = struct{}{} | ||
| semesterMap[domain.CourseSemesterAllYear] = struct{}{} | ||
| } | ||
| } |
There was a problem hiding this comment.
Year の推定が暦年（date.Year()）ベースになっており、学年/年度が4月開始の想定だと1〜3月が前年扱いになりません。例えば 2026/01 は学年度 2025 となるケースが多く、現在の実装だと yearMap が分断されて Year フィルタが nil になったり、誤った年度で絞り込む可能性があります。月が1〜3月の場合は年度を date.Year()-1 として扱うなど、学年度の定義に合わせて Year を算出してください。
| for sem := range semesterMap { | ||
| semesters = append(semesters, sem) | ||
| } | ||
|
|
There was a problem hiding this comment.
map から slice への変換結果が非決定順になるため、呼び出し側がスライスの順序に依存している場合（比較、ログ、キャッシュキー、SQLのIN句生成とテストの期待など）に結果が不安定になります。return 前に semesters を定義済みの順序（例: AllYear, H1, Q1, Q2, H2, Q3, Q4 などドメイン規約）でソートして、出力を決定的にするのが安全です。
| semesterOrder := map[domain.CourseSemester]int{ | |
| domain.CourseSemesterAllYear: 0, | |
| domain.CourseSemesterH1: 1, | |
| domain.CourseSemesterQ1: 2, | |
| domain.CourseSemesterQ2: 3, | |
| domain.CourseSemesterH2: 4, | |
| domain.CourseSemesterQ3: 5, | |
| domain.CourseSemesterQ4: 6, | |
| } | |
| sort.Slice(semesters, func(i, j int) bool { | |
| return semesterOrder[semesters[i]] < semesterOrder[semesters[j]] | |
| }) |
やったこと
日付からsemester指定
確認したこと
curlで確認
メモ