feat/CR-2025-Added new nlp to copy the csv file path and remove unnecessary code from other files#124
Conversation
WalkthroughThis pull request introduces a new Maven project for capturing file paths from Chrome downloads. It adds multiple Java classes to perform different file operations: capturing CSV and XLSX file paths, interacting with the Chrome downloads page via Selenium, and uploading a file. A utility class is added for file operations, and a properties file is updated with an API key. The new Maven POM file defines project metadata, dependencies, and build plugins for shading and source attachment. Changes
Sequence Diagram(s)sequenceDiagram
participant Action as Capture Action
participant FileUtil as FileUtilities
participant Browser as Chrome Browser
participant FS as Local Filesystem
Action->>FileUtil: copyFileFromDownloads(fileFormat, fileName)
FileUtil->>Browser: Open downloads page
Browser-->>FileUtil: Provide downloaded file info
FileUtil->>FS: Create local copy from file data
FS-->>FileUtil: Return file copy
FileUtil-->>Action: Return local file path
sequenceDiagram
participant Upload as UploadFile Action
participant Element as Target Web Element
Upload->>Element: Send file path for upload
Element-->>Upload: Acknowledge file receipt
Suggested Reviewers
Poem
✨ 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:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
capture_the_downloaded_file_path_from_chrome/pom.xml (2)
42-46: Consider specifying a test scope for TestNG dependencyThe TestNG dependency doesn't have a scope specified, which means it will be included in the runtime classpath. Since this is a testing framework, consider adding
<scope>test</scope>to keep it out of your production code.<dependency> <groupId>org.testng</groupId> <artifactId>testng</artifactId> <version>6.14.3</version> + <scope>test</scope> </dependency>
74-87: Consider configuration for maven-shade-pluginThe current shade plugin configuration doesn't specify which dependencies to include or exclude in the final JAR. Consider adding artifact relocation or filters to avoid potential conflicts with other dependencies in the runtime environment.
<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-shade-plugin</artifactId> <version>3.2.4</version> <executions> <execution> <phase>package</phase> <goals> <goal>shade</goal> </goals> + <configuration> + <createDependencyReducedPom>true</createDependencyReducedPom> + <filters> + <filter> + <artifact>*:*</artifact> + <excludes> + <exclude>META-INF/*.SF</exclude> + <exclude>META-INF/*.DSA</exclude> + <exclude>META-INF/*.RSA</exclude> + </excludes> + </filter> + </filters> + </configuration> </execution> </executions> </plugin>capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/UploadFile.java (2)
31-31: Improve success message with element detailsThe success message doesn't include which element was used for the upload. This information would be helpful for debugging and logging purposes.
- setSuccessMessage("Successfully uploaded the file to the element:"); + setSuccessMessage("Successfully uploaded the file to the element: " + element.getName());
32-36: Enhance error logging with file path informationThe current error message doesn't include the file path that was being uploaded, which would be valuable information for troubleshooting.
}catch (Exception e){ - logger.info("Unable to upload the file"+ ExceptionUtils.getStackTrace(e)); - setErrorMessage("Failed to upload the file::"+e.getMessage()); + logger.info("Unable to upload the file: " + filePath.getValue() + " to element: " + element.getName() + " " + ExceptionUtils.getStackTrace(e)); + setErrorMessage("Failed to upload the file " + filePath.getValue() + " to element " + element.getName() + ": " + e.getMessage()); result = Result.FAILED; }capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/CaptureDownloadedFilePath.java (3)
3-23: Remove unused importsThere are several unused imports in this file. Clean them up to improve code readability and maintainability.
import com.testsigma.sdk.annotation.TestData; import lombok.Data; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.openqa.selenium.JavascriptExecutor; import org.openqa.selenium.NoSuchElementException; -import org.openqa.selenium.WebDriver; -import org.openqa.selenium.support.ui.ExpectedCondition; -import org.openqa.selenium.support.ui.WebDriverWait; import java.io.File; -import java.time.Duration; -import java.util.ArrayList; -import java.util.List; -import java.util.Set;
44-44: Fix missing space in log messageThere's a missing space after "Local path" in the log message, which affects readability.
- logger.info("Local path"+downloadedExcelFile.getAbsolutePath()); + logger.info("Local path: "+downloadedExcelFile.getAbsolutePath());
38-38: Rename variable to match class functionalityThe variable name
pdfAndDocUtilitiessuggests it's for PDF and DOC files, but it's being used for Excel (XLSX) files. Rename the variable to better reflect its usage.- FileUtilities pdfAndDocUtilities = new FileUtilities(driver,logger); + FileUtilities fileUtilities = new FileUtilities(driver,logger);And update all references:
- File downloadedExcelFile = pdfAndDocUtilities.copyFileFromDownloads("xlsx",null); + File downloadedExcelFile = fileUtilities.copyFileFromDownloads("xlsx",null);capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/CaptureFilePath.java (2)
15-21: Consider making fields private for better encapsulation.Currently,
driverandloggerare package-private. Declaring themprivateis a recommended best practice to maintain encapsulation within the class unless there's a specific need for broader access.- WebDriver driver; - Logger logger; + private WebDriver driver; + private Logger logger;
22-47: Reduce duplication by reusingFileUtilities.This method largely replicates logic similar to what appears in
FileUtilities(e.g., opening a new tab, navigating tochrome://downloads/, waiting for the file to finish downloading, etc.). Consider delegating these tasks toFileUtilitiesto avoid code duplication and ease maintenance.capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/CaptureDownloadedCSVFilePath.java (3)
38-38: Rename variable to match the CSV context.Using a variable name
pdfAndDocUtilitiesmight confuse readers since the method call includes"csv". Consider renaming to something more generic or CSV-specific.- FileUtilities pdfAndDocUtilities = new FileUtilities(driver,logger); + FileUtilities fileUtilities = new FileUtilities(driver, logger);
42-43: Rename variable to reflect CSV rather than Excel.
downloadedExcelFileis misleading if referencing a CSV. Rename to provide clearer context.- File downloadedExcelFile = pdfAndDocUtilities.copyFileFromDownloads("csv",null); + File downloadedCsvFile = fileUtilities.copyFileFromDownloads("csv", null);
35-55: Consider adding dedicated unit tests for CSV-file capture.While the logging and runtime variable storage appear correct, there’s no direct test coverage showing that a CSV file is indeed captured and stored as intended. Adding targeted tests will improve reliability.
capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/FileUtilities.java (2)
27-60: Avoid duplicating logic found inCaptureFilePath.The same pattern of opening a new tab, navigating to
chrome://downloads/, and waiting for the file to complete appears across classes. Extract common routines into a shared method or consolidate calls to ensure DRY (Don’t Repeat Yourself) principles.
96-130: Review and test the injection approach for broader compatibility.Using an injected
<input type="file">+ FileReader can be brittle on older Chrome versions or potentially present security concerns. Consider exploring alternative methods (e.g., Chrome DevTools protocol) for robust file retrieval across environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
capture_the_downloaded_file_path_from_chrome/pom.xml(1 hunks)capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/CaptureDownloadedCSVFilePath.java(1 hunks)capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/CaptureDownloadedFilePath.java(1 hunks)capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/CaptureFilePath.java(1 hunks)capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/FileUtilities.java(1 hunks)capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/UploadFile.java(1 hunks)capture_the_downloaded_file_path_from_chrome/src/main/resources/testsigma-sdk.properties(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
capture_the_downloaded_file_path_from_chrome/src/main/resources/testsigma-sdk.properties
1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (5)
capture_the_downloaded_file_path_from_chrome/pom.xml (1)
24-71: LGTM! Dependencies are appropriate for file manipulation with SeleniumThe dependencies include everything needed for file handling in a Selenium environment: Selenium WebDriver, Appium for potential mobile testing, and utility libraries like commons-lang3 for robust exception handling.
capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/CaptureFilePath.java (2)
49-58: Validate first item usage when multiple files may exist.
getDownloadedFileLocalPath()always retrievesitems[0], which may not match the intended file in multi-download scenarios. Consider verifying or parameterizing which downloaded file is retrieved.
60-72: Confirm multi-download scenarios are handled correctly.
isFileDownloaded()flags a download as complete only if no items are in theIN_PROGRESSstate. This can be misleading if multiple downloads occur and some complete first while others remain in progress. Please verify whether the action or test usage disallows parallel downloads.capture_the_downloaded_file_path_from_chrome/src/main/java/com/testsigma/addons/FileUtilities.java (2)
61-73: Confirm concurrency requirements when multiple downloads occur.Similar to
CaptureFilePath, this check assumes only one file is downloading at a time. If multiple downloads run in parallel, “In Progress” state could persist even though the needed file is completed. Verify whether parallel downloads are disallowed or handled elsewhere.
86-95: Validate behavior for filename collisions.
getFilePathByFileNameInDownloadssearches for a substring match on file name. If multiple files contain the same substring, the first match is returned. Verify whether this is acceptable or if further filtering is needed.
| @@ -0,0 +1 @@ | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiI4MjhhM2FhZC1jOTI5LTE3YTUtNDJjMS03MTIyNDhkMGM5NmQiLCJ1bmlxdWVJZCI6IjI2NDEiLCJleHRlcm5hbFRlbmFudElkIjoiMjgxNyJ9.52Gv-nVNtGKTU1i_E6qZrBt-iev8IAlkMvNjf8X6dUwBuT2H28IRy3DH2HL0bK1CmGxBK7XI-AXiwdzavX-1pg No newline at end of file | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security risk: Hardcoded JWT token
The JWT token should not be hardcoded in the properties file as it poses a security risk when committed to version control. Consider using environment variables or secure configuration management instead.
-testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiI4MjhhM2FhZC1jOTI5LTE3YTUtNDJjMS03MTIyNDhkMGM5NmQiLCJ1bmlxdWVJZCI6IjI2NDEiLCJleHRlcm5hbFRlbmFudElkIjoiMjgxNyJ9.52Gv-nVNtGKTU1i_E6qZrBt-iev8IAlkMvNjf8X6dUwBuT2H28IRy3DH2HL0bK1CmGxBK7XI-AXiwdzavX-1pg
+testsigma-sdk.api.key=${TESTSIGMA_API_KEY}Consider implementing this safer approach:
- Update the property to use an environment variable
- Set up documentation on how to configure the environment variable
- Add the properties file to .gitignore to prevent future token commits
🏁 Script executed:
#!/bin/bash
# Check if there are other instances of hardcoded credentials
grep -r --include="*.properties" --include="*.xml" --include="*.java" "eyJhbGciOiJ" .Length of output: 24327
Critical: Remove Hardcoded JWT Tokens from ALL Properties Files
The grep scan revealed hardcoded JWT tokens in numerous testsigma-sdk.properties files across multiple modules, not just in the file under review. Hardcoding these secrets poses a critical security risk. Please address this by refactoring all such files to reference an environment variable.
-
Affected Files:
capture_the_downloaded_file_path_from_chrome/src/main/resources/testsigma-sdk.properties(as originally noted)- Plus over 50 additional modules (e.g.,
Add_character_between_string,JsonUtils_982187920716202948315267669918166316366,PickCurrentDateByZone, etc.) contain similar hardcoded tokens.
-
Action Items:
- Replace the hardcoded JWT token with an environment variable (e.g.,
${TESTSIGMA_API_KEY}) in all instances. - Update documentation to describe how to configure the
TESTSIGMA_API_KEYenvironment variable. - Consider adding these properties files, or at least the sensitive entries, to
.gitignoreor otherwise manage configuration securely.
- Replace the hardcoded JWT token with an environment variable (e.g.,
Example Diff:
- testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiI4MjhhM2FhZC1jOTI5LTE3YTUtNDJjMS03MTIyNDhkMGM5NmQiLCJ1bmlxdWVJZCI6IjI2NDEiLCJleHRlcm5hbFRlbmFudElkIjoiMjgxNyJ9.52Gv-nVNtGKTU1i_E6qZrBt-iev8IAlkMvNjf8X6dUwBuT2H28IRy3DH2HL0bK1CmGxBK7XI-AXiwdzavX-1pg
+ testsigma-sdk.api.key=${TESTSIGMA_API_KEY}📝 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.
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiI4MjhhM2FhZC1jOTI5LTE3YTUtNDJjMS03MTIyNDhkMGM5NmQiLCJ1bmlxdWVJZCI6IjI2NDEiLCJleHRlcm5hbFRlbmFudElkIjoiMjgxNyJ9.52Gv-nVNtGKTU1i_E6qZrBt-iev8IAlkMvNjf8X6dUwBuT2H28IRy3DH2HL0bK1CmGxBK7XI-AXiwdzavX-1pg | |
| testsigma-sdk.api.key=${TESTSIGMA_API_KEY} |
🧰 Tools
🪛 Gitleaks (8.21.2)
1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
| public class UploadFile extends WebAction { | ||
| @TestData(reference = "File-Path",isRuntimeVariable = true) | ||
| private com.testsigma.sdk.TestData filePath; | ||
|
|
||
| @Element(reference = "element-name") | ||
| private com.testsigma.sdk.Element element; | ||
|
|
||
|
|
||
| @Override | ||
| protected Result execute() throws NoSuchElementException { | ||
|
|
||
| Result result = Result.SUCCESS; | ||
| try{ | ||
| getElement(element).sendKeys(filePath.getValue().toString()); | ||
| setSuccessMessage("Successfully uploaded the file to the element:"); | ||
| }catch (Exception e){ | ||
| logger.info("Unable to upload the file"+ ExceptionUtils.getStackTrace(e)); | ||
| setErrorMessage("Failed to upload the file::"+e.getMessage()); | ||
| result = Result.FAILED; | ||
| } | ||
|
|
||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation for file existence
The current implementation doesn't check if the file actually exists before attempting to upload it. Adding this validation would prevent potential exceptions and provide clearer error messages.
protected Result execute() throws NoSuchElementException {
Result result = Result.SUCCESS;
try{
+ String filePathStr = filePath.getValue().toString();
+ File file = new File(filePathStr);
+ if (!file.exists()) {
+ setErrorMessage("File does not exist: " + filePathStr);
+ return Result.FAILED;
+ }
- getElement(element).sendKeys(filePath.getValue().toString());
+ getElement(element).sendKeys(filePathStr);
setSuccessMessage("Successfully uploaded the file to the element:");
}catch (Exception e){📝 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.
| public class UploadFile extends WebAction { | |
| @TestData(reference = "File-Path",isRuntimeVariable = true) | |
| private com.testsigma.sdk.TestData filePath; | |
| @Element(reference = "element-name") | |
| private com.testsigma.sdk.Element element; | |
| @Override | |
| protected Result execute() throws NoSuchElementException { | |
| Result result = Result.SUCCESS; | |
| try{ | |
| getElement(element).sendKeys(filePath.getValue().toString()); | |
| setSuccessMessage("Successfully uploaded the file to the element:"); | |
| }catch (Exception e){ | |
| logger.info("Unable to upload the file"+ ExceptionUtils.getStackTrace(e)); | |
| setErrorMessage("Failed to upload the file::"+e.getMessage()); | |
| result = Result.FAILED; | |
| } | |
| return result; | |
| } | |
| public class UploadFile extends WebAction { | |
| @TestData(reference = "File-Path",isRuntimeVariable = true) | |
| private com.testsigma.sdk.TestData filePath; | |
| @Element(reference = "element-name") | |
| private com.testsigma.sdk.Element element; | |
| @Override | |
| protected Result execute() throws NoSuchElementException { | |
| Result result = Result.SUCCESS; | |
| try{ | |
| String filePathStr = filePath.getValue().toString(); | |
| File file = new File(filePathStr); | |
| if (!file.exists()) { | |
| setErrorMessage("File does not exist: " + filePathStr); | |
| return Result.FAILED; | |
| } | |
| getElement(element).sendKeys(filePathStr); | |
| setSuccessMessage("Successfully uploaded the file to the element:"); | |
| }catch (Exception e){ | |
| logger.info("Unable to upload the file" + ExceptionUtils.getStackTrace(e)); | |
| setErrorMessage("Failed to upload the file::" + e.getMessage()); | |
| result = Result.FAILED; | |
| } | |
| return result; | |
| } | |
| } |
| @Data | ||
| @Action(actionText = "Copy the latest downloaded file and save the file path in runtime-variable variable-name", | ||
| description = "This addon will Copy the latest downloaded file and save the file path in runtime-variable variable-name", | ||
| applicationType = ApplicationType.WEB) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename class to match functionality
The class name CaptureDownloadedFilePath suggests it works with any file type, but the implementation specifically handles XLSX files. Consider renaming the class to CaptureDownloadedExcelFilePath to accurately reflect its functionality.
@Data
@Action(actionText = "Copy the latest downloaded file and save the file path in runtime-variable variable-name",
description = "This addon will Copy the latest downloaded file and save the file path in runtime-variable variable-name",
applicationType = ApplicationType.WEB)
-public class CaptureDownloadedFilePath extends WebAction {
+public class CaptureDownloadedExcelFilePath extends WebAction {Also update the @action annotation to specify that it handles Excel files:
@Data
-@Action(actionText = "Copy the latest downloaded file and save the file path in runtime-variable variable-name",
- description = "This addon will Copy the latest downloaded file and save the file path in runtime-variable variable-name",
+@Action(actionText = "Copy the latest downloaded Excel file and save the file path in runtime-variable variable-name",
+ description = "This addon will copy the latest downloaded Excel (.xlsx) file and save the file path in runtime-variable variable-name",📝 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.
| @Data | |
| @Action(actionText = "Copy the latest downloaded file and save the file path in runtime-variable variable-name", | |
| description = "This addon will Copy the latest downloaded file and save the file path in runtime-variable variable-name", | |
| applicationType = ApplicationType.WEB) | |
| @Data | |
| @Action(actionText = "Copy the latest downloaded Excel file and save the file path in runtime-variable variable-name", | |
| description = "This addon will copy the latest downloaded Excel (.xlsx) file and save the file path in runtime-variable variable-name", | |
| applicationType = ApplicationType.WEB) | |
| public class CaptureDownloadedExcelFilePath extends WebAction { | |
| // Rest of the class implementation | |
| } |
|
|
||
| try { | ||
| logger.info("Initiated execution"); | ||
| File downloadedExcelFile = pdfAndDocUtilities.copyFileFromDownloads("xlsx",null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hardcoded file extension limits functionality
The file extension "xlsx" is hardcoded, limiting the functionality to Excel files only. Consider making the file extension a configurable parameter to increase the reusability of this class.
+ @TestData(reference = "file-extension")
+ private com.testsigma.sdk.TestData fileExtension;
@Override
protected Result execute() throws NoSuchElementException {
Result result = Result.SUCCESS;
FileUtilities pdfAndDocUtilities = new FileUtilities(driver,logger);
try {
logger.info("Initiated execution");
- File downloadedExcelFile = pdfAndDocUtilities.copyFileFromDownloads("xlsx",null);
+ String extension = fileExtension != null ? fileExtension.getValue().toString() : "xlsx";
+ File downloadedFile = pdfAndDocUtilities.copyFileFromDownloads(extension, null);Then update the @action annotation to include the new parameter:
@Data
-@Action(actionText = "Copy the latest downloaded file and save the file path in runtime-variable variable-name",
+@Action(actionText = "Copy the latest downloaded file with extension file-extension and save the file path in runtime-variable variable-name",
JIRA
https://testsigma.atlassian.net/browse/CR-2025
Fix
Added new nlp to copy the csv file path and remove unnecessary code from other files
Summary by CodeRabbit