- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.5k
[Components] finage #10893 #18442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Components] finage #10893 #18442
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
 | 
| WalkthroughAdds three new Finage forex actions (last quote, last trade, previous close), expands the Finage app with symbol propDefinition, HTTP helper, base URL, and specific forex methods, removes authKeys, and updates package.json with a version bump and a new dependency. Changes
 Sequence Diagram(s)sequenceDiagram
  autonumber
  actor User
  participant Action as Pipedream Action
  participant App as Finage App
  participant HTTP as Finage API (api.finage.co.uk)
  Note over User,Action: Trigger action with symbol input
  User->>Action: Run (symbol, $)
  Action->>App: Call forex method<br/>(lastQuote | lastTrade | previousClose)
  rect rgb(235, 245, 255)
    Note right of App: Build request via _makeRequest<br/>• Base URL<br/>• API key param<br/>• Path by method
    App->>HTTP: GET /last/forex/{symbol}<br/>or /last/trade/forex/{symbol}<br/>or /agg/forex/prev-close/{symbol}
    HTTP-->>App: 200 JSON response
  end
  App-->>Action: Return data
  Action-->>User: Result + $summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
 Pre-merge checks and finishing touches❌ Failed checks (1 warning)
 ✅ Passed checks (2 passed)
 ✨ Finishing touches
 🧪 Generate unit tests
 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. Comment  | 
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
components/finage/actions/forex-last-trade/forex-last-trade.mjs (1)
23-23: Use template literal for $summary (nit).- $.export("$summary", "Successfully retrieved the last trade for " + this.symbol); + $.export("$summary", `Successfully retrieved the last trade for ${this.symbol}`);components/finage/actions/forex-previous-close/forex-previous-close.mjs (1)
23-23: Use template literal for $summary (nit).- $.export("$summary", "Successfully retrieved the previous close for " + this.symbol); + $.export("$summary", `Successfully retrieved the previous close for ${this.symbol}`);components/finage/actions/forex-last-quote/forex-last-quote.mjs (1)
23-23: Use template literal for $summary (nit).- $.export("$summary", "Successfully retrieved the last quote for " + this.symbol); + $.export("$summary", `Successfully retrieved the last quote for ${this.symbol}`);components/finage/finage.app.mjs (2)
7-21: Polish symbol prop: label/description and option labels.Improve UX and resilient response handling.
propDefinitions: { symbol: { type: "string", - label: "symbol", - description: "Description for symbol", + label: "Symbol", + description: "Forex pair symbol (e.g., EURUSD).", async options() { - const response = await this.getSymbols(); - const symbols = response.symbols; - return symbols.map(({ - symbol, name, - }) => ({ - value: symbol, - label: name, - })); + const { symbols = [] } = await this.getSymbols(); + return symbols.map(({ symbol, name }) => ({ + value: symbol, + label: name ? `${symbol} — ${name}` : symbol, + })); }, }, },Please confirm the getSymbols response shape (top-level
symbolsarray withsymbolandnamefields).
47-51: URL-encode symbol in paths.Prevents breakage for symbols with special chars.
return this._makeRequest({ - path: `/last/forex/${symbol}`, + path: `/last/forex/${encodeURIComponent(symbol)}`, ...args, }); @@ return this._makeRequest({ - path: `/last/trade/forex/${symbol}`, + path: `/last/trade/forex/${encodeURIComponent(symbol)}`, ...args, }); @@ return this._makeRequest({ - path: `/agg/forex/prev-close/${symbol}`, + path: `/agg/forex/prev-close/${encodeURIComponent(symbol)}`, ...args, });Also applies to: 56-60, 65-69
📜 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 (5)
- components/finage/actions/forex-last-quote/forex-last-quote.mjs(1 hunks)
- components/finage/actions/forex-last-trade/forex-last-trade.mjs(1 hunks)
- components/finage/actions/forex-previous-close/forex-previous-close.mjs(1 hunks)
- components/finage/finage.app.mjs(1 hunks)
- components/finage/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
- components/finage/package.json
🧬 Code graph analysis (4)
components/finage/actions/forex-previous-close/forex-previous-close.mjs (2)
components/finage/actions/forex-last-quote/forex-last-quote.mjs (1)
response(19-22)components/finage/actions/forex-last-trade/forex-last-trade.mjs (1)
response(19-22)
components/finage/finage.app.mjs (3)
components/finage/actions/forex-last-quote/forex-last-quote.mjs (1)
response(19-22)components/finage/actions/forex-last-trade/forex-last-trade.mjs (1)
response(19-22)components/finage/actions/forex-previous-close/forex-previous-close.mjs (1)
response(19-22)
components/finage/actions/forex-last-trade/forex-last-trade.mjs (2)
components/finage/actions/forex-last-quote/forex-last-quote.mjs (1)
response(19-22)components/finage/actions/forex-previous-close/forex-previous-close.mjs (1)
response(19-22)
components/finage/actions/forex-last-quote/forex-last-quote.mjs (2)
components/finage/actions/forex-last-trade/forex-last-trade.mjs (1)
response(19-22)components/finage/actions/forex-previous-close/forex-previous-close.mjs (1)
response(19-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (8)
components/finage/package.json (2)
3-3: Version bump looks good.0.1.0 aligns with new public actions.
15-16: No change needed — @pipedream/platform ^3.1.0 is up-to-date and consistent.components/finage/package.json uses "@pipedream/platform": "^3.1.0"; many components already use ^3.1.0 and npm dist-tag "latest" = 3.1.0.
components/finage/actions/forex-last-trade/forex-last-trade.mjs (1)
3-17: Action wiring LGTM.Props, run flow, and summary export follow our conventions and app method usage.
Also applies to: 18-25
components/finage/actions/forex-previous-close/forex-previous-close.mjs (1)
3-17: Action wiring LGTM.Matches the app surface and other forex actions.
Also applies to: 18-25
components/finage/actions/forex-last-quote/forex-last-quote.mjs (1)
3-17: Action wiring LGTM.Endpoints delegated via app.forexLastQuote are consistent.
Also applies to: 18-25
components/finage/finage.app.mjs (3)
70-74: Verified: /symbol-list/forex endpoint and response shape are correct. Returns a top-level "symbols" array where each item includes "symbol" and "name" (matches Finage docs).
34-41: Confirmed: Finage expects "apikey" (lowercase) — no change required.
Docs use ?apikey=YOUR_API_KEY; current code already sendsapikey.
47-51: Endpoints match Finage docs — no change requiredFinage docs confirm these endpoints and official pages:
- GET /last/forex/{symbol}: https://finage.co.uk/docs/api/forex/forex-last-quote
- GET /last/trade/forex/{symbol}: https://finage.co.uk/docs/api/forex/forex-last-trade
- GET /agg/forex/prev-close/{symbol}: https://finage.co.uk/docs/api/forex/forex-previous-close
- GET /symbol-list/forex: https://finage.co.uk/docs/api/fundamentals/full-symbol-list-api (use type=forex query)
Applies to occurrences at lines 47–51, 56–60, 65–69, 70–74.
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.
Hi @lcaresia, LGTM! Ready for QA!
WHY
Summary by CodeRabbit