Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/server/src/controllers/chatflows/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,14 @@ const deleteChatflow = async (req: Request, res: Response, next: NextFunction) =
const getAllChatflows = async (req: Request, res: Response, next: NextFunction) => {
try {
const { page, limit } = getPageAndLimitParams(req)
const search = req.query?.search as string | undefined

const apiResponse = await chatflowsService.getAllChatflows(
req.query?.type as ChatflowType,
req.user?.activeWorkspaceId,
page,
limit
limit,
search
)
return res.json(apiResponse)
} catch (error) {
Expand Down
16 changes: 11 additions & 5 deletions packages/server/src/services/chatflows/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,14 @@ const deleteChatflow = async (chatflowId: string, orgId: string, workspaceId: st
}
}

const getAllChatflows = async (type?: ChatflowType, workspaceId?: string, page: number = -1, limit: number = -1) => {
const getAllChatflows = async (type?: ChatflowType, workspaceId?: string, page: number = -1, limit: number = -1, search?: string) => {
try {
const appServer = getRunningExpressApp()

const queryBuilder = appServer.AppDataSource.getRepository(ChatFlow)
.createQueryBuilder('chat_flow')
.orderBy('chat_flow.updatedDate', 'DESC')

if (page > 0 && limit > 0) {
queryBuilder.skip((page - 1) * limit)
queryBuilder.take(limit)
}
if (type === 'MULTIAGENT') {
queryBuilder.andWhere('chat_flow.type = :type', { type: 'MULTIAGENT' })
} else if (type === 'AGENTFLOW') {
Expand All @@ -165,6 +161,16 @@ const getAllChatflows = async (type?: ChatflowType, workspaceId?: string, page:
queryBuilder.andWhere('chat_flow.type = :type', { type: 'CHATFLOW' })
}
if (workspaceId) queryBuilder.andWhere('chat_flow.workspaceId = :workspaceId', { workspaceId })
if (search) {
queryBuilder.andWhere(
'(LOWER(chat_flow.name) LIKE LOWER(:search) OR chat_flow.id LIKE :search)',
{ search: `%${search}%` }
)
Comment on lines +165 to +168
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The previous client-side search was case-insensitive. This LIKE implementation might be case-sensitive depending on the database's collation. To ensure consistent, case-insensitive search behavior across different databases, it's better to use LOWER() on both the column and the search term.

            queryBuilder.andWhere(
                '(LOWER(chat_flow.name) LIKE LOWER(:search) OR LOWER(chat_flow.id) LIKE LOWER(:search))',
                { search: `%${search}%` }
            )

}
if (page > 0 && limit > 0) {
queryBuilder.skip((page - 1) * limit)
queryBuilder.take(limit)
}
const [data, total] = await queryBuilder.getManyAndCount()

if (page > 0 && limit > 0) {
Expand Down
2 changes: 1 addition & 1 deletion packages/ui/src/ui-component/table/FlowListTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export const FlowListTable = ({
images = {},
icons = {},
isLoading,
filterFunction,
filterFunction = () => true,
updateFlowsApi,
setError,
isAgentCanvas,
Expand Down
38 changes: 24 additions & 14 deletions packages/ui/src/views/agentflows/index.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react'
import { useEffect, useState, useCallback, useRef } from 'react'
import { useNavigate } from 'react-router-dom'
import { useSelector } from 'react-redux'

Expand Down Expand Up @@ -28,6 +28,9 @@ import useApi from '@/hooks/useApi'
import { baseURL, AGENTFLOW_ICONS } from '@/store/constant'
import { useError } from '@/store/context/ErrorContext'

// utils
import { debounce } from 'lodash'

// icons
import { IconPlus, IconLayoutGrid, IconList, IconX, IconAlertTriangle } from '@tabler/icons-react'

Expand All @@ -54,17 +57,22 @@ const Agentflows = () => {
const [pageLimit, setPageLimit] = useState(DEFAULT_ITEMS_PER_PAGE)
const [total, setTotal] = useState(0)

const searchRef = useRef('')

const onChange = (page, pageLimit) => {
setCurrentPage(page)
setPageLimit(pageLimit)
refresh(page, pageLimit, agentflowVersion)
refresh(page, pageLimit, agentflowVersion, searchRef.current)
}

const refresh = (page, limit, nextView) => {
const refresh = (page, limit, nextView, searchVal) => {
const params = {
page: page || currentPage,
limit: limit || pageLimit
}
if (searchVal) {
params.search = searchVal
}
getAllAgentflows.request(nextView === 'v2' ? 'AGENTFLOW' : 'MULTIAGENT', params)
}

Expand All @@ -78,19 +86,22 @@ const Agentflows = () => {
if (nextView === null) return
localStorage.setItem('agentFlowVersion', nextView)
setAgentflowVersion(nextView)
refresh(1, pageLimit, nextView)
refresh(1, pageLimit, nextView, searchRef.current)
}

// eslint-disable-next-line react-hooks/exhaustive-deps
const debouncedSearch = useCallback(
debounce((value, version) => {
setCurrentPage(1)
refresh(1, pageLimit, version, value)
}, 300),
[pageLimit]
)
Comment on lines +92 to +99
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The eslint-disable-next-line react-hooks/exhaustive-deps is concerning as it can hide bugs related to stale closures. The dependency array for useCallback is incomplete. It should include refresh and setCurrentPage. While setCurrentPage is stable, refresh is not, as it's recreated on every render. This can lead to unpredictable behavior.

To fix this, you should make refresh stable with useCallback and add it to the dependency array here. This may require some refactoring to manage dependencies correctly and avoid re-creating the debounced function on every render.


const onSearchChange = (event) => {
setSearch(event.target.value)
}

function filterFlows(data) {
return (
data.name.toLowerCase().indexOf(search.toLowerCase()) > -1 ||
(data.category && data.category.toLowerCase().indexOf(search.toLowerCase()) > -1) ||
data.id.toLowerCase().indexOf(search.toLowerCase()) > -1
)
searchRef.current = event.target.value
debouncedSearch(event.target.value, agentflowVersion)
}

const addNew = () => {
Expand Down Expand Up @@ -304,7 +315,7 @@ const Agentflows = () => {
<>
{!view || view === 'card' ? (
<Box display='grid' gridTemplateColumns='repeat(3, 1fr)' gap={gridSpacing}>
{getAllAgentflows.data?.data.filter(filterFlows).map((data, index) => (
{getAllAgentflows.data?.data?.map((data, index) => (
<ItemCard
key={index}
onClick={() => goToCanvas(data)}
Expand All @@ -322,7 +333,6 @@ const Agentflows = () => {
images={images}
icons={icons}
isLoading={isLoading}
filterFunction={filterFlows}
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

Removing this filterFunction prop will cause FlowListTable to receive it as undefined. The table component then calls data.filter(filterFunction), which will throw a TypeError.

A quick fix is to pass a no-op filter: filterFunction={() => true}.

However, since filtering is now done on the server, the client-side filtering in FlowListTable is redundant. The best solution would be to remove the .filter(filterFunction) call from FlowListTable.jsx.

updateFlowsApi={getAllAgentflows}
setError={setError}
currentPage={currentPage}
Expand Down
35 changes: 22 additions & 13 deletions packages/ui/src/views/chatflows/index.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react'
import { useEffect, useState, useCallback, useRef } from 'react'
import { useNavigate } from 'react-router-dom'

// material-ui
Expand Down Expand Up @@ -27,6 +27,9 @@ import useApi from '@/hooks/useApi'
import { baseURL } from '@/store/constant'
import { useError } from '@/store/context/ErrorContext'

// utils
import { debounce } from 'lodash'

// icons
import { IconPlus, IconLayoutGrid, IconList } from '@tabler/icons-react'

Expand All @@ -48,18 +51,22 @@ const Chatflows = () => {
const [currentPage, setCurrentPage] = useState(1)
const [pageLimit, setPageLimit] = useState(DEFAULT_ITEMS_PER_PAGE)
const [total, setTotal] = useState(0)
const searchRef = useRef('')

const onChange = (page, pageLimit) => {
setCurrentPage(page)
setPageLimit(pageLimit)
applyFilters(page, pageLimit)
applyFilters(page, pageLimit, searchRef.current)
}

const applyFilters = (page, limit) => {
const applyFilters = (page, limit, searchVal) => {
const params = {
page: page || currentPage,
limit: limit || pageLimit
}
if (searchVal) {
params.search = searchVal
}
getAllChatflowsApi.request(params)
}

Expand All @@ -69,16 +76,19 @@ const Chatflows = () => {
setView(nextView)
}

// eslint-disable-next-line react-hooks/exhaustive-deps
const debouncedSearch = useCallback(
debounce((value) => {
setCurrentPage(1)
applyFilters(1, pageLimit, value)
}, 300),
[pageLimit]
)
Comment on lines +79 to +86
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The eslint-disable-next-line react-hooks/exhaustive-deps is concerning as it can hide bugs related to stale closures. The dependency array for useCallback is incomplete. It should include applyFilters and setCurrentPage. While setCurrentPage is stable, applyFilters is not, as it's recreated on every render. This can lead to unpredictable behavior.

To fix this, you should make applyFilters stable with useCallback and add it to the dependency array here. This may require some refactoring to manage dependencies correctly and avoid re-creating the debounced function on every render.


const onSearchChange = (event) => {
setSearch(event.target.value)
}

function filterFlows(data) {
return (
data?.name.toLowerCase().indexOf(search.toLowerCase()) > -1 ||
(data.category && data.category.toLowerCase().indexOf(search.toLowerCase()) > -1) ||
data?.id.toLowerCase().indexOf(search.toLowerCase()) > -1
)
searchRef.current = event.target.value
debouncedSearch(event.target.value)
}

const addNew = () => {
Expand Down Expand Up @@ -196,7 +206,7 @@ const Chatflows = () => {
<>
{!view || view === 'card' ? (
<Box display='grid' gridTemplateColumns='repeat(3, 1fr)' gap={gridSpacing}>
{getAllChatflowsApi.data?.data?.filter(filterFlows).map((data, index) => (
{getAllChatflowsApi.data?.data?.map((data, index) => (
<ItemCard key={index} onClick={() => goToCanvas(data)} data={data} images={images[data.id]} />
))}
</Box>
Expand All @@ -205,7 +215,6 @@ const Chatflows = () => {
data={getAllChatflowsApi.data?.data}
images={images}
isLoading={isLoading}
filterFunction={filterFlows}
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

Removing this filterFunction prop will cause FlowListTable to receive it as undefined. The table component then calls data.filter(filterFunction), which will throw a TypeError.

A quick fix is to pass a no-op filter: filterFunction={() => true}.

However, since filtering is now done on the server, the client-side filtering in FlowListTable is redundant. The best solution would be to remove the .filter(filterFunction) call from FlowListTable.jsx.

updateFlowsApi={getAllChatflowsApi}
setError={setError}
currentPage={currentPage}
Expand Down