-
Notifications
You must be signed in to change notification settings - Fork 15
Calendly (upgrade) - api v2 #467
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
Conversation
WalkthroughThis pull request updates the Calendly integration to version 3.0.0. It revises components to support Calendly API v2, including breaking changes to event output parameters and the introduction of new events for invitee no-shows. The modifications update webhook subscription methods to use a Context object and streamline asynchronous logic. Additionally, the configuration files for Invitee events have been restructured, and several outdated dependencies have been removed from the package. Changes
Suggested reviewers
🪧 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 (
|
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
🧹 Nitpick comments (9)
src/appmixer/calendly/bundle.json (1)
13-18
: Minor grammar fix and clarity improvement.
In the changelog entry at line 15, consider correcting "This change affect" to "This change affects." Also confirm that developers reading the notes are aware of the mapping changes in the new version.- (breaking change): Update components to use Calendly API v2. This change affect both InviteeCreated and InviteeCanceled. + (breaking change): Update components to use Calendly API v2. This change affects both InviteeCreated and InviteeCanceled.src/appmixer/calendly/events/InviteeNoShowDeleted/component.json (1)
304-305
: Icon check.
The base64-encoded icon is valid in JSON format. Just a note: if the icon must be updated frequently, consider storing it externally or referencing a smaller base64 placeholder for performance.src/appmixer/calendly/events/InviteeNoShowCreated/component.json (1)
304-315
: Base64 icon is valid.
Similar to the previous file, embedding a large icon in JSON is acceptable if minimal updates are expected. For frequent changes, external references are typically easier to maintain.src/appmixer/calendly/calendly-commons.js (2)
17-31
: HTTP request for subscription creation.
- Logging
webhookUrl
and the response data is helpful for debugging.- Returning
data.resource
directly is fine, but consider wrapping in a try/catch to handle request failures.- The addition of new
events
(invitee_no_show.created
andinvitee_no_show.deleted
) aligns with the new components.+ try { const { data } = await context.httpRequest(...); return data.resource; + } catch (err) { + context.logError({ step: 'registerWebhookSubscription', error: err }); + throw err; + }
44-48
: Ensure delete request is error-handled.
Including a log of the DELETE result or a try/catch block can help in diagnosing possible API removal failures.+ try { await context.httpRequest({ method: 'DELETE', url: webhookUri, ... }); + } catch (err) { + context.logError({ step: 'removeWebhookSubscription', error: err }); + throw err; + }src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (1)
1-1
: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode, making this directive unnecessary.
-'use strict';
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
src/appmixer/calendly/events/InviteeCreated/component.json (1)
17-282
: Comprehensive schema definition for Calendly API v2.The schema provides a detailed representation of the event data structure, which helps with client code understanding and type validation. This aligns well with the migration to Calendly API v2 mentioned in the PR summary.
However, some schema properties (like lines 64-66 for "invitee_scheduled_by") have a title but no type specified, which is inconsistent with other properties. Consider adding type information to these properties for consistency.
src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (1)
11-11
: Added authentication loggingAdding authentication context logging is good for debugging purposes but should be limited to development environments or handled with proper log levels to avoid leaking sensitive information in production.
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (1)
1-1
: Unnecessary 'use strict' directiveThe 'use strict' directive is redundant in ES modules as they are automatically in strict mode.
-'use strict';
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/appmixer/calendly/events/InviteeCreated/icon.png
is excluded by!**/*.png
📒 Files selected for processing (11)
src/appmixer/calendly/bundle.json
(2 hunks)src/appmixer/calendly/calendly-commons.js
(1 hunks)src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js
(2 hunks)src/appmixer/calendly/events/InviteeCanceled/component.json
(2 hunks)src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js
(2 hunks)src/appmixer/calendly/events/InviteeCreated/component.json
(2 hunks)src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js
(1 hunks)src/appmixer/calendly/events/InviteeNoShowCreated/component.json
(1 hunks)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js
(1 hunks)src/appmixer/calendly/events/InviteeNoShowDeleted/component.json
(1 hunks)src/appmixer/calendly/package.json
(0 hunks)
💤 Files with no reviewable changes (1)
- src/appmixer/calendly/package.json
🧰 Additional context used
📓 Path-based instructions (1)
`**/*/bundle.json`: Every pull request should include changes to the related `bundle.json` file. This file contains version information, so all changes must also include a version ...
**/*/bundle.json
: Every pull request should include changes to the relatedbundle.json
file. This file contains version information, so all changes must also include a version update.
src/appmixer/calendly/bundle.json
🧬 Code Graph Analysis (5)
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (3)
src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (4)
commons
(2-2)webhookData
(27-27)response
(45-45)webhookUri
(56-56)src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (4)
commons
(2-2)webhookData
(27-27)response
(45-45)webhookUri
(56-56)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (4)
commons
(2-2)webhookData
(26-26)response
(44-44)webhookUri
(55-55)
src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (4)
src/appmixer/calendly/calendly-commons.js (3)
context
(13-13)context
(17-30)context
(42-42)src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (3)
webhookData
(26-26)response
(44-44)commons
(2-2)src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (3)
webhookData
(27-27)response
(45-45)commons
(2-2)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (3)
webhookData
(26-26)response
(44-44)commons
(2-2)
src/appmixer/calendly/calendly-commons.js (4)
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (1)
webhookUri
(55-55)src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (1)
webhookUri
(56-56)src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (1)
webhookUri
(56-56)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (1)
webhookUri
(55-55)
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (4)
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (4)
commons
(2-2)webhookData
(26-26)response
(44-44)webhookUri
(55-55)src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (4)
commons
(2-2)webhookData
(27-27)response
(45-45)webhookUri
(56-56)src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (4)
commons
(2-2)webhookData
(27-27)response
(45-45)webhookUri
(56-56)src/appmixer/calendly/calendly-commons.js (3)
context
(13-13)context
(17-30)context
(42-42)
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (4)
src/appmixer/calendly/calendly-commons.js (3)
context
(13-13)context
(17-30)context
(42-42)src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (3)
webhookData
(26-26)response
(44-44)commons
(2-2)src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (3)
webhookData
(27-27)response
(45-45)commons
(2-2)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (3)
webhookData
(26-26)response
(44-44)commons
(2-2)
🪛 Biome (1.9.4)
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🔇 Additional comments (23)
src/appmixer/calendly/bundle.json (1)
3-3
: Great job updating the package version.
Having a new semantic version (3.0.0) clearly indicates the introduction of breaking changes.src/appmixer/calendly/events/InviteeNoShowDeleted/component.json (2)
1-10
: File structure and metadata look good.
The component’s “name”, “label”, version, and “description” fields provide a clear definition for the new event. The usage of"private": false
and"webhook": true
is consistent with other event components.
13-111
: Schema definitions are comprehensive.
Each property under"outPorts"
→"payload"
includes a descriptive title, which helps maintain clarity. However, consider verifying that all frequently used fields have the correct data types. For example, ensure objects like"routing_form_submission"
and"payment"
(lines 88-90, 135-138, 146-148, etc.) are typed if needed.src/appmixer/calendly/events/InviteeNoShowCreated/component.json (2)
1-10
: Component metadata is consistent.
Values for"name"
,"label"
,"version"
, and"description"
clearly establish this new event component.
112-303
: Schema coverage is thorough.
The nested objects (e.g.,"reconfirmation"
,"scheduled_event"
, and"tracking"
) are well-scoped. Check that your external consumers or tests handle these expanded properties correctly, especially new or optional ones.src/appmixer/calendly/calendly-commons.js (3)
7-9
: Parameter documentation update.
You’ve correctly documented@param {Context} context
and@param {string} event
. Good practice clarifying thatevent
can be one of the new Calendly event types.
12-14
: Parameter destructuring approach.
ExtractingaccessToken
andresource
fromcontext.auth
is clean and readable. Ensure a safe fallback in casecontext.auth
orprofileInfo
is undefined to avoid runtime errors.
37-42
: Method signature rename is consistent.
The newwebhookUri
parameter clarifies that it’s a URI, not just an ID. This consistency helps maintain clearer usage across your event modules.src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (3)
9-17
: LGTM! The start/stop methods follow the established pattern.The implementation correctly calls the corresponding webhook registration and unregistration methods.
19-35
: LGTM! Receive method handles webhooks appropriately.This method correctly processes incoming webhook messages, logs the data, and forwards it through the output port.
37-46
: LGTM! Register webhook implementation follows the established pattern.The method properly unregisters any existing webhook before registering a new one, and saves the URI in the state.
src/appmixer/calendly/events/InviteeCreated/component.json (2)
3-6
: LGTM! Added important metadata fields.Adding label, version, and updating the description improves component discoverability and documentation.
15-15
:❓ Verification inconclusive
Breaking change: Port name changed from "event" to "out".
This is a breaking change that requires client code to be updated. Ensure this is documented in migration guides and communicated to developers using this component.
🏁 Script executed:
#!/bin/bash # Search for usages of the old port name in the codebase to identify places that need updating echo "Searching for potential usage of the old port name 'event' from InviteeCreated" rg -A 2 -B 2 "appmixer.calendly.events.InviteeCreated" --glob "*.js" | grep -i "event"Length of output: 257
Action Required: Verify Client Integration for Port Name Change
The InviteeCreated component now uses
"name": "out"
on line 15 instead of the previous"event"
. This breaking change might impact client code that still references the old port name. Our automated search for the string “event” in connection with InviteeCreated did not return any results; however, the absence of output does not conclusively rule out all affected cases.
- File:
src/appmixer/calendly/events/InviteeCreated/component.json
(line 15)- Change: Port name updated from
"event"
to"out"
.- Impact: Consumer code referencing the old port name may fail; ensure migration guides are updated and that affected developers are informed.
Please perform a manual verification of client code integrations and update migration documentation as needed, ensuring that no usages of the old port name remain.
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (2)
26-35
: Improved webhook handling implementationThe simplified webhook handling with clear data extraction and logging improves code readability and maintainability. This approach with destructuring and explicit webhook data checking is consistent with other Calendly event components.
43-46
: Good async/await pattern for webhook registrationConverting the function to async and using await for sequential operations improves code clarity and error handling. The approach properly awaits unregistration before registering the new webhook.
src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (3)
26-35
: Improved webhook handling with explicit data extractionThe webhook data handling has been simplified with clear destructuring and explicit conditional checks, making the code more maintainable and easier to understand.
43-46
: Modernized webhook registration with async/awaitConverting to async/await improves code readability and error handling compared to the previous promise chain approach. The sequential flow is now clearer - first unregister, then register.
56-62
: Properly aligned variable name with state storageThis file correctly uses
webhookUri
for both storage (line 46) and retrieval (line 56), maintaining consistency with how the data is saved in the state.src/appmixer/calendly/events/InviteeCanceled/component.json (3)
3-6
: Added metadata improving component discoverabilityAdding label, version and improving the description makes the component more discoverable and easier to understand for users. The concise description clearly communicates the component's purpose.
14-16
: Updated output port name to standard conventionChanging the output port name from "event" to "out" improves consistency with other components and follows a more standard naming convention.
17-343
: Comprehensive schema definition for Calendly API v2The detailed schema definition for the payload ensures proper data type validation and provides clear field titles. This structured approach will help users understand the data they're working with and enable better data mapping in workflows.
Note: Some schema properties (lines 84-85, 87-88, 91-92, etc.) are missing type definitions, which might be intentional for flexible typing but should be verified.
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (2)
23-35
: Well-implemented webhook data handlingThe webhook data handling follows a consistent pattern with other Calendly event components, with proper checking for webhook existence, data extraction, and response handling.
42-46
: Proper async implementation for webhook registrationThe async implementation with await for sequential operations ensures clean error propagation and a clear flow. The function properly registers a webhook for the "invitee_no_show.created" event.
"outPorts": [ | ||
{ | ||
"name": "out", | ||
"options": [ | ||
{ | ||
"label": "Created At", | ||
"value": "created_at", | ||
"schema": { | ||
"type": "string" | ||
} | ||
}, | ||
{ | ||
"label": "Created By", | ||
"value": "created_by", | ||
"schema": { | ||
"type": "string" | ||
} | ||
}, | ||
{ | ||
"label": "Event", | ||
"value": "event", | ||
"schema": { | ||
"type": "string" | ||
} | ||
}, | ||
{ | ||
"label": "Payload", | ||
"value": "payload", | ||
"schema": { | ||
"type": "object", | ||
"properties": { | ||
"cancel_url": { | ||
"type": "string", | ||
"title": "Payload.Cancel Url" | ||
}, | ||
"created_at": { | ||
"type": "string", | ||
"title": "Payload.Created At" | ||
}, | ||
"email": { | ||
"type": "string", | ||
"title": "Payload.Email" | ||
}, | ||
"event": { | ||
"type": "string", | ||
"title": "Payload.Event" | ||
}, | ||
"first_name": { | ||
|
||
"title": "Payload.First Name" | ||
}, | ||
"invitee_scheduled_by": { | ||
"type": "string", | ||
"title": "Payload.Invitee Scheduled By" | ||
}, | ||
"last_name": { | ||
|
||
"title": "Payload.Last Name" | ||
}, | ||
"name": { | ||
"type": "string", | ||
"title": "Payload.Name" | ||
}, | ||
"new_invitee": { | ||
|
||
"title": "Payload.New Invitee" | ||
}, | ||
"no_show": { | ||
"type": "object", | ||
"properties": { | ||
"uri": { | ||
"type": "string", | ||
"title": "Payload.No Show.Uri" | ||
}, | ||
"created_at": { | ||
"type": "string", | ||
"title": "Payload.No Show.Created At" | ||
} | ||
}, | ||
"title": "Payload.No Show" | ||
}, | ||
"old_invitee": { | ||
"type": "string", | ||
"title": "Payload.Old Invitee" | ||
}, | ||
"payment": { | ||
|
||
"title": "Payload.Payment" | ||
}, | ||
"questions_and_answers": { | ||
"type": "array", | ||
"items": { | ||
"type": "object", | ||
"properties": { | ||
"answer": { | ||
"type": "string", | ||
"title": "Payload.Questions And Answers.Answer" | ||
}, | ||
"position": { |
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.
🛠️ Refactor suggestion
Review object schema for missing "type" attributes.
Fields like "first_name"
, "last_name"
, and "new_invitee"
do not specify "type"
. Missing type declarations can lead to inconsistent data handling downstream.
- "first_name": {
- "title": "Payload.First Name"
- },
+ "first_name": {
+ "type": "string",
+ "title": "Payload.First Name"
+ },
Please verify similar fields throughout the schema.
📝 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.
"outPorts": [ | |
{ | |
"name": "out", | |
"options": [ | |
{ | |
"label": "Created At", | |
"value": "created_at", | |
"schema": { | |
"type": "string" | |
} | |
}, | |
{ | |
"label": "Created By", | |
"value": "created_by", | |
"schema": { | |
"type": "string" | |
} | |
}, | |
{ | |
"label": "Event", | |
"value": "event", | |
"schema": { | |
"type": "string" | |
} | |
}, | |
{ | |
"label": "Payload", | |
"value": "payload", | |
"schema": { | |
"type": "object", | |
"properties": { | |
"cancel_url": { | |
"type": "string", | |
"title": "Payload.Cancel Url" | |
}, | |
"created_at": { | |
"type": "string", | |
"title": "Payload.Created At" | |
}, | |
"email": { | |
"type": "string", | |
"title": "Payload.Email" | |
}, | |
"event": { | |
"type": "string", | |
"title": "Payload.Event" | |
}, | |
"first_name": { | |
"title": "Payload.First Name" | |
}, | |
"invitee_scheduled_by": { | |
"type": "string", | |
"title": "Payload.Invitee Scheduled By" | |
}, | |
"last_name": { | |
"title": "Payload.Last Name" | |
}, | |
"name": { | |
"type": "string", | |
"title": "Payload.Name" | |
}, | |
"new_invitee": { | |
"title": "Payload.New Invitee" | |
}, | |
"no_show": { | |
"type": "object", | |
"properties": { | |
"uri": { | |
"type": "string", | |
"title": "Payload.No Show.Uri" | |
}, | |
"created_at": { | |
"type": "string", | |
"title": "Payload.No Show.Created At" | |
} | |
}, | |
"title": "Payload.No Show" | |
}, | |
"old_invitee": { | |
"type": "string", | |
"title": "Payload.Old Invitee" | |
}, | |
"payment": { | |
"title": "Payload.Payment" | |
}, | |
"questions_and_answers": { | |
"type": "array", | |
"items": { | |
"type": "object", | |
"properties": { | |
"answer": { | |
"type": "string", | |
"title": "Payload.Questions And Answers.Answer" | |
}, | |
"position": { | |
"outPorts": [ | |
{ | |
"name": "out", | |
"options": [ | |
{ | |
"label": "Created At", | |
"value": "created_at", | |
"schema": { | |
"type": "string" | |
} | |
}, | |
{ | |
"label": "Created By", | |
"value": "created_by", | |
"schema": { | |
"type": "string" | |
} | |
}, | |
{ | |
"label": "Event", | |
"value": "event", | |
"schema": { | |
"type": "string" | |
} | |
}, | |
{ | |
"label": "Payload", | |
"value": "payload", | |
"schema": { | |
"type": "object", | |
"properties": { | |
"cancel_url": { | |
"type": "string", | |
"title": "Payload.Cancel Url" | |
}, | |
"created_at": { | |
"type": "string", | |
"title": "Payload.Created At" | |
}, | |
"email": { | |
"type": "string", | |
"title": "Payload.Email" | |
}, | |
"event": { | |
"type": "string", | |
"title": "Payload.Event" | |
}, | |
"first_name": { | |
"type": "string", | |
"title": "Payload.First Name" | |
}, | |
"invitee_scheduled_by": { | |
"type": "string", | |
"title": "Payload.Invitee Scheduled By" | |
}, | |
"last_name": { | |
"title": "Payload.Last Name" | |
}, | |
"name": { | |
"type": "string", | |
"title": "Payload.Name" | |
}, | |
"new_invitee": { | |
"title": "Payload.New Invitee" | |
}, | |
"no_show": { | |
"type": "object", | |
"properties": { | |
"uri": { | |
"type": "string", | |
"title": "Payload.No Show.Uri" | |
}, | |
"created_at": { | |
"type": "string", | |
"title": "Payload.No Show.Created At" | |
} | |
}, | |
"title": "Payload.No Show" | |
}, | |
"old_invitee": { | |
"type": "string", | |
"title": "Payload.Old Invitee" | |
}, | |
"payment": { | |
"title": "Payload.Payment" | |
}, | |
"questions_and_answers": { | |
"type": "array", | |
"items": { | |
"type": "object", | |
"properties": { | |
"answer": { | |
"type": "string", | |
"title": "Payload.Questions And Answers.Answer" | |
}, | |
"position": { | |
... |
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.
fixed in recent commits
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.
@ZbynekPelunek Thank you for addressing the missing "type" attributes in the schema. Having proper type declarations helps ensure consistent data handling, so it's good to know you've fixed this in your recent commits.
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js
Outdated
Show resolved
Hide resolved
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js
Outdated
Show resolved
Hide resolved
} | ||
|
||
return commons.removeWebhookSubscription(webhookId, context.auth.accessToken); | ||
return commons.removeWebhookSubscription(webhookUri, context); |
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.
🛠️ Refactor suggestion
Inconsistent parameter order in removeWebhookSubscription
The parameter order here differs from other event handlers. This implementation passes webhookUri
first, then context
, which could create maintenance issues as other files use the opposite order.
- return commons.removeWebhookSubscription(webhookUri, context);
+ return commons.removeWebhookSubscription(context, webhookUri);
Alternatively, ensure the removeWebhookSubscription
method supports both parameter orders.
📝 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.
return commons.removeWebhookSubscription(webhookUri, context); | |
return commons.removeWebhookSubscription(context, webhookUri); |
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.
In all 4 components, the order is fine which is return commons.removeWebhookSubscription(webhookUri, context);
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.
@ZbynekPelunek You're right about the parameter order. The removeWebhookSubscription
method in calendly-commons.js
does indeed expect arguments as (webhookUri, context)
.
However, I noticed a small issue in the InviteeCanceled.js
file. The state is saved with the key webhookUri
, but retrieved using webhookId
:
// In registerWebhook:
return context.saveState({ webhookUri: response.uri });
// In unregisterWebhook:
let webhookUri = context.state.webhookId;
This inconsistency could prevent proper webhook unregistration. Consider updating it to:
const { webhookUri } = context.state;
Which matches how it's implemented in the other three components.
✏️ Learnings added
Learnt from: ZbynekPelunek
PR: clientIO/appmixer-connectors#467
File: src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js:61-61
Timestamp: 2025-04-09T10:27:31.974Z
Learning: In the Calendly connector, the parameter order for `removeWebhookSubscription` method in `calendly-commons.js` is `(webhookUri, context)`, not `(context, webhookUri)`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js
Outdated
Show resolved
Hide resolved
return commons.removeWebhookSubscription(webhookUri, context); | ||
} |
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.
🛠️ Refactor suggestion
Inconsistent parameter order in removeWebhookSubscription
The parameter order here is inconsistent with how the InviteeCreated component calls this method. For maintainability, maintain a consistent parameter order across all components.
-return commons.removeWebhookSubscription(webhookUri, context);
+return commons.removeWebhookSubscription(context, webhookUri);
Alternatively, verify that the commons.removeWebhookSubscription method handles both parameter orders correctly.
📝 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.
return commons.removeWebhookSubscription(webhookUri, context); | |
} | |
return commons.removeWebhookSubscription(context, webhookUri); | |
} |
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: 3
♻️ Duplicate comments (2)
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (1)
61-61
: Inconsistent parameter order in removeWebhookSubscription.The parameter order here differs from other event handlers. This implementation passes
webhookUri
first, thencontext
, which could create maintenance issues as other files use the opposite order.- return commons.removeWebhookSubscription(webhookUri, context); + return commons.removeWebhookSubscription(context, webhookUri);Alternatively, ensure the
removeWebhookSubscription
method supports both parameter orders.src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (1)
60-60
: Inconsistent parameter order in removeWebhookSubscription.The parameter order here is inconsistent with the pattern used in other components. For maintainability and consistency, consider using the same parameter order across all components.
-return commons.removeWebhookSubscription(webhookUri, context); +return commons.removeWebhookSubscription(context, webhookUri);
🧹 Nitpick comments (6)
src/appmixer/calendly/events/InviteeCanceled/component.json (3)
237-240
: Missing type property in schema definition.The
location.location
property in the schema is missing atype
property. This could affect validation and schema consistency.{ "location": { + "type": "string", "title": "Payload.Scheduled Event.Location.Location" } }
248-251
: Missing type property in schema definition.The
meeting_notes_html
property in the schema is missing atype
property. This could affect validation and schema consistency.{ + "type": "string", "title": "Payload.Scheduled Event.Meeting Notes Html" }
252-255
: Missing type property in schema definition.The
meeting_notes_plain
property in the schema is missing atype
property. This could affect validation and schema consistency.{ + "type": "string", "title": "Payload.Scheduled Event.Meeting Notes Plain" }
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (2)
1-1
: Redundant 'use strict' directive.The 'use strict' directive is redundant in JavaScript modules as they are automatically in strict mode. Consider removing it to follow modern JavaScript practices.
-'use strict';
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
4-7
: Incomplete module documentation.The module is missing a clear description comment explaining what this component does. Adding this would improve code maintainability and help other developers understand the purpose of this component.
/** + * Component which triggers whenever an invitee is marked as a no-show. * @extends {Component} */
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (1)
1-1
: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode, so this directive is unnecessary.
-'use strict';
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js
(2 hunks)src/appmixer/calendly/events/InviteeCanceled/component.json
(2 hunks)src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js
(2 hunks)src/appmixer/calendly/events/InviteeCreated/component.json
(2 hunks)src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js
(1 hunks)src/appmixer/calendly/events/InviteeNoShowCreated/component.json
(1 hunks)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js
(1 hunks)src/appmixer/calendly/events/InviteeNoShowDeleted/component.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/appmixer/calendly/events/InviteeNoShowCreated/component.json
- src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (4)
src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (4)
context
(56-56)webhookData
(27-27)response
(45-45)commons
(2-2)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (4)
context
(55-55)webhookData
(26-26)response
(44-44)commons
(2-2)src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (4)
context
(55-55)webhookData
(26-26)response
(44-44)commons
(2-2)src/appmixer/calendly/calendly-commons.js (3)
context
(13-13)context
(17-30)context
(42-42)
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (3)
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (4)
commons
(2-2)context
(56-56)webhookData
(27-27)response
(45-45)src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (4)
commons
(2-2)context
(56-56)webhookData
(27-27)response
(45-45)src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (4)
commons
(2-2)context
(55-55)webhookData
(26-26)response
(44-44)
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (3)
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (4)
commons
(2-2)context
(56-56)webhookData
(27-27)response
(45-45)src/appmixer/calendly/events/InviteeCreated/InviteeCreated.js (4)
commons
(2-2)context
(56-56)webhookData
(27-27)response
(45-45)src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (4)
commons
(2-2)context
(55-55)webhookData
(26-26)response
(44-44)
🪛 Biome (1.9.4)
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🔇 Additional comments (11)
src/appmixer/calendly/events/InviteeNoShowDeleted/component.json (1)
1-339
: Well-structured component configuration for the new event handler.The component configuration for invitee no-show deletion events is well-defined with comprehensive schema definitions that match Calendly's API v2 structure. The output ports and property mappings are thorough and will provide good flexibility for workflow automation.
src/appmixer/calendly/events/InviteeCanceled/InviteeCanceled.js (2)
26-34
: Improved webhook data handling with destructuring and logging.The new approach for handling webhook data with destructuring and proper async/await pattern is cleaner and more maintainable. The added logging will also help with debugging.
43-46
: Enhanced webhook registration with proper async handling.Converting to async/await pattern makes the code more readable and maintainable. The state variable has been appropriately renamed from
webhookId
towebhookUri
to better reflect what the value actually represents.src/appmixer/calendly/events/InviteeCanceled/component.json (1)
1-383
: Comprehensive update to align with Calendly API v2.The component configuration has been thoroughly updated to align with Calendly API v2, providing a detailed schema for the webhook payload. This will make it easier for users to access specific data points from cancellation events.
src/appmixer/calendly/events/InviteeNoShowCreated/InviteeNoShowCreated.js (2)
25-34
: Well-implemented webhook data handling.The implementation correctly extracts webhook data using destructuring, logs it for debugging purposes, and sends the data as JSON response. The use of async/await pattern makes the code more readable and maintainable.
42-46
: Properly structured webhook registration.The webhook registration code follows good practices by first unregistering any existing webhook, then registering a new one. The use of async/await pattern and proper state management ensures reliable webhook handling.
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js (1)
19-35
: LGTM! The webhook message handling looks good.The implementation properly checks for webhook messages and processes the data accordingly.
src/appmixer/calendly/events/InviteeCreated/component.json (4)
3-4
: LGTM! Good addition of label and version metadata.Adding the
label
andversion
fields improves component identification and versioning.
6-6
: LGTM! Description is more concise.The updated description is clearer and more concise.
15-15
: Note breaking change in port name.Changing the port name from
"event"
to"out"
is a breaking change. Ensure this is documented in release notes so users can update their flows.This is aligned with the broader Calendly API v2 update mentioned in the PR description, but needs proper documentation.
17-332
: LGTM! Comprehensive schema definition for Calendly v2 API.The restructured output schema provides a detailed and well-organized representation of the Calendly API v2 data structure. This will provide better data validation and improved developer experience.
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js
Show resolved
Hide resolved
src/appmixer/calendly/events/InviteeNoShowDeleted/InviteeNoShowDeleted.js
Show resolved
Hide resolved
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
♻️ Duplicate comments (1)
src/appmixer/calendly/events/InviteeCreated/component.json (1)
38-339
: Comprehensive "Payload" Schema Update:
The"Payload"
object has been updated to include an extensive set of properties that mirror the new Calendly API v2 structure. Notable points include:
Detailed nested definitions for objects such as
payment
,questions_and_answers
,reconfirmation
,scheduled_event
, andtracking
ensure robust data representation.The newly added fields
"meeting_notes_html"
and"meeting_notes_plain"
now include explicit type definitions, addressing previous review feedback.There are two
"status"
properties: one within the"scheduled_event"
object (titled Payload.Scheduled Event.Status) and another at the root of"Payload"
(titled Payload.Status). Please verify that the dual usage is intentional and that each field’s purpose is clearly documented to avoid ambiguity.Overall, the schema updates are thorough and align with the API v2 requirements.
🧹 Nitpick comments (3)
src/appmixer/calendly/events/InviteeNoShowDeleted/component.json (2)
13-342
: Output Ports and Payload Schema DefinitionThe
"outPorts"
array defines a single output port named"out"
with a robust set of options. The individual options for"Created At"
,"Created By"
,"Event"
, and especially the nested"Payload"
schema are detailed and cover a comprehensive list of fields relevant to Calendly events. Every field is explicitly typed and includes a title, which will aid downstream integrations and UI rendering.Make sure that the field names and types here match exactly with the data returned by Calendly API v2 so that mapping and validations work correctly. In addition, if similar payload schemas are used across multiple components, consider abstracting or sharing common definitions to reduce duplication.
343-345
: Component Icon InclusionThe
"icon"
field embeds a base64-encoded PNG image directly within the JSON. While this is acceptable for self-contained configurations, you might consider referencing an external asset if the image grows large or if multiple components use the same icon. This would improve maintainability and reduce file size.src/appmixer/calendly/events/InviteeCanceled/component.json (1)
388-389
: File Termination Check
Line 388 is marked with a tilde before the final closing brace. Please verify that this annotation does not introduce unintended characters in the actual JSON output and that the file ends correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/appmixer/calendly/events/InviteeCanceled/component.json
(2 hunks)src/appmixer/calendly/events/InviteeCreated/component.json
(2 hunks)src/appmixer/calendly/events/InviteeNoShowCreated/component.json
(1 hunks)src/appmixer/calendly/events/InviteeNoShowDeleted/component.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/appmixer/calendly/events/InviteeNoShowCreated/component.json
🔇 Additional comments (16)
src/appmixer/calendly/events/InviteeNoShowDeleted/component.json (2)
1-9
: Metadata Definition and Component IdentificationThe metadata block is well defined. The component's
name
,label
,version
,author
, anddescription
fields are clear and consistent with the Calendly API v2 upgrade objectives. The flags"private": false
,"webhook": true
, and"tick": true
appear appropriately set for an event component. Please ensure that the semantic of"tick": true
is documented elsewhere if its behavior isn’t immediately clear from this configuration.
10-12
: Authentication ConfigurationThe
"auth"
block cleanly specifies the Calendly integration by referring to the"appmixer:calendly"
service. This should integrate correctly with your authentication handling. No issues found here.src/appmixer/calendly/events/InviteeCreated/component.json (7)
3-4
: Added Label and Version Fields:
The addition of"label": "Invitee Created"
and"version": "1.0.1"
correctly reflects the updated naming conventions and versioning requirements for the Calendly integration. Please ensure that these values are consistent with your documentation and other integration components.
6-6
: Updated Description Field:
The"description"
has been refined to a more concise form—"Triggers when a new event is created by an invitee." This improves clarity, though please double-check that it fully captures all necessary details about the event trigger.
15-15
: Renamed Output Port:
Changing the port’s"name"
to"out"
standardizes the output naming as per the new integration conventions. This update streamlines the reference for downstream consumers.
17-23
: Defined "Created At" Output Option:
The new output option with"label": "Created At"
and its schema{ "type": "string" }
is clear and correctly typed for a timestamp value.
24-30
: Defined "Created By" Output Option:
The addition of the"Created By"
option with the string schema cleanly represents the creator information. This change aligns well with the revised output structure.
31-37
: Defined "Event" Output Option:
The"Event"
option is now explicitly defined with a string schema, ensuring that event identifiers or names are appropriately captured.
343-344
: Icon Field Validation:
The"icon"
field contains a base64-encoded PNG image. Confirm that this image meets your visual design standards and functions correctly across the integration.src/appmixer/calendly/events/InviteeCanceled/component.json (7)
3-4
: Component Metadata Addition
The addition of the"label": "Invitee Canceled"
and"version": "1.0.1"
fields enriches the component’s metadata. This makes the component easier to identify and version within the system. Please verify that these values align with your naming and versioning conventions across all Calendly integrations.
6-6
: Concise Description Update
The update of the"description"
field to"Triggers when an event is canceled."
makes it more succinct. Ensure that this new text is also updated in any relevant documentation or downstream integrations.
15-15
: OutPort Name Change
Renaming the outPort from"event"
to"out"
is a breaking change. Please confirm that all downstream consumers of this component have been updated accordingly to expect the port name"out"
.
17-23
: 'Created At' Option Addition
A new port option for"Created At"
has been added with a string schema. This is appropriate if the value represents a date/time string. Verify that the Calendly API provides this field in the expected format and that any front-end or integration logic consuming this value can correctly handle it.
24-30
: 'Created By' Option Addition
The addition of the"Created By"
port option with a string type is clear and consistent. Please ensure that the API reliably supplies this field for canceled events and that its format is documented.
31-37
: 'Event' Option Addition
A new"Event"
option has been introduced with a string schema. This addition appears consistent with the rest of the schema. Confirm that the value provided for this field is formatted as expected and that naming is consistent with other event-related fields.
38-383
:❓ Verification inconclusive
Expanded 'Payload' Schema Structure
This is the most significant change in the file. A comprehensive, nested schema for the"payload"
option has been introduced. Highlights include:
- Detailed nesting for cancellation details, scheduling, payment, and other event-related information.
- Each property now carries an associated
"title"
and explicit"type"
, which improves clarity and allows for downstream validation.Recommendations:
- Naming Consistency: Check that titles such as
"Payload.Cancel Url"
,"Payload.Cancellation.Canceled By"
, etc., follow a consistent capitalization and spacing standard. For example, consider whether"cancel_url"
should yield a title"Cancel URL"
instead of"Cancel Url"
.- Data Format Validation: Although JSON Schema defines the type, think about whether additional validation (e.g., date-time formats for fields like
"created_at"
) is necessary to fully align with the Calendly API v2 responses.- Documentation & Testing: Ensure that these schema changes are reflected in the component documentation and that unit tests (or schema validation tests) are available to catch any discrepancies when real payloads are received.
Overall, this restructuring greatly enhances the data representation of canceled events.
#!/bin/bash # Verify that the component file exists and display lines 38 to 383 for manual inspection. FILE="src/appmixer/calendly/events/InviteeCanceled/component.json" if [ -f "$FILE" ]; then echo "File exists. Displaying lines 38 to 383 for review:" sed -n '38,383p' "$FILE" else echo "Error: $FILE not found." fiSchema Enhancement & Consistency Check
The revised "payload" schema insrc/appmixer/calendly/events/InviteeCanceled/component.json
clearly distinguishes each nested property with explicit"type"
and"title"
, which greatly improves clarity and supports downstream validation.Key points to verify:
- Naming Consistency: Please check that title strings (e.g.,
"Payload.Cancel Url"
,"Payload.Cancellation.Canceled By"
, etc.) follow a uniform capitalization and spacing scheme. For instance, consider whether"cancel_url"
should produce a title like"Cancel URL"
instead of"Cancel Url"
.- Data Format Validation: Although the schema defines the data types, consider adding specific format constraints (such as
"format": "date-time"
for fields like"created_at"
) to align more precisely with the Calendly API v2 responses.- Documentation & Testing: Confirm that these schema updates are fully reflected in the component documentation and that unit or schema validation tests are in place to capture potential discrepancies during runtime.
Overall, the schema restructuring significantly enhances the data representation for canceled events. Please review the above points and verify the naming standards and format constraints align with our overall API expectations.
Summary by CodeRabbit
New Features
Chores