Fix trade not executing issues

[Faouzijedidi1]

User description

Description

Fix trade issues

---

PR Type

Bug fix, Tests

---

Description

• Refactored TradeService to use ExchangeInitService for initializing exchanges, improving modularity and error handling.
• Enhanced unit tests for TradeService to cover market and limit trades, as well as order cancellation scenarios.
• Added ExchangeInitService to the providers in TradeModule.

---

Changes walkthrough 📝

	Relevant files
Enhancement	trade.module.ts Include ExchangeInitService in Trade module providers        server/src/modules/trade/trade.module.ts - Added `ExchangeInitService` to providers. +2/-1
Tests	trade.service.spec.ts Enhance trade service tests with exchange and cancellation server/src/modules/trade/trade.service.spec.ts Added tests for ExchangeInitService. Refactored tests for market and limit trades. Added tests for order cancellation. +156/-93
Bug fix	trade.service.ts Refactor trade service to use ExchangeInitService                server/src/modules/trade/trade.service.ts Refactored to use ExchangeInitService for exchange initialization. Simplified exchange retrieval in trade methods. Added error handling for missing exchange configuration. +16/-40

---

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mr-market	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 16, 2024 1:16pm

[railway-app[bot]]

🚅 Previously deployed to Railway in the quiet-desk project. Environment has been deleted.

[github-actions[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling The method `getExchange` in `TradeService` throws an `InternalServerErrorException` if an exchange is not configured. This exception is generic and might not provide clear information about the actual issue. Consider throwing a more specific exception or adding more detailed error messages. Test Completeness The tests for `executeMarketTrade` and `executeLimitTrade` methods should also verify that the `updateTradeStatus` method of `TradeRepository` is called with the correct parameters after successful order creation.

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Validate inputs in the cancelOrder function to prevent invalid operations ___ **To enhance the security and reliability of the cancelOrder function, validate the inputs orderId and symbol to ensure they are not empty or invalid types before attempting to cancel the order.** [server/src/modules/trade/trade.service.ts [147]](https://github.com/Hu-Fi/Mr.Market/pull/212/files#diff-4432350be63f9e2463145381a3748ee74fcf5656391b0269244b96a3bd55bd3eR147-R147) ```diff async cancelOrder(orderId: string, symbol: string): Promise { + if (!orderId || !symbol) { + throw new BadRequestException('Invalid orderId or symbol.'); + } ``` Suggestion importance[1-10]: 10 Why: Validating inputs in the `cancelOrder` function is crucial for preventing invalid operations and enhancing security and reliability.	10
Error handling	Add error handling for unexpected responses from createOrder ___ **Consider adding error handling for the case when exchange.createOrder might return null or an unexpected value. This will prevent potential runtime errors and make the function more robust.** [server/src/modules/trade/trade.service.ts [76]](https://github.com/Hu-Fi/Mr.Market/pull/212/files#diff-4432350be63f9e2463145381a3748ee74fcf5656391b0269244b96a3bd55bd3eR76-R76) ```diff const order = await this.exchange.createOrder( +if (!order) { + throw new InternalServerErrorException('Failed to create order due to unexpected response.'); +} ``` Suggestion importance[1-10]: 9 Why: Adding error handling for unexpected responses from `createOrder` is a good practice to prevent runtime errors and improve the robustness of the function.	9
	Add error handling for the updateTradeStatus method to manage exceptions ___ **To avoid potential issues with unhandled promise rejections, consider adding a catch block for the updateTradeStatus method call within the cancelOrder function.** [server/src/modules/trade/trade.service.ts [151]](https://github.com/Hu-Fi/Mr.Market/pull/212/files#diff-4432350be63f9e2463145381a3748ee74fcf5656391b0269244b96a3bd55bd3eR151-R151) ```diff -await this.tradeRepository.updateTradeStatus(orderId, 'cancelled'); +await this.tradeRepository.updateTradeStatus(orderId, 'cancelled').catch(error => { + this.logger.error(`Failed to update trade status: ${error.message}`); + throw new InternalServerErrorException('Failed to update trade status.'); +}); ``` Suggestion importance[1-10]: 8 Why: Adding error handling for the `updateTradeStatus` method is a good practice to manage exceptions and avoid unhandled promise rejections.	8