Added 6 new actions to support advanced cell formatting, data validation, and protection #18995
Conversation
…ion, and protection
|
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 six Google Sheets action modules (add/update/delete conditional format rules, add protected range, set data validation, merge cells) and bumps the package version. Each action parses inputs, builds a Sheets API batchUpdate request, and calls Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Action as Action.run()
participant Parser as RangeParser
participant Builder as RequestBuilder
participant API as googleSheets.batchUpdate
Caller->>Action: invoke action(props, $)
Action->>Parser: parse worksheetId and range
Parser-->>Action: startRow/endRow, startCol/endCol, sheetId
Note right of Action: Build conditional-format / protected-range / validation / merge payload
Action->>Builder: assemble batchUpdate request
Builder-->>Action: request payload
Action->>API: googleSheets.batchUpdate(request)
API-->>Action: response
Action->>Caller: export $summary and return response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 16
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs(1 hunks)components/google_sheets/actions/add-protected-range/add-protected-range.mjs(1 hunks)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs(1 hunks)components/google_sheets/actions/merge-cells/merge-cells.mjs(1 hunks)components/google_sheets/actions/set-data-validation/set-data-validation.mjs(1 hunks)components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
components/google_sheets/actions/set-data-validation/set-data-validation.mjs (3)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
request(89-115)components/google_sheets/actions/merge-cells/merge-cells.mjs (1)
request(67-85)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (2)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (1)
request(477-489)components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (1)
request(481-495)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (2)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (2)
rule(312-320)protoToCssColor(321-347)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/merge-cells/merge-cells.mjs (3)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
request(89-115)components/google_sheets/actions/set-data-validation/set-data-validation.mjs (1)
request(94-121)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (3)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (3)
rule(307-315)protoToCssColor(317-343)request(477-489)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (1)
request(47-59)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
⏰ 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 (2)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (2)
3-15: LGTM!The RGB to CSS hex color conversion logic is correct.
481-497: LGTM!The API request structure correctly follows the Google Sheets
updateConditionalFormatRuleRequestformat.
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
components/google_sheets/actions/add-protected-range/add-protected-range.mjs
Outdated
Show resolved
Hide resolved
components/google_sheets/actions/set-data-validation/set-data-validation.mjs
Outdated
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
michelle0927
left a comment
michelle0927
left a comment
There was a problem hiding this comment.
Thank you for your contribution! I left a few comments.
Props and methods belonging to a component can be accessed using this.<propName> and this.<methodName>, so you can remove the word "props" from anywhere using this.props.<propName>.
You'll need to update the package.json version to "0.11.0".
If you want to test out the components, you can publish them to your account via the CLI.
https://pipedream.com/docs/cli/reference#pd-publish
Thanks, and let me know if you have any questions.
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
...ents/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (15)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
81-117: Critical: Column index calculation fails for multi-letter columns and end indices are not exclusive.The existing past review comment already identified this issue. Lines 101-102 use
charCodeAt(0) - 65/64which only works for single-letter columns (A-Z). Multi-letter columns like "AA" or "AB" will wrap to incorrect indices. Additionally, the Google Sheets API requires exclusive end indices (one past the last row/column), butendRowIndexis set toendRowdirectly.components/google_sheets/actions/set-data-validation/set-data-validation.mjs (1)
86-123: Critical: GridRange index calculation fails for multi-letter columns and lacks exclusive end indices.This is the same issue flagged in the past review. Lines 104-105 use
charCodeAt(0) - 65/64which breaks for columns beyond Z (AA, AB, etc.). The API also requires exclusive end indices, but the current code uses the parsed row/column values directly.components/google_sheets/actions/merge-cells/merge-cells.mjs (1)
59-87: Critical: GridRange calculation fails for multi-letter columns and lacks exclusive end indices.This duplicates the issue from the past review. Lines 77-78 use
charCodeAt(0) - 65/64which breaks for columns beyond Z. The API requires exclusive end indices but the code uses parsed values directly.components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (10)
54-60: Fix the description to match conditional formatting.The past review already identified this: the description incorrectly references "protect" instead of conditional formatting.
61-77: Fix the label to match the prop name.The past review already identified this: the label "Validation Type" doesn't match
conditionTypeand this is for conditional formatting, not validation.
94-108: Remove duplicate "SCIENTIFIC" option.The past review already identified this: "SCIENTIFIC" appears at both lines 102 and 107.
279-291: Fix prop naming convention to camelCase.The past review already identified this:
InterpolationPointTypeuses PascalCase instead of camelCase. It should beinterpolationPointTypeand all references updated.
297-301: MakenewIndexoptional.The past review already identified this: the API allows updating rules in-place without moving them, so
newIndexshould be optional.
311-319: Critical: Column index calculation fails for multi-letter columns.The past review already identified this:
charCodeAt(0) - 65/64only works for single-letter columns (A-Z). Use the existing_getColumnIndexhelper method instead.
320-346: ExtractprotoToCssColorto avoid duplication.The past review already identified this: this function is duplicated in add-conditional-format-rule.mjs and should be extracted to a shared location.
348-478: Refactor the large ternary operator for readability.The past review already identified this: the 130-line ternary is difficult to maintain. Use if-else statements or helper functions instead.
348-386: Critical: Handle undefinedrgbColorand use correct API schema.The past review already identified that
protoToCssColoris called on optionalrgbColorwithout null checks. Additionally, the API expectsColorStyle.rgbColorto be an object with{red, green, blue}properties, not a CSS string. The current implementation will cause 400 errors.
387-478: Critical: Handle undefinedrgbColorin boolean rule.The past review already identified that
protoToCssColoris called multiple times on optionalrgbColorwithout null checks, which will cause runtime errors.components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (2)
299-315: Critical: Column index calculation and color schema violations will cause 400 errors.Multiple critical issues will cause the batchUpdate request to fail:
- Lines 312-313 use
charCodeAt(0) - 65/64which only works for single-letter columns (A-Z), causing multi-letter columns like "AA" to resolve incorrectly.- The API requires exclusive end indices (one past the last row/column).
- The
protoToCssColorhelper returns CSS color strings, butColorStyle.rgbColorexpects an object with{red, green, blue}numeric properties per the API schema.
345-475: Critical: Handle undefinedrgbColorand fix color schema violations.
protoToCssColor(this.rgbColor)is called multiple times (lines 350, 362, 374, 397, 406, 415, 424, 433) but:
rgbColoris optional and will cause runtime errors when undefined- The helper returns CSS strings but the API expects
{red, green, blue}objectsAdditionally, line 453 assigns
this.rgbColordirectly (correct for the API) but is inconsistent with other usages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs(1 hunks)components/google_sheets/actions/add-protected-range/add-protected-range.mjs(1 hunks)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs(1 hunks)components/google_sheets/actions/merge-cells/merge-cells.mjs(1 hunks)components/google_sheets/actions/set-data-validation/set-data-validation.mjs(1 hunks)components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs(1 hunks)components/google_sheets/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
📚 Learning: 2024-10-08T16:42:59.225Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14229
File: components/americommerce/actions/update-customer/update-customer.mjs:89-94
Timestamp: 2024-10-08T16:42:59.225Z
Learning: When defining boolean properties in AmeriCommerce components (e.g., in `update-customer.mjs`), ensure that the label and description are consistent and clearly indicate the intent, especially when using negations like "No Account", to avoid confusion.
Applied to files:
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
📚 Learning: 2025-08-27T17:25:10.425Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/utils/type-guards.ts:23-33
Timestamp: 2025-08-27T17:25:10.425Z
Learning: In the connect-react package, the isOptionWithLabel type guard intentionally restricts value types to string|number for runtime filtering purposes, even though LabelValueOption<T> allows any T. This runtime behavior should be preserved over type safety improvements.
Applied to files:
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
🧬 Code graph analysis (6)
components/google_sheets/actions/set-data-validation/set-data-validation.mjs (2)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
request(89-115)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (2)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (1)
request(477-489)components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (1)
request(480-494)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/merge-cells/merge-cells.mjs (3)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
request(89-115)components/google_sheets/actions/set-data-validation/set-data-validation.mjs (1)
request(94-121)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (2)
components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (1)
request(47-59)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (3)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (3)
rule(307-315)protoToCssColor(317-343)request(477-489)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (1)
request(47-59)
⏰ 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/google_sheets/package.json (1)
3-3: LGTM - Version bump is appropriate.The minor version increment from 0.10.0 to 0.11.0 correctly reflects the addition of six new action modules without breaking changes.
components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (1)
46-61: LGTM - Delete action is correctly implemented.The action properly constructs the deleteConditionalFormatRuleRequest with the required sheetId and index parameters. The destructiveHint annotation is appropriately set to true.
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Outdated
Show resolved
Hide resolved
components/google_sheets/actions/set-data-validation/set-data-validation.mjs
Show resolved
Hide resolved
…dd-conditional-format-rule.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…validation.mjs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (1)
142-143: Column index calculation fails for multi-letter columns.Using
charCodeAt(0)only works for single-letter columns (A-Z). For columns beyond Z (AA, AB, etc.), this produces incorrect indices. A comprehensive solution was already proposed in previous review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 12731
File: components/hackerone/actions/get-members/get-members.mjs:3-28
Timestamp: 2024-10-08T15:33:38.240Z
Learning: When exporting a summary message in the `run` method of an action, ensure the message is correctly formatted. For example, in the `hackerone-get-members` action, the correct format is `Successfully retrieved ${response.data.length} members`.
Applied to files:
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
📚 Learning: 2024-10-08T16:42:59.225Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14229
File: components/americommerce/actions/update-customer/update-customer.mjs:89-94
Timestamp: 2024-10-08T16:42:59.225Z
Learning: When defining boolean properties in AmeriCommerce components (e.g., in `update-customer.mjs`), ensure that the label and description are consistent and clearly indicate the intent, especially when using negations like "No Account", to avoid confusion.
Applied to files:
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
📚 Learning: 2025-08-27T17:25:10.425Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/utils/type-guards.ts:23-33
Timestamp: 2025-08-27T17:25:10.425Z
Learning: In the connect-react package, the isOptionWithLabel type guard intentionally restricts value types to string|number for runtime filtering purposes, even though LabelValueOption<T> allows any T. This runtime behavior should be preserved over type safety improvements.
Applied to files:
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
📚 Learning: 2025-02-05T21:58:03.118Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 15479
File: packages/connect-react/src/components/ComponentForm.tsx:23-24
Timestamp: 2025-02-05T21:58:03.118Z
Learning: In the connect-react package, the `enableDebugging` property should be of type `boolean` as it's used for toggling debugging features and conditional rendering.
Applied to files:
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
🧬 Code graph analysis (1)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (4)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (2)
rule(311-319)request(480-494)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (1)
request(47-59)components/google_sheets/actions/set-data-validation/set-data-validation.mjs (1)
request(91-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (4)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (4)
1-14: LGTM! Clean imports and metadata.The ConfigurationError import and action metadata follow Pipedream conventions.
109-121: LGTM! Interpolation point type options are correct.The prop correctly uses camelCase and provides the standard interpolation point types for gradient rules.
206-222: LGTM! Request structure and execution are correct.The batchUpdate request is properly structured, and the summary export provides good user feedback. Good to see the response returned for further processing.
Based on learnings.
148-157: LGTM! Clean RGB color parsing with proper error handling.The helper gracefully handles both string and object inputs, provides a sensible default, and throws a clear ConfigurationError for invalid JSON.
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (2)
93-94: Column index conversion fails for multi-letter columns (AA, AB, etc.).Using
charCodeAt(0)only reads the first character, so column "AA" is treated as "A" (index 0 instead of 26), causing protected ranges beyond column Z to wrap incorrectly to column A.You need a helper function that converts full column strings to zero-based indices. For example:
+ // Helper to convert column letter(s) to zero-based index + const colToIndex = (col) => { + let index = 0; + for (let i = 0; i < col.length; i++) { + index = index * 26 + (col.charCodeAt(i) - 64); + } + return index - 1; + }; + const request = { spreadsheetId: this.sheetId, requestBody: { requests: [ { addProtectedRange: { protectedRange: { protectedRangeId: this.protectedRangeId, range: { sheetId: this.worksheetId, startRowIndex: startRow, - endRowIndex: endRow, + endRowIndex: endRow === startRow ? endRow + 1 : endRow, - startColumnIndex: startCol.charCodeAt(0) - 65, + startColumnIndex: colToIndex(startCol), - endColumnIndex: endCol.charCodeAt(0) - 64, + endColumnIndex: colToIndex(endCol) + 1, },
92-92: Single-cell ranges resolve to empty ranges.When
_parseRangeStringparses a single-cell input like "A1", bothstartRowandendRoware 0, creating an empty range[0, 0). The Sheets API requires exclusive end indices, so a single cell at row 0 needsendRowIndex: 1.The fix is included in the previous comment: use
endRow === startRow ? endRow + 1 : endRowto handle single-cell ranges correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (3)
components/google_sheets/actions/set-data-validation/set-data-validation.mjs (1)
request(91-118)components/google_sheets/actions/merge-cells/merge-cells.mjs (1)
request(67-85)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
⏰ 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
components/google_sheets/actions/add-protected-range/add-protected-range.mjs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (2)
88-88: This issue has already been flagged in previous reviews.The inclusion of
protectedRangeIdin the add operation was identified in past review comments. The Google Sheets API auto-generates IDs for new protected ranges, and including a user-provided ID can cause conflicts.
93-94: This issue has already been flagged in previous reviews.The column index calculation using
charCodeAt(0)was identified in past review comments. This approach only works for single-letter columns (A-Z) and fails for multi-letter columns like "AA" or "AB".components/google_sheets/actions/merge-cells/merge-cells.mjs (2)
45-45: This issue has already been flagged in previous reviews.The description incorrectly mentions "apply validation" instead of describing the merge operation. This was identified as a copy-paste error in past review comments.
77-78: This issue has already been flagged in previous reviews.The column index calculation using
charCodeAt(0)was identified in past review comments. This approach fails for multi-letter columns and single-cell ranges.components/google_sheets/actions/set-data-validation/set-data-validation.mjs (1)
100-101: This issue has already been flagged in previous reviews.The column index calculation using
charCodeAt(0)was identified in past review comments. This approach only works for single-letter columns (A-Z) and produces incorrect results for multi-letter columns.components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (4)
46-46: This issue has already been flagged in previous reviews.The description incorrectly mentions "protect" instead of describing conditional formatting. This was identified in past review comments.
50-50: This issue has already been flagged in previous reviews.The label "Validation Type" doesn't match the prop name
conditionTypeand should reference conditional formatting conditions, not validation. This was identified in past review comments.
148-149: This issue has already been flagged in previous reviews.The column index calculation using
charCodeAt(0)fails for multi-letter columns. This was comprehensively covered in past review comments.
171-189: Fix prop reference casing to match prop definition.Lines 171, 180, and 189 reference
this.InterpolationPointType(PascalCase), but the prop is defined asinterpolationPointType(camelCase) at line 109. This will cause the property to be undefined at runtime.Apply this diff:
colorStyle: { rgbColor: parseRgbColor(this.rgbColor), }, - type: this.InterpolationPointType, + type: this.interpolationPointType, value: "MIN",Apply the same fix at lines 180 and 189.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs(1 hunks)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs(1 hunks)components/google_sheets/actions/merge-cells/merge-cells.mjs(1 hunks)components/google_sheets/actions/set-data-validation/set-data-validation.mjs(1 hunks)components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-08T16:42:59.225Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14229
File: components/americommerce/actions/update-customer/update-customer.mjs:89-94
Timestamp: 2024-10-08T16:42:59.225Z
Learning: When defining boolean properties in AmeriCommerce components (e.g., in `update-customer.mjs`), ensure that the label and description are consistent and clearly indicate the intent, especially when using negations like "No Account", to avoid confusion.
Applied to files:
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
📚 Learning: 2025-08-27T17:25:10.425Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 18187
File: packages/connect-react/src/utils/type-guards.ts:23-33
Timestamp: 2025-08-27T17:25:10.425Z
Learning: In the connect-react package, the isOptionWithLabel type guard intentionally restricts value types to string|number for runtime filtering purposes, even though LabelValueOption<T> allows any T. This runtime behavior should be preserved over type safety improvements.
Applied to files:
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
🧬 Code graph analysis (5)
components/google_sheets/actions/add-protected-range/add-protected-range.mjs (1)
components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/set-data-validation/set-data-validation.mjs (2)
components/google_sheets/actions/merge-cells/merge-cells.mjs (2)
request(67-85)response(86-86)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (2)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (2)
request(218-232)response(234-234)components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (2)
request(206-218)response(219-219)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (3)
components/google_sheets/actions/add-conditional-format-rule/add-conditional-format-rule.mjs (4)
rule(136-146)parseRgbColor(148-157)request(206-218)response(219-219)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (2)
request(47-59)response(60-60)
components/google_sheets/actions/merge-cells/merge-cells.mjs (5)
components/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs (2)
request(218-232)response(234-234)components/google_sheets/actions/set-data-validation/set-data-validation.mjs (2)
request(90-117)response(118-118)components/google_sheets/actions/add-protected-range/add-protected-range.mjs (2)
request(81-106)response(107-107)components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (2)
request(47-59)response(60-60)components/google_sheets/google_sheets.app.mjs (4)
startRow(142-142)endRow(143-145)startCol(138-138)endCol(139-141)
⏰ 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 (1)
components/google_sheets/actions/delete-conditional-format-rule/delete-conditional-format-rule.mjs (1)
46-63: LGTM!The delete operation is correctly implemented using the Google Sheets API's
deleteConditionalFormatRulerequest. The index-based deletion is appropriate and the implementation is clean.
components/google_sheets/actions/set-data-validation/set-data-validation.mjs
Show resolved
Hide resolved
...ents/google_sheets/actions/update-conditional-format-rule/update-conditional-format-rule.mjs
Show resolved
Hide resolved
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
5 participants
WHY
Summary by CodeRabbit
New Features
Chores