Adapt for docker compose and add ability to update fees

[pvieira84]

• Add the possibility to specify ghostfolio security token as env variable and fetch the bearer token in the docker container
• New feature to update the fees on he activity (need to add Commission Details in the FlexQuery)
• Minor changes like variables for own IBKR account details and some more logging

[sourcery-ai]

Hey @pvieira84 - I've reviewed your changes and they look great!

General suggestions:

• Consider implementing a more flexible solution for symbol transformations to improve maintainability.
• Review the addition of debug print statements and ensure they are appropriate for production environments. Consider using a logging framework.
• Ensure that the new method for fetching the bearer token does not introduce security vulnerabilities.
• Clarify the rationale behind certain magic numbers and thresholds, such as the 'cash <= 1' condition, to improve code readability.
• Evaluate the increased complexity introduced by the new features, ensuring that the code remains maintainable and adheres to the Single Responsibility Principle.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

• X
• Mastodon
• LinkedIn
• Facebook

---

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

[sourcery-ai]

suggestion (code_refinement): Consider removing or adjusting the debug print statement for production code to maintain a clean and professional output. If this information is crucial, consider implementing a logging framework with appropriate log levels.

Suggested change

	print("Getting cash amount")
	import logging
	# Configure logging at the beginning of your script or application
	logging.basicConfig(level=logging.INFO, filename='SyncIBKR.log', filemode='a', format='%(name)s - %(levelname)s - %(message)s')
	# Replace the print statement with a logging call
	logging.info("Getting cash amount")

[sourcery-ai]

suggestion (code_refinement): The handling of specific symbols (e.g., 'V80A') with hardcoded replacements ('VNGA80.MI') could lead to maintainability issues as more exceptions are added. Consider creating a mapping or a more flexible solution for symbol transformations.

Suggested change

	elif "V80A" in trade.symbol:
	symbol = "VNGA80.MI"
	# Define a mapping for symbol transformations at the beginning of your script or function
	symbol_mapping = {
	"V80A": "VNGA80.MI",
	# Add more mappings here as needed
	}
	# Then, use the mapping to transform symbols
	if trade.symbol in symbol_mapping:
	symbol = symbol_mapping[trade.symbol]

[sourcery-ai]

question (code_clarification): Changing the condition to 'cash <= 1' for setting cash to an account might not be intuitive. Consider adding a comment to explain the rationale behind using '1' as the threshold, especially if it's related to avoiding setting negligible cash amounts.

Suggested change

	if cash <= 1:
	# Check if the cash value is 1 or less. This threshold is used to avoid setting negligible cash amounts to the account.
	# This might be particularly relevant in scenarios where only significant cash amounts should trigger certain actions.
	if cash <= 1:

[sourcery-ai]

🚨 suggestion (security): Logging the account ID upon creation is useful for tracking and debugging. However, ensure that this does not pose a security risk by inadvertently exposing sensitive information in logs.

Suggested change

	print("IBKR account: " + response.json()["id"])
	import logging
	# It's a good practice to configure logging at the beginning of your script or application
	logging.basicConfig(level=logging.INFO)
	# Replace the print statement with a logging call
	# Ensure to use appropriate logging level. INFO is used here as an example.
	logging.info("IBKR account: %s", response.json()["id"])

[sourcery-ai]

issue (complexity): The addition of the create_ghost_token method and the conditional logic for token handling in the __init__ method significantly increases the complexity of the SyncIBKR class. This class now takes on additional responsibilities beyond its original scope, which can make it harder to maintain and understand. Specifically:

1. The SyncIBKR class now handles both synchronization and token generation, which could be seen as a violation of the Single Responsibility Principle. Consider separating these concerns into different classes or modules to improve modularity and maintainability.

2. The introduction of hardcoded symbol transformations directly in the business logic (e.g., .USD-PAXOS to USD, VUAA to .L, V80A to VNGA80.MI) makes the code less flexible and more difficult to maintain. It's generally a good practice to separate data transformation logic from business logic, possibly by using a configuration file or a dedicated transformation service.

3. The new code introduces additional conditional logic and error handling paths without significantly improving the robustness of the error handling strategy. It's important to ensure that error handling is both comprehensive and easy to follow, which might not be the case with the added complexity.

4. The overall length and complexity of the code have increased with these changes. While more lines of code don't necessarily mean higher complexity, in this case, the added responsibilities, conditional logic, and hardcoded values contribute to a more complex and less maintainable codebase.

Consider simplifying the initialization logic by encapsulating the token handling in a separate method or class. This could help reduce the complexity and improve the readability and maintainability of the code. Separating concerns more clearly would also make it easier to test individual components in isolation.

[agusalex]

Thank you for your contributions!! sorry for the delay!

[agusalex]

LGTM