Move devant marketplace to same level as bi connectors#1529
Merged
kaje94 merged 2 commits intorelease/bi-1.8.xfrom Feb 20, 2026
Merged
Move devant marketplace to same level as bi connectors#1529kaje94 merged 2 commits intorelease/bi-1.8.xfrom
kaje94 merged 2 commits intorelease/bi-1.8.xfrom
Conversation
Contributor
📝 WalkthroughWalkthroughReworks Devant marketplace integration: replaces a callback with an optional DevantServicesSection component prop, adds searchText propagation and debounced filtering to connector lists, and removes marketplace-specific popup state in favor of a single AddConnectionPopupContent-driven flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Popup as DevantConnectorPopup
participant Content as AddConnectionPopupContent
participant Services as DevantServicesSection / DevantConnectorList
participant API as Marketplace API
Popup->>Content: render with props (searchText, handlers)
Content->>Services: render with searchText
Services->>API: fetch marketplaceServices(debouncedSearchText, filterType)
API-->>Services: return services
Services-->>Content: list of services / loading state
Content-->>Popup: invoke handler on selection (create/register flow)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRsSuggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
kaje94
changed the title
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionPopup/AddConnectionPopupContent.tsx (1)
79-82:⚠️ Potential issue | 🟡 MinorDouble fetch on mount: both effects call
fetchConnectors()simultaneously.The effect at line 79 (empty deps) and the effect at line 147 (
[filterType, fetchConnectors]) both fire on mount, causing two concurrent identical API calls. Consider removing the first one since the second already covers the initial load.Proposed fix
- useEffect(() => { - setIsSearching(true); - fetchConnectors(); - }, []);Also applies to: 147-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionPopup/AddConnectionPopupContent.tsx` around lines 79 - 82, Remove the redundant mount-only effect that calls fetchConnectors() (the useEffect with empty deps that sets setIsSearching(true) and calls fetchConnectors) to avoid double API requests; instead rely on the existing effect that depends on [filterType, fetchConnectors] to perform the initial load, and ensure any necessary setIsSearching(true) behavior is moved into or executed before the dependency-based effect (or inside fetchConnectors) so loading state is still handled correctly.workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorPopup.tsx (1)
535-547:⚠️ Potential issue | 🟠 MajorInline arrow function as
React.ComponentTypecauses remount on every parent render.Since
DevantServicesSectionis typed asReact.ComponentTypeand rendered via JSX (<DevantServicesSection ... />), passing an inline arrow function means React sees a new component type on every render ofDevantConnectorPopup. This unmounts and remounts the entireDevantConnectorListsubtree (losing its internal query cache, filter state, etc.) whenever any state in this parent changes.Extract the component or switch to a render-prop pattern that doesn't go through JSX reconciliation:
Option A: Extract a stable component using useCallback
+ const DevantServicesSectionComponent = useMemo(() => { + return function DevantServices({ searchText }: { searchText: string }) { + return ( + <DevantConnectorList + fileName={fileName} + target={target} + onItemSelect={(flow, item, availableNode) => { + setSelectedFlow(flow); + setSelectedMarketplaceItem(item); + setAvailableNode(availableNode); + }} + searchText={searchText} + /> + ); + }; + }, [fileName, target]); + ... <AddConnectionPopupContent {...props} handleSelectConnector={...} handleApiSpecConnection={...} - DevantServicesSection={({ searchText }) => ( - <DevantConnectorList - fileName={fileName} - target={target} - onItemSelect={(flow, item, availableNode) => { - setSelectedFlow(flow); - setSelectedMarketplaceItem(item); - setAvailableNode(availableNode); - }} - searchText={searchText} - /> - )} + DevantServicesSection={DevantServicesSectionComponent} />Note: The
onItemSelectcallback insideuseMemowill capture stale closures forsetSelectedFlow, etc. Since these are state setters (stable references fromuseState), this is safe. IfonItemSelectever needs to read current state, use a ref instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorPopup.tsx` around lines 535 - 547, The DevantServicesSection prop is being passed an inline arrow component which causes React to treat it as a new component type on each render and remount DevantConnectorList; extract a stable component or provide a stable render-prop instead: create a memoized/stable component (e.g. via useCallback/useMemo or a top-level DevantServicesSectionComponent) that returns <DevantConnectorList ... /> and reference that from DevantConnectorPopup so DevantConnectorList (and its internal state/query cache) is not unmounted; ensure the onItemSelect handler still calls the stable state setters setSelectedFlow, setSelectedMarketplaceItem, and setAvailableNode (these setters are stable so can be referenced safely).
🧹 Nitpick comments (4)
workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx (1)
125-131: Consider usingelse ifor constructing params in a single expression for clarity.The sequential
ifblocks mutatinggetMarketPlaceParamswork but are slightly fragile — a future "internal" case could accidentally combine with "thirdParty" fields if the conditions overlap.Proposed fix
- if(filterType === "internal") { - getMarketPlaceParams.isThirdParty = false; - } - if(filterType === "thirdParty") { + if (filterType === "internal") { + getMarketPlaceParams.isThirdParty = false; + } else if (filterType === "thirdParty") { getMarketPlaceParams.isThirdParty = true; getMarketPlaceParams.networkVisibilityFilter = "org,project,public"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx` around lines 125 - 131, The current sequential ifs in DevantConnectorList.tsx mutate getMarketPlaceParams based on filterType and risk accidentally applying both branches; change the logic to either use an else if (if (filterType === "internal") ... else if (filterType === "thirdParty") ...) or build getMarketPlaceParams in a single expression (e.g., derive a params object from filterType) so only the intended properties (isThirdParty and networkVisibilityFilter) are set for a given filterType; update the code that references getMarketPlaceParams accordingly.workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorPopup.tsx (2)
225-228: Remove commented-out code.This dead code (
showDevantMarketplacereferences) should be deleted rather than left commented out, as it adds noise and the VCS history preserves it.Proposed fix
} else if (currentStepIndex === 0) { setSelectedFlow(null); setSelectedMarketplaceItem(null); deleteTempConfig(); setAvailableNode(undefined); setOasConnectorName(""); setIDLFilePath(""); - - // if (showDevantMarketplace && !selectedFlow) { - // setShowDevantMarketplace(false); - // } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorPopup.tsx` around lines 225 - 228, Delete the commented-out dead code block referencing showDevantMarketplace/selectedFlow (the lines with "if (showDevantMarketplace && !selectedFlow) { setShowDevantMarketplace(false); }") inside DevantConnectorPopup (remove the commented lines and trailing comment markers) so the file no longer contains those unused references; rely on VCS history if the code needs to be restored.
522-534: Verify that extra props from the spread don't cause issues.
{...props}passesonClose,onNavigateToOverview,isPopup, andprojectPathwhich are not declared inAddConnectionPopupContent'sPropsinterface. While React ignores extra props on function components, this could cause console warnings ifAddConnectionPopupContentever becomes a wrapper around a DOM element, or confusion if the interface evolves.Consider explicitly passing only the needed props:
Proposed fix
<AddConnectionPopupContent - {...props} + fileName={fileName} + target={target} handleSelectConnector={(availableNode, _) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorPopup.tsx` around lines 522 - 534, The spread {...props} on the AddConnectionPopupContent call may pass extraneous keys (onClose, onNavigateToOverview, isPopup, projectPath) not declared in AddConnectionPopupContent's Props and could cause future warnings or confusion; replace the spread with an explicit prop list containing only the values used by AddConnectionPopupContent (e.g., explicitly pass the specific props it expects) and keep the existing handlers (handleSelectConnector, handleApiSpecConnection) and state setters (setAvailableNode, setSelectedFlow, DevantConnectionFlow.REGISTER_CREATE_THIRD_PARTY_FROM_BI_CONNECTOR, DevantConnectionFlow.REGISTER_CREATE_THIRD_PARTY_FROM_OAS) unchanged so consumers receive only the declared props.workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionPopup/AddConnectionPopupContent.tsx (1)
69-70: Remove debugconsole.logstatements before merging.Multiple
console.logcalls (lines 69–70, 103–104, 128) are present in the component. These appear to be development artifacts and should be cleaned up.Also applies to: 103-104, 128-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionPopup/AddConnectionPopupContent.tsx` around lines 69 - 70, Remove the leftover debug console.log calls in the AddConnectionPopupContent component: delete the console.log statements that print ">>> bi connectors", ">>> bi filtered connectors" and any other console.log usages in this file (including those referencing model and model.categories); if persistent runtime logging is required, replace with the project's logger (e.g., processLogger or a designated logger) or conditional debug flag instead of console.log to avoid leaking dev output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx`:
- Around line 190-197: The onItemClick currently forces
DevantConnectionType.INTERNAL for every marketplace item; update the
ConnectionSection onItemClick to compute the proper type using the clicked
item's isThirdParty flag (e.g., const type = item.isThirdParty ?
DevantConnectionType.THIRD_PARTY : DevantConnectionType.INTERNAL) and pass that
type into handleMarketplaceItemClick so handleMarketplaceItemClick receives the
correct DevantConnectionType for routing; change the onItemClick prop in
DevantConnectorList (the ConnectionSection call) accordingly.
---
Outside diff comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionPopup/AddConnectionPopupContent.tsx`:
- Around line 79-82: Remove the redundant mount-only effect that calls
fetchConnectors() (the useEffect with empty deps that sets setIsSearching(true)
and calls fetchConnectors) to avoid double API requests; instead rely on the
existing effect that depends on [filterType, fetchConnectors] to perform the
initial load, and ensure any necessary setIsSearching(true) behavior is moved
into or executed before the dependency-based effect (or inside fetchConnectors)
so loading state is still handled correctly.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorPopup.tsx`:
- Around line 535-547: The DevantServicesSection prop is being passed an inline
arrow component which causes React to treat it as a new component type on each
render and remount DevantConnectorList; extract a stable component or provide a
stable render-prop instead: create a memoized/stable component (e.g. via
useCallback/useMemo or a top-level DevantServicesSectionComponent) that returns
<DevantConnectorList ... /> and reference that from DevantConnectorPopup so
DevantConnectorList (and its internal state/query cache) is not unmounted;
ensure the onItemSelect handler still calls the stable state setters
setSelectedFlow, setSelectedMarketplaceItem, and setAvailableNode (these setters
are stable so can be referenced safely).
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/AddConnectionPopup/AddConnectionPopupContent.tsx`:
- Around line 69-70: Remove the leftover debug console.log calls in the
AddConnectionPopupContent component: delete the console.log statements that
print ">>> bi connectors", ">>> bi filtered connectors" and any other
console.log usages in this file (including those referencing model and
model.categories); if persistent runtime logging is required, replace with the
project's logger (e.g., processLogger or a designated logger) or conditional
debug flag instead of console.log to avoid leaking dev output.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx`:
- Around line 125-131: The current sequential ifs in DevantConnectorList.tsx
mutate getMarketPlaceParams based on filterType and risk accidentally applying
both branches; change the logic to either use an else if (if (filterType ===
"internal") ... else if (filterType === "thirdParty") ...) or build
getMarketPlaceParams in a single expression (e.g., derive a params object from
filterType) so only the intended properties (isThirdParty and
networkVisibilityFilter) are set for a given filterType; update the code that
references getMarketPlaceParams accordingly.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorPopup.tsx`:
- Around line 225-228: Delete the commented-out dead code block referencing
showDevantMarketplace/selectedFlow (the lines with "if (showDevantMarketplace &&
!selectedFlow) { setShowDevantMarketplace(false); }") inside
DevantConnectorPopup (remove the commented lines and trailing comment markers)
so the file no longer contains those unused references; rely on VCS history if
the code needs to be restored.
- Around line 522-534: The spread {...props} on the AddConnectionPopupContent
call may pass extraneous keys (onClose, onNavigateToOverview, isPopup,
projectPath) not declared in AddConnectionPopupContent's Props and could cause
future warnings or confusion; replace the spread with an explicit prop list
containing only the values used by AddConnectionPopupContent (e.g., explicitly
pass the specific props it expects) and keep the existing handlers
(handleSelectConnector, handleApiSpecConnection) and state setters
(setAvailableNode, setSelectedFlow,
DevantConnectionFlow.REGISTER_CREATE_THIRD_PARTY_FROM_BI_CONNECTOR,
DevantConnectionFlow.REGISTER_CREATE_THIRD_PARTY_FROM_OAS) unchanged so
consumers receive only the declared props.
...erina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx (2)
226-226: Redundant optional chaining on non-nullabledata.
datais typed asMarketplaceItem[](non-optional, line 213), sodata?.length(line 226) anddata?.map(line 240) can safely drop the?.operators.♻️ Proposed fix
- {data?.length === 0 ? ( + {data.length === 0 ? (- {data?.map((item) => { + {data.map((item) => {Also applies to: 240-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx` at line 226, The conditional and iteration use redundant optional chaining on a non-optional array: inside the DevantConnectorList component remove the unnecessary "?.": replace occurrences of data?.length and data?.map with data.length and data.map respectively (these occur where the component renders the empty-state branch and the map of items) so you use the non-nullable MarketplaceItem[] directly.
115-131: Prefer declarative construction over mutating aconstobject.
getMarketPlaceParamsis declaredconstand then hasisThirdPartyandnetworkVisibilityFilterconditionally mutated. This is a code smell —constprevents reassignment but not property mutation, which is misleading. Consider computing the object declaratively in one pass:♻️ Proposed refactor
- const getMarketPlaceParams: GetMarketplaceItemsParams = { - limit: 24, - offset: 0, - networkVisibilityFilter: "all", - networkVisibilityprojectId: platformExtState?.selectedContext?.project?.id, - sortBy: "createdTime", - query: debouncedSearchText || undefined, - searchContent: false, - }; - - if(filterType === "internal") { - getMarketPlaceParams.isThirdParty = false; - } - if(filterType === "thirdParty") { - getMarketPlaceParams.isThirdParty = true; - getMarketPlaceParams.networkVisibilityFilter = "org,project,public"; - } + const getMarketPlaceParams: GetMarketplaceItemsParams = { + limit: 24, + offset: 0, + networkVisibilityFilter: filterType === "thirdParty" ? "org,project,public" : "all", + networkVisibilityprojectId: platformExtState?.selectedContext?.project?.id, + sortBy: "createdTime", + query: debouncedSearchText || undefined, + searchContent: false, + ...(filterType === "internal" && { isThirdParty: false }), + ...(filterType === "thirdParty" && { isThirdParty: true }), + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx` around lines 115 - 131, The current code creates getMarketPlaceParams then mutates its properties based on filterType, which is misleading for a const; instead build the object declaratively in one expression: create the base object with limit, offset, networkVisibilityFilter, networkVisibilityprojectId (from platformExtState?.selectedContext?.project?.id), sortBy, query (debouncedSearchText || undefined) and searchContent, then spread conditional properties for filterType (e.g., when filterType === "internal" spread { isThirdParty: false }, and when filterType === "thirdParty" spread { isThirdParty: true, networkVisibilityFilter: "org,project,public" }); assign this to getMarketPlaceParams so no post-creation mutation of the object occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx`:
- Line 194: The prop type mismatch occurs because
DevantConnectorListProps.searchText is string | undefined but
ConnectionSection.searchText expects a non-optional string; fix by either
coercing the value at the call site in DevantConnectorList (pass searchText ??
'' or String(searchText)) when rendering <ConnectionSection ...
searchText={searchText} /> or relax the ConnectionSection prop signature to
accept searchText?: string | string | undefined (i.e., change its prop type to
string | undefined) and update its props interface and any usages; reference
DevantConnectorListProps.searchText and the ConnectionSection component prop
definition to locate the change.
---
Duplicate comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx`:
- Around line 74-105: The marketplace click routing bug was fixed in
handleMarketplaceItemClick but a duplicate/older handler (the similar block
around the other occurrence that previously used DevantConnectionType.INTERNAL
for all items) still exists; find the other handler/ call site that builds the
flow (the duplicate logic that invokes onItemSelect with DevantConnectionFlow
variants) and replace it with the same logic as handleMarketplaceItemClick:
compute availableNode via getKnownAvailableNode (using item.serviceType ->
"http"/"graphql"/"soap"/"grpc"), branch on item.isThirdParty to pick the correct
CREATE_* flow (REST -> CREATE_*_OAS; else availableNode -> CREATE_*_OTHER; else
-> CREATE_*_OTHER_SELECT_BI_CONNECTOR), and ensure the caller only passes the
single item to the handler and calls onItemSelect with (flow, item,
availableNode).
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx`:
- Line 226: The conditional and iteration use redundant optional chaining on a
non-optional array: inside the DevantConnectorList component remove the
unnecessary "?.": replace occurrences of data?.length and data?.map with
data.length and data.map respectively (these occur where the component renders
the empty-state branch and the map of items) so you use the non-nullable
MarketplaceItem[] directly.
- Around line 115-131: The current code creates getMarketPlaceParams then
mutates its properties based on filterType, which is misleading for a const;
instead build the object declaratively in one expression: create the base object
with limit, offset, networkVisibilityFilter, networkVisibilityprojectId (from
platformExtState?.selectedContext?.project?.id), sortBy, query
(debouncedSearchText || undefined) and searchContent, then spread conditional
properties for filterType (e.g., when filterType === "internal" spread {
isThirdParty: false }, and when filterType === "thirdParty" spread {
isThirdParty: true, networkVisibilityFilter: "org,project,public" }); assign
this to getMarketPlaceParams so no post-creation mutation of the object occurs.
...erina/ballerina-visualizer/src/views/BI/Connection/DevantConnections/DevantConnectorList.tsx
Show resolved
Hide resolved
96 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
1 participant
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.
Purpose
If workspace is associated with a Devant project, the devant marketplace will be shown above BI connectors in the connector config view
Screen.Recording.2026-02-20.at.16.37.05.mov
Related to https://github.com/wso2-enterprise/devant/issues/1938
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Improvements