Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

Mahdiyeh/Fix: get trader tokens from copytrading_list api #1594

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mahdiyeh-deriv
Copy link
Contributor

No description provided.

@@ -11,7 +11,7 @@ import validateToken from 'websockets/validateToken';
import { init as instrumentPromise } from '../instruments/instruments';

// While using copy trader, this cannot be NULL
const getLoggedInUserId = () => local_storage.get("oauth")[0].id;
const getLoggedInUserId = local_storage.get("oauth")[0].id;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const getLoggedInUserId = local_storage.get("oauth")[0].id;
const loggedInUserId = local_storage.get("oauth")[0].id;

@@ -22,7 +22,7 @@ const form_error_messages = {
REFRESH_FAILED: 'Refresh failed'.i18n(),
};

const getStorageName = () => `copyTrade_${getLoggedInUserId()}`;
const getStorageName = () => `copyTrade_${getLoggedInUserId}`;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const getStorageName = () => `copyTrade_${getLoggedInUserId}`;
const getStorageName = () => `copyTrade_${loggedInUserId}`;

@@ -98,12 +98,16 @@ const refreshTraderStats = (loginid, token, scope) => {
//Check if we already added this trader. If yes, then merge the changes
if (traderTokenDetails) {
_.merge(traderTokenDetails.traderStatistics, copyStatData.copytrading_statistics);
//check if copy trading is started

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//check if copy trading is started
// check if copy trading is started

if (settings.get_settings.allow_copiers) {
state.is_loading = false;
} else {
//update trader tokens and Refresh locally stored trader statistics

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//update trader tokens and Refresh locally stored trader statistics
// update trader tokens and refresh locally stored trader statistics

Comment on lines 369 to 370
for (let traderToken of traderTokens) {
try {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we put the try/catch block just before the for loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we don't want to continue the loop even if an exception occurs during one iteration, the exception handler should be outside the loop, you're right.

Comment on lines 414 to 418
liveapi.events.on("logout", () => {
if (getStorageName() && local_storage.get(getStorageName())) {
local_storage.remove(getStorageName());
}
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
liveapi.events.on("logout", () => {
if (getStorageName() && local_storage.get(getStorageName())) {
local_storage.remove(getStorageName());
}
});
liveapi.events.on("logout", () => {
const storageName = getStorageName();
if (storageName && local_storage.get(storageName)) {
local_storage.remove(storageName);
}
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To have 1 function call instead of 3 :trollface:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants