-
Notifications
You must be signed in to change notification settings - Fork 26
Replace tick stream with tick history #501
Replace tick stream with tick history #501
Conversation
@qingweibinary Review please? |
src/_actions/WatchlistActions.js
Outdated
if (isSubscribed) { | ||
api.subscribeToTick(symbol); | ||
try { | ||
await api.getTickHistory(symbol, { end: 'latest', count: 10, subscribe: 1 }); |
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.
we probably need adjust_start_time: 1
api.getTradingTimes(new Date()); | ||
}; | ||
|
||
export const getTickHistory = async (symbol) => { |
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 export looks really random, how about this?
we place this logic in action creator, then dispatch action from LiveData, which is the common pattern we have been using.
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.
@qingweibinary i don't think thats necessary. The method does change a state in any way beside making a call to an WS. HAd it been we are updating or changing a state example let's say logging history or something then would have made sense. I think it should be ok like this just like how we handle the one above it.
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.
OK
Replace tick stream with tick history for watchlist.
https://trello.com/c/CQUfIUva/2-handle-the-exception-this-market-is-presently-closed