Copier / only show actively copied traders

[aum-deriv]

Test Link

[review-deriv]

AI Code Review

⚠️ IMPORTANT: This review is AI-generated and should be validated by human reviewers
🔴 BUGS & LOGICAL ISSUES:

1. Missing error-handling logic for the “copy_start” request in AddTraderForm
   • Description: The form calls sendRequest({ copy_start: traderData.token }) but does not check for a successful server response before calling onAddTrader and clearing the input. If the copy_start call fails on the backend, the UI still assumes success.
   • Potential impacts: Users may be misled into thinking copying started when it actually did not. This can cause inconsistent states in the dashboard and confusion for end users.
   • Reproduction scenario:
   1) Enter an invalid token in AddTraderForm.
   2) submit, and see that the UI automatically considers the trader “added.”
   3) Meanwhile, the server might reject the token.
   4) After refreshing, the newly added token does not appear in the actual list.
   • Fix (example):
   const handleSubmit = async (e) => {
   e.preventDefault();
   const response = await sendRequest({ copy_start: traderData.token });
   if (response.error) {
   // Show or log error to user
   return;
   }
   onAddTrader?.(traderData);
   setTraderData({ token: "", isCopying: false });
   };

2. Incomplete “remove trader” behavior in CopierDashboard
   • Description: handleRemoveTrader calls setSnackbar and refreshList but does not send a request to remove the trader server-side or confirm that the user is no longer copying that trader. This may result in an out-of-sync UI if the backend keeps the trader in its records.
   • Potential impacts: The user’s UI might show the trader removed, but the server list still includes it. Refresh could bring the trader back, causing confusion.
   • Reproduction scenario:
   1) Add a trader via token.
   2) Click the remove button (assuming you have such a flow).
   3) The local UI no longer shows the trader, but the server is still tracking them.
   • Fix (example):
   const handleRemoveTrader = async (trader) => {
   // Send request to remove the trader on the server side
   const response = await sendRequest({ copytrading_remove: trader.token });
   if (response.error) {
   setSnackbar({
   isVisible: true,
   message: Error removing ${trader.name}: ${response.error.message},
   status: "negative",
   });
   return;
   }

   setSnackbar({
     isVisible: true,
     message: `Removed ${trader.name}`,
     status: "neutral",
   });
   refreshList();
   

   };

3. TraderCard always displaying "Stop Copying" without a “Start Copying” button
   • Description: The new TraderCard code removes the Start Copying flow and only shows a Stop Copying button. If a trader is not yet copying, the button text is misleading.
   • Potential impacts: Users cannot re-initiate a copy from the card, creating confusion about the trader’s actual copying status.
   • Reproduction scenario:
   1) Load the dashboard with a trader that has never been copied.
   2) The card shows “Stop Copying” even though copying was never started.
   • Fix (example):
   const TraderCard = ({ trader, onCopyClick, onStopCopy }) => {
   const isCopying = trader.isCopying;
   return (
   <>
   {isCopying ? (
   <Button onClick={() => onStopCopy(trader)}>Stop Copying
   ) : (
   <Button onClick={() => onCopyClick(trader)}>Start Copying
   )}
   </>
   );
   };

🟡 RELIABILITY CONCERNS:

1. Duplicates and invalid tokens in AddTraderForm
   • Edge case: A user might submit the same token multiple times, or an invalid token, with no user feedback.
   • Potential failure scenario: The server silently rejects duplicates or invalid tokens, but the UI remains unaware.
   • Mitigation:
   • Validate token format client-side if possible.
   • Check server responses for errors.
   • Provide immediate feedback to the user.

   Example:
   if (!traderData.token.trim()) {
   // Show error: "Token is required!"
   return;
   }
   // Then send request and handle response

2. Potential stale refresh states due to asynchronous server responses
   • Edge case: The refreshList call may not reflect a newly added or removed trader if the server hasn’t updated fully.
   • Potential failure scenario: The UI shows outdated lists if the request is still being processed on the server side.
   • Mitigation:
   • Consider waiting for a success response from the server’s copy_start or copy_stop before refreshing.
   • Use extra states (e.g., status: 'loading', 'success', 'error') for more granular UI feedback.

   Example:
   async function addNewTrader(token) {
   setIsLoading(true);
   const resp = await sendRequest({ copy_start: token });
   if (resp.error) {
   setError(resp.error.message);
   setIsLoading(false);
   return;
   }
   refreshList();
   }

💡 ROBUSTNESS IMPROVEMENTS:

1. Enhanced error handling in AddTraderForm
   • Suggestion: Display meaningful UI feedback any time a server call fails.
   • Code example:
   const handleSubmit = async (e) => {
   e.preventDefault();
   setSubmitting(true);
   try {
   const response = await sendRequest({ copy_start: traderData.token });
   if (response.error) {
   setError(response.error.message);
   return;
   }
   onAddTrader?.(traderData);
   setTraderData({ token: "", isCopying: false });
   } finally {
   setSubmitting(false);
   }
   };

2. Input validation for token
   • Suggestion: Validate whether the token meets certain length/format requirements before sending the request.
   • Code example:
   if (!/^[A-Za-z0-9]{10,}$/.test(traderData.token)) {
   setError("Invalid token format");
   return;
   }

3. Clearer state management for isCopying
   • Suggestion: Maintain explicit isCopying states in sync with the server’s response for each trader. Ensure the UI (TraderCard) toggles appropriately.
   • Code example:
   function handleCopyClick(trader) {
   setProcessingTrader(trader);
   sendRequest({ copy_start: trader.token }).then((response) => {
   if (!response.error) {
   setCopiedTrader(trader);
   }
   refreshList();
   });
   }

4. Confirm removal from server
   • Suggestion: For handleRemoveTrader, call an actual remove API (copytrading_remove) to keep local UI consistent.
   • Code example:
   const handleRemoveTrader = async (trader) => {
   const response = await sendRequest({ copytrading_remove: trader.token });
   if (!response.error) {
   refreshList();
   } else {
   setError(response.error.message);
   }
   };

[review-deriv]

AI Code Review

⚠️ IMPORTANT: This review is AI-generated and should be validated by human reviewers
🔴 BUGS & LOGICAL ISSUES:

1. Mismatch in trader.token usage from server response

   • Description: The code now relies on trader.token for stopping copy trading (copy_stop), but the server response (copytrading_list.traders) may not always include a token field. The original localStorage logic has been removed, yet the UI still references trader.token when calling onStopCopy.
   • Potential impacts: If the server does not return token data for traders, clicking “Stop Copying” could trigger undefined parameter errors or fail silently on the server.
   • Reproduction scenarios:
     • Add a trader via the new AddTraderForm, which only provides a token (no ID or name).
     • If the server’s copytrading_list endpoint returns traders objects without a token field, any subsequent “Stop Copying” request might fail.
   • Fix implementation with code example:
     // Ensure the backend returns a token or adjust the front-end to map the correct property
     // For example, add a fallback check:
     const handleStopCopy = (trader) => {
     if (!trader.token) {
     console.warn('No token available to stop copy trading.');
     return;
     }
     // Proceed with sendRequest({ copy_stop: trader.token })
     };

2. Name field usage leads to “Removed undefined” messages

   • Description: The handleRemoveTrader function references trader.name in the success message, but AddTraderForm no longer accepts or stores name. This leads to potential undefined references in UI messages.
   • Potential impacts: Confusing user experience when removing a trader, or errors if the code strictly expects a string.
   • Reproduction scenarios:
     • Add a trader with only token.
     • Remove that trader, which triggers setSnackbar showing “Removed undefined”.
   • Fix implementation with code example:
     // Provide a fallback for trader.name:
     setSnackbar({
     isVisible: true,
     message: Removed ${trader.name || 'Unknown Trader'},
     status: 'neutral',
     });

3. Always visible “Stop Copying” button

   • Description: TraderCard always shows the “Stop Copying” button, even if the user has not actually started copying. The isCopying state was removed, so there is no condition to hide it for traders who are not being copied.
   • Potential impacts: Users can click “Stop Copying” prematurely, causing unnecessary API calls or outright errors if the server expects an active copy session.
   • Reproduction scenarios:
     • Load the dashboard with fresh data.
     • Traders who were never started have the “Stop Copying” button by default.
   • Fix implementation with code example:
     // In TraderCard, conditionally render the “Stop Copying” button only if copying is active:
     {isCopying && (
     
     Stop Copying
     
     )}

4. onAddTrader invoked without ensuring correct server acknowledgement

   • Description: The AddTraderForm calls onAddTrader immediately after receiving a positive copy_start response, but it does not validate if the server truly lists the new trader in the next copytrading_list result.
   • Potential impacts: The parent tries to re-fetch or refresh, but UI states might become out of sync if the server fails to store the new token properly.
   • Reproduction scenarios:
     • Start a copy session for a token.
     • The server fails to persist it properly, but the form sets “Successfully started copy trading.”
   • Fix implementation with code example:
     // Validate the presence of the newly added trader in wsResponse before calling onAddTrader:
     if (!wsResponse.error && wsResponse.copy_start) {
     if (/* logic verifying new trader is recognized server-side */) {
     onAddTrader?.(traderData);
     }
     }

5. Unused isCopying property in AddTraderForm

   • Description: The traderData still initializes and resets isCopying to false, yet no code references it anymore.
   • Potential impacts: Minor code maintainability issue; can cause developer confusion about intended functionality.
   • Reproduction scenarios: Inspected in code; no direct usage remains.
   • Fix implementation with code example:
     // Remove isCopying from state if no longer used:
     const [traderData, setTraderData] = useState({
     token: "",
     });
     // Or reintroduce the isCopying logic in the UI if desired.

🟡 RELIABILITY CONCERNS:

1. Duplicate token submissions

   • Edge cases identified: If a user submits the same token multiple times, the server might handle it gracefully, or it might cause an error. Currently, no front-end check prevents duplicates.
   • Potential failure scenarios: Repeated calls to copy_start for the same token could result in server errors or unexpected copies.
   • Mitigation steps with code examples:
     // Before calling sendRequest, verify token does not already exist in the copyTraders list:
     if (apiTraders.some((t) => t.token === traderData.token)) {
     setSnackbar({ isVisible: true, message: "Token already copied", status: "fail" });
     return;
     }
     sendRequest({ copy_start: traderData.token });

2. Handling offline or server timeout

   • Edge cases identified: If the network drops after clicking “Start Copying,” isProcessing is set to true, but no fallback or retry logic is present.
   • Potential failure scenarios: The UI might remain stuck in a “processing” state if the server never responds or if wsResponse is indefinitely delayed.
   • Mitigation steps with code examples:
     // In handleSubmit or an effect, implement a fallback timer:
     useEffect(() => {
     let timeout;
     if (isProcessing) {
     timeout = setTimeout(() => {
     setIsProcessing(false);
     setSnackbar({ isVisible: true, message: "Request timeout", status: "fail" });
     }, 10000); // 10 seconds
     }
     return () => clearTimeout(timeout);
     }, [isProcessing]);

💡 ROBUSTNESS IMPROVEMENTS:

1. Provide clearer error handling for server-side issues

   • Error handling enhancements: Currently, the code checks wsResponse.error but only shows generic messages. Including HTTP codes or more descriptive errors from the server would help debugging.
   • Code example:
     if (wsResponse.error) {
     setSnackbar({
     isVisible: true,
     message: Error [${wsResponse.error.code}]: ${wsResponse.error.message},
     status: "fail",
     });
     }

2. Validate token format on front-end

   • Input validation additions: If the token must match a specific pattern (length, characters, etc.), ensure the UI enforces that before sending the request.
   • Code example:
     const handleSubmit = (e) => {
     e.preventDefault();
     if (!/^[A-Za-z0-9_-]{10,}$/.test(traderData.token)) {
     setSnackbar({ isVisible: true, message: "Invalid token format", status: "fail" });
     return;
     }
     // proceed with sending request
     };

3. Conditional rendering for “Stop Copying” vs “Start Copying” states

   • State management improvements: Re-introduce a field or a server-based check indicating whether the user is actively copying a given trader, and change the button label accordingly.
   • Code example:
     // On the server response, if “is_copying” is returned for each trader:
     {trader.is_copying ? (
     <Button onClick={() => onStopCopy(trader)}>Stop Copying
     ) : (
     <Button onClick={() => onCopyClick(trader)}>Start Copying
     )}

4. Remove reliance on local variables no longer in use

   • Code cleanup: Eliminate references to name or id in forms or states if the server no longer uses them.
   • Code example:
     // In AddTraderForm, remove extraneous form fields and associated states:
     // (Already removed, but verify no references remain elsewhere in the code.)

By addressing these issues—particularly the reliance on trader.token from the server response, clarifying the copying state, and adding stronger validation—you can significantly improve the overall robustness and correctness of the copy-trading feature.