-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
[TRIGGER] New form submitted for Formaloo #15669
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
Warning Rate limit exceeded@jcortes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 54 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
WalkthroughThis update removes customer-related action modules and streamlines the Formaloo app by replacing multiple customer properties with a single form-centric property. New modules for API constants, webhook management, and various form event sources (payment completed, form submission, and row updates) have been added. Additionally, package updates include a version bump and new dependencies. The control flow now favors form handling and webhook-based processing over the previous customer operations. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Formaloo App
participant WH as Webhook Module
participant API as API Server
participant DB as Database
App->>WH: Call activate()
WH->>WH: Generate secret (using uuid)
WH->>API: createWebhook(formSlug, secret)
API-->>WH: Return webhook slug
WH->>DB: Store webhook slug and secret
WH-->>App: Return activation confirmation
sequenceDiagram
participant Source as Event Source
participant WH as Webhook Module
participant Validator as Token Validator
participant Processor as Event Processor
Source->>WH: Incoming event (run method)
WH->>Validator: Validate token against stored secret
Validator-->>WH: Token valid/invalid
alt Token Valid
WH->>WH: Generate metadata (generateMeta)
WH->>Processor: Emit event with metadata and resource
else Token Invalid
WH-->>Source: Respond with error/status 401
end
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
1a6fb5e
to
3deaa79
Compare
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🧹 Nitpick comments (16)
components/formaloo/sources/form-submitted-instant/test-event.mjs (3)
5-6
: Standardize phone number format and fix typo in test data.
- The phone number should follow the E.164 format (e.g., "+11234567890")
- There appears to be a typo in the name "Joh Doe" (missing 'n')
- "field_QkcERTTt": "+1677673423", - "field_tClRSgaU": "Joh Doe" + "field_QkcERTTt": "+11677673423", + "field_tClRSgaU": "John Doe"
10-11
: Consider using more descriptive field slugs in test data.The current field slugs (
field_tClRSgaU
,field_QkcERTTt
) are not descriptive of their purpose. Consider using more meaningful identifiers in test data for better readability.- "slug": "field_tClRSgaU", + "slug": "field_fullName", ... - "slug": "field_QkcERTTt", + "slug": "field_phoneNumber",Also applies to: 22-23
39-40
: Consider using more recent timestamps in test data.The test data uses timestamps from 2020. Consider updating to more recent dates to prevent confusion about the age of the test data.
- "created_at": "2020-11-19T14:57:43.796281+01:00", - "updated_at": "2020-11-19T14:57:43.796281+01:00", + "created_at": "2025-02-19T14:57:43.796281+01:00", + "updated_at": "2025-02-19T14:57:43.796281+01:00",components/formaloo/sources/form-payment-completed/test-event.mjs (4)
1-8
: Add documentation and type information.Consider the following improvements:
- Add JSDoc comments explaining the purpose of this test event file and its usage.
- Consider adding TypeScript type definitions for better type safety and IDE support.
- Add comments mapping field slugs to their human-readable names for better maintainability.
+/** + * Test event data for form payment completion webhook. + * Used for testing the form-payment-completed source component. + * + * Field mappings: + * - field_QkcERTTt: Phone Number + * - field_cfUkTmtq: Food Selection + * - field_tClRSgaU: Full Name + */ export default { "event_type": "row_payment", "event_id": "xEJsGn", "data": { "field_QkcERTTt": "+1677673423", // Phone Number "field_cfUkTmtq": "choice_kQqwcAXH", // Food Selection "field_tClRSgaU": "Joh Doe" // Full Name },
14-14
: Fix test data inconsistencies and add validation.
- The test name "Joh Doe" appears to be a typo and should be "John Doe".
- The food item price (15.0) in
delta_score.currency
should be validated against the final payment amount.- "value": "Joh Doe", + "value": "John Doe",Also applies to: 31-31, 39-48
57-58
: Standardize timestamp formats.The
created_at
andupdated_at
timestamps in choice items use an unconventional timezone offset (+04:30). Consider standardizing to UTC or a consistent timezone format across all timestamps.Also applies to: 67-68, 77-78
104-110
: Add missing fields to readable_data.The
readable_data
object is missing the "Select the food you want to buy" field that exists inrendered_data
. Consider adding all relevant fields for consistency."readable_data": { "Full Name": "Joh Doe", "Phone Number": "+1677673423", + "Select the food you want to buy": "Pizza", "Order Status": "Successful", "Order Code": "4908799754916322", "payment amount": 15.0 },
components/formaloo/formaloo.app.mjs (5)
7-24
: Ensure stable pagination and error handling in formSlug's options method.The new
formSlug
property and itsoptions
method look correct for retrieving forms. However, the current logic only fetches a single page of forms. If you foresee a large number of forms, consider implementing pagination to ensure all forms are retrieved. Also, guard against API errors to avoid runtime exceptions if any error occurs while fetching the form list.async options({ page }) { const { data: { forms } } = await this.listForms({ params: { page: page + 1, }, }); - return forms.map(({ slug: value, title: label }) => ({ - label: label || value, - value, - })); + if (!forms || !Array.isArray(forms)) { + return []; + } + const results = forms.map(({ slug: value, title: label }) => ({ + label: label || value, + value, + })); + // Optional: handle the logic to merge multiple pages here if needed + return results; }
27-28
: Unify getUrl with constants.
getUrl(path)
hardcodes the base URL instead of using a shared constant. Consider leveraging the newBASE_URL
andVERSION_PATH
fromconstants.mjs
for consistency and easy maintenance.- return `https://api.formaloo.net/v2.0${path}`; + import constants from "./common/constants.mjs"; + ... + return `${constants.BASE_URL}${constants.VERSION_PATH}${path}`;
45-50
: Consider clarifying the post method naming.While the
post
method is simple and functional, a more explicit name (e.g.,createResource
,postRequest
, etc.) can make the code self-explanatory, especially if more POST methods appear later.
51-56
: Consider clarifying the delete method naming.Similarly, the
delete
method is concise, but renaming it (e.g.,deleteResource
) can improve readability, especially as more resource-specific HTTP actions are added.
57-62
: Implement multi-page retrieval in listForms.
listForms
retrieves only a single page of data. If your users can have many forms, consider implementing a loop to fetch all pages until no further results are available.listForms(args = {}) { return this._makeRequest({ path: "/forms/", ...args, }); } +// Example pagination approach: +async listAllForms(args = {}) { + let page = 1; + const allForms = []; + while (true) { + const { data: { forms, nextPage } } = await this._makeRequest({ + path: "/forms/", + params: { page }, + ...args, + }); + if (!forms?.length) break; + allForms.push(...forms); + if (!nextPage) break; + page += 1; + } + return allForms; +}components/formaloo/sources/common/webhook.mjs (2)
21-64
: Consider adding error handling for webhook operations.The activate and deactivate hooks handle webhook lifecycle well, but could benefit from additional error handling:
- Response validation in activate hook
- Error handling for failed webhook operations
Consider applying this enhancement:
async activate() { const { http: { endpoint: url }, createWebhook, formSlug, setWebhookSlug, setSecret, getData, } = this; const secret = uuid(); + try { const response = await createWebhook({ formSlug, data: { url, secret, active: true, send_rendered_data: true, send_raw_data: true, ...getData(), }, }); + if (!response?.data?.webhook?.slug) { + throw new Error('Invalid webhook response'); + } setWebhookSlug(response?.data?.webhook?.slug); setSecret(secret); + } catch (error) { + console.error('Failed to activate webhook:', error); + throw error; + } }
106-122
: Consider enhancing webhook security.The webhook validation is basic. Consider adding these security enhancements:
- Timing-safe string comparison for token validation
- Request timestamp validation to prevent replay attacks
Consider applying this enhancement:
async run({ body, headers, }) { const secret = this.getSecret(); const token = headers["x-formaloo-token"]; + const timestamp = headers["x-formaloo-timestamp"]; + // Validate timestamp to prevent replay attacks + if (timestamp) { + const requestTime = new Date(timestamp).getTime(); + const currentTime = Date.now(); + if (Math.abs(currentTime - requestTime) > 300000) { // 5 minutes + throw new Error("Request timestamp is too old"); + } + } - if (token !== secret) { + // Use timing-safe comparison + if (!crypto.timingSafeEqual(Buffer.from(token), Buffer.from(secret))) { throw new Error("Invalid token"); } this.http.respond({ status: 200, body: "", }); this.processResource(body); }components/formaloo/sources/form-submitted-instant/form-submitted-instant.mjs (1)
1-26
: Implementation looks good but consider version bump.The implementation correctly extends the common webhook functionality and properly implements the required methods. However, since this is a new feature with webhook security enhancements, consider starting with version 0.1.0 instead of 0.0.1 to follow semantic versioning.
- version: "0.0.1", + version: "0.1.0",components/formaloo/sources/form-payment-completed/form-payment-completed.mjs (1)
1-26
: Maintain version consistency across modules.The implementation is correct, but for consistency with the form-submitted-instant module, consider using the same version number.
- version: "0.0.1", + version: "0.1.0",
🛑 Comments failed to post (3)
components/formaloo/sources/form-payment-completed/test-event.mjs (1)
111-112:
⚠️ Potential issueFix timestamp inconsistency.
The
updated_at
timestamp (Nov 13) is earlier than thecreated_at
timestamp (Nov 19), which is logically incorrect for test data."created_at": "2020-11-19T14:57:43.796281+01:00", - "updated_at": "2020-11-13T10:23:43.234323+01:00", + "updated_at": "2020-11-19T15:23:43.234323+01:00",📝 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."created_at": "2020-11-19T14:57:43.796281+01:00", "updated_at": "2020-11-19T15:23:43.234323+01:00",
components/formaloo/formaloo.app.mjs (1)
36-43: 💡 Verification agent
🧩 Analysis chain
Verify token refresh or error handling cases.
This
_makeRequest
method directly usesoauth_access_token
. If the token can expire, ensure there's a mechanism to refresh or handle 401 errors gracefully. Otherwise, users may encounter silent failures.
🏁 Script executed:
#!/bin/bash # Attempt to locate any token refresh or re-auth logic across the codebase rg -A 5 "refresh"Length of output: 58465
Action Required: Enhance Token Expiration Handling
The current
_makeRequest
implementation incomponents/formaloo/formaloo.app.mjs
directly uses the OAuth access token without any visible mechanism to handle token expiration errors. In contrast, our codebase shows various components implementing token refresh logic (via error handling, axios interceptors, or dedicated refresh methods). Please verify that either:
- A global error interceptor or external middleware is responsible for catching 401 responses and triggering a token refresh, or
- You add error handling logic within
_makeRequest
(or related methods) to gracefully refresh the expired token and retry the request.If a refresh mechanism exists elsewhere for Formaloo requests, please include an inline comment specifying this to avoid confusion. Otherwise, please implement appropriate token refresh/error handling to prevent any silent failures due to an expired token.
components/formaloo/common/constants.mjs (1)
1-5:
⚠️ Potential issueAvoid hardcoding secrets in code.
Defining
SECRET
directly in this file may pose security risks if it is used in production. Consider storing it in environment variables or using an authentication property in$auth
.-const SECRET = "secret"; +// Example of retrieving from an environment variable: +const SECRET = process.env.FORMALOO_SECRET;📝 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.const BASE_URL = "https://api.formaloo.net"; const VERSION_PATH = "/v2.0"; const WEBHOOK_SLUG = "webhookSlug"; // Example of retrieving from an environment variable: const SECRET = process.env.FORMALOO_SECRET;
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 (9)
components/formaloo/sources/form-submitted-instant/test-event.mjs (3)
1-4
: Consider updating test event timestamps and documenting identifier purposes.The test event uses hardcoded timestamps from 2020. Consider using more recent dates for testing or documenting why these specific dates are important for testing scenarios.
Also, the event contains multiple identifiers (
event_id
,submit_code
,slug
). Consider adding comments to clarify the purpose of each identifier.export default { + // Unique identifier for the event "event_type": "form_submit", "event_id": "xEJsGn", ... + // ISO 8601 timestamps for testing different scenarios - "created_at": "2020-11-19T14:57:43.796281+01:00", - "updated_at": "2020-11-19T14:57:43.796281+01:00", + "created_at": "2024-02-19T14:57:43.796281+01:00", + "updated_at": "2024-02-19T14:57:43.796281+01:00", + // Unique code for form submission "submit_code": "UZF9XpVgY4sr5zmF", + // Unique identifier for the form "slug": "ZNtjCuScAjt349Rf" };Also applies to: 39-43
4-7
: Improve test data quality and consider adding validation examples.The test data contains:
- A potential typo in the name "Joh Doe" (missing 'n')
- A phone number that might need format validation
Consider updating the test data to demonstrate proper validation handling.
"data": { - "field_QkcERTTt": "+1677673423", - "field_tClRSgaU": "Joh Doe" + "field_QkcERTTt": "+1-555-0123", + "field_tClRSgaU": "John Doe" },
8-34
: Ensure consistency in field constraints and remove unnecessary nulls.The rendered data structure has some inconsistencies:
max_length
is only present in the "Full Name" fieldmax_value
andmin_value
are only present in the "Phone Number" field- Several fields contain
null
values that might be unnecessaryConsider standardizing the field structure and removing unnecessary fields.
"rendered_data": [ { "slug": "field_tClRSgaU", "type": "short_text", "title": "Full Name", "value": "John Doe", - "json_key": null, "position": 1, "raw_value": "John Doe", "admin_only": false, - "json_value": null, "max_length": 255 }, { "slug": "field_QkcERTTt", "type": "short_text", "title": "Phone Number", "value": "+1-555-0123", - "json_key": null, "position": 2, - "max_value": null, - "min_value": null, "raw_value": "+1-555-0123", "admin_only": false, - "json_value": null, + "max_length": 20 } ],components/formaloo/formaloo.app.mjs (4)
7-24
: LGTM! Consider adding validation constraints.The property definition is well-structured with clear labeling and proper pagination in the options method. The fallback to slug when title is empty is a good defensive programming practice.
Consider adding validation constraints such as:
formSlug: { type: "string", label: "Form Slug", description: "The slug of the form you want to add tags to.", + required: true, + pattern: "^[a-z0-9-]+$", async options({ page }) {
27-29
: Consider making the base URL configurable.While the implementation is clean, hardcoding the base URL and API version could make it difficult to switch environments or handle API version updates.
Consider extracting these as constants or configuration:
+const BASE_URL = "https://api.formaloo.net"; +const API_VERSION = "v2.0"; + methods: { getUrl(path) { - return `https://api.formaloo.net/v2.0${path}`; + return `${BASE_URL}/${API_VERSION}${path}`; },
45-56
: Consider adding other HTTP methods and error handling.The implementation of
post
anddelete
methods is clean and consistent. However, the API could benefit from a complete set of HTTP methods and proper error handling.Consider adding:
- Other HTTP methods:
+get(args = {}) { + return this._makeRequest({ + method: "GET", + ...args, + }); +}, +put(args = {}) { + return this._makeRequest({ + method: "PUT", + ...args, + }); +},
- Error handling wrapper:
_makeRequest({ $ = this, path, ...opts }) { - return axios($, { - url: this.getUrl(path), - headers: this._getHeaders(), - ...opts, - }); + try { + return axios($, { + url: this.getUrl(path), + headers: this._getHeaders(), + ...opts, + }); + } catch (error) { + const status = error.response?.status; + const message = error.response?.data?.message || error.message; + throw new Error(`Request failed with status ${status}: ${message}`); + } },
57-62
: Add JSDoc documentation for the method parameters.While the implementation is clean, adding documentation would improve the API's usability.
Consider adding JSDoc:
+/** + * List available forms + * @param {Object} args - Request arguments + * @param {Object} [args.params] - Query parameters + * @param {number} [args.params.page] - Page number for pagination + * @returns {Promise<Object>} Response containing forms data + */ listForms(args = {}) { return this._makeRequest({ path: "/forms/", ...args, }); },components/formaloo/sources/row-updated-instant/test-event.mjs (2)
1-7
: Consider using non-PII data in test events.The test event contains what appears to be PII (phone number and full name). Consider using clearly fictional data for test events.
- "field_QkcERTTt": "+1677673423", - "field_tClRSgaU": "Joh Doe" + "field_QkcERTTt": "+1234567890", + "field_tClRSgaU": "Test User"
8-34
: Field definitions are well-structured but could use additional validation constraints.The field definitions are comprehensive, but consider adding:
- Input validation patterns for the phone number field
- Minimum length constraints for the name field
📜 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 (14)
components/formaloo/actions/common/utils.mjs
(0 hunks)components/formaloo/actions/create-customer/create-customer.mjs
(0 hunks)components/formaloo/actions/update-customer/update-customer.mjs
(0 hunks)components/formaloo/common/constants.mjs
(1 hunks)components/formaloo/formaloo.app.mjs
(1 hunks)components/formaloo/package.json
(2 hunks)components/formaloo/sources/common/webhook.mjs
(1 hunks)components/formaloo/sources/customer-created/customer-created.mjs
(0 hunks)components/formaloo/sources/form-payment-completed/form-payment-completed.mjs
(1 hunks)components/formaloo/sources/form-payment-completed/test-event.mjs
(1 hunks)components/formaloo/sources/form-submitted-instant/form-submitted-instant.mjs
(1 hunks)components/formaloo/sources/form-submitted-instant/test-event.mjs
(1 hunks)components/formaloo/sources/row-updated-instant/row-updated-instant.mjs
(1 hunks)components/formaloo/sources/row-updated-instant/test-event.mjs
(1 hunks)
💤 Files with no reviewable changes (4)
- components/formaloo/sources/customer-created/customer-created.mjs
- components/formaloo/actions/common/utils.mjs
- components/formaloo/actions/update-customer/update-customer.mjs
- components/formaloo/actions/create-customer/create-customer.mjs
🚧 Files skipped from review as they are similar to previous changes (6)
- components/formaloo/common/constants.mjs
- components/formaloo/sources/form-payment-completed/form-payment-completed.mjs
- components/formaloo/sources/form-submitted-instant/form-submitted-instant.mjs
- components/formaloo/package.json
- components/formaloo/sources/common/webhook.mjs
- components/formaloo/sources/form-payment-completed/test-event.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (5)
components/formaloo/sources/form-submitted-instant/test-event.mjs (1)
35-38
: Well-structured readable data mapping.The
readable_data
object provides a clean, human-readable way to access form field values using their titles as keys. This is a good practice for improving code readability and debugging.components/formaloo/formaloo.app.mjs (1)
36-44
: LGTM! Clean refactor of request handling.The changes maintain the method's functionality while improving URL construction through the new
getUrl
method.components/formaloo/sources/row-updated-instant/row-updated-instant.mjs (2)
11-25
: Methods implementation looks clean and follows best practices.The methods are well-structured:
- Properly extends common methods
getData()
clearly indicates row update event supportgenerateMeta()
provides comprehensive metadata including ID, summary, and timestamp
1-10
:❓ Verification inconclusive
Verify the documentation link and consider version number.
The implementation looks good, but there are a few points to consider:
- Ensure the documentation link is accessible and up-to-date
- Consider starting with version "1.0.0" instead of "0.0.1" if this is a stable implementation
🏁 Script executed:
#!/bin/bash # Verify the documentation link curl -I "https://help.formaloo.com/en/articles/8568748-how-formaloo-webhook-works"Length of output: 3949
Documentation Link Verified & Version Number Consideration
- The documentation URL is accessible (HTTP status 200), confirming it’s up-to-date.
- If this module is considered stable, please consider updating the version number from "0.0.1" to "1.0.0" to better reflect its maturity.
- The deduplication strategy and overall configuration appear solid.
components/formaloo/sources/row-updated-instant/test-event.mjs (1)
35-43
: Verify timestamp format consistency.The timestamps use ISO 8601 format with timezone offset, which is good. However, the test data shows
updated_at
being earlier thancreated_at
, which is logically inconsistent.- "created_at": "2020-11-19T14:57:43.796281+01:00", - "updated_at": "2020-11-13T10:23:43.234323+01:00", + "created_at": "2020-11-13T10:23:43.234323+01:00", + "updated_at": "2020-11-19T14:57:43.796281+01:00",
3deaa79
to
9449bc9
Compare
WHY
Resolves #15635
Summary by CodeRabbit