-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[Components] etermin #13290 #18415
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] etermin #13290 #18415
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds an eTermin integration: two new Automations actions (Create Contact, Create Voucher), shared constants, expanded app propDefinitions and HTTP helpers (base URL, request wrapper with auth headers), API wrappers for contacts/vouchers, and a package version/dependency bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant W as Workflow
participant AC as CreateContact Action
participant APP as eTermin App
participant API as eTermin API
W->>AC: run({ $ })
AC->>APP: createContact({ updatewhenexistsgdt, salutation, title, lastname, firstname, company, birthday, email })
APP->>APP: _makeRequest(method: POST, path: /contact, body)
note right of APP: Inject auth headers\n(publickey, salt, signature)
APP->>API: POST /contact
API-->>APP: 201 Created + payload
APP-->>AC: Response
AC-->>W: $summary + response
sequenceDiagram
autonumber
participant W as Workflow
participant AV as CreateVoucher Action
participant APP as eTermin App
participant API as eTermin API
W->>AV: run({ $ })
AV->>APP: createVoucher({ id, description, priceinclvat, priceoriginal, ispercentage, customeremail, validfrom, validuntil })
APP->>APP: _makeRequest(method: POST, path: /voucher, body)
note right of APP: Inject auth headers\n(publickey, salt, signature)
APP->>API: POST /voucher
API-->>APP: 201 Created + payload
APP-->>AV: Response
AV-->>W: $summary + response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
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: 8
🧹 Nitpick comments (5)
components/etermin/package.json (1)
3-3: Version bump: add changelog and release notes.Please update CHANGELOG and PR “WHY” to explain the 0.7.0 changes and any migration notes.
components/etermin/common/constants.mjs (1)
1-6: Type mismatch: prop expects string, options are numbers.
updateWhenExistsgdtistype: "string"in the app, but options are numeric. Use string values to avoid implicit coercion.Apply this diff:
export default { UPDATE_OPTIONS: [ - 1, - 2, + "1", + "2", ], };components/etermin/actions/create-voucher/create-voucher.mjs (1)
9-53: Optional: align prop casing with app for consistency.Here we expose
priceInclVat,isPercentage, etc., consistent with app. Good. Consider documenting units/format (currency, date format).components/etermin/actions/create-contact/create-contact.mjs (1)
29-40: Optional: align prop keys with app’s camelCase.Consider renaming
lastname->lastName,firstname->firstNamefor consistency with app propDefinitions, or keep lowercase but be consistent in usage.components/etermin/etermin.app.mjs (1)
59-63: Consider numeric type (and units) for price fields
priceInclVatasstringcan cause accidental string concatenation in actions. Prefer number and document the currency/units (e.g., cents vs. major units).Apply:
- priceInclVat: { - type: "string", + priceInclVat: { + type: "integer", label: "Price Including VAT", - description: "Price including VAT of the voucher", + description: "Price including VAT of the voucher (in cents).", },
📜 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/etermin/actions/create-contact/create-contact.mjs(1 hunks)components/etermin/actions/create-voucher/create-voucher.mjs(1 hunks)components/etermin/common/constants.mjs(1 hunks)components/etermin/etermin.app.mjs(1 hunks)components/etermin/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-30T15:24:39.294Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Applied to files:
components/etermin/actions/create-contact/create-contact.mjs
🧬 Code graph analysis (2)
components/etermin/actions/create-contact/create-contact.mjs (1)
components/etermin/actions/create-voucher/create-voucher.mjs (1)
response(55-67)
components/etermin/actions/create-voucher/create-voucher.mjs (1)
components/etermin/actions/create-contact/create-contact.mjs (1)
response(61-73)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/etermin/package.json (1)
16-16: Verify @pipedream/platform ^3.1.0 compatibility.Confirm no breaking changes in axios wrapper or auth handling between ^3.0.0 and ^3.1.0 for these actions. Run a quick e2e on both actions.
components/etermin/actions/create-contact/create-contact.mjs (1)
60-73: PII in query string; send body as JSON instead.Switch POST payload from
paramstodatato avoid leaking PII in URLs.Apply this diff:
- const response = await this.app.createContact({ - $, - params: { + const response = await this.app.createContact({ + $, + data: { updatewhenexistsgdt: this.updateWhenExistsgdt, salutation: this.salutation, title: this.title, lastname: this.lastname, firstname: this.firstname, company: this.company, birthday: this.birthday, email: this.email, }, });If the API only accepts query params, confirm and document that decision due to PII implications.
components/etermin/etermin.app.mjs (1)
1-2: constants default export verified — leave import unchangedcomponents/etermin/common/constants.mjs exports a default object containing UPDATE_OPTIONS; the current
import constants from "./common/constants.mjs"is correct.
| updateWhenExistsgdt: { | ||
| type: "string", | ||
| label: "Update When Exists", | ||
| description: "Set to 1 to check if the contact is already existing. If so the existing contact gets updated. Needs email or firstname, lastname and birthday", | ||
| options: constants.UPDATE_OPTIONS, | ||
| }, |
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.
🧩 Analysis chain
Typo in prop key: “updateWhenExistsgdt” likely unintended; also tighten copy
The key name looks misspelled and will leak into action props. Rename to updateWhenExists and adjust description.
Apply:
- updateWhenExistsgdt: {
+ updateWhenExists: {
type: "string",
label: "Update When Exists",
- description: "Set to 1 to check if the contact is already existing. If so the existing contact gets updated. Needs email or firstname, lastname and birthday",
+ description: "Set to 1 to check if the contact already exists. If found, the existing contact is updated. Requires email, or first name + last name + birthday.",
options: constants.UPDATE_OPTIONS,
},If actions reference the old key, update them accordingly:
🏁 Script executed:
#!/bin/bash
# Find usages of the old key in eTermin actions
rg -nC2 '\bupdateWhenExistsgdt\b' --type=ts --type=js --type=jsx --type=tsx components/eterminLength of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== rg search for 'updateWhenExistsgdt' =="
if command -v rg >/dev/null 2>&1; then
rg -nC2 'updateWhenExistsgdt' || true
else
echo "rg not found; falling back to grep"
grep -R -n --line-number -C2 'updateWhenExistsgdt' . || true
fi
echo "== rg search for 'updateWhenExists' =="
if command -v rg >/dev/null 2>&1; then
rg -nC2 '\bupdateWhenExists\b' -S || true
else
grep -R -n --line-number -C2 'updateWhenExists' . || true
fi
echo "== list components/etermin files =="
if [ -d components/etermin ]; then
find components/etermin -maxdepth 2 -type f -print || true
if [ -f components/etermin/etermin.app.mjs ]; then
echo "== head of components/etermin/etermin.app.mjs =="
sed -n '1,200p' components/etermin/etermin.app.mjs || true
else
echo "components/etermin/etermin.app.mjs not found"
fi
else
echo "components/etermin directory not found"
fiLength of output: 4820
Rename prop key to updateWhenExists and update action usages
- Fix prop definition in components/etermin/etermin.app.mjs — rename key and tighten description (apply diff below).
- updateWhenExistsgdt: {
+ updateWhenExists: {
type: "string",
label: "Update When Exists",
- description: "Set to 1 to check if the contact is already existing. If so the existing contact gets updated. Needs email or firstname, lastname and birthday",
+ description: "Set to 1 to check if the contact already exists. If found, the existing contact is updated. Requires email, or first name + last name + birthday.",
options: constants.UPDATE_OPTIONS,
},- Update components/etermin/actions/create-contact/create-contact.mjs: change the propDefinition reference from
"updateWhenExistsgdt"→"updateWhenExists"and replace any use ofthis.updateWhenExistsgdtwiththis.updateWhenExists(leave the outgoing API param keyupdatewhenexistsgdtif that matches the API).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| updateWhenExistsgdt: { | |
| type: "string", | |
| label: "Update When Exists", | |
| description: "Set to 1 to check if the contact is already existing. If so the existing contact gets updated. Needs email or firstname, lastname and birthday", | |
| options: constants.UPDATE_OPTIONS, | |
| }, | |
| updateWhenExists: { | |
| type: "string", | |
| label: "Update When Exists", | |
| description: "Set to 1 to check if the contact already exists. If found, the existing contact is updated. Requires email, or first name + last name + birthday.", | |
| options: constants.UPDATE_OPTIONS, | |
| }, |
🤖 Prompt for AI Agents
In components/etermin/etermin.app.mjs around lines 8-13 the prop key is
misspelled as "updateWhenExistsgdt"; rename the prop key to "updateWhenExists"
and tighten the description accordingly (keep type, label, options but update
description text). Then in
components/etermin/actions/create-contact/create-contact.mjs update the
propDefinition reference from "updateWhenExistsgdt" to "updateWhenExists" and
replace any usages of this.updateWhenExistsgdt with this.updateWhenExists while
leaving the outgoing API parameter key (e.g., "updatewhenexistsgdt") unchanged
if required by the API.
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.
Agree with coderabbit. Need to fix the prop description.
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.
🧩 Analysis chain
@michelle0927 Thanks for confirming! Let me check for all references to the misspelled prop and provide the complete fix.
Based on the search results and the need to fix the prop key, here's the complete fix:
1. Fix the prop definition in components/etermin/etermin.app.mjs:
updateWhenExists: {
type: "string",
label: "Update When Exists",
description: "Set to 1 to check if the contact already exists. If found, the existing contact is updated. Requires email, or first name + last name + birthday.",
options: constants.UPDATE_OPTIONS,
},2. Update the action file components/etermin/actions/create-contact/create-contact.mjs:
- Change the propDefinition reference from
"updateWhenExistsgdt"to"updateWhenExists" - Update any usage of
this.updateWhenExistsgdttothis.updateWhenExists - Keep the API parameter key as
updatewhenexistsgdtif that's what the eTermin API expects
This ensures consistency across the codebase while maintaining API compatibility.
🏁 Script executed:
#!/bin/bash
# Find all references to the misspelled prop name
echo "=== Searching for 'updateWhenExistsgdt' references ==="
rg -n "updateWhenExistsgdt" components/etermin/
echo -e "\n=== Searching for any variations ==="
rg -n -i "updatewhenexists" components/etermin/
echo -e "\n=== List all eTermin action files ==="
find components/etermin/actions -name "*.mjs" -type f 2>/dev/null || echo "No action files found"
Length of output: 991
Perfect! I can see exactly where the misspelled prop is used. Here's the complete fix for all affected files:
1. Fix in components/etermin/etermin.app.mjs (Line 8):
updateWhenExists: {
type: "string",
label: "Update When Exists",
description: "Set to 1 to check if the contact already exists. If found, the existing contact is updated. Requires email, or first name + last name + birthday.",
options: constants.UPDATE_OPTIONS,
},2. Fix in components/etermin/actions/create-contact/create-contact.mjs:
You'll need to update 3 places in this file:
- Line 11: Change the prop name
updateWhenExists: {- Line 14: Update the propDefinition reference
"updateWhenExists",- Line 64: Update the property access (but keep the API parameter key)
updatewhenexistsgdt: this.updateWhenExists,This maintains API compatibility by keeping the parameter key updatewhenexistsgdt that the eTermin API expects, while fixing the internal prop naming throughout the codebase.
The create-voucher action doesn't use this prop, so no changes are needed there.
🧠 Learnings used
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
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.
@michelle0927 Thank you for confirming! The fix is already outlined in the previous comment. The misspelled prop key updateWhenExistsgdt should be renamed to updateWhenExists in both:
components/etermin/etermin.app.mjs- Update the prop definition and improve the descriptioncomponents/etermin/actions/create-contact/create-contact.mjs- Update the propDefinition reference and property access
The API parameter key updatewhenexistsgdt can remain unchanged to maintain compatibility with the eTermin API.
🧠 Learnings used
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#14229
File: components/americommerce/actions/update-customer/update-customer.mjs:89-94
Timestamp: 2024-10-08T16:42:59.225Z
Learning: When defining boolean properties in AmeriCommerce components (e.g., in `update-customer.mjs`), ensure that the label and description are consistent and clearly indicate the intent, especially when using negations like "No Account", to avoid confusion.
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: 5
🧹 Nitpick comments (3)
components/etermin/actions/create-contact/create-contact.mjs (1)
29-39: Optional: Align prop keys with app propDefinitions for consistency.Rename
lastname→lastName,firstname→firstNameto mirror app fields and avoid future mapping errors.- lastname: { + lastName: { propDefinition: [ app, "lastName", ], }, - firstname: { + firstName: { propDefinition: [ app, "firstName", ], },And update
runaccordingly.components/etermin/actions/create-voucher/create-voucher.mjs (1)
6-6: Doc link OK; consider adding required/optional notes.If API enforces required fields (e.g., id or price), reflect that in prop descriptions.
components/etermin/etermin.app.mjs (1)
59-83: Fix typos & verify voucher field types.File: components/etermin/etermin.app.mjs (lines 59–83)
- Typos: "vaucher" → "voucher"; "offer" → "voucher".
- Type suggestion: prefer number for priceInclVat (allow decimals/cents) and boolean for isPercentage — eTermin support shows vouchers can be absolute amounts (e.g. "10" → €10) or percentages (e.g. "50%"), but official API field types weren't found.
- Action: provide the exact API endpoint or a sample request body so we can confirm whether priceInclVat must be integer (cents) or decimal and whether isPercentage is 0/1 or true/false.
- description: "Indicates if the value of the vaucher is a percentage", + description: "Indicates if the value of the voucher is a percentage", @@ - description: "Email address of the customer receiving the offer", + description: "Email address of the customer receiving the voucher",
📜 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/etermin/actions/create-contact/create-contact.mjs(1 hunks)components/etermin/actions/create-voucher/create-voucher.mjs(1 hunks)components/etermin/common/constants.mjs(1 hunks)components/etermin/etermin.app.mjs(1 hunks)components/etermin/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-30T15:24:39.294Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14467
File: components/gainsight_px/actions/create-account/create-account.mjs:4-6
Timestamp: 2024-10-30T15:24:39.294Z
Learning: In `components/gainsight_px/actions/create-account/create-account.mjs`, the action name should be "Create Account" instead of "Create Memory".
Applied to files:
components/etermin/actions/create-contact/create-contact.mjs
🧬 Code graph analysis (2)
components/etermin/actions/create-voucher/create-voucher.mjs (1)
components/etermin/actions/create-contact/create-contact.mjs (1)
response(61-73)
components/etermin/actions/create-contact/create-contact.mjs (1)
components/etermin/actions/create-voucher/create-voucher.mjs (1)
response(55-67)
🔇 Additional comments (3)
components/etermin/package.json (1)
3-3: Version bump OK — confirm @pipedream/platform ^3.1.0 compatibility.Repository uses the platform axios helper widely (e.g. packages/prompts/src/index.ts imports axios from "@pipedream/platform"; implementation in platform/lib/axios.ts) — cannot auto-verify v3.1 compatibility. Run CI/tests and confirm v3.1 does not change the axios helper signature or defineComponent/export behavior before merging.
components/etermin/etermin.app.mjs (1)
89-106: POST payload must be application/x-www-form-urlencoded; confirm no POSTs useparamseTermin requires POST fields as form-encoded and headers "publickey", "salt", "signature". _makeRequest sets those headers, but I could not locate POST callers using
paramsunder components/etermin — change POST requests to send the payload in axiosdataas form-encoded (e.g., qs.stringify) and set Content-Type: application/x-www-form-urlencoded if any callers currently pass payload viaparams. Location: components/etermin/etermin.app.mjs (lines 89–106).components/etermin/common/constants.mjs (1)
2-5: Change UPDATE_OPTIONS to labeled "0"/"1" string values and verify eTermin parameter/valuesNo public eTermin docs were found for the parameter "updatewhenexistsgdt" — cannot confirm allowed values; apply the change below only after verifying the parameter name and that it accepts 0/1 (strings).
File: components/etermin/common/constants.mjs (lines 2-5)
export default { - UPDATE_OPTIONS: [ - 1, - 2, - ], + UPDATE_OPTIONS: [ + { label: "No (0) — don't update existing", value: "0" }, + { label: "Yes (1) — update if exists", value: "1" }, + ], };
| updateWhenExistsgdt: { | ||
| type: "string", | ||
| label: "Update When Exists", | ||
| description: "Set to 1 to check if the contact is already existing. If so the existing contact gets updated. Needs email or firstname, lastname and birthday", | ||
| options: constants.UPDATE_OPTIONS, | ||
| }, |
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.
Agree with coderabbit. Need to fix the prop description.
|
Hi @lcaresia, one more comment: Remember to add annotations to the new components. Slack context |
michelle0927
left a 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.
Please address the requested changes before moving to QA.
|
Hey @michelle0927
This is the new id that user set, not an existing id.
The docs explicit require params. |
* Added actions * Added actions * Added actions * Added actions * Update components/etermin/etermin.app.mjs --------- Co-authored-by: michelle0927 <[email protected]>
WHY
Summary by CodeRabbit