feat: add gamereplays tab with active/upcoming tournament tracking#246
feat: add gamereplays tab with active/upcoming tournament tracking#246undead2146 wants to merge 4 commits into
Conversation
| /// <param name="comment">The comment content.</param> | ||
| /// <param name="cancellationToken">Cancellation token.</param> | ||
| /// <returns>Result indicating success or failure.</returns> | ||
| Task<OperationResult<bool>> PostCommentAsync( |
There was a problem hiding this comment.
What is the difference between IGameReplaysService.PostCommentAsync() and IGameReplaysCommentService.PostCommentAsync()? They look identical, but the comment is slightly different (only difference being "tournament". What makes it clear that one targets a tournament and one does not? I don't think this is clear enough.
Same goes for GetCommentsAsync() in both files.
| /// <summary> | ||
| /// Gets or sets the redirect URI. | ||
| /// </summary> | ||
| public string RedirectUri { get; set; } = string.Empty; |
There was a problem hiding this comment.
What is the benefit of setting all these to string.Empty? Is it so bad if this were string? RedirectUri? I guess it won't matter when doing string.IsNullOrEmpty(), but I was just curious why you're so explicit in setting this to string.Empty.
Resolved merge conflicts by combining both GameReplays and Info features: - NavigationTab.cs: Keep both GameReplays and Info enum values - MainViewModel.cs: Merge both GameReplaysViewModel and InfoViewModel - MainView.axaml: Include both DataTemplates and tab buttons - AppServices.cs: Register all services (GameReplays, UploadThing, ReplayManager, MapManager) - SharedViewModelModule.cs: Register both GameReplaysViewModel and InfoViewModel - MainViewModelTests.cs: Update tests to cover both features Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
cf7fe92 to
54bfd3b
Compare
| private static bool Initialize(HttpClient client) | ||
| { | ||
| client.DefaultRequestHeaders.UserAgent.ParseAdd(ApiConstants.DefaultUserAgent); | ||
| client.Timeout = TimeSpan.FromSeconds(GameReplaysConstants.DefaultRequestTimeoutSeconds); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
static method never called - HttpClient timeout and user agent won't be configured
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub.Core/Services/GameReplays/GameReplaysHttpClient.cs
Line: 225:230
Comment:
static method never called - `HttpClient` timeout and user agent won't be configured
How can I resolve this? If you propose a fix, please make it concise.| // Register HTTP client as transient (creates new instance each time) | ||
| services.AddTransient<IGameReplaysHttpClient, GameReplaysHttpClient>(); |
There was a problem hiding this comment.
transient HttpClient registration creates new instances on each request, leading to socket exhaustion under load
| // Register HTTP client as transient (creates new instance each time) | |
| services.AddTransient<IGameReplaysHttpClient, GameReplaysHttpClient>(); | |
| // Use IHttpClientFactory to manage HttpClient lifecycle properly | |
| services.AddHttpClient<IGameReplaysHttpClient, GameReplaysHttpClient>(); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub.Core/Infrastructure/DependencyInjection/GameReplaysModule.cs
Line: 19:20
Comment:
transient `HttpClient` registration creates new instances on each request, leading to socket exhaustion under load
```suggestion
// Use IHttpClientFactory to manage HttpClient lifecycle properly
services.AddHttpClient<IGameReplaysHttpClient, GameReplaysHttpClient>();
```
How can I resolve this? If you propose a fix, please make it concise.| // TODO: Open authorization URL in browser | ||
| _logger.LogInformation("Initiated OAuth login flow"); |
There was a problem hiding this comment.
browser launch not implemented - OAuth flow won't work without opening the authorization URL in a browser
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/GameReplays/ViewModels/GameReplaysViewModel.cs
Line: 237:238
Comment:
browser launch not implemented - OAuth flow won't work without opening the authorization URL in a browser
How can I resolve this? If you propose a fix, please make it concise.| _logger.LogInformation("Copying tournament URL to clipboard: {Url}", tournament.TournamentUrl); | ||
|
|
||
| // This is a placeholder for actual clipboard logic | ||
| await Task.CompletedTask; |
There was a problem hiding this comment.
clipboard functionality not implemented - copy URL feature won't work
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/GameReplays/ViewModels/GameReplaysViewModel.cs
Line: 323:323
Comment:
clipboard functionality not implemented - copy URL feature won't work
How can I resolve this? If you propose a fix, please make it concise.| // Placeholder for URL opening | ||
| } |
There was a problem hiding this comment.
URL opening not implemented - view details button won't work
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/GameReplays/ViewModels/TournamentItemViewModel.cs
Line: 154:155
Comment:
URL opening not implemented - view details button won't work
How can I resolve this? If you propose a fix, please make it concise.| if (!string.IsNullOrEmpty(TournamentUrl)) | ||
| { | ||
| // Placeholder for clipboard copy | ||
| await Task.CompletedTask; |
There was a problem hiding this comment.
clipboard functionality not implemented - copy link feature won't work
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/GameReplays/ViewModels/TournamentItemViewModel.cs
Line: 168:168
Comment:
clipboard functionality not implemented - copy link feature won't work
How can I resolve this? If you propose a fix, please make it concise.| private GameReplaysTournaments? _cachedTournaments; | ||
| private System.DateTime _cacheExpiry; |
There was a problem hiding this comment.
in-memory cache not thread-safe - concurrent requests could corrupt cached data or cause race conditions
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub.Core/Services/GameReplays/GameReplaysService.cs
Line: 25:26
Comment:
in-memory cache not thread-safe - concurrent requests could corrupt cached data or cause race conditions
How can I resolve this? If you propose a fix, please make it concise.| private OAuthTokenResponse? _currentToken; | ||
| private OAuthUserInfo? _currentUser; |
There was a problem hiding this comment.
in-memory token storage means tokens are lost on app restart - users must re-authenticate every time the app launches
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub.Core/Services/GameReplays/GameReplaysAuthService.cs
Line: 27:28
Comment:
in-memory token storage means tokens are lost on app restart - users must re-authenticate every time the app launches
How can I resolve this? If you propose a fix, please make it concise.| using (var rng = RandomNumberGenerator.Create()) | ||
| { | ||
| rng.GetBytes(stateBytes); | ||
| } |
There was a problem hiding this comment.
RandomNumberGenerator.Create() is obsolete - use RandomNumberGenerator.Create() static methods directly
| using (var rng = RandomNumberGenerator.Create()) | |
| { | |
| rng.GetBytes(stateBytes); | |
| } | |
| var stateBytes = new byte[32]; | |
| RandomNumberGenerator.Fill(stateBytes); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub.Core/Services/GameReplays/GameReplaysAuthService.cs
Line: 193:196
Comment:
`RandomNumberGenerator.Create()` is obsolete - use `RandomNumberGenerator.Create()` static methods directly
```suggestion
var stateBytes = new byte[32];
RandomNumberGenerator.Fill(stateBytes);
```
How can I resolve this? If you propose a fix, please make it concise.
This PR adds a GameReplays tab to the tabs section which checks the tournaments post and displays the active and upcoming tournaments on GameReplays
Greptile Overview
Greptile Summary
This PR adds a GameReplays tab that displays active and upcoming tournaments from the GameReplays.org forum. The implementation includes HTML parsing to extract tournament data, OAuth 2.0 authentication scaffolding, and a modern UI with filtering capabilities.
Key Changes:
Critical Issues:
HttpClientregistered as transient instead of usingIHttpClientFactory, risking socket exhaustiongenhub_client/genhub_secret) in constants fileGameReplaysServiceInitializemethod inGameReplaysHttpClientmeans timeout/user-agent won't be configuredTest Artifact:
index.htmltest fixture committed to repository root should be moved to test directoryConfidence Score: 2/5
GameReplaysModule.cs(DI registration),GameReplaysHttpClient.cs(unused Initialize method),GameReplaysViewModel.cs(incomplete OAuth),GameReplaysConstants.cs(placeholder credentials), andGameReplaysService.cs(thread-safety)Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant GameReplaysView participant GameReplaysViewModel participant GameReplaysService participant GameReplaysAuthService participant GameReplaysHttpClient participant GameReplaysParser participant GameReplaysAPI Note over User,GameReplaysAPI: Tab Initialization Flow User->>GameReplaysView: Navigate to GameReplays tab GameReplaysView->>GameReplaysViewModel: InitializeAsync() GameReplaysViewModel->>GameReplaysService: GetTournamentsAsync() GameReplaysService->>GameReplaysHttpClient: GetHtmlAsync(tournament board URL) GameReplaysHttpClient->>GameReplaysAPI: HTTP GET /community/index.php?showtopic=1066376 GameReplaysAPI-->>GameReplaysHttpClient: HTML response GameReplaysHttpClient-->>GameReplaysService: HTML content GameReplaysService->>GameReplaysParser: ParseTournamentBoard(html) GameReplaysParser-->>GameReplaysService: GameReplaysTournaments GameReplaysService-->>GameReplaysViewModel: Tournaments data GameReplaysViewModel->>GameReplaysViewModel: UpdateFilteredTournaments() GameReplaysViewModel-->>GameReplaysView: Display tournaments Note over User,GameReplaysAPI: OAuth Login Flow (Incomplete) User->>GameReplaysView: Click Login GameReplaysView->>GameReplaysViewModel: LoginCommand GameReplaysViewModel->>GameReplaysService: InitiateLoginAsync() GameReplaysService->>GameReplaysAuthService: GetAuthorizationUrl() GameReplaysAuthService->>GameReplaysAuthService: GenerateState() GameReplaysAuthService-->>GameReplaysService: Authorization URL GameReplaysService-->>GameReplaysViewModel: Auth URL Note over GameReplaysViewModel: TODO: Browser launch not implemented GameReplaysViewModel-->>User: Log message only Note over User,GameReplaysAPI: OAuth Callback Flow (If implemented) GameReplaysAPI->>GameReplaysService: HandleCallbackAsync(code, state) GameReplaysService->>GameReplaysAuthService: ValidateState(state) GameReplaysAuthService-->>GameReplaysService: Valid GameReplaysService->>GameReplaysAuthService: ExchangeCodeForTokenAsync(code) GameReplaysAuthService->>GameReplaysHttpClient: PostJsonAsync(token endpoint) GameReplaysHttpClient->>GameReplaysAPI: POST /oauth/token GameReplaysAPI-->>GameReplaysHttpClient: Token response GameReplaysHttpClient-->>GameReplaysAuthService: OAuthTokenResponse GameReplaysAuthService->>GameReplaysAuthService: SaveTokenAsync() GameReplaysAuthService->>GameReplaysHttpClient: SetAuthCookie() GameReplaysAuthService->>GameReplaysHttpClient: GetJsonAsync(user info endpoint) GameReplaysHttpClient->>GameReplaysAPI: GET /oauth/resource GameReplaysAPI-->>GameReplaysHttpClient: User info GameReplaysHttpClient-->>GameReplaysAuthService: OAuthUserInfo GameReplaysAuthService-->>GameReplaysService: Success Note over User,GameReplaysAPI: Tournament Filtering User->>GameReplaysView: Select filter (Active/Upcoming) GameReplaysView->>GameReplaysViewModel: SelectFilterCommand GameReplaysViewModel->>GameReplaysViewModel: UpdateFilteredTournaments() GameReplaysViewModel-->>GameReplaysView: Update UIContext used:
dashboard- Use dedicated constants classes instead of hardcoding constants string, integers or variables in ser... (source)