Rollbacked the changes made for handling the file name length#4407
Merged
Conversation
Contributor
WalkthroughOpenApiBase now assigns AAM.Name directly from Operation.Summary (no 50-char truncation). InputBoxWindow removed its blanket MaxLength and added conditional MaxLength (30) logic in code-behind for folder prompts only. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)**/{Ginger,GingerCore}/**/{*Page,*Window}.xaml.cs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.cs📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-04-30T13:46:17.756ZApplied to files:
📚 Learning: 2025-08-26T07:40:08.345ZApplied to files:
📚 Learning: 2025-12-18T05:29:42.896ZApplied to files:
📚 Learning: 2024-07-26T22:04:12.930ZApplied to files:
📚 Learning: 2025-02-24T18:12:09.279ZApplied to files:
📚 Learning: 2025-04-30T13:21:34.994ZApplied to files:
📚 Learning: 2025-04-08T12:31:45.924ZApplied to files:
📚 Learning: 2025-11-28T05:32:43.529ZApplied to files:
📚 Learning: 2025-04-18T06:04:28.309ZApplied to files:
📚 Learning: 2025-12-18T05:19:56.687ZApplied to files:
🔇 Additional comments (2)
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}
Contributor
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/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.csGinger/GingerCore/GeneralLib/InputBoxWindow.xaml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
Reporter.ToLog(eLogLevel.ERROR, message)for logging errors
Files:
Ginger/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.cs
🧠 Learnings (3)
📚 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/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.xaml
📚 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/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.cs
🔇 Additional comments (1)
Ginger/GingerAutoPilot/APIModelLib/SwaggerApi/OpenApiBase.cs (1)
116-121: The removal of the 50-character truncation is safe and does not require validation. The repository save mechanism inSolutionRepository.csautomatically handles file path length constraints:
- File names are truncated to 255 characters if necessary
- Full paths are truncated to 260 characters (Windows MAX_PATH limit)
- Invalid file name characters are removed via
RemoveInvalidFileNameChars()Since
ApplicationAPIModel.Nameis persisted through this repository framework, length limits are enforced transparently during save operations. TheNameproperty has no explicit constraints by design—the length is controlled by the repository layer, not at the model level. There is no evidence of issues from unlimited name length in actual usage (UI display, filtering, or API consumers).Likely an incorrect or invalid review comment.
shahanemahesh
enabled auto-merge
January 9, 2026 11:57
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Ginger/GingerCore/GeneralLib/InputBoxWindow.xaml.cs (4)
37-41: Avoid using a finalizer to mutate UI/global state; rename constant to convey intent
~InputBoxWindow()settingCurrentInputBoxWindow = null;is non-deterministic and can run on the finalizer thread. Prefer clearing the static inOnClosed/Closedhandler (UI thread) or right afterShowDialog()returns. Also,MAX_LENGTHis ambiguous—this is specifically a folder-name constraint.Proposed fix (cleanup on close + clearer constant)
- private const int MAX_LENGTH = 30; - ~InputBoxWindow() - { - CurrentInputBoxWindow = null; - } + private const int MaxFolderNameLength = 30; + + protected override void OnClosed(EventArgs e) + { + base.OnClosed(e); + if (ReferenceEquals(CurrentInputBoxWindow, this)) + { + CurrentInputBoxWindow = null; + } + }
122-136: Don’t setValueTextBox.MaxLengthwhenValueTextBoxis hidden (or ensure the rule is applied to the actual input control)In this Init overload,
ValueTextBoxis collapsed and input happens viaxValueComboBox, so the MaxLength assignment is at best redundant and at worst misleading (future readers may think it’s enforced).
108-120: Fix null-unsafe folder detection and missing MaxLength validation across all threeInitoverloadsThe
message.Contains("Folder")pattern in lines 118, 125, and 141 is brittle and unsafe:
messagecan be null → throwsNullReferenceException- String matching breaks with localization or wording changes
MaxLengthis set but never enforced;OKButton_Clickdoesn't validate thatValueTextBox.Text(or pre-filled/bound values) stays within the limitProposed fix (centralize rule + null-safe + enforce on OK)
+ private bool IsFolderPrompt(string message) + { + return !string.IsNullOrEmpty(message) && + message.IndexOf("Folder", StringComparison.OrdinalIgnoreCase) >= 0; + } + + private void ApplyMaxLength(string message) + { + ValueTextBox.MaxLength = IsFolderPrompt(message) ? MAX_LENGTH : 0; + } + public void Init(string title, string message, ref string Value, bool isMultiline) { if (!isMultiline) { ValueTextBox.TextWrapping = TextWrapping.NoWrap; ValueTextBox.AcceptsReturn = false; } winTitle.Content = title; MessageLabel.Text = message; ValueTextBox.Text = Value; - ValueTextBox.MaxLength = message.Contains("Folder") ? MAX_LENGTH : 0; + ApplyMaxLength(message); ValueTextBox.Focus(); }Apply the same
ApplyMaxLength(message)call in the other two Init overloads (lines 125 and 141).private void OKButton_Click(object sender, RoutedEventArgs e) { + if (ValueTextBox.Visibility == Visibility.Visible && + ValueTextBox.MaxLength > 0 && + ValueTextBox.Text != null && + ValueTextBox.Text.Length > ValueTextBox.MaxLength) + { + Reporter.ToUser(eUserMsgKey.ValueIssue, $"Value cannot exceed {ValueTextBox.MaxLength} characters"); + ValueTextBox.Focus(); + return; + } if (xValueComboBox.Visibility == System.Windows.Visibility.Visible) { value = (string)xValueComboBox.SelectedValue; } else { value = ValueTextBox.Text; } OK = true; this.Close(); }
138-146: Validate folder path length before accepting the dialogWPF's
MaxLengthproperty only prevents keyboard input and does not auto-truncate or prevent binding updates from setting longer values. WhenObjFieldBindingestablishes a TwoWay binding withPropertyChangedtrigger, if the source object's property already contains a folder path exceeding 30 characters, it displays in the TextBox without truncation, andOKButton_Clickaccepts it without validation. Add validation inOKButton_Clickto reject or truncate over-length folder paths beforeClose(), or enforce the constraint at the source object level.
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Ginger/GingerCore/GeneralLib/InputBoxWindow.xamlGinger/GingerCore/GeneralLib/InputBoxWindow.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/GingerCore/GeneralLib/InputBoxWindow.xaml.cs
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
Reporter.ToLog(eLogLevel.ERROR, message)for logging errors
Files:
Ginger/GingerCore/GeneralLib/InputBoxWindow.xaml.cs
🧠 Learnings (11)
📓 Common learnings
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4193
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/UploadMockUpWizardPage.xaml.cs:56-58
Timestamp: 2025-04-30T13:46:17.756Z
Learning: The file size limit check in UploadMockUpWizardPage.xaml.cs uses a magic number (500000 bytes) which prashelke plans to refactor later to use a named constant for better maintainability.
📚 Learning: 2025-04-30T13:46:17.756Z
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4193
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/UploadMockUpWizardPage.xaml.cs:56-58
Timestamp: 2025-04-30T13:46:17.756Z
Learning: The file size limit check in UploadMockUpWizardPage.xaml.cs uses a magic number (500000 bytes) which prashelke plans to refactor later to use a named constant for better maintainability.
Applied to files:
Ginger/GingerCore/GeneralLib/InputBoxWindow.xaml.csGinger/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 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/GingerCore/GeneralLib/InputBoxWindow.xaml.csGinger/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 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/GingerCore/GeneralLib/InputBoxWindow.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: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/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 Learning: 2025-02-24T18:12:09.279Z
Learnt from: IamRanjeetSingh
Repo: Ginger-Automation/Ginger PR: 4128
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:322-325
Timestamp: 2025-02-24T18:12:09.279Z
Learning: For text length calculation in Ginger Automation's Web driver, only the value attribute should be used without any fallbacks to text content or inner text.
Applied to files:
Ginger/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 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/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 Learning: 2025-04-08T12:31:45.924Z
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4169
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:321-336
Timestamp: 2025-04-08T12:31:45.924Z
Learning: The GetTextLengthAsync method in Ginger Automation's Web driver should attempt to retrieve text from multiple sources (TextContent, InnerText, InputValue) to calculate text length, providing enhanced capabilities to read text length from different types of elements.
Applied to files:
Ginger/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 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/GingerCore/GeneralLib/InputBoxWindow.xaml
📚 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.xaml
🔇 Additional comments (2)
Ginger/GingerCore/GeneralLib/InputBoxWindow.xaml (2)
27-28: Good rollback point, but ensure “unlimited” is really unlimited across target frameworksRemoving the hard-coded
MaxLengthfrom XAML is fine as long as code-behind reliably applies folder-only limits and uses the correct “no limit” value for the repo’s WPF/.NET target.
1-12: Whitespace/format-only changes look safeAlso applies to: 24-24, 30-34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.