Merge official to master#4405
Conversation
merging master to official 202.5 RC
updating version
…onRogers Enhancement Excel Action and Refactoring
… length to subfolder creation
Handled the file name length while api model configuration, and added…
Handled the issue of value expression getting truncated
…un-on-Release-Branches Update codeql-analysis.yml
…e-removal Revert
…dOption BugFix/StoreToHiddenOption
As stated in bug: modifed error log to "Error" instead of "Warning"
Bug fixes for excel action
…ssue push latest fixes
WalkthroughThis PR introduces Excel action data selection modes (ByParameters and ByCellAddress), restricted mapping types for data mapping controls, comprehensive HTML report templates with placeholder-based rendering, supporting JavaScript libraries (jQuery, Bootstrap, chart rendering), Excel cell write operations, and coordinated UI updates across multiple editor pages and configuration surfaces. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ActExcelEditPage
participant UCDataMapping as UCDataMapping<br/>(Control)
participant ActExcel
participant ExcelNPOIOperations
rect rgb(220, 240, 255)
Note over User,ExcelNPOIOperations: Excel Data Selection Flow
User->>ActExcelEditPage: Select Data Selection Mode<br/>(ByParameters or ByCellAddress)
activate ActExcelEditPage
ActExcelEditPage->>ActExcelEditPage: DataSelection_Changed()
ActExcelEditPage->>ActExcel: Set DataSelectionMethod<br/>Update UI properties
ActExcelEditPage->>ActExcelEditPage: UpdateVisibility()<br/>Show/Hide relevant panels
deactivate ActExcelEditPage
User->>ActExcelEditPage: Trigger ReadData<br/>(with cell address or parameters)
activate ActExcelEditPage
ActExcelEditPage->>ActExcel: ReadData/ReadCellData
activate ActExcel
alt ByCellAddress Mode
ActExcel->>ActExcel: ParseCellAddress()<br/>Convert address to coords
ActExcel->>ExcelNPOIOperations: Read cell/range data
else ByParameters Mode
ActExcel->>ExcelNPOIOperations: Read sheet data<br/>by parameters
end
activate ExcelNPOIOperations
ExcelNPOIOperations-->>ActExcel: Cell data + addresses
deactivate ExcelNPOIOperations
ActExcel-->>ActExcelEditPage: OutputValues with<br/>CellAddress (if PullCellAddress)
deactivate ActExcel
deactivate ActExcelEditPage
end
sequenceDiagram
participant Page as Action/Config Page
participant UCDataMapping
participant Restriction as Type<br/>Restriction Engine
rect rgb(240, 220, 255)
Note over Page,Restriction: Mapping Type Restriction Flow
Page->>UCDataMapping: Initialize with<br/>RestrictedMappingTypes
activate UCDataMapping
UCDataMapping->>Restriction: OnRestrictedMappingTypesChanged()
activate Restriction
Restriction->>Restriction: ApplyTypeRestrictions()
Restriction->>Restriction: Prune restricted options<br/>from combo box
Restriction->>Restriction: Compute fallback value<br/>(prefer None)
Restriction-->>UCDataMapping: Update MappedType<br/>& refresh UI
deactivate Restriction
deactivate UCDataMapping
User->>UCDataMapping: Select restricted<br/>mapping type
activate UCDataMapping
UCDataMapping->>Restriction: Validate selection<br/>against RestrictedMappingTypes
alt Type is Restricted
Restriction-->>UCDataMapping: Revert & show warning
UCDataMapping-->>User: Error message:<br/>Type not allowed
else Type is Allowed
UCDataMapping-->>User: Accept & apply mapping
end
deactivate UCDataMapping
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 40
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (16)
Ginger/Ginger/Reports/GingerExecutionReport/Images/GingerLogo.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/Reports/GingerExecutionReport/PreviewDummyReport/RunSet.zipis excluded by!**/*.zip,!**/*.zipGinger/Ginger/Reports/GingerExecutionReport/assets/fonts/SourceSansPro-Regular.ttfis excluded by!**/*.ttf,!**/*.ttfGinger/Ginger/Reports/GingerExecutionReport/assets/fonts/fontawesome-webfont.eotis excluded by!**/*.eot,!**/*.eotGinger/Ginger/Reports/GingerExecutionReport/assets/fonts/fontawesome-webfont.ttfis excluded by!**/*.ttf,!**/*.ttfGinger/Ginger/Reports/GingerExecutionReport/assets/img/@BeatLogo - Copy.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/Reports/GingerExecutionReport/assets/img/@BeatLogo.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/Reports/GingerExecutionReport/assets/img/@Ginger - Copy.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/Reports/GingerExecutionReport/assets/img/@Ginger.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/Reports/GingerExecutionReport/assets/img/@item_next.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/Reports/GingerExecutionReport/assets/img/@item_prev.pngis excluded by!**/*.png,!**/*.pngGinger/Ginger/Reports/GingerExecutionReport/assets/img/comments-icon.jpgis excluded by!**/*.jpg,!**/*.jpgGinger/Ginger/Reports/GingerExecutionReport/assets/js/Recharts.min.jsis excluded by!**/*.min.js,!**/*.min.jsGinger/Ginger/Reports/GingerExecutionReport/assets/js/prop-types.min.jsis excluded by!**/*.min.js,!**/*.min.jsGinger/Ginger/Reports/GingerExecutionReport/assets/js/react-dom.min.jsis excluded by!**/*.min.js,!**/*.min.jsGinger/Ginger/Reports/GingerExecutionReport/assets/js/react-with-addons.min.jsis excluded by!**/*.min.js,!**/*.min.js
📒 Files selected for processing (31)
Ginger/Ginger/Actions/ActionEditPage.xaml.csGinger/Ginger/Actions/ActionEditPages/ActDataSourcePage.xaml.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xamlGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/Ginger/Reports/GingerExecutionReport/ActionReport.htmlGinger/Ginger/Reports/GingerExecutionReport/ActivityGroupReport.htmlGinger/Ginger/Reports/GingerExecutionReport/ActivityReport.htmlGinger/Ginger/Reports/GingerExecutionReport/BusinessFlowReport.htmlGinger/Ginger/Reports/GingerExecutionReport/GingerExecutionReport.htmlGinger/Ginger/Reports/GingerExecutionReport/GingerRunnerReport.htmlGinger/Ginger/Reports/GingerExecutionReport/RunSetReport.htmlGinger/Ginger/Reports/GingerExecutionReport/assets/css/bootstrap.cssGinger/Ginger/Reports/GingerExecutionReport/assets/css/font-awesome.cssGinger/Ginger/Reports/GingerExecutionReport/assets/css/styles.cssGinger/Ginger/Reports/GingerExecutionReport/assets/js/babel.jsGinger/Ginger/Reports/GingerExecutionReport/assets/js/bootstrap.jsGinger/Ginger/Reports/GingerExecutionReport/assets/js/circlechart.jsGinger/Ginger/Reports/GingerExecutionReport/assets/js/donutchart.jsGinger/Ginger/Reports/GingerExecutionReport/assets/js/jquery.jsGinger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.csGinger/Ginger/UserControlsLib/UCDataMapping.xaml.csGinger/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.csGinger/GingerCore/GeneralLib/InputBoxWindow.xamlGinger/GingerCoreCommon/GingerCoreCommon.csprojGinger/GingerCoreCommon/InterfacesLib/IExcelOperations.csGinger/GingerCoreNET/ActionsLib/ActExcel.csGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.csGinger/GingerCoreNETUnitTest/LinuxTransformationTests/ActExcelTests.csGinger/GingerCoreTest/GingerCoreTest.csprojGinger/GingerTest/GingerTest.csproj
🧰 Additional context used
📓 Path-based instructions (3)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
Reporter.ToLog(eLogLevel.ERROR, message)for logging errors
Files:
Ginger/GingerCoreNETUnitTest/LinuxTransformationTests/ActExcelTests.csGinger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.csGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/GingerCoreCommon/InterfacesLib/IExcelOperations.csGinger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.csGinger/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.csGinger/Ginger/Actions/ActionEditPages/ActDataSourcePage.xaml.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/Ginger/UserControlsLib/UCDataMapping.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.csGinger/Ginger/Actions/ActionEditPage.xaml.cs
**/*.csproj
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Target framework should be .NET 8.0 with Windows 10.0.17763.0 compatibility
Files:
Ginger/GingerCoreCommon/GingerCoreCommon.csprojGinger/GingerCoreTest/GingerCoreTest.csprojGinger/GingerTest/GingerTest.csproj
**/{Ginger,GingerCore}/**/{*Page,*Window}.xaml.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name pages and windows following the pattern:
{Feature}Pageor{Feature}Window
Files:
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.csGinger/Ginger/Actions/ActionEditPages/ActDataSourcePage.xaml.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/Ginger/Actions/ActionEditPage.xaml.cs
🧠 Learnings (39)
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Name actions following the pattern: `Act{PlatformType}{ActionType}` (e.g., `ActBrowserElement`, `ActConsoleCommand`)
Applied to files:
Ginger/GingerCoreNETUnitTest/LinuxTransformationTests/ActExcelTests.csGinger/Ginger/Reports/GingerExecutionReport/ActionReport.htmlGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.csGinger/Ginger/Actions/ActionEditPage.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Actions should inherit from `Act` class and implement platform-specific logic
Applied to files:
Ginger/GingerCoreNETUnitTest/LinuxTransformationTests/ActExcelTests.csGinger/Ginger/Actions/ActionEditPage.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/*Test.cs : Test files should end with `Test.cs` and use MSTest framework with `[TestClass]` and `[TestMethod]` attributes
Applied to files:
Ginger/GingerCoreNETUnitTest/LinuxTransformationTests/ActExcelTests.cs
📚 Learning: 2025-12-18T05:29:42.896Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4385
File: Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs:468-475
Timestamp: 2025-12-18T05:29:42.896Z
Learning: In the Ginger codebase, assume file extensions are lowercase (e.g., .xlsx, .xls). Do not perform case-insensitive extension checks (such as StringComparison.OrdinalIgnoreCase) when using EndsWith() or similar methods to verify file extensions; rely on lowercase comparisons or convert to lowercase first. This ensures consistency and avoids unnecessary case-insensitive comparisons across C# source files.
Applied to files:
Ginger/GingerCoreNETUnitTest/LinuxTransformationTests/ActExcelTests.csGinger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.csGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/GingerCoreCommon/InterfacesLib/IExcelOperations.csGinger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.csGinger/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.csGinger/Ginger/Actions/ActionEditPages/ActDataSourcePage.xaml.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/Ginger/UserControlsLib/UCDataMapping.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.csGinger/Ginger/Actions/ActionEditPage.xaml.cs
📚 Learning: 2025-08-25T08:48:11.915Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4281
File: Ginger/GingerCore/GingerCore.csproj:538-538
Timestamp: 2025-08-25T08:48:11.915Z
Learning: In the Ginger codebase, the OWASPZAPDotNetAPI package reference in GingerCore.csproj is required for architectural reasons, even when direct usage is not immediately visible in the codebase. This was confirmed by AmanPrasad43 during the security testing feature implementation.
Applied to files:
Ginger/GingerCoreCommon/GingerCoreCommon.csprojGinger/GingerCoreTest/GingerCoreTest.csprojGinger/GingerTest/GingerTest.csprojGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
📚 Learning: 2025-01-16T04:09:15.662Z
Learnt from: IamRanjeetSingh
Repo: Ginger-Automation/Ginger PR: 4058
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:648-649
Timestamp: 2025-01-16T04:09:15.662Z
Learning: The Ginger project primarily targets .NET 8.0, which supports C# 12 features including collection expressions with spread operator.
Applied to files:
Ginger/GingerCoreCommon/GingerCoreCommon.csproj
📚 Learning: 2025-03-20T11:10:30.816Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:30.816Z
Learning: The `Amdocs.Ginger.Common.SourceControlLib` namespace is required in files that reference the `GingerSolution` class for source control operations in the Ginger automation framework.
Applied to files:
Ginger/GingerCoreCommon/GingerCoreCommon.csprojGinger/GingerCoreTest/GingerCoreTest.csprojGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/GingerTest/GingerTest.csprojGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/Ginger.PluginPackage.json : Plugin packages should be defined by `Ginger.PluginPackage.json` manifest
Applied to files:
Ginger/GingerCoreCommon/GingerCoreCommon.csproj
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/*.csproj : Target framework should be .NET 8.0 with Windows 10.0.17763.0 compatibility
Applied to files:
Ginger/GingerCoreCommon/GingerCoreCommon.csproj
📚 Learning: 2025-04-18T06:04:28.309Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4180
File: Ginger/Ginger/SourceControl/CreateNewBranch.xaml.cs:54-58
Timestamp: 2025-04-18T06:04:28.309Z
Learning: In the Ginger application, password binding for PasswordBox controls uses PasswordBox.PasswordCharProperty in the BindingHandler.ObjFieldBinding call, followed by a direct assignment to ensure the password is correctly set. This pattern is intentional and working as expected despite appearing unconventional from a standard WPF perspective.
Applied to files:
Ginger/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 Learning: 2025-12-18T05:19:56.687Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4385
File: Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml:0-0
Timestamp: 2025-12-18T05:19:56.687Z
Learning: In XAML DataTriggers, enum properties can be compared to string values because WPF automatically converts enums to strings during evaluation. This pattern is valid for XAML files like ActExcelEditPage.xaml where eExcelActionType is compared to string literals such as "ReadCellByIndex" or "GetSheetDetails". Apply this guideline broadly to XAML files that bind to enum properties; verify that such comparisons behave as expected in the specific UI components and avoid relying on string literals if the enum values change.
Applied to files:
Ginger/GingerCore/GeneralLib/InputBoxWindow.xamlGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml
📚 Learning: 2024-09-16T16:34:20.667Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 3909
File: Ginger/Ginger/RunSetPageLib/NewRunSetPage.xaml.cs:2328-2346
Timestamp: 2024-09-16T16:34:20.667Z
Learning: In `NewRunSetPage.xaml.cs`, the methods `GenerateHTMLReportFromRemote()` and `GenerateHTMLReportFromLocal()` are kept separate because the application first tries to load remote data, and if not found, it falls back to loading local data.
Applied to files:
Ginger/Ginger/Reports/GingerExecutionReport/GingerRunnerReport.htmlGinger/Ginger/Reports/GingerExecutionReport/GingerExecutionReport.htmlGinger/Ginger/Reports/GingerExecutionReport/ActivityReport.htmlGinger/Ginger/Reports/GingerExecutionReport/ActionReport.htmlGinger/Ginger/Reports/GingerExecutionReport/ActivityGroupReport.htmlGinger/Ginger/Reports/GingerExecutionReport/BusinessFlowReport.htmlGinger/Ginger/Reports/GingerExecutionReport/RunSetReport.html
📚 Learning: 2024-10-28T11:17:30.080Z
Learnt from: IamRanjeetSingh
Repo: Ginger-Automation/Ginger PR: 3971
File: Ginger/GingerCoreNET/External/Katalon/Conversion/KatalonWebElementEntityToHTMLElementInfoConverter.cs:223-225
Timestamp: 2024-10-28T11:17:30.080Z
Learning: In the `Ginger/GingerCoreNET/External/Katalon/Conversion/KatalonWebElementEntityToHTMLElementInfoConverter.cs` file, when optional web element properties like `id` or `name` are not found during conversion, we should log warnings at `eLogLevel.WARN` level, as per the team's preference.
Applied to files:
Ginger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.cs
📚 Learning: 2025-09-13T07:36:47.950Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4301
File: Ginger/GingerCoreCommon/WorkSpaceLib/Solution.cs:499-503
Timestamp: 2025-09-13T07:36:47.950Z
Learning: In the Ginger project, CLI mode can be detected using WorkSpace.Instance.GingerCLIMode enum instead of checking if UserProfile is null. The user AmanPrasad43 added a UserProfile null check in Solution.cs PostDeserialization method to avoid creating GingerPlay configuration during CLI execution, which is a valid concern for CLI scenarios.
Applied to files:
Ginger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: IamRanjeetSingh
Repo: Ginger-Automation/Ginger PR: 3488
File: Ginger/GingerCoreNET/DataSource/LiteDB.cs:53-60
Timestamp: 2024-07-26T22:04:12.930Z
Learning: User IamRanjeetSingh has reviewed and accepted the side effects of triggering `TryUpgradeDataFile` upon setting `FileFullPath` in `GingerLiteDB` class.
Applied to files:
Ginger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 0
File: :0-0
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The `WorkSpace.Instance.Solution.GetSolutionTargetApplications()` method in the Ginger project is designed to always return a value, making additional error handling unnecessary according to the project's contributors.
Applied to files:
Ginger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.cs
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 0
File: :0-0
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The error handling for the `GetTestCaseFromActivityGroup` method in `ExportToRQM.cs` has been improved to handle cases where an activity group does not have any activities more gracefully.
Applied to files:
Ginger/Ginger/Reports/GingerExecutionReport/ActivityGroupReport.html
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it provides access to the `ObservableList<>` class which is used throughout the file for collections of RunSetConfig, AnalyzerItemBase, and ApplicationPOMModel objects.
Applied to files:
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.cs
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it imports the ObservableList<> class which is used throughout the file.
Applied to files:
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.cs
📚 Learning: 2025-06-16T10:37:13.073Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T10:37:13.073Z
Learning: In Ginger codebase, both `using amdocs.ginger.GingerCoreNET;` and `using Amdocs.Ginger.CoreNET;` are valid and serve different purposes. The first (lowercase) contains the WorkSpace class and related workspace functionality, while the second (proper case) contains drivers like GenericAppiumDriver and other core functionality. Both may be required in files that use types from both namespaces.
Applied to files:
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.cs
📚 Learning: 2025-04-01T06:48:16.412Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4150
File: Ginger/GingerCoreNET/Application Models/Delta/PomDelta/MultiPomRunSetMapping.cs:28-28
Timestamp: 2025-04-01T06:48:16.412Z
Learning: The property in MultiPomRunSetMapping class was renamed from 'runSetConfig' to 'RunSetConfig' to follow C# PascalCase naming conventions for public properties.
Applied to files:
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.cs
📚 Learning: 2025-04-01T11:43:47.188Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4150
File: Ginger/GingerCoreNET/Application Models/Delta/PomDelta/MultiPomRunSetMapping.cs:38-39
Timestamp: 2025-04-01T11:43:47.188Z
Learning: The property in MultiPomRunSetMapping class was renamed from 'applicationPOMModels' to 'ApplicationPOMModels' to follow C# PascalCase naming conventions for public properties.
Applied to files:
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.cs
📚 Learning: 2025-12-18T05:20:03.536Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4385
File: Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml:0-0
Timestamp: 2025-12-18T05:20:03.536Z
Learning: In the Ginger codebase, WPF DataTriggers can use string values (e.g., "ReadCellByIndex", "GetSheetDetails") to compare against enum properties (e.g., eExcelActionType) because WPF automatically handles enum-to-string conversion during DataTrigger evaluation. This pattern is confirmed to work correctly and is acceptable in XAML files.
Applied to files:
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.csGinger/Ginger/Actions/ActionEditPage.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: WPF UI components should be in the Ginger/ directory; console components should be cross-platform compatible
Applied to files:
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.cs
📚 Learning: 2024-09-22T17:01:36.055Z
Learnt from: Maheshkale447
Repo: Ginger-Automation/Ginger PR: 3918
File: Ginger/GingerCoreCommon/Repository/BusinessFlowLib/Activity.cs:1271-1287
Timestamp: 2024-09-22T17:01:36.055Z
Learning: Prefers the original implementation of the `VariablesSummary` property using explicit list creation over LINQ for better readability.
Applied to files:
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.cs
📚 Learning: 2025-09-04T16:40:33.092Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:9182-9223
Timestamp: 2025-09-04T16:40:33.092Z
Learning: In PR Ginger-Automation/Ginger#4294 (SeleniumDriver.cs GenerateSvgAttributeBasedXPath), maintainer declined changing XPath to remove extra quotes around EscapeXPathString(...) results; keep current quoting as-is in this area for now.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActDataSourcePage.xaml.cs
📚 Learning: 2025-07-10T07:12:52.786Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4249
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:7199-7202
Timestamp: 2025-07-10T07:12:52.786Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs, text values used in XPath expressions should be escaped using a helper method (such as EscapeXPathString) to handle single quotes and ensure valid XPath syntax.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActDataSourcePage.xaml.cs
📚 Learning: 2025-09-04T16:40:09.511Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4294
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs:6019-6061
Timestamp: 2025-09-04T16:40:09.511Z
Learning: In Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/Selenium/SeleniumDriver.cs (AddSvgSpecificLocators, around lines ~6000–6065) for PR Ginger-Automation/Ginger#4294, maintainer prashelke indicated that adjusting XPath to remove outer quotes around EscapeXPathString(...) is not required. Do not suggest changing the quoting around EscapeXPathString for SVG-specific locators in future reviews unless maintainers request it.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActDataSourcePage.xaml.cs
📚 Learning: 2025-08-26T07:40:08.345Z
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4285
File: Ginger/Ginger/Actions/ActionEditPages/ActSecurityTestingEditPage.xaml:23-23
Timestamp: 2025-08-26T07:40:08.345Z
Learning: In the Ginger codebase ActSecurityTestingEditPage.xaml file, the large bottom margin of 530 on the "Acceptable Alerts" label (Margin="10,10,0,530") is intentional design choice, not an error.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml
📚 Learning: 2024-07-26T22:04:12.930Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3429
File: Ginger/Ginger/AutomatePageLib/AddActionMenu/WindowExplorer/Common/WindowExplorerPage.xaml.cs:1123-1126
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has removed an unnecessary empty line at 1123 in `WindowExplorerPage.xaml.cs` for better code compactness.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xamlGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
📚 Learning: 2025-04-30T10:48:07.912Z
Learnt from: rathimayur
Repo: Ginger-Automation/Ginger PR: 4195
File: Ginger/Ginger/Actions/ActionEditPage.xaml.cs:2523-2536
Timestamp: 2025-04-30T10:48:07.912Z
Learning: In the ActionEditPage, the browse button is only visible/clickable when there are items in the grid, preventing null reference exceptions when accessing CurrentItem.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml
📚 Learning: 2025-07-09T13:46:53.701Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4245
File: Ginger/Ginger/Actions/ActionEditPages/ValidationDBPage.xaml.cs:142-143
Timestamp: 2025-07-09T13:46:53.701Z
Learning: In ValidationDBPage.xaml.cs, the new constructor overload that accepts SelectedContentArgs and ActDBValidation intentionally removes the SelectionChanged event handlers for AppNameComboBox and DBNameComboBox without restoring them. This is expected behavior since this constructor sets up the combo boxes with fixed values and makes them non-editable, so the normal selection changed behavior is not needed.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xamlGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4254
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs:70-73
Timestamp: 2025-07-16T14:42:32.229Z
Learning: In Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs, prashelke prefers to keep the current `!WorkSpace.Instance.BetaFeatures.ShowPOMForAI` logic for binding and showing AI POM controls, rather than changing it to positive logic.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/Ginger/Actions/ActionEditPage.xaml.cs
📚 Learning: 2025-07-09T13:44:51.210Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4245
File: Ginger/Ginger/UserControlsLib/UCDataMapping.xaml.cs:658-667
Timestamp: 2025-07-09T13:44:51.210Z
Learning: In UCDataMapping.xaml.cs SetDatabaseValues() method, the line xDBValueExpression.Visibility = Visibility.Visible is intentionally set unconditionally and works correctly with the visibility logic in SetValueControlsView(). This is the expected behavior per user confirmation.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/Ginger/UserControlsLib/UCDataMapping.xaml.cs
📚 Learning: 2025-11-28T05:32:43.529Z
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : On action failure, set `act.Error` and `act.Status = eRunStatus.Failed`
Applied to files:
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs
📚 Learning: 2025-06-16T09:52:40.873Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T09:52:40.873Z
Learning: The `GenericAppiumDriver` class is located in the `Amdocs.Ginger.CoreNET` namespace, not in a namespace that matches its file path structure. File paths don't always correspond to namespace declarations in this codebase.
Applied to files:
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs
📚 Learning: 2025-06-16T10:36:01.993Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4228
File: Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs:83-84
Timestamp: 2025-06-16T10:36:01.993Z
Learning: For the `UnLockType` property in `Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs`, the current naming convention with capital 'L' in the middle should be maintained per project preferences.
Applied to files:
Ginger/Ginger/Actions/ActionEditPage.xaml.cs
📚 Learning: 2025-06-13T13:47:39.193Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4228
File: Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs:384-394
Timestamp: 2025-06-13T13:47:39.193Z
Learning: For the `eUnlockTypes` enum in `Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs`, the identifiers must remain lowercase (`pin`, `password`, `pattern`, `none`) per project requirements.
Applied to files:
Ginger/Ginger/Actions/ActionEditPage.xaml.cs
📚 Learning: 2025-07-09T13:45:55.367Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4245
File: Ginger/Ginger/UserControlsLib/UCDataMapping.xaml.cs:230-234
Timestamp: 2025-07-09T13:45:55.367Z
Learning: In UCDataMapping.xaml.cs ValueExpression binding code, using WorkSpace.Instance.SolutionRepository.GetAllRepositoryItems<ProjEnvironment>().FirstOrDefault() without null checks for the mContext creation is intentional and working as expected per user confirmation.
Applied to files:
Ginger/Ginger/Actions/ActionEditPage.xaml.cs
🧬 Code graph analysis (8)
Ginger/Ginger/Reports/GingerExecutionReport/assets/js/donutchart.js (1)
Ginger/Ginger/Reports/GingerExecutionReport/assets/js/circlechart.js (2)
Recharts(3-3)DonutChart(19-41)
Ginger/Ginger/Reports/GingerExecutionReport/assets/js/circlechart.js (1)
Ginger/Ginger/Reports/GingerExecutionReport/assets/js/donutchart.js (3)
inputData(2-2)Recharts(10-10)DonutChart(59-76)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (3)
Ginger/GingerCoreCommon/InterfacesLib/IExcelOperations.cs (1)
WriteCellData(86-86)Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (1)
Col(469-488)
Ginger/GingerCoreCommon/InterfacesLib/IExcelOperations.cs (1)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (1)
WriteCellData(460-540)
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.cs (1)
Ginger/Ginger/UserControlsLib/UCDataMapping.xaml.cs (4)
UCDataMapping(42-1405)UCDataMapping(196-224)RestrictedMappingType(80-90)RestrictedMappingType(85-89)
Ginger/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.cs (3)
Ginger/Ginger/ApplicationModelsLib/APIModels/APIModelPage.xaml.cs (1)
Convert(1077-1085)Ginger/GingerCoreCommon/Repository/ApplicationModelLib/ApplicationModelBase.cs (1)
ToString(53-56)Ginger/GingerCoreCommon/Repository/RepositoryFolder.cs (1)
ToString(87-90)
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (3)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (8)
WriteData(282-296)DataTable(51-73)DataTable(92-108)DataTable(110-127)DataTable(152-155)DataTable(198-280)WriteCellData(460-540)List(333-351)Ginger/GingerCoreCommon/InterfacesLib/IExcelOperations.cs (6)
WriteData(49-49)DataTable(35-35)DataTable(37-37)DataTable(59-59)WriteCellData(86-86)List(75-75)Ginger/GingerCoreCommon/Actions/Act.cs (3)
AddOrUpdateInputParamValue(828-851)AddOrUpdateReturnParamActual(1308-1335)AddOrUpdateReturnParamActualWithPath(1359-1387)
Ginger/Ginger/Actions/ActionEditPage.xaml.cs (1)
Ginger/Ginger/UserControlsLib/UCDataMapping.xaml.cs (4)
UCDataMapping(42-1405)UCDataMapping(196-224)RestrictedMappingType(80-90)RestrictedMappingType(85-89)
🪛 Biome (2.1.2)
Ginger/Ginger/Reports/GingerExecutionReport/assets/css/font-awesome.css
[error] 4-4: Unexpected value or character.
Expected one of:
(parse)
[error] 4-4: Unexpected value or character.
Expected one of:
(parse)
[error] 4-4: Unexpected value or character.
Expected one of:
(parse)
[error] 4-4: Unexpected value or character.
Expected one of:
(parse)
[error] 4-4: Unexpected value or character.
Expected one of:
(parse)
[error] 4-4: Generic font family missing.
Consider adding a generic font family as a fallback.
For examples and more information, see the MDN Web Docs
(lint/a11y/useGenericFontNames)
Ginger/Ginger/Reports/GingerExecutionReport/assets/css/styles.css
[error] 1-1: Unexpected value or character.
Expected one of:
(parse)
[error] 1-1: Unexpected value or character.
Expected one of:
(parse)
[error] 1-1: Unknown type selector is not allowed.
See MDN web docs for more details.
Consider replacing the unknown type selector with valid one.
(lint/correctness/noUnknownTypeSelector)
[error] 1-1: Generic font family missing.
Consider adding a generic font family as a fallback.
For examples and more information, see the MDN Web Docs
(lint/a11y/useGenericFontNames)
[error] 1-1: Generic font family missing.
Consider adding a generic font family as a fallback.
For examples and more information, see the MDN Web Docs
(lint/a11y/useGenericFontNames)
[error] 1-1: Generic font family missing.
Consider adding a generic font family as a fallback.
For examples and more information, see the MDN Web Docs
(lint/a11y/useGenericFontNames)
[error] 1-1: Generic font family missing.
Consider adding a generic font family as a fallback.
For examples and more information, see the MDN Web Docs
(lint/a11y/useGenericFontNames)
[error] 1-1: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
Ginger/Ginger/Reports/GingerExecutionReport/assets/js/jquery.js
[error] 1-1: Comparing to itself is potentially pointless.
(lint/suspicious/noSelfCompare)
[error] 1-1: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 1-1: Comparing to itself is potentially pointless.
(lint/suspicious/noSelfCompare)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: Do not reassign a function declaration.
Reassigned here.
Reassigned here.
Reassigned here.
Use a local variable instead.
(lint/suspicious/noFunctionAssign)
[error] 1-1: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🔇 Additional comments (27)
Ginger/GingerCoreNET/SolutionRepositoryLib/UpgradeLib/SolutionUpgrade.cs (1)
105-105: LGTM! Log level correctly changed to ERROR.The change from
eLogLevel.WARNtoeLogLevel.ERRORis appropriate because the solution load is being aborted, which is an error condition rather than a warning. This aligns with the coding guideline to useReporter.ToLog(eLogLevel.ERROR, message)for logging errors.As per coding guidelines, C# files should use ERROR level for error conditions.
Ginger/GingerCoreTest/GingerCoreTest.csproj (2)
3-3: Target framework complies with coding guidelines.The target framework
net8.0-windows10.0.17763.0correctly matches the requirement for .NET 8.0 with Windows 10.0.17763.0 compatibility.As per coding guidelines, the target framework should be .NET 8.0 with Windows 10.0.17763.0 compatibility.
66-66: Confirm that Microsoft.Office.Interop.Excel version 16.0.18925.20022 is used and track underlying Office security updates.Version 16.0.18925.20022 is the latest stable release. The package itself is a signed interop wrapper with no direct CVEs; however, security vulnerabilities in the underlying Microsoft Office/Excel binaries should be monitored separately through Microsoft's Office security update channels. The .csproj file is properly configured with target framework net8.0-windows10.0.17763.0, meeting the Windows compatibility requirement.
Ginger/GingerTest/GingerTest.csproj (2)
4-4: Target framework complies with coding guidelines.The target framework
net8.0-windows10.0.17763.0correctly matches the requirement for .NET 8.0 with Windows 10.0.17763.0 compatibility.As per coding guidelines, the target framework should be .NET 8.0 with Windows 10.0.17763.0 compatibility.
27-27: Package addition complies with project requirements.The
Microsoft.Office.Interop.Excelversion 16.0.18925.20022 is the current stable release with no reported security vulnerabilities specific to this build. The addition is consistent with the same package in GingerCoreTest.csproj and the project's target framework (net8.0-windows10.0.17763.0) complies with coding guidelines.Ginger/GingerCoreCommon/GingerCoreCommon.csproj (3)
11-11: Verify Windows compatibility requirement for this project.The coding guidelines specify that .csproj files should target ".NET 8.0 with Windows 10.0.17763.0 compatibility." Currently, this project only specifies
net8.0without Windows-specific targeting.If Windows compatibility is required, consider updating the target framework to:
<TargetFramework>net8.0-windows10.0.17763.0</TargetFramework>However, since this is GingerCoreCommon (a core/common library), it may intentionally be cross-platform. Please verify whether Windows-specific targeting is required for this particular project.
As per coding guidelines, target framework should include Windows 10.0.17763.0 compatibility.
21-21: LGTM!Copyright year range appropriately updated to 2026.
36-48: Update Microsoft.CodeAnalysis to current version; verify LiteDB version conflict.
- Microsoft.CodeAnalysis (3.7.0) is outdated; current version is 5.0.0. Update to the latest version.
- System.Net.Http (4.3.4) is already the latest stable version available on NuGet (no newer version exists).
- Newtonstein.Json (13.0.3) is nearly current (latest is 13.0.4).
- LiteDB (5.0.21) is already the latest stable version.
However, there is an internal conflict: the project references LiteDB 5.0.21 via PackageReference but also directly references
litedb\4.1.4\lib\net40\LiteDB.dll. Remove the obsolete .NET 4.0 direct reference.Likely an incorrect or invalid review comment.
Ginger/Ginger/UserControlsLib/UCDataMapping.xaml.cs (4)
59-66: DependencyProperty + callback wiring forRestrictedMappingTypeslooks correctThe DP registration and
OnRestrictedMappingTypesChangedcallback correctly route changes intoApplyTypeRestrictions(). The null default and optional nature of the property are handled downstream via null checks, so this hook is safe.
228-237: Good: applying restrictions duringDataContextChangedCalling
ApplyTypeRestrictions()here ensures that, even ifRestrictedMappingTypeswas set before the combo was ready, the final control instance enforces the restrictions once data/context are in place. This is a reasonable safety net and should avoid inconsistent state across templated usages.
239-262: Loaded‑time enforcement complements DP callbackRe‑invoking
ApplyTypeRestrictions()onLoadedis a pragmatic way to handle template timing issues and any late setup work on the combo box. As implemented, the method is idempotent enough (early exit when no restrictions) that this should not add measurable overhead.
1337-1402: Item name extraction and fallback selection helpers are robust
GetItemNamecovers the expected shapes for combo items (enum,ComboBoxItemwithTag/Content, plain string, and a finalToString()fallback), which should make the restriction logic resilient to minor template differences.
SelectFallback:
- Prefers a known safe option (
None) when available.- Otherwise chooses the first remaining entry.
- Falls back to the preferred name even if not present (keeps binding consistent with enum semantics).
This is a sensible approach for recovering from invalid selections.
Ginger/Ginger/Run/BusinessFlowRunConfigurationsPage.xaml.cs (2)
39-39: Static import forUCDataMappingis appropriate
using static Ginger.UserControlsLib.UCDataMapping;is justified here since you referenceeDataTypein the template configuration and it improves readability without ambiguity.
185-189: Runset mapping correctly restricts direct variables; verify requirementThe new
UCDataMapping.TemplateOptionswiring:
- Binds
_OutputVariabelsSourcePropertyas expected.- Adds
_RestrictedMappingTypeswitheDataType.Variablerestricted and a clear reason ("Direct variables are not allowed in Runset configuration.").This integrates cleanly with the new restriction mechanism and will prevent selecting direct variables as mapped runtime values in Runset configuration.
Please confirm that all intended Runset scenarios are covered without needing direct variable mappings (and that any existing tests relying on that behavior have been updated or are no longer valid).
Ginger/Ginger/Actions/ActionEditPage.xaml.cs (3)
59-60: Static import is justified by frequent UCDataMapping usageBringing
UCDataMappingstatic members into scope (eDataTypein particular) makes the template configuration below clearer and is consistent with the other call sites using restricted mappings.
1014-1021: StoreTo mapping restrictions on Action page are wired correctlyFor the
ActReturnValue.Fields.StoreToValuecolumn:
_VariabelsSourceList = mStoreToVarsListprovides a curated list of allowed “Store To” variable names._RestrictedMappingTypesdisallows:
eDataType.Value– raw user literals,eDataType.ValueExpression– value expressions.- This is consistent with a design where storing results must go into variables/outputs rather than arbitrary values or expressions.
From a behavior standpoint this is sensible; just ensure this aligns with existing use cases where users might previously have configured Value/ValueExpression mappings in this grid.
1314-1316: Bracedelseblock aroundGetGridViewForParamValueInputValuesis fineThe only change here is adding braces around the
elsebranch, with no behavioral impact. This slightly improves maintainability without side effects.Ginger/GingerCoreNETUnitTest/LinuxTransformationTests/ActExcelTests.cs (1)
344-344: No functional change in testsOnly the final closing brace/newline was adjusted; there is no impact on the test behavior.
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml (2)
47-72: Well-organized UI additions for data selection modes.The new UI elements are properly structured:
- Radio buttons with
GroupName="DataSelection"ensure mutual exclusivity- Panels default to
Visibility="Collapsed"and are controlled by code-behind- The layout follows existing patterns and maintains consistency with the rest of the page
85-89: LGTM!The checkbox addition is clean and follows the existing style patterns. Visibility management is correctly delegated to the code-behind.
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (3)
290-363: Solid implementation of GetSheetDetails.The method correctly:
- Counts non-empty rows to provide accurate metadata
- Handles the header row offset appropriately
- Provides comprehensive output values (counts, addresses, range)
- Uses
Reporter.ToLog(eLogLevel.ERROR, ...)per coding guidelines- Handles empty sheet edge case
538-558: Clean integration of ByCellAddress write mode.The implementation properly:
- Uses
ValueExpression.Calculate()for value resolution- Delegates to the new
WriteCellDatamethod- Provides appropriate feedback via
ExInfoandError
600-617: Useful validation helper with user-friendly error messages.The
CheckMandatoryFieldsExistsmethod provides:
- Dynamic property access via the indexer
- Human-readable field names by splitting camelCase
- Consistent error messaging pattern
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (1)
331-357: Proper state management for data selection mode switching.The handler correctly:
- Performs null checks on radio buttons
- Updates the
DataSelectionMethodproperty- Clears fields from the opposing mode to prevent stale data
- Triggers visibility update
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (1)
460-540: Well-structured implementation with proper error handling and appropriate exception coverage.The
WriteCellDatamethod is well-implemented with:
- Proper thread synchronization via
lock(lockObj)- Support for both
.xlsxand.xlsformats- Clear separation of read and write phases
- Comprehensive error handling with
Reporter.ToLog(eLogLevel.ERROR, ...)as per coding guidelines- Proper resource cleanup in
finallyblock- Exception handling that adequately covers invalid address formats (both
CellRangeAddress.ValueOf()andCellReferenceconstructors are within the try-catch block and logged appropriately)The implementation is consistent with existing patterns in the codebase, where
CellReferenceinstantiation is performed directly without explicit pre-validation, relying on exception handling instead. The approach is sufficient given the proper error logging and method return behavior.Ginger/Ginger/Reports/GingerExecutionReport/GingerRunnerReport.html (2)
1-244: Template structure looks good overall.The HTML template follows a consistent pattern with other report templates in the PR. The collapsible sections, placeholder structure, and chart containers are well-organized. The modal implementation provides expected image viewing functionality.
14-16: The JSX transpilation setup is correct. Babel is included inassets/js/babel.js, and the{js_path}placeholder on line 13 is the proper injection point for loading external JavaScript dependencies before the JSX script block executes. This pattern is consistent across all report templates in the codebase and requires no changes.Likely an incorrect or invalid review comment.
| int quoteCount = wColVal.Count(ch => ch == '"'); | ||
| if (wColVal.Contains("\"") && quoteCount == 1 && condVal.Length >= 4) | ||
| { | ||
| wColVal = wColVal + " " + condVal[3]; | ||
| } |
There was a problem hiding this comment.
Partial fix may still truncate values with multiple embedded spaces.
The fix handles the case where a value contains exactly one embedded quote and 4 tokens, but values with more spaces (e.g., "some long value with spaces") will still be truncated. Consider using a more robust approach:
🔎 Suggested fix to handle all remaining tokens
else if (condVal.Length > 1)
{
wColVal = condVal[2].Replace("$$$", " ");
int quoteCount = wColVal.Count(ch => ch == '"');
- if (wColVal.Contains("\"") && quoteCount == 1 && condVal.Length >= 4)
+ if (wColVal.Contains("\"") && quoteCount == 1)
{
- wColVal = wColVal + " " + condVal[3];
+ // Append all remaining tokens to reconstruct the full value
+ for (int k = 3; k < condVal.Length; k++)
+ {
+ wColVal = wColVal + " " + condVal[k];
+ }
}
}📝 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.
| int quoteCount = wColVal.Count(ch => ch == '"'); | |
| if (wColVal.Contains("\"") && quoteCount == 1 && condVal.Length >= 4) | |
| { | |
| wColVal = wColVal + " " + condVal[3]; | |
| } | |
| int quoteCount = wColVal.Count(ch => ch == '"'); | |
| if (wColVal.Contains("\"") && quoteCount == 1) | |
| { | |
| // Append all remaining tokens to reconstruct the full value | |
| for (int k = 3; k < condVal.Length; k++) | |
| { | |
| wColVal = wColVal + " " + condVal[k]; | |
| } | |
| } |
| using Amdocs.Ginger.Common.InterfacesLib; | ||
| using Amdocs.Ginger.CoreNET.ActionsLib; | ||
| using GingerCore.Actions; | ||
| using Org.BouncyCastle.Ocsp; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused import.
Org.BouncyCastle.Ocsp namespace appears unrelated to Excel operations and is not used anywhere in this file.
🔎 Suggested fix
using GingerCore.Actions;
-using Org.BouncyCastle.Ocsp;
using System;📝 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.
| using Org.BouncyCastle.Ocsp; | |
| using GingerCore.Actions; | |
| using System; |
🤖 Prompt for AI Agents
In Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs around line
24, the using directive "using Org.BouncyCastle.Ocsp;" is unused and unrelated
to Excel operations; remove that line from the file (and save), then rebuild to
ensure no references remain and clean up any resulting unused-using warnings.
| private void UpdateVisibility() | ||
| { | ||
| // 1. Radio Buttons Visibility (Only for ReadCellData & WriteData) | ||
| bool showRadios = mAct.ExcelActionType is ActExcel.eExcelActionType.ReadData or ActExcel.eExcelActionType.ReadCellData or ActExcel.eExcelActionType.WriteData; xRadioDataSelectionPanel.Visibility = showRadios ? Visibility.Visible : Visibility.Collapsed; | ||
|
|
||
| // 2. Determine if "By Address" is active | ||
| // Safety check: rdByAddress might be null during init, so we check visibility too | ||
| bool isByAddress = (mAct.GetInputParamValue(nameof(mAct.DataSelectionMethod)) == "ByCellAddress" ? true : false); | ||
| rdByAddress.IsChecked = isByAddress; | ||
| rdByParams.IsChecked = !isByAddress; | ||
| // 3. Main Input Panels Logic | ||
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.GetSheetDetails) | ||
| { | ||
| // Hide inputs for GetSheetDetails | ||
| pnlDatabaseOptions.Visibility = Visibility.Collapsed; | ||
| xAddressInputPanel.Visibility = Visibility.Collapsed; | ||
| } | ||
| else if (isByAddress) | ||
| { | ||
| // ADDRESS MODE: Show Address Box, Hide Database params | ||
| pnlDatabaseOptions.Visibility = Visibility.Collapsed; | ||
| xAddressInputPanel.Visibility = Visibility.Visible; | ||
| } | ||
| else | ||
| { | ||
| // PARAM MODE: Show Database params, Hide Address Box | ||
| pnlDatabaseOptions.Visibility = Visibility.Visible; | ||
| xAddressInputPanel.Visibility = Visibility.Collapsed; | ||
| } | ||
|
|
||
| // 4. "Pull Cell Address" Checkbox (Only ReadData) | ||
| if ((mAct.ExcelActionType == ActExcel.eExcelActionType.ReadData || mAct.ExcelActionType == ActExcel.eExcelActionType.ReadCellData) && rdByParams.IsChecked == true) | ||
| { | ||
| xPullCellAddressCheckBox.Visibility = Visibility.Visible; | ||
| } | ||
| else | ||
| { | ||
| xPullCellAddressCheckBox.Visibility = Visibility.Collapsed; | ||
| } | ||
|
|
||
| // 5. Write Value Field (SetDataUsed) | ||
| // Show for WriteData (Always) | ||
| // Show for ReadData | ||
| // HIDE for ReadCellData | ||
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData ) | ||
| { | ||
| ColMappingRulesSection.Visibility = Visibility.Visible; | ||
| } | ||
| else | ||
| { | ||
| ColMappingRulesSection.Visibility = Visibility.Collapsed; | ||
| } | ||
|
|
||
| // 6. Bulk Write Field (ColMappingRules) | ||
| // Show ONLY for WriteData in PARAMETER mode. | ||
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData && isByAddress) | ||
| { | ||
| this.ColMappingRulesSection.Visibility = Visibility.Collapsed; | ||
| ColMappingRulesSection.Visibility = Visibility.Visible; | ||
| SetDataUsedSection.Visibility = Visibility.Collapsed; | ||
|
|
||
| } | ||
| else if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData && !isByAddress) | ||
| { | ||
| ColMappingRulesSection.Visibility = Visibility.Visible; | ||
| SetDataUsedSection.Visibility = Visibility.Visible; | ||
| } | ||
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData) | ||
| else | ||
| { | ||
| this.ColMappingRulesSection.Visibility = Visibility.Visible; | ||
| ColMappingRulesSection.Visibility = Visibility.Collapsed; | ||
| SetDataUsedSection.Visibility = Visibility.Collapsed; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Centralized visibility management is well-structured.
The UpdateVisibility() method consolidates all visibility logic which is good for maintainability. However, line 91 combines multiple statements on one line which reduces readability.
🔎 Suggested readability improvement for line 91
- bool showRadios = mAct.ExcelActionType is ActExcel.eExcelActionType.ReadData or ActExcel.eExcelActionType.ReadCellData or ActExcel.eExcelActionType.WriteData; xRadioDataSelectionPanel.Visibility = showRadios ? Visibility.Visible : Visibility.Collapsed;
+ bool showRadios = mAct.ExcelActionType is ActExcel.eExcelActionType.ReadData
+ or ActExcel.eExcelActionType.ReadCellData
+ or ActExcel.eExcelActionType.WriteData;
+ xRadioDataSelectionPanel.Visibility = showRadios ? Visibility.Visible : Visibility.Collapsed;📝 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.
| private void UpdateVisibility() | |
| { | |
| // 1. Radio Buttons Visibility (Only for ReadCellData & WriteData) | |
| bool showRadios = mAct.ExcelActionType is ActExcel.eExcelActionType.ReadData or ActExcel.eExcelActionType.ReadCellData or ActExcel.eExcelActionType.WriteData; xRadioDataSelectionPanel.Visibility = showRadios ? Visibility.Visible : Visibility.Collapsed; | |
| // 2. Determine if "By Address" is active | |
| // Safety check: rdByAddress might be null during init, so we check visibility too | |
| bool isByAddress = (mAct.GetInputParamValue(nameof(mAct.DataSelectionMethod)) == "ByCellAddress" ? true : false); | |
| rdByAddress.IsChecked = isByAddress; | |
| rdByParams.IsChecked = !isByAddress; | |
| // 3. Main Input Panels Logic | |
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.GetSheetDetails) | |
| { | |
| // Hide inputs for GetSheetDetails | |
| pnlDatabaseOptions.Visibility = Visibility.Collapsed; | |
| xAddressInputPanel.Visibility = Visibility.Collapsed; | |
| } | |
| else if (isByAddress) | |
| { | |
| // ADDRESS MODE: Show Address Box, Hide Database params | |
| pnlDatabaseOptions.Visibility = Visibility.Collapsed; | |
| xAddressInputPanel.Visibility = Visibility.Visible; | |
| } | |
| else | |
| { | |
| // PARAM MODE: Show Database params, Hide Address Box | |
| pnlDatabaseOptions.Visibility = Visibility.Visible; | |
| xAddressInputPanel.Visibility = Visibility.Collapsed; | |
| } | |
| // 4. "Pull Cell Address" Checkbox (Only ReadData) | |
| if ((mAct.ExcelActionType == ActExcel.eExcelActionType.ReadData || mAct.ExcelActionType == ActExcel.eExcelActionType.ReadCellData) && rdByParams.IsChecked == true) | |
| { | |
| xPullCellAddressCheckBox.Visibility = Visibility.Visible; | |
| } | |
| else | |
| { | |
| xPullCellAddressCheckBox.Visibility = Visibility.Collapsed; | |
| } | |
| // 5. Write Value Field (SetDataUsed) | |
| // Show for WriteData (Always) | |
| // Show for ReadData | |
| // HIDE for ReadCellData | |
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData ) | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Visible; | |
| } | |
| else | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Collapsed; | |
| } | |
| // 6. Bulk Write Field (ColMappingRules) | |
| // Show ONLY for WriteData in PARAMETER mode. | |
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData && isByAddress) | |
| { | |
| this.ColMappingRulesSection.Visibility = Visibility.Collapsed; | |
| ColMappingRulesSection.Visibility = Visibility.Visible; | |
| SetDataUsedSection.Visibility = Visibility.Collapsed; | |
| } | |
| else if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData && !isByAddress) | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Visible; | |
| SetDataUsedSection.Visibility = Visibility.Visible; | |
| } | |
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData) | |
| else | |
| { | |
| this.ColMappingRulesSection.Visibility = Visibility.Visible; | |
| ColMappingRulesSection.Visibility = Visibility.Collapsed; | |
| SetDataUsedSection.Visibility = Visibility.Collapsed; | |
| } | |
| } | |
| private void UpdateVisibility() | |
| { | |
| // 1. Radio Buttons Visibility (Only for ReadCellData & WriteData) | |
| bool showRadios = mAct.ExcelActionType is ActExcel.eExcelActionType.ReadData | |
| or ActExcel.eExcelActionType.ReadCellData | |
| or ActExcel.eExcelActionType.WriteData; | |
| xRadioDataSelectionPanel.Visibility = showRadios ? Visibility.Visible : Visibility.Collapsed; | |
| // 2. Determine if "By Address" is active | |
| // Safety check: rdByAddress might be null during init, so we check visibility too | |
| bool isByAddress = (mAct.GetInputParamValue(nameof(mAct.DataSelectionMethod)) == "ByCellAddress" ? true : false); | |
| rdByAddress.IsChecked = isByAddress; | |
| rdByParams.IsChecked = !isByAddress; | |
| // 3. Main Input Panels Logic | |
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.GetSheetDetails) | |
| { | |
| // Hide inputs for GetSheetDetails | |
| pnlDatabaseOptions.Visibility = Visibility.Collapsed; | |
| xAddressInputPanel.Visibility = Visibility.Collapsed; | |
| } | |
| else if (isByAddress) | |
| { | |
| // ADDRESS MODE: Show Address Box, Hide Database params | |
| pnlDatabaseOptions.Visibility = Visibility.Collapsed; | |
| xAddressInputPanel.Visibility = Visibility.Visible; | |
| } | |
| else | |
| { | |
| // PARAM MODE: Show Database params, Hide Address Box | |
| pnlDatabaseOptions.Visibility = Visibility.Visible; | |
| xAddressInputPanel.Visibility = Visibility.Collapsed; | |
| } | |
| // 4. "Pull Cell Address" Checkbox (Only ReadData) | |
| if ((mAct.ExcelActionType == ActExcel.eExcelActionType.ReadData || mAct.ExcelActionType == ActExcel.eExcelActionType.ReadCellData) && rdByParams.IsChecked == true) | |
| { | |
| xPullCellAddressCheckBox.Visibility = Visibility.Visible; | |
| } | |
| else | |
| { | |
| xPullCellAddressCheckBox.Visibility = Visibility.Collapsed; | |
| } | |
| // 5. Write Value Field (SetDataUsed) | |
| // Show for WriteData (Always) | |
| // Show for ReadData | |
| // HIDE for ReadCellData | |
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData ) | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Visible; | |
| } | |
| else | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Collapsed; | |
| } | |
| // 6. Bulk Write Field (ColMappingRules) | |
| // Show ONLY for WriteData in PARAMETER mode. | |
| if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData && isByAddress) | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Visible; | |
| SetDataUsedSection.Visibility = Visibility.Collapsed; | |
| } | |
| else if (mAct.ExcelActionType == ActExcel.eExcelActionType.WriteData && !isByAddress) | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Visible; | |
| SetDataUsedSection.Visibility = Visibility.Visible; | |
| } | |
| else | |
| { | |
| ColMappingRulesSection.Visibility = Visibility.Collapsed; | |
| SetDataUsedSection.Visibility = Visibility.Collapsed; | |
| } | |
| } |
🤖 Prompt for AI Agents
In Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs around lines
88 to 159, line 91 currently crams multiple statements on a single line
(declaring showRadios and setting xRadioDataSelectionPanel.Visibility), which
hurts readability; split that line into two clear statements: first compute and
assign the bool showRadios, then on the next line set
xRadioDataSelectionPanel.Visibility based on showRadios (preserving the same
logic), so each operation is on its own line and the method is easier to read
and maintain.
| <div id="myModal" class="modal"> | ||
| <span class="close">×</span> | ||
| <img class="modal-content" id="current_image_in_modal"> | ||
| <div id="caption">sdafasd asdfasdf</div> |
There was a problem hiding this comment.
Remove placeholder/debug text from modal caption.
Same as in GingerRunnerReport.html, this placeholder text should be removed.
🔎 Proposed fix
- <div id="caption">sdafasd asdfasdf</div>
+ <div id="caption"></div>📝 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.
| <div id="caption">sdafasd asdfasdf</div> | |
| <div id="caption"></div> |
🤖 Prompt for AI Agents
In Ginger/Ginger/Reports/GingerExecutionReport/ActionReport.html around line
190, the modal caption contains placeholder/debug text ("sdafasd asdfasdf");
remove that placeholder and replace it with the correct caption or a binding
used elsewhere (or an empty/hidden caption if none is needed) to match the
behavior in GingerRunnerReport.html; ensure you remove only the placeholder text
and not the surrounding div structure unless the caption is unnecessary, and
verify the UI shows the intended caption or no caption accordingly.
| function show_modal(image_id) { | ||
| var img = document.getElementById(image_id); | ||
| var modalImg = document.getElementById("current_image_in_modal"); | ||
| var captionText = document.getElementById("caption"); | ||
|
|
||
| modal.style.display = "block"; | ||
| modalImg.src = img.src; | ||
| captionText.innerHTML = img.alt; | ||
| } |
There was a problem hiding this comment.
Add null check to show_modal function.
Same defensive check as recommended for GingerRunnerReport.html.
🔎 Proposed fix
function show_modal(image_id) {
var img = document.getElementById(image_id);
+ if (!img) return;
var modalImg = document.getElementById("current_image_in_modal");
var captionText = document.getElementById("caption");
modal.style.display = "block";
modalImg.src = img.src;
captionText.innerHTML = img.alt;
}📝 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.
| function show_modal(image_id) { | |
| var img = document.getElementById(image_id); | |
| var modalImg = document.getElementById("current_image_in_modal"); | |
| var captionText = document.getElementById("caption"); | |
| modal.style.display = "block"; | |
| modalImg.src = img.src; | |
| captionText.innerHTML = img.alt; | |
| } | |
| function show_modal(image_id) { | |
| var img = document.getElementById(image_id); | |
| if (!img) return; | |
| var modalImg = document.getElementById("current_image_in_modal"); | |
| var captionText = document.getElementById("caption"); | |
| modal.style.display = "block"; | |
| modalImg.src = img.src; | |
| captionText.innerHTML = img.alt; | |
| } |
🤖 Prompt for AI Agents
In Ginger/Ginger/Reports/GingerExecutionReport/ActionReport.html around lines
197 to 205, the show_modal function assumes DOM elements exist and directly
accesses img, modalImg, captionText and modal; add defensive null checks for
document.getElementById results (and modal) before using them, returning early
if any required element is missing, so you avoid runtime errors when elements
are absent; keep the behavior the same when all elements are present.
| <AssemblyVersion>25.5.0.0</AssemblyVersion> | ||
| <FileVersion>25.5.0.0</FileVersion> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n Ginger/GingerCoreCommon/GingerCoreCommon.csproj | head -50Repository: Ginger-Automation/Ginger
Length of output: 2558
🏁 Script executed:
cd Ginger/GingerCoreCommon && git log --oneline -n 10 GingerCoreCommon.csproj 2>/dev/null || echo "No git history available"Repository: Ginger-Automation/Ginger
Length of output: 129
🏁 Script executed:
git diff HEAD~1 Ginger/GingerCoreCommon/GingerCoreCommon.csproj 2>/dev/null | head -100 || echo "Cannot show git diff"Repository: Ginger-Automation/Ginger
Length of output: 50
🏁 Script executed:
git show HEAD:Ginger/GingerCoreCommon/GingerCoreCommon.csproj 2>/dev/null | head -50Repository: Ginger-Automation/Ginger
Length of output: 2208
🏁 Script executed:
git show HEAD~1:Ginger/GingerCoreCommon/GingerCoreCommon.csproj 2>/dev/null | grep -E "(AssemblyVersion|FileVersion|Version>|TargetFramework)" || echo "Cannot retrieve previous version"Repository: Ginger-Automation/Ginger
Length of output: 100
🏁 Script executed:
git log --all --oneline -- Ginger/GingerCoreCommon/GingerCoreCommon.csproj | head -15Repository: Ginger-Automation/Ginger
Length of output: 758
🏁 Script executed:
git show master:Ginger/GingerCoreCommon/GingerCoreCommon.csproj 2>/dev/null | grep -E "(AssemblyVersion|FileVersion|Version>)" | head -5Repository: Ginger-Automation/Ginger
Length of output: 192
🏁 Script executed:
git show merge-official-to-master:Ginger/GingerCoreCommon/GingerCoreCommon.csproj 2>/dev/null | grep -E "(AssemblyVersion|FileVersion|Version>)" | head -5 || echo "Branch may not exist"Repository: Ginger-Automation/Ginger
Length of output: 50
🏁 Script executed:
fd -e csproj -x grep -l "TargetFramework" {} \; | head -10 | xargs grep -h "TargetFramework" | sort | uniq -cRepository: Ginger-Automation/Ginger
Length of output: 453
🏁 Script executed:
git show official:Ginger/GingerCoreCommon/GingerCoreCommon.csproj 2>/dev/null | grep -E "(AssemblyVersion|FileVersion|Version>|TargetFramework)" | head -5 || echo "official branch query"Repository: Ginger-Automation/Ginger
Length of output: 50
Version downgrade and TargetFramework guideline violation detected.
The AssemblyVersion, FileVersion, and package Version have been downgraded from 25.5.1.0/2025.5.1-Beta to 25.5.0.0/2025.5.0. Version downgrades can cause critical issues including assembly binding failures, NuGet resolution conflicts, and breaking changes for existing integrations.
Additionally, the TargetFramework violates the coding guideline. Change line 11 from <TargetFramework>net8.0</TargetFramework> to <TargetFramework>net8.0-windows10.0.17763.0</TargetFramework> to comply with the requirement for Windows 10.0.17763.0 compatibility.
| /// <summary> | ||
| /// For writing data into a specific cell in excel sheet | ||
| /// </summary> | ||
| /// <param name="fileName"></param> | ||
| /// <param name="sheetName"></param> | ||
| /// <param name="address"></param> | ||
| /// <param name="value"></param> | ||
| /// <param name="headerRowNumber"></param> | ||
| /// <returns></returns> | ||
| bool WriteCellData(string fileName, string sheetName, string address, string value); |
There was a problem hiding this comment.
Remove stale XML documentation parameter.
The XML documentation mentions a headerRowNumber parameter (line 84-85) that doesn't exist in the method signature. This appears to be leftover from copying documentation from another method.
🔎 Suggested fix
/// <summary>
/// For writing data into a specific cell in excel sheet
/// </summary>
/// <param name="fileName"></param>
/// <param name="sheetName"></param>
/// <param name="address"></param>
/// <param name="value"></param>
- /// <param name="headerRowNumber"></param>
/// <returns></returns>
bool WriteCellData(string fileName, string sheetName, string address, string value);📝 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.
| /// <summary> | |
| /// For writing data into a specific cell in excel sheet | |
| /// </summary> | |
| /// <param name="fileName"></param> | |
| /// <param name="sheetName"></param> | |
| /// <param name="address"></param> | |
| /// <param name="value"></param> | |
| /// <param name="headerRowNumber"></param> | |
| /// <returns></returns> | |
| bool WriteCellData(string fileName, string sheetName, string address, string value); | |
| /// <summary> | |
| /// For writing data into a specific cell in excel sheet | |
| /// </summary> | |
| /// <param name="fileName"></param> | |
| /// <param name="sheetName"></param> | |
| /// <param name="address"></param> | |
| /// <param name="value"></param> | |
| /// <returns></returns> | |
| bool WriteCellData(string fileName, string sheetName, string address, string value); |
🤖 Prompt for AI Agents
In Ginger/GingerCoreCommon/InterfacesLib/IExcelOperations.cs around lines 77 to
86, the XML doc comments list a parameter named headerRowNumber that is not
present in the method signature; remove the stale <param> entry (or update the
signature if the parameter is intended) so the XML documentation matches the
method bool WriteCellData(string fileName, string sheetName, string address,
string value); ensure all remaining <param> tags correspond exactly to actual
parameters and update the summary if needed.
| using NPOI.OpenXmlFormats.Dml.Diagram; | ||
| using NPOI.SS.Util; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused import detected.
The NPOI.OpenXmlFormats.Dml.Diagram namespace (line 27) doesn't appear to be used in this file. Only NPOI.SS.Util is needed for CellRangeAddress and CellReference.
🔎 Suggested fix
-using NPOI.OpenXmlFormats.Dml.Diagram;
using NPOI.SS.Util;📝 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.
| using NPOI.OpenXmlFormats.Dml.Diagram; | |
| using NPOI.SS.Util; | |
| using NPOI.SS.Util; |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/ActExcel.cs around lines 27 to 28, remove the
unused using directive for NPOI.OpenXmlFormats.Dml.Diagram because it is not
referenced; keep the using NPOI.SS.Util for CellRangeAddress and CellReference
and ensure no other types from the removed namespace are used elsewhere in the
file.
| if (DataSelectionMethod == eDataSelectionMethod.ByCellAddress) | ||
| { | ||
| DataTable dtAddr = excelOperator.ReadCellData(CalculatedFileName, CalculatedSheetName, CalculatedFilter, SelectAllRows, CalculatedHeaderRowNum); | ||
| if (dtAddr == null) { Error = "No data found at " + CalculatedFilter; return; } | ||
|
|
||
| // Parse start address (e.g. "A5") to calculate paths | ||
| var startCoord = ParseCellAddress(CalculatedFilter.Split(':')[0]); | ||
|
|
||
| for (int j = 0; j < dtAddr.Rows.Count; j++) | ||
| { | ||
| DataRow r = dtAddr.Rows[j]; | ||
| int currentRowNum = startCoord.Row + j; | ||
|
|
||
| for (int i = 0; i < dtAddr.Columns.Count; i++) | ||
| { | ||
| string colName = dtAddr.Columns[i].ColumnName; | ||
| string val = r[i].ToString(); | ||
|
|
||
| string cellAddress = GetColumnName(startCoord.Col + i) + currentRowNum; | ||
|
|
||
| AddOrUpdateReturnParamActualWithPath(colName, val, cellAddress); | ||
|
|
||
| } | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Potential NullReferenceException on CalculatedFilter.
If SelectCellAddress is empty/null, CalculatedFilter will be null when DataSelectionMethod == ByCellAddress, and CalculatedFilter.Split(':')[0] on line 380 will throw a NullReferenceException.
🔎 Suggested fix
if (DataSelectionMethod == eDataSelectionMethod.ByCellAddress)
{
+ if (string.IsNullOrWhiteSpace(CalculatedFilter))
+ {
+ Error = "Cell Address is required for ByCellAddress mode";
+ return;
+ }
+
DataTable dtAddr = excelOperator.ReadCellData(CalculatedFileName, CalculatedSheetName, CalculatedFilter, SelectAllRows, CalculatedHeaderRowNum);
if (dtAddr == null) { Error = "No data found at " + CalculatedFilter; return; }
// Parse start address (e.g. "A5") to calculate paths
var startCoord = ParseCellAddress(CalculatedFilter.Split(':')[0]);🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/ActExcel.cs around lines 374 to 399, the code
assumes CalculatedFilter is non-null and calls CalculatedFilter.Split(':')[0],
which can throw a NullReferenceException if CalculatedFilter is null/empty when
DataSelectionMethod == ByCellAddress; add a guard that checks
string.IsNullOrWhiteSpace(CalculatedFilter) before using it and, if empty, set
Error to an explanatory message and return (or otherwise handle the condition),
otherwise proceed to split and parse the start address; ensure the guard covers
both null and whitespace and the error message includes context (e.g.,
"CalculatedFilter is empty for ByCellAddress selection").
| private (int Col, int Row) ParseCellAddress(string address) | ||
| { | ||
| try | ||
| { | ||
| if (ex.Message != null && ex.Message.StartsWith("Cannot find column")) | ||
| if (string.IsNullOrEmpty(address)) return (1, 1); | ||
| var match = Regex.Match(address, @"([A-Z]+)([0-9]+)"); | ||
| if (!match.Success) return (1, 1); | ||
|
|
||
| string colStr = match.Groups[1].Value; | ||
| int row = int.Parse(match.Groups[2].Value); | ||
|
|
||
| int colIndex = 0; | ||
| foreach (char c in colStr) | ||
| { | ||
| Error = $"{ex.Message} at Row Number {HeaderRowNum}"; | ||
| colIndex = colIndex * 26 + (c - 'A' + 1); | ||
| } | ||
| else | ||
| return (colIndex, row); | ||
| } | ||
| catch { return (1, 1); } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent error handling may mask parsing issues.
The ParseCellAddress method returns (1, 1) on any failure without logging, which could lead to incorrect cell addresses being reported without any indication of the problem.
🔎 Suggested fix to add logging
private (int Col, int Row) ParseCellAddress(string address)
{
try
{
if (string.IsNullOrEmpty(address)) return (1, 1);
var match = Regex.Match(address, @"([A-Z]+)([0-9]+)");
if (!match.Success) return (1, 1);
string colStr = match.Groups[1].Value;
int row = int.Parse(match.Groups[2].Value);
int colIndex = 0;
foreach (char c in colStr)
{
colIndex = colIndex * 26 + (c - 'A' + 1);
}
return (colIndex, row);
}
- catch { return (1, 1); }
+ catch (Exception ex)
+ {
+ Reporter.ToLog(eLogLevel.WARN, $"Failed to parse cell address '{address}', defaulting to (1,1)", ex);
+ return (1, 1);
+ }
}📝 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.
| private (int Col, int Row) ParseCellAddress(string address) | |
| { | |
| try | |
| { | |
| if (ex.Message != null && ex.Message.StartsWith("Cannot find column")) | |
| if (string.IsNullOrEmpty(address)) return (1, 1); | |
| var match = Regex.Match(address, @"([A-Z]+)([0-9]+)"); | |
| if (!match.Success) return (1, 1); | |
| string colStr = match.Groups[1].Value; | |
| int row = int.Parse(match.Groups[2].Value); | |
| int colIndex = 0; | |
| foreach (char c in colStr) | |
| { | |
| Error = $"{ex.Message} at Row Number {HeaderRowNum}"; | |
| colIndex = colIndex * 26 + (c - 'A' + 1); | |
| } | |
| else | |
| return (colIndex, row); | |
| } | |
| catch { return (1, 1); } | |
| } | |
| private (int Col, int Row) ParseCellAddress(string address) | |
| { | |
| try | |
| { | |
| if (string.IsNullOrEmpty(address)) return (1, 1); | |
| var match = Regex.Match(address, @"([A-Z]+)([0-9]+)"); | |
| if (!match.Success) return (1, 1); | |
| string colStr = match.Groups[1].Value; | |
| int row = int.Parse(match.Groups[2].Value); | |
| int colIndex = 0; | |
| foreach (char c in colStr) | |
| { | |
| colIndex = colIndex * 26 + (c - 'A' + 1); | |
| } | |
| return (colIndex, row); | |
| } | |
| catch (Exception ex) | |
| { | |
| Reporter.ToLog(eLogLevel.WARN, $"Failed to parse cell address '{address}', defaulting to (1,1)", ex); | |
| return (1, 1); | |
| } | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/ActExcel.cs around lines 469 to 488, the
ParseCellAddress method swallows all exceptions and silently returns (1,1);
change the empty catch to catch(Exception ex) and log the failure (including the
input address and exception details) using the project’s logging facility before
returning the fallback value so parsing errors are visible in logs for
debugging.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
6 participants
Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.