-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Microsoft Dynamics - new sources #18849
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
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a reusable polling base and pagination support, introduces five opportunity-related event sources for Microsoft Dynamics 365 Sales, adds listOpportunities and paginate to the app, and applies metadata/version bumps to actions and package.json. Changes
Sequence Diagram(s)sequenceDiagram
participant Timer as Timer
participant Source as Source (e.g., new-opportunity-created)
participant Common as Common Base
participant App as microsoft_dynamics_365_sales.app
participant API as Dynamics 365 API
participant DB as Storage
Timer->>Source: run()
Source->>Common: processEvent(max)
Common->>DB: _getLastTs()
DB-->>Common: lastTs
Common->>Source: getResourceFn()
Source-->>Common: listOpportunities
Common->>Source: getArgs()
Source-->>Common: args (orderby, params)
Common->>App: paginate({ fn: listOpportunities, args, max })
loop pages
App->>API: _makeRequest(path or url)
API-->>App: results + @odata.nextLink
App-->>Common: yielded items
end
loop items
Common->>Source: getTsField()
Source-->>Common: "createdon" / "modifiedon"
Common->>Source: getRelevantResults(batch)
Source->>DB: _getX() / _setX() (state read/write)
Source-->>Common: relevant items
Common->>Source: generateMeta(item)
Source-->>Common: meta { id, summary, ts }
Common->>Common: emit(item, meta)
end
Common->>DB: _setLastTs(maxTs)
DB-->>Common: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
🧹 Nitpick comments (6)
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (1)
110-131: Consider avoiding mutation of the args parameter.The pagination logic is correct and handles OData-style pagination properly. However, line 129 mutates the input
argsobject by settingargs.url = nextLink. While this works in the current usage context (wheregetArgs()creates fresh objects), mutating input parameters can lead to unexpected behavior if the args object is reused by callers.Consider this alternative that avoids mutation:
async *paginate({ fn, args = {}, max, }) { let count = 0; - let nextLink = null; + let currentArgs = { ...args }; do { - const response = await fn(args); + const response = await fn(currentArgs); const items = response.value; if (!items?.length) { return; } for (const item of items) { yield item; if (max && ++count >= max) { return; } } - nextLink = response["@odata.nextLink"]; - args.url = nextLink; - } while (nextLink); + const nextLink = response["@odata.nextLink"]; + if (!nextLink) break; + currentArgs = { ...args, url: nextLink }; + } while (true); },components/microsoft_dynamics_365_sales/sources/common/common.mjs (3)
43-51: Ensure results are ordered by timestamp DESC for correctness of early-break.The
breakon “older than lastTs” is only correct if pages are sorted bygetTsField()descending. Please verify every extending source sets$orderby: "<tsField> desc"ingetArgs(). Optionally, set a safe default when absent.Optional fallback:
const args = this.getArgs(); const tsField = this.getTsField(); + // Default to DESC order by timestamp if caller didn’t specify + if (!args?.params?.$orderby) { + args.params = { ...(args.params ?? {}), "$orderby": `${tsField} desc` }; + }
70-73: Emit oldest-first for chronological consistency (optional).Emitting in ascending timestamp order avoids “newest-first” bursts.
- relevantResults.forEach((item) => { + relevantResults + .sort((a, b) => Date.parse(a[this.getTsField()]) - Date.parse(b[this.getTsField()])) + .forEach((item) => { const meta = this.generateMeta(item); this.$emit(item, meta); });
7-16: Allow users to cap work per run.Expose an optional
maxResultsprop and pass it toprocessEventto bound pagination.props: { microsoftDynamics365Sales, db: "$.service.db", timer: { type: "$.interface.timer", default: { intervalSeconds: DEFAULT_POLLING_SOURCE_TIMER_INTERVAL, }, }, + maxResults: { + type: "integer", + label: "Max items to fetch per run", + optional: true, + default: 50, + }, }, ... async run() { - await this.processEvent(); + await this.processEvent(this.maxResults); },Also applies to: 76-78
components/microsoft_dynamics_365_sales/sources/opportunity-close-probability-updated/opportunity-close-probability-updated.mjs (2)
49-51: Strengthen event id to avoid collisions and capture multiple changes at same timestamp.Including separators and the new value yields clearer, more unique ids.
- id: `${opportunity.opportunityid}${ts}`, + id: `${opportunity.opportunityid}:${opportunity.modifiedon}:${opportunity.closeprobability}`,
22-28: Reduce payload with $select (optional).Limit fields to those used to shrink responses and speed polling.
return { params: { "$orderby": "modifiedon desc", + "$select": "opportunityid,name,modifiedon,closeprobability", }, };
📜 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 (10)
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/microsoft_dynamics_365_sales.app.mjs(3 hunks)components/microsoft_dynamics_365_sales/package.json(2 hunks)components/microsoft_dynamics_365_sales/sources/common/common.mjs(1 hunks)components/microsoft_dynamics_365_sales/sources/new-opportunity-created/new-opportunity-created.mjs(1 hunks)components/microsoft_dynamics_365_sales/sources/opportunity-close-date-updated/opportunity-close-date-updated.mjs(1 hunks)components/microsoft_dynamics_365_sales/sources/opportunity-close-probability-updated/opportunity-close-probability-updated.mjs(1 hunks)components/microsoft_dynamics_365_sales/sources/opportunity-estimated-value-updated/opportunity-estimated-value-updated.mjs(1 hunks)components/microsoft_dynamics_365_sales/sources/opportunity-stage-updated/opportunity-stage-updated.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
components/microsoft_dynamics_365_sales/sources/opportunity-close-date-updated/opportunity-close-date-updated.mjs (1)
components/microsoft_dynamics_365_sales/sources/common/common.mjs (3)
results(53-53)relevantResults(65-65)ts(55-55)
components/microsoft_dynamics_365_sales/sources/opportunity-stage-updated/opportunity-stage-updated.mjs (1)
components/microsoft_dynamics_365_sales/sources/common/common.mjs (3)
results(53-53)relevantResults(65-65)ts(55-55)
components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (3)
components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs (1)
response(153-156)components/microsoft_dynamics_365_sales/sources/common/common.mjs (2)
args(44-44)items(47-51)components/zep/actions/get-threads/get-threads.mjs (1)
max(39-39)
components/microsoft_dynamics_365_sales/sources/opportunity-close-probability-updated/opportunity-close-probability-updated.mjs (1)
components/microsoft_dynamics_365_sales/sources/common/common.mjs (3)
results(53-53)relevantResults(65-65)ts(55-55)
components/microsoft_dynamics_365_sales/sources/opportunity-estimated-value-updated/opportunity-estimated-value-updated.mjs (1)
components/microsoft_dynamics_365_sales/sources/common/common.mjs (3)
results(53-53)relevantResults(65-65)ts(55-55)
components/microsoft_dynamics_365_sales/sources/common/common.mjs (6)
components/zep/actions/get-threads/get-threads.mjs (1)
max(39-39)components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (1)
items(118-118)components/microsoft_dynamics_365_sales/sources/opportunity-close-date-updated/opportunity-close-date-updated.mjs (2)
ts(47-47)relevantResults(34-34)components/microsoft_dynamics_365_sales/sources/opportunity-close-probability-updated/opportunity-close-probability-updated.mjs (2)
ts(47-47)relevantResults(34-34)components/microsoft_dynamics_365_sales/sources/opportunity-estimated-value-updated/opportunity-estimated-value-updated.mjs (2)
ts(47-47)relevantResults(34-34)components/microsoft_dynamics_365_sales/sources/opportunity-stage-updated/opportunity-stage-updated.mjs (2)
ts(47-47)relevantResults(34-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (10)
components/microsoft_dynamics_365_sales/actions/create-custom-entity/create-custom-entity.mjs (1)
9-9: LGTM! Metadata version bump.The version update aligns with the broader PR changes introducing new opportunity sources.
components/microsoft_dynamics_365_sales/package.json (1)
3-3: LGTM! Package version and dependency updates.The minor version bump to 0.2.0 appropriately reflects the addition of new opportunity-level sources, and the platform dependency update aligns with the new functionality.
Also applies to: 16-16
components/microsoft_dynamics_365_sales/actions/find-contact/find-contact.mjs (1)
7-7: LGTM! Metadata version bump.Consistent with other version updates in this PR.
components/microsoft_dynamics_365_sales/sources/new-opportunity-created/new-opportunity-created.mjs (1)
1-39: LGTM! Clean implementation for new opportunity creation trigger.The source correctly extends the common base, orders opportunities by creation date, and generates appropriate metadata. The deploy hook appropriately fetches the 10 most recent opportunities for initial state.
components/microsoft_dynamics_365_sales/sources/opportunity-close-date-updated/opportunity-close-date-updated.mjs (2)
32-44: LGTM! Correct delta detection for close date changes.The change detection logic correctly filters for opportunities where
estimatedclosedatehas changed. Note that the conditional at line 37 ensures events are only emitted for opportunities that were previously tracked, avoiding false positives during initial state population.
46-53: LGTM! Metadata generation includes timestamp for uniqueness.The composite ID
${opportunity.opportunityid}${ts}correctly allows multiple update events for the same opportunity over time while maintaining uniqueness.components/microsoft_dynamics_365_sales/sources/opportunity-estimated-value-updated/opportunity-estimated-value-updated.mjs (1)
1-55: LGTM! Consistent implementation pattern for estimated value tracking.This source correctly implements the same delta-detection pattern as the close-date source, tracking changes to
estimatedvalueand only emitting events for previously-seen opportunities.components/microsoft_dynamics_365_sales/sources/opportunity-stage-updated/opportunity-stage-updated.mjs (1)
1-55: LGTM! Consistent implementation for stage change tracking.This source correctly follows the established delta-detection pattern, tracking changes to
stageidwith appropriate state management and metadata generation.components/microsoft_dynamics_365_sales/microsoft_dynamics_365_sales.app.mjs (2)
41-60: LGTM! URL override support enables pagination.The addition of the
urlparameter to_makeRequestmaintains backward compatibility while enabling explicit URL specification for pagination scenarios.
89-94: LGTM! Clean opportunities listing method.The new
listOpportunitiesmethod follows the established pattern of other resource listing methods in this module.
...ales/sources/opportunity-close-probability-updated/opportunity-close-probability-updated.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.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/microsoft_dynamics_365_sales/sources/opportunity-close-date-updated/opportunity-close-date-updated.mjs(1 hunks)components/microsoft_dynamics_365_sales/sources/opportunity-stage-updated/opportunity-stage-updated.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/microsoft_dynamics_365_sales/sources/opportunity-close-date-updated/opportunity-close-date-updated.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/microsoft_dynamics_365_sales/sources/opportunity-stage-updated/opportunity-stage-updated.mjs (1)
components/microsoft_dynamics_365_sales/sources/common/common.mjs (3)
results(53-53)relevantResults(65-65)ts(55-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (5)
components/microsoft_dynamics_365_sales/sources/opportunity-stage-updated/opportunity-stage-updated.mjs (5)
1-1: LGTM!Import statement follows the established pattern for source modules in this component.
3-10: LGTM!Metadata follows the standard pattern: spreads common base, declares appropriate key/name/description, uses semantic versioning (0.0.1 for new source), and sets dedupe to "unique" for event identification.
13-18: LGTM!State persistence helpers follow the expected pattern:
_getStages()safely defaults to an empty object, and_setStages()persists the updated map. The underscore prefix correctly indicates these are internal methods.
19-31: LGTM!Resource function, query arguments, and timestamp field are correctly configured:
listOpportunitiesis the appropriate resource, ordering bymodifiedon descensures the polling logic processes the most recent records first, and the timestamp field is consistent throughout.
46-53: LGTM!Metadata generation correctly constructs a unique ID from
opportunityidand timestamp, and provides a clear summary. The function assumesmodifiedon,opportunityid, andnameare present in the API response, which is reasonable as these are standard fields in the Dynamics 365 opportunity entity.
Resolves #18819
Summary by CodeRabbit
New Features
Chores