Add get-match command for detailed match information - #4
Conversation
Adds a new command to fetch detailed match information from the API: - New command: get-match <match-id> with -o flag for output format - Reuses existing API models, domain models, and output formatters - Adds ToDomainMatchFromDetailResponse mapper function - Comprehensive test coverage for command and mapper - Supports table, JSON, and YAML output formats
📝 WalkthroughWalkthroughAdds a new GetMatch CLI command that requests detailed match data from the Blast API (10s timeout), parses JSON, maps to the domain model, and formats output. Integrates into the root CLI, introduces a mapper wrapper, defines BaseURL constant, and includes comprehensive unit tests and minor URL replacements. Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Cmd as GetMatchCmd
participant API as Blast API
participant Parser as JSON Parser
participant Mapper as Domain Mapper
participant Formatter as Output Formatter
User->>Cmd: Run(ctx) with MatchID
Cmd->>API: GET /matches/{MatchID}/detailed (10s)
API-->>Cmd: HTTP Response (status + body)
alt 404
Cmd-->>User: Error "match not found"
else Non-OK
Cmd-->>User: Error "unexpected status code"
else 200
Cmd->>Parser: Parse JSON body
alt parse error
Parser-->>Cmd: Parse error
Cmd-->>User: Error "failed to parse JSON"
else parsed
Parser-->>Cmd: MatchResponse
Cmd->>Mapper: ToDomainMatchFromDetailResponse(MatchResponse)
alt mapping error
Mapper-->>Cmd: Error
Cmd-->>User: Error "failed to map match"
else mapped
Mapper-->>Cmd: domain.Match
Cmd->>Formatter: GetMatchesFormatter(Output) -> formatted output
Formatter-->>User: Formatted match output
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @mgranderath, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR introduces a new get-match command, which is well-implemented and follows existing patterns in the codebase. The test coverage is good, handling various API response scenarios effectively. I have a couple of suggestions to further improve maintainability and test robustness: one regarding a hardcoded API URL, and another to strengthen the tests for JSON/YAML output formats by verifying the actual output content.
| } | ||
|
|
||
| func (g *GetMatchCmd) Run(ctx *Context) error { | ||
| url := fmt.Sprintf("https://api.blast.tv/v2/matches/%s/detailed", g.MatchID) |
There was a problem hiding this comment.
The API base URL https://api.blast.tv is hardcoded. To improve maintainability and avoid duplication, consider defining this as a package-level constant. This would make it easier to update the API endpoint in the future and ensure consistency across the application, especially since it's also used in the test file.
For example, you could add this at the package level:
const blastAPIBaseURL = "https://api.blast.tv"And then use it here:
url := fmt.Sprintf("%s/v2/matches/%s/detailed", blastAPIBaseURL, g.MatchID)| err := cmd.Run(ctx) | ||
| assert.NoError(t, err) |
There was a problem hiding this comment.
This test could be more robust. It currently only checks that the command runs without error. It should also verify that the HTTP mock was consumed (gock.IsDone()) and that valid JSON was written to standard output. This feedback also applies to the 'YAML output format' test.
You will need to add os, io, and encoding/json to your imports to apply this suggestion.
oldStdout := os.Stdout
r, w, _ := os.Pipe()
os.Stdout = w
err := cmd.Run(ctx)
w.Close()
out, _ := io.ReadAll(r)
os.Stdout = oldStdout
assert.NoError(t, err)
assert.True(t, gock.IsDone())
assert.True(t, json.Valid(out))- Add internal/api/blast/client.go with BaseURL constant - Update all commands to use blast.BaseURL instead of hardcoded URLs: - get_match.go - get_matches.go - get_brackets.go - list_tournaments.go This improves maintainability by centralizing the API base URL, making it easier to update if the API endpoint ever changes.
1 participant
Summary
This PR adds a new
get-matchcommand to fetch detailed information for a specific match using the Blast.tv API.Changes
New command:
get-match <match-id>- Retrieves detailed match information-oflagrlcs-cli get-match 667d39fa-65cb-46c1-b939-3554cdbd5cc0 -o jsonCode reuse:
MatchResponseAPI model frommatch_models.godomain.Matchdomain modelNew mapper function:
ToDomainMatchFromDetailResponse()ininternal/mapper/match.goTest coverage:
API Endpoint
Testing
go test ./internal/cmd/... ./internal/mapper/... -vChecklist
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.