Skip to content

External Configuration Pages Current Entity Saved items Issue fixed#4291

Merged
Maheshkale447 merged 2 commits into
Releases/Betafrom
BugFix/ExternalIntegrationCurrentSavingDirtyMark
Sep 3, 2025
Merged

External Configuration Pages Current Entity Saved items Issue fixed#4291
Maheshkale447 merged 2 commits into
Releases/Betafrom
BugFix/ExternalIntegrationCurrentSavingDirtyMark

Conversation

@AmanPrasad43

@AmanPrasad43 AmanPrasad43 commented Sep 3, 2025

Copy link
Copy Markdown
Contributor

Thank you for your contribution.
Before submitting this PR, please make sure:

  • PR description and commit message should describe the changes done in this PR
  • Verify the PR is pointing to correct branch i.e. Release or Beta branch if the code fix is for specific release , else point it to master
  • Latest Code from master or specific release branch is merged to your branch
  • No unwanted\commented\junk code is included
  • No new warning upon build solution
  • Code Summary\Comments are added to my code which explains what my code is doing
  • Existing unit test cases are passed
  • New Unit tests are added for your development
  • Sanity Tests are successfully executed for New and Existing Functionality
  • Verify that changes are compatible with all relevant browsers and platforms.
  • After creating pull request there should not be any conflicts
  • Resolve all Codacy comments
  • Builds and checks are passed before PR is sent for review
  • Resolve code review comments
  • Update the Help Library document to match any feature changes

Summary by CodeRabbit

  • Bug Fixes
    • Opened configuration pages (AskLisa, GingerOps, GingerPlay, WireMock, ZAP) now correctly select and focus the loaded item in the workspace.
    • Toolbars and actions reliably operate on the intended configuration immediately after opening.
    • Provides a consistent UI context and reduces confusion when switching between configuration pages.

@coderabbitai

coderabbitai Bot commented Sep 3, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Five ExternalConfigurations pages now set WorkSpace.Instance.CurrentSelectedItem to the loaded configuration immediately after StartDirtyTracking() and before calling SetControls(); no public API or other logic changes.

Changes

Cohort / File(s) Summary
External Config Pages: set CurrentSelectedItem during Init
Ginger/Ginger/ExternalConfigurations/AskLisaConfigurationsPage.xaml.cs, Ginger/Ginger/ExternalConfigurations/GingerOpsConfigurationPage.xaml.cs, Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs, Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml.cs, Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
In each Init(), after StartDirtyTracking(), assign WorkSpace.Instance.CurrentSelectedItem to the respective configuration instance, then call SetControls(); no signature, error-handling, or other logic changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Page as Config Page (Init)
  participant WS as WorkSpace
  participant UI as Controls

  User->>Page: Open configuration page
  activate Page
  Page->>Page: StartDirtyTracking()
  Page->>WS: Set CurrentSelectedItem = <ConfigInstance>
  Note right of WS #DFF2E1: Workspace selection updated
  Page->>UI: SetControls()
  deactivate Page
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • Maheshkale447
  • prashelke
  • MeniKadosh1

Poem

I hop in code with whiskers bright,
I set the selection just right.
Dirty tracked and controls in tune,
Five configs hum—beneath the moon.
🐰✨


📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2c4049b and a83fca0.

📒 Files selected for processing (1)
  • Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-28T09:27:18.793Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebAgentConfigEditPage.xaml.cs:556-571
Timestamp: 2025-08-28T09:27:18.793Z
Learning: In Ginger's WebAgentConfigEditPage.xaml.cs ZAP configuration, when ZAP Security Testing is enabled, the manual proxy UI should remain enabled/accessible even though ZAP will override the proxy settings in the background. This allows users to see and modify their proxy configurations for transparency and future use when ZAP is disabled. Confirmed by AmanPrasad43 in PR #4286.

Applied to files:

  • Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
📚 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/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
🧬 Code graph analysis (1)
Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (1)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
  • WorkSpace (61-1006)
🔇 Additional comments (2)
Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (2)

47-48: Init order fixed — aligns with workspace dirty tracking expectations.

Calling StartDirtyTracking → setting CurrentSelectedItem → SetControls prevents bindings from capturing pre-tracked state. Good consistency with other pages.


45-46: Confirm persistence when no configuration exists.

If none exists, a new ZAPConfiguration is created but not added to the repository. Verify that later save flows persist it; otherwise dirty tracking may track a transient instance.

Would you like a follow-up patch to AddRepositoryItem on first creation?

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BugFix/ExternalIntegrationCurrentSavingDirtyMark

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
Ginger/Ginger/ExternalConfigurations/AskLisaConfigurationsPage.xaml.cs (1)

90-99: Fix misleading comment (ClientId vs ClientSecret).

The comment above xClientId handler references “client secret”.

Apply:

-        // Encrypt the client secret unless it's already encrypted or a value expression.
-        // This ensures that sensitive data is stored securely without altering predefined expressions or duplicating encryption.
+        // Encrypt the client ID unless it's already encrypted or a value expression.
+        // Ensures sensitive data is stored securely without altering predefined expressions or duplicating encryption.
Ginger/Ginger/ExternalConfigurations/GingerOpsConfigurationPage.xaml.cs (1)

68-71: Fix typo in user-facing validation text.

“Identitiy” → “Identity”.

Apply:

-            xISURLTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Identitiy Service URL cannot be empty"));
+            xISURLTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("Identity Service URL cannot be empty"));
Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (3)

63-66: Validation rule bound to wrong control and message mismatch.

Rule added to API key textbox with “ZAP URL cannot be empty”. Add correct rules for both fields.

Apply:

-            xZAPAPIkeyTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("ZAP URL cannot be empty"));
+            xZAPURLTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("ZAP URL cannot be empty"));
+            xZAPAPIkeyTextBox.ValueTextBox.AddValidationRule(new ValidateEmptyValue("ZAP API key cannot be empty"));

136-140: Possible compile-time issue: string.EndsWith('/') overload.

Use string overload or char check.

Apply:

-                if (zapUrl.EndsWith('/'))
+                if (zapUrl.EndsWith("/"))
                 {
                     zapUrl = zapUrl.TrimEnd('/');
                 }

144-146: Add a reasonable HTTP timeout.

Avoid hanging UI on slow endpoints.

Apply:

-                using HttpClient client = new HttpClient();
+                using HttpClient client = new HttpClient();
+                client.Timeout = TimeSpan.FromSeconds(15);
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between fd572e4 and 2c4049b.

📒 Files selected for processing (5)
  • Ginger/Ginger/ExternalConfigurations/AskLisaConfigurationsPage.xaml.cs (1 hunks)
  • Ginger/Ginger/ExternalConfigurations/GingerOpsConfigurationPage.xaml.cs (1 hunks)
  • Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1 hunks)
  • Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml.cs (1 hunks)
  • Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-02-14T07:02:48.416Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4100
File: Ginger/Ginger/ApplicationModelsLib/APIModels/APIModelWizard/AddAPIModelExtraConfigsPage.xaml:28-30
Timestamp: 2025-02-14T07:02:48.416Z
Learning: In AddAPIModelExtraConfigsPage.xaml, the WireMock mapping checkbox (xWireMockMappingToggle) is initially disabled and only gets enabled after successful validation of WireMock configuration and connection test in the CheckWireMockConfiguration method.

Applied to files:

  • Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml.cs
📚 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/ExternalConfigurations/WireMockConfigurationPage.xaml.cs
  • Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs
  • Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
  • Ginger/Ginger/ExternalConfigurations/AskLisaConfigurationsPage.xaml.cs
  • Ginger/Ginger/ExternalConfigurations/GingerOpsConfigurationPage.xaml.cs
📚 Learning: 2025-08-28T09:27:18.793Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4286
File: Ginger/Ginger/Drivers/DriversConfigsEditPages/WebAgentConfigEditPage.xaml.cs:556-571
Timestamp: 2025-08-28T09:27:18.793Z
Learning: In Ginger's WebAgentConfigEditPage.xaml.cs ZAP configuration, when ZAP Security Testing is enabled, the manual proxy UI should remain enabled/accessible even though ZAP will override the proxy settings in the background. This allows users to see and modify their proxy configurations for transparency and future use when ZAP is disabled. Confirmed by AmanPrasad43 in PR #4286.

Applied to files:

  • Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
🧬 Code graph analysis (5)
Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml.cs (1)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
  • WorkSpace (61-1006)
Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
  • WorkSpace (61-1006)
Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (1)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
  • WorkSpace (61-1006)
Ginger/Ginger/ExternalConfigurations/AskLisaConfigurationsPage.xaml.cs (1)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
  • WorkSpace (61-1006)
Ginger/Ginger/ExternalConfigurations/GingerOpsConfigurationPage.xaml.cs (1)
Ginger/GingerCoreNET/WorkSpaceLib/WorkSpace.cs (1)
  • WorkSpace (61-1006)
🔇 Additional comments (5)
Ginger/Ginger/ExternalConfigurations/WireMockConfigurationPage.xaml.cs (1)

46-49: Set CurrentSelectedItem after StartDirtyTracking — good.

This aligns save/dirty context with the loaded WireMockConfiguration and matches the PR’s intent.

Ginger/Ginger/ExternalConfigurations/GingerPlayConfigurationpage.xaml.cs (1)

52-55: Correct placement of CurrentSelectedItem.

Assigned immediately after StartDirtyTracking and before bindings. Looks good.

Ginger/Ginger/ExternalConfigurations/AskLisaConfigurationsPage.xaml.cs (1)

46-49: Consistent selection update.

Setting CurrentSelectedItem after StartDirtyTracking matches the pattern across pages.

Ginger/Ginger/ExternalConfigurations/GingerOpsConfigurationPage.xaml.cs (1)

47-50: Selection set post-StartDirtyTracking — OK.

Ensures workspace points at the edited GingerOpsConfiguration.

Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs (1)

48-49: Selection update is correct.

Assigned after StartDirtyTracking; aligns with other pages.

Comment thread Ginger/Ginger/ExternalConfigurations/ZAPConfigurationPage.xaml.cs
@Maheshkale447
Maheshkale447 merged commit afffe35 into Releases/Beta Sep 3, 2025
11 checks passed
@Maheshkale447
Maheshkale447 deleted the BugFix/ExternalIntegrationCurrentSavingDirtyMark branch September 3, 2025 16:46
@coderabbitai coderabbitai Bot mentioned this pull request Sep 13, 2025
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants