Skip to content

Conversation

@mathsuky
Copy link
Contributor

@mathsuky mathsuky commented Oct 24, 2025

User description

closes #1387
endのみを指定しなかった場合の仕様を定義し忘れていた(と思う)ので,それの追記を合わせて行いました


PR Type

Enhancement, Tests, Documentation


Description

  • エディション統計ハンドラを実装

  • 期間指定のデフォルト挙動を定義

  • サービスエラーのHTTP変換を実装

  • 単体テストとOpenAPI更新


Diagram Walkthrough

flowchart LR
  H["HTTP GET /editions/{editionID}/play-stats"] -- "parse params (start/end)" --> D["determine start/end defaults"]
  D -- "call service" --> S["GamePlayLogService.GetEditionPlayStats"]
  S -- "success" --> M["map domain -> OpenAPI model"]
  M -- "200 OK JSON" --> R["Response"]
  S -- "ErrInvalidEdition" --> N["404 Not Found"]
  S -- "other error" --> E["500 Internal Server Error"]
Loading

File Walkthrough

Relevant files
Enhancement
game_play_log.go
エディション統計取得ハンドラの本実装                                                                             

src/handler/v2/game_play_log.go

  • GetEditionPlayStats を実装
  • start/end のデフォルト計算を追加
  • ドメイン統計をOpenAPIレスポンスへ変換
  • サービスエラーを404/500へ変換
+53/-3   
Tests
game_play_log_test.go
エディション統計ハンドラの包括的テスト                                                                           

src/handler/v2/game_play_log_test.go

  • GetEditionPlayStats のテスト追加
  • 期間指定の全パターンを検証
  • 404/500 のエラーパスを検証
  • レスポンスボディの内容を検証
+243/-0 
Documentation
v2.yaml
期間指定仕様のOpenAPIドキュメント更新                                                                     

docs/openapi/v2.yaml

  • end のみ指定時の仕様を追記
  • 期間指定の挙動を明確化
+1/-0     

@mathsuky mathsuky requested a review from a team as a code owner October 24, 2025 11:13
@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 ✅

1387 - PR Code Verified

Compliant requirements:

  • OpenAPI定義に従って GetEditionPlayStats ハンドラを実装する
  • 期間指定パラメータ(start/end)のデフォルト挙動を仕様として明確化する
  • サービス層のエラーをHTTPステータスに適切にマッピングする
  • OpenAPI定義の更新を行う
  • 単体テストを追加し、主要パス(正常系・異常系)をカバーする

Requires further human verification:

  • OpenAPIとの完全一致(型・required/optional・フォーマット)を生成コード含めて最終確認
  • 実運用時間帯・タイムゾーン仕様の整合性検証(UTC/JSTなどの期待)
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Time Range Logic

end未指定時にtime.Now()を使用し、startend比較の許容差をテスト側で1時間にしているため、タイムゾーンや処理遅延で境界がぶれる可能性があります。固定時刻の注入や丸めの方針を検討してください。

if params.End != nil {
	end = *params.End
} else {
	end = time.Now()
}
if params.Start != nil {
	start = *params.Start
} else {
	start = end.Add(-24 * time.Hour)
}
Error Mapping

ドメインエラーのHTTP変換がErrInvalidEditionの404とその他500のみ。バリデーションエラー等、将来の追加エラー型に応じたマッピング拡張余地があります。メッセージの国際化や詳細情報の有無も検討対象です。

if errors.Is(err, service.ErrInvalidEdition) {
	return echo.NewHTTPError(http.StatusNotFound, "edition not found")
}
if err != nil {
	log.Printf("error: failed to get edition play stats: %v\n", err)
	return echo.NewHTTPError(http.StatusInternalServerError, "failed to get edition play stats")
}
Spec Consistency

OpenAPIの説明にendのみ指定時の挙動を追記。実装はend-24hstartとしているが、端点の包含/排他や時台の丸め方(例:分秒があるendに対しての切り捨て)との整合を確認してください。

## 期間の指定について
- startとendを両方指定しない場合:現在時刻から24時間前までの統計データを返します
- startのみ指定した場合:指定された開始時刻から現在時刻までの統計データを返します
- endのみ指定した場合:指定された終了時刻から24時間前までの統計データを返します
- startとendを両方指定した場合:指定された期間の統計データを返します
統計データは時台ごとに区切られており、各時台(例:14時台は14:00:00〜14:59:59)の統計値が含まれます。

@mathsuky mathsuky requested a review from ikura-hamu October 24, 2025 11:14
@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

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.64%. Comparing base (91e3bb6) to head (f1ac876).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1408      +/-   ##
==========================================
+ Coverage   50.95%   52.64%   +1.68%     
==========================================
  Files         136      136              
  Lines       12090     9908    -2182     
==========================================
- Hits         6161     5216     -945     
+ Misses       5619     4380    -1239     
- 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個だけコメントをしたので確認をお願いします。
あと、startとendのバリデーションを入れてもいいかなと思いました。具体的には、endがstartより早いとか、期間が長すぎるとかです。期間が長すぎると、期間の長さに応じてスライスの長さをとってたりするので、悪意あるユーザーがめっちゃ長い期間を入れてメモリを使い切らせるとかが考えられます。上限10年とかでいいと思います。バリデーションはserviceの方でやるといいと思います。

gomock.Any(),
testCase.editionID,
gomock.Cond(func(start time.Time) bool {
return start.Sub(testCase.expectedStart).Abs() < time.Hour
Copy link
Member

Choose a reason for hiding this comment

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

ここHourの幅はいらない気がするんですが、付けた意図は何ですか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

意図していたのは秒の幅でしたのでその様に修正しました:pray:

@mathsuky mathsuky requested a review from ikura-hamu October 27, 2025 11:49
@mathsuky
Copy link
Contributor Author

ad85373に入れるべき内容が一部次のコミットに入ってしまいました:pray:
クエリパラメタのバリデーションについて同様の変更をGetGamePlayStatsのserviceに入れましたが,handlerについては @ebisen14441444 さんが実装中かと思って特に触っていません

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.

ありがとう、okです。マージしちゃってください

@mathsuky mathsuky merged commit 9907c1a into main Oct 27, 2025
11 checks passed
@mathsuky mathsuky deleted the feat/imp-GetEditionPlayStats branch October 27, 2025 12:23
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.

GetEditionPlayStats の handler の実装

3 participants