Skip to content

Conversation

@inochidainini
Copy link
Contributor

@inochidainini inochidainini commented Oct 24, 2025

User description

#1345
新たにブランチを作り変更を加えました


PR Type

Enhancement, Tests


Description

  • 終了時刻更新の実装を追加

  • レコード未更新時のエラー対応

  • 終了時刻更新の統合テスト追加

  • テスト用データセット構築を実装


Diagram Walkthrough

flowchart LR
  repo["GamePlayLogV2.UpdateGamePlayLogEndTime 実装"] --> db["DB更新: end_timeを設定"]
  db -- "RowsAffected==0" --> err1["ErrNoRecordUpdated返却"]
  tests["TestUpdateGamePlayLogEndTime 追加"] --> seed["テストデータ作成"]
  tests --> assert1["更新成功の検証"]
  tests --> assert2["未更新エラーの検証"]
Loading

File Walkthrough

Relevant files
Enhancement
v2_game_play_log.go
終了時刻更新の実装と未更新時の判定                                                                               

src/repository/gorm2/v2_game_play_log.go

  • 終了時刻更新処理を実装
  • DB取得とエラーハンドリング追加
  • RowsAffectedで未更新判定を実装
  • 適切なエラーメッセージにラップ
+22/-3   
Tests
v2_game_play_log_test.go
UpdateGamePlayLogEndTimeの正常・異常系テスト追加                                         

src/repository/gorm2/v2_game_play_log_test.go

  • スキップ削除しテスト有効化
  • 必要な関連データの生成とクリーンアップ
  • 正常系: end_time更新を検証
  • 異常系: 未存在IDでErrNoRecordUpdated確認
+100/-2 

@inochidainini inochidainini requested a review from a team as a code owner October 24, 2025 13:54
@github-actions
Copy link

Migrate lint ✅

Lint output
Notice: Starting with v0.38, 'atlas migrate lint' will only be available in Atlas Pro.
The command and existing code remain open source and available in the Community Edition.
Learn more: https://atlasgo.io/blog-v037#deprecate-lint

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1345 - Partially compliant

Compliant requirements:

  • repository層のUpdateGamePlayLogEndTimeを実装
  • レコード未更新時にErrNoRecordUpdatedを返す
  • 終了時刻更新の統合テストを追加
  • テスト用データセットの準備を実装

Non-compliant requirements:

  • 実装がinterfaceコメントの細則(例: 既にend_timeが設定済みの場合の扱い等)がある場合、その準拠度はコードからは断定できない

Requires further human verification:

  • interfaceコメントに定義された細かな仕様(例: タイムゾーン・同値更新の扱い・同時実行制御要件)に完全準拠しているかの確認
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

競合更新

単純なUpdateで同時実行時の上書きリスクがある可能性。必要ならwhere句にend_time IS NULLなどの条件を加えるべきか仕様確認が必要。

result := db.
	Model(&schema.GamePlayLogTable{}).
	Where("id = ?", playLogID.UUID()).
	Update("end_time", endTime)

err = result.Error

if err != nil {
	return fmt.Errorf("update end_time: %w", err)
}

if result.RowsAffected == 0 {
	return repository.ErrNoRecordUpdated
}
時刻比較の不安定性

WithinDurationでtime.Second許容だが、DB保存時の精度やタイムゾーン差異で稀にフレークしうる。DBラウンドトリップ後の正規化やTruncateで安定化を検討。

	var updatedLog schema.GamePlayLogTable
	err = db.
		First(&updatedLog, "id = ?", uuid.UUID(testCase.playLogID)).Error
	assert.NoError(t, err)
	assert.WithinDuration(t, testCase.endTime, updatedLog.EndTime.Time, time.Second)
}
テスト後片付け

Cleanupで関連レコード削除順は良いが、外部キー制約の有無により失敗時に後続へ影響の可能性。Delete結果やエラーを無視しているため失敗時の診断が難しい。

t.Cleanup(func() {
	db.Delete(&schema.GamePlayLogTable{}, "id = ?", playLog.ID)
	db.Delete(&schema.GameVersionTable2{}, "id = ?", gameVersion.ID)
	db.Delete(&schema.GameVideoTable2{}, "id = ?", gameVideo.ID)
	db.Delete(&schema.GameImageTable2{}, "id = ?", gameImage.ID)
	db.Delete(&schema.GameTable2{}, "id = ?", game.ID)
	db.Delete(&schema.EditionTable{}, "id = ?", edition.ID)
})

@github-actions
Copy link

github-actions bot commented Oct 24, 2025

Migrate lint ✅

Lint output
Notice: Starting with v0.38, 'atlas migrate lint' will only be available in Atlas Pro.
The command and existing code remain open source and available in the Community Edition.
Learn more: https://atlasgo.io/blog-v037#deprecate-lint

@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.38%. Comparing base (91e3bb6) to head (8adfd39).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/repository/gorm2/v2_game_play_log.go 71.42% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1409      +/-   ##
==========================================
+ Coverage   50.95%   52.38%   +1.42%     
==========================================
  Files         136      136              
  Lines       12090     9858    -2232     
==========================================
- Hits         6161     5164     -997     
+ Misses       5619     4382    -1237     
- Partials      310      312       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@ikura-hamu ikura-hamu left a comment

Choose a reason for hiding this comment

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

1個だけお願いします。あとPRの名前を意味があるものに直しておいてください

err = db.
First(&updatedLog, "id = ?", uuid.UUID(testCase.playLogID)).Error
assert.NoError(t, err)
assert.WithinDuration(t, testCase.endTime, updatedLog.EndTime.Time, time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

他のフィールドに変更が入ってしまっていないかも確かめてください

@inochidainini inochidainini changed the title 修正 updateGamePlayLogEndTimeの実装 Oct 25, 2025
Copy link
Member

@ikura-hamu ikura-hamu left a comment

Choose a reason for hiding this comment

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

ありがとう、マージしちゃってください
issueもcloseしてください

@inochidainini inochidainini merged commit 5681439 into main Oct 25, 2025
11 checks passed
@inochidainini inochidainini deleted the updateGamePlayLogEndTime2 branch October 25, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants