Bug fix/Source control download screen data fix#4169
Conversation
WalkthroughThis pull request modifies the source control configuration and credential management logic. The branch setting logic now checks if the branch value is empty and retrieves the current branch when needed. Conditions in retrieving source control properties have been updated to support single solutions, and error logging is added for repository URL retrieval. Additionally, the credential saving process is refined by introducing a control variable and updating the key used to fetch credentials from the Windows Credential Manager. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLIHelper
participant WS as WorkSpace
participant UP as UserProfile
participant SC as SourceControlIntegration
participant Reporter as Reporter
CLI->>WS: LoadSolutionAsync (captures DoNotSaveCredentialsOnUserProfile)
WS->>WS: OpenSolution()
WS->>WS: Set Solution property
alt If solution has SourceControl
WS->>UP: GetSourceControlPropertyFromUserProfileUsingURL(solution.SourceControl, solution.Guid)
end
WS->>SC: SetSourceControlDetailsFromUserProfile (check Branch property)
alt Branch is null/empty
SC->>SC: GetCurrentBranchForSolution()
SC-->>SC: Return current branch
else Branch exists
SC-->>SC: Use provided branch
end
WS->>Reporter: Log error if repository URL retrieval fails
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (2)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
Ginger/Ginger/SourceControl/SourceControlConnDetailsPage.xaml.cs(2 hunks)Ginger/GingerCoreCommon/UserCofig/UserProfile.cs(2 hunks)Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs(2 hunks)Ginger/GingerCoreNET/UserCofig/UserProfileOperations.cs(1 hunks)Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
Ginger/GingerCoreNET/UserCofig/UserProfileOperations.cs (1)
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/GingerCoreCommon/SourceControlLib/SourceControlBase.cs:58-59
Timestamp: 2025-04-08T10:41:31.325Z
Learning: Plain string storage for credentials in the SourceControlBase class is acceptable for now as confirmed by the developer. Security improvements for credential storage may be planned for a future iteration.
Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (1)
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/GingerCoreCommon/UserCofig/UserProfile.cs:748-765
Timestamp: 2025-04-08T10:41:31.325Z
Learning: The GetSolutionSourceControlInfo method in UserProfile class, which retrieves or creates a GingerSolution entry for a specific solution GUID, has been confirmed by the developer to be working as expected without issues of duplicate entries.
🧬 Code Definitions (3)
Ginger/Ginger/SourceControl/SourceControlConnDetailsPage.xaml.cs (5)
Ginger/GingerCoreCommon/SourceControlLib/GingerSolution.cs (1)
SourceControlInfo(47-300)Ginger/GingerCoreNET/SourceControl/SourceControlIntegration.cs (2)
SourceControlIntegration(37-718)GetCurrentBranchForSolution(705-716)Ginger/GingerCoreNET/SourceControl/GitSourceControlShellWrapper.cs (1)
GetCurrentBranchForSolution(98-101)Ginger/GingerCoreNET/SourceControl/SVNSourceControlShellWrapper.cs (1)
GetCurrentBranchForSolution(105-108)Ginger/GingerCoreNET/SourceControl/GITSourceControl.cs (1)
GetCurrentBranchForSolution(1778-1784)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (3)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (2)
WorkSpace(62-1011)OpenSolution(430-592)Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (2)
UserProfile(82-1010)UserProfile(326-391)Ginger/GingerCoreCommon/UserCofig/IUserProfileOperations.cs (1)
UserProfile(32-32)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (3)
Ginger/GingerCoreNET/UserCofig/UserProfileOperations.cs (1)
UserProfile(285-316)Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (3)
UserProfile(82-1010)UserProfile(326-391)GetSourceControlPropertyFromUserProfileUsingURL(886-912)Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)
🔇 Additional comments (8)
Ginger/GingerCoreNET/UserCofig/UserProfileOperations.cs (1)
352-352:Details
❓ Verification inconclusive
Update to credential retrieval key
The credential retrieval key has been changed from the previous "Ginger_SourceControl_GIT" to "Ginger_SolutionSourceControl". This standardizes credential retrieval to work with a solution-specific naming convention.
Let me verify if there are any other places in the codebase that might still be using the old credential key format:
🏁 Script executed:
#!/bin/bash # Search for any references to old credential key format rg "Ginger_SourceControl_GIT" --type=csLength of output: 39
Credential Retrieval Key Update Confirmed – Please Manually Verify Remaining References
- The credential key in
Ginger/GingerCoreNET/UserCofig/UserProfileOperations.cs(line 352) has been updated from"Ginger_SourceControl_GIT"to"Ginger_SolutionSourceControl", standardizing the naming convention.- A search for occurrences of
"Ginger_SourceControl_GIT"using ripgrep returned no results; however, the output was minimal, so please manually verify across the codebase to ensure no residual references remain.Ginger/Ginger/SourceControl/SourceControlConnDetailsPage.xaml.cs (2)
118-125: Improved branch handling with fallback to current branchThis change ensures that when the branch information is missing in the user profile, the system attempts to retrieve the current branch dynamically. This improves the data display in the source control download screen.
137-137: Minor formatting improvementAdded a space after the assignment operator for better code readability.
Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (1)
903-904: Set the recent download solution GUID before savingAdded setting of
RecentDownloadedSolutionGuidto ensure proper tracking of the solution before invokingSetSourceControlPropertyOnUserProfile. This maintains consistent state tracking.Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (2)
542-545: Enhanced source control property initializationAdded logic to retrieve source control properties from the user profile when opening a solution with source control. This improves the integration between the user profile and source control systems.
614-617: Added error logging for repository URL retrieval issuesEnhanced error handling by adding logging when there's an error during repository URL retrieval. This helps with diagnosing source control connection issues.
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (2)
291-294: Fixed credential saving logic.The condition now properly uses the captured original value of
DoNotSaveCredentialsOnUserProfileinstead of the temporarily modified value, ensuring that credentials are only saved if the original user setting allows it.
306-306: Removed unnecessary conditional check.The code now always proceeds with saving source control parameters on the user profile when called from
LoadSolutionAsync, as the conditional check has been moved to the caller.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (2)
284-288: 🧹 Nitpick (assertive)Improved credential handling with state preservation.
Created a variable to store the original
DoNotSaveCredentialsOnUserProfilestate and temporarily modifies it to ensure proper solution loading. This approach preserves the user's original preference while allowing the necessary operations to complete.Consider using a try-finally block to ensure the original state is always restored, even if an exception occurs during solution loading:
var DoNotSaveCredentialsOnUserProfile = WorkSpace.Instance.UserProfile.DoNotSaveCredentialsOnUserProfile; +bool originalValue = WorkSpace.Instance.UserProfile.DoNotSaveCredentialsOnUserProfile; +try { if (WorkSpace.Instance.UserProfile.DoNotSaveCredentialsOnUserProfile) { WorkSpace.Instance.UserProfile.DoNotSaveCredentialsOnUserProfile = false; } var result = OpenSolution(); if (result && !DoNotSaveCredentialsOnUserProfile) { SetSourceControlParaOnUserProfile(); } return result; +} +finally { + WorkSpace.Instance.UserProfile.DoNotSaveCredentialsOnUserProfile = originalValue; +}
290-293: 🧹 Nitpick (assertive)Updated condition for saving source control properties.
Modified the condition to use the stored
DoNotSaveCredentialsOnUserProfilevariable instead of directly accessing the property, ensuring the original user preference is respected.Add a comment explaining the credential handling strategy for better readability:
var result = OpenSolution(); +// Only save source control parameters if solution loaded successfully and original preference allows saving if (result && !DoNotSaveCredentialsOnUserProfile) { SetSourceControlParaOnUserProfile(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
Ginger/GingerCoreCommon/UserCofig/UserProfile.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs(1 hunks)Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (1)
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/GingerCoreCommon/UserCofig/UserProfile.cs:748-765
Timestamp: 2025-04-08T10:41:31.325Z
Learning: The GetSolutionSourceControlInfo method in UserProfile class, which retrieves or creates a GingerSolution entry for a specific solution GUID, has been confirmed by the developer to be working as expected without issues of duplicate entries.
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs (1)
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#4128
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:322-325
Timestamp: 2025-04-08T10:41:40.827Z
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.
🧬 Code Definitions (2)
Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (1)
Ginger/GingerCoreCommon/SourceControlLib/GingerSolution.cs (1)
GingerSolution(6-45)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (3)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
WorkSpace(62-1011)Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (2)
UserProfile(82-1010)UserProfile(326-391)Ginger/GingerCoreNET/UserCofig/UserProfileOperations.cs (1)
UserProfile(285-316)
🔇 Additional comments (3)
Ginger/GingerCoreCommon/UserCofig/UserProfile.cs (2)
891-891: Fix source control property retrieval for single solution.Changed the condition to check if there's at least one solution in the collection (
Count > 0) instead of more than one solution. This ensures the method can handle the case when there's only one solution available in the GingerSolutions collection.
903-904: Ensure proper tracking of the recent solution GUID.Added the assignment of
solutionGuidtoRecentDownloadedSolutionGuidbefore callingSetSourceControlPropertyOnUserProfile. This ensures that the recent solution GUID is properly tracked when working with source control.Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
305-305: Removed unnecessary credential check in SetSourceControlParaOnUserProfile.The check for
DoNotSaveCredentialsOnUserProfilehas been removed from this method as it's now handled at the caller level. This simplifies the method and centralizes the credential saving logic.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs (1)
321-337: Enhanced text retrieval logic for text length calculation.The changes to
GetTextLengthAsyncmethod now attempt to retrieve text from multiple sources (TextContent, InnerText, InputValue), providing better support for different types of elements.I can see this approach aligns with your learning about improving capabilities to read text length from various element types. This is a good enhancement for flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs (2)
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#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.
Learnt from: IamRanjeetSingh
PR: Ginger-Automation/Ginger#4128
File: Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs:322-325
Timestamp: 2025-04-08T10:41:40.827Z
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.
🔇 Additional comments (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Web/ActionHandlers/ActUIElementHandler.cs (1)
337-337: Improved error message for missing text.The error message has been updated to "Text value not found" which more accurately describes the issue when no text can be retrieved from any source.
2 participants
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit