UN-1184 [FEAT] Add documentation link popover for connectors#1787
Merged
hari-kuriakose merged 6 commits intomainfrom Feb 26, 2026
Merged
UN-1184 [FEAT] Add documentation link popover for connectors#1787hari-kuriakose merged 6 commits intomainfrom
hari-kuriakose merged 6 commits intomainfrom
Conversation
Contributor
Summary by CodeRabbitRelease Notes
WalkthroughAdds connector documentation URLs end-to-end: new DOC_URL constant and metadata field, get_doc_url() on connector classes and connector kit, and UI plumbing to surface a documentation link in the Add Source / Configure Source modal. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Frontend UI
participant API as Backend API
participant Kit as ConnectorKit
User->>UI: Open Add Source modal
UI->>API: GET /connectors (list)
API->>Kit: request connectors list
Kit->>Kit: collect metadata (includes get_doc_url())
Kit-->>API: connectors list (includes doc_url)
API-->>UI: connectors list JSON
UI->>UI: set selectedDocUrl from chosen source.doc_url
UI->>UI: render ConfigureDs with documentation popover linking to selectedDocUrl
User->>UI: click doc icon -> open documentation URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
kirtimanmishrazipstack
changed the title
kirtimanmishrazipstack
requested review from
a team,
Deepak-Kesavan and
gaya3-zipstack
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/input-output/add-source-modal/AddSourceModal.jsx (1)
69-75:⚠️ Potential issue | 🟡 Minor
sourcesListmissing from the dependency array may leaveselectedDocUrlempty during the edit flow.When editing an existing connector,
getSourceDetailssetsselectedSourceIdbefore thegetListOfSourcescall (triggered by[open]) has resolved. ThesourcesList.find(...)on Line 70 will returnundefined, leavingselectedDocUrlas"". AddingsourcesListto the dependency array would re-resolve the doc URL once the list is available.Proposed fix
- }, [selectedSourceId]); + }, [selectedSourceId, sourcesList]);
🧹 Nitpick comments (1)
frontend/src/components/input-output/configure-ds/ConfigureDs.css (1)
29-33: Consider using the Ant Design theme token instead of hardcoding the icon color.
#1890ffis the legacy Ant Design v4 primary color. Since the project uses antd v5, the default primary is#1677ff. Hardcoding may drift from the app's theme. Consider using a CSS variable or the design token (e.g., viatoken.colorPrimaryfrom the theme) for consistency.
Contributor
|
Please fill in the PR teamplate with appropriate information @kirtimanmishrazipstack |
Contributor
gaya3-zipstack
left a comment
gaya3-zipstack
left a comment
There was a problem hiding this comment.
Other than template missing information, backed changes look fine. Please fix PR template @kirtimanmishrazipstack
johnyrahul
approved these changes
Feb 18, 2026
Contributor
|
NIT: @kirtimanmishrazipstack I feel it would be better to include the link directly in the blue info box instead of using a mouse-over help tooltip. “OAuth not supported yet. View setup guide →” may be With a short quick guide(not mandatory). Please check with Tanushree. |
Contributor
Author
@muhammad-ali-e I think this is better to have info button. Had a chat with tanushree |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/input-output/add-source-modal/AddSourceModal.jsx (1)
72-78: Implementation follows existing pattern - consider addingsourcesListto dependencies.The new
selectedDocUrlassignment correctly mirrors the existingselectedSourceNamepattern. However, the dependency array is missingsourcesList, which could cause stale values if the sources list updates after a source is already selected (e.g., race condition during edit mode wheregetSourceDetails()may resolve beforegetListOfSources()).This is a pre-existing pattern issue, so it can be deferred, but adding
sourcesListto the dependency array would make both assignments more robust.♻️ Optional fix for dependency array
- }, [selectedSourceId]); + }, [selectedSourceId, sourcesList]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/input-output/add-source-modal/AddSourceModal.jsx` around lines 72 - 78, The useEffect that sets selectedSourceName and selectedDocUrl currently only depends on selectedSourceId and can read stale data if sourcesList changes; update the dependency array of that useEffect to include sourcesList so the effect re-runs when the list of sources updates (locate the useEffect in AddSourceModal.jsx that calls setSelectedSourceName and setSelectedDocUrl and add sourcesList to its dependencies).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/input-output/add-source-modal/AddSourceModal.jsx`:
- Around line 72-78: The useEffect that sets selectedSourceName and
selectedDocUrl currently only depends on selectedSourceId and can read stale
data if sourcesList changes; update the dependency array of that useEffect to
include sourcesList so the effect re-runs when the list of sources updates
(locate the useEffect in AddSourceModal.jsx that calls setSelectedSourceName and
setSelectedDocUrl and add sourcesList to its dependencies).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
backend/connector_processor/connector_processor.pyfrontend/src/components/input-output/add-source-modal/AddSourceModal.jsxfrontend/src/components/input-output/add-source/AddSource.jsxfrontend/src/components/input-output/configure-ds/ConfigureDs.jsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/input-output/configure-ds/ConfigureDs.jsx
- backend/connector_processor/connector_processor.py
- frontend/src/components/input-output/add-source/AddSource.jsx
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
Contributor
|
Sonar report can be ignored. |
hari-kuriakose
deleted the
UN-1184-add-doc-link-for-connectors-while-adding
branch
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
5 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.
What
Why
How
Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.