-
Notifications
You must be signed in to change notification settings - Fork 435
Main #553
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
Main #553
Conversation
This commit introduces a new broker integration for mstock, following the existing architecture for broker plugins. The integration includes: - Authentication API for TOTP-based login. - Order and data APIs for placing orders, and retrieving positions, holdings, and funds. - A WebSocket streaming adapter for real-time data. - A callback endpoint and login page for the mstock broker. - Configuration variables and test cases.
feat: Add mstock broker integration
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.
2 issues found across 10 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="broker/mstock/api/order_api.py">
<violation number="1" location="broker/mstock/api/order_api.py:6">
trigger_price is accepted but never forwarded in the payload, so stop-loss orders lose their trigger price and will fail.</violation>
<violation number="2" location="broker/mstock/api/order_api.py:44">
trigger_price is ignored during order modification, so stop-loss orders cannot be updated correctly.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
broker/mstock/api/order_api.py
Outdated
| except Exception as e: | ||
| return None, str(e) | ||
|
|
||
| def modify_order(api_key, auth_token, order_id, variety, tradingsymbol, exchange, transaction_type, quantity, product, order_type, price=0, trigger_price=0): |
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.
trigger_price is ignored during order modification, so stop-loss orders cannot be updated correctly.
Prompt for AI agents
Address the following comment on broker/mstock/api/order_api.py at line 44:
<comment>trigger_price is ignored during order modification, so stop-loss orders cannot be updated correctly.</comment>
<file context>
@@ -0,0 +1,145 @@
+ except Exception as e:
+ return None, str(e)
+
+def modify_order(api_key, auth_token, order_id, variety, tradingsymbol, exchange, transaction_type, quantity, product, order_type, price=0, trigger_price=0):
+ """
+ Modifies an existing order.
</file context>
✅ Addressed in 7317481
broker/mstock/api/order_api.py
Outdated
| import os | ||
| from utils.httpx_client import get_httpx_client | ||
|
|
||
| def place_order(api_key, auth_token, variety, tradingsymbol, exchange, transaction_type, quantity, product, order_type, price=0, trigger_price=0, squareoff=0, stoploss=0, trailing_stoploss=0, disclosed_quantity=0, validity='DAY', amo='NO', ret='DAY'): |
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.
trigger_price is accepted but never forwarded in the payload, so stop-loss orders lose their trigger price and will fail.
Prompt for AI agents
Address the following comment on broker/mstock/api/order_api.py at line 6:
<comment>trigger_price is accepted but never forwarded in the payload, so stop-loss orders lose their trigger price and will fail.</comment>
<file context>
@@ -0,0 +1,145 @@
+import os
+from utils.httpx_client import get_httpx_client
+
+def place_order(api_key, auth_token, variety, tradingsymbol, exchange, transaction_type, quantity, product, order_type, price=0, trigger_price=0, squareoff=0, stoploss=0, trailing_stoploss=0, disclosed_quantity=0, validity='DAY', amo='NO', ret='DAY'):
+ """
+ Places an order with the broker.
</file context>
✅ Addressed in 7317481
|
Thanks for the PR. thats plenty of work. Will open an account with mstock and test it out and approve the PR |
Adds mstock to the list of valid brokers in the installation scripts.
feat: Add mstock to installation scripts
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.
Reviewed changes from recent commits (found 5 issues).
5 issues found across 15 files
Prompt for AI agents (all 5 issues)
Understand the root cause of the following 5 issues and fix them.
<file name="broker/mstock/api/order_api.py">
<violation number="1" location="broker/mstock/api/order_api.py:148">
get_trade_book now calls the wrong REST path, returning an error instead of the trade book. Update the endpoint so it appends only the desired route segment to the base URL.</violation>
</file>
<file name=".sample.env">
<violation number="1" location=".sample.env:22">
VALID_BROKERS now contains mstock twice; remove the duplicate to keep the documented broker list consistent and avoid redundant entries when the value is split.</violation>
</file>
<file name="broker/mstock/api/data.py">
<violation number="1" location="broker/mstock/api/data.py:5">
This line calls os.getenv but the module no longer imports os, so get_positions (and get_holdings) will raise NameError at runtime.</violation>
</file>
<file name="broker/mstock/api/funds.py">
<violation number="1" location="broker/mstock/api/funds.py:53">
Avoid logging the full funds payload because it exposes sensitive balances in plain text. Replace it with a non-sensitive message or remove the log entirely.</violation>
</file>
<file name="broker/mstock/mapping/order_data.py">
<violation number="1" location="broker/mstock/mapping/order_data.py:195">
Guard against `holdings_data['data']` being None before checking for `total_holding`; otherwise this crashes on null API responses.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| } | ||
|
|
||
| def get_trade_book(auth): | ||
| trades_data = get_api_response("/typea/tradebook", auth) |
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.
get_trade_book now calls the wrong REST path, returning an error instead of the trade book. Update the endpoint so it appends only the desired route segment to the base URL.
Prompt for AI agents
Address the following comment on broker/mstock/api/order_api.py at line 148:
<comment>get_trade_book now calls the wrong REST path, returning an error instead of the trade book. Update the endpoint so it appends only the desired route segment to the base URL.</comment>
<file context>
@@ -100,46 +118,87 @@ def cancel_order(api_key, auth_token, order_id, variety):
- except Exception as e:
- return None, str(e)
+def get_trade_book(auth):
+ trades_data = get_api_response("/typea/tradebook", auth)
-def get_trade_book(api_key, auth_token):
</file context>
| # Valid Brokers Configuration | ||
|
|
||
| VALID_BROKERS = 'fivepaisa,fivepaisaxts,aliceblue,angel,compositedge,dhan,dhan_sandbox,definedge,firstock,flattrade,fyers,groww,ibulls,iifl,indmoney,kotak,motilal,paytm,pocketful,shoonya,tradejini,upstox,wisdom,zebu,zerodha' | ||
| VALID_BROKERS = 'mstock,fivepaisa,fivepaisaxts,aliceblue,angel,compositedge,dhan,dhan_sandbox,definedge,firstock,flattrade,fyers,groww,ibulls,iifl,indmoney,kotak,motilal,paytm,pocketful,shoonya,tradejini,upstox,wisdom,zebu,zerodha,mstock' |
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.
VALID_BROKERS now contains mstock twice; remove the duplicate to keep the documented broker list consistent and avoid redundant entries when the value is split.
Prompt for AI agents
Address the following comment on .sample.env at line 22:
<comment>VALID_BROKERS now contains mstock twice; remove the duplicate to keep the documented broker list consistent and avoid redundant entries when the value is split.</comment>
<file context>
@@ -19,7 +19,7 @@ REDIRECT_URL = 'http://127.0.0.1:5000/<broker>/callback' # Change if different
# Valid Brokers Configuration
-VALID_BROKERS = 'fivepaisa,fivepaisaxts,aliceblue,angel,compositedge,dhan,dhan_sandbox,definedge,firstock,flattrade,fyers,groww,ibulls,iifl,indmoney,kotak,motilal,paytm,pocketful,shoonya,tradejini,upstox,wisdom,zebu,zerodha,mstock'
+VALID_BROKERS = 'mstock,fivepaisa,fivepaisaxts,aliceblue,angel,compositedge,dhan,dhan_sandbox,definedge,firstock,flattrade,fyers,groww,ibulls,iifl,indmoney,kotak,motilal,paytm,pocketful,shoonya,tradejini,upstox,wisdom,zebu,zerodha,mstock'
# mstock Configuration
</file context>
| VALID_BROKERS = 'mstock,fivepaisa,fivepaisaxts,aliceblue,angel,compositedge,dhan,dhan_sandbox,definedge,firstock,flattrade,fyers,groww,ibulls,iifl,indmoney,kotak,motilal,paytm,pocketful,shoonya,tradejini,upstox,wisdom,zebu,zerodha,mstock' | |
| VALID_BROKERS = 'mstock,fivepaisa,fivepaisaxts,aliceblue,angel,compositedge,dhan,dhan_sandbox,definedge,firstock,flattrade,fyers,groww,ibulls,iifl,indmoney,kotak,motilal,paytm,pocketful,shoonya,tradejini,upstox,wisdom,zebu,zerodha' |
| from broker.mstock.mapping.order_data import transform_positions_data, transform_holdings_data | ||
|
|
||
| def get_positions(auth_token): | ||
| api_key = os.getenv('BROKER_API_KEY') |
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 line calls os.getenv but the module no longer imports os, so get_positions (and get_holdings) will raise NameError at runtime.
Prompt for AI agents
Address the following comment on broker/mstock/api/data.py at line 5:
<comment>This line calls os.getenv but the module no longer imports os, so get_positions (and get_holdings) will raise NameError at runtime.</comment>
<file context>
@@ -1,70 +1,46 @@
-def get_positions(api_key, auth_token):
+def get_positions(auth_token):
+ api_key = os.getenv('BROKER_API_KEY')
"""
Retrieves the user's positions.
</file context>
| formatted_value = "0.00" | ||
| filtered_data[openalgo_key] = formatted_value | ||
|
|
||
| logger.info(f"filteredMargin Data: {filtered_data}") |
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.
Avoid logging the full funds payload because it exposes sensitive balances in plain text. Replace it with a non-sensitive message or remove the log entirely.
Prompt for AI agents
Address the following comment on broker/mstock/api/funds.py at line 53:
<comment>Avoid logging the full funds payload because it exposes sensitive balances in plain text. Replace it with a non-sensitive message or remove the log entirely.</comment>
<file context>
@@ -0,0 +1,67 @@
+ formatted_value = "0.00"
+ filtered_data[openalgo_key] = formatted_value
+
+ logger.info(f"filteredMargin Data: {filtered_data}")
+ return filtered_data
+
</file context>
| logger.info(f"filteredMargin Data: {filtered_data}") | |
| logger.info("Filtered margin data fetched successfully.") |
| totalprofitandloss = 0 | ||
| totalpnlpercentage = 0 | ||
|
|
||
| if 'data' in holdings_data and 'total_holding' in holdings_data['data']: |
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.
Guard against holdings_data['data'] being None before checking for total_holding; otherwise this crashes on null API responses.
Prompt for AI agents
Address the following comment on broker/mstock/mapping/order_data.py at line 195:
<comment>Guard against `holdings_data['data']` being None before checking for `total_holding`; otherwise this crashes on null API responses.</comment>
<file context>
@@ -0,0 +1,207 @@
+ totalprofitandloss = 0
+ totalpnlpercentage = 0
+
+ if 'data' in holdings_data and 'total_holding' in holdings_data['data']:
+ total_holding = holdings_data['data']['total_holding']
+ totalholdingvalue = total_holding.get('total_holding_value', 0)
</file context>
| if 'data' in holdings_data and 'total_holding' in holdings_data['data']: | |
| if holdings_data.get('data') and 'total_holding' in holdings_data['data']: |
Summary by cubic
Added mstock broker support with TOTP login, order and portfolio APIs, and a WebSocket adapter for real-time data. This lets users trade and fetch positions/holdings/funds via OpenAlgo using mstock.
New Features
Migration
Written for commit de46167. Summary will update automatically on new commits.