[Components] procore - new components #15843
Conversation
jcortes
added
action
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces several new modules to support Procore API integrations. New action modules enable incident, manpower log, RFI, submittal, and timesheet creation via dedicated API calls. Common modules for constants, resource names, and utilities have been added, along with a revamped Procore application interface. Multiple new "instant" event source modules and a webhook management module are provided, while legacy source modules are removed. Package dependencies and version numbers have also been updated. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/System
participant R as run() Method
participant F as Create* Function
participant API as Procore API
U->>R: Invoke action with parameters
R->>F: Call create* function with structured data
F->>API: Send POST request to Procore endpoint
API-->>F: Return response data (e.g., incident ID)
F-->>R: Return API result
R-->>U: Return summary message with resource ID
sequenceDiagram
participant U as User/System
participant W as Webhook Module (activate)
participant DB as Database
participant API as Procore API
U->>W: Trigger webhook activation
W->>W: Generate token and prepare webhook data
W->>API: Request creation of webhook
API-->>W: Return webhook ID
W->>DB: Store webhook and trigger IDs
W-->>U: Confirm activation details
Assessment against linked issues
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
components/procore/actions/create-submittal/create-submittal.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/procore/actions/create-incident/create-incident.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/procore/actions/create-manpower-log/create-manpower-log.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (24)
components/procore/common/utils.mjs (2)
1-7: Well-structured async iteration helper but documentation could be improved.The
iteratefunction provides a clean way to convert async iterables to arrays, but it lacks JSDoc documentation and error handling for non-iterable inputs.Consider adding JSDoc documentation and basic error handling:
+/** + * Asynchronously iterates over the provided iterable and collects all items into an array + * @param {AsyncIterable} iterations - The async iterable to process + * @returns {Promise<Array>} Array containing all the iterated items + */ async function iterate(iterations) { + if (!iterations || typeof iterations[Symbol.asyncIterator] !== 'function') { + throw new TypeError('Input must be an async iterable'); + } const items = []; for await (const item of iterations) { items.push(item); } return items; }
9-12: Clean implementation of nested property accessor.The
getNestedPropertyfunction elegantly handles property access using the optional chaining operator for safe traversal.For improved maintainability, consider adding JSDoc documentation:
+/** + * Safely retrieves a nested property from an object using dot notation + * @param {Object} obj - The object to traverse + * @param {string} propertyString - Dot-notation string representing the path to the property + * @returns {*} The value at the specified path or undefined if not found + */ function getNestedProperty(obj, propertyString) { const properties = propertyString.split("."); return properties.reduce((prev, curr) => prev?.[curr], obj); }components/procore/common/resource-names.mjs (1)
3-123: Comprehensive resource mapping with potential for improved organization.The resource name mapping is thorough and follows consistent naming conventions, but the large flat structure might become difficult to maintain as it grows.
Consider these improvements for better maintainability:
- Freeze the object to prevent accidental modifications:
-export default { +export default Object.freeze({ BUDGET_LINE_ITEMS: "Budget Line Items", // ... -}; +});
- Consider grouping related resources into categories:
export default Object.freeze({ - BUDGET_LINE_ITEMS: "Budget Line Items", - BUDGET_MODIFICATIONS: "Budget Modifications", - // ...other budget items + BUDGET: { + LINE_ITEMS: "Budget Line Items", + MODIFICATIONS: "Budget Modifications", + // ...other budget items + }, // ... });components/procore/sources/new-event-instant/new-event-instant.mjs (1)
19-31: Add error handling to the metadata generation.The generateMeta method builds event metadata from webhook payloads but lacks error handling for malformed inputs.
Add defensive programming to handle cases where body properties might be missing:
generateMeta(body) { + if (!body) { + return { + id: "unknown", + summary: "New Event: unknown", + ts: Date.now(), + }; + } return { - id: body.id, - summary: `New Event: ${body.event_type}`, - ts: new Date(body.timestamp).getTime(), + id: body.id || "unknown", + summary: `New Event: ${body.event_type || "unknown"}`, + ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(), }; },components/procore/actions/create-rfi/create-rfi.mjs (3)
113-113: Fix typo in success messageThere's a typo in "Successfully" (written as "Succesfully").
- $.export("$summary", `Succesfully created RFI with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created RFI with ID \`${response.id}\`.`);
74-81: Add error handling for API requestThe
createRfimethod should include error handling to provide a better user experience when API requests fail.createRfi({ projectId, ...args } = {}) { - return this.app.post({ - path: `/projects/${projectId}/rfis`, - ...args, - }); + try { + return this.app.post({ + path: `/projects/${projectId}/rfis`, + ...args, + }); + } catch (error) { + console.error(`Error creating RFI: ${error.message}`); + throw error; + } },
83-115: Validate required inputs before making API requestThe
runmethod should validate that required inputs are present before attempting to create an RFI. This will provide clearer error messages to users.async run({ $ }) { const { createRfi, companyId, projectId, subject, questionBody, rfiManagerId, reference, isPrivate, locationId, } = this; + // Validate required inputs + if (!subject) { + throw new Error("Subject is required"); + } + if (!questionBody) { + throw new Error("Question body is required"); + } + if (!rfiManagerId) { + throw new Error("RFI Manager is required"); + } const response = await createRfi({ $, companyId, projectId, data: { rfi: { subject, question: { body: questionBody, }, rfi_manager_id: rfiManagerId, reference, private: isPrivate, location_id: locationId, }, }, }); $.export("$summary", `Succesfully created RFI with ID \`${response.id}\`.`); return response; },components/procore/common/constants.mjs (2)
4-9: Inconsistent naming convention in VERSION_PATH objectThere's an inconsistency in the naming convention for version paths. While most keys use uppercase (V1_1, V2), the
v1_3key uses lowercase. This could lead to confusion when referencing these constants.const VERSION_PATH = { DEFAULT: "/v1.0", V1_1: "/v1.1", - v1_3: "/v1.3", + V1_3: "/v1.3", V2: "/v2.0", };
1-58: Add JSDoc comments for better code documentationThe constants file lacks documentation comments that explain the purpose and usage of these constants. Adding JSDoc comments would improve the maintainability of this module.
Consider adding comments to explain the purpose of key constant groups, for example:
+/** + * Placeholder for API version in URL construction + */ const VERSION_PLACEHOLDER = "{version}"; +/** + * Base URL for Procore API requests + */ const BASE_URL = `https://api.procore.com/rest${VERSION_PLACEHOLDER}`; +/** + * API version paths for different endpoints + */ const VERSION_PATH = { DEFAULT: "/v1.0", V1_1: "/v1.1", v1_3: "/v1.3", V2: "/v2.0", };components/procore/actions/create-submittal/create-submittal.mjs (2)
115-115: Fix typo in success messageThere's a spelling error in the success message.
- $.export("$summary", `Succesfully created submittal with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`);
76-84: Refactor method to clarify parameter handlingThe
createSubmittalmethod receivescompanyIdandprojectIdfrom the caller, but only usesprojectIdin the function body. Also, the$parameter is passed to the function but not explicitly defined in the function signature.createSubmittal({ - projectId, ...args + $, projectId, companyId, data, ...args } = {}) { return this.app.post({ versionPath: constants.VERSION_PATH.V1_1, path: `/projects/${projectId}/submittals`, + $, + companyId, + data, ...args, }); },components/procore/actions/create-incident/create-incident.mjs (3)
1-2: Import constants module for consistencyThe
create-submittal.mjsfile imports and uses the constants module for API version paths, but this file doesn't. For consistency across actions and to future-proof the code, import and use the constants module.import app from "../../procore.app.mjs"; +import constants from "../../common/constants.mjs";Then update the
createIncidentmethod to use version paths consistently:createIncident({ projectId, ...args } = {}) { return this.app.post({ + versionPath: constants.VERSION_PATH.DEFAULT, // Or appropriate version path: `/projects/${projectId}/incidents`, ...args, }); },
100-100: Fix typo in success messageThere's a spelling error in the success message.
- $.export("$summary", `Succesfully created incident with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created incident with ID \`${response.id}\`.`);
63-70: Refactor method to clarify parameter handlingSimilar to the other action components, this method receives parameters from the caller that aren't explicitly defined in the function signature.
createIncident({ - projectId, ...args + $, projectId, companyId, data, ...args } = {}) { return this.app.post({ path: `/projects/${projectId}/incidents`, + $, + companyId, + data, ...args, }); },components/procore/actions/create-timesheet/create-timesheet.mjs (2)
60-60: Fix typo in success messageThere's a spelling error in the success message.
- $.export("$summary", `Succesfully created timesheet with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created timesheet with ID \`${response.id}\`.`);
1-2: Import constants module for consistencySimilar to the incident creation action, this file should import and use the constants module for API version paths to maintain consistency across actions.
import app from "../../procore.app.mjs"; +import constants from "../../common/constants.mjs";Then update the
createTimesheetmethod to use version paths consistently:createTimesheet({ projectId, ...args } = {}) { return this.app.post({ + versionPath: constants.VERSION_PATH.DEFAULT, // Or appropriate version path: `/projects/${projectId}/timesheets`, ...args, }); },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
82-112: Test coverage recommendation.
Therunmethod is comprehensive, but ensuring test coverage would allow you to verify that all fields (datetime, notes, numWorkers, etc.) are handled properly. Consider adding unit tests or integration tests.components/procore/procore.app.mjs (1)
292-340: Iterative approach ingetIterationsis well-structured.
The generator-based pagination logic aligns with typical usage patterns. Make sure you handle partial data or empty responses from the Procore API safely.components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
16-40: Graceful fallback whenprojectIdis missing.
Your console log helps, but consider returning a more structured warning to the user if the project ID is critical for fetching details.components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs (1)
4-10: Add documentation for component statusThe PR title indicates these components are untested. Consider adding a note in the description or as a comment to indicate the testing status.
Consider adding a note about testing status:
export default { ...common, name: "New Change Order Package Event (Instant)", key: "procore-new-change-order-package-event-instant", + // Note: This component is currently untested description: "Emit new event when a new change order package event is created. [See the documentation](https://developers.procore.com/reference/rest/hooks?version=latest).", version: "0.0.1", type: "source",components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
29-38: Consider documenting pagination behaviorThis component uses pagination to fetch budget snapshots, which is different from other similar components that fetch a single resource. It might be helpful to document the expected structure of the returned data.
Add a comment explaining the pagination results:
if (!projectId) { console.log("If you need to get more details about the budget snapshot, please provide a project ID."); return body; } +// Retrieve all rows of the budget snapshot using pagination const snapshotRows = await app.paginate({ resourcesFn: app.getBudgetViewSnapshot, resourcesFnArgs: { companyId, budgetViewSnapshotId, params: { project_id: projectId, }, }, });components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
1-49: General pattern across webhook componentsI notice a consistent pattern across all the webhook event source components. Consider creating a more generalized module that could reduce code duplication for these similar instant event handlers.
You could create a factory function or higher-order component that takes the resource name, API method, and metadata details as parameters and returns a configured instant event source. This would reduce duplication and make it easier to maintain these components.
For example, you could create something like:
// instantEventSourceFactory.mjs import common from "../common/webhook.mjs"; export default function createInstantEventSource({ name, key, resourceName, resourceIdParam, fetchResourceFn, missingProjectIdMessage, }) { return { ...common, name, key, description: `Emit new event when a new ${name.toLowerCase()} is created. [See the documentation](https://developers.procore.com/reference/rest/hooks?version=latest).`, version: "0.0.1", type: "source", methods: { ...common.methods, getResourceName() { return resourceName; }, async getDataToEmit(body) { const { app, companyId, projectId } = this; const resourceId = body.resource_id; if (!projectId) { console.log(missingProjectIdMessage); return body; } try { const resource = await fetchResourceFn(app, { companyId, [resourceIdParam]: resourceId, params: { project_id: projectId, }, }); return { ...body, resource, }; } catch (error) { console.error(`Error fetching resource details: ${error.message}`); return body; } }, generateMeta(body) { return { id: body.id, summary: `New ${name}: ${body.resource_id}`, ts: new Date(body.timestamp).getTime(), }; }, }, }; }Then you could use it like:
// new-change-order-package-event-instant.mjs import createInstantEventSource from "../common/instantEventSourceFactory.mjs"; import resourceNames from "../../common/resource-names.mjs"; export default createInstantEventSource({ name: "Change Order Package Event", key: "procore-new-change-order-package-event-instant", resourceName: resourceNames.CHANGE_ORDER_PACKAGES, resourceIdParam: "changeOrderPackageId", fetchResourceFn: (app, args) => app.getChangeOrderPackage(args), missingProjectIdMessage: "If you need to get more details about the change order package, please provide a project ID.", });This approach would make it easier to maintain these components and add new ones in the future.
components/procore/sources/common/webhook.mjs (2)
35-35: Correct spelling of the function name.The method name is spelled as
additionalPropperties. Rename it toadditionalPropertiesto match typical naming conventions and avoid confusion.- async additionalPropperties() { + async additionalProperties() {
119-119: Validate the existence of thesuccessfield before mapping.If the
successfield doesn’t exist or is not an array, this code may storeundefinedin the database. Consider gracefully handling empty or invalid responses.- setWebhookTriggerIds(triggerResponses?.success.map(({ id }) => id)); + setWebhookTriggerIds( + Array.isArray(triggerResponses?.success) + ? triggerResponses.success.map(({ id }) => id) + : [] + );
📜 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 (30)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(2 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/submittal/submittal.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/procore.app.js
- components/procore/sources/new-event/new-event.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/sources/common.js
- components/procore/sources/rfi/rfi.js
🧰 Additional context used
🪛 GitHub Actions: Pull Request Checks
components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
[error] 1-1: Component folder name, component file name without extension and component key without slug should be the same! See the docs: https://pipedream.com/docs/components/guidelines/#folder-structure
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
🔇 Additional comments (20)
components/procore/common/utils.mjs (1)
14-17: Good export pattern.The default export pattern cleanly exports both utility functions in a single object.
components/procore/package.json (2)
3-3: Version bump is appropriate for new features.Increasing from 0.0.4 to 0.1.0 follows semantic versioning for adding new features without breaking changes.
19-19:❓ Verification inconclusive
Verify compatibility with major platform version upgrade.
The @pipedream/platform dependency has been updated from ^1.1.1 to ^3.0.3, which is a major version jump that might introduce breaking changes.
Please ensure all component code is compatible with this new platform version. You might want to check:
🏁 Script executed:
#!/bin/bash # Search for any deprecated API usage in the codebase cd components/procore grep -r "deprecated" --include="*.mjs" --include="*.js" . # Look for any explicit version checks or version-specific code grep -r "platform.*version\|version.*platform" --include="*.mjs" --include="*.js" .Length of output: 167
Compatibility Check Needed for @pipedream/platform Upgrade
The update from^1.1.1to^3.0.3is a major version bump. While our automated search for deprecated APIs and version-specific code (using grep) returned no output, this result isn’t conclusive evidence that there are no breaking changes. Please perform a manual review—especially in critical flows that rely on this dependency—to ensure no API changes adversely affect the component.components/procore/common/resource-names.mjs (1)
1-2: Good documentation with source references.Including references to the official Procore API documentation and CSV source is helpful for future maintenance.
components/procore/sources/new-event-instant/new-event-instant.mjs (2)
3-9: Good component metadata with documentation link.The module provides clear metadata including name, key, and a helpful description with a link to the API documentation.
Since the PR title mentions these components are "untested", ensure this module is properly tested before promoting to production use.
10-18: Clean property extension pattern.The props section correctly extends common properties and adds the resourceName property using the recommended propDefinition pattern.
components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs (1)
1-47: LGTM! Component implements webhook handler correctlyThe component correctly imports common webhook functionality and implements the necessary methods for handling submittal events.
components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs (1)
1-47: LGTM! Component implements webhook handler correctlyThe component correctly imports common webhook functionality and implements the necessary methods for handling RFI events.
components/procore/actions/create-submittal/create-submittal.mjs (1)
1-118: Add unit tests for this componentThe PR title indicates these components are untested. To ensure reliability, especially for API integrations, unit tests should be added to verify the functionality works as expected.
Would you like me to help generate unit test templates for this component? This could include tests for:
- Validating proper API calls are made with correct parameters
- Handling of optional parameters
- Error handling scenarios
- Response processing
components/procore/actions/create-manpower-log/create-manpower-log.mjs (3)
1-2: Use explicit relative import paths or confirm correctness.
Make sure"../../procore.app.mjs"is the correct relative path. Mistakes in path references could break the import in certain environments.
3-8: Export metadata looks clear.
The key, name, description, version, and type fields are well-structured. This metadata will help developers and users quickly identify the purpose of this action.
9-71: Validate required props or handle undefined gracefully.
Some props (e.g.,companyId,projectId) are presumably required for API calls. Ensure the action prevents unexpected undefined values in downstream processes.components/procore/procore.app.mjs (5)
1-5: Imports and constants references appear valid.
No issues detected with the imports from@pipedream/platformor local modules. Good to see well-organized references.
6-53: Ensure correct integer parsing forcompanyIdandprojectId.
The prop definitions specify integer types. Confirm values are correctly parsed (e.g., from strings) in all usage contexts to avoid unexpected type issues.
66-71: Consider defaulting to empty array or descriptive fallback forlocationId.
The optionallocationIdreturns an empty array if nocompanyIdorprojectIdis provided. Confirm this fallback is handled gracefully by downstream code.
145-186: Centralizing request logic is a great design choice.
The_getBaseUrl(),_getHeaders(), andmakeRequest()methods keep the code DRY and easy to maintain.
187-199: Pagination approach for companies and projects is consistent.
Using params{ per_page, page }is fine, but confirm large data sets are handled gracefully.components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (3)
1-3: Confirm dependencies on../common/webhook.mjs.
Ensure thatcommonis properly exported inwebhook.mjsso all inherited logic remains valid.
4-11: Metadata for event source is descriptive.
The name, key, and description provide clarity on the module's purpose and usage.
41-47: Leverage robust ID checks ingenerateMeta().
You're constructing an event ID frombody.id, but ifidis unexpectedly missing, it could cause inconsistencies. Validatebody.idor default gracefully.
...onents/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
Outdated
Show resolved
Hide resolved
...onents/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
Outdated
Show resolved
Hide resolved
...onents/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
Show resolved
Hide resolved
...rces/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
Show resolved
Hide resolved
jcortes
force-pushed
the
procore-new-components
branch
from
2869d06 to
1a6b1ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
73-80: 🛠️ Refactor suggestionAdd error handling for API failures.
Currently, if an error occurs during
createManpowerLog(), the action might silently fail at runtime. Consider wrapping API calls in robust try-catch blocks or returning informative errors.createManpowerLog({ projectId, ...args } = {}) { + try { return this.app.post({ path: `/projects/${projectId}/manpower_logs`, ...args, }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + } },components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs (1)
16-41: 🛠️ Refactor suggestionAdd error handling for the API call
The
getDataToEmitmethod makes an API call to retrieve change order package details, but it doesn't handle potential API errors. If theapp.getChangeOrderPackagecall fails, the error will propagate up and might cause the entire webhook processing to fail.async getDataToEmit(body) { const { app, companyId, projectId, } = this; const { resource_id: changeOrderPackageId } = body; if (!projectId) { console.log("If you need to get more details about the change order package, please provide a project ID."); return body; } + try { const resource = await app.getChangeOrderPackage({ companyId, changeOrderPackageId, params: { project_id: projectId, }, }); return { ...body, resource, }; + } catch (error) { + console.error(`Error fetching change order package details: ${error.message}`); + return body; + } },components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
16-44: 🛠️ Refactor suggestionAdd error handling for pagination function
The
getDataToEmitmethod uses pagination to fetch budget view snapshot details, but it doesn't handle potential API errors. If theapp.paginatecall fails, the error will propagate up and might cause the webhook processing to fail.async getDataToEmit(body) { const { app, companyId, projectId, } = this; const { resource_id: budgetViewSnapshotId } = body; if (!projectId) { console.log("If you need to get more details about the budget snapshot, please provide a project ID."); return body; } + try { const snapshotRows = await app.paginate({ resourcesFn: app.getBudgetViewSnapshot, resourcesFnArgs: { companyId, budgetViewSnapshotId, params: { project_id: projectId, }, }, }); return { ...body, snapshotRows, }; + } catch (error) { + console.error(`Error fetching budget view snapshot details: ${error.message}`); + return body; + } },
🧹 Nitpick comments (11)
components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (1)
24-27: Consider enhancing the log message with more guidanceThe log message could be more helpful by providing guidance on where to find or configure the project ID.
- console.log("If you need to get more details about the timecard entry, please provide the project ID."); + console.log("If you need to get more details about the timecard entry, please provide a project ID in the component configuration.");components/procore/actions/create-rfi/create-rfi.mjs (2)
113-113: Fix typo in success messageThere's a spelling error in the success message.
- $.export("$summary", `Succesfully created RFI with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created RFI with ID \`${response.id}\`.`);
73-82: Consider using API versioning consistentlyThe
createRfimethod doesn't specify a version path, unlike thecreateSubmittalmethod in the other file which usesconstants.VERSION_PATH.V1_1. Consider using versioning consistently across all API calls.createRfi({ projectId, ...args } = {}) { return this.app.post({ + versionPath: constants.VERSION_PATH.V1_1, // or the appropriate version path: `/projects/${projectId}/rfis`, ...args, }); },Don't forget to import constants:
+import constants from "../../common/constants.mjs"; import app from "../../procore.app.mjs";components/procore/actions/create-submittal/create-submittal.mjs (1)
115-115: Fix typo in success messageThere's a spelling error in the success message.
- $.export("$summary", `Succesfully created submittal with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`);components/procore/actions/create-incident/create-incident.mjs (2)
100-100: Fix typo in success messageThere's a spelling error in the success message.
- $.export("$summary", `Succesfully created incident with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created incident with ID \`${response.id}\`.`);
62-71: Consider using API versioning consistentlyThe
createIncidentmethod doesn't specify a version path, unlike thecreateSubmittalmethod which usesconstants.VERSION_PATH.V1_1. Consider using versioning consistently across all API calls.createIncident({ projectId, ...args } = {}) { return this.app.post({ + versionPath: constants.VERSION_PATH.V1_1, // or the appropriate version path: `/projects/${projectId}/incidents`, ...args, }); },Don't forget to import constants:
+import constants from "../../common/constants.mjs"; import app from "../../procore.app.mjs";components/procore/actions/create-manpower-log/create-manpower-log.mjs (2)
45-49: Consider using numeric type fornumHoursThe property is defined as a string type, but since it represents a numeric value (hours), it would be more appropriate to use a numeric type like 'number' or 'integer'.
numHours: { - type: "string", + type: "number", label: "Number Of Hours", description: "The number of hours for each worker", optional: true, },
110-110: Fix typo in success messageThere's a spelling error in the success message.
-$.export("$summary", `Succesfully created manpower log with ID \`${response.id}\`.`); +$.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`);components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
24-27: Consider providing richer user feedback
When theprojectIdis missing, the debug log may not be sufficient. You might want to throw a more explicit exception or handle this scenario upstream to ensure the user is aware that the change event details could not be fetched.components/procore/sources/common/webhook.mjs (2)
35-64: Minor naming nitpick
The method nameadditionalProppertiesmight be a typo foradditionalProperties. Renaming it will improve clarity.-async additionalPropperties() { +async additionalProperties() {
214-216: Add error handling forgetDataToEmit
Currently, this method simply returns the body, but if you extend it in the future to make external API calls, ensure you handle potential errors as you do in your other derived classes.
📜 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 (30)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(2 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/common.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/new-event/new-event.js
- components/procore/procore.app.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/submittal/submittal.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
🚧 Files skipped from review as they are similar to previous changes (10)
- components/procore/package.json
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore/common/utils.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/common/resource-names.mjs
- components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore/actions/create-timesheet/create-timesheet.mjs
- components/procore/common/constants.mjs
- components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
🔇 Additional comments (8)
components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (1)
1-49: Overall implementation looks good!The module is well-structured and follows the established pattern for Procore webhook sources. The component correctly imports the common webhook functionality and resource names, and implements the necessary methods to handle timecard entry events.
components/procore/actions/create-rfi/create-rfi.mjs (1)
1-116: Implementation looks good, but testing is neededThe action is well-structured with appropriate props and methods for creating an RFI in Procore.
Since this component is marked as "untested" in the PR title, please ensure you test this functionality thoroughly before merging.
components/procore/actions/create-submittal/create-submittal.mjs (1)
1-118: Implementation looks good, but testing is neededThe action is well-structured with appropriate props and methods for creating a submittal in Procore.
Since this component is marked as "untested" in the PR title, please ensure you test this functionality thoroughly before merging.
components/procore/actions/create-incident/create-incident.mjs (1)
1-103: Implementation looks good, but testing is neededThe action is well-structured with appropriate props and methods for creating an incident in Procore.
Since this component is marked as "untested" in the PR title, please ensure you test this functionality thoroughly before merging.
components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (2)
16-35: Add error handling for the API call
ThegetDataToEmitmethod callsapp.getChangeEventwithout handling potential HTTP or network errors. A surrounding try/catch block will protect the webhook flow from unexpected failures.Below is an example fix, matching a prior recommendation:
async getDataToEmit(body) { const { app, companyId, projectId, } = this; const { resource_id: changeEventId } = body; if (!projectId) { console.log("If you need to get more details about the commitment change order, please provide a project ID."); return body; } + try { const resource = await app.getChangeEvent({ companyId, changeEventId, params: { project_id: projectId, }, }); return { ...body, resource, }; + } catch (error) { + console.error(`Error fetching change event details: ${error.message}`); + return body; + } }
41-47: Validate the timestamp field
ThegenerateMetamethod referencesbody.timestamp, but there's no guarantee this property exists or follows a valid date format. Consider a fallback timestamp to avoid runtime errors and keep event ordering.components/procore/sources/common/webhook.mjs (2)
156-158: Implement thegenerateMetamethod
Currently, it throws aConfigurationError, which can cause runtime failures in derived sources. Provide a real implementation or ensure child classes override it.
177-179: Override or removegetResourceName
The base method throws an error, but derived classes may rely on it for generating triggers. If every derived class must override it, consider turning it into an abstract or mandatory requirement in your docs.
jcortes
force-pushed
the
procore-new-components
branch
from
1a6b1ac to
75fee6f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
components/procore/actions/create-submittal/create-submittal.mjs (2)
115-115: Fix typo in success message.There's a spelling error in the success message.
- $.export("$summary", `Succesfully created submittal with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`);
76-84: Add error handling for API calls.The
createSubmittalmethod lacks error handling, which could lead to silent failures or confusing error messages.createSubmittal({ projectId, ...args } = {}) { + try { return this.app.post({ versionPath: constants.VERSION_PATH.V1_1, path: `/projects/${projectId}/submittals`, ...args, }); + } catch (error) { + throw new Error(`Error creating submittal: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (2)
45-49: ChangenumHourstype from string to number.The
numHoursproperty is defined with type "string" but would be more appropriately defined as a number since it represents a quantity.numHours: { - type: "string", + type: "number", label: "Number Of Hours", description: "The number of hours for each worker", optional: true, },
110-110: Fix typo in success message.There's a spelling error in the success message.
- $.export("$summary", `Succesfully created manpower log with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`);components/procore/procore.app.mjs (1)
279-280: Fix version path casing inconsistency.The version path constant is using lowercase
v1_3while other version paths use uppercase format (likeV1_1).- versionPath: constants.VERSION_PATH.V1_3, + versionPath: constants.VERSION_PATH.V1_3,Verify that the constant is defined using uppercase in the constants file.
components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
29-35: Add error handling for getPrimeContract API call.The call to
app.getPrimeContractlacks error handling, which could cause the webhook to fail silently if the API request encounters an issue.- const resource = await app.getPrimeContract({ - companyId, - primeContractId, - params: { - project_id: projectId, - }, - }); + let resource; + try { + resource = await app.getPrimeContract({ + companyId, + primeContractId, + params: { + project_id: projectId, + }, + }); + } catch (error) { + console.error(`Failed to fetch prime contract details: ${error.message}`); + // Return the original body if we can't get additional details + return body; + }components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
45-51: Validate the incoming timestamp
Ifbody.timestampis invalid,new Date(body.timestamp).getTime()will yieldNaN. Consider falling back toDate.now()or providing error handling to ensure a valid timestamp.- ts: new Date(body.timestamp).getTime(), + ts: Number.isNaN(new Date(body.timestamp).getTime()) + ? Date.now() + : new Date(body.timestamp).getTime(),components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
41-47: Validate the incoming timestamp
Just like other event sources, relying onnew Date(body.timestamp).getTime()without checking validity can yieldNaN. Consider a fallback or additional validation.components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
41-47: Validate the incoming timestamp
Similarly, guard against invalidbody.timestampvalues to prevent potentialNaNresults.components/procore/sources/common/webhook.mjs (2)
35-35: Fix method name spelling
The function is namedadditionalPropperties, which is a misspelling. Rename it toadditionalPropertiesfor clarity and consistency.- async additionalPropperties() { + async additionalProperties() {
66-153: Consider robust error handling in activation/deactivation hooks
Theactivateanddeactivatehooks rely on multiple async API calls. If any step fails, you risk partial success states (e.g., webhook created but triggers not fully set), which can lead to inconsistent data. Wrap the API calls in try/catch blocks and implement a rollback strategy or partial clean-up as needed.
📜 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 (30)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(2 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/common.js
- components/procore/sources/submittal/submittal.js
- components/procore/sources/new-event/new-event.js
- components/procore/procore.app.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/purchase-order/purchase-order.js
🚧 Files skipped from review as they are similar to previous changes (12)
- components/procore/common/resource-names.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore/common/utils.mjs
- components/procore/actions/create-incident/create-incident.mjs
- components/procore/common/constants.mjs
- components/procore/actions/create-timesheet/create-timesheet.mjs
- components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore/package.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (9)
components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
73-80: Add better error handling for API failures.Currently, if an error occurs during
createManpowerLog(), the action might silently fail at runtime. Consider wrapping API calls in robust try-catch blocks or returning informative errors.+ try { return this.app.post({ path: `/projects/${projectId}/manpower_logs`, ...args, }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + }components/procore/procore.app.mjs (2)
229-229: Fix prime contract endpoint path to use plural form.The Procore API uses plural form for resource endpoints. Update the path to use
/prime_contracts/instead of/prime_contract/.- path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`,
292-336: Add robust error handling to pagination logic.The
getIterationsgenerator method lacks error handling for API calls, which could cause the entire pagination process to fail if an error occurs during any request.async *getIterations({ resourcesFn, resourcesFnArgs, resourceName, max = constants.DEFAULT_MAX, }) { let page = 1; let resourcesCount = 0; while (true) { + try { const response = await resourcesFn({ ...resourcesFnArgs, params: { ...resourcesFnArgs?.params, page, per_page: constants.DEFAULT_LIMIT, }, }); const nextResources = resourceName ? utils.getNestedProperty(response, resourceName) : response; if (!nextResources?.length) { console.log("No more resources found"); return; } for (const resource of nextResources) { yield resource; resourcesCount += 1; if (resourcesCount >= max) { console.log("Reached max resources"); return; } } if (nextResources.length < constants.DEFAULT_LIMIT) { console.log("No next page found"); return; } page += 1; + } catch (error) { + console.error(`Error fetching resources on page ${page}: ${error.message}`); + throw error; // Re-throw to allow the caller to handle the error + } } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
16-40: Verify this component will work with the corrected prime contract endpoint.The
getDataToEmitmethod callsapp.getPrimeContract, which currently uses an incorrect singular endpoint path (/prime_contract/). Once the path is corrected in the app file to use plural form (/prime_contracts/), verify that this component still works correctly.components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
29-38: Add error handling for the pagination call
This code directly callsapp.paginatewithout catching potential errors, which could cause unhandled exceptions. The exact fix requested in a previous review is still applicable.components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
29-35: Add error handling for the API call
Similar to previous feedback,app.getPurchaseOrderContractmay fail due to network or API issues. Adding a try/catch block ensures more robust error handling.components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
29-35: Add error handling for the API call
Continue the pattern of adding a try/catch block aroundapp.getChangeEventto handle unexpected failures gracefully.components/procore/sources/common/webhook.mjs (2)
156-158: Implement or remove the defaultgenerateMeta
Leaving this unimplemented can cause runtime errors if derived classes forget to override it. This was flagged in a previous review.
177-179: Implement or overridegetResourceName
This method throws aConfigurationErrorby default, which will break sources if unimplemented. Also flagged in a prior review.
jcortes
force-pushed
the
procore-new-components
branch
from
75fee6f to
a294ed8
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
16-26: 🛠️ Refactor suggestionAdd error handling for pagination calls.
app.paginatecould fail, potentially causing the source to crash or silently fail. Wrap it in a try/catch block or handle errors more gracefully to enhance reliability.async getDataToEmit(body) { const { app, companyId, projectId, } = this; const { resource_id: budgetViewSnapshotId } = body; + try { const snapshotRows = await app.paginate({ resourcesFn: app.getBudgetViewSnapshot, resourcesFnArgs: { companyId, budgetViewSnapshotId, params: { project_id: projectId, }, }, }); + return { + ...body, + snapshotRows, + }; + } catch (err) { + console.error("Error during paginate:", err.message); + return body; + } }components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
16-26: 🛠️ Refactor suggestionHandle potential errors when fetching change event details.
The code directly calls
app.getChangeEventwithout a try/catch. If the API call fails, the error may cause downstream issues or incomplete event data. Add error handling to gracefully handle failures.async getDataToEmit(body) { const { app, companyId, } = this; const { resource_id: changeEventId, project_id: projectId } = body; + try { const resource = await app.getChangeEvent({ companyId, changeEventId, params: { project_id: projectId, }, }); return { ...body, resource, }; + } catch (err) { + console.error("Failed to retrieve change event details:", err.message); + return body; + } }
🧹 Nitpick comments (25)
components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
4-5: Consider addressing ESLint disabled rules.While disabling these rules may be intentional for sandbox components, it would be good to document why these specific rules are being disabled or consider addressing them if possible.
components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs (1)
4-5: Consider addressing ESLint disabled rules.While disabling these rules may be intentional for sandbox components, it would be good to document why these specific rules are being disabled or consider addressing them if possible.
components/procore_sandbox/actions/create-rfi/create-rfi.mjs (1)
4-5: Consider addressing ESLint disabled rules.While disabling these rules may be intentional for sandbox components, it would be good to document why these specific rules are being disabled or consider addressing them if possible.
components/procore_sandbox/actions/create-submittal/create-submittal.mjs (1)
4-5: Consider addressing ESLint disabled rules.While disabling these rules may be intentional for sandbox components, it would be good to document why these specific rules are being disabled or consider addressing them if possible.
components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
4-6: ESLint disables should be temporaryThe ESLint disable comments for required properties should be addressed before the final version. While they might be necessary during development, make sure to follow Pipedream's linting requirements for production code.
components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (2)
4-6: ESLint disables should be temporaryThe ESLint disable comments for required properties should be addressed before the final version. While they might be necessary during development, make sure to follow Pipedream's linting requirements for production code.
14-15: Remove unnecessary trailing newlineThere's an extra empty line at the end of the file that should be removed.
version: "0.0.1", }; -components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
4-6: ESLint disables should be temporaryThe ESLint disable comments for required properties should be addressed before the final version. While they might be necessary during development, make sure to follow Pipedream's linting requirements for production code.
components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs (1)
4-6: ESLint disables should be temporaryThe ESLint disable comments for required properties should be addressed before the final version. While they might be necessary during development, make sure to follow Pipedream's linting requirements for production code.
components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (1)
16-37: Add error handling for missing projectIdThe
getDataToEmitmethod doesn't have any error handling or log message when theprojectIdis missing. This may cause issues if the Procore API requires aprojectIdto retrieve timecard entry details.Consider adding a check and log message when
projectIdis not provided:async getDataToEmit(body) { const { app, companyId, } = this; const { resource_id: timecardEntryId, project_id: projectId, } = body; + if (!projectId) { + console.log("If you need to get more details about the timecard entry, please provide a project ID."); + return body; + } + const resource = await app.getTimecardEntry({ companyId, timecardEntryId, params: { project_id: projectId, }, }); return { ...body, resource, }; },components/procore_sandbox/common/utils.mjs (2)
3-33: Optimize the reduce method to avoid O(n²) time complexityThe current implementation of
buildPropDefinitionsuses spread syntax on accumulators in the reduce function, which can lead to O(n²) time complexity for large objects.Consider refactoring to avoid spread on the accumulator:
function buildPropDefinitions({ app = {}, props = {}, }) { return Object.entries(props) .reduce((newProps, [ key, prop, ]) => { if (!prop.propDefinition) { - return { - ...newProps, - [key]: prop, - }; + newProps[key] = prop; + return newProps; } const [ , ...propDefinitionItems ] = prop.propDefinition; - return { - ...newProps, - [key]: { - ...prop, - propDefinition: [ - app, - ...propDefinitionItems, - ], - }, - }; + newProps[key] = { + ...prop, + propDefinition: [ + app, + ...propDefinitionItems, + ], + }; + return newProps; }, {}); }🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
35-50: Consider adding error handling for missing component propertiesThe
getAppPropsfunction doesn't have any error handling for cases wherecomponentorcomponent.propsmight be undefined or null. This could cause runtime errors in edge cases.Consider adding more robust error handling:
function getAppProps(component = {}) { + if (!component || !component.props) { + return { props: { app } }; + } + const { // eslint-disable-next-line no-unused-vars app: procore, ...otherProps } = component.props; return { props: { app, ...buildPropDefinitions({ app, props: otherProps, }), }, }; }components/procore/actions/create-submittal/create-submittal.mjs (3)
116-116: Fix typo in success messageThere's a typo in the success message: "Succesfully" should be "Successfully".
- $.export("$summary", `Succesfully created submittal with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`);
77-85: Add error handling in createSubmittal methodThe
createSubmittalmethod doesn't include any error handling for API failures. Consider adding try/catch or allowing error propagation for better debugging.For example:
createSubmittal({ projectId, ...args } = {}) { + if (!projectId) { + throw new Error("Project ID is required to create a submittal"); + } return this.app.post({ versionPath: constants.VERSION_PATH.V1_1, path: `/projects/${projectId}/submittals`, ...args, }); },
10-75: Consider adding validation for required fieldsThe component requires several fields but doesn't validate them before making the API call. Consider adding validation for required fields like
number.components/procore/actions/create-incident/create-incident.mjs (4)
100-100: Fix typo in success messageThere's a typo in the success message: "Succesfully" should be "Successfully".
- $.export("$summary", `Succesfully created incident with ID \`${response.id}\`.`); + $.export("$summary", `Successfully created incident with ID \`${response.id}\`.`);
63-70: Add error handling in createIncident methodThe
createIncidentmethod doesn't include any error handling for API failures or for cases where required parameters are missing. Consider adding validation for theprojectIdparameter.For example:
createIncident({ projectId, ...args } = {}) { + if (!projectId) { + throw new Error("Project ID is required to create an incident"); + } return this.app.post({ path: `/projects/${projectId}/incidents`, ...args, }); },
32-36: Improve eventDate documentation and validationThe
eventDatefield description is good, but adding validation would help users provide correctly formatted dates.Consider adding a more specific type or validation in the run method:
eventDate: { type: "string", label: "Event Date", description: "Iso8601 datetime of Incident occurrence. If time is unknown, send in the date at `0:00` project time converted to UTC. Eg. `2025-04-01T00:00:00Z`.", + pattern: "^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z$", + examples: ["2025-04-01T00:00:00Z"], },Or add validation in the run method before making the API call.
72-102: Add validation for required fields before API callThe component requires several fields but doesn't validate them before making the API call. Consider adding validation for required fields like
titleandeventDate.async run({ $ }) { const { createIncident, companyId, projectId, title, description, eventDate, isPrivate, recordable, timeUnknown, } = this; + // Validate required fields + if (!title) { + throw new Error("Title is required"); + } + if (!eventDate) { + throw new Error("Event Date is required"); + } + const response = await createIncident({ $, companyId, projectId, data: { incident: { title, description, event_date: eventDate, private: isPrivate, recordable, time_unknown: timeUnknown, }, }, }); $.export("$summary", `Succesfully created incident with ID \`${response.id}\`.`); return response; },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
26-32: Catch potential errors when retrieving prime contract
IfgetPrimeContractfails, the code will throw an unhandled error. A try-catch block or equivalent error handling pattern can improve resilience.components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
38-45: Validate timestamp presence.The
generateMetafunction correctly extracts a timestamp frombody.timestamp. Ifbody.timestampis missing or not in a valid format,new Date(body.timestamp).getTime()may returnNaN. Consider defensive checks to ensure a valid timestamp is available.generateMeta(body) { + if (!body.timestamp || Number.isNaN(new Date(body.timestamp).getTime())) { + console.warn("Missing or invalid timestamp. Defaulting to current time."); + return { + id: body.id, + summary: `New Purchase Order Event: ${body.resource_id}`, + ts: Date.now(), + }; + } return { id: body.id, summary: `New Purchase Order Event: ${body.resource_id}`, ts: new Date(body.timestamp).getTime(), }; },components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs (1)
39-45: Check for invalid timestamps.As with similar event sources, consider handling cases where
body.timestampmight be invalid or missing.components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
42-48: Check for invalid or missing timestamps.
new Date(body.timestamp).getTime()can returnNaNifbody.timestampis undefined or invalid. Consider a fallback or validation to avoid potential errors or ambiguous timestamps.components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
38-44: Validate timestamp usage.
new Date(body.timestamp).getTime()might result in an invalid timestamp ifbody.timestampis null or incorrectly formatted. Validate thattimestampis a valid date to avoid erroneous or missing event times.components/procore/sources/common/webhook.mjs (1)
220-236: Handle missing or malformed body data.If
bodylacks the expected fields (e.g.,resource_id,timestamp),processResource(resource)might behave unexpectedly. Consider validating required fields or gracefully handling incomplete requests.
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/submittal/submittal.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/common.js
- components/procore/procore.app.js
- components/procore/sources/new-event/new-event.js
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/sources/rfi/rfi.js
✅ Files skipped from review due to trivial changes (2)
- components/procore_sandbox/package.json
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore/common/utils.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (48)
components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (2)
1-13: Code follows the standard sandbox component pattern.The implementation correctly extends the main Procore component while applying sandbox-specific properties. This sandbox wrapper pattern ensures testing can be done in isolation from production components.
10-12: Verify the component key and description.The key correctly follows the naming convention with the
procore_sandbox-prefix. The description includes a link to the API documentation, which is helpful for developers.components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs (2)
1-13: Code follows the standard sandbox component pattern.The implementation correctly extends the main Procore component while applying sandbox-specific properties. This sandbox wrapper pattern is consistent with other components in this PR.
10-12: Verify the component key and description.The key correctly follows the naming convention with the
procore_sandbox-prefix. The description explains that the component emits events based on selected resource names and includes a helpful link to the API documentation.components/procore_sandbox/actions/create-rfi/create-rfi.mjs (2)
1-13: Code follows the standard sandbox component pattern.The implementation correctly extends the main Procore component while applying sandbox-specific properties. This sandbox wrapper pattern is consistent with the other components in this PR.
10-12: Verify the component key and description.The key correctly follows the naming convention with the
procore_sandbox-prefix. The description clearly explains the action's purpose and includes a helpful link to the API documentation.components/procore_sandbox/actions/create-submittal/create-submittal.mjs (3)
1-13: Code follows the standard sandbox component pattern.The implementation correctly extends the main Procore component while applying sandbox-specific properties. This sandbox wrapper pattern is consistent across all the components in this PR.
10-12: Verify the component key and description.The key correctly follows the naming convention with the
procore_sandbox-prefix. The description clearly explains the action's purpose and includes a helpful link to the API documentation.
7-13: Remember to add tests for these components.Per the PR objectives, these components are currently untested. Consider adding tests to verify their functionality before merging to production.
components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs (1)
1-13: Code structure looks good but needs testing as indicated in PR titleThe component correctly extends the main Procore component and utilizes common utilities. The ESLint disables for required properties appear necessary for this pattern, and the documentation link is properly included.
Since the PR title mentions these components are "untested", I recommend adding test cases to verify the webhook functionality works correctly with the Procore sandbox environment before merging.
Could you confirm that this component has been manually tested with the Procore sandbox API? If not, consider adding automated tests or documenting test steps to verify this component's functionality.
components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs (1)
1-13: Component structure is consistent with other sandbox componentsThe implementation correctly extends the main Procore component and incorporates necessary properties for the sandbox environment. The version numbering (0.0.1) appropriately indicates this is a new component.
As mentioned in the PR title that these components are "untested", ensure you add proper test coverage for this action component.
Have you verified that the API endpoint in the documentation link works correctly in the sandbox environment? The timesheet creation functionality should be tested with various input combinations to ensure robustness.
components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs (1)
1-13: Implementation follows the established pattern for sandbox componentsThis component correctly extends the main Procore component and follows the same pattern as other sandbox components. The documentation link provides a good reference for users.
As with the other components in this PR, testing is needed according to the PR title.
Consider adding integration tests that verify webhook registration and event processing specifically for submittal events in the sandbox environment. This will ensure the component behaves as expected when deployed.
components/procore_sandbox/actions/create-incident/create-incident.mjs (1)
1-13: Follows consistent implementation pattern but requires testingThe component correctly extends the main Procore component and maintains consistency with other sandbox components. The documentation link is appropriate for the create incident functionality.
Since these components are marked as "untested" in the PR title, testing should be prioritized.
Before finalizing this PR, I recommend:
- Creating a test plan for each component
- Verifying API responses in the sandbox environment
- Ensuring error handling is properly implemented for scenarios like invalid incident details
This will help prevent issues when these components are used in production workflows.
components/procore_sandbox/procore_sandbox.app.mjs (1)
1-11: Clean implementation of environment-specific appNice refactoring to inherit the base Procore functionality while maintaining sandbox-specific differences. The use of the spread operator for both the main export and methods object provides clean inheritance while the
getEnvironment()method appropriately identifies this as a sandbox environment.components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (2)
7-13: Consistent component extension patternGood work following the established pattern for extending components across environments. The sandbox implementation cleanly inherits from the base component while providing its unique key and proper versioning.
4-6:❓ Verification inconclusive
Consider addressing ESLint disabled rules
While the current implementation works, you've disabled two ESLint rules that are typically important for Pipedream components. Is there a specific reason why
typeandnameproperties are omitted?
🏁 Script executed:
#!/bin/bash # Check if other event sources follow the same pattern with disabled ESLint rules grep -r "eslint-disable pipedream/required-properties" --include="*.mjs" components/procore*/sources/Length of output: 2976
Attention: Clarify ESLint Rule Disabling in Event Sources
It appears that disabling the
piperdream/required-properties-typeandpiperdream/required-properties-namerules is a common pattern across all event instant source files. If this is an intentional decision, please add a brief inline comment that explains why these properties are omitted. Otherwise, consider updating the implementation to include the missingtypeandnameproperties to conform with standard Pipedream component practices.
- File:
components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(and similar files across the repository)- Action: Add an inline rationale if the disables are intentional, or address the missing properties if not.
components/procore/common/resource-names.mjs (1)
1-123: Well-organized resource name constantsGreat job creating a centralized module for Procore resource names. The comprehensive list with consistent naming conventions will make the code more maintainable and less error-prone. The comments explaining the source of the constants are also helpful.
components/procore/package.json (3)
3-3: Version increment follows semantic versioningIncreasing from 0.0.4 to 0.1.0 is appropriate for a minor feature release with backward-compatible additions.
5-5: Conversion to ES modulesThe change from
.jsto.mjsfor the main entry point aligns with modern JavaScript practices and explicitly indicates ES module usage.
16-17:✅ Verification successful
Verify uuid package version
The uuid version
^11.1.0appears unusually high - the latest stable release as of early 2024 was in the 9.x range. Please verify this is the correct version you intend to use.
🏁 Script executed:
#!/bin/bash # Check latest version of uuid package npm view uuid versions | tail -n 5 # Check if this version is being used in other Pipedream components grep -r "\"uuid\":" --include="package.json" components/Length of output: 283
UUID version confirmed: The version specified in
components/procore/package.json(^11.1.0) is valid based on the npm registry evidence. The registry now includes versions in the 11.x range (with 11.1.0 as the latest), so the concern that the version appears unusually high compared to earlier 9.x releases no longer applies.
- The npm registry output confirms that version 11.1.0 is a valid stable release.
- The previous assertion regarding an unusually high version is now outdated.
- Please disregard the suggestion to change the uuid version unless further usage-related issues arise.
components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (1)
7-7: Key now matches file/folder nameThe component key now correctly matches the folder and file name structure. This resolves the previous issue that was causing the pipeline to fail.
components/procore/actions/create-timesheet/create-timesheet.mjs (2)
27-31: Consider adding explicit date format validation
The timesheet date must comply with Procore’sYYYY-MM-DDformat requirements, but the code does not enforce or validate it.
34-37: Add a default parameter object
Unlike other actions,createTimesheet({ projectId, ...args })does not define a default argument, which may lead to errors when invoked without arguments.components/procore/actions/create-manpower-log/create-manpower-log.mjs (2)
73-80: Improve error handling
Adding a try-catch block around thecreateManpowerLog()API call helps surface errors clearly if the request fails.
44-49:❓ Verification inconclusive
Revisit the data type for
numHours
numHoursis defined as a string, but it may be more appropriate to model it as an integer or float, especially if it represents the duration of work.Run the following script to search for usage of
numHoursand verify whether it is indeed expected to be numeric:
🏁 Script executed:
#!/bin/bash rg -A4 'numHours'Length of output: 1414
Revisit the data type for
numHoursIt’s currently defined as a string (lines 44–49 of
components/procore/actions/create-manpower-log/create-manpower-log.mjs), but since it represents the number of hours worked, it might be more appropriate to use a numeric type (e.g., number or float). Please verify whether downstream processes expect a numeric value and consider updating the schema to reflect this if so.components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (4)
1-3: Imports look good.These imports from shared modules are set up consistently.
4-10: Event source definition is clear.The basic configuration (name, key, description, version, type) effectively describes the event source. No issues spotted.
11-15: Resource name method is concise.Defining the resource name in a separate method aids clarity and reusability. No concerns here.
16-37: Add error handling for the API call.Although functional, the
getDataToEmitmethod does not handle potential errors when callingapp.getPurchaseOrderContract. If this call fails (e.g., due to connectivity or authorization issues), the error will propagate upward and may halt the source’s event emission. A try/catch block (or equivalent error-handling strategy) can help gracefully handle failures.This recommendation replicates a prior comment from an older revision.
async getDataToEmit(body) { const { app, companyId, } = this; const { resource_id: purchaseOrderContractId, project_id: projectId, } = body; - const resource = await app.getPurchaseOrderContract({ - companyId, - purchaseOrderContractId, - params: { - project_id: projectId, - }, - }); - return { - ...body, - resource, - }; + try { + const resource = await app.getPurchaseOrderContract({ + companyId, + purchaseOrderContractId, + params: { + project_id: projectId, + }, + }); + return { + ...body, + resource, + }; + } catch (error) { + console.error(`Error fetching purchase order contract details: ${error.message}`); + // Return the original webhook payload so we don't disrupt further processing + return body; + } }components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs (4)
1-3: Imports look good.Imports from shared modules and resource names are consistent with the other event sources.
4-10: Event source configuration is straightforward.No issues found with the basic event source settings.
11-15: Resource name is well-defined.Use of the shared resource-names module ensures consistency.
16-38: Add error handling for the API call.Similar to the purchase order event source, the
getDataToEmitmethod should handle potential errors fromapp.getChangeOrderPackage. An unhandled error can crash the webhook flow.This repeats a prior suggestion from older commits.
async getDataToEmit(body) { const { app, companyId, } = this; const { resource_id: changeOrderPackageId, project_id: projectId, } = body; - const resource = await app.getChangeOrderPackage({ - companyId, - changeOrderPackageId, - params: { - project_id: projectId, - }, - }); - return { - ...body, - resource, - }; + try { + const resource = await app.getChangeOrderPackage({ + companyId, + changeOrderPackageId, + params: { + project_id: projectId, + }, + }); + return { + ...body, + resource, + }; + } catch (error) { + console.error(`Error fetching change order package details: ${error.message}`); + return body; + } }components/procore/procore.app.mjs (9)
1-5: Imports and support modules are appropriate.The usage of
axiosfrom@pipedream/platform, along with internal constants, resource names, and utilities, is consistent and clear.
6-8: Application definition is well-formed.Specifying
type: "app"andapp: "procore"is straightforward and aligns with typical patterns.
9-172: Property definitions are thorough and user-friendly.Each prop definition (companyId, projectId, etc.) is described well, returning dynamic options with pagination. This makes for a good user experience in the UI.
186-202: getUrl and getHeaders are neatly structured.They concisely establish the base URL and request headers, accommodating the environment, versioning, and optional company scope. No issues noted.
203-211: Centralized request logic improves maintainability.The
makeRequestmethod standardizes HTTP requests, which is helpful for consistency across different methods. Good approach.
224-236: Listing companies and projects with versioned endpoints.The usage of
versionPath: constants.VERSION_PATH.V1_1for the/projectsendpoint is correct, reflecting the Procore API’s versioning approach.
262-269: Use plural endpoint for prime contracts.The Procore API documentation indicates the path for prime contracts is
/prime_contracts/. The current code uses a singular form (/prime_contract/), which may cause 404 or other errors.This echoes an earlier comment provided in a previous review.
getPrimeContract({ primeContractId, ...args } = {}) { return this.makeRequest({ - path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`, ...args, }); },
337-381: Add robust error handling to pagination logic.In
getIterations, an error inresourcesFncurrently escapes unhandled. Implementing a try/catch around the request would prevent the entire iteration from silently failing and provide a clearer error message.This recommendation has been previously suggested.
while (true) { + try { const response = await resourcesFn({ ...resourcesFnArgs, params: { ...resourcesFnArgs?.params, page, per_page: constants.DEFAULT_LIMIT, }, }); const nextResources = resourceName ? utils.getNestedProperty(response, resourceName) : response; if (!nextResources?.length) { console.log("No more resources found"); return; } for (const resource of nextResources) { yield resource; resourcesCount += 1; if (resourcesCount >= max) { console.log("Reached max resources"); return; } } if (nextResources.length < constants.DEFAULT_LIMIT) { console.log("No next page found"); return; } page += 1; + } catch (error) { + console.error(`Error fetching resources on page ${page}: ${error.message}`); + throw error; // Avoid silent failures + } }
382-387: Iterating helper is consistent.The
paginatemethod properly wraps the generator for convenience. This is a good pattern to unify iteration logic.components/procore/common/constants.mjs (3)
50-65: LGTM overall.Exporting a consolidated object of constants simplifies cross-module references and promotes maintainability. No immediate issues detected here.
7-7:❓ Verification inconclusive
Consider validating placeholder usage.
BASE_URLrelies on string placeholders (ENV_PLACEHOLDERandVERSION_PLACEHOLDER). Ensure these placeholders are always replaced with valid values at runtime to avoid unexpected API call failures or malformed requests.
🏁 Script executed:
#!/bin/bash # Description: Check all usage occurrences of BASE_URL to ensure placeholders are replaced. rg -A 3 "BASE_URL"Length of output: 66344
Action: Validate both runtime replacements for
ENV_PLACEHOLDERandVERSION_PLACEHOLDER.The search output confirms that in
components/procore/common/constants.mjstheBASE_URLis defined with placeholders. In the corresponding app code (e.g. incomponents/procore/procore.app.mjs), we see theENV_PLACEHOLDERis replaced via a string replacement, but there’s no similar evidence thatVERSION_PLACEHOLDERis being substituted at runtime. Please review the usage to ensure that both placeholders are always updated with valid values (or fallbacks) to avoid malformed API URLs or unexpected API call failures.
- Verify that runtime logic properly replaces both
ENV_PLACEHOLDERandVERSION_PLACEHOLDER.- If not, consider adding validations or default values for any missing substitutions.
31-37:✅ Verification successful
Ensure that constant names align with their usage.
While these constants (
TOKEN,IS_FIRST_RUN,LAST_DATE_AT) indicate keys for storing state, confirm they're actually used in the codebase with consistent naming. Unused or misnamed constants can lead to confusion and technical debt.
🏁 Script executed:
#!/bin/bash # Description: Verify usage references for these constants in the codebase. rg "(TOKEN|IS_FIRST_RUN|LAST_DATE_AT)"Length of output: 8824
Constants Naming Verification Complete
The constants
TOKEN,IS_FIRST_RUN, andLAST_DATE_ATdefined incomponents/procore/common/constants.mjsare used consistently in the procore components (e.g., they are referenced with the expected string values"token","isFirstRun", and"lastDateAt"). This confirms that in the context of procore, the naming aligns with their usage.
- Usage in procore state handling (e.g., in
/sources/common/webhook.mjs) correctly refers to these constants.- Note: There is a slight naming discrepancy in another module—
components/dropinblog/common/constants.mjsdefinesLAST_DATE_ATas"lastCreatedAt". Consider reviewing this for global consistency if needed.components/procore/sources/common/webhook.mjs (3)
176-178: Remind implementers to provide the resource name.
getResourceName()also throws aConfigurationError. Ensure that each derived module implements it to avoid errors when creating webhooks.
153-157:✅ Verification successful
Clarify placeholder methods.
generateMeta()throws aConfigurationError, indicating child classes must override it. This is a valid design choice, but ensure each child source indeed overrides it to prevent runtime errors.
🏁 Script executed:
#!/bin/bash # Description: Verify that all derived classes override generateMeta properly. rg -A 5 "generateMeta"Length of output: 105963
Ensure all sources override the placeholder generateMeta() method
The implementation in components/procore/sources/common/webhook.mjs is an intentional placeholder that throws a
ConfigurationErrorto signal that derived classes must provide their own implementation. Our verification confirms that almost all child source files overridegenerateMeta()appropriately. Please ensure that any new source extending this base also provides a custom implementation to avoid runtime errors.
79-81:❓ Verification inconclusive
Confirm token generation meets security requirements.
While generating a base64-encoded UUID usually provides enough randomness, verify that this approach satisfies security requirements. If stronger entropy is needed, consider using a cryptographically secure token.
🏁 Script executed:
#!/bin/bash # Description: Check references to token usage; ensure no stricter cryptography is needed. rg "token"Length of output: 67317
Token Generation Security – Confirm Cryptographic Robustness
After reviewing the token usage across the codebase, the pattern used in
components/procore/sources/common/webhook.mjs—generating a UUID and encoding it in base64—is in line with how tokens are handled elsewhere. That said, please verify that theuuid()function is indeed producing a cryptographically secure (v4) UUID. If this token is used in a security‐critical context, you might consider switching to a method such as Node’s built-incrypto.randomBytesto provide stronger entropy.
...rces/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
Show resolved
Hide resolved
...rocore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
Show resolved
Hide resolved
...rocore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
Show resolved
Hide resolved
components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
Show resolved
Hide resolved
...onents/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
Show resolved
Hide resolved
jcortes
force-pushed
the
procore-new-components
branch
from
a294ed8 to
6800610
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
components/procore/actions/create-timesheet/create-timesheet.mjs (3)
34-41: 🛠️ Refactor suggestionAdd default parameter and improve parameter handling
Unlike other action methods, this one doesn't provide a default empty object for parameters, which could lead to errors if called without arguments.
Apply this improvement:
createTimesheet({ - projectId, ...args + $, projectId, companyId, data, ...args - }) { + } = {}) { return this.app.post({ path: `/projects/${projectId}/timesheets`, + $, + companyId, + data, ...args, }); },
27-31: 🛠️ Refactor suggestionValidate Timesheet Date to Use ISO 8601 Format
The current implementation lacks explicit validation for the timesheet's date property. Procore expects dates to adhere to the ISO 8601 standard.
Add validation in the run method:
async run({ $ }) { const { createTimesheet, companyId, projectId, date, } = this; + // Validate date is in ISO 8601 format (YYYY-MM-DD) + const dateRegex = /^\d{4}-\d{2}-\d{2}$/; + if (!dateRegex.test(date)) { + throw new Error("Date must be in ISO 8601 format (YYYY-MM-DD)"); + } const response = await createTimesheet({
43-63: 🛠️ Refactor suggestionImplement error handling
Currently, if the API call fails, no exception is caught, potentially causing a silent failure.
Implement error handling:
async run({ $ }) { const { createTimesheet, companyId, projectId, date, } = this; + try { const response = await createTimesheet({ $, companyId, projectId, data: { timesheet: { date, }, }, }); $.export("$summary", `Successfully created timesheet with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create timesheet: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
73-80: 🛠️ Refactor suggestionAdd better error handling for API failures
Currently, if an error occurs during
createManpowerLog(), the action might silently fail at runtime.Implement error handling:
createManpowerLog({ projectId, ...args } = {}) { + try { return this.app.post({ path: `/projects/${projectId}/manpower_logs`, ...args, }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
38-44: 🛠️ Refactor suggestionValidate Timestamp Presence and Validity
In the
generateMetafunction, ifbody.timestampis missing or not in a valid format,new Date(body.timestamp).getTime()will yieldNaN, which can cause issues downstream.Add validation for the timestamp:
generateMeta(body) { + const timestamp = body.timestamp + ? new Date(body.timestamp).getTime() + : Date.now(); + + // Check if timestamp is valid + const ts = isNaN(timestamp) ? Date.now() : timestamp; return { id: body.id, summary: `New Prime Contract Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts, }; },
🧹 Nitpick comments (12)
components/procore_sandbox/common/utils.mjs (5)
3-33: Consider optimizing the reduce operation for performanceThe
buildPropDefinitionsfunction uses spread syntax within a reducer, which can lead to O(n²) time complexity as flagged by static analysis. For larger objects, this could cause performance issues.function buildPropDefinitions({ app = {}, props = {}, }) { - return Object.entries(props) - .reduce((newProps, [ - key, - prop, - ]) => { - if (!prop.propDefinition) { - return { - ...newProps, - [key]: prop, - }; - } - - const [ - , ...propDefinitionItems - ] = prop.propDefinition; - - return { - ...newProps, - [key]: { - ...prop, - propDefinition: [ - app, - ...propDefinitionItems, - ], - }, - }; - }, {}); + const newProps = {}; + for (const [key, prop] of Object.entries(props)) { + if (!prop.propDefinition) { + newProps[key] = prop; + continue; + } + + const [, ...propDefinitionItems] = prop.propDefinition; + + newProps[key] = { + ...prop, + propDefinition: [ + app, + ...propDefinitionItems, + ], + }; + } + return newProps; }🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
35-50: Clarify the purpose of the unused variableThe
procorevariable is explicitly marked as unused with an eslint comment. If this variable is truly unnecessary, consider removing the renaming.function getAppProps(component = {}) { const { - // eslint-disable-next-line no-unused-vars - app: procore, + app: _, // Ignoring the component's app property as we use the imported app ...otherProps } = component.props; return { props: { app, ...buildPropDefinitions({ app, props: otherProps, }), }, }; }
1-55: Add JSDoc comments for better function documentationThe utility functions lack JSDoc comments explaining their purpose, parameters, and return values, which would improve code readability and maintainability.
import app from "../procore_sandbox.app.mjs"; +/** + * Builds property definitions by ensuring each prop with a propDefinition + * includes the app as the first element in the propDefinition array. + * + * @param {Object} options - The options object + * @param {Object} options.app - The app object to include in prop definitions + * @param {Object} options.props - The properties to process + * @return {Object} A new object with processed properties + */ function buildPropDefinitions({ app = {}, props = {}, }) { // ... existing implementation } +/** + * Creates a new props object that includes the imported app and + * processed properties from the component. + * + * @param {Object} component - The component object + * @param {Object} component.props - The component's properties + * @return {Object} An object with a props property containing the app and processed properties + */ function getAppProps(component = {}) { // ... existing implementation } export default { buildPropDefinitions, getAppProps, };🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
1-55: Add error handling for edge casesThe functions lack error handling for potential edge cases, such as if
prop.propDefinitionexists but isn't an array, or ifcomponent.propsis undefined.function buildPropDefinitions({ app = {}, props = {}, }) { return Object.entries(props) .reduce((newProps, [ key, prop, ]) => { - if (!prop.propDefinition) { + if (!prop.propDefinition || !Array.isArray(prop.propDefinition)) { return { ...newProps, [key]: prop, }; } const [ , ...propDefinitionItems ] = prop.propDefinition; return { ...newProps, [key]: { ...prop, propDefinition: [ app, ...propDefinitionItems, ], }, }; }, {}); } function getAppProps(component = {}) { + if (!component || !component.props) { + return { props: { app } }; + } + const { // eslint-disable-next-line no-unused-vars app: procore, ...otherProps } = component.props; return { props: { app, ...buildPropDefinitions({ app, props: otherProps, }), }, }; }🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
1-2: Consider adding a comment explaining the purpose of this moduleAdding a file-level comment would help developers understand the overall purpose of this utility module within the Procore integration.
+/** + * Utility functions for Procore component properties management. + * These utilities help standardize how component properties are handled + * across different Procore integration components. + */ import app from "../procore_sandbox.app.mjs";components/procore/actions/create-submittal/create-submittal.mjs (1)
28-32: Consider adding validation for the required number fieldThe number field is required but lacks validation to ensure it's not empty before submission.
Add validation in the run method:
async run({ $ }) { const { createSubmittal, companyId, projectId, number, description, title, type, isPrivate, revision, locationId, } = this; + + if (!number?.trim()) { + throw new Error("Number is required for creating a submittal"); + }components/procore/actions/create-manpower-log/create-manpower-log.mjs (2)
32-37: Add validation for datetime formatThe datetime field description specifies a format (
2025-04-01T00:00:00Z), but there's no validation to ensure the input conforms to this format.Add validation in the run method:
async run({ $ }) { const { createManpowerLog, companyId, projectId, datetime, notes, numWorkers, numHours, userId, locationId, } = this; + // Validate datetime format if provided + if (datetime) { + const isoDatetimeRegex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/; + if (!isoDatetimeRegex.test(datetime)) { + throw new Error("Datetime must be in ISO 8601 format (YYYY-MM-DDThh:mm:ssZ)"); + } + } const response = await createManpowerLog({
44-49: Consider changing numHours type to numeric
numHoursis defined as a string type, but it represents a numeric value (hours). This might cause issues if calculations are performed on this value.Change the type to numeric:
numHours: { - type: "string", + type: "number", label: "Number Of Hours", description: "The number of hours for each worker", optional: true, },components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
42-48: Handle invalid timestamps.
Ifbody.timestampis missing or invalid,new Date(body.timestamp).getTime()might yieldNaN. Consider a safer fallback:generateMeta(body) { return { id: body.id, summary: `New Budget Snapshot Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: isNaN(new Date(body.timestamp).getTime()) + ? Date.now() + : new Date(body.timestamp).getTime(), }; },components/procore/sources/common/webhook.mjs (3)
35-63: Consider supporting multi-page project listings.Currently,
additionalProps()fetches only one page of data. If users have more than 100 projects, they must manually page through them. For a smoother user experience, consider iterating over all pages or allowing a “Load more” mechanism.
64-120: Handle partial trigger creation failures.When bulk-creating triggers, a single trigger failure may cause partial success. Consider adding checks or a fallback to ensure the overall flow is more robust, especially if one or more triggers cannot be created.
220-237: Add test coverage for therun()method.The request handling logic, token validation, and response codes appear correct, but there are no tests to confirm they behave as intended. Consider adding tests to verify the success and failure paths.
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/new-event/new-event.js
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/common.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/submittal/submittal.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/procore.app.js
🚧 Files skipped from review as they are similar to previous changes (28)
- components/procore_sandbox/package.json
- components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs
- components/procore_sandbox/procore_sandbox.app.mjs
- components/procore_sandbox/actions/create-rfi/create-rfi.mjs
- components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
- components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/common/resource-names.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/package.json
- components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
- components/procore_sandbox/actions/create-submittal/create-submittal.mjs
- components/procore/common/utils.mjs
- components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore_sandbox/actions/create-incident/create-incident.mjs
- components/procore/actions/create-incident/create-incident.mjs
- components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (8)
components/procore/procore.app.mjs (2)
262-269: Use plural endpoint for prime contracts.
A previous review recommended using the plural form/prime_contracts/for this endpoint. The current code still uses the singular form, which might not match Procore's REST API specification.- path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`,
337-381: Add robust error handling to pagination logic.
A previous review suggested wrapping the call toresourcesFnin a try/catch block within thewhile (true)loop to gracefully handle potential network or API errors.components/procore/common/constants.mjs (1)
1-66: All constants look good!
The constants here are clearly defined, consistently named, and well-organized for maintainability.components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
26-35: Consider adding error handling to pagination.
As seen in similar modules, if thepaginatecall fails, the error might bubble up. A try/catch could safeguard the event emission process.components/procore/sources/common/webhook.mjs (4)
1-34: Looks solid overall.The imports and base props (including
webhookType) are clearly defined. This section appears functionally correct.
121-152: Deactivation logic is consistent.Deleting triggers and the hook in sequence is correct. No issues or improvements stand out here.
155-157:generateMeta()is still unimplemented.This repeats a prior note: since this method is invoked (line 218) but throws an error, any actual webhook event will cause a runtime error. Either provide a basic implementation or override it in a subclass.
176-178:getResourceName()is still unimplemented.This method is required (line 103) to build triggers for the chosen resource. Overriding it with the correct resource name or implementing logic dynamically would avoid the
ConfigurationError.
...onents/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
Show resolved
Hide resolved
jcortes
changed the title
jcortes
force-pushed
the
procore-new-components
branch
from
6800610 to
ee74d49
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
components/procore_sandbox/common/utils.mjs (3)
1-55: Great utility functions for prop management, with minor performance concerns.The utility functions effectively handle component properties and app context integration. However, there are performance optimizations that could be made.
The static analysis tool correctly identified potential performance issues with using the spread operator in reducers. This can lead to O(n²) time complexity when working with large objects:
function buildPropDefinitions({ app = {}, props = {}, }) { return Object.entries(props) .reduce((newProps, [ key, prop, ]) => { if (!prop.propDefinition) { - return { - ...newProps, - [key]: prop, - }; + newProps[key] = prop; + return newProps; } const [ , ...propDefinitionItems ] = prop.propDefinition; - return { - ...newProps, - [key]: { - ...prop, - propDefinition: [ - app, - ...propDefinitionItems, - ], - }, - }; + newProps[key] = { + ...prop, + propDefinition: [ + app, + ...propDefinitionItems, + ], + }; + return newProps; }, {}); }This approach mutates the accumulator directly, which is more efficient for large objects.
🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
35-50: Consider documenting the purpose of the utility functions.The
getAppPropsfunction swaps out the component's app property with the imported app, which is an important behavior that should be documented./** * Processes component props by replacing the component's app property * with the imported app instance and processing all other props * through buildPropDefinitions. * @param {Object} component - The component with props to process * @returns {Object} An object with processed props */ function getAppProps(component = {}) {
36-40: Unused variable could be simplified.The eslint comment acknowledges that
procoreis unused, but there's a cleaner approach.const { - // eslint-disable-next-line no-unused-vars - app: procore, + app: _, // Explicitly ignore ...otherProps } = component.props;Or even simpler:
const { - // eslint-disable-next-line no-unused-vars - app: procore, + app, // We can remove this line entirely if we don't need it ...otherProps } = component.props;components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs (1)
36-40: Consider improving error handlingThe current approach of simply logging errors and returning the original body is functional but basic. Consider adding more detailed error reporting or retrying mechanisms for more robust webhook processing.
} catch (error) { - console.log(error.message || error); + console.error(`Failed to fetch submittal data: ${error.message || error}`); + // Optional: Add monitoring or reporting for failed webhook enrichment return body; }components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
44-49: Consider changing numHours type to numericThe
numHoursproperty is defined as a string, but it represents a numeric value. Using a numeric type would be more appropriate for this field.numHours: { - type: "string", + type: "number", label: "Number Of Hours", description: "The number of hours for each worker", optional: true, },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
38-41: Enhance error logging with detailed informationThe current error handling simply logs the error message. Consider adding more context to help with debugging.
} catch (error) { - console.log(error.message || error); + console.error(`Failed to fetch prime contract data for ID ${primeContractId} in project ${projectId}: ${error.message || error}`); return body; }components/procore/procore.app.mjs (1)
174-176: Make the environment configurable for improved testing flexibility
Right now, thegetEnvironment()method always returns production. If you plan on expanding support for sandbox or other environments, consider making this user-configurable or determined by environment variables.components/procore/sources/common/webhook.mjs (4)
35-63: Handle large project lists gracefully.
Currently, theoptionsmethod fetches a maximum of 100 projects at once. Consider adding pagination logic or caching if you anticipate more than 100 projects, to avoid partial listings.
65-120: Consider robust error handling during activation.
Theactivatehook lacks fallback or rollback logic ifcreateWebhooksHook()orbulkCreateWebhooksTriggers()partially fails (e.g., some triggers fail to create). Consider a try/catch block or transaction-like approach to handle partial failures more gracefully.
121-152: Consider robust error handling during deactivation.
Similar to theactivatehook, thedeactivatehook relies onbulkDeleteWebhooksTriggers()anddeleteWebhooksHook(). If there is a partial failure, it could leave inconsistent webhook data. Adding a fallback or retry approach would improve reliability.
158-163: Consider storing tokens securely.
Storing the token as plain text in the database might be fine for a small integration, but for better security, you may want to encrypt or obfuscate it, or move it to an environment variable if needed.
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/common.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/submittal/submittal.js
- components/procore/procore.app.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/new-event/new-event.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/purchase-order/purchase-order.js
🚧 Files skipped from review as they are similar to previous changes (28)
- components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/package.json
- components/procore_sandbox/actions/create-rfi/create-rfi.mjs
- components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
- components/procore_sandbox/actions/create-incident/create-incident.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/procore_sandbox.app.mjs
- components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs
- components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore_sandbox/actions/create-submittal/create-submittal.mjs
- components/procore/common/utils.mjs
- components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
- components/procore/package.json
- components/procore/common/resource-names.mjs
- components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore/actions/create-incident/create-incident.mjs
- components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore/actions/create-timesheet/create-timesheet.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (14)
components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs (1)
43-48: Validate timestamp before conversionIf
body.timestampis missing or in an invalid format,new Date(body.timestamp).getTime()will returnNaN, which could cause issues downstream.generateMeta(body) { return { id: body.id, summary: `New Submittal Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(), }; },components/procore/actions/create-submittal/create-submittal.mjs (1)
87-117: Code implementation looks good but lacks error handlingThe component follows a good pattern for Procore API integration, matching the structure of other actions. However, there's no error handling if the API call fails, which could result in unhandled exceptions.
Consider implementing error handling for the API call:
async run({ $ }) { const { createSubmittal, companyId, projectId, number, description, title, type, isPrivate, revision, locationId, } = this; + try { const response = await createSubmittal({ $, companyId, projectId, data: { submittal: { number, description, title, type, private: isPrivate, revision, location_id: locationId, }, }, }); $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create submittal: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (2)
73-80: Add better error handling for API failures.Currently, if an error occurs during
createManpowerLog(), the action might silently fail at runtime. Consider wrapping API calls in robust try-catch blocks or returning informative errors.+ try { return this.app.post({ path: `/projects/${projectId}/manpower_logs`, ...args, }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + }
82-112: Add error handling to run methodThe run method lacks error handling for the API call, which could result in unhandled exceptions.
Implement error handling:
async run({ $ }) { const { createManpowerLog, companyId, projectId, datetime, notes, numWorkers, numHours, userId, locationId, } = this; + try { const response = await createManpowerLog({ $, companyId, projectId, data: { manpower_log: { datetime, notes, num_workers: numWorkers, num_hours: numHours, user_id: userId, location_id: locationId, }, }, }); $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create manpower log: ${error.message}`); + } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
43-49: Validate Timestamp Presence and ValidityIn the function defined in
components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs, the timestamp is derived via:ts: new Date(body.timestamp).getTime(),If
body.timestampis missing or not in a valid format, this expression will yieldNaN, which can cause issues downstream.Implement a safeguard:
generateMeta(body) { return { id: body.id, summary: `New Prime Contract Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(), }; },components/procore/procore.app.mjs (2)
262-269: Use plural/prime_contracts/endpoint for consistency with Procore docs
The Procore API documentation indicates that prime contracts use the plural endpoint/prime_contracts/. Retaining the singular form risks incorrect calls or 404 errors.- path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`,
337-381: Add robust error handling for pagination
A try/catch around the paginated requests helps prevent unhandled exceptions if any given request fails, aligning with best practices for resilient systems.while (true) { + try { const response = await resourcesFn({ ... }); ... + } catch (error) { + console.error(`Error fetching resources on page ${page}: ${error}`); + throw error; + } }components/procore/common/constants.mjs (1)
1-66: Looks good!
All constants are clearly named, well-organized, and appear consistent with your usage patterns. No issues flagged.components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
26-45: Great addition of try/catch error handling
This ensures that API errors won't crash the entire flow, improving overall reliability for the source.components/procore/sources/common/webhook.mjs (5)
1-2: All good on imports.
No concerns here. The imports for UUID andConfigurationErrorlook fine.
6-33: Props look solid.
The defined props are comprehensive and logically grouped. The previous mismatch betweenwhebhookTypeandthis.webhookTypeappears resolved.
155-157: Implement thegenerateMetamethod to avoid runtime errors.
Currently, calling this method (line 218) will throw aConfigurationError. Provide an implementation to emit event metadata properly.
177-178: Define or overridegetResourceName.
This method is required to specify the resource for event triggers. As written, it will always throw an error if invoked.
220-236: Request validation and response logic look correct.
Therunmethod properly checks the bearer token, returns401if unauthorized, and200otherwise, thenemits the parsed payload. This meets basic webhook best practices.
...ents/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
Show resolved
Hide resolved
jcortes
force-pushed
the
procore-new-components
branch
from
ee74d49 to
e118338
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
components/procore/actions/create-submittal/create-submittal.mjs (1)
87-118: 🛠️ Refactor suggestionAdd error handling to prevent unhandled exceptions
The
runmethod lacks error handling for the API call, which could result in unhandled exceptions if the request fails.async run({ $ }) { const { createSubmittal, companyId, projectId, number, description, title, type, isPrivate, revision, locationId, } = this; + try { const response = await createSubmittal({ $, companyId, projectId, data: { submittal: { number, description, title, type, private: isPrivate, revision, location_id: locationId, }, }, }); $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create submittal: ${error.message}`); + } },components/procore/actions/create-timesheet/create-timesheet.mjs (3)
34-41: 🛠️ Refactor suggestionAdd default parameter and improve parameter handling
Unlike other action methods, this one doesn't provide a default empty object for parameters, which could lead to errors if called without arguments.
createTimesheet({ - projectId, ...args + $, projectId, companyId, data, ...args -}) { +} = {}) { return this.app.post({ path: `/projects/${projectId}/timesheets`, + $, + companyId, + data, ...args, }); },
27-31: 🛠️ Refactor suggestionValidate timesheet date to use ISO 8601 format
The current implementation lacks explicit validation for the date property. Procore expects dates to adhere to the ISO 8601 standard (YYYY-MM-DD).
While the description includes an example format, consider adding validation to ensure any provided date complies with the required format.
43-63: 🛠️ Refactor suggestionAdd error handling to prevent unhandled exceptions
The
runmethod lacks error handling for the API call, which could result in unhandled exceptions if the request fails.async run({ $ }) { const { createTimesheet, companyId, projectId, date, } = this; + try { const response = await createTimesheet({ $, companyId, projectId, data: { timesheet: { date, }, }, }); $.export("$summary", `Successfully created timesheet with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create timesheet: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
82-112: 🛠️ Refactor suggestionAdd error handling to run method
The run method lacks error handling for the API call, which could result in unhandled exceptions.
async run({ $ }) { const { createManpowerLog, companyId, projectId, datetime, notes, numWorkers, numHours, userId, locationId, } = this; + try { const response = await createManpowerLog({ $, companyId, projectId, data: { manpower_log: { datetime, notes, num_workers: numWorkers, num_hours: numHours, user_id: userId, location_id: locationId, }, }, }); $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create manpower log: ${error.message}`); + } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
43-48: 🛠️ Refactor suggestionValidate timestamp presence and validity
In the
generateMetafunction, the timestamp is derived vianew Date(body.timestamp).getTime(). Ifbody.timestampis missing or not in a valid format, this expression will yieldNaN, which can cause issues downstream.generateMeta(body) { return { id: body.id, summary: `New Prime Contract Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(), }; },
🧹 Nitpick comments (7)
components/procore_sandbox/common/utils.mjs (4)
3-33: Consider optimizing thebuildPropDefinitionsfunction to avoid O(n²) complexity.The function currently uses spread syntax on accumulators within the reduce operation, which can lead to O(n²) time complexity as the number of properties grows. This issue is flagged by the Biome static analysis tool.
function buildPropDefinitions({ app = {}, props = {}, }) { - return Object.entries(props) - .reduce((newProps, [ - key, - prop, - ]) => { - if (!prop.propDefinition) { - return { - ...newProps, - [key]: prop, - }; - } - - const [ - , ...propDefinitionItems - ] = prop.propDefinition; - - return { - ...newProps, - [key]: { - ...prop, - propDefinition: [ - app, - ...propDefinitionItems, - ], - }, - }; - }, {}); + const newProps = {}; + + Object.entries(props).forEach(([key, prop]) => { + if (!prop.propDefinition) { + newProps[key] = prop; + return; + } + + const [, ...propDefinitionItems] = prop.propDefinition; + + newProps[key] = { + ...prop, + propDefinition: [ + app, + ...propDefinitionItems, + ], + }; + }); + + return newProps; }This refactored approach avoids the accumulator spread operations while maintaining the same functionality.
🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
35-50: Potentially misleading variable renaming in destructuring.The function renames
apptoprocoreduring destructuring, but then imports and uses a differentappinstance. This could cause confusion for developers trying to understand the code flow.function getAppProps(component = {}) { const { - // eslint-disable-next-line no-unused-vars - app: procore, + app: _, // Explicitly showing we're ignoring the component's app prop ...otherProps } = component.props; return { props: { app, // Using the imported app instance ...buildPropDefinitions({ app, props: otherProps, }), }, }; }
1-1: Consider making the imported app configurable.The utility functions are tightly coupled to the specific imported app instance, which could limit reusability across different app variations or testing scenarios.
-import app from "../procore_sandbox.app.mjs"; +import defaultApp from "../procore_sandbox.app.mjs"; function buildPropDefinitions({ app = {}, props = {}, }) { // Function implementation remains the same } -function getAppProps(component = {}) { +function getAppProps(component = {}, customApp = defaultApp) { const { // eslint-disable-next-line no-unused-vars app: procore, ...otherProps } = component.props; return { props: { - app, + app: customApp, ...buildPropDefinitions({ - app, + app: customApp, props: otherProps, }), }, }; }This change would make the utilities more flexible while maintaining backward compatibility.
Also applies to: 35-50
41-48: Add error handling for edge cases.The utility functions don't handle potential edge cases, such as when
componentorcomponent.propsare undefined or when propDefinition is not an array.Consider adding validation to ensure robustness:
function getAppProps(component = {}) { + if (!component || typeof component !== 'object') { + return { props: { app } }; + } + const { // eslint-disable-next-line no-unused-vars app: procore, ...otherProps - } = component.props; + } = component.props || {}; return { props: { app, ...buildPropDefinitions({ app, props: otherProps, }), }, }; }Similarly in
buildPropDefinitions, consider adding validation forprop.propDefinitionto ensure it's an array before destructuring.components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
32-37: Validate datetime format in ISO 8601While the description includes an example format (
2025-04-01T00:00:00Z), the component doesn't validate that the provided datetime adheres to the expected ISO 8601 format.Consider adding validation or a more detailed description to ensure users provide correctly formatted datetime strings. This will help prevent API errors due to format issues.
components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (2)
16-41: Use error-level logging for clarity.
While the error is handled in atry/catch, consider usingconsole.errorinstead ofconsole.logto emphasize the severity of failures:- console.log(error.message || error); + console.error(error.message || error);Also, double-check that returning the unmodified
bodyon error is the desired behavior. Depending on your requirements, it may be helpful to emit partial or alternative data, or even halt execution.
43-49: Validate the presence of timestamps.
Ifbody.timestampis missing or invalid,new Date(body.timestamp).getTime()may yieldNaN. Consider a fallback or additional validation:return { id: body.id, summary: `New Commitment Change Order Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(), };
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/submittal/submittal.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/sources/new-event/new-event.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/common.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/procore.app.js
🚧 Files skipped from review as they are similar to previous changes (28)
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/actions/create-rfi/create-rfi.mjs
- components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
- components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/package.json
- components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs
- components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
- components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore/common/utils.mjs
- components/procore/common/resource-names.mjs
- components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore_sandbox/actions/create-incident/create-incident.mjs
- components/procore_sandbox/procore_sandbox.app.mjs
- components/procore_sandbox/actions/create-submittal/create-submittal.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/actions/create-incident/create-incident.mjs
- components/procore/package.json
- components/procore/common/constants.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore/sources/common/webhook.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (9)
components/procore_sandbox/common/utils.mjs (1)
52-55: LGTM - Clean module exports!The module exports an object with named functions, which is a good practice for utility modules as it allows for selective importing by consumers.
components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
26-41: Excellent error handling implementationThe error handling in the
getDataToEmitmethod is well-implemented. It properly catches any errors from the API call to retrieve prime contract details and provides a fallback by returning the original body. This makes the component more robust and prevents webhook processing from failing completely when encountering API issues.components/procore/procore.app.mjs (2)
266-266: Use the correct plural endpoint for prime contracts.According to Procore’s documentation, the endpoint should be
/prime_contracts/instead of/prime_contract/, which could cause errors or unexpected behavior.- path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`,
337-381: Add error handling to the pagination logic.If
resourcesFnthrows an error while fetching data, thewhile (true)loop will break. Consider wrapping the network call in a try/catch block so that you can log and handle errors gracefully.components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
51-51: Validate the presence ofbody.timestamp.If
body.timestampis missing or not a valid date,new Date(body.timestamp).getTime()returnsNaN, which may cause issues downstream. Consider adding a safeguard.components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs (1)
48-48: Verify presence ofbody.timestampor provide a fallback.If
body.timestampis missing or invalid,new Date(body.timestamp).getTime()will yieldNaN. Ensure this field is always set properly before converting to a timestamp, or handle the fallback scenario:ts: new Date(body.timestamp).getTime(), + // or, safely handle missing timestamp: + // ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(),components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (3)
1-3: Imports look good.
These imports cleanly bring in common webhook functionality and resource names, and there's no indication of unused imports or version conflicts.
4-10: Source metadata is well-defined.
The name, key, description, and version are clear and descriptive, matching typical patterns for Pipedream sources.
13-15: Resource name method is straightforward.
ReturningresourceNames.CHANGE_EVENTScorrectly aligns with the intended resource type for Procore.
jcortes
force-pushed
the
procore-new-components
branch
from
e118338 to
3ee8b3b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
components/procore/actions/create-submittal/create-submittal.mjs (1)
87-117: 🛠️ Refactor suggestionAdd error handling for API calls
The API call in the run method lacks error handling, which could result in unhandled exceptions when the API request fails.
async run({ $ }) { const { createSubmittal, companyId, projectId, number, description, title, type, isPrivate, revision, locationId, } = this; + try { const response = await createSubmittal({ $, companyId, projectId, data: { submittal: { number, description, title, type, private: isPrivate, revision, location_id: locationId, }, }, }); $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create submittal: ${error.message}`); + } },components/procore/actions/create-timesheet/create-timesheet.mjs (2)
27-31: 🛠️ Refactor suggestionAdd date format validation for ISO 8601
The date field is described as requiring a specific format, but there's no validation to ensure it adheres to the ISO 8601 standard as expected by Procore's API.
Consider adding a custom validation function that checks if the date follows the required format:
date: { type: "string", label: "Date", description: "The date of the timesheet. Eg. `2025-04-01`.", + validate: (value) => { + const regex = /^\d{4}-\d{2}-\d{2}$/; + if (!regex.test(value)) { + return "Date must be in YYYY-MM-DD format (ISO 8601)"; + } + return true; + }, },
43-63: 🛠️ Refactor suggestionImplement error handling in run method
The run method makes an API call without any error handling, which could result in unhandled exceptions if the API call fails.
async run({ $ }) { const { createTimesheet, companyId, projectId, date, } = this; + try { const response = await createTimesheet({ $, companyId, projectId, data: { timesheet: { date, }, }, }); $.export("$summary", `Successfully created timesheet with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create timesheet: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
82-112: 🛠️ Refactor suggestionAdd error handling to run method
The run method lacks error handling for the API call, which could result in unhandled exceptions.
async run({ $ }) { const { createManpowerLog, companyId, projectId, datetime, notes, numWorkers, numHours, userId, locationId, } = this; + try { const response = await createManpowerLog({ $, companyId, projectId, data: { manpower_log: { datetime, notes, num_workers: numWorkers, num_hours: numHours, user_id: userId, location_id: locationId, }, }, }); $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create manpower log: ${error.message}`); + } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
43-48: 🛠️ Refactor suggestionValidate timestamp presence and validity
If
body.timestampis missing or invalid,new Date(body.timestamp).getTime()will yieldNaN, which can cause downstream issues.generateMeta(body) { return { id: body.id, summary: `New Prime Contract Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(), }; },
🧹 Nitpick comments (8)
components/procore_sandbox/common/utils.mjs (2)
35-50: Clarify the app property extraction and usage pattern.The function extracts the
appproperty fromcomponent.propsand renames it toprocore, but then doesn't use it (indicated by the eslint-disable comment). Instead, it uses the importedappmodule. This pattern is confusing and could lead to issues if the component's app property differs from the imported app.Consider one of these approaches:
- Use the extracted app property instead of the imported one:
function getAppProps(component = {}) { const { - // eslint-disable-next-line no-unused-vars - app: procore, + app: componentApp, ...otherProps } = component.props; return { props: { - app, + app: componentApp, ...buildPropDefinitions({ - app, + app: componentApp, props: otherProps, }), }, }; }
- Or add a comment explaining why the imported app is used instead of the component's app property.
1-55: Add JSDoc documentation to improve maintainability.The utility functions lack documentation, which would help other developers understand their purpose, parameters, and return values.
Add JSDoc comments to both functions:
import app from "../procore_sandbox.app.mjs"; +/** + * Builds property definitions by ensuring each property with a propDefinition includes the app reference. + * @param {Object} options - The options object + * @param {Object} [options.app={}] - The app reference to include in propDefinitions + * @param {Object} [options.props={}] - The properties to process + * @returns {Object} The processed properties with updated propDefinitions + */ function buildPropDefinitions({ app = {}, props = {}, }) { // ...existing code } +/** + * Extracts and processes component properties to ensure they include the app reference. + * @param {Object} [component={}] - The component object + * @returns {Object} An object containing processed props with the app reference + */ function getAppProps(component = {}) { // ...existing code }Additionally, consider adding error handling for unexpected inputs or invalid property definitions.
🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
components/procore/procore.app.mjs (2)
21-27: Consider refactoring repetitive “label-value” mappings into a helper function.
You repeat the same pattern of mapping API results into{ label, value }objects across multiple prop definitions. Centralizing this logic into a reusable helper could reduce code duplication and simplify maintenance.- return results.map(({ - id: value, name: label, - }) => ({ - label, - value, - })); + // Example helper usage: + const formatOptions = (items) => items.map(({ id: value, name: label }) => ({ label, value })); + ... + return formatOptions(results);Also applies to: 49-55, 88-94, 115-121, 138-144, 164-170
203-211: Add try/catch around themakeRequestmethod for improved error handling.
Currently, any request error will bubble up with minimal context. Wrapping this call in a try/catch (or implementing a global request interceptor) ensures you can log or handle errors consistently across all API calls.async makeRequest({ $ = this, path, headers, versionPath, companyId, ...args } = {}) { + try { return await axios($, { ...args, url: this.getUrl(path, versionPath), headers: this.getHeaders(headers, companyId), }); + } catch (error) { + console.error(`Error in makeRequest: ${error.message}`); + throw error; + } }components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
26-45: Consider rethrowing errors ingetDataToEmitfor visibility.
Logging the error is useful, but rethrowing (or returning an error object) can help consumers of this source handle failures appropriately, ensuring no silent failures in webhook processing.} catch (error) { console.log(error.message || error); + throw error; // Alternatively, return a structured error response }components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
16-42: Rethrow errors ingetDataToEmitto avoid silent failures.
Console logging is valuable, but rethrowing or returning an error ensures consuming systems can properly detect and handle the failure.} catch (error) { console.log(error.message || error); + throw error; }components/procore/sources/common/webhook.mjs (2)
38-93: Lack of error handling in theactivate()hook.
IfcreateWebhooksHookorbulkCreateWebhooksTriggersfail, the code won't attempt to roll back partially created resources. This could leave the system in an inconsistent state. Consider wrapping these calls in atry/catchblock or implementing a rollback strategy.
94-125: No fallback on partial failures indeactivate()hook.
Similar toactivate(), if a call tobulkDeleteWebhooksTriggersordeleteWebhooksHookfails, there’s no rollback or retry logic. Consider adding error handling to ensure resources are cleaned up properly, even if one of these calls fails.
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/common.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/procore.app.js
- components/procore/sources/submittal/submittal.js
- components/procore/sources/new-event/new-event.js
🚧 Files skipped from review as they are similar to previous changes (28)
- components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore_sandbox/actions/create-submittal/create-submittal.mjs
- components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs
- components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
- components/procore_sandbox/actions/create-incident/create-incident.mjs
- components/procore/common/resource-names.mjs
- components/procore_sandbox/procore_sandbox.app.mjs
- components/procore/common/utils.mjs
- components/procore/package.json
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/actions/create-rfi/create-rfi.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore_sandbox/package.json
- components/procore/common/constants.mjs
- components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/actions/create-incident/create-incident.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (13)
components/procore/actions/create-timesheet/create-timesheet.mjs (1)
34-41: Add default parameter and improve parameter handlingUnlike other action methods, this one doesn't provide a default empty object for parameters.
createTimesheet({ - projectId, ...args + $, projectId, companyId, data, ...args -}) { +} = {}) { return this.app.post({ path: `/projects/${projectId}/timesheets`, + $, + companyId, + data, ...args, }); },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
73-80: Add better error handling for API failuresThe
createManpowerLogmethod lacks proper error handling.createManpowerLog({ projectId, ...args } = {}) { + try { return this.app.post({ path: `/projects/${projectId}/manpower_logs`, ...args, }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
26-41: Good implementation of error handlingThe error handling in the
getDataToEmitmethod appropriately catches and logs any errors during the API call, fallback to returning the original body. This ensures the webhook processing continues even if the additional data cannot be retrieved.components/procore/procore.app.mjs (3)
262-268: Use the plural endpoint/prime_contracts/to match Procore’s documented API.
Procore documentation and prior reviews indicate the endpoint should be/prime_contracts/. Using/prime_contract/may prevent correct data retrieval.- path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`,
337-381: Add robust error handling to thegetIterationspagination logic.
A past review comment suggests wrapping the call toresourcesFnin a try/catch block. This prevents the entire generator from failing silently when the API call encounters an error.while (true) { + try { const response = await resourcesFn({ ... }); ... + } catch (error) { + console.error(`Error fetching paginated resources (page ${page}): ${error.message}`); + throw error; + } }
230-236:❓ Verification inconclusive
Verify differing API version usage for projects (V1_1) and users (V1_3).
It’s possible these calls intentionally use different versions, but confirm it’s correct to mix versions for project and user data. If not, consider aligning them.Also applies to: 311-320
🏁 Script executed:
#!/bin/bash # Compare usage of V1_1 vs V1_3 endpoints across the codebase rg --json "versionPath: constants.VERSION_PATH"Length of output: 4032
API Version Usage Check: Verify Consistency Between V1_1 and V1_3 Usage
The code in
components/procore/procore.app.mjsshows that endpoints for project-related calls (e.g., inlistProjectsat lines 230–236) useconstants.VERSION_PATH.V1_1, while another section (around line 316) usesconstants.VERSION_PATH.V1_3. Please confirm that this intentional version mix (projects using V1_1 versus users or related calls using V1_3) aligns with the API requirements. If this discrepancy is unintended, consider updating the versions for consistency.
- Verify that using V1_1 for project endpoints and V1_3 for user endpoints (or other related endpoints) is correct.
- In particular, check the rationale for the difference between lines 230–236 (V1_1) and lines 311–320 (V1_3).
components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
48-53: Validate thebody.timestampfield ingenerateMeta.
Ifbody.timestampis missing or invalid,new Date(body.timestamp).getTime()results inNaN. A past review comment highlighted the same risk. Consider fallback logic or an explicit check.generateMeta(body) { return { id: body.id, summary: `New Budget Snapshot Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp + ? new Date(body.timestamp).getTime() + : Date.now(), }; }components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
44-49: Validate thebody.timestampfield ingenerateMeta.
Ifbody.timestampis missing or invalid,new Date(body.timestamp).getTime()becomesNaN. A previous review comment mentioned the same issue in related files. Provide a fallback or validation.generateMeta(body) { return { id: body.id, summary: `New Commitment Change Order Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp + ? new Date(body.timestamp).getTime() + : Date.now(), }; }components/procore/sources/common/webhook.mjs (5)
1-5: Imports look good.
No immediate concerns regarding correctness or security.
6-36: Consider validating prop inputs and handling empty cases gracefully.
Though these props are well-defined, it may be beneficial to ensure thateventTypesisn't empty or has a fallback behavior for scenarios where the user doesn't provide any event types.Do you want me to run a verification script to look for usage of these props downstream and confirm that empty or invalid inputs won't break the flow?
128-130: UnimplementedgenerateMetamethod (same issue previously flagged).
This method is invoked inprocessResource(), causing a runtime error. You must implement or override this method.
149-151: UnimplementedgetResourceNamemethod (same issue previously flagged).
This method is required for proper trigger creation. You must implement or override it in derived classes.
193-210: Verify token-based authorization logic inrun().
Currently, any mismatch in theAuthorizationheader returns a401and exits. Ensure you have tested scenarios where:
- No
Authorizationheader is present- The token is expired or invalid
- Authorization is needed for partial event processing
If more robust security or custom error handling is required (e.g., logging or rate-limiting), consider implementing it here.
Do you want me to generate a test script that sends various
Authorizationheaders to confirm the correct status codes and responses?
jcortes
force-pushed
the
procore-new-components
branch
from
3ee8b3b to
16d21fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
components/procore_sandbox/common/utils.mjs (1)
3-33: 🛠️ Refactor suggestionOptimize reducer implementation to avoid O(n²) complexity.
The current implementation uses spread operators (
...newProps) inside the reducer accumulator, which can lead to O(n²) time complexity when processing large property objects. This issue has been flagged by the static analysis tool.Consider using a for...of loop instead:
function buildPropDefinitions({ app = {}, props = {}, }) { - return Object.entries(props) - .reduce((newProps, [ - key, - prop, - ]) => { - if (!prop.propDefinition) { - return { - ...newProps, - [key]: prop, - }; - } - - const [ - , ...propDefinitionItems - ] = prop.propDefinition; - - return { - ...newProps, - [key]: { - ...prop, - propDefinition: [ - app, - ...propDefinitionItems, - ], - }, - }; - }, {}); + const newProps = {}; + + for (const [key, prop] of Object.entries(props)) { + if (!prop.propDefinition) { + newProps[key] = prop; + continue; + } + + const [, ...propDefinitionItems] = prop.propDefinition; + + newProps[key] = { + ...prop, + propDefinition: [ + app, + ...propDefinitionItems, + ], + }; + } + + return newProps; }🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
components/procore/sources/common/webhook.mjs (2)
128-130:⚠️ Potential issueImplement
generateMetaor remove its usage to prevent runtime errors.Because it’s invoked at line 190, this unimplemented method will throw a
ConfigurationError. Provide a meaningful implementation or omit the call.
149-151:⚠️ Potential issueImplement
getResourceNameor remove its usage to prevent activation failures.This method is called at line 76 when creating triggers. As it’s not implemented, the code will throw a
ConfigurationError, breaking the activation flow.
🧹 Nitpick comments (8)
components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (1)
39-41: Consider more robust error handling instead of only logging.Relying solely on
console.log()can lead to silent failures if callers expect an exception or structured error handling. A try/catch with an explicit error throw or$export could help track failures in your pipeline.Here's a possible example:
try { const resource = await app.getTimecardEntry({ companyId, timecardEntryId, params: { project_id: projectId, }, }); return { ...body, resource, }; } catch (error) { - console.log(error.message || error); + $.export("error", error); + throw new Error(`Failed to retrieve timecard entry: ${error.message}`); }components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
16-46: Useconsole.errorfor error messages and consider verifyingprojectId.Currently, errors are logged with
console.log. To better convey issue severity and maintain standard logging practices, consider usingconsole.error. Additionally, consider handling cases whereprojectIdmay be missing or invalid before invokingapp.paginate, to prevent unexpected errors.try { const snapshotRows = await app.paginate({ ... }); return { ...body, snapshotRows, }; } catch (error) { - console.log(error.message || error); + console.error(error.message || error); return body; }components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
16-42: Useconsole.errorfor error logging and verifyprojectIdconsistency.To ensure proper visibility of errors, replace
console.logwithconsole.error. Also, ifprojectIdis critical for retrieving purchase order details, consider adding checks to avoid potential undefined behavior.try { const resource = await app.getPurchaseOrderContract({ ... }); return { ...body, resource, }; } catch (error) { - console.log(error.message || error); + console.error(error.message || error); return body; }components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (2)
16-42: Useconsole.errorfor error messages and validate the presence ofprojectId.Replace
console.log(error)withconsole.error(error)for clarity, and consider early-return checks forprojectIdif it’s mandatory forapp.getChangeEvent.try { const resource = await app.getChangeEvent({ ... }); return { ...body, resource, }; } catch (error) { - console.log(error.message || error); + console.error(error.message || error); return body; }
43-49: Ensurebody.timestampis validated or defaulted.As with the other event sources, if
body.timestampis missing or invalid, this could lead to aNaNtimestamp. Add validation or fallback logic to ensure robust event metadata.components/procore/sources/common/webhook.mjs (3)
38-93: Consider adding failure handling in the activation process.If creating the webhook or triggers fails, there’s no catch for partial or complete failures. This might leave the system in an inconsistent state. Consider wrapping these operations in try/catch blocks or adding a rollback mechanism for partial failures.
94-125: Gracefully handle partial failures in the deactivation process.Similar to the activation flow, if trigger deletion succeeds but webhook deletion fails (or vice versa), you may leave behind orphaned hooks or triggers. Consider adding error handling or a transaction-like approach to preserve consistency.
197-201: Enhance error feedback for unauthorized access.Returning a bare 401 status without an explanatory body may hinder debugging. Consider adding a brief responsive message or logging for clarity.
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/new-event/new-event.js
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/common.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/procore.app.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/submittal/submittal.js
🚧 Files skipped from review as they are similar to previous changes (28)
- components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/package.json
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore_sandbox/actions/create-submittal/create-submittal.mjs
- components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore_sandbox/actions/create-rfi/create-rfi.mjs
- components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
- components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs
- components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore_sandbox/procore_sandbox.app.mjs
- components/procore_sandbox/actions/create-incident/create-incident.mjs
- components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/common/resource-names.mjs
- components/procore/package.json
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/common/utils.mjs
- components/procore/actions/create-timesheet/create-timesheet.mjs
- components/procore/common/constants.mjs
- components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore/actions/create-incident/create-incident.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (11)
components/procore_sandbox/common/utils.mjs (2)
35-50: LGTM! Good function design for component prop handling.The function properly extracts the app property, renaming it to avoid confusion, and reuses the buildPropDefinitions function to process other properties. The ESLint disable comment is appropriately used for the unused variable.
52-55: LGTM! Clean export of utility functions.The export syntax correctly makes both utility functions available to other modules.
components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (1)
43-49:❓ Verification inconclusive
Verify that the correct ID is used for event metadata.
The code uses
body.idingenerateMeta(), whereas the resource ID appears to be inbody.resource_id. Confirm thatbody.idis indeed populated (and unique) for the timecard entry. Otherwise, consider usingbody.resource_idas theid.
🏁 Script executed:
#!/bin/bash # Description: Search for references to `resource_id` vs `body.id` in this component and check for usage patterns. rg -A 3 'generateMeta' rg -A 3 'body\.id' rg -A 3 'body\.resource_id'Length of output: 65703
Action: Verify the correctness of the event metadata ID
The current implementation uses
body.idto populate the metadata, while the event’s summary referencesbody.resource_id. In other similar components, we see that the unique identifier is typically taken directly from the event payload (e.g.,topic.idorAlertId), which suggests that using a distinct identifier is intentional. However, please confirm that for Procore’s timecard event,body.idis indeed available and uniquely identifies the event entry. If not, it might be more appropriate to usebody.resource_idfor the metadata’sid.• Check the incoming payload structure for new timecard entries to ensure that
body.idis reliably populated and unique.
• If you find thatbody.idis undefined or not unique across events, update the implementation to usebody.resource_idas the identifier.components/procore/actions/create-submittal/create-submittal.mjs (1)
87-118: Add error handling for API failures.The API call in
run()lacks a try-catch block, which can cause unhandled exceptions at runtime. Re-throwing an error or exporting it via$ensures that downstream processes handle it gracefully.async run({ $ }) { const { createSubmittal, companyId, projectId, number, description, title, type, isPrivate, revision, locationId, } = this; - const response = await createSubmittal({ - $, - companyId, - projectId, - data: { - submittal: { - number, - description, - title, - type, - private: isPrivate, - revision, - location_id: locationId, - }, - }, - }); - $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`); - return response; + try { + const response = await createSubmittal({ + $, + companyId, + projectId, + data: { + submittal: { + number, + description, + title, + type, + private: isPrivate, + revision, + location_id: locationId, + }, + }, + }); + $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`); + return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create submittal: ${error.message}`); + } }components/procore/actions/create-manpower-log/create-manpower-log.mjs (3)
32-37: Add datetime format validation.Revisit the previous suggestion to validate
datetimeso that the provided string matches the ISO 8601 format. This prevents issues when optional fields are supplied in an invalid format.datetime: { type: "string", label: "Datetime", description: "The datetime of the record. Eg. `2025-04-01T00:00:00Z`.", optional: true, + validate: (value) => { + if (!value) return true; // Skip validation if not provided + const regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/; + if (!regex.test(value)) { + return "Datetime must be in ISO 8601 format (YYYY-MM-DDThh:mm:ssZ)"; + } + return true; + }, },
73-80: Add robust error handling for the API call.Wrapping the
createManpowerLog()request in a try-catch block ensures any failures are caught and surfaced, preventing silent runtime errors.createManpowerLog({ projectId, ...args } = {}) { - return this.app.post({ - path: `/projects/${projectId}/manpower_logs`, - ...args, - }); + try { + return this.app.post({ + path: `/projects/${projectId}/manpower_logs`, + ...args, + }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + } },
82-112: Wrap the run method in a try-catch block.Similar to the API call, the top-level
async run()should handle errors gracefully to prevent unhandled exceptions.async run({ $ }) { const { createManpowerLog, companyId, projectId, datetime, notes, numWorkers, numHours, userId, locationId, } = this; - const response = await createManpowerLog({ - $, - companyId, - projectId, - data: { - manpower_log: { - datetime, - notes, - num_workers: numWorkers, - num_hours: numHours, - user_id: userId, - location_id: locationId, - }, - }, - }); - $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`); - return response; + try { + const response = await createManpowerLog({ + $, + companyId, + projectId, + data: { + manpower_log: { + datetime, + notes, + num_workers: numWorkers, + num_hours: numHours, + user_id: userId, + location_id: locationId, + }, + }, + }); + $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`); + return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create manpower log: ${error.message}`); + } }components/procore/procore.app.mjs (2)
262-269: Use plural resource path for prime contracts.According to Procore API documentation, prime contracts require a plural path (
/prime_contracts/). Verify if/prime_contract/is correct or if it should be/prime_contracts/.getPrimeContract({ primeContractId, ...args } = {}) { return this.makeRequest({ - path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`, ...args, }); },
337-381: Add error handling for paginated requests.A failure at any point in
getIterations()halts iteration, but no explicit handling is in place. Consider catching errors and either re-throwing them or handling them to avoid partial data or silent failures.while (true) { + try { const response = await resourcesFn({ ...resourcesFnArgs, params: { ...resourcesFnArgs?.params, page, per_page: constants.DEFAULT_LIMIT, }, }); const nextResources = resourceName ? utils.getNestedProperty(response, resourceName) : response; if (!nextResources?.length) { console.log("No more resources found"); return; } for (const resource of nextResources) { yield resource; resourcesCount += 1; if (resourcesCount >= max) { console.log("Reached max resources"); return; } } if (nextResources.length < constants.DEFAULT_LIMIT) { console.log("No next page found"); return; } page += 1; + } catch (error) { + console.error(`Error fetching resources on page ${page}: ${error.message}`); + throw error; + } }components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
47-53: Validatebody.timestampto avoid potentialNaNresult.This is a known concern from previous discussions. If
body.timestampis missing or invalid,new Date(body.timestamp).getTime()will produceNaN, potentially causing downstream issues. Consider adding a fallback or validation.components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
43-49: Add checks forbody.timestampbefore computingts.If
body.timestampis missing or malformed,new Date(body.timestamp).getTime()can yieldNaN. Implement a fallback or guard clause to ensure the emitted event has a valid timestamp.Would you like to confirm the presence of
timestampin all possible event payloads, or should we add a default if it's missing?
jcortes
force-pushed
the
procore-new-components
branch
from
16d21fe to
69028f0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/procore/actions/create-submittal/create-submittal.mjs (1)
87-118:⚠️ Potential issueAdd error handling to run method
The run method lacks error handling for the API call, which could result in unhandled exceptions.
async run({ $ }) { const { createSubmittal, companyId, projectId, number, description, title, type, isPrivate, revision, locationId, } = this; + try { const response = await createSubmittal({ $, companyId, projectId, data: { submittal: { number, description, title, type, private: isPrivate, revision, location_id: locationId, }, }, }); $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create submittal: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
82-112:⚠️ Potential issueAdd error handling to run method
The run method lacks error handling for the API call, which could result in unhandled exceptions.
async run({ $ }) { const { createManpowerLog, companyId, projectId, datetime, notes, numWorkers, numHours, userId, locationId, } = this; + try { const response = await createManpowerLog({ $, companyId, projectId, data: { manpower_log: { datetime, notes, num_workers: numWorkers, num_hours: numHours, user_id: userId, location_id: locationId, }, }, }); $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create manpower log: ${error.message}`); + } },
🧹 Nitpick comments (10)
components/procore_sandbox/common/utils.mjs (1)
35-50: Clarify app variable naming and usage.The current implementation has potentially confusing app references:
- You import
appat the top of the file- Extract
appfrom component.props but rename it toprocore(and mark it unused)- Then use the imported
appin the returned objectThis naming and variable usage could lead to confusion. Consider one of these clearer approaches:
- function getAppProps(component = {}) { - const { - // eslint-disable-next-line no-unused-vars - app: procore, - ...otherProps - } = component.props; - return { - props: { - app, - ...buildPropDefinitions({ - app, - props: otherProps, - }), - }, - }; - } // Option 1: Use consistent naming if intentionally ignoring component's app + function getAppProps(component = {}) { + const { + // eslint-disable-next-line no-unused-vars + app: componentApp, // Clearly named to show it's different from imported app + ...otherProps + } = component.props; + return { + props: { + app, // Using the imported app + ...buildPropDefinitions({ + app, + props: otherProps, + }), + }, + }; + } // Option 2: Actually use the component's app if that was the intention + function getAppProps(component = {}) { + const { + app: componentApp, + ...otherProps + } = component.props; + return { + props: { + app: componentApp || app, // Use component's app or fall back to imported app + ...buildPropDefinitions({ + app: componentApp || app, + props: otherProps, + }), + }, + }; + }components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (2)
46-48: Add timestamp validation to prevent NaN valuesThe timestamp conversion doesn't check if
body.timestampexists or is valid before converting it to milliseconds. Ifbody.timestampis missing or invalid, this will result inNaN, which can cause downstream issues.generateMeta(body) { return { id: body.id, summary: `New Timecard Entry Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp ? new Date(body.timestamp).getTime() : Date.now(), }; },
26-41: Enhance error logging in getDataToEmitThe current error handling only logs to console, which might not be sufficient for troubleshooting in a production environment. Consider adding more context to the error message.
try { const resource = await app.getTimecardEntry({ companyId, timecardEntryId, params: { project_id: projectId, }, }); return { ...body, resource, }; } catch (error) { - console.log(error.message || error); + console.log(`Failed to get timecard entry details (ID: ${timecardEntryId}): ${error.message || error}`); + console.log(`projectId: ${projectId}, companyId: ${companyId}`); return body; }components/procore/actions/create-submittal/create-submittal.mjs (1)
77-85: Add error handling to createSubmittal methodThe createSubmittal method directly returns the result of app.post without any error handling, which could lead to unhandled exceptions if network issues occur.
createSubmittal({ projectId, ...args } = {}) { + try { return this.app.post({ versionPath: constants.VERSION_PATH.V1_1, path: `/projects/${projectId}/submittals`, ...args, }); + } catch (error) { + throw new Error(`Error in createSubmittal: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
45-49: Consider changing numHours type to numberThe numHours property is defined as a string type, which seems inconsistent for a field representing hours. Consider changing it to a number type for better data consistency.
numHours: { - type: "string", + type: "number", label: "Number Of Hours", description: "The number of hours for each worker", optional: true, },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
26-41: Enhance error handling in getDataToEmitThe error handling is minimal, only logging the error message to the console without additional context. Consider adding more information for easier troubleshooting.
try { const resource = await app.getPrimeContract({ companyId, primeContractId, params: { project_id: projectId, }, }); return { ...body, resource, }; } catch (error) { - console.log(error.message || error); + console.log(`Failed to get prime contract details (ID: ${primeContractId}): ${error.message || error}`); + console.log(`projectId: ${projectId}, companyId: ${companyId}`); return body; }components/procore/procore.app.mjs (1)
203-211: Consider adding error handling for makeRequest
Currently,makeRequestdoes not handle request errors. A simple try/catch here can ensure that any unexpected failures are logged or handled appropriately without requiring each caller to re-implement error handling.+async makeRequest({ + $ = this, path, headers, versionPath, companyId, ...args +} = {}) { + try { + return await axios($, { + ...args, + url: this.getUrl(path, versionPath), + headers: this.getHeaders(headers, companyId), + }); + } catch (error) { + console.error(`Request failed: ${error.message}`); + throw error; + } +}components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
44-49: Verify presence of body.timestamp
Similar to other Procore event sources, ensurebody.timestampcan never be undefined or invalid to preventNaNfrom creeping in. A quick validation or fallback measure can help.generateMeta(body) { return { id: body.id, summary: `New Purchase Order Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: Number.isNaN(new Date(body.timestamp).getTime()) + ? Date.now() + : new Date(body.timestamp).getTime(), }; },components/procore/sources/common/webhook.mjs (2)
182-184: Add validation to thegetDataToEmitmethod.This method currently returns the raw webhook body without any validation or sanitization, which could lead to processing invalid data or security issues.
- async getDataToEmit(body) { - return body; - } + async getDataToEmit(body) { + // Validate the body has the expected structure + if (!body || typeof body !== "object") { + console.log("Invalid webhook payload received:", body); + return null; + } + + // You can add additional validation logic here + return body; + }
194-207: Improve the error handling in therunmethod.The current implementation doesn't properly handle potential errors that might occur during processing. Additionally, consider using a constant-time comparison for the token validation to protect against timing attacks.
async run({ body, headers: { authorization }, }) { - const token = this.getToken(); - if (authorization !== `Bearer ${token}`) { - console.log("Authorization header does not match the expected token"); - return this.http.respond({ - status: 401, - }); - } - this.http.respond({ - status: 200, - }); - - const resource = await this.getDataToEmit(body); - this.processResource(resource); + try { + const token = this.getToken(); + const expectedAuth = `Bearer ${token}`; + + // Basic validation of authorization header + if (!authorization || typeof authorization !== 'string') { + console.log("Missing or invalid authorization header"); + return this.http.respond({ + status: 401, + }); + } + + // Compare authorization token + if (authorization !== expectedAuth) { + console.log("Authorization header does not match the expected token"); + return this.http.respond({ + status: 401, + }); + } + + // Respond immediately to the webhook request + this.http.respond({ + status: 200, + }); + + // Process the webhook data asynchronously + const resource = await this.getDataToEmit(body); + if (resource) { + this.processResource(resource); + } + } catch (error) { + console.error("Error processing webhook:", error); + + // Ensure we respond to the webhook even if processing fails + if (!this.http.responded) { + this.http.respond({ + status: 500, + }); + } + } },
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/new-event/new-event.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/common.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/submittal/submittal.js
- components/procore/procore.app.js
- components/procore/sources/purchase-order/purchase-order.js
🚧 Files skipped from review as they are similar to previous changes (28)
- components/procore_sandbox/actions/create-submittal/create-submittal.mjs
- components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore_sandbox/actions/create-rfi/create-rfi.mjs
- components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
- components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
- components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs
- components/procore_sandbox/package.json
- components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore_sandbox/procore_sandbox.app.mjs
- components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore/common/resource-names.mjs
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/actions/create-incident/create-incident.mjs
- components/procore_sandbox/actions/create-incident/create-incident.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/common/utils.mjs
- components/procore/actions/create-timesheet/create-timesheet.mjs
- components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/actions/create-rfi/create-rfi.mjs
- components/procore/common/constants.mjs
- components/procore/package.json
- components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (13)
components/procore_sandbox/common/utils.mjs (1)
6-32: Optimize reducer implementation to avoid O(n²) complexity.The current implementation uses spread operators (
...newProps) inside the reducer accumulator, which can lead to O(n²) time complexity when processing large property objects. This was flagged by static analysis.Consider using an alternative approach that doesn't rely on spreads in the accumulator:
function buildPropDefinitions({ app = {}, props = {}, }) { - return Object.entries(props) - .reduce((newProps, [ - key, - prop, - ]) => { - if (!prop.propDefinition) { - return { - ...newProps, - [key]: prop, - }; - } - - const [ - , ...propDefinitionItems - ] = prop.propDefinition; - - return { - ...newProps, - [key]: { - ...prop, - propDefinition: [ - app, - ...propDefinitionItems, - ], - }, - }; - }, {}); + const newProps = {}; + + for (const [key, prop] of Object.entries(props)) { + if (!prop.propDefinition) { + newProps[key] = prop; + continue; + } + + const [, ...propDefinitionItems] = prop.propDefinition; + + newProps[key] = { + ...prop, + propDefinition: [ + app, + ...propDefinitionItems, + ], + }; + } + + return newProps; }🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
components/procore/actions/create-manpower-log/create-manpower-log.mjs (2)
32-37: Add datetime format validationThe datetime field includes an example format but lacks validation to ensure it follows the ISO 8601 standard required by Procore's API.
datetime: { type: "string", label: "Datetime", description: "The datetime of the record. Eg. `2025-04-01T00:00:00Z`.", optional: true, + validate: (value) => { + if (!value) return true; // Skip validation if not provided since it's optional + const regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/; + if (!regex.test(value)) { + return "Datetime must be in ISO 8601 format (YYYY-MM-DDThh:mm:ssZ)"; + } + return true; + }, },
73-80: Add error handling to createManpowerLog methodCurrently, if an error occurs during
createManpowerLog(), the action might silently fail at runtime.createManpowerLog({ projectId, ...args } = {}) { + try { return this.app.post({ path: `/projects/${projectId}/manpower_logs`, ...args, }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
46-48: Validate timestamp presence and validityIf
body.timestampis missing or not in a valid format, the expression will yieldNaN, which can cause issues downstream.generateMeta(body) { return { id: body.id, summary: `New Prime Contract Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: body.timestamp && !isNaN(new Date(body.timestamp)) + ? new Date(body.timestamp).getTime() + : Date.now(), }; },components/procore/procore.app.mjs (4)
262-269: Use plural endpoint for prime contracts
This endpoint should be updated to use the plural form/prime_contracts/{primeContractId}—matching Procore’s official documentation—rather than the current singular form.
337-381: Add robust error handling in pagination
Similar to a past suggestion, wrapping the API call within try/catch will prevent unexpected crashes when an error occurs during resource retrieval.
178-179: Validate OAuth token retrieval
Ensure thatthis.$auth.oauth_access_tokenis always a valid token, since failing to retrieve a fresh token will cause downstream calls to fail. You might consider refreshing or validating tokens before each request if Procore’s OAuth flow requires it.
311-320:❓ Verification inconclusive
Confirm correctness of version path for listing users
Before merging, confirm that thelistUsersmethod truly requiresconstants.VERSION_PATH.V1_3. If the Procore API endpoint for users is on a different version, you may encounter compatibility issues.
🏁 Script executed:
#!/bin/bash # Verifying the "listUsers" API endpoint version in Procore docs: # (User must confirm the correct version path or check any mismatch) echo "Please confirm if the listUsers endpoint is indeed v1.3."Length of output: 122
Action Required: Verify Procore API Endpoint Version
The current implementation of
listUsersusesconstants.VERSION_PATH.V1_3. The executed script only echoed a prompt without providing confirmation. Please double-check the Procore API documentation to ensure that version v1.3 is indeed the correct endpoint version for listing users. If a different version is required, update the version path accordingly.
- Location:
components/procore/procore.app.mjs(lines 311-320)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (2)
1-3: Imports and common inheritance look good
No issues with the import statements or the spread operator combining the common webhook properties.
48-53: Validate timestamp field
As previously flagged, ifbody.timestampis missing or malformed,new Date(body.timestamp).getTime()will yieldNaN. Consider adding a fallback or validation to avoid unexpected results.components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs (1)
26-41: Solid error-handling pattern
Wrapping the Procore API request in a try/catch block here will help ensure the source remains resilient in case of network or API errors. Good job!components/procore/sources/common/webhook.mjs (2)
124-126: Implement thegenerateMetamethod to avoid runtime errors.This method is invoked in the
processResourcemethod (line 190), yet it throws aConfigurationErrorby default. Implement it to provide the necessary metadata for emitted events.Example implementation:
- generateMeta() { - throw new ConfigurationError("generateMeta is not implemented"); - } + generateMeta(resource) { + return { + id: resource?.id ?? Date.now(), + summary: resource?.title || resource?.name || "Event from Procore", + ts: Date.now(), + }; + }
145-147: DefinegetResourceNameor override it in derived classes.This method is required for creating triggers (line 75). It currently throws a
ConfigurationError, which will break the source if invoked.- getResourceName() { - throw new ConfigurationError("getResourceName is not implemented"); - } + getResourceName() { + // Return resource name here, e.g. "budget_snapshot", "rfi", etc. + return this.resourceName || "default_resource"; + }
|
/approve |
jcortes
force-pushed
the
procore-new-components
branch
from
69028f0 to
1a74dc3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
components/procore/actions/create-submittal/create-submittal.mjs (1)
87-118: 🛠️ Refactor suggestionAdd error handling to run method
The component lacks error handling for API calls, which could lead to unhandled exceptions if the Procore API returns an error.
Implement try/catch block for better error handling:
async run({ $ }) { const { createSubmittal, companyId, projectId, number, description, title, type, isPrivate, revision, locationId, } = this; + try { const response = await createSubmittal({ $, companyId, projectId, data: { submittal: { number, description, title, type, private: isPrivate, revision, location_id: locationId, }, }, }); $.export("$summary", `Successfully created submittal with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create submittal: ${error.message}`); + } },components/procore/actions/create-manpower-log/create-manpower-log.mjs (2)
73-80: 🛠️ Refactor suggestionAdd error handling to createManpowerLog method
The method lacks error handling for API calls, which could lead to unhandled errors at runtime.
Implement try/catch block for better error handling:
createManpowerLog({ projectId, ...args } = {}) { + try { return this.app.post({ path: `/projects/${projectId}/manpower_logs`, ...args, }); + } catch (err) { + throw new Error(`Error creating manpower log: ${err.message}`); + } },
82-112: 🛠️ Refactor suggestionAdd error handling to run method
The component lacks error handling for API calls, which could lead to unhandled exceptions if the Procore API returns an error.
Implement try/catch block for better error handling:
async run({ $ }) { const { createManpowerLog, companyId, projectId, datetime, notes, numWorkers, numHours, userId, locationId, } = this; + try { const response = await createManpowerLog({ $, companyId, projectId, data: { manpower_log: { datetime, notes, num_workers: numWorkers, num_hours: numHours, user_id: userId, location_id: locationId, }, }, }); $.export("$summary", `Successfully created manpower log with ID \`${response.id}\`.`); return response; + } catch (error) { + $.export("error", error); + throw new Error(`Failed to create manpower log: ${error.message}`); + } },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
43-49:⚠️ Potential issueValidate timestamp to avoid potential
NaN.
Ifbody.timestampis missing or invalid,new Date(body.timestamp).getTime()yieldsNaN. Use a fallback or validation logic to ensure a valid timestamp.Apply this diff to handle invalid timestamps gracefully:
generateMeta(body) { return { id: body.id, summary: `New Prime Contract Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: Number.isNaN(new Date(body.timestamp).getTime()) + ? Date.now() + : new Date(body.timestamp).getTime(), }; },components/procore/procore.app.mjs (1)
262-269:⚠️ Potential issueEndpoint path mismatch with Procore docs.
According to Procore’s API docs, prime contracts should use the plural endpoint:/prime_contracts/{primeContractId}.Proposed fix:
getPrimeContract({ primeContractId, ...args } = {}) { return this.makeRequest({ - path: `/prime_contract/${primeContractId}`, + path: `/prime_contracts/${primeContractId}`, ...args, }); },components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (1)
47-53:⚠️ Potential issueValidate
timestampto avoidNaNints.
Ifbody.timestampis missing or invalid,new Date(body.timestamp).getTime()results inNaN. Provide a fallback or validation.Recommended fix:
generateMeta(body) { return { id: body.id, summary: `New Budget Snapshot Event: ${body.resource_id}`, - ts: new Date(body.timestamp).getTime(), + ts: Number.isNaN(new Date(body.timestamp).getTime()) + ? Date.now() + : new Date(body.timestamp).getTime(), }; },
🧹 Nitpick comments (9)
components/procore_sandbox/common/utils.mjs (1)
35-50: Review the app property handling for clarity.The function destructures the
appproperty fromcomponent.propsand renames it toprocorebut never uses it. Instead, you're using the importedappmodule. This could lead to confusion about the source of the app object being used.Consider using a more explicit naming approach:
function getAppProps(component = {}) { const { - // eslint-disable-next-line no-unused-vars - app: procore, + // We ignore the component's app property and use the imported app + app: componentApp, ...otherProps } = component.props; return { props: { app, ...buildPropDefinitions({ app, props: otherProps, }), }, }; }components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs (2)
38-41: Improve error handling for API failuresThe current implementation logs errors but silently continues by returning the original body. This could lead to confusing behavior downstream when the expected resource data is missing.
Consider adding more robust error handling:
} catch (error) { console.log(error.message || error); + console.log("If you need to get more details about the timecard entry, please provide a project ID."); return body; }
16-42: API call lacks fallback strategy for missing projectIdThe code attempts to get a timecard entry but doesn't have a clear fallback if
projectIdis undefined, which would cause Procore's API to reject the request.Add a conditional check before making the API call:
try { + if (!projectId) { + console.log("Project ID is required to fetch detailed timecard entry information"); + return body; + } const resource = await app.getTimecardEntry({ companyId, timecardEntryId, params: { project_id: projectId, }, });components/procore/actions/create-submittal/create-submittal.mjs (1)
28-32: Consider marking number field as requiredThe
numberfield doesn't have anoptionalproperty specified, which implicitly makes it required. For clarity and consistency with other fields, consider explicitly marking it as required.number: { type: "string", label: "Number", description: "The Number of the Submittal", + optional: false, },components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
44-49: Change numHours type from string to numberThe
numHoursfield is defined as a string type, but its description suggests it represents a numeric value. This could lead to data type issues or validation problems when interacting with the Procore API.Change the type to better match the field's purpose:
numHours: { - type: "string", + type: "number", label: "Number Of Hours", description: "The number of hours for each worker", optional: true, },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (1)
16-42: Consider additional fallback forprimeContractId.
Ifresource_idis missing or invalid, the call toapp.getPrimeContractmight fail. A quick guard (e.g., return early ifprimeContractIdis undefined) could improve resilience.components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (2)
16-42: Consider strengthening error handling and logging behavior.
Currently, the catch block only logs the error withconsole.logand returnsbody. Usingconsole.errormight provide clearer visibility for production logs. Additionally, ensure you verify thatprojectIdandresource_idare present in the request, as missing values may lead to unpredictable behavior.
43-49: Ensure safe handling of optional/unknown timestamp and ID fields.
Ifbody.timestamporbody.idis missing or undefined,new Date(body.timestamp).getTime()orbody.idcould produce unexpected results. Consider adding fallback logic or validations to avoid potential runtime errors.components/procore/sources/common/webhook.mjs (1)
92-121: Consider adding error handling to thedeactivatehook.
If removing triggers or deleting the webhook fails midway, the component might be left in an inconsistent state. A try/catch block would help ensure partial resources are properly cleaned up.
📜 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 (47)
components/procore/actions/create-incident/create-incident.mjs(1 hunks)components/procore/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore/common/constants.mjs(1 hunks)components/procore/common/resource-names.mjs(1 hunks)components/procore/common/utils.mjs(1 hunks)components/procore/package.json(1 hunks)components/procore/procore.app.js(0 hunks)components/procore/procore.app.mjs(1 hunks)components/procore/sources/budget-snapshot/budget-snapshot.js(0 hunks)components/procore/sources/commitment-change-order/commitment-change-order.js(0 hunks)components/procore/sources/common.js(0 hunks)components/procore/sources/common/webhook.mjs(1 hunks)components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore/sources/new-event/new-event.js(0 hunks)components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)components/procore/sources/prime-contract-change-order/prime-contract-change-order.js(0 hunks)components/procore/sources/prime-contract/prime-contract.js(0 hunks)components/procore/sources/purchase-order/purchase-order.js(0 hunks)components/procore/sources/rfi/rfi.js(0 hunks)components/procore/sources/submittal/submittal.js(0 hunks)components/procore_sandbox/actions/create-incident/create-incident.mjs(1 hunks)components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs(1 hunks)components/procore_sandbox/actions/create-rfi/create-rfi.mjs(1 hunks)components/procore_sandbox/actions/create-submittal/create-submittal.mjs(1 hunks)components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs(1 hunks)components/procore_sandbox/common/utils.mjs(1 hunks)components/procore_sandbox/package.json(1 hunks)components/procore_sandbox/procore_sandbox.app.mjs(1 hunks)components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs(1 hunks)components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs(1 hunks)
💤 Files with no reviewable changes (10)
- components/procore/sources/prime-contract/prime-contract.js
- components/procore/sources/rfi/rfi.js
- components/procore/sources/common.js
- components/procore/sources/commitment-change-order/commitment-change-order.js
- components/procore/sources/submittal/submittal.js
- components/procore/sources/new-event/new-event.js
- components/procore/sources/purchase-order/purchase-order.js
- components/procore/procore.app.js
- components/procore/sources/budget-snapshot/budget-snapshot.js
- components/procore/sources/prime-contract-change-order/prime-contract-change-order.js
🚧 Files skipped from review as they are similar to previous changes (27)
- components/procore_sandbox/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs
- components/procore_sandbox/sources/new-event-instant/new-event-instant.mjs
- components/procore_sandbox/actions/create-submittal/create-submittal.mjs
- components/procore_sandbox/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs
- components/procore_sandbox/actions/create-manpower-log/create-manpower-log.mjs
- components/procore_sandbox/sources/new-timecard-entry-event-instant/new-timecard-entry-event-instant.mjs
- components/procore_sandbox/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs
- components/procore_sandbox/actions/create-incident/create-incident.mjs
- components/procore_sandbox/package.json
- components/procore_sandbox/procore_sandbox.app.mjs
- components/procore/common/utils.mjs
- components/procore/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/common/resource-names.mjs
- components/procore_sandbox/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore_sandbox/actions/create-timesheet/create-timesheet.mjs
- components/procore/sources/new-event-instant/new-event-instant.mjs
- components/procore/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
- components/procore/actions/create-incident/create-incident.mjs
- components/procore/sources/new-change-order-package-event-instant/new-change-order-package-event-instant.mjs
- components/procore/actions/create-timesheet/create-timesheet.mjs
- components/procore/common/constants.mjs
- components/procore/sources/new-purchase-order-event-instant/new-purchase-order-event-instant.mjs
- components/procore_sandbox/sources/new-submittal-event-instant/new-submittal-event-instant.mjs
- components/procore/package.json
- components/procore_sandbox/actions/create-rfi/create-rfi.mjs
- components/procore_sandbox/sources/new-rfi-event-instant/new-rfi-event-instant.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/procore_sandbox/common/utils.mjs
[error] 13-13: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (16)
components/procore_sandbox/common/utils.mjs (2)
3-33: Optimize reducer implementation to avoid O(n²) complexity.The current implementation uses spread operators (
...newProps) inside the reducer accumulator, which can lead to O(n²) time complexity when processing large property objects. This issue was flagged by static analysis.Consider using a for...of loop instead of reduce to avoid performance issues:
function buildPropDefinitions({ app = {}, props = {}, }) { - return Object.entries(props) - .reduce((newProps, [ - key, - prop, - ]) => { - if (!prop.propDefinition) { - return { - ...newProps, - [key]: prop, - }; - } - - const [ - , ...propDefinitionItems - ] = prop.propDefinition; - - return { - ...newProps, - [key]: { - ...prop, - propDefinition: [ - app, - ...propDefinitionItems, - ], - }, - }; - }, {}); + const newProps = {}; + + for (const [key, prop] of Object.entries(props)) { + if (!prop.propDefinition) { + newProps[key] = prop; + continue; + } + + const [, ...propDefinitionItems] = prop.propDefinition; + + newProps[key] = { + ...prop, + propDefinition: [ + app, + ...propDefinitionItems, + ], + }; + } + + return newProps; }🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
[error] 23-23: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
52-55: LGTM!Appropriately exports both utility functions.
components/procore/actions/create-rfi/create-rfi.mjs (1)
49-64: Review required assignee IDs configurationThe
assigneIdsproperty is explicitly marked asoptional: false, making it required. Confirm if this is intentional as it forces users to provide assignees for every RFI.Most Procore components make fields optional when possible to improve flexibility. Consider whether this field should truly be required.
components/procore/actions/create-manpower-log/create-manpower-log.mjs (1)
32-37: Add datetime format validationThe datetime field includes an example format but lacks validation to ensure it follows the ISO 8601 standard required by Procore's API.
Add validation to ensure the datetime follows the required format:
datetime: { type: "string", label: "Datetime", description: "The datetime of the record. Eg. `2025-04-01T00:00:00Z`.", optional: true, + validate: (value) => { + if (!value) return true; // Skip validation if not provided since it's optional + const regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/; + if (!regex.test(value)) { + return "Datetime must be in ISO 8601 format (YYYY-MM-DDThh:mm:ssZ)"; + } + return true; + }, },components/procore/sources/new-prime-contract-event-instant/new-prime-contract-event-instant.mjs (2)
1-3: No concerns regarding import statements.
These imports look consistent and properly reference the shared modules.
4-15: Structural clarity confirmed.
Exporting an object that spreadscommonwhile adding specific metadata for prime contract events is a clean approach.components/procore/procore.app.mjs (3)
1-62: Solid prop definitions.
The props forcompanyId,projectId,resourceName, andeventTypesare well-structured with suitable asyncoptionslogic for dynamic selection.
63-172: Comprehensive property listing for additional IDs.
Defining optional user IDs permits flexible configuration. The async selectors inlistLocations/listUsershelp in large codebases. Great approach.
203-229: General request flow is well-designed.
ThemakeRequestmethod abstracts key logic (URL, headers, etc.), centralizing the request flow for clarity.components/procore/sources/new-budget-snapshot-event-instant/new-budget-snapshot-event-instant.mjs (2)
1-15: Neat extension of shared webhook logic.
Reusingcommonwhile customizing the resource name for budget snapshots helps maintain consistency across event sources.
16-46: Cautious approach to data retrieval.
Wrapping pagination in a try/catch ensures partial results are returned in case of errors. This fosters resilience in the webhook flow. Good practice.components/procore/sources/new-commitment-change-order-event-instant/new-commitment-change-order-event-instant.mjs (1)
1-3: Imports look good.
No issues identified with the import statements.components/procore/sources/common/webhook.mjs (4)
38-91: Add error handling to theactivatehook
If an error occurs while creating the webhook or triggers, partial resources may remain and cause unexpected behavior. Wrap these operations in a try/catch block, and in case of an error, clean up any partially created resources for a consistent state.
124-126: Implement thegenerateMetamethod to avoidConfigurationError.
This method is invoked by child classes or references in the code, so leaving it unimplemented may lead to errors. Provide a default or a validated metadata object to prevent runtime exceptions.
145-147: OverridegetResourceNameor provide a default implementation.
This method is required for event triggers and cannot remain unimplemented, or it will result inConfigurationError.
185-192: Add error handling to theprocessResourcemethod.
IfgenerateMetaor other code inside this method throws, it will break event emission. Wrap it in a try/catch block to prevent unhandled exceptions and optionally emit fallback metadata for visibility.
|
/approve |
WHY
Resolves #15769
Summary by CodeRabbit
New Features
Chores