design issues fix#4333
Conversation
WalkthroughCentralizes Ginger Play gateway URL retrieval into a new GingerPlayUtils method and updates callers (TwoLevelMenu and GingerPlay config) to use it; replaces some conditional URL-launch flows with direct banner display in one handler. Multiple XAML files adjust right-pane margins, typography, and remove or alter ImageMakerControl usages. TwoLevelMenu Upgrade control reimplemented using a Button with a ControlTemplate and new gradient/hover styling. No public API breaks; one utility method added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Ginger UI (GingerPlay / TwoLevelMenu)
participant Utils as GingerPlayUtils
participant OS as OS (Process.Start)
participant Banner as BannerWindow
User->>UI: Click "Learn More" / "Upgrade"
UI->>Utils: GetGingerPlayGatewayURLIfConfigured()
alt URL returned (non-empty)
Utils-->>UI: URL
UI->>OS: Process.Start(URL)
alt Process.Start success
OS-->>User: Browser opened
else Process.Start throws
OS-->>UI: Exception
UI-->>User: Show error message
end
else URL null/empty
Utils-->>UI: null/empty
UI->>Banner: ShowGingerPlayBannerWindow()
Banner-->>User: Display banner UI
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-08-26T07:40:08.345ZApplied to files:
🔇 Additional comments (7)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs(2 hunks)Ginger/Ginger/UserControlsLib/TwoLevelMenuLib/TwoLevelMenuPage.xaml.cs(2 hunks)Ginger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#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/Ginger/UserControlsLib/TwoLevelMenuLib/TwoLevelMenuPage.xaml.csGinger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs
📚 Learning: 2025-04-25T13:29:45.059Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#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/Ginger/UserControlsLib/TwoLevelMenuLib/TwoLevelMenuPage.xaml.csGinger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs
📚 Learning: 2025-06-16T10:37:13.073Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#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/GeneralLib/GingerPlayUtils.cs
📚 Learning: 2025-03-20T11:10:33.780Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:33.780Z
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/GingerCoreNET/GeneralLib/GingerPlayUtils.cs
🧬 Code graph analysis (3)
Ginger/Ginger/UserControlsLib/TwoLevelMenuLib/TwoLevelMenuPage.xaml.cs (1)
Ginger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs (2)
GingerPlayUtils(9-42)GetGingerPlayGatewayURLIfConfigured(24-41)
Ginger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs (3)
Ginger/GingerCoreCommon/External/Configurations/GingerPlayConfiguration.cs (1)
GingerPlayConfiguration(23-253)Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
WorkSpace(61-1006)Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
Reporter(28-357)
Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)
Ginger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs (2)
GingerPlayUtils(9-42)GetGingerPlayGatewayURLIfConfigured(24-41)
🔇 Additional comments (4)
Ginger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs (2)
2-2: LGTM: Needed import for Reporter/eLogLevelRequired for Reporter.ToLog usage.
24-41: Preserve legacy URLs in GetGingerPlayGatewayURLIfConfigured to avoid UI regressions
- In Ginger/GingerCoreNET/GeneralLib/GingerPlayUtils.cs, after checking GingerPlayGatewayUrl and GingerPlayEnabled, fallback to CentralizedAccountReportURL or CentralizedHTMLReportServiceURL when non-empty.
- Continue logging only at DEBUG and return null on all “not configured/error” paths.
⛔ Skipped due to learnings
Learnt from: AmanPrasad43 PR: Ginger-Automation/Ginger#4314 File: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/DynamicExecutionManager.cs:960-976 Timestamp: 2025-09-30T06:03:09.397Z Learning: In GingerPlayDetails configuration mapping (file: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/DynamicExecutionManager.cs), both EnableAccountReportService and EnableHTMLReportService should be mapped from gingerPlayConfig.GingerPlayReportServiceEnabled as they represent the same underlying service.Ginger/Ginger/UserControlsLib/TwoLevelMenuLib/TwoLevelMenuPage.xaml.cs (1)
24-24: LGTM: New utility importImport is correct for using GingerPlayUtils.
Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)
27-27: LGTM: New utility importRequired for GingerPlayUtils usage.
3 participants
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
Style
Bug Fixes