-
Notifications
You must be signed in to change notification settings - Fork 5.5k
17095 components offorte #17141
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
17095 components offorte #17141
Conversation
Sources - New Event (Instant) Actions - Create Contact Organisation - Create Contact Person - Create Proposal - List Proposal Temaplates - List Proposals - List Users - Search Contact Organisation - Search Contact People - Send Proposal
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a comprehensive Offorte integration, adding new actions, utility modules, and a webhook-based event source. The changes implement API interactions for contacts, proposals, users, templates, and events, providing both CRUD operations and search capabilities. Utility and constants modules support data handling and option standardization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant OfforteApp
participant OfforteAPI
User->>Action: Provide input parameters
Action->>OfforteApp: Call relevant method with inputs
OfforteApp->>OfforteAPI: Make HTTP request
OfforteAPI-->>OfforteApp: Return API response
OfforteApp-->>Action: Return processed data
Action-->>User: Output summary and data
sequenceDiagram
participant OfforteAPI
participant WebhookSource
participant PD_HTTP
participant PD_DB
WebhookSource->>PD_HTTP: Register webhook endpoint
WebhookSource->>OfforteAPI: Create webhook with events
OfforteAPI-->>WebhookSource: Return webhook ID
WebhookSource->>PD_DB: Store webhook ID
OfforteAPI-->>WebhookSource: Send event to endpoint
WebhookSource->>PD_DB: Retrieve webhook ID
WebhookSource-->>PD_HTTP: Emit event with summary and timestamp
WebhookSource->>OfforteAPI: Delete webhook on deactivate
Assessment against linked issues
Suggested labels
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/offorte/actions/get-contact-details/get-contact-details.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 (
|
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: 6
🔭 Outside diff range comments (1)
components/offorte/package.json (1)
3-17: Pin platform dependency to an exact minor to avoid future breakageUsing a caret (
^3.1.0) means any 3.x release will be installed. Pipedream platform updates sometimes introduce breaking changes; consider pinning to~3.1.0(patch-level) or an exact version until the integration is proven compatible.- "@pipedream/platform": "^3.1.0" + "@pipedream/platform": "~3.1.0"
♻️ Duplicate comments (3)
components/offorte/actions/list-proposals/list-proposals.mjs (1)
20-26: Handle non-array API responses as with templates actionSimilar to the template listing action, guard against the API returning an object envelope.
- $.export("$summary", `Successfully fetched ${response.length} proposals`); + const count = Array.isArray(response) ? response.length : response?.data?.length ?? 0; + $.export("$summary", `Successfully fetched ${count} proposals`);components/offorte/actions/create-contact-person/create-contact-person.mjs (1)
140-155: Same array-vs-parseObject concern fortagsSee previous comments – using
parseObjecton an already-array input will throw.components/offorte/actions/search-contact-organisation/search-contact-organisation.mjs (1)
17-26: Same response-shape concern as contact people searchMirrors the previous comment – double-check the API returns an array.
🧹 Nitpick comments (13)
components/offorte/sources/new-event-instant/test-event.mjs (1)
1-5: Confirm epoch accuracy and add newlineThe hard-coded timestamp (
1718534400) represents seconds, whereas other integrations sometimes expect milliseconds. Double-check the downstream consumer ofts; if milliseconds are required, multiply by1_000.Also add a trailing newline to adhere to POSIX text-file conventions.
components/offorte/package.json (1)
12-14: Add missing fields for public NPM packages
repository,license, andenginesare currently absent. Including them improves discoverability and compliance with open-source requirements.components/offorte/actions/list-proposals/list-proposals.mjs (1)
13-17: Expose default / required value forstatuspropIf
statusis optional at the API level, consider addingoptional: trueor setting a sensible default; otherwise mark itrequired: trueto guide users in the UI.components/offorte/actions/create-contact-organisation/create-contact-organisation.mjs (1)
122-149: Minor: many address fields are required by default
street,zipcode,city,state,countryare most likely optional in Offorte.
Not marking them withoptional: trueforces the UI to ask for unnecessary data and blocks valid requests.components/offorte/actions/create-proposal/create-proposal.mjs (1)
61-72:statusis mandatory but undocumented as requiredUnlike other optional props,
statuslacksoptional: true, making it compulsory in the UI even though the API accepts a default (concept). Either mark it optional or document the requirement explicitly.components/offorte/actions/create-contact-person/create-contact-person.mjs (1)
1-3: Shadowing the importedofforteidentifierYou import
offorte(the app) and later destructureoffortefromthis, shadowing the original binding. While it works, it decreases readability and may trip IDE tooling. Recommend renaming the destructured variable:-const { - offorte, +const { + offorte: offorteClient,components/offorte/sources/new-event-instant/new-event-instant.mjs (2)
25-35: Consider persisting the full webhook objectOnly the
webhook_idis stored. Persisting the full response (or at least the secret) aids debugging and later updates.
41-46: Use numeric timestamp notDate.parse
Date.parsereturns NaN on invalid input and assumes UTC.
Ifbody.date_createdis already ISO-8601, convert withnew Date(...).
Otherwise expose parsing failures.- ts: Date.parse(body.date_created), + ts: new Date(body.date_created).getTime(),components/offorte/common/constants.mjs (1)
1-116: Consider freezing exported option arrays to prevent accidental runtime mutationThese option arrays are intended to be treated as enums. Freezing them (e.g.
export const STATUS_OPTIONS = Object.freeze([...])) makes accidental writes during runtime impossible and signals immutability to future maintainers.-export const STATUS_OPTIONS = [ +export const STATUS_OPTIONS = Object.freeze([ { label: "Edit", value: "edit" }, ... -]; +]);Replicate for the other two arrays.
This is a low-effort, forward-looking hardening step.
components/offorte/common/utils.mjs (1)
1-24:parseObjectsilently swallows JSON.parse errors – log or bubble?If malformed JSON is passed, the current implementation eats the exception and returns the original string, making debugging hard. At minimum, consider
console.debug-level logging so integrators can trace bad input.components/offorte/actions/send-proposal/send-proposal.mjs (1)
59-61: Error message typo – align prop nameThe thrown message uses back-ticks around
Send Message Id, but the prop isSend Message ID(sendMessageId). Consistency helps users map UI → error.components/offorte/offorte.app.mjs (2)
33-41: Consider returning consistent option format for tags.The
tagsprop returns an array of strings (names only), while other prop definitions return objects withlabelandvalueproperties. For consistency, consider returning the same format.tags: { type: "string[]", label: "Tags", description: "The tags of the contact", async options() { const response = await this.listTags(); - return response.map(({ name }) => name); + return response.map(({ name }) => ({ + label: name, + value: name, + })); }, },
226-233: Remove unnecessary trailing slash in the API path.The path includes a trailing slash which might cause issues or be redundant.
listProposals({ status, ...opts }) { return this._makeRequest({ - path: `/proposals/${status}/`, + path: `/proposals/${status}`, ...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 (16)
components/offorte/actions/create-contact-organisation/create-contact-organisation.mjs(1 hunks)components/offorte/actions/create-contact-person/create-contact-person.mjs(1 hunks)components/offorte/actions/create-proposal/create-proposal.mjs(1 hunks)components/offorte/actions/get-contact-details/get-contact-details.mjs(1 hunks)components/offorte/actions/list-proposal-templates/list-proposal-templates.mjs(1 hunks)components/offorte/actions/list-proposals/list-proposals.mjs(1 hunks)components/offorte/actions/list-users/list-users.mjs(1 hunks)components/offorte/actions/search-contact-organisation/search-contact-organisation.mjs(1 hunks)components/offorte/actions/search-contact-people/search-contact-people.mjs(1 hunks)components/offorte/actions/send-proposal/send-proposal.mjs(1 hunks)components/offorte/common/constants.mjs(1 hunks)components/offorte/common/utils.mjs(1 hunks)components/offorte/offorte.app.mjs(1 hunks)components/offorte/package.json(2 hunks)components/offorte/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/offorte/sources/new-event-instant/test-event.mjs(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
components/offorte/common/utils.mjs
[error] 36-36: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (7)
components/offorte/actions/list-proposal-templates/list-proposal-templates.mjs (1)
12-18: ```shell
#!/bin/bash
set -eecho "🔍 Displaying first 50 lines of components/offorte/offorte.app.mjs for import context:"
head -n 50 components/offorte/offorte.app.mjs</details> <details> <summary>components/offorte/actions/list-users/list-users.mjs (1)</summary> `12-18`: I’ll locate the implementation of `listUsers` to confirm its return shape. ```bash #!/bin/bash # Search for listUsers definitions and calls rg -n "listUsers(" -C3components/offorte/actions/create-contact-organisation/create-contact-organisation.mjs (1)
114-120: Let’s inspect theparseObjectimplementation in the Offorte utils:#!/bin/bash set -e file="components/offorte/common/utils.mjs" echo "→ File: $file" # Show parseObject definition with context grep -R -n "parseObject" -C5 "$file"components/offorte/actions/create-proposal/create-proposal.mjs (1)
111-113: ```shell
#!/bin/bash
set -eLocate offorte.app.mjs
file=$(fd offorte.app.mjs)
echo "offorte.app.mjs found at: $file"Display entire file content around createProposal
echo "-------- Entire offorte.app.mjs --------"
sed -n '1,300p' "$file"</details> <details> <summary>components/offorte/actions/search-contact-people/search-contact-people.mjs (1)</summary> `17-26`: **Verify response shape – assumes array with `.length`** `listContacts` may return `{ data: [...] }` (common pattern in this code-base). `response.length` will then be `undefined`, producing `"Successfully found undefined contact(s)"`. Please confirm the SDK returns a plain array; if not, adjust: ```js const { data } = response; $.export("$summary", `Successfully found ${data.length} contact(s)`); return data;components/offorte/offorte.app.mjs (2)
11-16: Verify thefieldIdparameter usage andfullnamefield casing.The
fieldIdparameter in the options function is not documented in the prop description. Additionally, verify that the API returnsfullname(all lowercase) rather thanfullName(camelCase).#!/bin/bash # Description: Search for usage of fieldId parameter and fullname field in the codebase # Check for fieldId usage in contactId prop rg -A 5 'fieldId.*=.*"id"' --type js # Check for fullname vs fullName usage rg -i 'fullname|fullName' --type js
130-134: Verify the authorization header format.The authorization header is set to just the API key value. Most APIs expect a specific format like
Bearer ${token}orToken ${token}.What is the correct authorization header format for the Offorte API?
…ails.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
….mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
jcortes
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.
Hi @luancazarine lgtm! Ready for QA!
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: 2
♻️ Duplicate comments (1)
components/offorte/offorte.app.mjs (1)
152-157:listUsers(and all list helpers) lack pagination and filtering*With the
_makeRequestfix you’ll now get an array, but large workspaces can exceed default limits.
Introducepage/page_sizeparameters or document their absence.This was raised in an earlier review and is still outstanding.
🧹 Nitpick comments (1)
components/offorte/offorte.app.mjs (1)
234-237: Trailing slash inlistProposalspath may break some HTTP clients
/proposals/${status}/produces a double “//” whenstatusis empty and an unnecessary trailing slash otherwise. Safe variant:-return this._makeRequest({ - path: `/proposals/${status}/`, +return this._makeRequest({ + path: `/proposals/${status}`,Minor, but avoids 301s/404s depending on the API router.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/offorte/actions/get-contact-details/get-contact-details.mjs(1 hunks)components/offorte/offorte.app.mjs(1 hunks)components/offorte/sources/new-event-instant/new-event-instant.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/offorte/actions/get-contact-details/get-contact-details.mjs
- components/offorte/sources/new-event-instant/new-event-instant.mjs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (1)
components/offorte/offorte.app.mjs (1)
135-139: Header casing and missing content-type
- HTTP headers are case-insensitive, but most APIs/documentation use
Authorization, notauthorization.- For JSON POST/PUT calls you should explicitly set
Content-Type: application/jsonto avoid surprises.-_headers() { - return { - "authorization": `${this.$auth.api_key}`, - }; +_headers() { + return { + "Authorization": this.$auth.api_key, + "Content-Type": "application/json", + }; }Please confirm Offorte’s auth scheme (Bearer vs. raw key) and adjust accordingly.
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 (1)
components/offorte/sources/new-event-instant/test-event.mjs (1)
1-1: Nit: add trailing newline at EOF for POSIX compatibilityWhile harmless at runtime, adding a terminal newline keeps git diff noise down and aligns with the repo’s style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/offorte/sources/new-event-instant/test-event.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (1)
components/offorte/sources/new-event-instant/test-event.mjs (1)
1-31: Sample looks correct – please just confirm it mirrors the real webhook payloadNothing functionally wrong here.
Since this object will be surfaced to users as the “Example Event”, double-check that:
- Field names / casing exactly match Offorte’s webhook (e.g.
"type"vs"event_type","date_created"precision, etc.).- Numeric identifiers are returned as strings in the live API (they’re currently quoted).
If the payload is confirmed, no further action needed. 👍
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: 1
♻️ Duplicate comments (2)
components/offorte/offorte.app.mjs (2)
11-17:response.mapstill explodes – option loader expects an array but receives the raw Axios response
this.listContacts()returns the full Axios response object, soresponse.mapthrowsTypeError: response.map is not a function.
Root cause is_makeRequestreturning the whole response instead ofresponse.data.
203-211: Returnresponse.dataand add basic error handling in_makeRequestWithout unwrapping the data, every consumer must remember to access
response.data, which is already causing runtime crashes in the option loaders.
Wrapping the call in atry/catchalso surfaces meaningful errors.- _makeRequest({ - $ = this, path, ...opts - }) { - return axios($, { - url: this._baseUrl() + path, - headers: this._headers(), - ...opts, - }); - }, + async _makeRequest({ + $ = this, path, ...opts + }) { + try { + const { data } = await axios($, { + url: this._baseUrl() + path, + headers: this._headers(), + ...opts, + }); + return data; + } catch (error) { + const msg = error.response + ? `${error.response.status} ${error.response.statusText}` + : error.message; + throw new Error(`Offorte API request failed: ${msg}`); + } + },
📜 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 (1)
components/offorte/offorte.app.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/offorte/offorte.app.mjs (1)
196-199: Verify header format – Offorte appears to expectAuthorization: Bearer <token>(orX-Api-Key)The header key is lowercase and the value is the raw API key. Double-check the API docs – many endpoints require either an
Authorization: Bearer …header or a customX-Api-Keyheader.
|
/approve |
jcortes
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.
Hi @luancazarine please fix gh action errors first!
|
/approve |
* [Components] offorte PipedreamHQ#17095 Sources - New Event (Instant) Actions - Create Contact Organisation - Create Contact Person - Create Proposal - List Proposal Temaplates - List Proposals - List Users - Search Contact Organisation - Search Contact People - Send Proposal * pnpm update * Update components/offorte/actions/get-contact-details/get-contact-details.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update components/offorte/sources/new-event-instant/new-event-instant.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add prop name * fix test-event file * some adjusts * pnpm update * pnpm update * Fix export key in get-contact-details action --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Resolves #17095
Summary by CodeRabbit