Skip to content

improvement(knowledge): show selector with saved option in connector edit modal#4240

Merged
waleedlatif1 merged 5 commits intostagingfrom
waleedlatif1/kb-edit-selector
Apr 21, 2026
Merged

improvement(knowledge): show selector with saved option in connector edit modal#4240
waleedlatif1 merged 5 commits intostagingfrom
waleedlatif1/kb-edit-selector

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • The connector edit modal now renders the selector dropdown (pre-selected with the saved option) instead of a raw ID input for fields with canonicalParamId, matching the add-connector flow and the selector behavior in the workflow editor
  • Added basic/advanced toggle so users can still reveal and edit the raw ID
  • Fixes a latent bug where selector-type fields (e.g. Teams teamSelector, channelSelector) rendered as empty inputs because state was canonical-keyed but rendered by field.id
  • Canonical keys are resolved on save, preserving sync-engine-injected keys (tagSlotMapping, disabledTagIds) via {...connector.sourceConfig, ...resolved} merge

Type of Change

  • Improvement

Testing

Tested manually against Teams (two canonical pairs with dependency chain), verified dropdown pre-selection, toggle behavior, dependency clearing, and save round-trip.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@gitguardian
Copy link
Copy Markdown

gitguardian bot commented Apr 20, 2026

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Apr 21, 2026 3:33am

Request Review

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 20, 2026

PR Summary

Medium Risk
Touches connector configuration state/serialization and edit-save logic; regressions could cause incorrect config values to be submitted or fields to clear unexpectedly, but scope is UI-layer and localized.

Overview
Unifies connector config-field behavior across add and edit modals by introducing useConnectorConfigFields, which centralizes canonical-field grouping, basic/advanced visibility, dependency-driven clearing, and canonical-key resolution for submission.

The edit connector modal now renders selector fields via ConnectorSelectorField (pre-populated from saved canonical values) and adds a toggle to switch between selector vs manual ID input for canonical pairs; saving and change detection now operate on resolved canonical keys while preserving sync-engine injected keys via merge.

Reviewed by Cursor Bugbot for commit e6f17ca. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 20, 2026

Greptile Summary

This PR fixes the edit-connector modal rendering selector dropdowns (pre-populated with the saved value) instead of raw ID inputs for canonicalParamId fields, and adds the basic/advanced toggle to expose manual entry. The duplicate canonical-pair and dependency-clearing logic previously baked into add-connector-modal.tsx is extracted into a new useConnectorConfigFields hook, and ConnectorSelectorField is moved to a shared path consumed by both modals.

Confidence Score: 5/5

Safe to merge — no logic or data-integrity issues found; all prior review concerns have been addressed.

All remaining findings are P2 style suggestions. The canonical-pair logic, dependency clearing, seed-once initialSourceConfig, internal-key preservation, and resolveSourceConfig all behave correctly. Previous thread concerns (shared path, mount lifecycle, memoization) are resolved or deliberately accepted.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts New shared hook extracting canonical-pair state, dependency clearing, and resolve logic; well-structured with correct Set-based deduplication and sibling expansion.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal.tsx Refactored to use shared hook; correctly seeds initialSourceConfig once, preserves internal keys (tagSlotMapping, disabledTagIds) via spread-merge on save, renders selector dropdowns for canonical-pair fields.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx Duplicate canonical/dependency logic removed and replaced with useConnectorConfigFields hook; imports updated to shared connector-selector-field path.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx Moved from add-connector-modal internals to shared location; resolveDepValue correctly reads the active canonical sibling's value for dependency resolution.
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/index.ts Barrel export for the shared component; uses absolute alias path instead of the conventional relative path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[EditConnectorModal / AddConnectorModal] -->|initialSourceConfig| B[useConnectorConfigFields]
    B --> C{canonicalGroups}
    C -->|mode: basic| D[ConnectorSelectorField\n selector Combobox]
    C -->|mode: advanced| E[Input\n raw ID]
    B --> F[dependentFieldIds\n sibling-expanded]
    F -->|handleFieldChange| G[clear downstream fields]
    B --> H[resolveSourceConfig\n canonical-keyed output]
    H -->|save| I[updateConnector / createConnector\n ...sourceConfig + changedEntries]
Loading

Reviews (4): Last reviewed commit: "refactor(kb-connectors): tighten state p..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 force-pushed the waleedlatif1/kb-edit-selector branch from c0866de to 958d106 Compare April 20, 2026 23:54
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

…it save

Avoids writing spurious empty-string keys for untouched optional fields when
another field triggers a save.
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

- edit modal: replace useMemo([]) + eslint-disable with useState lazy
  initializer for initialSourceConfig — same mount-once semantics
  without the escape hatch.
- add modal: drop useCallback on handleConnectNewAccount (no observer
  saw the reference) and inline the one call site.
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e6f17ca. Configure here.

@waleedlatif1 waleedlatif1 merged commit 94202ed into staging Apr 21, 2026
13 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/kb-edit-selector branch April 21, 2026 03:50
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.

1 participant