-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New Components - planning_center #16625
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis change introduces a new Planning Center integration with several polling sources: "New Person Added to List," "New Calendar Event," and "New Donation Received." It includes base components for API interaction, event polling, and state management. Legacy files and configuration are removed or updated to reflect the new structure and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Source as Polling Source (e.g., New Donation)
participant App as Planning Center App
participant API as Planning Center API
participant DB as Database
Source->>DB: Get last processed timestamp
Source->>App: Call paginated API method (e.g., listDonations)
loop For each page of results
App->>API: GET /giving/v2/donations (with OAuth)
API-->>App: Return donation data
App-->>Source: Return donation data
Source->>Source: Filter new items by timestamp
Source->>DB: Update last processed timestamp
Source->>Source: Emit event for each new item
end
Assessment against linked issues
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
components/planning_center/planning_center.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/planning_center/sources/common/base.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/planning_center/sources/new-donation/new-donation.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ 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: 1
🧹 Nitpick comments (6)
components/planning_center/sources/new-donation/new-donation.mjs (1)
1-27
: Consider implementing the isSorted methodThe component correctly extends the common base and specifies ordering by creation date descending. However, unlike the calendar event source, this component doesn't explicitly implement
isSorted()
. Since you're explicitly ordering by creation date descending, consider adding anisSorted()
method that returnstrue
to optimize event processing.+ isSorted() { + return true; + },components/planning_center/sources/new-list-result/new-list-result.mjs (1)
1-37
: Consider implementing the isSorted methodThe component correctly extends the common base, adds the necessary listId prop, and specifies ordering by creation date descending. Similar to the donation source, consider adding an
isSorted()
method that returnstrue
since you're explicitly ordering results by creation date.+ isSorted() { + return true; + },components/planning_center/planning_center.app.mjs (1)
72-96
: Consider adding error handling to the paginate methodThe paginate method is well-implemented using the async generator pattern. However, consider adding error handling to gracefully handle API failures:
do { - const { data } = await fn(args); + try { + const { data } = await fn(args); + for (const item of data) { + yield item; + if (max && ++count >= max) { + return; + } + } + total = data?.length; + args.params.offset += args.params.per_page; + } catch (error) { + console.error("Error while paginating Planning Center API:", error); + throw error; + } } while (total);Also consider adding rate limiting handling if the Planning Center API has rate limits.
components/planning_center/sources/common/base.mjs (3)
22-30
: Consider adding JSDoc comments for these overridable methods.These methods are designed to be overridden by subclasses. Adding JSDoc comments would make it clearer for developers extending this base component.
+/** + * Override to provide custom arguments for API calls + * @returns {Object} Arguments to pass to the resource function + */ getArgs() { return {}; }, +/** + * Extract timestamp from an item + * @param {Object} item - The item to extract timestamp from + * @returns {number} Timestamp in milliseconds + */ getTs(item) { return Date.parse(item.attributes.created_at); }, +/** + * Whether the data is sorted by timestamp + * @returns {boolean} True if data is sorted by timestamp + */ isSorted() { return true; },
1-82
: Add fallback for timestamp extraction.The
getTs
method assumes items always have acreated_at
attribute. Consider adding a fallback mechanism for items without this attribute.getTs(item) { - return Date.parse(item.attributes.created_at); + if (item.attributes && item.attributes.created_at) { + return Date.parse(item.attributes.created_at); + } + // Fallback to current time if created_at is not available + console.warn("Item missing created_at attribute:", item); + return Date.now(); },
1-82
: Consider adding class-level JSDoc description.Adding a description at the top of the component would help developers understand its purpose and how to extend it.
+/** + * Base component for Planning Center sources. + * + * This component provides the foundation for incremental event processing with stateful tracking. + * Subclasses must implement the following methods: + * - getResourceFn(): Returns the API function to call + * - getSummary(item): Returns a summary string for an event + * + * Optional methods to override: + * - getArgs(): Returns arguments for the API call + * - getTs(item): Extracts timestamp from an item + * - isSorted(): Whether the data is sorted by timestamp + */ export default { props: { planningCenter,
📜 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 (8)
components/planning_center/.gitignore
(0 hunks)components/planning_center/app/planning_center.app.ts
(0 hunks)components/planning_center/package.json
(1 hunks)components/planning_center/planning_center.app.mjs
(1 hunks)components/planning_center/sources/common/base.mjs
(1 hunks)components/planning_center/sources/new-calendar-event/new-calendar-event.mjs
(1 hunks)components/planning_center/sources/new-donation/new-donation.mjs
(1 hunks)components/planning_center/sources/new-list-result/new-list-result.mjs
(1 hunks)
💤 Files with no reviewable changes (2)
- components/planning_center/.gitignore
- components/planning_center/app/planning_center.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 (12)
components/planning_center/package.json (1)
3-17
: Package.json updates support the new component architectureThe updates to version, main entry point, and dependencies correctly align with the new Planning Center integration. The version bump from 0.0.2 to 0.1.0 is appropriate given the substantial changes to the component structure.
components/planning_center/sources/new-calendar-event/new-calendar-event.mjs (1)
1-23
: Implementation follows good component patternsThe source component properly extends the common base and implements the necessary methods for detecting new calendar events. Good job including the documentation link in the description.
Note that
isSorted()
returning false indicates the API doesn't return data in a consistent order, which could affect how events are processed chronologically.components/planning_center/planning_center.app.mjs (4)
1-4
: Constants are appropriately definedThe constants for default and maximum limits are well-defined at the top of the file for easy reference and modification.
5-28
: Prop definition for listId is well-implementedThe listId prop definition with its async options loader is well-structured. It correctly uses pagination and maps the API response to the expected options format.
29-71
: API methods are well-organizedThe base URL, request helper, and API-specific methods are well-organized and follow good practices. Each endpoint has its own dedicated method, making the code modular and maintainable.
1-98
:✅ Verification successful
Verify the MAX_LIMIT constant aligns with Planning Center's API
The code sets MAX_LIMIT to 100 items per page. Verify this is consistent with Planning Center's API documentation to avoid potential issues with pagination.
🌐 Web query:
What is the maximum page size for Planning Center API endpoints?
💡 Result:
The maximum page size for Planning Center API endpoints is 100 items per page when using the per_page parameter. This information is referenced in community discussions about API integrations, where it is stated: “the default number of orders per page is 15 and it is possible to configure it up to 100 using the per_page parameter”[3].
Official Planning Center API documentation does not directly specify this value, but third-party integration and developer experiences consistently report 100 as the upper limit per page for paginated requests to Planning Center’s endpoints[3].
Citations:
- 1: https://developer.planning.center/docs
- 2: https://www.planningcenter.com/developers
- 3: https://discuss.airbyte.io/t/pagination-issue-in-custom-connector/9860
- 4: https://support.planningcenteronline.com/hc/en-us/articles/18032518633115-Image-Sizing
- 5: https://community.ptc.com/t5/ThingWorx-Developers/ODATA-REST-API-max-page-size-gt-2000/td-p/688966
- 6: https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/azure-subscription-service-limits
- 7: https://community.developer.atlassian.com/t/max-page-size-for-events-endpoint/81758
- 8: https://learn.microsoft.com/en-us/azure/data-api-builder/whats-new/version-1-2
- 9: https://developer.zendesk.com/api-reference/introduction/rate-limits/
- 10: https://developer.hashicorp.com/terraform/enterprise/api-docs
MAX_LIMIT constant is correct
Verified that Planning Center’s API supports a maximumper_page
value of 100 items per page. No changes needed.components/planning_center/sources/common/base.mjs (6)
1-3
: Imports look good.The file correctly imports the Planning Center app and default polling interval constant.
4-14
: Props structure looks well-organized.The component properly defines the Planning Center app, database service for state persistence, and timer interface with appropriate default polling interval.
16-21
: State management methods are implemented correctly.The implementation for getting and setting the last timestamp in the database looks good.
31-41
: Event emission and metadata generation look good.The methods properly handle event emission with appropriate metadata.
67-72
: Abstract methods are correctly implemented.The base component correctly throws errors for methods that must be implemented by subclasses.
74-81
: Lifecycle hooks look good.The deploy hook correctly processes a limited number of events initially, and the run method processes events continuously.
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 @michelle0927, LGTM! Ready for QA!
Resolves #15143
Summary by CodeRabbit