New Components - oanda #16542
New Components - oanda #16542
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
|
""" WalkthroughThis update introduces a comprehensive OANDA integration with new actions, sources, utility modules, and supporting constants. Three new actions allow users to create orders, list trades, and fetch historical prices from OANDA accounts. Three new polling sources emit events for new open trades, new transactions, and new open positions, each with sample event data for testing. A shared base module for sources is added, providing state management and event emission utilities. The OANDA app implementation is expanded with property definitions and methods for interacting with the OANDA REST API. A constants module centralizes enumerations and mappings for order types, trade states, and other OANDA-specific options. The package metadata is updated with a new version and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action/Source
participant OANDA App
participant OANDA API
User->>Action/Source: Configure and trigger
Action/Source->>OANDA App: Call API method (e.g., createOrder, listTrades)
OANDA App->>OANDA API: Send HTTP request
OANDA API-->>OANDA App: Respond with data
OANDA App-->>Action/Source: Return processed result
Action/Source-->>User: Emit event or return action result
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/oanda/actions/get-historical-prices/get-historical-prices.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (10)
components/oanda/sources/new-transaction/test-event.mjs (1)
1-14: Test event looks good but uses a future date.The transaction test event structure is appropriate with all the necessary fields for testing the new transaction source. However, note that the
timefield is set to a future date (2025-04-30).While this works fine as test data, you might consider using a past or present date to avoid potential confusion for users testing the component.
components/oanda/sources/new-open-trade/test-event.mjs (1)
1-15: Test event structure is appropriate but uses a future date.The open trade test event includes all necessary fields with appropriate sample values. However, the
openTimefield is set to a future date (2025-05-01).While acceptable for test data, consider using a past or present date to avoid potential confusion for users testing the component.
components/oanda/actions/list-trades/list-trades.mjs (1)
47-52: Check prop name consistency with API parameter.The prop is named
beforeIdbut it's passed asbeforeID(with capital "ID") in the params object (line 63). While this might be intentional to match the API's expected parameter name, consider being consistent in your naming convention or adding a comment to clarify this difference.- beforeId: { + beforeID: { type: "string", - label: "Before ID", + label: "Before ID", description: "The maximum Trade ID to return. If not provided the most recent Trades in the Account are returned.", optional: true, },components/oanda/actions/create-order/create-order.mjs (1)
115-128: Consider improving property handling logicThe
additionalPropsmethod directly modifies the existing props object. While this works, it can lead to unexpected behavior if properties are accessed during initialization.Consider this alternative approach that returns new property definitions instead of modifying existing ones:
additionalProps(props) { if (!this.type) { return {}; } const fields = constants.REQUIRED_FIELDS_FOR_ORDER_TYPES[this.type]; + const additionalProps = {}; for (const [ key, value, ] of Object.entries(fields)) { - props[key].hidden = false; - props[key].optional = !value; + additionalProps[key] = { + ...props[key], + hidden: false, + optional: !value, + }; } - return {}; + return additionalProps; },components/oanda/sources/new-open-position/new-open-position.mjs (1)
27-34: Hash collision risk with md5Using md5 hash of the stringified position object might lead to collisions if the position object contains floating-point numbers or properties that can appear in different orders.
Consider using a more reliable identifier if available from the API:
generateMeta(item) { - const hash = md5(JSON.stringify(item)); + // Use position ID if available, otherwise fallback to hash + const id = item.position?.id || + `${item.instrument}_${item.long.units}_${item.short.units}_${Date.now()}`; return { - id: hash, + id, summary: `New or Updated Position for instrument ${item.instrument}`, ts: Date.now(), }; },components/oanda/sources/new-open-trade/new-open-trade.mjs (1)
14-40: Optimize processing with array methodsThe filtering and iteration can be streamlined using array methods for more concise code.
Consider refactoring to use more modern array methods:
async processEvent(max) { const lastId = this._getLastId(); - let count = 0; const { trades, lastTransactionID, } = await this.oanda.listOpenTrades({ isDemo: this.isDemo, accountId: this.accountId, }); if (!trades.length) { return; } - const newTrades = trades.filter(({ id }) => id > lastId); - - for (const trade of newTrades.reverse()) { - this.emitItem(trade); - count++; - if (max && count >= max) { - break; - } - } + const newTrades = trades + .filter(({ id }) => id > lastId) + .sort((a, b) => b.id - a.id) // Sort in descending order + .slice(0, max || Infinity); // Limit the number of trades if max is specified + + newTrades.forEach(trade => this.emitItem(trade)); this._setLastId(lastTransactionID); },components/oanda/sources/common/base.mjs (2)
42-46: Fix typo in error messageThere's a typo in the error message for the
processEventmethod.processEvent() { - throw new Error("processEvent is not iplemented"); + throw new Error("processEvent is not implemented"); },
30-47: Add namespace to state keysThe state management methods use generic keys that might conflict across different component instances.
Consider namespacing the database keys to prevent potential conflicts:
methods: { + _getDbKey(key) { + return `${this.key}_${key}`; + }, _getLastId() { - return this.db.get("lastId") || 1; + return this.db.get(this._getDbKey("lastId")) || 1; }, _setLastId(lastId) { - this.db.set("lastId", lastId); + this.db.set(this._getDbKey("lastId"), lastId); }, emitItem(item) { const meta = this.generateMeta(item); this.$emit(item, meta); }, processEvent() { throw new Error("processEvent is not implemented"); }, generateMeta() { throw new Error("generateMeta is not implemented"); }, },components/oanda/common/constants.mjs (1)
181-181: Correct misspelling of "Thursday".There's a typo in the
WEEKLY_ALIGNMENT_DAYSarray where "Thursday" is misspelled as "Thrusday".Fix the spelling:
- "Thrusday", + "Thursday",components/oanda/oanda.app.mjs (1)
105-112: Add method documentation for listTransactions.The
listTransactionsmethod would benefit from additional documentation to clarify the expected parameters, especially since it's being used for pagination in thenew-transactionsource.Add JSDoc comments to clarify parameter usage:
/** + * List transactions for a specified account ID within an ID range + * @param {Object} opts - The options object + * @param {String} opts.accountId - The account ID to list transactions for + * @param {Object} opts.params - The query parameters + * @param {Number} opts.params.from - The starting transaction ID + * @param {Number} opts.params.to - The ending transaction ID + * @param {String} opts.params.type - Optional filter for transaction type + * @param {boolean} opts.isDemo - Whether to use the demo/practice API + * @returns {Promise<Object>} The transaction data + */ listTransactions({ accountId, ...opts }) { return this._makeRequest({ path: `/accounts/${accountId}/transactions/idrange`, ...opts, }); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
components/oanda/actions/create-order/create-order.mjs(1 hunks)components/oanda/actions/get-historical-prices/get-historical-prices.mjs(1 hunks)components/oanda/actions/list-trades/list-trades.mjs(1 hunks)components/oanda/common/constants.mjs(1 hunks)components/oanda/oanda.app.mjs(1 hunks)components/oanda/package.json(2 hunks)components/oanda/sources/common/base.mjs(1 hunks)components/oanda/sources/new-open-position/new-open-position.mjs(1 hunks)components/oanda/sources/new-open-position/test-event.mjs(1 hunks)components/oanda/sources/new-open-trade/new-open-trade.mjs(1 hunks)components/oanda/sources/new-open-trade/test-event.mjs(1 hunks)components/oanda/sources/new-transaction/new-transaction.mjs(1 hunks)components/oanda/sources/new-transaction/test-event.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (9)
components/oanda/package.json (2)
3-3: Version bump appropriately reflects new functionality.The version increment from 0.6.0 to 0.7.0 correctly follows semantic versioning for the new OANDA integration features (new actions, sources, and utility modules).
16-17: Dependencies updated to support new functionality.The @pipedream/platform update and new md5 dependency are appropriate for the new OANDA integration features. The md5 package is likely needed for generating unique identifiers or hashing in the new components.
components/oanda/sources/new-open-position/test-event.mjs (1)
1-36: Well-structured test event with comprehensive position data.The open position test event provides a detailed representation of a trading position with both long and short sides, trade IDs, and profit/loss values. The structure aligns well with what would be expected from the OANDA API.
components/oanda/actions/list-trades/list-trades.mjs (3)
4-9: Action metadata is well-defined.The action's key, name, description, version, and type are all properly defined with a helpful documentation link.
10-53: Props are well-defined with appropriate descriptions.The props configuration is well-structured:
- Using propDefinitions from the OANDA app for consistency
- Using constants for the trade states
- Including optional filters with clear descriptions
- Setting appropriate dependencies between props (account selection depends on demo flag)
54-70: Well-implemented run method with proper response handling.The run method correctly:
- Calls the OANDA API with appropriate parameters
- Exports a summary message with proper pluralization
- Returns the complete response object
The implementation provides a clean interface for listing trades with flexible filtering options.
components/oanda/common/constants.mjs (1)
1-445: Well-structured constants file with comprehensive options.The constants file is well-organized and provides a comprehensive set of options, types, and required fields that will be useful throughout the integration. The structure of providing both values and human-readable labels for options is particularly helpful for UI display.
components/oanda/oanda.app.mjs (2)
1-131: Well-designed API client with comprehensive methods.The OANDA app implementation provides a clean, well-structured API client with appropriate methods for interacting with the OANDA REST API. The property definitions with dynamic options are particularly valuable for creating intuitive user interfaces.
16-22: 🛠️ Refactor suggestionHandle empty accounts response gracefully.
In the
accountIdproperty'soptionsmethod, there's a possibility thataccountscould be undefined or null, which could cause errors.Improve null handling:
async options({ isDemo }) { const { accounts } = await this.listAccounts({ isDemo, }); - return accounts?.map(({ id }) => id) || []; + if (!accounts || !Array.isArray(accounts)) { + console.warn("No accounts found or unexpected response format"); + return []; + } + return accounts.map(({ id }) => id); },Likely an incorrect or invalid review comment.
components/oanda/actions/get-historical-prices/get-historical-prices.mjs
Outdated
Show resolved
Hide resolved
components/oanda/actions/get-historical-prices/get-historical-prices.mjs
Show resolved
Hide resolved
components/oanda/actions/get-historical-prices/get-historical-prices.mjs
Show resolved
Hide resolved
Resolves #15149
Summary by CodeRabbit