-
Notifications
You must be signed in to change notification settings - Fork 5.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Components - microsoft_dynamics_365_sales #15478
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces new functionality for Microsoft Dynamics 365 Sales, encompassing actions for finding contacts and creating custom entities. It includes a module for creating custom entities with various configuration options, a component for searching contacts with flexible filtering, and a static array of language codes for internationalization. Additionally, the application methods have been expanded to enhance API interactions, and the package configuration has been updated to reflect new dependencies and versioning. Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (1)
6-21
: Consider renaming the destructured variable to avoid confusion.In the
contactId
prop options, destructuringcontactid
asvalue
can be confusing since it overlaps with the variable namedvalue
returned bythis.listContacts()
. You might rename them to something more descriptive, e.g.id
andcontacts
.components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs (1)
64-83
: Utility methods are helpful, but reviseremoveSpaces
.
removeSpaces(str.replace(" ", ""))
will only remove the first space. To handle all spaces, consider a global replace or a trim if needed.- return str.replace(" ", ""); + return str.replace(/\s+/g, "");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs
(1 hunks)components/microsoft_dynamics_365_sales/actions/find-contact/find-contact.mjs
(1 hunks)components/microsoft_dynamics_365_sales/common/language-codes.mjs
(1 hunks)components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs
(1 hunks)components/microsoft_dynamics_365_sales/package.json
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/microsoft_dynamics_365_sales/common/language-codes.mjs
⏰ 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 (23)
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (9)
22-35
: Looks good!The
solutionId
prop definition mirrorscontactId
and appears correct for retrieving solution data dynamically.
38-40
: Base URL logic confirmed.Constructing the base URL from the
org_domain
is straightforward and appears correct.
60-65
: Implementation confirmed.Fetching contacts via
_makeRequest
is consistent with the approach used throughout the module.
66-73
: Publisher retrieval looks good.Short, focused, and consistent with the rest of the code.
74-81
: Solution retrieval confirmed.No issues with constructing the path for getting a solution by its ID.
82-87
: List solutions logic is fine.The approach matches the existing methods for listing resources.
88-95
: Entity retrieval is straightforward.Just ensure that valid entity identifiers are passed in. The method is minimal and consistent.
96-101
: Entity creation endpoint usage confirmed.
createCustomEntity
properly setsmethod: "POST"
and targets/EntityDefinitions
. The pattern aligns with the rest of the code.
104-104
: End of export confirmed.No additional feedback.
components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs (8)
1-4
: Imports look good.Bringing in your Microsoft app, language codes, and the
pluralize
library is aligned with the requirements.
5-10
: Action metadata is well-defined.The action key, name, description, and version are properly declared.
11-62
: Props are comprehensive and well-documented.You've provided robust property definitions, including a default language code and optional fields for advanced customization.
85-90
: Solution lookup is straightforward.Retrieving the solution by
solutionId
is consistent with the rest of the module’s pattern.
91-95
: Publisher lookup confirmed.Calling
getPublisher
with the solution’s publisher ID is correct for referencing the prefix data.
96-101
: Creating the custom entity with_makeRequest
.Checks out: the code uses
method: "POST"
and merges additional configuration properly.
104-145
: Metadata object structure is thorough.You define labels, schema names, default owners, and more. This aligns with the Microsoft Dynamics 365 web API approach.
147-156
: Final retrieval and summary are clear.Extracting the entity’s ID from the headers, fetching it, and exporting a summary is a neat workflow.
components/microsoft_dynamics_365_sales/actions/find-contact/find-contact.mjs (3)
1-2
: Imports confirmed.Bringing in the Microsoft app reference is consistent with your app’s structure.
3-29
: Action definition is properly labeled.Key, name, description, and version fields are accurate. Props reference the
contactId
definition in the parent app, plus optional name and filter fields.
30-58
: Dynamic filter construction is well-executed.Combining user inputs with logical “and” is straightforward. Ensuring
$filter
is undefined if no terms are present is a clean approach.components/microsoft_dynamics_365_sales/package.json (3)
3-3
: Version Update VerificationThe version has been updated to
"0.1.0"
which is appropriate for introducing new functionality (e.g., the Microsoft Dynamics 365 Sales components). Ensure that this update aligns with your semantic versioning and release strategy.
14-14
: Publish Configuration UpdateThe
"publishConfig": { "access": "public" }
update is correct for ensuring public distribution of this package. Please verify that this setting is consistent with your deployment policies.
15-17
: Dependency Addition ReviewThe added dependencies
"@pipedream/platform": "^3.0.3"
and"pluralize": "^8.0.0"
appear to support the new functionality for Microsoft Dynamics 365 Sales. Confirm that these dependency versions are compatible with your existing components and that any changes in behavior are documented.
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs (2)
64-83
: Helper Methods: Ensure Robustness and Consistency
parseAttributes
(lines 65-70):
- The method correctly checks if
additionalAttributes
exists and whether it is a string or an object.- Suggestion: Wrap the JSON parsing in a try/catch block to handle potential JSON parsing errors and report a more descriptive error if the input is malformed.
Proposed change:- parseAttributes() { - return this.additionalAttributes - ? typeof this.additionalAttributes === "string" - ? JSON.parse(this.additionalAttributes) - : this.additionalAttributes - : []; - }, + parseAttributes() { + try { + if (!this.additionalAttributes) return []; + return typeof this.additionalAttributes === "string" + ? JSON.parse(this.additionalAttributes) + : this.additionalAttributes; + } catch (error) { + throw new Error("Invalid JSON format in additionalAttributes"); + } + },
removeSpaces
(lines 72-74):
This one-liner function effectively removes all whitespace from a string.
buildLocalizedLabelArray
(lines 75-83):
The function cleanly wraps a given label into the format expected by the Dynamics 365 API.
85-156
:run
Method: Logical Flow with API IntegrationThe asynchronous
run
method implements the main logic as follows:
Solution and Publisher Retrieval (lines 86-94):
It first retrieves the solution usingthis.microsoft.getSolution
and then the publisher using the solution’s publisher ID. Ensure that both API methods reliably return the expected fields (e.g.,_publisherid_value
).Parsing Additional Attributes (line 96):
Uses the helper method to ensure attributes are provided in the correct format.Creating the Custom Entity (lines 98-145):
The payload is well-constructed. The data object builds:
- The primary attribute metadata (including schema, display name, and attribute settings).
- Display names using localized labels and a pluralized version for the collection name.
- Conditional inclusion of the description.
- Custom settings for activities/notes and ownership type.
The use of the spread operator (
...attributes
) to include additional attributes is appropriate if the expectation is thatattributes
is an array.Extracting the Entity ID (line 147):
The code extracts the entity ID using string manipulation on the header.
Suggestion: Validate that the headerodata-entityid
exists before using substring operations.
Proposed change:- const entityId = headers["odata-entityid"].substring(headers["odata-entityid"].lastIndexOf("/") + 1); + const entityHeader = headers["odata-entityid"]; + if (!entityHeader) { + throw new Error("Missing 'odata-entityid' header in response"); + } + const entityId = entityHeader.substring(entityHeader.lastIndexOf("/") + 1);
- Entity Retrieval and Summary Export (lines 148-156):
The subsequent retrieval of the created entity and export of a summary message are clear and concise.Overall, the method cleanly integrates multiple asynchronous API calls to create and verify a custom entity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs
(1 hunks)
⏰ 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 (2)
components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs (2)
1-4
: Import Statements are Clear and OrganizedThe imports use clean relative paths and load the necessary modules (the Dynamics 365 app interface, language codes, and the pluralize library). No issues were found.
5-63
: Export Object and Props Definition: Well-StructuredThe exported object defines the action’s key, name, description, and version clearly. The properties (props) are configured with appropriate types, default values, and helpful descriptions. In particular:
- The
solutionId
prop correctly uses thepropDefinition
from the Microsoft Dynamics 365 app.- The
languageCode
is set with a default of 1033 and marked optional.- Additional props like
additionalAttributes
,hasActivities
, andhasNotes
are given clear labels and descriptions.Consider verifying that all prop labels and descriptions fully align with Microsoft's documentation requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @michelle0927 lgtm! Ready for QA!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (1)
41-59
:⚠️ Potential issueAdd error handling to _makeRequest.
The method should handle API errors and provide meaningful error messages.
_makeRequest({ $ = this, path, headers, ...opts }) { + if (!path) { + throw new Error('Path is required for API requests'); + } + return axios($, { url: `${this._baseUrl()}${path}`, headers: { ...headers, "Authorization": `Bearer ${this.$auth.oauth_access_token}`, "odata-maxversion": "4.0", "odata-version": "4.0", "content-type": "application/json", "If-None-Match": null, }, ...opts, - }); + }).catch((error) => { + const status = error?.response?.status; + const message = error?.response?.data?.error?.message || error.message; + throw new Error(`Dynamics 365 API request failed: ${status} - ${message}`); + }); },
🧹 Nitpick comments (1)
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (1)
38-102
: Refactor API paths into constants and add rate limiting.Consider the following improvements:
- Define API paths as constants for better maintainability
- Implement rate limiting to prevent API throttling
- Add input validation for method parameters
+const API_PATHS = { + CONTACTS: '/contacts', + PUBLISHERS: (id) => `/publishers(${id})`, + SOLUTIONS: '/solutions', + SOLUTION: (id) => `/solutions(${id})`, + ENTITY_DEFINITIONS: '/EntityDefinitions', +}; + +const RATE_LIMIT = { + MAX_REQUESTS: 100, + WINDOW_MS: 60000, // 1 minute +}; + +let requestCount = 0; +let windowStart = Date.now(); + methods: { // ... existing methods ... + + async _checkRateLimit() { + const now = Date.now(); + if (now - windowStart > RATE_LIMIT.WINDOW_MS) { + requestCount = 0; + windowStart = now; + } + if (requestCount >= RATE_LIMIT.MAX_REQUESTS) { + throw new Error('Rate limit exceeded. Please try again later.'); + } + requestCount++; + }, + _makeRequest({ $ = this, path, headers, ...opts }) { + await this._checkRateLimit(); // ... rest of the method ... }, listContacts(opts = {}) { return this._makeRequest({ - path: "/contacts", + path: API_PATHS.CONTACTS, ...opts, }); }, getPublisher({ publisherId, ...opts }) { + if (!publisherId) { + throw new Error('Publisher ID is required'); + } return this._makeRequest({ - path: `/publishers(${publisherId})`, + path: API_PATHS.PUBLISHERS(publisherId), ...opts, }); }, // ... update other methods similarly ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs
Show resolved
Hide resolved
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs
Show resolved
Hide resolved
/approve |
Resolves #15441
Summary by CodeRabbit
Release Notes for Microsoft Dynamics 365 Sales Integration
New Features
Improvements
Version Update