-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[Components] taggun #10912 #18508
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
base: master
Are you sure you want to change the base?
[Components] taggun #10912 #18508
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds three new Taggun actions (URL Data Extraction, Verbose URL Data Extraction, Submit Feedback), introduces shared language constants, and significantly expands the Taggun app with propDefinitions and helper methods for authenticated HTTP requests to Taggun API endpoints. Updates package version and adds a platform dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Action as Taggun Action
participant App as Taggun App
participant API as Taggun API
rect rgba(230,245,255,0.6)
note over User,Action: URL Data Extraction
User->>Action: Run "URL Data Extraction"
Action->>App: app.urlDataExtraction({ url, referenceId, refresh, near, language, incognito, $ })
App->>App: _makeRequest(POST /receipt/v1/simple/url, apikey)
App->>API: POST /receipt/v1/simple/url
API-->>App: Response
App-->>Action: Response
Action-->>User: Summary + Response
end
rect rgba(235,255,235,0.6)
note over User,Action: Verbose URL Data Extraction
User->>Action: Run "Verbose URL Data Extraction"
Action->>App: app.verboseUrlDataExtraction({...})
App->>API: POST /receipt/v1/verbose/url
API-->>App: Response
App-->>Action: Response
Action-->>User: Summary + Response
end
rect rgba(255,240,230,0.6)
note over User,Action: Submit Feedback
User->>Action: Run "Submit Feedback"
Action->>App: app.submitFeedback({ referenceId, totalAmount, taxAmount, merchantName, currencyCode, $ })
App->>API: POST /account/v1/feedback
API-->>App: Response
App-->>Action: Response
Action-->>User: Summary + Response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 4
🧹 Nitpick comments (9)
components/taggun/actions/url-data-extraction/url-data-extraction.mjs (1)
49-63
: Consider clarifying the summary message.Line 61: The summary says "Successfully sent the image for processing", but this action accepts a URL parameter, not an image directly. While the URL may point to an image, consider using "Successfully sent the URL for processing" or "Successfully submitted receipt URL for extraction" for clarity.
- $.export("$summary", "Successfully sent the image for processing"); + $.export("$summary", "Successfully submitted receipt URL for extraction");components/taggun/actions/verbose-url-data-extraction/verbose-url-data-extraction.mjs (1)
49-63
: Consider clarifying the summary message.Line 61: The summary says "Successfully sent the image for processing", but this action accepts a URL parameter, not an image directly. Consider using "Successfully submitted receipt URL for extraction" or similar for clarity.
- $.export("$summary", "Successfully sent the image for processing"); + $.export("$summary", "Successfully submitted receipt URL for extraction");components/taggun/taggun.app.mjs (7)
7-67
: Prop definitions: small UX/docs tweaks
- Clarify expected formats:
- Near: specify “lat,long” (e.g., “-36.8485,174.7633”) if that’s what Taggun expects.
- Currency code: mention “ISO 4217 (e.g., USD, EUR)”.
- Consider defaults to reduce friction: language "en", refresh false, incognito true (privacy-by-default).
Example diffs:
@@ language: { type: "string", label: "Language", description: "Preferred language for extracted text", - options: constants.LANGUAGE_OPTIONS, + options: constants.LANGUAGE_OPTIONS, + default: "en", optional: true, }, @@ incognito: { type: "boolean", label: "Incognito", - description: "Do not store or use the image for system training", - optional: true, + description: "Do not store or use the image for system training", + optional: true, + default: true, }, @@ near: { type: "string", label: "Near", - description: "Provide a nearby location to improve result accuracy", + description: "Nearby location to improve accuracy. Format: \"lat,long\" (e.g., \"-36.8485,174.7633\").", optional: true, }, @@ currencyCode: { type: "string", label: "Currency Code", - description: "The expected currency code from the user.", + description: "Expected currency code (ISO 4217, e.g., USD, EUR).", optional: true, },
69-71
: Allow base URL override for tests/sandboxingHelpful for mocks and regional endpoints.
-_baseUrl() { - return "https://api.taggun.io/api"; -}, +_baseUrl() { + return process.env.TAGGUN_BASE_URL || "https://api.taggun.io/api"; +},
89-95
: urlDataExtraction: convenience payload and guardOptionally accept named args and build the JSON body here so actions stay thin, and validate required
url
.-async urlDataExtraction(args = {}) { - return this._makeRequest({ - path: "/receipt/v1/simple/url", - method: "post", - ...args, - }); -}, +async urlDataExtraction({ + url, + referenceId, + refresh, + near, + language, + incognito, + totalAmount, + taxAmount, + merchantName, + currencyCode, + ...rest +} = {}) { + if (!url) throw new Error("url is required"); + return this._makeRequest({ + path: "/receipt/v1/simple/url", + method: "post", + data: { + url, + referenceId, + refresh, + near, + language, + incognito, + totalAmount, + taxAmount, + merchantName, + currencyCode, + }, + ...rest, + }); +},
96-102
: submitFeedback: define expected body shape and minimal validationDocument/guard required fields (e.g.,
referenceId
,score
and/ormessage
) to prevent 4xx.If you share the expected Taggun payload, I can wire the schema and validation.
103-109
: verboseUrlDataExtraction: mirror simple endpoint ergonomicsApply the same named-args pattern and validation for consistency.
-async verboseUrlDataExtraction(args = {}) { - return this._makeRequest({ - path: "/receipt/v1/verbose/url", - method: "post", - ...args, - }); -}, +async verboseUrlDataExtraction(params = {}) { + const { url } = params || {}; + if (!url) throw new Error("url is required"); + return this._makeRequest({ + path: "/receipt/v1/verbose/url", + method: "post", + data: params, + }); +},
82-86
: Header key casing and future expansion
apikey
works; consider centralizing headers via a helper (e.g.,_defaultHeaders()
) to add telemetry (User-Agent) later without touching call sites.+_defaultHeaders() { + return { + apikey: `${this.$auth.api_key}`, + "Content-Type": "application/json", + Accept: "application/json", + "User-Agent": "Pipedream-Taggun-Component", + }; +},Then spread
_defaultHeaders()
inside_makeRequest
.
31-36
: Amounts as strings vs numbersIf Taggun expects numeric values, consider
type: "integer"
/"string"
with parsing in methods, or document that strings should be parseable numbers.Also applies to: 37-42, 43-54
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
components/taggun/actions/submit-feedback/submit-feedback.mjs
(1 hunks)components/taggun/actions/url-data-extraction/url-data-extraction.mjs
(1 hunks)components/taggun/actions/verbose-url-data-extraction/verbose-url-data-extraction.mjs
(1 hunks)components/taggun/common/constants.mjs
(1 hunks)components/taggun/package.json
(2 hunks)components/taggun/taggun.app.mjs
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/taggun/actions/submit-feedback/submit-feedback.mjs (2)
components/taggun/actions/url-data-extraction/url-data-extraction.mjs (1)
response
(50-60)components/taggun/actions/verbose-url-data-extraction/verbose-url-data-extraction.mjs (1)
response
(50-60)
components/taggun/actions/verbose-url-data-extraction/verbose-url-data-extraction.mjs (1)
components/taggun/actions/url-data-extraction/url-data-extraction.mjs (1)
response
(50-60)
components/taggun/actions/url-data-extraction/url-data-extraction.mjs (2)
components/taggun/actions/submit-feedback/submit-feedback.mjs (1)
response
(43-52)components/taggun/actions/verbose-url-data-extraction/verbose-url-data-extraction.mjs (1)
response
(50-60)
⏰ 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). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (6)
components/taggun/package.json (2)
3-3
: LGTM! Appropriate version bump for new features.The version bump from 0.0.1 to 0.1.0 correctly reflects the addition of new actions and functionality.
15-16
: LGTM! Platform dependency added.The addition of the
@pipedream/platform
dependency is standard and necessary for Pipedream components.components/taggun/actions/url-data-extraction/url-data-extraction.mjs (1)
3-48
: LGTM! Action metadata and props are well-structured.The action correctly uses propDefinitions from the app for consistent prop configuration across actions.
components/taggun/actions/submit-feedback/submit-feedback.mjs (1)
9-56
: LGTM! Action structure is correct.The action correctly uses propDefinitions and constructs the data payload for the feedback submission.
components/taggun/actions/verbose-url-data-extraction/verbose-url-data-extraction.mjs (1)
3-48
: LGTM! Action metadata and props are well-structured.The action correctly uses propDefinitions from the app for consistent prop configuration.
components/taggun/taggun.app.mjs (1)
1-2
: No changes needed for constants import
The default export incomponents/taggun/common/constants.mjs
includesLANGUAGE_OPTIONS
, so usingconstants.LANGUAGE_OPTIONS
is correct.
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.
I made a couple of comments, please check them before we move forward
key: "taggun-verbose-url-data-extraction", | ||
name: "Verbose URL Data Extraction", |
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.
Can't "verbose" be a boolean option in the "Extract Data from URL" component, therefore eliminating the need for this separate component, since it seems to have the same props?
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.
Wasn't extract-data-from-url
meant to replace url-data-extraction
and verbose-url-data-extraction
? They're both still present, but I think they should be removed.
WHY
Summary by CodeRabbit