HubSpot (minor) add custom properties and caching #449
Conversation
WalkthroughThe pull request updates the HubSpot integration components to version 4.1.0. It introduces a new asynchronous utility, Changes
Possibly related PRs
Suggested reviewers
✨ 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 (
|
… document custom properties addition in bundle.json
There was a problem hiding this comment.
Pull Request Overview
This PR introduces caching for HubSpot properties used by various CRM endpoints and refactors duplicated API calls into a shared utility function. Key changes include:
- Adding the async getObjectProperties function in commons.js to centralize fetching and caching of object properties.
- Replacing direct API calls with getObjectProperties in GetDealsProperties, GetContactsProperties, NewContact, UpdatedContact, NewDeal, UpdatedDeal, and UpdateDeal.
- Updating the imports to ensure the new caching logic is available across files.
Reviewed Changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/appmixer/hubspot/commons.js | Added getObjectProperties with caching logic for HubSpot object properties. |
| src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js | Replaced direct API call with getObjectProperties call. |
| src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js | Replaced direct API call with getObjectProperties call. |
| src/appmixer/hubspot/crm/NewContact/NewContact.js | Replaced API call with getObjectProperties to provide names for default properties. |
| src/appmixer/hubspot/crm/UpdatedContact/UpdatedContact.js | Updated to use getObjectProperties for retrieving contact properties by name. |
| src/appmixer/hubspot/crm/NewDeal/NewDeal.js | Updated to use getObjectProperties and return names for deal properties. |
| src/appmixer/hubspot/crm/UpdatedDeal/UpdatedDeal.js | Added getObjectProperties import and replaced direct API call with its usage. |
| src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js | Minor cleanup with no major functional changes. |
Files not reviewed (3)
- src/appmixer/hubspot/bundle.json: Language not supported
- src/appmixer/hubspot/crm/CreateContact/component.json: Language not supported
- src/appmixer/hubspot/crm/GetDealsProperties/component.json: Language not supported
There was a problem hiding this comment.
Pull Request Overview
This PR introduces caching of HubSpot property data and enables the handling of custom properties for contact and deal operations. Key changes include:
- Replacing direct API calls for fetching properties with a new getObjectProperties function that implements caching.
- Integrating custom property merging into payloads for Create and Update operations on Contacts and Deals.
- Updating properties retrieval in several modules (e.g., NewContact, NewDeal, GetDealsProperties) to use the new caching mechanism.
Reviewed Changes
Copilot reviewed 11 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/appmixer/hubspot/crm/UpdatedDeal/UpdatedDeal.js | Uses caching via getObjectProperties for deal properties retrieval. |
| src/appmixer/hubspot/crm/UpdatedContact/UpdatedContact.js | Uses caching via getObjectProperties for contact properties retrieval. |
| src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js | Merges custom properties into the deal update payload. |
| src/appmixer/hubspot/crm/UpdateContact/UpdateContact.js | Merges custom properties into the contact update payload. |
| src/appmixer/hubspot/crm/NewDeal/NewDeal.js | Uses caching via getObjectProperties for deal properties retrieval during creation. |
| src/appmixer/hubspot/crm/NewContact/NewContact.js | Uses caching via getObjectProperties for contact properties retrieval during creation. |
| src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js | Fetches deal properties using the new caching mechanism. |
| src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js | Fetches contact properties using the new caching mechanism. |
| src/appmixer/hubspot/crm/CreateDeal/CreateDeal.js | Merges custom properties into the deal creation payload. |
| src/appmixer/hubspot/crm/CreateContact/CreateContact.js | Merges custom properties into the contact creation payload. |
| src/appmixer/hubspot/commons.js | Adds the getObjectProperties function with caching and locking. |
Files not reviewed (7)
- src/appmixer/hubspot/bundle.json: Language not supported
- src/appmixer/hubspot/crm/CreateContact/component.json: Language not supported
- src/appmixer/hubspot/crm/CreateDeal/component.json: Language not supported
- src/appmixer/hubspot/crm/GetContactsProperties/component.json: Language not supported
- src/appmixer/hubspot/crm/GetDealsProperties/component.json: Language not supported
- src/appmixer/hubspot/crm/UpdateContact/component.json: Language not supported
- src/appmixer/hubspot/crm/UpdateDeal/component.json: Language not supported
Comments suppressed due to low confidence (5)
src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js:21
- [nitpick] The use of the key 'AND' in customProperties is unclear. Consider renaming it to something more descriptive, such as 'customPropertiesList', to improve clarity.
const customFieldsArray = context.messages.in.content.customProperties?.AND || [];
src/appmixer/hubspot/crm/UpdateContact/UpdateContact.js:25
- [nitpick] The use of the key 'AND' in customProperties is unclear. Consider renaming it to a more descriptive key, such as 'customPropertiesList', for better readability.
const customFieldsArray = context.messages.in.content.customProperties?.AND || [];
src/appmixer/hubspot/crm/CreateDeal/CreateDeal.js:20
- [nitpick] The key 'AND' used for retrieving custom properties is ambiguous. A more descriptive name like 'customPropertiesList' might enhance code clarity.
const customFieldsArray = context.messages.in.content.customProperties?.AND || [];
src/appmixer/hubspot/crm/CreateContact/CreateContact.js:24
- [nitpick] Consider renaming the key 'AND' in customProperties to a clearer term (e.g., 'customPropertiesList') for improved readability.
const customFieldsArray = context.messages.in.content.customProperties?.AND || [];
src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js:34
- There is no visible test coverage verifying that custom properties are correctly merged into the payload. Consider adding tests to ensure this behavior works as expected.
...customProperties
src/appmixer/hubspot/commons.js
Outdated
| const cacheKeyPrefix = 'hubspot_properties_' + objectType; | ||
| let lock; | ||
| try { | ||
| lock = await context.lock('hubspot_properties'); |
There was a problem hiding this comment.
[nitpick] Using a static lock key ('hubspot_properties') for all object types could lead to collisions when getObjectProperties is called concurrently for different object types. Consider incorporating the objectType into the lock key.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/appmixer/hubspot/crm/CreateContact/CreateContact.js (1)
48-59: Consider including custom properties in the response.While custom properties are correctly sent to the HubSpot API, they aren't included in the response object that's sent back via
context.sendJson(). Consider adding them to provide a complete view of the created contact.return context.sendJson({ id: data.id, website: properties.website ? properties.website : '', city: properties.city ? properties.city : '', firstname: properties.firstname ? properties.firstname : '', zip: properties.zip ? properties.zip : '', lastname: properties.lastname ? properties.lastname : '', phone: properties.phone ? properties.phone : '', state: properties.state ? properties.state : '', address: properties.address ? properties.address : '', email: properties.email ? properties.email : '', + // Include any custom properties that were returned + ...Object.entries(properties) + .filter(([key]) => Object.keys(customProperties).includes(key)) + .reduce((acc, [key, value]) => ({ ...acc, [key]: value || '' }), {}) }, 'contact');src/appmixer/hubspot/commons.js (2)
43-43: Consider adding a comment explaining the cache TTL valueThe default TTL of 60 seconds (1 minute) seems reasonable for caching object properties, but it would be helpful to add a comment explaining why this value was chosen and what factors might influence changing it.
- const objectPropertiesCacheTTL = context.config.objectPropertiesCacheTTL || (60 * 1000); + // Default cache TTL set to 1 minute as property definitions rarely change + // Can be configured via context.config.objectPropertiesCacheTTL if needed + const objectPropertiesCacheTTL = context.config.objectPropertiesCacheTTL || (60 * 1000);
47-47: Lock key could be more specificThe lock key 'hubspot_properties' is generic and could potentially cause contention if multiple different object types are being accessed simultaneously. Consider making it more specific by including the objectType.
- lock = await context.lock('hubspot_properties'); + lock = await context.lock(`hubspot_properties_${objectType}`);src/appmixer/hubspot/crm/UpdateDeal/component.json (1)
51-81: Inspector Index Ordering Note
Both"dealstage"and"pipeline"fields are assigned an index of3in the inspector configuration. While this may be intentional for grouping purposes, please confirm that this duplicate index does not lead to any unintended ordering issues in the UI. If a sequential order is desired, consider adjusting one of the indices (for example, setting"pipeline"’s index to4).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/appmixer/hubspot/bundle.json(2 hunks)src/appmixer/hubspot/commons.js(1 hunks)src/appmixer/hubspot/crm/CreateContact/CreateContact.js(2 hunks)src/appmixer/hubspot/crm/CreateContact/component.json(3 hunks)src/appmixer/hubspot/crm/CreateDeal/CreateDeal.js(1 hunks)src/appmixer/hubspot/crm/CreateDeal/component.json(3 hunks)src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js(1 hunks)src/appmixer/hubspot/crm/GetContactsProperties/component.json(2 hunks)src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js(2 hunks)src/appmixer/hubspot/crm/GetDealsProperties/component.json(2 hunks)src/appmixer/hubspot/crm/NewContact/NewContact.js(2 hunks)src/appmixer/hubspot/crm/NewDeal/NewDeal.js(2 hunks)src/appmixer/hubspot/crm/UpdateContact/UpdateContact.js(2 hunks)src/appmixer/hubspot/crm/UpdateContact/component.json(3 hunks)src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js(1 hunks)src/appmixer/hubspot/crm/UpdateDeal/component.json(3 hunks)src/appmixer/hubspot/crm/UpdatedContact/UpdatedContact.js(2 hunks)src/appmixer/hubspot/crm/UpdatedDeal/UpdatedDeal.js(2 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
src/appmixer/hubspot/crm/CreateDeal/CreateDeal.js (3)
src/appmixer/hubspot/crm/CreateContact/CreateContact.js (5)
customFieldsArray(24-24)context(8-19)context(21-21)customProperties(25-28)payload(30-44)src/appmixer/hubspot/crm/UpdateContact/UpdateContact.js (5)
customFieldsArray(25-25)context(8-20)context(22-22)customProperties(26-29)payload(31-45)src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js (5)
customFieldsArray(21-21)context(8-16)context(18-18)customProperties(22-25)payload(27-36)
src/appmixer/hubspot/commons.js (2)
src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js (2)
context(9-9)properties(11-11)src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js (2)
context(9-9)properties(11-11)
src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js (4)
src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js (2)
require(3-3)properties(11-11)src/appmixer/hubspot/crm/NewDeal/NewDeal.js (1)
require(3-3)src/appmixer/hubspot/crm/NewContact/NewContact.js (1)
require(3-3)src/appmixer/hubspot/commons.js (1)
properties(55-55)
src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js (4)
src/appmixer/hubspot/crm/NewDeal/NewDeal.js (1)
require(3-3)src/appmixer/hubspot/crm/UpdatedDeal/UpdatedDeal.js (1)
require(3-3)src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js (5)
require(3-3)context(9-9)hs(10-10)Hubspot(2-2)properties(11-11)src/appmixer/hubspot/commons.js (1)
properties(55-55)
🔇 Additional comments (41)
src/appmixer/hubspot/crm/GetContactsProperties/component.json (2)
3-3: Author information updated to organization-levelThe author field has been updated from an individual attribution to the organization level. This appears to be part of a standardization effort across components.
19-19: Version incremented appropriatelyVersion number has been bumped from "1.1.0" to "1.1.1" to reflect the changes in this PR. This follows proper semantic versioning practices.
src/appmixer/hubspot/bundle.json (2)
3-3: Version updated for feature additionsThe bundle version has been incremented from "4.0.5" to "4.1.0", which correctly reflects the addition of new features while maintaining backward compatibility.
61-64: Changelog entries accurately document new featuresThe changelog entries for version 4.1.0 properly document the two main features being added:
- Custom properties for specific components
- Caching of Contact and Deal properties
This aligns with the PR objectives and helps users understand what's changed in this version.
src/appmixer/hubspot/crm/GetDealsProperties/component.json (2)
3-3: Author information standardizedThe author attribution has been updated from individual to organization level, consistent with the changes in other components.
19-19: Version incremented appropriatelyVersion number has been bumped from "1.0.1" to "1.0.2" for this component to reflect the changes in this PR.
src/appmixer/hubspot/crm/NewDeal/NewDeal.js (2)
3-3: Added import for new utility functionAdded import for the
getObjectPropertiesutility function from the commons module. This is part of the implementation of property caching mentioned in the PR description.
60-60: Replaced direct API call with cached utility functionThe code now uses the
getObjectPropertiesutility function instead of making a direct API call. This change implements the caching functionality for deal properties as specified in the PR objectives.This refactoring:
- Reduces code duplication across components
- Centralizes property fetching logic
- Enables caching to improve performance
- Makes the code more maintainable
src/appmixer/hubspot/crm/UpdateContact/UpdateContact.js (2)
25-29: Implementation of custom properties extraction looks good.The code effectively extracts custom properties from the input context using optional chaining for safe access and defaults to an empty array if not present. The
reducemethod elegantly transforms the array of fields into a usable object format.
42-43: Properly integrates custom properties into the payload.Spreading the custom properties at the end of the properties object ensures they don't override the standard properties while allowing for flexible extension of the contact data.
src/appmixer/hubspot/crm/UpdatedDeal/UpdatedDeal.js (2)
3-3: Good addition of the getObjectProperties import.The import of the
getObjectPropertiesutility from the commons module supports the caching functionality mentioned in the PR objectives.
66-66: Effective implementation of property caching.Replacing the direct API call with
getObjectPropertiesimproves the code by implementing caching for deal properties, which aligns with the PR objectives and should enhance performance for repeated calls.src/appmixer/hubspot/crm/CreateDeal/CreateDeal.js (2)
20-24: Good implementation of custom properties extraction.The code follows a consistent pattern seen in other components for extracting and processing custom properties, making the implementation coherent across the codebase.
32-33: Properly integrates custom properties into the deal payload.The code maintains the existing default value for pipeline while correctly spreading the custom properties at the end to ensure they don't override standard properties.
src/appmixer/hubspot/crm/CreateContact/CreateContact.js (2)
24-28: Good implementation of custom properties extraction.The extraction and transformation of custom properties is consistent with the pattern used in other components, maintaining code uniformity across the codebase.
41-42: Custom properties correctly integrated into the contact payload.The implementation properly adds custom properties to the payload while maintaining the existing structure.
src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js (2)
21-25: Adding support for custom properties is a good enhancementThis implementation correctly extracts custom properties from the input context and transforms them into a format compatible with the HubSpot API. The use of optional chaining (
?.) and default empty array provides good error handling.
33-34:❓ Verification inconclusive
Custom properties are properly merged into the payload
The code correctly includes custom properties in the payload by using the spread operator. This implementation provides flexibility for users to add any additional fields supported by the HubSpot API.
One thing to verify is whether there could be name collisions between standard properties and custom properties. If a custom property has the same name as a standard property (like "amount" or "dealname"), it would override the standard value.
🏁 Script executed:
#!/bin/bash # Check if there are any validation or handling for property name collisions in other HubSpot components grep -A 10 -B 10 "customProperties" src/appmixer/hubspot/crm/Length of output: 109
Action: Verify Custom Properties Merging & Collision Handling
The code in
src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.jscorrectly merges custom properties into the payload using the spread operator. However, the original verification command did not search recursively through the directory, so it remains unclear whether other modules implement logic to prevent naming collisions (e.g., when a custom property like "amount" or "dealname" overrides a standard property).To address this, please re-run the following command to search for occurrences of
customPropertiesacross all HubSpot components:Review the output to confirm whether any module includes safeguards against property name collisions. If additional validation exists in other parts of the codebase, please verify that its behavior aligns with the intended merging in UpdateDeal; otherwise, note the absence of collision checks.
🏁 Script executed:
#!/bin/bash # Recursively search for customProperties usage in the HubSpot CRM components grep -R "customProperties" src/appmixer/hubspot/crm/Length of output: 2334
Attention: Review Custom Properties Collision Handling in UpdateDeal.js
The updated merging implementation in
src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.jscorrectly uses the spread operator to include custom properties in the payload. However, as with similar components (e.g., CreateContact, CreateDeal, and UpdateContact), there is no dedicated collision-handling logic present. This means that if a custom property uses the same key as a standard property, it will override the standard value. Please verify that this behavior is intentional and that no additional safeguards are necessary.
Location to review:
src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js(lines 33-34)Action required:
- Confirm if name collisions (for instance, using keys like "amount" or "dealname") are acceptable in your design.
- If collisions should be prevented, consider implementing validation or collision-handling logic similar (or complementary) to other parts of the codebase.
src/appmixer/hubspot/crm/NewContact/NewContact.js (2)
3-3: Importing the new utility functionThe import statement correctly adds the
getObjectPropertiesfunction from the commons module.
60-60: Using cached property retrieval instead of direct API callThis change improves performance by using the new caching mechanism instead of making a direct API call to HubSpot. The parameters passed to
getObjectProperties('contacts', 'names') align with the requirements of this component.src/appmixer/hubspot/commons.js (1)
41-71: Well-implemented caching utility for HubSpot propertiesThis new utility function is a great addition that will significantly improve performance by caching property definitions. The implementation includes several good practices:
- Lock mechanism to prevent concurrent API calls for the same data
- Configurable cache TTL with a reasonable default (60 seconds)
- Caching both the full property objects and just the names for different use cases
- Proper error handling with a finally block to release the lock
The function is well-structured and handles the different output formats needed by triggers (property names) and actions (full property objects).
src/appmixer/hubspot/crm/UpdatedContact/UpdatedContact.js (2)
3-3: Updated import to include the new utility functionThe import statement now correctly includes both the existing
WATCHED_PROPERTIES_CONTACTconstant and the newgetObjectPropertiesfunction.
68-68: Using cached property retrieval instead of direct API callSimilar to the NewContact component, this change improves performance by utilizing the new caching mechanism instead of making a direct API call to HubSpot. The parameters are consistent with what's needed for this component.
src/appmixer/hubspot/crm/UpdateContact/component.json (3)
3-3: Author information updated.The author information has been updated from an individual contributor to the organization name.
34-35: Custom properties capability added.Added support for custom properties as an object type, allowing dynamic property addition when updating contacts.
109-137: Added UI support for custom properties.This change enhances the component's UI to allow users to add custom contact properties. The implementation correctly:
- Uses an expression type with AND logic
- Sources property names from the GetContactsProperties component
- Provides appropriate tooltips and labels
This aligns with the PR objective to add custom properties support for the UpdateContact action.
src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js (3)
3-3: Imported the new getObjectProperties utility.Added import for the shared caching utility that will improve performance.
11-13: Implemented property caching.Replaced direct API calls with the new
getObjectPropertiesutility function. This implements caching for deal properties, which will improve performance by reducing redundant API calls.The function appropriately passes the context, HubSpot client instance, object type ('deals'), and scope ('all').
68-77: Added custom fields filtering functionality.This new method filters deal properties to only show those with a
createdUserId(custom properties) and aformFieldflag. This enables presenting only relevant properties in the custom properties dropdown.Implementation is clean and follows functional programming patterns with chained array methods.
src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js (3)
3-3: Imported the new getObjectProperties utility.Added import for the shared caching utility that will improve performance when fetching contact properties.
11-14: Implemented contact properties caching.Replaced direct API calls with the new
getObjectPropertiesutility function. This change is consistent with the approach used in GetDealsProperties.js and aligns with the PR objective to add caching for HubSpot properties.
16-24: Added custom fields filtering functionality.Similar to the implementation in GetDealsProperties.js, this method filters contact properties to show only those with a
createdUserIdandformField. This maintains consistency across the codebase.The implementation is clean and follows functional programming patterns with chained array methods.
src/appmixer/hubspot/crm/CreateDeal/component.json (3)
3-3: Author information updated.The author information has been updated from an individual contributor to the organization name, maintaining consistency with other component files.
32-33: Custom properties capability added.Added support for custom properties as an object type, allowing dynamic property addition when creating deals.
101-127: Added UI support for custom properties.This change enhances the component's UI to allow users to add custom deal properties. The implementation correctly:
- Uses an expression type with AND logic
- Sources property names from the GetDealsProperties component using the new customFieldsToSelectArray transformation
- Provides appropriate tooltips and labels
This aligns with the PR objective to add custom properties support for the CreateDeal action.
src/appmixer/hubspot/crm/CreateContact/component.json (3)
3-3: Updated Author Field
The author has been updated to "Appmixer [email protected]" which aligns with the new branding. Please verify that this change is consistent with other HubSpot components.
32-33: Addition of Custom Properties in Schema
A new field"customProperties"of type"object"has been added alongside"zip". Ensure that downstream logic (in the component’s code) properly handles this additional field when constructing the payload for contact creation.
100-127: Inspector Configuration for Custom Properties
The inspector section now includes a"customProperties"entry as an expression with a detailed configuration (including fields for"name"and"value"). This configuration allows users to dynamically select a property from the source endpoint and provide its value. Please verify that:
- The
"exclusiveFields": ["name"]setting is documented and behaves as expected in the frontend UI.- The source URL (
/component/appmixer/hubspot/crm/GetContactsProperties?outPort=out) and the transform path ("./GetContactsProperties#customFieldsToSelectArray") return data in the expected format.src/appmixer/hubspot/crm/UpdateDeal/component.json (3)
3-3: Updated Author Field
The author field has been updated to "Appmixer [email protected]" to reflect the new branding. Confirm that all HubSpot CRM components use the same author format.
30-31: New Custom Properties Field in Input Schema
The input schema now includes a new"customProperties"field of type"object", alongside the"amount"field. Ensure that the payload construction logic for updating deals includes support for these additional properties.
100-125: Inspector Configuration for Custom Properties in UpdateDeal
A new inspector entry for"customProperties"has been added with type"expression".
- The configuration (including
"exclusiveFields","levels", and nested"fields") mirrors the pattern used in the CreateContact component but targets deals.- Verify that the source URL (
/component/appmixer/hubspot/crm/GetDealsProperties?outPort=out) and transform function ("./GetDealsProperties#customFieldsToSelectArray") return properly formatted options for selection.
|
|
and 🐇 reviewing the same PR is wild 🤣 And when they agree on something you know you (in this case I) pushed something smelly 💩…es in CreateContact component
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/appmixer/hubspot/commons.js (1)
41-73: Good implementation of property caching with proper lockingThe implementation of
getObjectPropertiesis well-structured with caching and lock mechanisms to prevent redundant API calls. I appreciate that you've incorporated the object type into the lock key as previously suggested to avoid lock collisions between different object types.A couple of observations:
There's no explicit error handling for the API call. Consider adding try/catch and logging for the actual API request to make troubleshooting easier.
The function assumes the API will always return data in the expected format. Consider adding validation for the API response structure to handle potential format changes gracefully.
async getObjectProperties(context, hubspot, objectType, output = 'all') { // Default cache TTL set to 1 minute as property definitions rarely change // Can be configured via context.config.objectPropertiesCacheTTL if needed const objectPropertiesCacheTTL = context.config.objectPropertiesCacheTTL || (60 * 1000); const cacheKeyPrefix = 'hubspot_properties_' + objectType; let lock; try { lock = await context.lock(`hubspot_properties_${objectType}`); const cached = await context.staticCache.get(cacheKeyPrefix + '_' + output); if (cached) { return cached; } // Get all properties from HubSpot. + try { const { data } = await hubspot.call('get', `crm/v3/properties/${objectType}`); + // Validate response structure + if (!data || !Array.isArray(data.results)) { + throw new Error(`Unexpected response format from HubSpot properties API for ${objectType}`); + } const properties = data.results.map(property => property.name); // Save to cache both versions: triggers and actions. await context.staticCache.set(cacheKeyPrefix + '_all', data.results, objectPropertiesCacheTTL); await context.staticCache.set(cacheKeyPrefix + '_names', properties, objectPropertiesCacheTTL); // For triggers return array of names: ['email', 'firstname', ...] if (output === 'names') { return properties; } // For actions return array of objects: [{ name: 'email', type: 'string', ... }, ...] return data.results; + } catch (error) { + await context.log({ + step: 'Error fetching HubSpot properties', + objectType, + error: error.message, + stack: error.stack + }); + throw error; + } } finally { await lock?.unlock(); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/appmixer/hubspot/commons.js(1 hunks)src/appmixer/hubspot/crm/CreateContact/CreateContact.js(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/appmixer/hubspot/commons.js (2)
src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js (2)
context(9-9)properties(11-11)src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js (2)
context(9-9)properties(11-11)
🪛 Biome (1.9.4)
src/appmixer/hubspot/crm/CreateContact/CreateContact.js
[error] 62-62: 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)
🔇 Additional comments (2)
src/appmixer/hubspot/crm/CreateContact/CreateContact.js (2)
24-28: Good implementation of custom properties extractionThe extraction of custom properties from the input is well-implemented with a fallback to an empty array if no custom properties are provided. This ensures the reducer won't fail even if the input structure is incomplete.
Consider adding validation for the custom field names to ensure they comply with HubSpot's naming requirements and to prevent potential injection attacks.
42-42: Good use of spread operator for merging custom propertiesUsing the spread operator to merge custom properties into the payload is clean and maintainable. This approach ensures all custom properties are included in the HubSpot API call.
|
shouldn't we match the selected properties with the output schema ? shouldn't this match to the output schema? meaning, that the NewContact should return only |
exclude hardcoded properties from additional properties
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/appmixer/hubspot/crm/UpdateDeal/component.json (1)
100-127: New Inspector Input for Additional Properties
A new inspector field"additionalProperties"has been introduced with type"expression". It provides a configurable interface to add custom or HubSpot-specific properties via a subfield configuration (including"name"and"value").
- The
"name"subfield uses a select input with its options sourced dynamically from GetDealsProperties.- The
"exclusiveFields"property ensures that the"name"field remains unique in the context.Nitpick: The tooltip reads "Addition HubSpot or custom properties to add to the deal." Consider correcting "Addition" to "Additional" for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/appmixer/hubspot/bundle.json(2 hunks)src/appmixer/hubspot/crm/CreateContact/CreateContact.js(3 hunks)src/appmixer/hubspot/crm/CreateContact/component.json(4 hunks)src/appmixer/hubspot/crm/CreateDeal/CreateDeal.js(1 hunks)src/appmixer/hubspot/crm/CreateDeal/component.json(3 hunks)src/appmixer/hubspot/crm/GetContact/component.json(2 hunks)src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js(1 hunks)src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js(2 hunks)src/appmixer/hubspot/crm/UpdateContact/UpdateContact.js(2 hunks)src/appmixer/hubspot/crm/UpdateContact/component.json(3 hunks)src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js(1 hunks)src/appmixer/hubspot/crm/UpdateDeal/component.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/appmixer/hubspot/crm/UpdateContact/UpdateContact.js
- src/appmixer/hubspot/crm/CreateContact/CreateContact.js
- src/appmixer/hubspot/crm/CreateDeal/CreateDeal.js
- src/appmixer/hubspot/crm/UpdateDeal/UpdateDeal.js
- src/appmixer/hubspot/bundle.json
- src/appmixer/hubspot/crm/UpdateContact/component.json
- src/appmixer/hubspot/crm/CreateContact/component.json
🧰 Additional context used
🧬 Code Definitions (1)
src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js (6)
src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js (3)
require(3-3)context(9-9)properties(11-11)src/appmixer/hubspot/crm/UpdatedDeal/UpdatedDeal.js (1)
require(3-3)src/appmixer/hubspot/crm/NewContact/NewContact.js (1)
require(3-3)src/appmixer/hubspot/crm/NewDeal/NewDeal.js (1)
require(3-3)src/appmixer/hubspot/crm/UpdatedContact/UpdatedContact.js (1)
require(3-3)src/appmixer/hubspot/commons.js (1)
properties(57-57)
🔇 Additional comments (15)
src/appmixer/hubspot/crm/GetContact/component.json (2)
76-78: Good implementation for matching output properties with input selectionsThis enhancement elegantly addresses the issue raised in the PR comments about matching selected properties with the output schema. By mapping
in/propertiestoinputs/in/properties, you ensure that only the properties selected in the input will be included in the component's output.
6-6: Version increment looks appropriateThe version update from 1.0.2 to 1.0.3 is appropriate given the feature enhancement for property filtering.
src/appmixer/hubspot/crm/GetDealsProperties/GetDealsProperties.js (3)
3-3: Good implementation of caching dependenciesImporting the
getObjectPropertiesutility andWATCHED_PROPERTIES_DEALconstant sets up the foundation for the caching enhancement mentioned in the PR objectives.
11-13: Improved efficiency with cachingReplacing the direct API call with the
getObjectPropertiesutility function implements the caching functionality for deal properties, which will improve performance for components that need to access these properties frequently.
68-77: Well-implemented support for custom propertiesThe new
additionalFieldsToSelectArraymethod effectively implements the custom properties feature mentioned in the PR objectives. It properly filters properties that:
- Have a form field
- Are not included in the hardcoded
WATCHED_PROPERTIES_DEALarrayThis enables users to select from both standard and custom properties that aren't hardcoded into the component.
src/appmixer/hubspot/crm/GetContactsProperties/GetContactsProperties.js (3)
4-4: Good implementation of caching dependenciesImporting the
getObjectPropertiesutility andWATCHED_PROPERTIES_CONTACTconstant sets up the foundation for the caching enhancement mentioned in the PR objectives.
12-23: Effective implementation of output property filteringThis implementation properly addresses the concern raised by vtals in the PR comments about matching selected properties with the output schema. The conditional logic ensures that when properties are specified in the input, only those properties are returned in the output.
The caching mechanism via
getObjectPropertiesalso improves performance by reducing redundant API calls.
26-34: Well-implemented support for custom propertiesThe new
additionalFieldsToSelectArraymethod effectively implements the custom properties feature for contacts. It properly filters properties that:
- Have a form field
- Are not included in the hardcoded
WATCHED_PROPERTIES_CONTACTarrayThis enables users to select from both standard and custom properties that aren't hardcoded into the component.
src/appmixer/hubspot/crm/CreateDeal/component.json (3)
3-3: Author update and version increment look appropriateThe author information update and version increment from 1.0.3 to 1.0.4 are appropriate given the feature enhancements for custom properties.
Also applies to: 6-6
33-33: Good schema update for custom propertiesAdding the
additionalPropertiesfield of type object to the schema properly implements the custom properties feature mentioned in the PR objectives, allowing users to add custom properties when creating a deal.
101-127: Well-structured UI configuration for custom propertiesThe UI configuration for
additionalPropertiesis well-structured and provides a good user experience:
- The property selection dropdown is populated using the new
additionalFieldsToSelectArraymethod- The UI is organized with proper labels and tooltips
- The implementation follows the same pattern as other properties in the component
This implementation successfully fulfills the PR objective of adding custom properties for the CreateDeal action.
src/appmixer/hubspot/crm/UpdateDeal/component.json (4)
3-3: Update Author Attribution
The author field has been updated from the previous attribution to "Appmixer [email protected]". This change aligns with the rebranding and should be consistent across all related documentation and components.
6-6: Version Bump to 1.0.3
The version has been updated to "1.0.3", appropriately reflecting the new enhancements in this component. Ensure that this version bump is consistent with the overall release strategy for the HubSpot integration.
30-31: Extend Input Schema with Custom Properties
A new field"additionalProperties"of type"object"has been added alongside the"amount"field in the inPorts schema. This change enables users to include extra properties during deal updates. Please verify that any payload validations or transformations downstream properly handle this new object structure.
94-99: Inspector Configuration for "Amount" Field
The inspector section for the"amount"field is defined with type"text", label, tooltip, and index 7. The structure remains consistent, ensuring that it is rendered correctly in the editor.
|
@vtalas
|
GetContactwith itspropertiesif definedSummary by CodeRabbit
New Features
Chores