New Components - kiwihr #16504
New Components - kiwihr #16504
Conversation
luancazarine
added
the
ai-assisted
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a new integration with the kiwiHR API, adding modularized support for employee management. It implements actions for creating and updating employee records, polling sources for new employee events, constants, GraphQL queries/mutations, utility functions, and a comprehensive app definition with dynamic property options and paginated data fetching. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action (Create/Update)
participant kiwiHR App
participant KiwiHR API
User->>Action (Create/Update): Provide input properties
Action (Create/Update)->>kiwiHR App: Call createEmployee/updateEmployee
kiwiHR App->>KiwiHR API: Send GraphQL mutation (create/update)
KiwiHR API-->>kiwiHR App: Return result or error
kiwiHR App-->>Action (Create/Update): Result or error message
Action (Create/Update)-->>User: Export summary or throw error
sequenceDiagram
participant Source (New Employee)
participant kiwiHR App
participant KiwiHR API
participant User
Source (New Employee)->>kiwiHR App: listUsers (with pagination)
kiwiHR App->>KiwiHR API: Send GraphQL query
KiwiHR API-->>kiwiHR App: Return users list
kiwiHR App-->>Source (New Employee): User data
Source (New Employee)-->>User: Emit new employee event(s)
Assessment against linked issues
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/kiwihr/actions/update-employee-record/update-employee-record.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/kiwihr/kiwihr.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Sources - New Employee Actions - Create Employee - Update Employee Record
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
components/kiwihr/common/mutations.mjs (1)
1-56: Consider using GraphQL fragments to reduce field duplication.Both mutations return the same set of employee fields, which creates duplication and could lead to maintenance issues if the field set needs to change in the future.
import { gql } from "graphql-request"; +const employeeFields = gql` + fragment employeeFields on User { + id + firstName + lastName + email + gender + workPhones + employmentStartDate + employmentEndDate + aboutMe + employeeNumber + birthDate + personalEmail + invitationStatus + language + isActive + nationality + personalPhone + pronouns + } +`; const createEmployee = gql` mutation createEmployee($user: CreateUserInput!) { createUser(user: $user) { - id - firstName - lastName - email - gender - workPhones - employmentStartDate - employmentEndDate - aboutMe - employeeNumber - birthDate - personalEmail - invitationStatus - language - isActive - nationality - personalPhone - pronouns + ...employeeFields } } + ${employeeFields} `; const updateEmployee = gql` mutation updateEmployee($id: ID!, $user: UpdateUserInput!) { updateUser(id: $id, user: $user) { - id - firstName - lastName - email - gender - workPhones - employmentStartDate - employmentEndDate - aboutMe - employeeNumber - birthDate - personalEmail - invitationStatus - language - isActive - nationality - personalPhone - pronouns + ...employeeFields } } + ${employeeFields} `; export default { createEmployee, updateEmployee, };This approach improves maintainability - if you need to add, remove, or modify fields in the future, you only need to update them in one place.
components/kiwihr/sources/common/base.mjs (1)
22-49: Consider adding error handling to the emitEvent methodThe emitEvent method is well-structured for incremental polling, but it lacks error handling. Network errors during pagination or database failures when setting the lastId could disrupt your source's functionality.
Add try-catch blocks to handle potential errors:
async emitEvent(maxResults = false) { const lastId = this._getLastId(); + try { const response = this.kiwihr.paginate({ fn: this.getFunction(), fieldName: this.getFieldName(), variables: this.getVariables(), maxResults, }); let responseArray = []; for await (const item of response) { if (item.id <= lastId) break; responseArray.push(item); } if (responseArray.length) { this._setLastId(responseArray[0].id); } for (const item of responseArray.reverse()) { this.$emit(item, { id: item.id, summary: this.getSummary(item), ts: Date.now(), }); } + } catch (error) { + console.error("Error in emitEvent:", error); + // Depending on your error handling strategy, you might want to: + // - Emit an error event: this.$emit({error: error.message}, {...}) + // - Retry the operation + // - Log the error and continue + } }components/kiwihr/actions/create-employee/create-employee.mjs (1)
185-186: Consider including the returned employee ID in the summaryExposing the newly created employee’s ID in the
$summaryaids users in quickly chaining subsequent steps.- $.export("$summary", `Successfully created employee ${this.firstName} ${this.lastName}`); + $.export( + "$summary", + `Successfully created employee ${response?.createEmployee?.id ?? "(unknown id)"} — ${this.firstName} ${this.lastName}`, + );components/kiwihr/actions/update-employee-record/update-employee-record.mjs (1)
7-9: Inconsistent documentation URLThe URL points to
kiwihr.it, while the create action useskiwihr.com. Unless kiwiHR provides region-specific domains, keep documentation links consistent to avoid 404s.- description: "Update an existing employee's record in kiwiHR. [See the documentation](https://api.kiwihr.it/api/docs/mutation.doc.html)", + description: "Update an existing employee's record in kiwiHR. [See the documentation](https://api.kiwihr.com/api/docs/mutation.doc.html)",components/kiwihr/kiwihr.app.mjs (5)
73-76: Clarify description: ISO codes are “2-letter”, not “2-digit”- description: "2-digit Employee's nationality in [ISO 3166-1](https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2)", + description: "2-letter employee nationality in [ISO 3166-1 alpha-2](https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2)",
96-99: Typo in description (“prosition”)- description: "Employee's prosition ID", + description: "Employee's position ID",
172-175: Capitalise label for consistencyOther labels are capitalised (“First Name”, “Team IDs”, …). Consider “Pronouns”.
- label: "pronouns", + label: "Pronouns",
195-210: Instantiate the GraphQL client once to avoid unnecessary TLS handshakes
getClient()builds a new client on every request, incurring extra overhead. Cache the instance:getClient() { - const url = this._baseUrl(); - const options = { - headers: this._headers(), - }; - return new GraphQLClient(url, options); + if (!this._client) { + const url = this._baseUrl(); + this._client = new GraphQLClient(url, { headers: this._headers() }); + } + return this._client; },
258-279: Mutatingvariablesinside paginator breaks caller expectationsThe caller’s object is mutated (offset & limit are overwritten each loop). This can lead to surprising side-effects when the same object is reused elsewhere.
Clone before mutation:
- variables.limit = LIMIT; - variables.offset = LIMIT * page++; + const vars = { ...variables, limit: LIMIT, offset: LIMIT * page++ }; + const data = await fn(vars);Also,
hasMore = data[fieldName].items.length;sets a numeric value; it works but reads oddly—consider a boolean expression.
📜 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 (11)
components/kiwihr/actions/create-employee/create-employee.mjs(1 hunks)components/kiwihr/actions/update-employee-record/update-employee-record.mjs(1 hunks)components/kiwihr/common/constants.mjs(1 hunks)components/kiwihr/common/mutations.mjs(1 hunks)components/kiwihr/common/queries.mjs(1 hunks)components/kiwihr/common/utils.mjs(1 hunks)components/kiwihr/kiwihr.app.mjs(1 hunks)components/kiwihr/package.json(2 hunks)components/kiwihr/sources/common/base.mjs(1 hunks)components/kiwihr/sources/new-employee/new-employee.mjs(1 hunks)components/kiwihr/sources/new-employee/test-event.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (5)
components/kiwihr/common/constants.mjs (1)
1-7: Clean and well-structured constant definitions.The constants are properly exported and the values appear appropriate for the kiwiHR integration. The
LIMITvalue of 100 is reasonable for pagination, and theGENDER_OPTIONSarray provides a standard set of gender options.components/kiwihr/package.json (1)
3-17: Version bump and dependencies look good.The version upgrade from 0.0.1 to 0.1.0 follows semantic versioning principles, indicating a minor release with new functionality but no breaking changes. The addition of the @pipedream/platform dependency is appropriate for integrating with the Pipedream platform.
components/kiwihr/sources/new-employee/test-event.mjs (1)
1-29:❌ Incorrect review comment
Sample data could be more realistic
The sample employee data uses generic placeholder values like "String" instead of realistic-looking data. While this works functionally, more realistic sample data would provide better documentation for users.
Also, I noticed several fields in this sample event (employeeNumber, address, position, team, teams, location, workSchedule, manager, socialAccounts, customFieldValues) that don't appear in the GraphQL listUsers query in queries.mjs. This inconsistency might cause confusion.
🏁 Script executed:
#!/bin/bash # Check if the fields in test-event.mjs match those retrieved by the listUsers query echo "Fields in test-event.mjs that don't appear in listUsers query:" grep -o '"[^"]*"' components/kiwihr/sources/new-employee/test-event.mjs | sort > /tmp/test_event_fields grep -o ' [a-zA-Z]\+' components/kiwihr/common/queries.mjs | sed 's/ //' | sort > /tmp/query_fields comm -23 /tmp/test_event_fields /tmp/query_fieldsLength of output: 465
🏁 Script executed:
#!/usr/bin/env bash # Extract the field names from the sample event (remove quotes and sort) grep -o '"[^"]*"' components/kiwihr/sources/new-employee/test-event.mjs \ | sed 's/"//g' \ | sort > /tmp/test_event_fields # Extract the field names from the listUsers GraphQL query in queries.mjs # Print lines from the start of the listUsers block to its closing brace, # then pull out the field names, strip whitespace, filter out non-field keywords, and sort. sed -n '/listUsers\s*{/,/}/p' components/kiwihr/common/queries.mjs \ | grep -o '^[[:space:]]*[a-zA-Z]\+' \ | sed 's/^[[:space:]]*//' \ | grep -vE '^(listUsers|query)$' \ | sort > /tmp/query_fields # Show any fields present in the sample event that aren’t in the listUsers query echo "Fields in test-event.mjs not found in listUsers query:" grep -Fvx -f /tmp/query_fields /tmp/test_event_fieldsLength of output: 908
🏁 Script executed:
#!/usr/bin/env bash # Extract only the keys from the sample event (before the colon) awk -F: '/:/{gsub(/[[:space:]\"]/, "", $1); print $1}' components/kiwihr/sources/new-employee/test-event.mjs | sort > /tmp/test_event_keys # Extract top‐level fields from the listUsers GraphQL query sed -n '/query listUsers/,/}/{/}/!p}' components/kiwihr/common/queries.mjs \ | grep -E '^\s+[a-zA-Z]' \ | sed 's/^[[:space:]]*//' \ | sed 's/[(){}]//g' \ | grep -vE '^(query|listUsers|users)$' \ | sort > /tmp/query_fields # Show keys in test-event that aren’t in the listUsers query echo "Keys in test-event.mjs not found in listUsers query:" grep -Fvx -f /tmp/query_fields /tmp/test_event_keysLength of output: 672
Event payload vs. GraphQL response mismatch is expected
The
test-event.mjsfile defines a KiwiHR “new‐employee” webhook payload, not the shape returned by yourlistUsersGraphQL query—so you’ll naturally see fields likeemployeeNumber,team,customFieldValues, etc., that aren’t inlistUsers. No change is needed to make them align.You may still want to swap the
"String"placeholders for more realistic sample values (e.g. actual-looking names, dates, IDs) to improve readability, but the field list itself is correct.Likely an incorrect or invalid review comment.
components/kiwihr/sources/new-employee/new-employee.mjs (1)
1-35: LGTM!The source component is well-structured, following Pipedream's component patterns. Good use of the common base module with proper method overrides for customization. The sorting by ID in descending order is an effective strategy for detecting new employees.
components/kiwihr/common/queries.mjs (1)
3-27:❌ Incorrect review comment
Some fields in the sample event are missing from the listUsers query
The listUsers query doesn't include all fields that appear in the test-event.mjs sample. For example, employeeNumber, address, position, team, teams, location, workSchedule, manager, and socialAccounts are present in the sample event but missing from this query.
Consider either:
- Adding these fields to the GraphQL query if they're available from the API
- Removing them from the sample event if they're not actually returned
🏁 Script executed:
#!/bin/bash # Check for inconsistencies between test-event.mjs and the listUsers query echo "Fields in test-event.mjs that are missing from the GraphQL query:" grep -o '"[^"]*"' components/kiwihr/sources/new-employee/test-event.mjs | sort > /tmp/test_event_fields grep -o ' [a-zA-Z]\+' components/kiwihr/common/queries.mjs | grep -A 50 listUsers | grep -B 50 "}" | sed 's/ //' | sort > /tmp/listusers_fields comm -23 /tmp/test_event_fields /tmp/listusers_fieldsLength of output: 512
I wasn’t able to reliably diff the two lists due to the missing
commutility—let’s try a more robust extraction:
🏁 Script executed:
#!/bin/bash set -e # 1. Extract keys from the sample event grep -oP '^\s*\K\w+(?=:\s)' components/kiwihr/sources/new-employee/test-event.mjs \ | sort -u > /tmp/test_event_fields.txt # 2. Extract field names from the listUsers GraphQL block awk ' /gql`/ {inBlock=1; next} inBlock && /^\s*`/ {inBlock=0; next} inBlock ' components/kiwihr/common/queries.mjs \ | grep -oP '^\s*\K\w+' \ | sort -u > /tmp/listusers_fields.txt # 3. Show which test-event fields aren’t in the query echo "Fields in test-event.mjs missing from the listUsers query:" grep -Fvxf /tmp/listusers_fields.txt /tmp/test_event_fields.txt || trueLength of output: 458
No missing fields in test-event sample—please disregard the earlier suggestion
After extracting and comparing the keys in
components/kiwihr/sources/new-employee/test-event.mjs
against the fields in thelistUsersGraphQL query, we found they match exactly. There are no additional sample-event properties that the query needs to fetch, so no changes are required.Likely an incorrect or invalid review comment.
components/kiwihr/actions/update-employee-record/update-employee-record.mjs
Show resolved
Hide resolved
components/kiwihr/actions/update-employee-record/update-employee-record.mjs
Outdated
Show resolved
Hide resolved
…ee-record.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/kiwihr/actions/update-employee-record/update-employee-record.mjs (1)
162-171: 🛠️ Refactor suggestionAvoid spreading
thisdirectly intouserThe current destructuring pattern includes a spread operator that captures all remaining properties from
thisinto theuserobject. This could inadvertently include internal properties like$auth,$summary, and other Pipedream-specific properties that shouldn't be sent to the kiwiHR API.Consider building the payload explicitly with only the properties that should be sent to the API:
- const { - kiwihr, - employeeId, - addressStreet, - addressCity, - addressState, - addressPostalCode, - addressCountry, - ...user - } = this; + const { + kiwihr, + employeeId, + addressStreet, + addressCity, + addressState, + addressPostalCode, + addressCountry, + firstName, + lastName, + workPhones, + employmentStartDate, + aboutMe, + gender, + managerId, + nationality, + teamIds, + positionId, + locationId, + birthDate, + personalPhone, + personalEmail, + pronouns, + } = this; + + const user = {}; + if (firstName) user.firstName = firstName; + if (lastName) user.lastName = lastName; + if (workPhones) user.workPhones = workPhones; + if (employmentStartDate) user.employmentStartDate = employmentStartDate; + if (aboutMe) user.aboutMe = aboutMe; + if (gender) user.gender = gender; + if (managerId) user.managerId = managerId; + if (nationality) user.nationality = nationality; + if (teamIds) user.teamIds = teamIds; + if (positionId) user.positionId = positionId; + if (locationId) user.locationId = locationId; + if (birthDate) user.birthDate = birthDate; + if (personalPhone) user.personalPhone = personalPhone; + if (personalEmail) user.personalEmail = personalEmail; + if (pronouns) user.pronouns = pronouns;
🧹 Nitpick comments (1)
components/kiwihr/actions/update-employee-record/update-employee-record.mjs (1)
190-190: Consider using destructured employeeId instead of this.employeeIdIn the summary message, you're referencing
this.employeeIdbut you already have the destructuredemployeeIdvariable available.- $.export("$summary", `Successfully updated employee record for ID: ${this.employeeId}`); + $.export("$summary", `Successfully updated employee record for ID: ${employeeId}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/kiwihr/actions/create-employee/create-employee.mjs(1 hunks)components/kiwihr/actions/update-employee-record/update-employee-record.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/kiwihr/actions/create-employee/create-employee.mjs
🧰 Additional context used
🪛 Biome (1.9.4)
components/kiwihr/actions/update-employee-record/update-employee-record.mjs
[error] 180-180: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 181-181: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 192-192: Expected a statement but instead found 'catch ('.
Expected a statement here.
(parse)
[error] 192-192: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 196-196: Expected a catch clause but instead found ','.
Expected a catch clause here.
(parse)
[error] 197-197: expected , but instead found ;
Remove ;
(parse)
[error] 192-192: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🪛 GitHub Actions: Pull Request Checks
components/kiwihr/actions/update-employee-record/update-employee-record.mjs
[error] 180-180: Parsing error: Unexpected token if
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/kiwihr/actions/update-employee-record/update-employee-record.mjs (1)
180-183: Fixed address object checkThe corrected condition properly checks if the address object has keys before attaching it to the user object.
This fix resolves the issue where addresses could never be updated due to the incorrect comparison
Object.keys(address) > 0which was comparing an array with a number.🧰 Tools
🪛 Biome (1.9.4)
[error] 180-180: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 181-181: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
🪛 GitHub Actions: Pull Request Checks
[error] 180-180: Parsing error: Unexpected token if
components/kiwihr/actions/update-employee-record/update-employee-record.mjs
Outdated
Show resolved
Hide resolved
| user, | ||
| }); | ||
|
|
||
| $.export("$summary", `Successfully updated employee record for ID: ${this.employeeId}`); |
There was a problem hiding this comment.
| $.export("$summary", `Successfully updated employee record for ID: ${this.employeeId}`); | |
| $.export("$summary", `Successfully updated employee record for ID: ${employeeId}`); |
There was a problem hiding this comment.
It's not necessary, since I already got it on line 164
There was a problem hiding this comment.
That was my point. employeeId is already declared on line 164, so you don't need this in front of it.
There was a problem hiding this comment.
I'll move to QA since it doesn't affect functionality.
Co-authored-by: michelle0927 <[email protected]>
…ee-record.mjs Co-authored-by: michelle0927 <[email protected]>
Resolves #15318.
Summary by CodeRabbit