Bug fixes for excel action#4402
Conversation
WalkthroughUI, action logic, and NPOI Excel operations updated: UI visibility/validation now driven by ByCellAddress vs ByParameters; ActExcel read/write routing and row-index calculations adjusted; WriteCellData now supports range addresses and workbook cleanup moved to finally. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ActExcelEditPage (UI)
participant Core as ActExcel (Action Logic)
participant NPOI as ExcelNPOIOperations (NPOI)
Note over UI,Core: User chooses DataSelection (ByCellAddress / ByParameters) and action type
UI->>Core: Submit action (type, selection mode, addresses/rows, value)
alt ReadData / ByParameters
Core->>NPOI: Read sheet rows (standard path)
NPOI-->>Core: Rows with values + addresses
Core-->>UI: Return params populated (value + address)
else ReadData / ByCellAddress
Core->>NPOI: Read using direct start index for cell addresses
NPOI-->>Core: Cell addresses + values
Core-->>UI: Return params populated
else WriteData / AddressRange
Core->>NPOI: Write value to range (iterate cells)
NPOI-->>Core: Acknowledge writes
Core-->>UI: Update status
end
Note right of NPOI: Workbook closed in finally block to ensure cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (1)
405-427: Fix off-by-N error inPullCellAddresscell address calculation.When reading Excel data, the
currentRowNumis calculated as1 + currentRowwherecurrentRowis a 0-indexed loop counter. However, the DataTable returned byReadData()contains only data rows (no header), and the actual Excel row numbers depend on where the header row is located. If the header is on row 1, data starts on row 2; if on row 2, data starts on row 3, etc.With the current calculation, all cell addresses start from row 1 regardless of the actual data location in Excel, producing incorrect addresses like
A1,B1instead of the correctA2,B2(or higher depending on header position).The fix must account for the header row offset:
- int currentRowNum = 1 + currentRow; + int headerRow = int.Parse(CalculatedHeaderRowNum); + int currentRowNum = headerRow + 1 + currentRow;
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/ActionsLib/ActExcel.csGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs
🧰 Additional context used
📓 Path-based instructions (2)
**/{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/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
Reporter.ToLog(eLogLevel.ERROR, message)for logging errors
Files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/GingerCoreNET/ActionsLib/ActExcel.cs
🧠 Learnings (8)
📚 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/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.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.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/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.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 **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Name actions following the pattern: `Act{PlatformType}{ActionType}` (e.g., `ActBrowserElement`, `ActConsoleCommand`)
Applied to files:
Ginger/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 **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Actions should inherit from `Act` class and implement platform-specific logic
Applied to files:
Ginger/GingerCoreNET/ActionsLib/ActExcel.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/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 **/{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
🧬 Code graph analysis (3)
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (3)
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (3)
ActExcel(37-641)ReadData(365-462)WriteData(527-591)Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (1)
WriteData(282-296)Ginger/GingerCoreCommon/InterfacesLib/IExcelOperations.cs (1)
WriteData(49-49)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (1)
Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (5)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (6)
WriteData(282-296)DataTable(51-73)DataTable(92-108)DataTable(110-127)DataTable(152-155)DataTable(198-280)Ginger/GingerCoreCommon/InterfacesLib/IExcelOperations.cs (4)
WriteData(49-49)DataTable(35-35)DataTable(37-37)DataTable(59-59)Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (1)
DataTable(319-331)Ginger/GingerCoreCommon/Actions/Act.cs (2)
AddOrUpdateReturnParamActual(1308-1335)AddOrUpdateReturnParamActualWithPath(1359-1387)
🔇 Additional comments (8)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (2)
494-517: Range writing logic looks correct.The implementation correctly:
- Distinguishes range addresses (containing
:) from single-cell addresses- Uses
CellRangeAddress.ValueOfto parse range notation- Creates rows/cells as needed with null-coalescing operators
- Writes the same value to all cells in the range
The single-cell path uses
CellReferenceappropriately.
535-539: Good addition offinallyblock for resource cleanup.This ensures the workbook is closed even if an exception occurs during the write phase, preventing resource leaks.
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (3)
251-268: Good refactor: switch statement improves readability.The switch statement is cleaner than chained if/else conditions and provides explicit handling for each action type. The default case correctly logs an error for unmatched actions using
Reporter.ToLog(eLogLevel.ERROR, ...), which follows the coding guidelines.
389-390: LGTM: ByCellAddress path correctly stores cell addresses.The change ensures cell addresses are always stored in the return parameter path when using the ByCellAddress data selection method, which aligns with the enhanced range-writing feature.
294-294: LGTM: Simplified GetSheetDetails.Directly passing
CalculatedHeaderRowNumtoReadDatais cleaner.Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (3)
119-126: LGTM: Visibility constraint for PullCellAddress checkbox.Restricting the "Pull Cell Address" checkbox to only show when
ReadDatais selected and "By Parameters" mode is active is correct, as this feature is only relevant for parameter-based data reading.
149-158: LGTM: Visibility logic for WriteData sections.The logic correctly shows:
ColMappingRulesSectiononly for WriteData (By Address mode)- Both
ColMappingRulesSectionandSetDataUsedSectionfor WriteData (By Parameters mode)- Hides both for other action types
342-354: Good: Field cleanup when switching data selection modes.Clearing related fields (
PullCellAddress,SelectRowsWhere,PrimaryKeyColumnfor By Address;PullCellAddress,SelectCellAddressfor By Parameters) when switching modes prevents stale data from affecting the action execution.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.csGinger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
Reporter.ToLog(eLogLevel.ERROR, message)for logging errors
Files:
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
**/{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/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
🧠 Learnings (9)
📚 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/GingerCoreNET/Run/Excels/ExcelNPOIOperations.csGinger/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.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/ActionEditPages/ActExcelEditPage.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.cs
📚 Learning: 2024-08-06T14:31:48.012Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3851
File: Ginger/Ginger/SolutionWindows/AccessibilityRulePage.xaml.cs:98-103
Timestamp: 2024-08-06T14:31:48.012Z
Learning: The `LoadGridData` method in `AccessibilityRulePage` should ensure that the `actAccessibilityTesting` field is properly initialized before using it to avoid runtime errors.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
📚 Learning: 2025-04-30T13:21:34.994Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4193
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/AIGeneratedPreviewWizardPage.xaml.cs:131-138
Timestamp: 2025-04-30T13:21:34.994Z
Learning: In AIGeneratedPreviewWizardPage.xaml.cs, the current code checks if the response ends with triple backticks and removes them manually. This method is fragile and should be replaced with a more robust regex-based parser. The user plans to address this in a future update.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
📚 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:1139-1142
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has updated the code by removing or addressing the commented code as suggested in the review.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.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/Ginger/Actions/ActionEditPages/ActExcelEditPage.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.cs
🧬 Code graph analysis (1)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (2)
Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (1)
Col(463-482)
🔇 Additional comments (4)
Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (1)
535-539: Good improvement to resource cleanup.Moving
workbook?.Close()to afinallyblock ensures the workbook is properly closed even when exceptions occur, preventing potential file handle leaks.Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (3)
293-298: Mode-aware validation correctly addresses the previous issue.The validation logic now correctly determines required fields based on
DataSelectionMethod, ensuring that only the relevant fields for the active mode are validated. This resolves the confusing nested validation from the previous implementation.
119-126: UI visibility logic correctly differentiates between modes.The changes appropriately:
- Restrict "Pull Cell Address" checkbox to ReadData with By Parameters mode (line 119)
- Show both
ColMappingRulesSectionandSetDataUsedSectionfor WriteData in By Parameters mode (lines 149-153)This provides proper UI feedback based on the selected data selection method.
Also applies to: 149-158
341-354: Field clearing on mode switch prevents stale data issues.Appropriately clearing mode-specific fields when switching between By Address and By Parameters modes ensures that obsolete values don't inadvertently affect action execution.
| // Handle Range vs Single Cell Writing | ||
| if (address.Contains(":")) | ||
| { | ||
| // --- RANGE LOGIC (e.g. "A5:A11") --- | ||
| CellRangeAddress range = CellRangeAddress.ValueOf(address); | ||
|
|
||
| cell.SetCellValue(value); | ||
| for (int r = range.FirstRow; r <= range.LastRow; r++) | ||
| { | ||
| IRow row = sheet.GetRow(r) ?? sheet.CreateRow(r); | ||
| for (int c = range.FirstColumn; c <= range.LastColumn; c++) | ||
| { | ||
| ICell cell = row.GetCell(c) ?? row.CreateCell(c); | ||
| cell.SetCellValue(value); // Writes the same value to all cells in range | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| // --- SINGLE CELL LOGIC (e.g. "A5") --- | ||
| CellReference cellRef = new CellReference(address); | ||
| IRow row = sheet.GetRow(cellRef.Row) ?? sheet.CreateRow(cellRef.Row); | ||
| ICell cell = row.GetCell(cellRef.Col) ?? row.CreateCell(cellRef.Col); | ||
| cell.SetCellValue(value); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating the address format before parsing.
The range and single-cell logic is well-implemented, correctly creating rows/cells as needed. However, unlike ReadCellData (which uses the regex field at line 43 to validate the address format), WriteCellData passes the address directly to CellRangeAddress.ValueOf() or CellReference. If an invalid address is provided (e.g., "invalid", "123"), NPOI will throw an unclear exception.
🔎 Proposed fix to add address validation
+ // Validate address format
+ if (!regex.IsMatch(address))
+ {
+ Reporter.ToLog(eLogLevel.ERROR, $"Invalid cell address format: {address}");
+ return false;
+ }
+
// Handle Range vs Single Cell Writing
if (address.Contains(":"))
{
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (1)
293-298: LGTM! Mode-aware validation correctly implemented.The validation logic appropriately checks the
DataSelectionMethodand validates only the fields required for the active mode (eitherSelectCellAddressfor By Address orSelectRowsWherefor By Parameters). This resolves the previous validation issue.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
🧰 Additional context used
📓 Path-based instructions (2)
**/{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/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
Reporter.ToLog(eLogLevel.ERROR, message)for logging errors
Files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
🧠 Learnings (9)
📚 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.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/ActionEditPages/ActExcelEditPage.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.cs
📚 Learning: 2024-08-06T14:31:48.012Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3851
File: Ginger/Ginger/SolutionWindows/AccessibilityRulePage.xaml.cs:98-103
Timestamp: 2024-08-06T14:31:48.012Z
Learning: The `LoadGridData` method in `AccessibilityRulePage` should ensure that the `actAccessibilityTesting` field is properly initialized before using it to avoid runtime errors.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
📚 Learning: 2025-04-30T13:21:34.994Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4193
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/AIGeneratedPreviewWizardPage.xaml.cs:131-138
Timestamp: 2025-04-30T13:21:34.994Z
Learning: In AIGeneratedPreviewWizardPage.xaml.cs, the current code checks if the response ends with triple backticks and removes them manually. This method is fragile and should be replaced with a more robust regex-based parser. The user plans to address this in a future update.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
📚 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:1139-1142
Timestamp: 2024-07-26T22:04:12.930Z
Learning: The user has updated the code by removing or addressing the commented code as suggested in the review.
Applied to files:
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.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/Ginger/Actions/ActionEditPages/ActExcelEditPage.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.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/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs
🧬 Code graph analysis (1)
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (3)
Ginger/GingerCoreNET/ActionsLib/ActExcel.cs (6)
ActExcel(37-641)ReadData(365-462)WriteData(527-591)List(611-635)CheckMandatoryFieldsExists(593-609)ReadCellData(483-525)Ginger/GingerCoreNET/Run/Excels/ExcelNPOIOperations.cs (2)
WriteData(282-296)List(333-351)Ginger/GingerCoreCommon/InterfacesLib/IExcelOperations.cs (2)
WriteData(49-49)List(75-75)
🔇 Additional comments (4)
Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs (4)
119-119: LGTM! Pull Cell Address checkbox visibility correctly restricted.The condition now appropriately shows the Pull Cell Address checkbox only when in ReadData mode AND By Parameters is selected, which aligns with the updated By Address/By Parameters architecture.
149-153: LGTM! WriteData visibility logic correctly handles By Parameters mode.The new branch appropriately shows both
ColMappingRulesSectionandSetDataUsedSectionwhen in WriteData mode with By Parameters selected, providing the correct UI for this mode.
323-323: LGTM! ReadData now correctly treated like ReadCellData for filter application.The condition now appropriately includes
ReadDataalongsideReadCellDatawhen applying filters, ensuring consistent data retrieval behavior across both action types.
352-353: LGTM! Field clearing logic correct for By Parameters mode.The code appropriately clears
PullCellAddresscheckbox andSelectCellAddresswhen switching to By Parameters mode, preventing confusion between modes.
|
|
||
| mAct.DataSelectionMethod = ActExcel.eDataSelectionMethod.ByCellAddress; | ||
| rdByParams.IsChecked = false; | ||
| //xAddressInputPanel.Visibility = Visibility.Visible; | ||
| //if (mAct.ExcelActionType is ActExcel.eExcelActionType.WriteData ) | ||
| //{ | ||
| // ColMappingRulesSection.Visibility = Visibility.Visible; | ||
| // SetDataUsedSection.Visibility = Visibility.Collapsed; | ||
|
|
||
| //} | ||
| //else | ||
| //{ | ||
| // ColMappingRulesSection.Visibility = Visibility.Collapsed; | ||
| // SetDataUsedSection.Visibility = Visibility.Collapsed; | ||
|
|
||
| //} | ||
| xPullCellAddressCheckBox.IsChecked = false; | ||
| mAct.SelectRowsWhere = string.Empty; | ||
| mAct.PrimaryKeyColumn = string.Empty; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor cleanup: Remove empty comment and redundant radio button setting.
The field-clearing logic when switching to By Address mode is correct and follows good practices. However:
- Line 341 contains an empty comment that should be removed.
- Line 343 (
rdByParams.IsChecked = false) is redundant—radio button groups handle mutual exclusion automatically.
🔎 Proposed cleanup
-
mAct.DataSelectionMethod = ActExcel.eDataSelectionMethod.ByCellAddress;
- rdByParams.IsChecked = false;
xPullCellAddressCheckBox.IsChecked = false;
mAct.SelectRowsWhere = string.Empty;
mAct.PrimaryKeyColumn = string.Empty;📝 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.
| mAct.DataSelectionMethod = ActExcel.eDataSelectionMethod.ByCellAddress; | |
| rdByParams.IsChecked = false; | |
| //xAddressInputPanel.Visibility = Visibility.Visible; | |
| //if (mAct.ExcelActionType is ActExcel.eExcelActionType.WriteData ) | |
| //{ | |
| // ColMappingRulesSection.Visibility = Visibility.Visible; | |
| // SetDataUsedSection.Visibility = Visibility.Collapsed; | |
| //} | |
| //else | |
| //{ | |
| // ColMappingRulesSection.Visibility = Visibility.Collapsed; | |
| // SetDataUsedSection.Visibility = Visibility.Collapsed; | |
| //} | |
| xPullCellAddressCheckBox.IsChecked = false; | |
| mAct.SelectRowsWhere = string.Empty; | |
| mAct.PrimaryKeyColumn = string.Empty; | |
| mAct.DataSelectionMethod = ActExcel.eDataSelectionMethod.ByCellAddress; | |
| xPullCellAddressCheckBox.IsChecked = false; | |
| mAct.SelectRowsWhere = string.Empty; | |
| mAct.PrimaryKeyColumn = string.Empty; |
🤖 Prompt for AI Agents
In Ginger/Ginger/Actions/ActionEditPages/ActExcelEditPage.xaml.cs around lines
341 to 346, remove the stray empty comment on line 341 and delete the redundant
radio-button assignment on line 343 (rdByParams.IsChecked = false) since radio
groups enforce mutual exclusion; keep the remaining field-clearing assignments
(xPullCellAddressCheckBox.IsChecked = false, mAct.SelectRowsWhere =
string.Empty, mAct.PrimaryKeyColumn = string.Empty) intact.
2 participants
Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.