Updated Ginger Play configuration check for AI fine Tune Option Remov…#4282
Merged
Conversation
…e Beta Feature Check
Contributor
WalkthroughIntegrates GingerPlayConfiguration into the POM Learn Wizard to gate AI features via a solution-scoped configuration. Adds IsGingerPlayConfigured API in core. Updates the GingerPlay configuration page to use the new API, removes a credentials check method, and alters test-connection preflight validation flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Wizard as POMLearnConfigWizardPage
participant Repo as Repo<GINPlayConfig>
participant Config as GingerPlayConfiguration
User->>Wizard: Open wizard (Init)
Wizard->>Repo: Load first GingerPlayConfiguration
alt Found
Repo-->>Wizard: Existing item
else Not found
Wizard->>Repo: Create & save new item
end
Wizard->>Config: IsGingerPlayConfigured()
alt Configured AND AgentRunning
Wizard-->>User: Show/enable AI UI
else Not configured or Agent stopped
Wizard-->>User: Hide/disable AI UI
end
sequenceDiagram
autonumber
actor User
participant UI as GingerPlayConfigurationpage
participant Config as GingerPlayConfiguration
participant Auth as Token Service
User->>UI: Click "Test Connection"
UI->>Config: IsGingerPlayConfigured()
alt Not configured (per AreRequiredFieldsEmpty inversion)
UI-->>User: Show "Required fields empty" flow
else Configured
UI->>Auth: GetOrValidateToken()
Auth-->>UI: Success/Failure
UI-->>User: Display result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Maheshkale447
approved these changes
Aug 25, 2025
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)
75-77: Reinstate front-end validation for Client ID and Client SecretOnly Gateway URL has a validation rule now. Given IsGingerPlayConfigured() requires all three fields, add matching UI validations for consistent UX.
Apply this diff:
private void ApplyValidationRules() { // check if fields have been populated (font-end validation) xGatewayURLTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Gateway URL cannot be empty")); + xClientIdTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Client ID cannot be empty")); + xClientSecretTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Client Secret cannot be empty")); }Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (2)
86-90: Always bind xLearnPOMByAI; gate enablement instead of binding/visibilityConditional binding prevents the control from reflecting user choices or future config changes at runtime. Bind unconditionally and control enablement/visibility elsewhere.
Apply this diff:
- if(GingerPlayConfiguration.IsGingerPlayConfigured()) - { - xLearnPOMByAI.BindControl(mBasePOMWizard.mPomLearnUtils, nameof(PomLearnUtils.LearnPOMByAI)); - } + xLearnPOMByAI.BindControl(mBasePOMWizard.mPomLearnUtils, nameof(PomLearnUtils.LearnPOMByAI));
219-223: Show AI capability visibly but disabled when not configured (aligns with product preference)Per retrieved learnings for this area, AI features should be discoverable (visible) even when not enabled. Only disable when config/agent isn’t ready.
Apply this diff:
- if(GingerPlayConfiguration.IsGingerPlayConfigured()) - { - xLearnPOMByAI.Visibility = Visibility.Visible; - } + xLearnPOMByAI.Visibility = Visibility.Visible; isEnableFriendlyLocator = true;Enablement is already correctly handled here:
- Line 427: xLearnPOMByAI.IsEnabled = xAgentControlUC.AgentIsRunning && GingerPlayConfiguration.IsGingerPlayConfigured();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs(5 hunks)Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs(2 hunks)Ginger/GingerCoreCommon/External/Configurations/GingerPlayConfiguration.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml:71-103
Timestamp: 2025-08-22T14:57:03.672Z
Learning: In Ginger POM wizard, prashelke prefers to show AI-powered features as visible to users but disabled by default (Visibility="Visible", IsEnabled="False") to promote feature discoverability rather than hiding them completely. Users should be able to see what AI capabilities are available even when not yet enabled.
Learnt from: prashelke
PR: Ginger-Automation/Ginger#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.
📚 Learning: 2025-07-16T14:42:32.229Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#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/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs
📚 Learning: 2025-08-22T14:57:03.672Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#4280
File: Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml:71-103
Timestamp: 2025-08-22T14:57:03.672Z
Learning: In Ginger POM wizard, prashelke prefers to show AI-powered features as visible to users but disabled by default (Visibility="Visible", IsEnabled="False") to promote feature discoverability rather than hiding them completely. Users should be able to see what AI capabilities are available even when not yet enabled.
Applied to files:
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.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 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/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.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/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.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/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs
📚 Learning: 2025-03-20T11:10:30.816Z
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#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/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs
📚 Learning: 2025-06-16T09:52:40.873Z
Learnt from: prashelke
PR: Ginger-Automation/Ginger#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/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs
🧬 Code graph analysis (2)
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (2)
Ginger/GingerCoreCommon/External/Configurations/GingerPlayConfiguration.cs (2)
GingerPlayConfiguration(23-240)IsGingerPlayConfigured(232-238)Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
WorkSpace(61-1006)
Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)
Ginger/GingerCoreCommon/External/Configurations/GingerPlayConfiguration.cs (1)
IsGingerPlayConfigured(232-238)
🔇 Additional comments (2)
Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)
80-105: No remaining inverted‐semantics dependencies foundI searched the entire codebase for all references to AreRequiredFieldsEmpty() and found only these call sites:
- GingerOpsConfigurationPage.xaml.cs (line 77)
- WireMockConfigurationPage.xaml.cs (line 75)
- GingerPlayConfigurationPage.xaml.cs (line 97) (already updated to use
!AreRequiredFieldsEmpty())- GingerOpsConfigPageTest.cs (two test cases)
The Play page’s only caller has been inverted to match the new semantics, and there are no other usages elsewhere. No further changes are needed.
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs (1)
427-428: LGTM: enablement combines agent state + configuration readinessThe IsEnabled gating correctly respects both AgentIsRunning and IsGingerPlayConfigured(). This pairs well with the “visible but disabled” change above.
Comment on lines
+78
to
80
| GingerPlayConfiguration = WorkSpace.Instance.SolutionRepository.GetAllRepositoryItems<GingerPlayConfiguration>().Count == 0 | ||
| ? new GingerPlayConfiguration() : WorkSpace.Instance.SolutionRepository.GetFirstRepositoryItem<GingerPlayConfiguration>(); | ||
| ObservableList<ApplicationPlatform> TargetApplications = GingerCore.General.ConvertListToObservableList(WorkSpace.Instance.Solution.ApplicationPlatforms.Where(x => ApplicationPOMModel.PomSupportedPlatforms.Contains(x.Platform)).ToList()); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure a single, persisted configuration instance (avoid ad-hoc new)
Creating a new GingerPlayConfiguration when none exist means the wizard may gate AI based on a transient object that isn’t persisted. Use the same creation path as the configuration page to guarantee a single repo-backed instance.
Apply this diff:
- GingerPlayConfiguration = WorkSpace.Instance.SolutionRepository.GetAllRepositoryItems<GingerPlayConfiguration>().Count == 0
- ? new GingerPlayConfiguration() : WorkSpace.Instance.SolutionRepository.GetFirstRepositoryItem<GingerPlayConfiguration>();
+ GingerCoreNET.GeneralLib.General.CreateGingerPlayConfiguration();
+ GingerPlayConfiguration = WorkSpace.Instance.SolutionRepository.GetFirstRepositoryItem<GingerPlayConfiguration>();🤖 Prompt for AI Agents
In
Ginger/Ginger/ApplicationModelsLib/POMModels/POMWizardLib/LearnWizard/POMLearnConfigWizardPage.xaml.cs
around lines 78-80, the code creates a transient new GingerPlayConfiguration()
when none exist; instead obtain a single persisted repo-backed instance using
the same creation path as the configuration page (i.e., ask the
SolutionRepository to create/add the new GingerPlayConfiguration and return that
repo item). Replace the ternary new GingerPlayConfiguration() branch with a call
that creates and registers the config in WorkSpace.Instance.SolutionRepository
(or uses the existing helper that the configuration page uses), so the returned
GingerPlayConfiguration is persisted in the repository rather than an ad-hoc
in-memory object.
Comment on lines
80
to
83
| public bool AreRequiredFieldsEmpty() | ||
| { | ||
| return string.IsNullOrEmpty(gingerPlayConfiguration.GingerPlayGatewayUrl); | ||
| } | ||
|
|
||
| public bool AreCredentialsFieldsEmpty() | ||
| { | ||
| return string.IsNullOrEmpty(gingerPlayConfiguration.GingerPlayClientId) || string.IsNullOrEmpty(gingerPlayConfiguration.GingerPlayClientSecret); | ||
| return gingerPlayConfiguration.IsGingerPlayConfigured(); | ||
| } |
Contributor
There was a problem hiding this comment.
Method name vs. behavior is inverted (will cause confusion/bugs)
AreRequiredFieldsEmpty() returns “configured”, not “empty”. This is misleading and error-prone.
Apply this minimal, local fix to align name and behavior without ripples:
- public bool AreRequiredFieldsEmpty()
- {
- return gingerPlayConfiguration.IsGingerPlayConfigured();
- }
+ public bool AreRequiredFieldsEmpty()
+ {
+ // Empty == not configured
+ return !gingerPlayConfiguration.IsGingerPlayConfigured();
+ }Follow-up (optional but recommended): rename the method to IsConfigured or AreRequiredFieldsFilled for clarity across the codebase. I can provide a repo-wide rename script if needed.
📝 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.
Suggested change
| public bool AreRequiredFieldsEmpty() | |
| { | |
| return string.IsNullOrEmpty(gingerPlayConfiguration.GingerPlayGatewayUrl); | |
| } | |
| public bool AreCredentialsFieldsEmpty() | |
| { | |
| return string.IsNullOrEmpty(gingerPlayConfiguration.GingerPlayClientId) || string.IsNullOrEmpty(gingerPlayConfiguration.GingerPlayClientSecret); | |
| return gingerPlayConfiguration.IsGingerPlayConfigured(); | |
| } | |
| public bool AreRequiredFieldsEmpty() | |
| { | |
| // Empty == not configured | |
| return !gingerPlayConfiguration.IsGingerPlayConfigured(); | |
| } |
🤖 Prompt for AI Agents
In Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs
around lines 80-83, the method AreRequiredFieldsEmpty() currently returns
gingerPlayConfiguration.IsGingerPlayConfigured(), which is the opposite of its
name; change the implementation to return the negation of
IsGingerPlayConfigured() so the method truthfully indicates emptiness (i.e.,
return !gingerPlayConfiguration.IsGingerPlayConfigured()). Optionally, consider
renaming the method to IsConfigured or AreRequiredFieldsFilled in a separate
follow-up to avoid future confusion.
Comment on lines
+97
to
105
| if (!AreRequiredFieldsEmpty()) | ||
| { | ||
| Reporter.ToUser(eUserMsgKey.RequiredFieldsEmpty); | ||
| return; | ||
| } | ||
|
|
||
| if (!AreCredentialsFieldsEmpty()) | ||
| else | ||
| { | ||
| bool isAuthorized = await GingerPlayAPITokenManager.GetOrValidateToken(); | ||
| // Show main connection result |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invert the preflight conditional to match the corrected semantics
Once AreRequiredFieldsEmpty() is fixed to return true when empty, the xTestConBtn_Click condition becomes straightforward and self-documenting.
Apply this diff:
- if (!AreRequiredFieldsEmpty())
- {
- Reporter.ToUser(eUserMsgKey.RequiredFieldsEmpty);
- return;
- }
- else
- {
- bool isAuthorized = await GingerPlayAPITokenManager.GetOrValidateToken();
- // Show main connection result
- connectionResultMessage = ShowConnectionResult(isAuthorized);
- }
+ if (AreRequiredFieldsEmpty())
+ {
+ Reporter.ToUser(eUserMsgKey.RequiredFieldsEmpty);
+ return;
+ }
+
+ bool isAuthorized = await GingerPlayAPITokenManager.GetOrValidateToken();
+ connectionResultMessage = ShowConnectionResult(isAuthorized);📝 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.
Suggested change
| if (!AreRequiredFieldsEmpty()) | |
| { | |
| Reporter.ToUser(eUserMsgKey.RequiredFieldsEmpty); | |
| return; | |
| } | |
| if (!AreCredentialsFieldsEmpty()) | |
| else | |
| { | |
| bool isAuthorized = await GingerPlayAPITokenManager.GetOrValidateToken(); | |
| // Show main connection result | |
| if (AreRequiredFieldsEmpty()) | |
| { | |
| Reporter.ToUser(eUserMsgKey.RequiredFieldsEmpty); | |
| return; | |
| } | |
| bool isAuthorized = await GingerPlayAPITokenManager.GetOrValidateToken(); | |
| connectionResultMessage = ShowConnectionResult(isAuthorized); |
🤖 Prompt for AI Agents
In Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs
around lines 97 to 105, the preflight conditional is inverted: after fixing
AreRequiredFieldsEmpty() to return true when fields are empty, change the if to
check AreRequiredFieldsEmpty() directly, call
Reporter.ToUser(eUserMsgKey.RequiredFieldsEmpty) and return in that branch, and
remove the else so the subsequent await
GingerPlayAPITokenManager.GetOrValidateToken() runs when required fields are
present.
Comment on lines
+232
to
+238
| public bool IsGingerPlayConfigured() | ||
| { | ||
| return GingerPlayEnabled && | ||
| !string.IsNullOrEmpty(GingerPlayGatewayUrl) && | ||
| !string.IsNullOrEmpty(GingerPlayClientId) && | ||
| !string.IsNullOrEmpty(GingerPlayClientSecret); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden IsGingerPlayConfigured against whitespace and weak URL values
Use IsNullOrWhiteSpace for credentials and (at minimum) trim inputs before evaluating. Today, a value like " " for URL gets normalized to " /" by the setter and would incorrectly pass a simple IsNullOrEmpty check.
Apply this diff:
- public bool IsGingerPlayConfigured()
- {
- return GingerPlayEnabled &&
- !string.IsNullOrEmpty(GingerPlayGatewayUrl) &&
- !string.IsNullOrEmpty(GingerPlayClientId) &&
- !string.IsNullOrEmpty(GingerPlayClientSecret);
- }
+ public bool IsGingerPlayConfigured()
+ {
+ return GingerPlayEnabled
+ && !string.IsNullOrWhiteSpace(GingerPlayGatewayUrl)
+ && !string.IsNullOrWhiteSpace(GingerPlayClientId)
+ && !string.IsNullOrWhiteSpace(GingerPlayClientSecret);
+ }Note: Consider trimming in the GingerPlayGatewayUrl setter and optionally validating with Uri.TryCreate to avoid accepting " /" as a “configured” URL. I can draft that setter change if you want.
📝 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.
Suggested change
| public bool IsGingerPlayConfigured() | |
| { | |
| return GingerPlayEnabled && | |
| !string.IsNullOrEmpty(GingerPlayGatewayUrl) && | |
| !string.IsNullOrEmpty(GingerPlayClientId) && | |
| !string.IsNullOrEmpty(GingerPlayClientSecret); | |
| } | |
| public bool IsGingerPlayConfigured() | |
| { | |
| return GingerPlayEnabled | |
| && !string.IsNullOrWhiteSpace(GingerPlayGatewayUrl) | |
| && !string.IsNullOrWhiteSpace(GingerPlayClientId) | |
| && !string.IsNullOrWhiteSpace(GingerPlayClientSecret); | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreCommon/External/Configurations/GingerPlayConfiguration.cs
around lines 232 to 238, the IsGingerPlayConfigured method currently uses
IsNullOrEmpty and can be fooled by whitespace or weak URL values; update the
check to use string.IsNullOrWhiteSpace for GingerPlayGatewayUrl,
GingerPlayClientId and GingerPlayClientSecret and ensure inputs are trimmed
before evaluation (preferably trim in the GingerPlayGatewayUrl setter and client
id/secret setters); additionally, consider validating GingerPlayGatewayUrl with
Uri.TryCreate (absolute/HTTP(S) as appropriate) so values like " /" are not
treated as a valid configured URL.
Maheshkale447
approved these changes
Aug 25, 2025
This was referenced Sep 3, 2025
15 tasks
15 tasks
This was referenced Oct 9, 2025
Merged
15 tasks
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
2 participants
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.
…e Beta Feature Check
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Bug Fixes