-
Notifications
You must be signed in to change notification settings - Fork 20
Feat improve watchlist #173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThis PR refactors the watchlist feature by cleaning up deprecated methods, extending the create logic to accept initial tickers, and updating request/response models and test fixtures to align with the new API contract. Class diagram for updated Watchlist request modelsclassDiagram
class CreateWatchlistRequest {
+str name
+List[str] | None tickers
}
class UpdateWatchlistRequest {
+str id
+List[str] tickers
}
Class diagram for removed deprecated Watchlist methodsclassDiagram
class WatchlistService {
-add(request: AddToWatchlistRequest) : WatchlistResponse
-remove(request: RemoveFromWatchlistRequest) : WatchlistResponse
-list() : List[WatchlistProduct]
+add_to_watchlist(request: UpdateWatchlistRequest) : Watchlist
+watchlist(request: GetWatchlistRequest) : Watchlist
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/test_watchlist.py:13` </location>
<code_context>
- assert len(watched) == 10
-
@pytest.mark.parametrize(
"exchange, symbols",
</code_context>
<issue_to_address>
No explicit tests for the new tickers field in CreateWatchlistRequest.
Please add tests for CreateWatchlistRequest with tickers set to None, an empty list, and a large list to ensure all scenarios are handled correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Improve watchlist support by enabling creation with initial symbols, cleaning up deprecated methods and constants, extending trade types, and refreshing test fixtures to match updated API responses.
New Features:
Enhancements:
Tests: