feat/CUS-12907-Write cell value to existing or new sheet by name in Excel#399
Conversation
📝 WalkthroughWalkthroughMaven SDK version is bumped to 1.2.26_cloud with project version 1.0.25. Browser detection in ExcelUtilitiesFactory is enhanced to read Capabilities first and fall back to JavaScript user-agent inspection. WriteCellValueInNewSheetFilePath gains runtime variable output support to emit the updated file path, and success messages are refined. ChangesExcel Actions Cloud Dependencies and Runtime Features
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java (3)
56-57:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove double semicolons causing syntax errors.
Both lines have double semicolons at the end, which will cause compilation failures.
🐛 Proposed fix
- int rowIndex = Integer.parseInt(testData1.getValue().toString());; - int columnIndex = Integer.parseInt(testData2.getValue().toString());; + int rowIndex = Integer.parseInt(testData1.getValue().toString()); + int columnIndex = Integer.parseInt(testData2.getValue().toString());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java` around lines 56 - 57, Remove the stray double semicolons that cause compilation errors in WriteCellValueInNewSheetFilePath.java: locate the lines that assign rowIndex and columnIndex (using Integer.parseInt(testData1.getValue().toString()) and Integer.parseInt(testData2.getValue().toString())) and replace the trailing ";;" with a single semicolon so each statement ends with only one semicolon.
22-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix duplicate word in action text.
The action text contains "data data-value" which appears to have a duplicate word. This may confuse users.
📝 Proposed fix
-@Action(actionText = "Write the data data-value into the Excelfile excel-path with Cell value rowNo,columnNo in new sheet name sheet-name and store the path in runtime variable variable-name(supports upload section)", +@Action(actionText = "Write the data-value into the Excelfile excel-path with Cell value rowNo,columnNo in new sheet name sheet-name and store the path in runtime variable variable-name(supports upload section)",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java` at line 22, Update the Action annotation on class WriteCellValueInNewSheetFilePath to remove the duplicated word "data" in the actionText attribute; locate the `@Action` annotation string (the value starting with "Write the data data-value ...") and change it to "Write the data-value into the Excelfile excel-path with Cell value rowNo,columnNo in new sheet name sheet-name and store the path in runtime variable variable-name(supports upload section)" so the action text is clear and not duplicated.
108-121:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftRuntime data and success message must only be set when the operation succeeds.
The runtime data (lines 108-109) and success message (lines 118-121) are set unconditionally, even when the write operation fails. If the
FileOutputStreamwrite fails (lines 100-105),resultis set toFAILEDand an error message is set, but the code continues to populate runtime data and set a success message. This creates an inconsistent state where both error and success messages are present, and the runtime variable contains a path that may point to an incomplete or corrupt file.🐛 Proposed fix
Move runtime data population and success message construction inside a check for success:
try (FileOutputStream fileOut = new FileOutputStream(excelFile)) { workbook.write(fileOut); } catch (IOException e) { String errorMessage = ExceptionUtils.getStackTrace(e); result = com.testsigma.sdk.Result.FAILED; setErrorMessage(errorMessage); logger.warn(errorMessage); + return result; } System.out.println("Excel file path: " + excelFile.getAbsolutePath()); runTimeData.setKey(variableName.getValue().toString()); runTimeData.setValue(excelFile.getAbsolutePath()); logger.info("Data written successfully to Excel file.File path: " + excelFile.getAbsolutePath()); + + String sheetMsg = sheetExisted + ? "Sheet '" + newsheetName + "' already exists. Updated the data successfully." + : "Sheet '" + newsheetName + "' did not exist. Created new sheet and written data successfully."; + setSuccessMessage(sheetMsg + "<br>File path: " + excelFile.getAbsolutePath()); + } catch (IOException e) { String errorMessage = ExceptionUtils.getStackTrace(e); result = com.testsigma.sdk.Result.FAILED; setErrorMessage(errorMessage); logger.warn(errorMessage); + return result; } - String sheetMsg = sheetExisted - ? "Sheet '" + newsheetName + "' already exists. Updated the data successfully." - : "Sheet '" + newsheetName + "' did not exist. Created new sheet and written data successfully."; - setSuccessMessage(sheetMsg + "<br>File path: " + excelFile.getAbsolutePath()); - return result;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java` around lines 108 - 121, Runtime data and success message are being set even when the write operation fails; ensure they are only set on success. After the try/catch that performs the FileOutputStream write in WriteCellValueInNewSheetFilePath (where result is set to FAILED and setErrorMessage(...) is called on exception), move the runTimeData.setKey(...), runTimeData.setValue(...), logger.info(...) and the setSuccessMessage(...) (which uses sheetMsg and newsheetName) inside the success path (i.e., only when result != com.testsigma.sdk.Result.FAILED or when the try completed without exception) so they are not executed when an IOException occurs; keep the catch logging and setErrorMessage flow unchanged. Ensure sheetMsg is computed and used only after confirming the write succeeded.excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java (1)
55-57:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRemove double semicolons causing syntax errors.
All three lines have double semicolons at the end, which will cause compilation failures.
🐛 Proposed fix
- int rowIndex = Integer.parseInt(testData1.getValue().toString());; - int columnIndex = Integer.parseInt(testData2.getValue().toString());; - int sheetIndex = Integer.parseInt(testData4.getValue().toString());; + int rowIndex = Integer.parseInt(testData1.getValue().toString()); + int columnIndex = Integer.parseInt(testData2.getValue().toString()); + int sheetIndex = Integer.parseInt(testData4.getValue().toString());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java` around lines 55 - 57, Remove the stray double semicolons at the end of the integer parsing lines in WriteCellvalueWithSheetFilepath (variables rowIndex, columnIndex, sheetIndex); edit the assignments that call Integer.parseInt(testData1.getValue().toString()), Integer.parseInt(testData2.getValue().toString()), and Integer.parseInt(testData4.getValue().toString()) to end with a single semicolon so the declarations compile without syntax errors.
🧹 Nitpick comments (1)
excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java (1)
82-82: ⚡ Quick winConsider validating sheet index bounds to provide clearer error messages.
The code calls
getSheetAt(sheetIndex)without verifying thatsheetIndexis within the valid range[0, workbook.getNumberOfSheets()). While Apache POI will throw anIllegalArgumentExceptionif the index is out of bounds, adding explicit validation would provide a more user-friendly error message.♻️ Proposed validation
try (FileInputStream fis = new FileInputStream(excelFile); Workbook workbook = new XSSFWorkbook(fis)) { + int totalSheets = workbook.getNumberOfSheets(); + if (sheetIndex < 0 || sheetIndex >= totalSheets) { + String errorMessage = "Invalid sheet index: " + sheetIndex + + ". Valid range is 0 to " + (totalSheets - 1); + setErrorMessage(errorMessage); + logger.warn(errorMessage); + return com.testsigma.sdk.Result.FAILED; + } Sheet sheet = workbook.getSheetAt(sheetIndex);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java` at line 82, The call Sheet sheet = workbook.getSheetAt(sheetIndex) should validate sheetIndex before calling getSheetAt; check that sheetIndex is >= 0 and < workbook.getNumberOfSheets() (use workbook.getNumberOfSheets()) and if out of range throw or log a clear, user-friendly IllegalArgumentException or Testsigma exception referencing the invalid index and valid range; update the method containing this line (where sheetIndex and workbook are used) to perform this bounds check and return/abort with the improved error message instead of letting POI throw its default exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.java`:
- Around line 56-57: Remove the stray double semicolons that cause compilation
errors in WriteCellValueInNewSheetFilePath.java: locate the lines that assign
rowIndex and columnIndex (using
Integer.parseInt(testData1.getValue().toString()) and
Integer.parseInt(testData2.getValue().toString())) and replace the trailing ";;"
with a single semicolon so each statement ends with only one semicolon.
- Line 22: Update the Action annotation on class
WriteCellValueInNewSheetFilePath to remove the duplicated word "data" in the
actionText attribute; locate the `@Action` annotation string (the value starting
with "Write the data data-value ...") and change it to "Write the data-value
into the Excelfile excel-path with Cell value rowNo,columnNo in new sheet name
sheet-name and store the path in runtime variable variable-name(supports upload
section)" so the action text is clear and not duplicated.
- Around line 108-121: Runtime data and success message are being set even when
the write operation fails; ensure they are only set on success. After the
try/catch that performs the FileOutputStream write in
WriteCellValueInNewSheetFilePath (where result is set to FAILED and
setErrorMessage(...) is called on exception), move the runTimeData.setKey(...),
runTimeData.setValue(...), logger.info(...) and the setSuccessMessage(...)
(which uses sheetMsg and newsheetName) inside the success path (i.e., only when
result != com.testsigma.sdk.Result.FAILED or when the try completed without
exception) so they are not executed when an IOException occurs; keep the catch
logging and setErrorMessage flow unchanged. Ensure sheetMsg is computed and used
only after confirming the write succeeded.
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java`:
- Around line 55-57: Remove the stray double semicolons at the end of the
integer parsing lines in WriteCellvalueWithSheetFilepath (variables rowIndex,
columnIndex, sheetIndex); edit the assignments that call
Integer.parseInt(testData1.getValue().toString()),
Integer.parseInt(testData2.getValue().toString()), and
Integer.parseInt(testData4.getValue().toString()) to end with a single semicolon
so the declarations compile without syntax errors.
---
Nitpick comments:
In
`@excelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java`:
- Line 82: The call Sheet sheet = workbook.getSheetAt(sheetIndex) should
validate sheetIndex before calling getSheetAt; check that sheetIndex is >= 0 and
< workbook.getNumberOfSheets() (use workbook.getNumberOfSheets()) and if out of
range throw or log a clear, user-friendly IllegalArgumentException or Testsigma
exception referencing the invalid index and valid range; update the method
containing this line (where sheetIndex and workbook are used) to perform this
bounds check and return/abort with the improved error message instead of letting
POI throw its default exception.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9df3a053-75e5-48d3-bc92-89c105cc8d63
📒 Files selected for processing (4)
excelActions_cloud/pom.xmlexcelActions_cloud/src/main/java/com/testsigma/addons/util/ExcelUtilitiesFactory.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellValueInNewSheetFilePath.javaexcelActions_cloud/src/main/java/com/testsigma/addons/web/WriteCellvalueWithSheetFilepath.java
Publish this addon as PUBLIC
Addon Name: ExcelActions_cloud
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira : https://testsigma.atlassian.net/browse/CUS-12907
Write cell value to existing or new sheet by name in Excel
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores