-
Notifications
You must be signed in to change notification settings - Fork 11
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
Multiple enhancements including support for multiple accounts sync and better symbol mapping to YAHOO #25
Conversation
…d better symbol mapping to YAHOO. Using Ghostfolio API filtering to fetch all activities from an account instead of comparing all activities. Using Ghostfolio API filtering to delete all activities from an account instead of comparing all activities. Support for flex query output containing multiple IBKR accounts data to sync multiple accounts. Use ISIN symbol type from IBKR to upload new activities since it provides better mapping to YAHOO data and no need for the mapping file. Add IBKR trade id to the uploaded activity comment to enable precise comparison when computing the diff. Diff retrocompatibility with previous versions of symbols (mapping file etc). Find the IBKR account base currency from the flex query. Get trade fee from the Trade node instead of UnbundledCommissionDetail node. Use hours, minutes and seconds in the activity date. Use logging instead of print. Add operation to print all activities from an account. Add underscores to some operations to be more readable. If an environment variable does not have enough comma separated values, use the last one (good for unique stuff like API keys). Caching of ghostfolio account id in the class for all operations to avoid multiple API calls. Updated dependencies to remove security issues.
Reviewer's Guide by SourceryThis pull request introduces multiple enhancements to the IBKR sync functionality, including support for multiple accounts, improved symbol mapping, and more robust activity tracking. The changes involve fetching and parsing data from IBKR Flex Queries, transforming it into a format compatible with the Ghostfolio API, and then synchronizing the data. The implementation includes filtering activities using the Ghostfolio API, using ISIN symbols for better mapping, and adding trade IDs to activity comments for precise comparisons. Sequence diagram for enhanced IBKR sync processsequenceDiagram
participant IBKR
participant SyncIBKR
participant Ghostfolio
SyncIBKR->>IBKR: Download Flex Query
IBKR-->>SyncIBKR: Return Flex Query Response
SyncIBKR->>SyncIBKR: Parse Query & Extract Account Statement
SyncIBKR->>SyncIBKR: Get Account Currency
SyncIBKR->>Ghostfolio: Create/Get IBKR Account ID
Ghostfolio-->>SyncIBKR: Return Account ID
SyncIBKR->>Ghostfolio: Set Cash Balance
loop For Each Trade
SyncIBKR->>SyncIBKR: Get Symbol (ISIN preferred)
SyncIBKR->>SyncIBKR: Format Activity with Trade ID
end
SyncIBKR->>Ghostfolio: Get Existing Activities (filtered)
Ghostfolio-->>SyncIBKR: Return Activities
SyncIBKR->>SyncIBKR: Compare Activities using Trade IDs
SyncIBKR->>Ghostfolio: Import New Activities
Updated class diagram for SyncIBKRclassDiagram
class SyncIBKR {
-account_id: Optional[str]
-ghost_token: str
-ghost_host: str
-ibkr_account_id: str
-ghost_account_name: str
-ghost_currency: str
-symbol_mapping: dict
+sync_ibkr()
+get_symbol_for_trade(trade: Trade, data_source: str)
+set_cash_to_account(account_id: str, cash: dict)
+get_all_acts_for_account(account_id: str, range: str, symbol: str)
+get_account_flex_statement(query: FlexQueryResponse)
-create_ghost_token(ghost_host: str, ghost_key: str)
-create_or_get_IBKR_accountId()
}
note for SyncIBKR "Enhanced to support:
- Multiple accounts
- ISIN symbol mapping
- Trade ID tracking"
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 @NautillusSs - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider splitting future large changes into smaller, focused pull requests to make review easier. However, the current changes are well structured and documented.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Overall looking really good thank you so much! left some comments
from SyncIBKR import SyncIBKR | ||
from pretty_print import pretty_print_table | ||
|
||
template = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" |
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.
this is good thank you!
|
||
GETALLACTS = "GETALLACTS" | ||
GET_ALL_ACTS = "GET_ALL_ACTS" |
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.
noice
@@ -8,4 +8,5 @@ symbol_mapping: | |||
EGLN: EGLN.L | |||
IWDA: IWDA.AS | |||
VWCE: VWCE.DE | |||
ASML: ASML.AS |
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.
awesome!
Thank you so much for your contribution, this means a lot for the project! ❤️ |
Thank you for reviewing so quickly and merging! |
Multiple enhancements including support for multiple accounts sync and better symbol mapping to YAHOO
Hello I have 2 accounts on IBKR and had to make a few changes to the code to support both of them and sync both in one run. I also added some enhancements listed below. Apologies for the big amount of changes. I wanted to make it all work locally and then push it. Happy to discuss. :)
Summary by Sourcery
New Features: