New Components - workday #18022
New Components - workday #18022
Conversation
WalkthroughAdds Workday app HTTP helpers, paginated helpers, dynamic propDefinitions, multiple worker- and org-related actions, a polling base, and a "New Worker Created" polling source with sample event. Updates package.json to v0.1.0 and adds @pipedream/platform dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant User/Workflow
participant Action
participant WorkdayApp
participant WorkdayAPI
User/Workflow->>Action: invoke action with props
Action->>WorkdayApp: call method (e.g., getWorker/createJobChange)
WorkdayApp->>WorkdayAPI: HTTP request via axios
WorkdayAPI-->>WorkdayApp: Response
WorkdayApp-->>Action: return data
Action-->>User/Workflow: Result (+ $summary)
sequenceDiagram
participant Source
participant DB
participant WorkdayApp
participant WorkdayAPI
Source->>DB: read previousIds
loop paginate pages
Source->>WorkdayApp: listWorkers($, offset, limit)
WorkdayApp->>WorkdayAPI: GET /workers
WorkdayAPI-->>WorkdayApp: page results
WorkdayApp-->>Source: return workers[]
end
Source->>Source: filter new workers
Source->>DB: update previousIds
Source-->>Source: emit events for new workers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (11)
components/workday/workday.app.mjs (2)
12-23: Enhance dynamic options: support search passthrough for better UX at scalePass the
searchstring to the underlying list APIs to filter server-side and reduce dropdown load for large tenants.Apply (example for workerId; repeat similarly for the other two):
- async options({ page }) { + async options({ page, search }) { const { data } = await this.listWorkers({ params: - { + { limit: DEFAULT_LIMIT, offset: page * DEFAULT_LIMIT, + ...(search ? { search } : {}), }, }); return data?.map((worker) => ({ label: worker.descriptor, value: worker.id, })) || []; },And similarly add
, searchand forward...(search ? { search } : {})underparamsforsupervisoryOrganizationIdandjobChangeReasonIdif their endpoints support it.Also applies to: 29-40, 46-57
68-70: Normalize domain to avoid double protocolIf users enter a domain with protocol,
_baseUrl()can producehttps://https://.... Normalize by stripping any scheme.Apply:
- _baseUrl() { - return `https://${this.$auth.domain}/ccx/api/v1/${this.$auth.tenant_id}`; - }, + _baseUrl() { + const domain = (this.$auth.domain || "").replace(/^https?:\/\//, ""); + return `https://${domain}/ccx/api/v1/${this.$auth.tenant_id}`; + },components/workday/actions/list-worker-payslips/list-worker-payslips.mjs (2)
25-32: Confirm paginate yield shape or flatten defensivelyIf
paginateyields arrays (pages) instead of individual items,data.lengthwill count pages, not payslips. Either confirm the yield shape, or flatten defensively.Proposed defensive flatten within the loop:
- for await (const result of results) { - data.push(result); - } + for await (const result of results) { + if (Array.isArray(result)) data.push(...result); + else data.push(result); + }
39-40: Summary may be misleading if results are paged
data.lengthreflects the number of pushed items. If pages were pushed (arrays), the summary could misreport the count. This is resolved by the defensive flatten above.components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (1)
27-31: Optional: flatten results defensivelySame consideration as other paginated actions—flatten if the generator yields pages.
- for await (const result of results) { - data.push(result); - } + for await (const result of results) { + if (Array.isArray(result)) data.push(...result); + else data.push(result); + }components/workday/actions/search-workers/search-workers.mjs (2)
36-39: Optional: flatten generator results defensivelyIf the paginator yields pages, push spread items to maintain a correct count and return shape.
- for await (const result of results) { - data.push(result); - } + for await (const result of results) { + if (Array.isArray(result)) data.push(...result); + else data.push(result); + }
41-42: Summary count depends on generator yield shapeEnsure the summary reflects item count (not pages). The flatten suggestion above addresses this.
components/workday/actions/create-job-change/create-job-change.mjs (2)
29-34: Fix Markdown strikethrough in descriptionThe tildes around “Manager” render as strikethrough. Remove them to avoid confusing UI text.
moveManagersTeam: { type: "boolean", label: "Move Managers Team", - description: "Indicates whether teams reporting to the ~Manager~ moved with them during the Change Job Event", + description: "Indicates whether teams reporting to the Manager moved with them during the Change Job Event", optional: true, },
36-40: Clarify label for effective date/timeOptional: rename the label to “Effective Date” to better communicate expected value (ISO 8601).
- label: "Effective", + label: "Effective Date",components/workday/sources/new-worker-created/new-worker-created.mjs (2)
20-25: Use source data timestamp if available for more accurate metadataUsing
Date.now()is acceptable, but if the worker payload exposes a created/updated timestamp, prefer it fortsto reflect event time over processing time.Example if a field exists:
- ts: Date.now(), + ts: worker.createdAt ?? Date.now(),
32-41: Potential unbounded growth ofpreviousIds
previousIdsgrows over time and may become large for tenants with many workers. Consider a pruning strategy (e.g., keeping only IDs seen in the last N days) or using an anchor (if API supportsupdatedFrom) to bound storage and fetch time.
📜 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 (12)
components/workday/actions/change-business-title/change-business-title.mjs(1 hunks)components/workday/actions/create-job-change/create-job-change.mjs(1 hunks)components/workday/actions/get-worker/get-worker.mjs(1 hunks)components/workday/actions/list-organization-types/list-organization-types.mjs(1 hunks)components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs(1 hunks)components/workday/actions/list-worker-payslips/list-worker-payslips.mjs(1 hunks)components/workday/actions/search-workers/search-workers.mjs(1 hunks)components/workday/package.json(2 hunks)components/workday/sources/common/base-polling.mjs(1 hunks)components/workday/sources/new-worker-created/new-worker-created.mjs(1 hunks)components/workday/sources/new-worker-created/test-event.mjs(1 hunks)components/workday/workday.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/workday/package.json
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/workday/sources/new-worker-created/new-worker-created.mjs
🧬 Code Graph Analysis (8)
components/workday/actions/search-workers/search-workers.mjs (4)
components/workday/actions/list-organization-types/list-organization-types.mjs (1)
results(19-25)components/workday/actions/list-worker-payslips/list-worker-payslips.mjs (1)
results(25-32)components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (1)
results(19-25)components/workday/sources/new-worker-created/new-worker-created.mjs (1)
results(28-30)
components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (4)
components/workday/actions/list-organization-types/list-organization-types.mjs (1)
results(19-25)components/workday/actions/search-workers/search-workers.mjs (1)
results(25-34)components/workday/actions/list-worker-payslips/list-worker-payslips.mjs (1)
results(25-32)components/workday/sources/new-worker-created/new-worker-created.mjs (1)
results(28-30)
components/workday/actions/list-organization-types/list-organization-types.mjs (4)
components/workday/actions/search-workers/search-workers.mjs (1)
results(25-34)components/workday/actions/list-worker-payslips/list-worker-payslips.mjs (1)
results(25-32)components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (1)
results(19-25)components/workday/sources/new-worker-created/new-worker-created.mjs (1)
results(28-30)
components/workday/actions/change-business-title/change-business-title.mjs (2)
components/workday/actions/create-job-change/create-job-change.mjs (1)
response(43-56)components/workday/actions/get-worker/get-worker.mjs (1)
response(19-22)
components/workday/actions/get-worker/get-worker.mjs (2)
components/workday/actions/create-job-change/create-job-change.mjs (1)
response(43-56)components/workday/actions/change-business-title/change-business-title.mjs (1)
response(24-30)
components/workday/actions/create-job-change/create-job-change.mjs (2)
components/workday/actions/change-business-title/change-business-title.mjs (1)
response(24-30)components/workday/actions/get-worker/get-worker.mjs (1)
response(19-22)
components/workday/workday.app.mjs (4)
components/workday/actions/list-organization-types/list-organization-types.mjs (1)
data(27-27)components/workday/actions/search-workers/search-workers.mjs (1)
data(36-36)components/workday/actions/list-worker-payslips/list-worker-payslips.mjs (1)
data(34-34)components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (1)
data(27-27)
components/workday/sources/new-worker-created/new-worker-created.mjs (4)
components/workday/actions/list-organization-types/list-organization-types.mjs (1)
results(19-25)components/workday/actions/search-workers/search-workers.mjs (1)
results(25-34)components/workday/actions/list-worker-payslips/list-worker-payslips.mjs (1)
results(25-32)components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (1)
results(19-25)
⏰ 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: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (11)
components/workday/package.json (2)
3-3: Version bump looks goodPackage version bumped to 0.1.0 to reflect new features.
15-17: @pipedream/platform dependency is up to dateThe pinned version ^3.1.0 matches the latest release on npm (3.1.0), so no upgrade is necessary.
components/workday/actions/list-organization-types/list-organization-types.mjs (1)
18-34: LGTM: follows the new paginate patternAction cleanly pages results, respects
maxResults, and exports a useful summary. Will work once thepaginatefix lands.components/workday/actions/get-worker/get-worker.mjs (1)
18-25: LGTM: straightforward fetch with summaryUses propDefinition for the worker ID and calls the app method directly. Errors will bubble to the platform as expected.
components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (1)
19-25: LGTM: correct usage of paginate with args and maxResultsThe pagination pattern and arg passing look consistent with other actions.
components/workday/actions/change-business-title/change-business-title.mjs (1)
31-33: LGTM: clear summary and return behaviorThe success summary and returning the API response align with action patterns in this package.
components/workday/actions/search-workers/search-workers.mjs (1)
24-34: paginate yields individual items – no changes needed
Verified that incomponents/workday/workday.app.mjs, theasync *paginateimplementation usesyield item(lines 159–160), so it emits each record one by one rather than an array. The existing loops and summaries will operate correctly as written.components/workday/actions/create-job-change/create-job-change.mjs (1)
57-58: LGTM: clear summary and returnThe success message and returning the API response is consistent with other actions.
components/workday/sources/common/base-polling.mjs (1)
1-15: Base shape and timer defaults look goodThe base comp cleanly wires
workday,db, and a sensible default polling interval from the platform.components/workday/sources/new-worker-created/new-worker-created.mjs (2)
32-41: Dedupe strategy matches team learning (mark all seen, emit up to limit)You update
previousIdsfor both emitted and non-emitted workers, then slice tolimit. This aligns with the retrieved learning to avoid retroactive floods on first deploy and only emit events going forward.Also applies to: 49-51
21-24: Confirmed:worker.idis Workday’s canonical, stable identifier
All components undercomponents/workday—including the app layer inworkday.app.mjs(wherevalue: worker.idis used and API paths are built with/workers/${workerId})—consistently useworker.idas the primary ID. You can safely use it for deduplication.
components/workday/actions/change-business-title/change-business-title.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/workday/workday.app.mjs (1)
140-168: Fix broken pagination: lostthisbinding, overrides callerlimit/offset, and fragile hasMore logic
- Calling
fn(args)loses the method’sthiscontext (e.g.,listWorkers), causing runtime errors.- You overwrite caller-provided
params.limitandparams.offset.- You assume
totalis returned at the top level. With@pipedream/platformaxios,totaltypically lives underres.data.total(if present), and many endpoints won’t return it at all.- You increment
offsetbyargs.params.limit, which could be mutated; store a local copy.- Current logic will stop after the first page if
totalis absent (sincecount < undefinedis false), and will also break even when more data exists.Apply this refactor:
- async *paginate({ - fn, args = {}, max, - }) { - args = { - ...args, - params: { - ...args?.params, - limit: DEFAULT_LIMIT, - offset: 0, - }, - }; - let hasMore, count = 0; - do { - const { - data, total, - } = await fn(args); - if (!data?.length) { - return; - } - for (const item of data) { - yield item; - count++; - if (max && count >= max) { - return; - } - } - hasMore = count < total; - args.params.offset += args.params.limit; - } while (hasMore); - }, + async *paginate({ fn, args = {}, max }) { + args = { + ...args, + params: { + ...args?.params, + limit: args?.params?.limit ?? DEFAULT_LIMIT, + offset: args?.params?.offset ?? 0, + }, + }; + let count = 0; + while (true) { + // Preserve method binding for app methods like listWorkers + const res = await fn.call(this, args); + const pageData = Array.isArray(res?.data) + ? res.data + : Array.isArray(res) + ? res + : []; + if (!pageData.length) break; + for (const item of pageData) { + yield item; + count++; + if (max && count >= max) return; + } + const total = typeof res?.data?.total === "number" + ? res.data.total + : typeof res?.total === "number" + ? res.total + : undefined; + const limit = args.params.limit; + const hasMore = total !== undefined + ? count < total + : pageData.length === limit; + if (!hasMore) break; + args.params.offset += limit; + } + },
🧹 Nitpick comments (2)
components/workday/workday.app.mjs (2)
12-22: Defensive default forpagein dynamic options to avoidNaNoffsetsIf
pageis ever undefined,page * DEFAULT_LIMITbecomesNaN, breaking pagination in the dropdowns. Defaultpageto 0.- async options({ page }) { + async options({ page = 0 }) { const { data } = await this.listWorkers({ params:Apply the same change to supervisoryOrganizationId and jobChangeReasonId option loaders.
Also applies to: 29-40, 46-57
71-81: Allow caller headers while ensuring Authorization remains setCurrently,
...optscomes afterheaders, so callers can inadvertently overwrite the Authorization header entirely. If that’s not desired, merge headers explicitly to preserve Authorization and allow augmentation:- return axios($, { - url: `${this._baseUrl()}${path}`, - headers: { - Authorization: `Bearer ${this.$auth.oauth_access_token}`, - }, - ...opts, - }); + const { headers: userHeaders, ...rest } = opts ?? {}; + return axios($, { + url: `${this._baseUrl()}${path}`, + headers: { + Authorization: `Bearer ${this.$auth.oauth_access_token}`, + ...(userHeaders || {}), + }, + ...rest, + });If overriding Authorization is needed in some calls, keep the current behavior. Otherwise, merging prevents accidental 401s.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/workday/sources/common/base-polling.mjs(1 hunks)components/workday/workday.app.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/workday/sources/common/base-polling.mjs
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (2)
components/workday/workday.app.mjs (2)
114-121: Verify path casing for payslips endpointWorkday endpoints can be case-sensitive. Confirm that
/paySlipsis the correct path segment for the tenant API; some implementations use lowercase (/payslips).Want me to check other Workday actions/sources for consistent usage?
59-65: maxResults propDefinition is consumed by Workday actions
ThemaxResultsdefinition incomponents/workday/workday.app.mjsis referenced by the following action/trigger modules—they all passthis.maxResultsinto the app’spaginatehelper, so it should remain as-is:
- components/workday/actions/list-worker-payslips/list-worker-payslips.mjs (max: this.maxResults)
- components/workday/actions/search-workers/search-workers.mjs (max: this.maxResults)
- components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs (max: this.maxResults)
- components/workday/actions/list-organization-types/list-organization-types.mjs (max: this.maxResults)
No changes required here.
luancazarine
left a comment
luancazarine
left a comment
There was a problem hiding this comment.
Hi @michelle0927, LGTM! Ready for QA!
Resolves #18012
Summary by CodeRabbit