Workday API #18842
Workday API #18842
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
WalkthroughAdds ~80 new Workday action modules, a JSON parsing utility, and a major expansion of the Workday app (many new propDefinitions and ~40+ list/get/create/update/delete methods); includes selective action version bumps and a package version increment. (46 words) Changes
Sequence Diagram(s)%%{init:{"themeVariables":{"actorBorder":"#2b6cb0","actorBg":"#ebf8ff","noteBg":"#fff7ed","noteBorder":"#f6ad55"}}}%%
sequenceDiagram
participant User
participant Action as Action Module
participant Utils as utils.parseJsonInput
participant App as workday.app
participant Workday as Workday API
User->>Action: Trigger action with props
alt JSON inputs present
Action->>Utils: parseJsonInput(topics/lessons)
end
Action->>Action: Validate props (ids, dates, arrays)
alt validation fails
Action-->>User: throw ConfigurationError
else
Action->>Action: Build data payload (include optional fields)
Action->>App: this.workday.<method>({ id?, data?, $ })
App->>Workday: HTTP request
Workday-->>App: Response
App-->>Action: Response
Action->>Action: $.export("$summary", "...")
Action-->>User: Return response + summary
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/workday/workday.app.mjs (1)
1003-1032: paginate() stops after first page — wrong destructure and hasMore logic.
axiosreturns{ data: payload };totalisn’t at top-level, sototalisundefined, makinghasMorefalse.async *paginate({ fn, args = {}, max, }) { @@ - do { - const { - data, total, - } = await fn(args); + do { + const resp = await fn(args); + const payload = resp?.data ?? resp ?? {}; + const { data, total } = payload; if (!data?.length) { return; } @@ - hasMore = count < total; - args.params.offset += args.params.limit; + const pageSize = args.params.limit ?? data.length; + // Prefer API-provided total; otherwise infer from page size + hasMore = typeof total === "number" + ? count < total + : data.length === pageSize; + args.params.offset += pageSize; } while (hasMore);
🧹 Nitpick comments (34)
components/workday/actions/get-interview-details/get-interview-details.mjs (1)
15-19: Consider using propDefinition for consistency.The interviewId is defined inline, whereas other similar actions in this PR use
propDefinition(e.g.,contentIdin get-content-details.mjs,mentorshipIdin get-mentorship-details.mjs). Using propDefinition would improve consistency and reduce duplication if the prop definition is reusable.If a propDefinition exists in workday.app.mjs, apply this pattern:
- interviewId: { - type: "string", - label: "Interview ID", - description: "The unique identifier for the interview.", - }, + interviewId: { + propDefinition: [ + workday, + "interviewId", + ], + },components/workday/actions/create-tax-rate/create-tax-rate.mjs (1)
64-72: Consider simplifying validation logic for readability.The validation checks are functional but could be more readable. Line 64 combines multiple conditions in a single expression.
Consider this more readable approach:
- if (this.stateInstance && (typeof this.stateInstance !== "object" || !this.stateInstance.id || !this.stateInstance.id.trim())) { + if (!this.stateInstance || typeof this.stateInstance !== "object") { + throw new ConfigurationError("stateInstance must be an object."); + } + if (!this.stateInstance.id || !this.stateInstance.id.trim()) { throw new ConfigurationError("stateInstance is required to have a non-empty id property."); }components/workday/actions/list-job-profiles/list-job-profiles.mjs (1)
1-2: Consider adding blank line for consistency.Several other action files in this PR include a blank line after the import statement. Adding one here would improve consistency across the codebase.
Apply this diff:
import workday from "../../workday.app.mjs"; + export default {components/workday/actions/get-prospect-details/get-prospect-details.mjs (1)
1-2: Consider adding blank line for consistency.Most action files in this PR include a blank line after the import statement for better readability.
Apply this diff:
import workday from "../../workday.app.mjs"; + export default {components/workday/actions/list-jobs/list-jobs.mjs (1)
1-2: Consider adding blank line for consistency.Adding a blank line after the import would align with the style used in most other action files.
Apply this diff:
import workday from "../../workday.app.mjs"; + export default {components/workday/actions/create-home-contact-information-change/create-home-contact-information-change.mjs (1)
24-27: Consider removing redundant validation.If
workerIdis marked as required in the propDefinition, the Pipedream platform automatically validates its presence. This manual check creates inconsistency with other actions in the PR (e.g., get-job-pay-group.mjs, get-worker-anytime-feedback-events.mjs) that rely on propDefinition validation.Apply this diff to align with the established pattern:
- if (!this.workerId || !this.workerId.trim()) { - throw new ConfigurationError("Worker ID is required."); - } const response = await this.workday.createHomeContactInformationChange({If the propDefinition doesn't mark workerId as required, then keep this validation but add the
required: trueattribute to the propDefinition for consistency.components/workday/sources/common/utils.mjs (1)
4-10: Consider handling JSON parse errors more gracefully.The function silently catches JSON parse errors and returns the original string. While this prevents exceptions, it may hide malformed JSON that the caller expects to be parsed.
Consider logging parse errors or adding an optional parameter to control error handling behavior, especially for debugging purposes.
components/workday/actions/get-work-contact-information-change/get-work-contact-information-change.mjs (2)
15-19: Consider using propDefinition for consistency.While this prop uses a plain string type, other similar actions (e.g.,
get-succession-plan-details.mjs) usepropDefinitionto wire the ID prop. UsingpropDefinitionwould provide better consistency across the codebase and potentially enable dynamic options/validation from the Workday app.
21-28: Consider validating the required ID parameter.The action doesn't validate that
workContactInformationChangeIdis provided before making the API call. While the Workday API will likely return an error, failing early with a clear message improves the user experience.async run({ $ }) { + if (!this.workContactInformationChangeId || !this.workContactInformationChangeId.trim()) { + throw new ConfigurationError("Work Contact Information Change ID is required."); + } const response = await this.workday.getWorkContactInformationChange({Note: You'll need to import
ConfigurationErrorfrom@pipedream/platform(seeupdate-payroll-input.mjsfor reference).components/workday/actions/list-work-contact-information-changes/list-work-contact-information-changes.mjs (1)
20-20: Minor inconsistency in summary message.The summary uses "Found" while other list actions in this PR use "Fetched" (e.g.,
list-feedback-badges.mjs,list-pay-group-details.mjs). Consider using "Fetched" for consistency across all list actions.- $.export("$summary", `Found ${response?.data?.length || 0} changes`); + $.export("$summary", `Fetched ${response?.data?.length || 0} work contact information changes`);components/workday/actions/get-succession-plan-details/get-succession-plan-details.mjs (1)
23-30: Consider validating the required ID parameter.While using
propDefinitionis good practice, the action doesn't validate thatsuccessionPlanIdis provided before making the API call. Adding validation would improve the user experience by failing early with a clear message.async run({ $ }) { + if (!this.successionPlanId || !this.successionPlanId.trim()) { + throw new ConfigurationError("Succession Plan ID is required."); + } const response = await this.workday.getSuccessionPlan({Note: You'll need to import
ConfigurationErrorfrom@pipedream/platform.components/workday/actions/get-org-assignment-change-details/get-org-assignment-change-details.mjs (1)
24-31: Consider validating the required ID parameter.The action doesn't validate that
organizationAssignmentChangeIdis provided before making the API call. Adding validation would improve the user experience by failing early with a clear message.async run({ $ }) { + if (!this.organizationAssignmentChangeId || !this.organizationAssignmentChangeId.trim()) { + throw new ConfigurationError("Organization Assignment Change ID is required."); + } const response = await this.workday.getOrganizationAssignmentChange({Note: You'll need to import
ConfigurationErrorfrom@pipedream/platform(seeupdate-payroll-input.mjsfor reference).components/workday/actions/validate-phone-number/validate-phone-number.mjs (1)
16-20: Consider adding input validation for the required field.The
completePhoneNumberfield is required but lacks validation to ensure it's provided and non-empty. While Pipedream may enforce required fields at the platform level, explicit validation provides clearer error messages and prevents API calls with invalid data.Consider adding validation in the
runfunction:async run({ $ }) { + if (!this.completePhoneNumber || !this.completePhoneNumber.trim()) { + throw new ConfigurationError("Complete Phone Number is required and cannot be empty."); + } const response = await this.workday.validatePhoneNumber({components/workday/actions/close-mentorship/close-mentorship.mjs (1)
52-77: Consider adding date logic validation.While not critical for API functionality, validating that
endDateis afterstartDatewould improve user experience by catching logical errors before making the API call.You could add after the existing validation:
+ if (new Date(this.endDate) <= new Date(this.startDate)) { + throw new ConfigurationError("End Date must be after Start Date."); + } + const data = {components/workday/actions/create-mentorship-for-me/create-mentorship-for-me.mjs (1)
56-86: Optional: Consider adding date logic validation.Similar to the suggestion for close-mentorship, you could validate that
endDateis afterstartDateto catch logical errors early.if (this.mentorType !== undefined) { if (typeof this.mentorType !== "object" || !this.mentorType.id || !this.mentorType.id.trim()) { throw new ConfigurationError("If provided, mentorType must be an object with a non-empty id property."); } } + if (new Date(this.endDate) <= new Date(this.startDate)) { + throw new ConfigurationError("End Date must be after Start Date."); + } const data = {components/workday/actions/create-work-contact-information-change/create-work-contact-information-change.mjs (1)
24-27: Consider removing redundant validation.The manual validation on lines 25-27 checks if
workerIdis empty, butworkerIdis already a required prop (not markedoptional: trueon line 17-22), so the Pipedream platform will enforce this before the action runs.Apply this diff to remove the redundant validation:
async run({ $ }) { - if (!this.workerId || !this.workerId.trim()) { - throw new ConfigurationError("Worker ID is required."); - } const response = await this.workday.createWorkContactInformationChange({If there's a specific reason for the manual validation (e.g., checking for whitespace-only values), consider adding a comment explaining why.
components/workday/actions/create-payroll-input/create-payroll-input.mjs (1)
33-42: Consider validating for empty string IDs for consistency.The validation checks for the existence of
worker.idandpayComponent.idbut doesn't verify they're non-empty strings. Other similar actions in this PR (e.g., create-mentorship-for-worker at lines 64-65) include.trim()checks to ensure IDs aren't empty strings.Apply this diff for consistency:
-if (!this.worker || typeof this.worker !== "object" || !this.worker.id) { +if (!this.worker || typeof this.worker !== "object" || !this.worker.id || !this.worker.id.trim()) { throw new ConfigurationError("worker (object with non-empty id) is required."); } -if (!this.payComponent || typeof this.payComponent !== "object" || !this.payComponent.id) { +if (!this.payComponent || typeof this.payComponent !== "object" || !this.payComponent.id || !this.payComponent.id.trim()) { throw new ConfigurationError("payComponent (object with non-empty id) is required."); }components/workday/actions/create-digital-course/create-digital-course.mjs (3)
18-22: Useobject[]for structured arrays; avoid forcing JSON strings.
Typingtopicsandlessonsasstring[]then parsing JSON harms UX and validation. Preferobject[]and accept objects directly; keep parsing only as a fallback.- topics: { - type: "string[]", + topics: { + type: "object[]", + optional: false, label: "Topics", - description: "The topics of the learning course event. Example: `[ { \"descriptor\": \"Leadership\", \"id\": \"topic-id-1\" } ]`", + description: "Array of topic objects. Example: [{ \"descriptor\": \"Leadership\", \"id\": \"topic-id-1\" }]", }, @@ - lessons: { - type: "string[]", + lessons: { + type: "object[]", + optional: false, label: "Lessons", - description: "The course lessons of the learning course event. Example: `[ { \"title\": \"Lesson 1\", \"type\": { \"id\": \"type-id\" }, \"order\": 1, \"url\": \"https://...\", \"required\": true } ]`", + description: "Array of lesson objects. Example: [{ \"title\": \"Lesson 1\", \"type\": { \"id\": \"type-id\" }, \"order\": 1, \"url\": \"https://...\", \"required\": true }]", },Also mark other required fields as non-optional for clearer UI.
- title: { + title: { + optional: false, @@ - availabilityStatus: { + availabilityStatus: { + optional: false, @@ - description: { + description: { + optional: false,Also applies to: 33-37
61-63: Tighten validation fororderto integer ≥ 1 and validate URL format.
UseNumber.isInteger(l.order)and a basic URL check to prevent API errors earlier.- if (!l.title || !l.type?.id || typeof l.order !== "number" || !l.url) { - throw new ConfigurationError("Each lesson must include `title`, `type` (object with id), `order` (integer), and `url`."); + const validOrder = Number.isInteger(l.order) && l.order >= 1; + let validUrl = true; + try { new URL(l.url); } catch { validUrl = false; } + if (!l.title || !l.type?.id || !validOrder || !validUrl) { + throw new ConfigurationError("Each lesson must include `title`, `type` (object with id), `order` (integer ≥ 1), and a valid `url`."); }
84-90: Improve summary with identifier when available.
If API returns an ID, include it for traceability.- $.export("$summary", "Digital course created"); + $.export("$summary", `Digital course created${response?.id ? ` (ID: ${response.id})` : ""}`);components/workday/actions/list-payroll-inputs/list-payroll-inputs.mjs (2)
21-22: Make summary resilient to response shape.
Guard for array at top-level or underitems.- $.export("$summary", `Fetched ${response?.data?.length || 0} payroll inputs`); + const count = Array.isArray(response?.data) + ? response.data.length + : Array.isArray(response?.items) + ? response.items.length + : Array.isArray(response) ? response.length : 0; + $.export("$summary", `Fetched ${count} payroll inputs`);
13-16: Add optional pagination props to prevent large payloads.Expose optional
limitandoffsetprops (the API uses offset, not cursor) and pass them viaparamstolistPayrollInputs:props: { workday, limit: { type: "integer", label: "Limit", description: "Maximum number of records to return", optional: true, }, offset: { type: "integer", label: "Offset", description: "Number of records to skip", optional: true, }, }, async run({ $ }) { const response = await this.workday.listPayrollInputs({ $, ...(this.limit || this.offset ? { params: { limit: this.limit, offset: this.offset } } : {}), }); // ... }This pattern already exists in the app file's
payrollInputIdoptions (workday.app.mjs:318-323).components/workday/actions/get-job-profile-details/get-job-profile-details.mjs (1)
3-5: Align naming/summary with other “Get …” actions.
For consistency, consider dropping “Details” and using the “for ID …” summary format.- name: "Get Job Profile Details", + name: "Get Job Profile", @@ - $.export("$summary", `Fetched job profile ${this.jobProfileId}`); + $.export("$summary", `Fetched job profile for ID ${this.jobProfileId}`);Also applies to: 27-28
components/workday/actions/create-prospect/create-prospect.mjs (1)
17-21: Markcandidateas required for clearer UX.
Reflects server-side requirement in the UI.candidate: { type: "object", + optional: false, label: "Candidate", description: "The candidate object. Must include `name.country.id`. Example: { \"name\": { \"country\": { \"id\": \"USA\" } } }", },components/workday/actions/create-org-assignment-change/create-org-assignment-change.mjs (3)
47-52: Hardenposition.idtypingEnsure
idis a string before calling.trim()to prevent “trim is not a function” on non-strings.- if (!this.position || typeof this.position !== "object" || !this.position.id || !this.position.id.trim()) { + if ( + !this.position + || typeof this.position !== "object" + || typeof this.position.id !== "string" + || !this.position.id.trim() + ) { throw new ConfigurationError("Position object is required, with a non-empty id property."); }
64-71: Align inclusion condition with validation semanticsAfter switching validation to
!= null, mirror that here sonullisn’t accidentally included later.- if (this.massActionWorksheet) data.massActionWorksheet = this.massActionWorksheet; - if (this.massActionHeader) data.massActionHeader = this.massActionHeader; + if (this.massActionWorksheet != null) data.massActionWorksheet = this.massActionWorksheet; + if (this.massActionHeader != null) data.massActionHeader = this.massActionHeader;
5-6: Naming consistency (optional)File path uses “org” while the
keyuses “organization.” Consider aligning to one form to ease discovery and debugging.components/workday/actions/create-succession-plan/create-succession-plan.mjs (1)
41-54: Strengthen id typing in validationsGuard
.trim()with a string check for all required object refs.- if ( - !this.position || typeof this.position !== "object" || !this.position.id || !this.position.id.trim() - ) { + if (!this.position || typeof this.position !== "object" + || typeof this.position.id !== "string" || !this.position.id.trim()) { throw new ConfigurationError("Position is required and must be an object with a non-empty id property."); } - if ( - !this.supervisoryOrg || typeof this.supervisoryOrg !== "object" || !this.supervisoryOrg.id || !this.supervisoryOrg.id.trim() - ) { + if (!this.supervisoryOrg || typeof this.supervisoryOrg !== "object" + || typeof this.supervisoryOrg.id !== "string" || !this.supervisoryOrg.id.trim()) { throw new ConfigurationError("Supervisory Organization is required and must be an object with a non-empty id property."); } - if ( - !this.successionPlan || typeof this.successionPlan !== "object" || !this.successionPlan.id || !this.successionPlan.id.trim() - ) { + if (!this.successionPlan || typeof this.successionPlan !== "object" + || typeof this.successionPlan.id !== "string" || !this.successionPlan.id.trim()) { throw new ConfigurationError("Succession Plan is required and must be an object with a non-empty id property."); }components/workday/actions/create-distribution-request/create-distribution-request.mjs (1)
51-62: Type-checkidstrings in validationsPrevent
.trim()on non-strings and keep optional fields null-safe.- if (!this.builder || typeof this.builder !== "object" || !this.builder.id || !this.builder.id.trim()) { + if (!this.builder || typeof this.builder !== "object" || typeof this.builder.id !== "string" || !this.builder.id.trim()) { throw new ConfigurationError("Builder is required and must be an object with a non-empty id property."); } - if (!this.category || typeof this.category !== "object" || !this.category.id || !this.category.id.trim()) { + if (!this.category || typeof this.category !== "object" || typeof this.category.id !== "string" || !this.category.id.trim()) { throw new ConfigurationError("Category is required and must be an object with a non-empty id property."); } - if (this.discoverableBuilder && (typeof this.discoverableBuilder !== "object" || !this.discoverableBuilder.id || !this.discoverableBuilder.id.trim())) { + if (this.discoverableBuilder && (typeof this.discoverableBuilder !== "object" + || typeof this.discoverableBuilder.id !== "string" || !this.discoverableBuilder.id.trim())) { throw new ConfigurationError("Discoverable Builder (if provided) must be an object with a non-empty id property."); } - if (this.relatedRole && (typeof this.relatedRole !== "object" || !this.relatedRole.id || !this.relatedRole.id.trim())) { + if (this.relatedRole && (typeof this.relatedRole !== "object" + || typeof this.relatedRole.id !== "string" || !this.relatedRole.id.trim())) { throw new ConfigurationError("Related Role (if provided) must be an object with a non-empty id property."); }components/workday/actions/update-work-contact-information-change/update-work-contact-information-change.mjs (1)
25-31: Accept object or JSON string foralternateWorkLocationfor consistency with other actions.Other actions use
parseJsonInput; support both object and stringified JSON.+import { parseJsonInput } from "../../sources/common/utils.mjs"; @@ - alternateWorkLocation: { - type: "object", + alternateWorkLocation: { + type: "string", label: "Alternate Work Location", - description: "Object for alternate work location. Must include at least `id`. Example: `{ id: \"...\", descriptor: \"...\"}`", + description: "Alternate work location as object or JSON. Must include `id`. Example: `{ \"id\": \"...\", \"descriptor\": \"...\"}`", optional: true, }, @@ - if (this.alternateWorkLocation) { + if (this.alternateWorkLocation) { + const alt = parseJsonInput(this.alternateWorkLocation); if ( - typeof this.alternateWorkLocation !== "object" - || !this.alternateWorkLocation.id - || typeof this.alternateWorkLocation.id !== "string" - || !this.alternateWorkLocation.id.length + typeof alt !== "object" + || !alt?.id + || typeof alt.id !== "string" + || !alt.id.trim().length ) { throw new ConfigurationError("alternateWorkLocation must include a non-empty 'id' property."); } - data.alternateWorkLocation = this.alternateWorkLocation; + data.alternateWorkLocation = alt; }If you prefer to keep the UI as an object prop, still parse and trim
id. Based on patterns in other Workday actions. [Based on learnings]Also applies to: 46-56
components/workday/workday.app.mjs (4)
395-405: Harden requests: set headers and a sane timeout.Add JSON Accept/Content-Type and a default timeout to improve reliability.
_makeRequest({ $ = this, path, ...opts }) { return axios($, { - url: `${this._baseUrl()}${path}`, - headers: { - Authorization: `Bearer ${this.$auth.oauth_access_token}`, - }, + url: `${this._baseUrl()}${path}`, + headers: { + Authorization: `Bearer ${this.$auth.oauth_access_token}`, + Accept: "application/json", + "Content-Type": "application/json", + }, + timeout: 30000, ...opts, }); },
81-85: Use DEFAULT_LIMIT consistently for option paging.Keep paging uniform across props.
- params: { - limit: 50, - offset: page * 50, - }, + params: { + limit: DEFAULT_LIMIT, + offset: page * DEFAULT_LIMIT, + },
505-512: Binary responses: ensure callers setresponseType.
/people/{id}/photoslikely returns binary. Confirm downstream actions pass{ responseType: "arraybuffer" }viaopts, or default it here.
59-75: Option mappers: minor resiliency tweak.Where possible, prefer
item.descriptor || item.idfor labels (with fallback toString(item)) across all option loaders for consistency.Also applies to: 76-92, 93-109, 110-126, 127-143, 144-160
| startDate: { | ||
| type: "string", | ||
| label: "Start Date", | ||
| description: "Start date for the mentorship (ISO 8601). Example: `2025-10-18T07:00:00.000Z`", | ||
| }, | ||
| endDate: { | ||
| type: "string", | ||
| label: "End Date", | ||
| description: "End date for the mentorship (ISO 8601). Example: `2025-10-18T07:00:00.000Z`", | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add validation for required date fields.
The startDate and endDate fields are required but lack validation. This is inconsistent with the validation provided for closeMentorshipReason (lines 53-60). Without validation, the action could fail with unclear errors if these fields are missing or empty.
Apply this diff to add validation:
async run({ $ }) {
+ if (!this.startDate || !this.startDate.trim()) {
+ throw new ConfigurationError("Start Date is required and cannot be empty.");
+ }
+ if (!this.endDate || !this.endDate.trim()) {
+ throw new ConfigurationError("End Date is required and cannot be empty.");
+ }
if (
!this.closeMentorshipReason ||🤖 Prompt for AI Agents
In components/workday/actions/close-mentorship/close-mentorship.mjs around lines
30 to 39, the startDate and endDate schema entries are missing validation; make
them required and add a validate function like closeMentorshipReason has that
ensures the value is present and non-empty and also checks the value is a valid
ISO 8601 date (e.g., attempt Date.parse(value) and reject if NaN). Update both
startDate and endDate to include required: true and a validate handler that
returns a descriptive error string when empty or not a valid ISO date.
| if (!this.availabilityStatus || typeof this.availabilityStatus !== "object" || !this.availabilityStatus.id) { | ||
| throw new ConfigurationError("`availabilityStatus` is required and must be an object with a non-empty 'id'."); | ||
| } |
There was a problem hiding this comment.
Ensure availabilityStatus.id is a non-empty string.
Avoid sending non-string/blank IDs.
- if (!this.availabilityStatus || typeof this.availabilityStatus !== "object" || !this.availabilityStatus.id) {
+ if (!this.availabilityStatus || typeof this.availabilityStatus !== "object"
+ || typeof this.availabilityStatus.id !== "string"
+ || !this.availabilityStatus.id.trim()) {
throw new ConfigurationError("`availabilityStatus` is required and must be an object with a non-empty 'id'.");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!this.availabilityStatus || typeof this.availabilityStatus !== "object" || !this.availabilityStatus.id) { | |
| throw new ConfigurationError("`availabilityStatus` is required and must be an object with a non-empty 'id'."); | |
| } | |
| if (!this.availabilityStatus || typeof this.availabilityStatus !== "object" | |
| || typeof this.availabilityStatus.id !== "string" | |
| || !this.availabilityStatus.id.trim()) { | |
| throw new ConfigurationError("`availabilityStatus` is required and must be an object with a non-empty 'id'."); | |
| } |
🤖 Prompt for AI Agents
In components/workday/actions/create-digital-course/create-digital-course.mjs
around lines 69 to 71, the validation currently only checks that
availabilityStatus.id exists but does not ensure it's a non-empty string; update
the guard to verify typeof this.availabilityStatus.id === "string" and that
this.availabilityStatus.id.trim().length > 0, and throw the same
ConfigurationError if the check fails so blank or non-string IDs are rejected
before proceeding.
| discoverableBuilder: { | ||
| type: "object", | ||
| label: "Discoverable Builder", | ||
| description: "A discoverable journey builder object. Example: `{ \"id\": \"00000000000000000000000000000000\" }`", | ||
| }, |
There was a problem hiding this comment.
Props marked required but treated optional in code
discoverableBuilder and relatedRole are validated/used only if provided, but the props lack optional: true. This will force users to supply them in the UI.
discoverableBuilder: {
type: "object",
label: "Discoverable Builder",
description: "A discoverable journey builder object. Example: `{ \"id\": \"00000000000000000000000000000000\" }`",
+ optional: true,
},
@@
relatedRole: {
type: "object",
label: "Related Role",
description: "Related role object. Example: `{ \"id\": \"00000000000000000000000000000000\" }`",
+ optional: true,
},Also applies to: 38-42
🤖 Prompt for AI Agents
In
components/workday/actions/create-distribution-request/create-distribution-request.mjs
around lines 27-31 and 38-42, the properties discoverableBuilder and relatedRole
are currently defined without optional: true but the code only validates/uses
them when provided; to fix this add optional: true to both property definitions
so the UI won't force users to supply them and the schema matches runtime
behavior.
| mentorType: { | ||
| type: "object", | ||
| label: "Mentor Type", | ||
| description: "Object with at least an `id` property. Example payload: `{\"id\": \"00000000000000000000000000000000\"}`", | ||
| }, |
There was a problem hiding this comment.
Mark the mentorType prop as optional.
The prop definition doesn't include optional: true, but the validation logic (lines 70-74) treats it as optional. This inconsistency could confuse users and cause configuration errors.
Apply this diff:
mentorType: {
type: "object",
label: "Mentor Type",
description: "Object with at least an `id` property. Example payload: `{\"id\": \"00000000000000000000000000000000\"}`",
+ optional: true,
},🤖 Prompt for AI Agents
In
components/workday/actions/create-mentorship-for-worker/create-mentorship-for-worker.mjs
around lines 43 to 47, the mentorType property is declared but not marked
optional even though runtime validation treats it as optional; add "optional:
true" to the mentorType prop definition so schema matches validation logic and
update the label/description only if necessary to reflect that it can be
omitted.
| const data = { | ||
| endDate: this.endDate, | ||
| startDate: this.startDate, | ||
| purpose: this.purpose, | ||
| mentor: this.mentor, | ||
| mentee: this.mentee, | ||
| mentorType: this.mentorType, | ||
| }; | ||
| if (this.comment) data.comment = this.comment; | ||
| if (this.descriptor) data.descriptor = this.descriptor; |
There was a problem hiding this comment.
Conditionally add mentorType to the payload.
Line 82 unconditionally includes mentorType in the data object, which will add mentorType: undefined to the API payload when the field is not provided. This could cause API errors or unexpected behavior.
Apply this diff to conditionally include mentorType like comment and descriptor:
const data = {
endDate: this.endDate,
startDate: this.startDate,
purpose: this.purpose,
mentor: this.mentor,
mentee: this.mentee,
- mentorType: this.mentorType,
};
+if (this.mentorType) data.mentorType = this.mentorType;
if (this.comment) data.comment = this.comment;
if (this.descriptor) data.descriptor = this.descriptor;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const data = { | |
| endDate: this.endDate, | |
| startDate: this.startDate, | |
| purpose: this.purpose, | |
| mentor: this.mentor, | |
| mentee: this.mentee, | |
| mentorType: this.mentorType, | |
| }; | |
| if (this.comment) data.comment = this.comment; | |
| if (this.descriptor) data.descriptor = this.descriptor; | |
| const data = { | |
| endDate: this.endDate, | |
| startDate: this.startDate, | |
| purpose: this.purpose, | |
| mentor: this.mentor, | |
| mentee: this.mentee, | |
| }; | |
| if (this.mentorType) data.mentorType = this.mentorType; | |
| if (this.comment) data.comment = this.comment; | |
| if (this.descriptor) data.descriptor = this.descriptor; |
🤖 Prompt for AI Agents
In
components/workday/actions/create-mentorship-for-worker/create-mentorship-for-worker.mjs
around lines 76 to 85, mentorType is currently always included in the data
payload causing mentorType: undefined to be sent when not provided; modify the
code to only add mentorType to data if this.mentorType is defined (similar to
how comment and descriptor are handled) so the key is omitted when undefined.
components/workday/actions/update-mentorship/update-mentorship.mjs
Outdated
Show resolved
Hide resolved
| id: { | ||
| type: "string", | ||
| label: "ID", | ||
| description: "Optional ID for the event (usually system-generated).", | ||
| optional: true, | ||
| }, |
There was a problem hiding this comment.
Don’t include id in the PATCH body; it’s path-managed and not updatable.
Sending body id can be rejected or cause confusion with the path workContactInformationChangeId. Remove the prop and its assignment.
- id: {
- type: "string",
- label: "ID",
- description: "Optional ID for the event (usually system-generated).",
- optional: true,
- },
@@
- if (this.id) data.id = this.id;Also applies to: 58-58
🤖 Prompt for AI Agents
In
components/workday/actions/update-work-contact-information-change/update-work-contact-information-change.mjs
around lines 37-42 (also remove at line 58), the PATCH request body incorrectly
includes the id property which is managed via the URL path
(workContactInformationChangeId) and must not be sent in the payload; remove the
id field from the body schema and any code that assigns or maps id into the
request body so the PATCH body contains only updatable fields and relies on the
path parameter for the resource identifier.
| const data = {}; | ||
| if (this.alternateWorkLocation) { | ||
| if ( | ||
| typeof this.alternateWorkLocation !== "object" | ||
| || !this.alternateWorkLocation.id | ||
| || typeof this.alternateWorkLocation.id !== "string" | ||
| || !this.alternateWorkLocation.id.length | ||
| ) { | ||
| throw new ConfigurationError("alternateWorkLocation must include a non-empty 'id' property."); | ||
| } | ||
| data.alternateWorkLocation = this.alternateWorkLocation; | ||
| } | ||
| if (this.descriptor) data.descriptor = this.descriptor; | ||
| if (this.id) data.id = this.id; | ||
|
|
There was a problem hiding this comment.
Prevent empty PATCH payloads (guard before sending).
If no fields are set, this sends {} and likely 400s. Fail fast with a clear message.
async run({ $ }) {
- const data = {};
+ const data = {};
@@
- if (this.descriptor) data.descriptor = this.descriptor;
- if (this.id) data.id = this.id;
+ if (this.descriptor) data.descriptor = this.descriptor;
+
+ if (!Object.keys(data).length) {
+ throw new ConfigurationError("Select at least one field to update.");
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
components/workday/actions/update-work-contact-information-change/update-work-contact-information-change.mjs
around lines 45 to 59, the code builds a data object from optional fields but
can end up sending an empty {} payload; add a guard after assembling data that
checks if Object.keys(data).length === 0 and if so throw a ConfigurationError
with a clear message like "No fields provided to update; at least one updatable
field must be set" so the action fails fast instead of sending an empty PATCH.
| const parseObject = (obj) => { | ||
| if (!obj) return {}; |
There was a problem hiding this comment.
Reconsider the falsy value handling.
Returning {} for all falsy values (including null, undefined, 0, false, "") may not be appropriate. This behavior could mask legitimate values like 0 or false, and converts null/undefined into empty objects, which may not reflect the caller's intent.
Consider:
- Returning the input value directly for non-object falsy primitives (
0,false,""), or - Returning
null/undefinedas-is, or - Documenting this behavior explicitly if intentional
const parseObject = (obj) => {
- if (!obj) return {};
+ if (obj === null || obj === undefined) return obj;
+ if (obj === "" || obj === 0 || obj === false) return obj;🤖 Prompt for AI Agents
In components/workday/sources/common/utils.mjs around lines 1-2, the current
early return treats all falsy values as an empty object which incorrectly
converts legitimate primitives like 0, false, and "" and masks null/undefined
semantics; change the guard so only null/undefined are normalised (or only
non-object inputs are returned as-is) — specifically, preserve and return
non-object primitives and null/undefined as they were, and only coerce or return
{} when the input is truly intended to be an object (e.g., when typeof obj ===
'object' and obj is not null); update the function accordingly and add a brief
comment documenting the chosen behavior.
| export default { | ||
| parseObject, | ||
| parseJsonInput: (value) => { | ||
| return value | ||
| ? parseObject(value) | ||
| : undefined; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Inconsistent handling of falsy values between parseObject and parseJsonInput.
parseObject returns {} for falsy inputs, while parseJsonInput returns undefined for the same inputs. This inconsistency can lead to unexpected behavior when these functions are used interchangeably.
Consider aligning their behavior or clearly documenting when to use each.
🤖 Prompt for AI Agents
In components/workday/sources/common/utils.mjs around lines 29-36, parseObject
returns an empty object for falsy inputs while parseJsonInput returns undefined;
align their behavior by making parseJsonInput return the same default as
parseObject (i.e., call parseObject(value) and return its result for falsy
Inputs) or alternatively change parseObject to return undefined — implement the
chosen consistent behavior across both functions and update any related
callers/tests or documentation to reflect the unified return semantics.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/workday/actions/update-mentorship/update-mentorship.mjs (1)
54-65: Dead validation code: startDate makes the check impossible.The validation on line 63 checks if
datahas zero keys, butdatais initialized withstartDateon line 56, andstartDateis a required field (lines 36-40). This meansdatawill always have at least one key, making the validation on lines 63-65 unreachable dead code that can never execute.This issue was previously flagged. Apply one of these fixes:
Option A: Remove the dead validation (if startDate should remain required):
const data = { startDate: this.startDate, }; if (this.comment) data.comment = this.comment; if (this.purpose) data.purpose = this.purpose; if (this.endDate) data.endDate = this.endDate; if (this.descriptor) data.descriptor = this.descriptor; - - if (Object.keys(data).length === 0) { - throw new ConfigurationError("At least one field to update must be provided."); - }Option B: Make startDate optional and enforce at-least-one-field semantics:
const data = {}; + if (this.startDate) data.startDate = this.startDate; if (this.comment) data.comment = this.comment; if (this.purpose) data.purpose = this.purpose; if (this.endDate) data.endDate = this.endDate; if (this.descriptor) data.descriptor = this.descriptor; if (Object.keys(data).length === 0) { throw new ConfigurationError("At least one field to update must be provided."); }And mark
startDateas optional in the prop definition (line 40):startDate: { type: "string", label: "Start Date", description: "Updated start date. Example: `2025-10-18T07:00:00.000Z`", + optional: true, },
🧹 Nitpick comments (2)
components/workday/sources/new-worker-created/new-worker-created.mjs (1)
32-43: Suggest adding bounds to previousIds tracking.The
previousIdsobject grows indefinitely as new workers are tracked, which could lead to memory issues in long-running sources with high worker turnover.Consider implementing one of the following strategies:
- Time-based expiry: Remove IDs older than a certain threshold (e.g., 30 days)
- Size limit: Cap the object size and evict oldest entries (LRU pattern)
- Sliding window: Only track IDs from the last N workers
Example with size limit:
const previousIds = this._getPreviousIds(); + const maxTrackedIds = 10000; // Adjust based on expected volume let workers = []; for await (const worker of results) { if (previousIds[worker.id]) { continue; } workers.push(worker); previousIds[worker.id] = true; } + // Prune old IDs if exceeding limit + const idKeys = Object.keys(previousIds); + if (idKeys.length > maxTrackedIds) { + const excess = idKeys.length - maxTrackedIds; + idKeys.slice(0, excess).forEach(id => delete previousIds[id]); + } + this._setPreviousIds(previousIds);components/workday/actions/list-organization-types/list-organization-types.mjs (1)
32-35: Optional: Consider simplifying array collection.The manual for-await loop works correctly. If desired, this pattern could be condensed in the future, though the current implementation is clear and maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
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/delete-payroll-input/delete-payroll-input.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/actions/update-mentorship/update-mentorship.mjs(1 hunks)components/workday/package.json(1 hunks)components/workday/sources/new-worker-created/new-worker-created.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- components/workday/actions/list-supervisory-organizations/list-supervisory-organizations.mjs
- components/workday/actions/list-worker-payslips/list-worker-payslips.mjs
- components/workday/actions/create-job-change/create-job-change.mjs
- components/workday/actions/change-business-title/change-business-title.mjs
- components/workday/package.json
- components/workday/actions/search-workers/search-workers.mjs
🧰 Additional context used
🧬 Code graph analysis (2)
components/workday/actions/delete-payroll-input/delete-payroll-input.mjs (2)
components/workday/actions/create-payroll-input/create-payroll-input.mjs (1)
response(50-53)components/workday/actions/get-content-details/get-content-details.mjs (1)
response(24-27)
components/workday/actions/update-mentorship/update-mentorship.mjs (3)
components/workday/actions/close-mentorship/close-mentorship.mjs (2)
data(62-66)response(70-74)components/workday/actions/create-mentorship-for-worker/create-mentorship-for-worker.mjs (2)
data(76-83)response(87-90)components/workday/actions/create-mentorship-for-me/create-mentorship-for-me.mjs (2)
data(69-74)response(79-82)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (10)
components/workday/actions/get-worker/get-worker.mjs (1)
7-7: LGTM! Version bump is appropriate.The patch version increment from 0.0.3 to 0.0.4 aligns with the broader Workday integration update in this PR. No functional changes are required for this action.
components/workday/sources/new-worker-created/new-worker-created.mjs (1)
9-9: LGTM: Version bump is appropriate.The patch-level version increment is consistent with the broader Workday integration expansion in this PR.
components/workday/actions/list-organization-types/list-organization-types.mjs (1)
7-7: Version bump aligns with package update.The version increment to "0.0.4" is appropriate given the broader package update (0.1.1 → 0.1.2) and substantial changes to the underlying
workday.app.mjsdependencies.components/workday/actions/delete-payroll-input/delete-payroll-input.mjs (4)
1-2: LGTM!The imports are appropriate for a Pipedream action that performs validation and calls the Workday API.
4-13: LGTM!The metadata is well-structured and the
destructiveHint: trueannotation correctly indicates that this action performs a destructive operation.
15-23: LGTM!The props are correctly defined using the recommended
propDefinitionpattern.
24-34: The review comment is incorrect—the validation should not be removed.The
payrollInputIdpropDefinition incomponents/workday/workday.app.mjs(lines 314–330) does not includerequired: true. The manual validation at lines 25–27 is necessary and defensive; it prevents both empty strings and whitespace-only values from reaching the API call. Removing it would allow invalid input to proceed.Likely an incorrect or invalid review comment.
components/workday/actions/update-mentorship/update-mentorship.mjs (3)
1-3: LGTM!Imports are clean and appropriate for the action's requirements.
4-13: LGTM!Action metadata follows Pipedream conventions with appropriate key naming, clear description, and documentation reference.
67-74: LGTM!The API call structure and response handling follow standard Pipedream patterns and are consistent with related mentorship actions in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
components/workday/actions/create-organization-assignment-change/create-organization-assignment-change.mjs (1)
53-62: Null-safe validation for optional objects still needed.The concerns raised in the previous review regarding null-safety and type checking for
massActionWorksheetandmassActionHeaderremain unaddressed. These validations should guard againstnulland verify thatidis a string before calling.trim().
🧹 Nitpick comments (1)
components/workday/actions/create-home-contact-information-change/create-home-contact-information-change.mjs (1)
25-27: Consider whether workerId validation is necessary.The explicit validation for
workerIdmay be redundant if thepropDefinitionalready makes it a required field. However, the trim check does provide additional protection against whitespace-only values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/workday/actions/create-home-contact-information-change/create-home-contact-information-change.mjs(1 hunks)components/workday/actions/create-organization-assignment-change/create-organization-assignment-change.mjs(1 hunks)components/workday/actions/create-work-contact-information-change/create-work-contact-information-change.mjs(1 hunks)components/workday/actions/get-job-posting/get-job-posting.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
components/workday/actions/create-home-contact-information-change/create-home-contact-information-change.mjs (1)
components/workday/actions/create-work-contact-information-change/create-work-contact-information-change.mjs (1)
response(28-32)
components/workday/actions/create-organization-assignment-change/create-organization-assignment-change.mjs (2)
components/workday/actions/create-home-contact-information-change/create-home-contact-information-change.mjs (1)
response(28-32)components/workday/actions/create-work-contact-information-change/create-work-contact-information-change.mjs (1)
response(28-32)
components/workday/actions/create-work-contact-information-change/create-work-contact-information-change.mjs (1)
components/workday/actions/create-home-contact-information-change/create-home-contact-information-change.mjs (1)
response(28-32)
components/workday/actions/get-job-posting/get-job-posting.mjs (3)
components/workday/actions/delete-payroll-input/delete-payroll-input.mjs (1)
response(28-31)components/workday/actions/get-feedback-badge-details/get-feedback-badge-details.mjs (1)
response(24-27)components/workday/actions/get-give-requested-feedback-event-details/get-give-requested-feedback-event-details.mjs (1)
response(24-27)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (7)
components/workday/actions/get-job-posting/get-job-posting.mjs (3)
1-12: LGTM! Clean action metadata and structure.The import, key naming, description, and annotations are all well-formed. The documentation link provides helpful reference, and the annotations correctly reflect this as a non-destructive, read-only operation.
14-22: LGTM! Proper use of propDefinition pattern.The props correctly leverage propDefinition to reference the jobPostingId from the workday app file, following Pipedream best practices for prop reusability.
23-30: LGTM! Clean implementation following established patterns.The run method correctly delegates to the workday app's
getJobPostingmethod and exports a helpful summary. The implementation is consistent with other similar actions in this PR (e.g.,get-feedback-badge-details,get-give-requested-feedback-event-details).components/workday/actions/create-home-contact-information-change/create-home-contact-information-change.mjs (1)
1-2: LGTM!The imports are appropriate for this action.
components/workday/actions/create-organization-assignment-change/create-organization-assignment-change.mjs (1)
64-78: LGTM!The payload construction correctly includes required fields and conditionally adds optional fields. The API delegation and response handling follow the established pattern.
components/workday/actions/create-work-contact-information-change/create-work-contact-information-change.mjs (2)
1-2: LGTM!The imports are appropriate and both are used in the code.
25-27: Good defensive validation.The validation correctly checks for missing or empty workerId values, providing clear error messaging.
...ay/actions/create-home-contact-information-change/create-home-contact-information-change.mjs
Show resolved
Hide resolved
...ay/actions/create-home-contact-information-change/create-home-contact-information-change.mjs
Show resolved
Hide resolved
...ay/actions/create-home-contact-information-change/create-home-contact-information-change.mjs
Show resolved
Hide resolved
...kday/actions/create-organization-assignment-change/create-organization-assignment-change.mjs
Show resolved
Hide resolved
...kday/actions/create-organization-assignment-change/create-organization-assignment-change.mjs
Show resolved
Hide resolved
...ay/actions/create-work-contact-information-change/create-work-contact-information-change.mjs
Show resolved
Hide resolved
...ay/actions/create-work-contact-information-change/create-work-contact-information-change.mjs
Show resolved
Hide resolved
...ay/actions/create-work-contact-information-change/create-work-contact-information-change.mjs
Show resolved
Hide resolved
|
Hi @michelle0927,Could you please verify this PR. |
.../workday/actions/get-home-contact-information-change/get-home-contact-information-change.mjs
Outdated
Show resolved
Hide resolved
components/workday/actions/get-interview-details/get-interview-details.mjs
Show resolved
Hide resolved
…/get-home-contact-information-change.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/workday/actions/get-home-contact-information-change/get-home-contact-information-change.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/workday/actions/get-home-contact-information-change/get-home-contact-information-change.mjs (4)
components/workday/actions/get-job-posting/get-job-posting.mjs (1)
response(24-27)components/workday/actions/delete-payroll-input/delete-payroll-input.mjs (1)
response(28-31)components/workday/actions/get-content-details/get-content-details.mjs (1)
response(24-27)components/workday/actions/get-feedback-badge-details/get-feedback-badge-details.mjs (1)
response(24-27)
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
components/workday/actions/get-home-contact-information-change/get-home-contact-information-change.mjs (2)
29-29: Previous issue resolved correctly.The summary now correctly uses
this.homeContactInformationChangeIdinstead ofthis.id. Nice fix!
25-27: The review comment is incorrect; no changes needed.The parameter naming in the action file is correct. The Workday app method
getHomeContactInformationChangeexplicitly expects the parameterhomeContactInformationChangeId(workday.app.mjs, line 471), notid. While other methods likegetJobPosting,getContent, andgetFeedbackBadgeuseid, this is because their underlying API signatures are different. The action code accurately matches the Workday app method signature.Likely an incorrect or invalid review comment.
.../workday/actions/get-home-contact-information-change/get-home-contact-information-change.mjs
Outdated
Show resolved
Hide resolved
…/get-home-contact-information-change.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
@Lokeshchand33 Thank you so much for your contribution! Looks good. I just left one comment/question above about async options for interviewId.
michelle0927
moved this from In Review
to Changes Required
in Component (Source and Action) Backlog
* Added actions * Added actions * Added actions * Added actions * Update components/etermin/etermin.app.mjs --------- Co-authored-by: michelle0927 <[email protected]>
* use notion-helper * pnpm-lock.yaml * versions
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/workday/workday.app.mjs (3)
540-631: Remove redundantmethod: "GET"specifications.Many new methods explicitly specify
method: "GET"(lines 543, 552, 579, 588, 595, 605, 614, 628, and others), but this is unnecessary since GET is the default HTTP method for axios requests. The existing methods in lines 433-456 demonstrate the preferred pattern of omitting this.Apply this diff pattern to all GET methods:
listWorkContactInformationChanges(opts = {}) { return this._makeRequest({ path: "/workContactInformationChanges", - method: "GET", ...opts, }); }
592-608: Remove unnecessaryasynckeywords.Several methods are declared
asyncbut don't useawaitinternally (e.g.,listJobPostings,getJobPosting,listProspectsat lines 592, 600, 625). Since_makeRequestreturns a Promise directly, theasynckeyword is unnecessary unless you're usingawait.-async listJobPostings(opts = {}) { +listJobPostings(opts = {}) { return this._makeRequest({ path: "/jobPostings", - method: "GET", ...opts, }); }
715-721: Fix unconventional comma placement.Line 718-719 has unusual formatting where the comma appears at the start of the next line. This is inconsistent with the formatting used throughout the rest of the file.
createOrganizationAssignmentChange(opts = {}) { return this._makeRequest({ path: "/organizationAssignmentChanges", - method: "POST" - , ...opts, + method: "POST", + ...opts, }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/workday/workday.app.mjs(2 hunks)
⏰ 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). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/workday/workday.app.mjs (1)
483-1021: Implementation successfully covers required Workday API endpoints.The added methods comprehensively implement the endpoints specified in issue #18802:
- Person-related: homeContactInformationChanges, people, phoneValidation, workContactInformationChanges ✓
- Recruiting: interviews, jobPostings, prospects ✓
- Staffing: jobChanges, jobFamilies, jobProfiles, jobs, organizationAssignmentChanges, supervisoryOrganizations, workers ✓
- Talent Management: mentorships (CRUD), successionPlans ✓
- Holiday, Journeys, Learning, Payroll, Performance Enablement: all covered ✓
The methods follow the existing
_makeRequestpattern and integrate well with the authentication flow.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/workday/actions/update-mentorship/update-mentorship.mjs (1)
2-2: Remove unused import.The
ConfigurationErrorimport is no longer used after the validation code was removed in the previous iteration.Apply this diff to remove the unused import:
-import { ConfigurationError } from "@pipedream/platform";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/workday/actions/update-mentorship/update-mentorship.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/workday/actions/update-mentorship/update-mentorship.mjs (3)
components/workday/actions/close-mentorship/close-mentorship.mjs (2)
data(62-66)response(70-74)components/workday/actions/create-mentorship-for-worker/create-mentorship-for-worker.mjs (2)
data(76-83)response(87-90)components/workday/actions/create-mentorship-for-me/create-mentorship-for-me.mjs (2)
data(69-74)response(79-82)
⏰ 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: Verify TypeScript components
- GitHub Check: pnpm publish
michelle0927
moved this from Changes Required
to Ready for QA
in Component (Source and Action) Backlog
8 participants
WHY
This PR implements the Workday API integration as described in [ACTION] Workday API #18802.
Closes #18802.
Summary by CodeRabbit
New Features
Chores