-
Notifications
You must be signed in to change notification settings - Fork 5.5k
CalendarHero new components #15921
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
CalendarHero new components #15921
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe changes remove the existing Changes
Sequence Diagram(s)sequenceDiagram
participant ActionModule as List Meetings Action
participant AppModule as CalendarHero App Module
participant API as CalendarHero API
ActionModule->>AppModule: run(context) for listing meetings
AppModule->>API: GET /meeting endpoint request
API-->>AppModule: Response with meeting data
AppModule-->>ActionModule: Returns response with meeting count
sequenceDiagram
participant Source as New Event Source
participant AppModule as CalendarHero App Module
participant API as CalendarHero API
Source->>AppModule: activate hook: createWebhook(event, webhookURL)
AppModule->>API: POST /webhook/{event}
API-->>AppModule: Registration confirmation
Note over Source,AppModule: Wait for incoming event trigger
Source->>AppModule: deactivate hook: deleteWebhook(event)
AppModule->>API: DELETE /webhook/{event}
API-->>AppModule: Removal confirmation
Possibly related PRs
Suggested labels
Suggested reviewers
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
Scope: all 2 workspace projects Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 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 (2)
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:
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: 1
🧹 Nitpick comments (5)
components/calendarhero/actions/list-meetings/list-meetings.mjs (2)
11-20: Consider adding date parameter validationWhile the props are well-defined with good descriptions and examples, there's no validation to ensure that the provided strings are valid ISO 8601 dates or that the start date is before the end date.
You could add validation in the run method:
async run({ $ }) { const { app, ...params } = this; + // Validate date format and range + const startDate = new Date(params.start); + const endDate = new Date(params.end); + + if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) { + throw new Error("Invalid date format. Please use ISO 8601 format (e.g., 2025-03-10T09:00:00Z)"); + } + + if (startDate >= endDate) { + throw new Error("Start date must be before end date"); + } + const response = await app.listMeetings({ $, params, });
30-34: Consider adding fallback for empty responsesUnlike the list-meeting-types action, there's no fallback handling if the response is undefined or doesn't have a length property. This could cause an error if the API response format changes.
- const { length } = response; + const { length = 0 } = response ?? {}; $.export("$summary", `Successfully listed ${length} meeting${length === 1 ? "" : "s"}`);components/calendarhero/sources/new-event-instant/new-event-instant.mjs (1)
50-60: Consider basic validation of webhook payloadThe run method should include basic validation of the webhook payload structure to prevent errors if unexpected data is received.
async run({ body }) { + // Basic validation of webhook payload + if (!body || typeof body !== "object") { + console.error("Received invalid webhook payload:", body); + return; + } + const ts = Date.now(); const id = body.id ?? ts; this.$emit(body, { id, - summary: `New event${id - ? ` (ID ${id})` - : ""}`, + summary: id !== ts ? `New event (ID ${id})` : "New event", ts, }); },components/calendarhero/calendarhero.app.mjs (2)
20-31: Consider adding pagination supportThe listMeetings and listMeetingTypes methods are implemented correctly, but they don't handle pagination. If the CalendarHero API returns paginated results, these methods should account for that to retrieve all available data.
Additionally, consider adding JSDoc comments to document the expected parameters and return values for better developer experience.
32-46: Add validation for webhook event parameterThe webhook methods look good functionally, but they accept an event parameter without validation. Consider validating this parameter against the allowed event types to prevent errors.
createWebhook({ event, ...args }) { + // Validate event type is supported + const validEvents = ["meeting.created", "meeting.updated"]; // Add actual valid events + if (!validEvents.includes(event)) { + throw new Error(`Invalid event type: ${event}. Valid events are: ${validEvents.join(", ")}`); + } return this._makeRequest({ method: "post", url: `webhook/${event}`, ...args, }); },Also apply similar validation to the deleteWebhook method.
📜 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 (8)
components/calendarhero/.gitignore(0 hunks)components/calendarhero/actions/list-meeting-types/list-meeting-types.mjs(1 hunks)components/calendarhero/actions/list-meetings/list-meetings.mjs(1 hunks)components/calendarhero/app/calendarhero.app.ts(0 hunks)components/calendarhero/calendarhero.app.mjs(1 hunks)components/calendarhero/common/constants.mjs(1 hunks)components/calendarhero/package.json(1 hunks)components/calendarhero/sources/new-event-instant/new-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (2)
- components/calendarhero/.gitignore
- components/calendarhero/app/calendarhero.app.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/calendarhero/common/constants.mjs (1)
1-34: Well-structured constant definition for webhook event typesThe
WEBHOOK_EVENT_TYPE_OPTIONSconstant is clearly defined with consistent formatting for each event type. The array structure with label/value pairs follows good practices for option definitions that will be used in UI components.A minor suggestion would be to add a JSDoc comment at the beginning of the file explaining the purpose of these constants, but this is optional.
components/calendarhero/actions/list-meeting-types/list-meeting-types.mjs (1)
3-22: Well-implemented action componentThe action follows Pipedream's best practices with clear metadata, documentation links, and a concise implementation. I particularly like the defensive programming in line 16 using the nullish coalescing operator with an empty object fallback.
The summary message with dynamic pluralization is a nice touch for UX.
components/calendarhero/package.json (3)
3-3: Version bump reflects new functionalityThe version increment from 0.0.3 to 0.1.0 appropriately follows semantic versioning principles, indicating new functionality that maintains backward compatibility.
5-5: Simplified main entry point pathThe main entry point has been simplified from a nested path to the root level, which makes imports cleaner and the package structure more straightforward.
15-16: Properly declared dependencyAdding the @pipedream/platform dependency is necessary as it's used in the new calendarhero.app.mjs file. Explicitly declaring dependencies follows best practices.
components/calendarhero/calendarhero.app.mjs (2)
1-6: Basic app structure looks goodThe app module is correctly structured with proper imports and export defaults. The type and app properties are appropriately set.
8-19: Verify Authorization header formatThe _makeRequest method looks good overall, but the Authorization header is set without a scheme prefix (like "Bearer"). Verify this is the correct format for CalendarHero's API.
- Authorization: `${this.$auth.api_key}`, + Authorization: `Bearer ${this.$auth.api_key}`,Also, consider adding basic error handling for API requests to improve debugging and user experience.
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 @GTFalcao, LGTM! Ready for QA!
Closes #15833
Summary by CodeRabbit
New Features
Chores