From 45cafd50216048c4d353a7875fb812c88add35e2 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Thu, 23 Jul 2026 15:33:47 +0530 Subject: [PATCH 01/11] UN-3769 [FEAT] Sortable resource list table with server-side sort, search & pagination Replace the sparse ListView/ViewTools list UI with a shared sortable ResourceTable (Name / Owned By / Created Date / Actions) across Adapters, Workflows, Prompt Studio and Connectors. The Owned By column shows the owner avatar/name/email plus co-owner count and opens the co-owner modal. Sort (name/owner/created via the header dropdowns), owner-inclusive search and pagination are server-driven through a new apply_search_and_sort helper, whose pk__in re-wrap lifts the Postgres DISTINCT ON each for_user() manager carries so any column is orderable. Prompt Studio re-applies its prompt_count annotation after the re-wrap. Delete the now-unused ListView and ViewTools. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/adapter_processor_v2/views.py | 19 +- backend/connector_v2/views.py | 19 +- .../prompt_studio_core_v2/views.py | 32 +- backend/utils/list_query.py | 75 ++++ backend/workflow_manager/workflow_v2/views.py | 29 +- .../list-of-tools/ListOfTools.jsx | 276 ++++++++----- .../custom-tools/view-tools/ViewTools.css | 1 - .../custom-tools/view-tools/ViewTools.jsx | 91 ---- .../tool-settings/ToolSettings.jsx | 274 +++++++----- .../components/widgets/list-view/ListView.css | 163 -------- .../components/widgets/list-view/ListView.jsx | 336 --------------- .../widgets/resource-table/ResourceTable.css | 169 ++++++++ .../widgets/resource-table/ResourceTable.jsx | 389 ++++++++++++++++++ .../workflows/workflow/Workflows.jsx | 74 ++-- frontend/src/hooks/usePaginatedList.js | 37 +- frontend/src/pages/ConnectorsPage.jsx | 181 ++++++-- 16 files changed, 1240 insertions(+), 925 deletions(-) create mode 100644 backend/utils/list_query.py delete mode 100644 frontend/src/components/custom-tools/view-tools/ViewTools.css delete mode 100644 frontend/src/components/custom-tools/view-tools/ViewTools.jsx delete mode 100644 frontend/src/components/widgets/list-view/ListView.css delete mode 100644 frontend/src/components/widgets/list-view/ListView.jsx create mode 100644 frontend/src/components/widgets/resource-table/ResourceTable.css create mode 100644 frontend/src/components/widgets/resource-table/ResourceTable.jsx diff --git a/backend/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index 7dfacf4434..cb2fedfd62 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -28,6 +28,7 @@ from tenant_account_v2.organization_member_service import OrganizationMemberService from tool_instance_v2.models import ToolInstance from utils.filtering import FilterHelper +from utils.list_query import apply_search_and_sort from utils.pagination import OptionalPagination from utils.user_context import UserContext @@ -190,14 +191,16 @@ def get_queryset(self) -> QuerySet | None: ): queryset = queryset.filter(**filter_args) - search = self.request.query_params.get("search") - if search: - queryset = queryset.filter(adapter_name__icontains=search) - - # Order by the DISTINCT ON field so pagination is deterministic and the - # admin/service branch (no distinct) is ordered too. Not modified_at: - # that would conflict with the DISTINCT ON in for_user(). - return queryset.order_by("id") + # Owner-inclusive search + per-column sort (name/owner/created); re-wraps + # via pk__in to drop the DISTINCT ON in for_user() so any column sorts. + return apply_search_and_sort( + queryset, + model=AdapterInstance, + name_field="adapter_name", + request=self.request, + select_related=("created_by",), + prefetch_related=("memberships__user",), + ) def get_serializer_class( self, diff --git a/backend/connector_v2/views.py b/backend/connector_v2/views.py index c4a3741f25..a2d5ca5810 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -21,6 +21,7 @@ from rest_framework.versioning import URLPathVersioning from tenant_account_v2.organization_member_service import OrganizationMemberService from utils.filtering import FilterHelper +from utils.list_query import apply_search_and_sort from utils.pagination import OptionalPagination from utils.user_context import UserContext @@ -105,10 +106,6 @@ def get_queryset(self) -> QuerySet | None: if filter_args: queryset = queryset.filter(**filter_args) - search = self.request.query_params.get("search") - if search: - queryset = queryset.filter(connector_name__icontains=search) - # Filter by connector_mode connector_mode_param = self.request.query_params.get("connector_mode") if connector_mode_param: @@ -127,10 +124,16 @@ def get_queryset(self) -> QuerySet | None: ) queryset = queryset.none() - # Order by the DISTINCT ON field so pagination is deterministic and the - # admin/service branch (no distinct) is ordered too. Not modified_at: - # that would conflict with the DISTINCT ON in for_user(). - return queryset.order_by("id") + # Owner-inclusive search + per-column sort (name/owner/created); re-wraps + # via pk__in to drop the DISTINCT ON in for_user() so any column sorts. + return apply_search_and_sort( + queryset, + model=ConnectorInstance, + name_field="connector_name", + request=self.request, + select_related=("created_by",), + prefetch_related=("memberships__user",), + ) def _get_connector_metadata(self, connector_id: str) -> dict[str, str] | None: """Gets connector metadata for the ConnectorInstance. diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index c7251278ef..7dacff3623 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -31,6 +31,7 @@ from tool_instance_v2.models import ToolInstance from utils.file_storage.helpers.prompt_studio_file_helper import PromptStudioFileHelper from utils.hubspot_notify import notify_hubspot_event +from utils.list_query import apply_search_and_sort from utils.pagination import OptionalPagination from utils.user_context import UserContext from utils.user_session import UserSessionUtils @@ -158,7 +159,20 @@ def get_queryset(self) -> QuerySet | None: "memberships__user" ) if self.action == "list": - # Subquery avoids conflict with distinct("tool_id") from for_user() + # Owner-inclusive search + per-column sort (name/owner/created); + # re-wraps via pk__in to drop the DISTINCT ON in for_user() so any + # column sorts. + qs = apply_search_and_sort( + qs, + model=CustomTool, + name_field="tool_name", + request=self.request, + select_related=("created_by",), + prefetch_related=("memberships__user",), + ) + # The pk__in re-wrap drops annotations, so re-apply prompt_count on + # the re-wrapped result. Subquery avoids conflict with the + # distinct("tool_id") that for_user() carries. prompt_count_sq = ( ToolStudioPrompt.objects.filter(tool_id=OuterRef("pk")) .order_by() @@ -166,19 +180,9 @@ def get_queryset(self) -> QuerySet | None: .annotate(cnt=Count("prompt_id")) .values("cnt") ) - # modified_at needs no annotation: prompt writes bump the parent - # row at the source (ToolStudioPrompt.save/delete, sync_prompts), - # keeping the plain field orderable. Only prompt writes bump — - # profile/document edits and queryset-level prompt writes do not; - # any new write path must bump CustomTool itself - qs = qs.select_related("created_by").annotate( - _prompt_count=Subquery(prompt_count_sq), - ) - search = self.request.query_params.get("search") - if search: - qs = qs.filter(tool_name__icontains=search) - # Order by the DISTINCT ON field so pagination is deterministic and the - # admin/service branch (no distinct) is ordered too. + return qs.annotate(_prompt_count=Subquery(prompt_count_sq)) + # Order by the DISTINCT ON field so pagination is deterministic for the + # admin/service (non-list) branch. return qs.order_by("tool_id") def get_object(self): diff --git a/backend/utils/list_query.py b/backend/utils/list_query.py new file mode 100644 index 0000000000..401eff498b --- /dev/null +++ b/backend/utils/list_query.py @@ -0,0 +1,75 @@ +"""Shared list-query helpers for resource list endpoints. + +Provides owner-inclusive ``?search`` and per-column sorting (name / owner / +created) with a ``pk__in`` re-wrap so managers ending in Postgres ``DISTINCT +ON`` can still be ordered by an arbitrary column. +""" + +from typing import Any + +from django.db.models import Model, Q, QuerySet +from rest_framework.request import Request + +# ``name`` maps to the resource-specific name field each caller passes; +# owner/created are shared across every list endpoint. +OWNER_SORT_FIELD = "created_by__email" +CREATED_SORT_FIELD = "created_at" + + +def apply_search_and_sort( + queryset: QuerySet[Any], + *, + model: type[Model], + name_field: str, + request: Request, + select_related: tuple[str, ...] = (), + prefetch_related: tuple[str, ...] = (), + default_sort_by: str = "name", +) -> QuerySet[Any]: + """Apply owner-inclusive ``?search`` and ``?sort_by``/``?order`` to a list + queryset. + + ``sort_by`` is ``name`` | ``owner`` | ``created`` (default ``name``); + ``order`` is ``asc`` | ``desc`` (default ``asc``). The queryset is re-wrapped + via ``pk__in`` to drop any ``DISTINCT ON`` (so ordering by a non-distinct + column is legal) and a ``pk`` tiebreaker is appended for stable pagination. + ``select_related`` / ``prefetch_related`` are re-attached to the re-wrapped + queryset to keep the list free of N+1 owner/co-owner lookups. + + Args: + queryset: The already org-scoped, ``for_user``-filtered list queryset. + model: The concrete resource model, used to re-wrap via ``pk__in``. + name_field: The resource's name column (e.g. ``adapter_name``). + request: DRF request carrying ``search`` / ``sort_by`` / ``order``. + select_related: FK joins to re-attach after the re-wrap. + prefetch_related: Reverse/M2M prefetches to re-attach after the re-wrap. + default_sort_by: Sort key used when ``?sort_by`` is absent. + + Returns: + An ordered queryset ready for pagination. + """ + params = request.query_params + + search = params.get("search") + if search: + queryset = queryset.filter( + Q(**{f"{name_field}__icontains": search}) + | Q(**{f"{OWNER_SORT_FIELD}__icontains": search}) + ) + + sort_field = { + "name": name_field, + "owner": OWNER_SORT_FIELD, + "created": CREATED_SORT_FIELD, + }.get(params.get("sort_by") or default_sort_by, name_field) + order_prefix = "-" if (params.get("order") or "asc").lower() == "desc" else "" + + # Ordering the source by ``pk`` keeps the DISTINCT ON (always the pk) valid + # while stripping the model's default ordering, so the outer query is free + # to sort by any column. + rewrapped = model.objects.filter(pk__in=queryset.order_by("pk").values("pk")) + if select_related: + rewrapped = rewrapped.select_related(*select_related) + if prefetch_related: + rewrapped = rewrapped.prefetch_related(*prefetch_related) + return rewrapped.order_by(f"{order_prefix}{sort_field}", "pk") diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index ef9f52f95f..5dcb1bc969 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -21,6 +21,7 @@ from rest_framework.versioning import URLPathVersioning from rest_framework.views import APIView from utils.filtering import FilterHelper +from utils.list_query import apply_search_and_sort from utils.organization_utils import filter_queryset_by_organization, resolve_organization from utils.pagination import OptionalPagination @@ -110,24 +111,18 @@ def get_queryset(self) -> QuerySet: if filter_args else Workflow.objects.for_user(self.request.user) ) - # Avoid per-row queries for owner/co-owner + creator fields in list views - queryset = queryset.select_related("created_by").prefetch_related( - "memberships__user" - ) - - search = self.request.query_params.get("search") - if search: - queryset = queryset.filter(workflow_name__icontains=search) - # `id` tiebreaker keeps ordering deterministic across paginated requests - # (the for_user() manager uses plain .distinct(), so there is no default) - order_by = self.request.query_params.get("order_by") - if order_by == "asc": - queryset = queryset.order_by("modified_at", "id") - else: - queryset = queryset.order_by("-modified_at", "id") - - return queryset + # Owner-inclusive search + per-column sort (name/owner/created); re-wraps + # via pk__in (harmless here — for_user() uses plain .distinct()) and + # re-attaches the owner/co-owner joins to avoid N+1 in list views. + return apply_search_and_sort( + queryset, + model=Workflow, + name_field="workflow_name", + request=self.request, + select_related=("created_by",), + prefetch_related=("memberships__user",), + ) def get_serializer_class(self) -> serializers.Serializer: if self.action == "execute": diff --git a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx index 38fb925a9c..70be4167f6 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -1,11 +1,12 @@ import { ArrowDownOutlined, PlusOutlined } from "@ant-design/icons"; import { Space } from "antd"; import PropTypes from "prop-types"; -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; +import { usePaginatedList } from "../../../hooks/usePaginatedList"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import { useAlertStore } from "../../../store/alert-store"; import { useSessionStore } from "../../../store/session-store"; @@ -13,12 +14,16 @@ import { groupsService } from "../../groups/groups-service.js"; import { ToolNavBar } from "../../navigations/tool-nav-bar/ToolNavBar"; import { CoOwnerManagement } from "../../widgets/co-owner-management/CoOwnerManagement"; import { CustomButton } from "../../widgets/custom-button/CustomButton"; +import { EmptyState } from "../../widgets/empty-state/EmptyState.jsx"; +import { ResourceTable } from "../../widgets/resource-table/ResourceTable"; import { SharePermission } from "../../widgets/share-permission/SharePermission"; +import { SpinnerLoader } from "../../widgets/spinner-loader/SpinnerLoader.jsx"; import { AddCustomToolFormModal } from "../add-custom-tool-form-modal/AddCustomToolFormModal"; import { ImportTool } from "../import-tool/ImportTool"; -import { ViewTools } from "../view-tools/ViewTools"; import "./ListOfTools.css"; +const DEFAULT_PAGE_SIZE = 10; + const DefaultCustomButtons = ({ setOpenImportTool, isImportLoading, @@ -52,7 +57,7 @@ DefaultCustomButtons.propTypes = { }; function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { - const [isListLoading, setIsListLoading] = useState(false); + const [isLoading, setIsLoading] = useState(false); const [openAddTool, setOpenAddTool] = useState(false); const [openImportTool, setOpenImportTool] = useState(false); const [isImportLoading, setIsImportLoading] = useState(false); @@ -64,8 +69,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const handleException = useExceptionHandler(); const groupsApi = groupsService(); - const [listOfTools, setListOfTools] = useState([]); - const [filteredListOfTools, setFilteredListOfTools] = useState([]); + // undefined = not fetched yet (spinner); [] = fetched-empty (empty state) + const [displayList, setDisplayList] = useState(); const [isEdit, setIsEdit] = useState(false); const [promptDetails, setPromptDetails] = useState(null); const [openSharePermissionModal, setOpenSharePermissionModal] = @@ -73,6 +78,10 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const [isPermissionEdit, setIsPermissionEdit] = useState(false); const [isShareLoading, setIsShareLoading] = useState(false); const [allUserList, setAllUserList] = useState([]); + const [allGroupList, setAllGroupList] = useState([]); + // Ref forwards the fetch fn to the pagination hook (avoids declaration order). + const fetchListRef = useRef(null); + const promptStudioCoOwnerService = useMemo( () => ({ getAllUsers: () => @@ -106,6 +115,39 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { [axiosPrivate, sessionDetails?.orgId, sessionDetails?.csrfToken], ); + const { + pagination, + setPagination, + searchTerm, + setSearchTerm, + sort, + handlePaginationChange, + handleSearch, + handleSortChange, + } = usePaginatedList({ + fetchData: (...args) => fetchListRef.current?.(...args), + defaultPageSize: DEFAULT_PAGE_SIZE, + }); + + // Refresh the current page (preserves page + active search/sort) after mutations + const handleListRefresh = useCallback( + () => + fetchListRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ), + [ + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ], + ); + const { coOwnerOpen, setCoOwnerOpen, @@ -119,49 +161,85 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { } = useCoOwnerManagement({ service: promptStudioCoOwnerService, setAlertDetails, - onListRefresh: () => getListOfTools(), + onListRefresh: handleListRefresh, }); - const [allGroupList, setAllGroupList] = useState([]); - useEffect(() => { - getListOfTools(); - }, []); + const getListOfTools = useCallback( + ( + page = 1, + pageSize = DEFAULT_PAGE_SIZE, + search = "", + sortBy = "", + order = "asc", + ) => { + const params = { page, page_size: pageSize }; + if (search) { + params.search = search; + } + if (sortBy) { + params.sort_by = sortBy; + params.order = order; + } + setIsLoading(true); + axiosPrivate({ + method: "GET", + url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, + headers: { "X-CSRFToken": sessionDetails?.csrfToken }, + params, + }) + .then((res) => { + const data = res?.data; + // Endpoint is opt-in paginated: envelope when we send ?page, else a + // bare array. Handle both so any non-paginated caller stays unaffected. + const results = data?.results ?? data ?? []; + const total = data?.count ?? results.length; + // Deleting the last row on a page leaves it empty; step back a page. + if (results.length === 0 && page > 1 && total > 0) { + getListOfTools(page - 1, pageSize, search, sortBy, order); + return; + } + setDisplayList(results); + setPagination((prev) => ({ + ...prev, + current: page, + pageSize, + total, + })); + }) + .catch((err) => { + setAlertDetails( + handleException(err, "Failed to get the list of tools"), + ); + // Avoid an indefinite spinner when the first fetch fails. + setDisplayList((prev) => prev ?? []); + }) + .finally(() => { + setIsLoading(false); + }); + }, + [ + sessionDetails?.orgId, + sessionDetails?.csrfToken, + axiosPrivate, + setPagination, + setAlertDetails, + handleException, + ], + ); + fetchListRef.current = getListOfTools; useEffect(() => { - setFilteredListOfTools(listOfTools); - }, [listOfTools]); - - const getListOfTools = () => { - const requestOptions = { - method: "GET", - url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, - headers: { - "X-CSRFToken": sessionDetails?.csrfToken, - }, - }; - - setIsListLoading(true); - axiosPrivate(requestOptions) - .then((res) => { - const data = res?.data; - setListOfTools(data); - setFilteredListOfTools(data); - }) - .catch((err) => { - setAlertDetails( - handleException(err, "Failed to get the list of tools"), - ); - }) - .finally(() => { - setIsListLoading(false); - }); - }; + setSearchTerm(""); + setDisplayList(undefined); + getListOfTools(1, DEFAULT_PAGE_SIZE, "", "", "asc"); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); const handleAddNewTool = (body) => { let method = "POST"; let url = `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`; - const isEdit = editItem && Object.keys(editItem)?.length > 0; - if (isEdit) { + const isEditFlow = editItem && Object.keys(editItem)?.length > 0; + if (isEditFlow) { method = "PATCH"; url += `${editItem?.tool_id}/`; } @@ -178,8 +256,10 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { axiosPrivate(requestOptions) .then((res) => { - const tool = res?.data; - updateList(isEdit, tool); + setEditItem(null); + // Refetch the current page to reflect server truth rather than + // splicing a stale list (list-only fields like prompt_count). + handleListRefresh(); setOpenAddTool(false); resolve(res?.data); }) @@ -189,31 +269,12 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { }); }; - const updateList = (isEdit, data) => { - let tools = [...listOfTools]; - - if (isEdit) { - // Merge — the PATCH response (CustomToolSerializer) lacks list-only - // fields like prompt_count; replacing wholesale would drop them - tools = tools.map((item) => - item?.tool_id === data?.tool_id ? { ...item, ...data } : item, - ); - setEditItem(null); - } else { - tools.push(data); - } - setListOfTools(tools); - }; - const handleEdit = (_event, tool) => { - const editToolData = [...listOfTools].find( - (item) => item?.tool_id === tool.tool_id, - ); - if (!editToolData) { + if (!tool) { return; } setIsEdit(true); - setEditItem(editToolData); + setEditItem(tool); setOpenAddTool(true); }; @@ -227,29 +288,12 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { }; axiosPrivate(requestOptions) - .then(() => { - const tools = [...listOfTools].filter( - (filterToll) => filterToll?.tool_id !== tool.tool_id, - ); - setListOfTools(tools); - }) + .then(() => handleListRefresh()) .catch((err) => { setAlertDetails(handleException(err, "Failed to Delete")); }); }; - const onSearch = (search, setSearch) => { - if (search?.length === 0) { - setSearch(listOfTools); - } - const filteredList = [...listOfTools].filter((tool) => { - const name = tool.tool_name?.toUpperCase(); - const searchUpperCase = search.toUpperCase(); - return name.includes(searchUpperCase); - }); - setSearch(filteredList); - }; - const showAddTool = () => { setEditItem(null); setIsEdit(false); @@ -315,7 +359,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { setOpenImportTool(false); // Refresh the list of tools to show the new imported project - getListOfTools(); + handleListRefresh(); }) .catch((err) => { setAlertDetails(handleException(err, "Failed to import project")); @@ -325,7 +369,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { }); }; - const handleShare = (_event, promptProject, isEdit) => { + const handleShare = (_event, promptProject, isEditShare) => { const requestOptions = { method: "GET", url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/users/${promptProject?.tool_id}`, @@ -346,7 +390,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { .then((res) => { setOpenSharePermissionModal(true); setPromptDetails(res?.data); - setIsPermissionEdit(isEdit); + setIsPermissionEdit(isEditShare); }) .catch((err) => { setAlertDetails(handleException(err)); @@ -410,27 +454,6 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { handleCoOwnerAction(tool.tool_id); }; - const defaultContent = ( -
- -
- ); - const customButtonsElement = useMemo( () => ( handleSearch(value)} customButtons={customButtonsElement} segmentOptions={segmentOptions} segmentValue={segmentValue} segmentFilter={onSegmentChange} />
-
{defaultContent}
+
+
+ {displayList === undefined && } + {displayList?.length === 0 && !searchTerm && ( + + )} + {displayList?.length === 0 && searchTerm && ( + + )} + {displayList?.length > 0 && ( + + )} +
+
{openAddTool && ( ; - } - - if (isEmpty) { - let text = "No tools available"; - let btnText = "New Tool"; - if (type) { - text = `No ${type.toLowerCase()} available`; - btnText = type; - } - return ( - setOpenAddTool(true)} - /> - ); - } - - if (!listOfTools?.length) { - return ; - } - - return ( - - ); -} - -ViewTools.propTypes = { - isLoading: PropTypes.bool.isRequired, - isEmpty: PropTypes.bool.isRequired, - listOfTools: PropTypes.array, - setOpenAddTool: PropTypes.func, - handleEdit: PropTypes.func.isRequired, - handleDelete: PropTypes.func.isRequired, - handleShare: PropTypes.func, - handleCoOwner: PropTypes.func, - titleProp: PropTypes.string.isRequired, - descriptionProp: PropTypes.string, - iconProp: PropTypes.string, - idProp: PropTypes.string.isRequired, - centered: PropTypes.bool, - isClickable: PropTypes.bool, - showOwner: PropTypes.bool, - showModified: PropTypes.bool, - type: PropTypes.string, -}; - -export { ViewTools }; diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 04f41d7694..fb057c5d79 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -1,23 +1,25 @@ import { PlusOutlined } from "@ant-design/icons"; import PropTypes from "prop-types"; -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; -import { useListSearch } from "../../../hooks/useListSearch"; +import { usePaginatedList } from "../../../hooks/usePaginatedList"; import usePostHogEvents from "../../../hooks/usePostHogEvents"; import { IslandLayout } from "../../../layouts/island-layout/IslandLayout"; import { useAlertStore } from "../../../store/alert-store"; import { useSessionStore } from "../../../store/session-store"; -import { ViewTools } from "../../custom-tools/view-tools/ViewTools"; import { groupsService } from "../../groups/groups-service.js"; import { AddSourceModal } from "../../input-output/add-source-modal/AddSourceModal"; import "../../input-output/data-source-card/DataSourceCard.css"; import { ToolNavBar } from "../../navigations/tool-nav-bar/ToolNavBar"; import { CoOwnerManagement } from "../../widgets/co-owner-management/CoOwnerManagement"; import { CustomButton } from "../../widgets/custom-button/CustomButton"; +import { EmptyState } from "../../widgets/empty-state/EmptyState.jsx"; +import { ResourceTable } from "../../widgets/resource-table/ResourceTable"; import { SharePermission } from "../../widgets/share-permission/SharePermission"; +import { SpinnerLoader } from "../../widgets/spinner-loader/SpinnerLoader.jsx"; import "./ToolSettings.css"; const titles = { @@ -36,6 +38,8 @@ const btnText = { ocr: "New OCR", }; +const DEFAULT_PAGE_SIZE = 10; + function ToolSettings({ type }) { const [isLoading, setIsLoading] = useState(false); const [isShareLoading, setIsShareLoading] = useState(false); @@ -48,10 +52,14 @@ function ToolSettings({ type }) { useState(false); const [isPermissonEdit, setIsPermissionEdit] = useState(false); const [editItemId, setEditItemId] = useState(null); + // undefined = not fetched yet (spinner); [] = fetched-empty (empty state) + const [displayList, setDisplayList] = useState(); const { sessionDetails } = useSessionStore(); const { setAlertDetails } = useAlertStore(); const axiosPrivate = useAxiosPrivate(); const handleException = useExceptionHandler(); + // Ref forwards the fetch fn to the pagination hook (avoids declaration order). + const fetchListRef = useRef(null); const adapterCoOwnerService = useMemo( () => ({ @@ -86,6 +94,39 @@ function ToolSettings({ type }) { [sessionDetails?.orgId, sessionDetails?.csrfToken], ); + const { + pagination, + setPagination, + searchTerm, + setSearchTerm, + sort, + handlePaginationChange, + handleSearch, + handleSortChange, + } = usePaginatedList({ + fetchData: (...args) => fetchListRef.current?.(...args), + defaultPageSize: DEFAULT_PAGE_SIZE, + }); + + // Refresh the current page (preserves page + active search/sort) after mutations + const handleListRefresh = useCallback( + () => + fetchListRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ), + [ + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ], + ); + const { coOwnerOpen, setCoOwnerOpen, @@ -99,83 +140,113 @@ function ToolSettings({ type }) { } = useCoOwnerManagement({ service: adapterCoOwnerService, setAlertDetails, - onListRefresh: () => getAdapters(), + onListRefresh: handleListRefresh, }); const { posthogEventText, setPostHogCustomEvent } = usePostHogEvents(); - const { - listRef, - displayList, - setDisplayList, - setMasterList, - updateMasterList, - onSearch, - clearSearch, - } = useListSearch("adapter_name"); + + const getAdapters = useCallback( + ( + page = 1, + pageSize = DEFAULT_PAGE_SIZE, + search = "", + sortBy = "", + order = "asc", + ) => { + if (!type) { + return; + } + const params = { + adapter_type: type.toUpperCase(), + page, + page_size: pageSize, + }; + if (search) { + params.search = search; + } + if (sortBy) { + params.sort_by = sortBy; + params.order = order; + } + setIsLoading(true); + axiosPrivate({ + method: "GET", + url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter`, + params, + }) + .then((res) => { + const data = res?.data; + // Endpoint is opt-in paginated: envelope when we send ?page, else a + // bare array. Handle both so shared (dropdown) callers stay unaffected. + const results = data?.results ?? data ?? []; + const total = data?.count ?? results.length; + // Deleting the last row on a page leaves it empty; step back a page. + if (results.length === 0 && page > 1 && total > 0) { + getAdapters(page - 1, pageSize, search, sortBy, order); + return; + } + setDisplayList(results); + setPagination((prev) => ({ + ...prev, + current: page, + pageSize, + total, + })); + }) + .catch((err) => { + setAlertDetails(handleException(err)); + // Avoid an indefinite spinner when the first fetch fails. + setDisplayList((prev) => prev ?? []); + }) + .finally(() => { + setIsLoading(false); + }); + }, + [ + type, + sessionDetails?.orgId, + axiosPrivate, + setPagination, + setAlertDetails, + handleException, + ], + ); + fetchListRef.current = getAdapters; useEffect(() => { - clearSearch(); - setMasterList([]); + setSearchTerm(""); + setDisplayList(undefined); if (!type) { return; } - getAdapters(); + getAdapters(1, DEFAULT_PAGE_SIZE, "", "", "asc"); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [type]); - const getAdapters = () => { - const requestOptions = { - method: "GET", - url: `/api/v1/unstract/${ - sessionDetails?.orgId - }/adapter?adapter_type=${type.toUpperCase()}`, - }; - setIsLoading(true); - axiosPrivate(requestOptions) - .then((res) => { - setMasterList(res?.data || []); - }) - .catch((err) => { - setAlertDetails(handleException(err)); - }) - .finally(() => { - setIsLoading(false); - }); - }; - - const addNewItem = (row, isEdit) => { - if (isEdit) { - updateMasterList((currentList) => - currentList.map((tableRow) => { - if (tableRow?.id !== row?.id) { - return tableRow; - } - return { ...tableRow, adapter_name: row?.adapter_name }; - }), - ); - } else { - updateMasterList((currentList) => [...currentList, row]); - } - }; - - const handleDeleteSuccess = (adapterId) => { - updateMasterList((currentList) => - currentList.filter((row) => row?.id !== adapterId), - ); - }; + // New/edited adapters land on some page under the active sort — refetch the + // current page to reflect server truth rather than splicing a stale array. + const addNewItem = () => handleListRefresh(); const handleDelete = (_event, adapter) => { - const requestOptions = { + setIsLoading(true); + axiosPrivate({ method: "DELETE", url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/${adapter?.id}/`, - headers: { - "X-CSRFToken": sessionDetails?.csrfToken, - }, - }; + headers: { "X-CSRFToken": sessionDetails?.csrfToken }, + }) + .then(() => handleListRefresh()) + .catch((err) => setAlertDetails(handleException(err))); + }; - setIsLoading(true); - axiosPrivate(requestOptions) - .then(() => handleDeleteSuccess(adapter?.id)) - .catch((err) => setAlertDetails(handleException(err))) - .finally(() => setIsLoading(false)); + const handleEdit = (_event, item) => { + if (item?.is_deprecated) { + setAlertDetails({ + type: "error", + content: + "This adapter has been deprecated and cannot be edited. Please remove it or use an alternative adapter.", + }); + return; + } + setEditItemId(item?.id); }; const handleShare = (_event, adapter, isEdit) => { @@ -268,7 +339,9 @@ function ToolSettings({ type }) { }; const handleCoOwner = (_event, adapter) => { - if (!adapter?.id) return; + if (!adapter?.id) { + return; + } if (adapter?.is_deprecated) { setAlertDetails({ type: "error", @@ -297,8 +370,7 @@ function ToolSettings({ type }) { title={titles[type]} enableSearch searchKey={type} - setSearchList={setDisplayList} - onSearch={onSearch} + onSearch={(value) => handleSearch(value)} customButtons={
- { - // Check if adapter is deprecated - if (item?.is_deprecated) { - setAlertDetails({ - type: "error", - content: - "This adapter has been deprecated and cannot be edited. Please remove it or use an alternative adapter.", - }); - return; - } - setEditItemId(item?.id); - }} - idProp="id" - titleProp="adapter_name" - descriptionProp="description" - iconProp="icon" - isEmpty={!listRef.current.length} - centered - isClickable={false} - handleShare={handleShare} - handleCoOwner={handleCoOwner} - showOwner={true} - showModified - type="Adapter" - /> + {displayList === undefined && } + {displayList?.length === 0 && !searchTerm && ( + + )} + {displayList?.length === 0 && searchTerm && ( + + )} + {displayList?.length > 0 && ( + + )}
diff --git a/frontend/src/components/widgets/list-view/ListView.css b/frontend/src/components/widgets/list-view/ListView.css deleted file mode 100644 index 0e7ea8c8cf..0000000000 --- a/frontend/src/components/widgets/list-view/ListView.css +++ /dev/null @@ -1,163 +0,0 @@ -/* Styles for ListView */ - -.list-view-wrapper { - height: 100%; - overflow: hidden auto; - width: 70%; - min-width: 800px; - max-width: 1400px; - padding: 0px 10px 40px 10px; -} - -.list-view-item { - display: grid; - grid-auto-flow: row; - row-gap: 2px; - cursor: pointer; -} - -.cur-pointer { - padding: 16px 0 !important; - z-index: 1; - overflow: hidden; -} - -.action-button-container { - display: flex; - align-items: center; - gap: 24px; - padding-right: 12px; -} - -.action-icon-buttons { - font-size: 18px; - z-index: 20; - color: #092c4c; - cursor: pointer; - transition: color 0.2s ease; -} - -.list-view-description { - font-size: 13px; - margin-bottom: 0 !important; - /* keep rows uniform; full text is in the ellipsis tooltip */ - max-width: 520px; -} - -.list-view-row { - width: 100%; -} - -/* Left column: title + description stacked; shrinks with ellipsis */ -.list-view-left { - display: flex; - flex: 1 1 45%; - flex-direction: column; - gap: 4px; - min-width: 0; -} - -/* Middle column: owner + updated. align-items only centers the badge and - the updated-text against each other — the column's own vertical position - comes from the row Flex's align in ListView.jsx */ -.list-view-meta { - align-items: center; - display: flex; - flex: 0 1 auto; - gap: 20px; - min-width: 0; -} - -.list-view-meta .shared-username { - max-width: 200px; -} - -.list-view-divider { - border-inline-start: solid rgba(5, 5, 5, 0.13); - height: 20px; -} - -.adapter-cover-img .fit-cover { - width: 38px; - height: 38px; - object-fit: contain; -} - -.adapters-list-profile-container { - align-items: center; - display: flex; - justify-content: center; - /* let this shrink below its content width so the email ellipsizes - instead of forcing the row wider (the email itself never wraps — - it's nowrap + capped at 200px) */ - min-width: 0; -} - -.adapters-list-user-prefix { - margin: 0 5px; - font-weight: 500; - white-space: nowrap; -} - -.adapters-list-title { - font-size: 16px; -} - -.adapter-cover-img { - display: flex; - align-items: center; - gap: 5px; -} - -/* flex item: allow the title to shrink so its ellipsis can engage */ -.adapter-cover-img .adapters-list-title { - min-width: 0; -} - -.list-view-modified-container { - align-items: center; - display: flex; - gap: 6px; - white-space: nowrap; -} - -.list-view-modified-text { - font-size: 12px; -} - -.list-view-info-icon { - color: #8c8c8c; - font-size: 13px; -} - -.edit-icon:hover { - color: #1890ff; -} - -.delete-icon:hover { - color: #ff4d4f; -} - -.share-icon:hover { - color: #1890ff; -} - -.owner-badge-btn { - background: none; - border: none; - padding: 0; - font: inherit; - color: inherit; -} - -.owner-clickable { - cursor: pointer; -} - -.owner-clickable:hover .adapters-list-user-avatar { - background-color: #1890ff; -} - -.owner-clickable:hover .shared-username { - color: #1890ff; -} diff --git a/frontend/src/components/widgets/list-view/ListView.jsx b/frontend/src/components/widgets/list-view/ListView.jsx deleted file mode 100644 index 14407c69c5..0000000000 --- a/frontend/src/components/widgets/list-view/ListView.jsx +++ /dev/null @@ -1,336 +0,0 @@ -import { - Avatar, - Flex, - Image, - List, - Popconfirm, - Tooltip, - Typography, -} from "antd"; -import PropTypes from "prop-types"; -import "./ListView.css"; -import { - DeleteOutlined, - EditOutlined, - InfoCircleOutlined, - QuestionCircleOutlined, - ShareAltOutlined, - UserOutlined, -} from "@ant-design/icons"; -import { useNavigate } from "react-router-dom"; - -import { formattedDateTime, timeAgo } from "../../../helpers/GetStaticData"; -import { useSessionStore } from "../../../store/session-store"; - -// Tooltip lines render when the value is present (`!= null`: 0 shows, -// null/undefined hide). The "Updated" block itself is opt-in via -// showModified — field presence alone doesn't prove the page's value is -// an honest "last modified". -const renderItemMetadata = (item) => ( -
- {item?.created_at && ( -
Created: {formattedDateTime(item.created_at)}
- )} - {item?.modified_at && ( -
Modified: {formattedDateTime(item.modified_at)}
- )} - {item?.model != null &&
Model: {item.model}
} - {item?.prompt_count != null &&
Prompts: {item.prompt_count}
} -
-); - -function ListView({ - listOfTools, - handleEdit, - handleDelete, - handleShare, - handleCoOwner, - titleProp, - descriptionProp, - iconProp, - idProp, - centered, - isClickable = true, - showOwner = true, - showModified = false, - type, -}) { - const navigate = useNavigate(); - const { sessionDetails } = useSessionStore(); - const handleDeleteClick = (event, tool) => { - event.stopPropagation(); // Stop propagation to prevent list item click - handleDelete(event, tool); - }; - - const handleShareClick = (event, tool, isEdit) => { - event.stopPropagation(); // Stop propagation to prevent list item click - handleShare(event, tool, isEdit); - }; - - const handleCoOwnerClick = (event, tool) => { - event.stopPropagation(); - handleCoOwner(event, tool); - }; - - const renderOwnerBadge = (item) => { - // ``is_owner``/``co_owners_count`` come from resources migrated to the - // membership model (co-owners). Resources not yet migrated fall back to the - // created_by-email comparison so their owner badge is unchanged. - const hasMembership = item?.co_owners_count !== undefined; - let name = "-"; - if (hasMembership) { - name = item?.is_owner ? "Me" : item?.created_by_email || "-"; - } else if (item?.created_by_email) { - name = - item.created_by_email === sessionDetails?.email - ? "Me" - : item.created_by_email; - } - const extra = - item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; - const ownerLabel = `${name}${extra}`; - - const badgeContent = ( - <> - } - /> - - Owned By: - - - {ownerLabel} - - - ); - - if (handleCoOwner) { - return ( - - - - ); - } - - return ( -
{badgeContent}
- ); - }; - - const renderTitle = (item) => { - let title = null; - if (iconProp && item[iconProp].length > 4) { - title = ( -
- - - {item[titleProp]} - -
- ); - } else if (iconProp) { - title = ( - - {`${item[iconProp]} ${item[titleProp]}`} - - ); - } else { - title = ( - - {item[titleProp]} - - ); - } - - return title; - }; - - const renderMeta = (item) => { - // Empty on malformed input — hide the label instead of "Invalid date" - const updatedAgo = showModified ? timeAgo(item?.modified_at) : ""; - if (!showOwner && !updatedAgo) { - return null; - } - // No click handler here: the co-owner button stops its own propagation, - // and everything else should bubble to the row like the rest of it - return ( -
- {showOwner && renderOwnerBadge(item)} - {updatedAgo && ( -
- - Updated {updatedAgo} - - renderItemMetadata(item)}> - - -
- )} -
- ); - }; - - const renderActions = (item) => ( -
event.stopPropagation()} - role="none" - > - - { - if (item?.is_deprecated) { - return; - } - handleEdit(event, item); - }} - className={`action-icon-buttons edit-icon ${ - item?.is_deprecated ? "disabled-icon" : "" - }`} - style={{ - cursor: item?.is_deprecated ? "not-allowed" : "pointer", - opacity: item?.is_deprecated ? 0.4 : 1, - }} - /> - - {handleShare && ( - - { - if (item?.is_deprecated) { - return; - } - handleShareClick(event, item, true); - }} - style={{ - cursor: item?.is_deprecated ? "not-allowed" : "pointer", - opacity: item?.is_deprecated ? 0.4 : 1, - }} - /> - - )} - } - onConfirm={(event) => { - handleDeleteClick(event, item); - }} - > - - - - -
- ); - - return ( - { - return ( - { - isClickable - ? navigate(`${item[idProp]}`) - : handleShareClick(event, item, false); - }} - className="cur-pointer" - > - -
- {renderTitle(item)} - {item[descriptionProp] ? ( - - {item[descriptionProp]} - - ) : null} -
- {renderMeta(item)} - {renderActions(item)} - - - ); - }} - /> - ); -} - -ListView.propTypes = { - listOfTools: PropTypes.array.isRequired, - handleEdit: PropTypes.func.isRequired, - handleDelete: PropTypes.func.isRequired, - handleShare: PropTypes.func, - handleCoOwner: PropTypes.func, - titleProp: PropTypes.string.isRequired, - descriptionProp: PropTypes.string, - iconProp: PropTypes.string, - idProp: PropTypes.string.isRequired, - centered: PropTypes.bool, - isClickable: PropTypes.bool, - showOwner: PropTypes.bool, - showModified: PropTypes.bool, - type: PropTypes.string, -}; - -export { ListView }; diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.css b/frontend/src/components/widgets/resource-table/ResourceTable.css new file mode 100644 index 0000000000..47807e90f0 --- /dev/null +++ b/frontend/src/components/widgets/resource-table/ResourceTable.css @@ -0,0 +1,169 @@ +/* Styles for the sortable resource list table (Name / Owned By / Created / Actions) */ + +.resource-table { + /* Fill the content width like the design; proportional column widths (set on + each column) keep it balanced without one column hogging the slack. */ + width: 100%; + padding: 0 4px 24px 4px; +} + +/* Let wide content scroll inside the table instead of the page body */ +.resource-table .ant-table-content { + overflow-x: auto; +} + +/* Keep every cell aligned with the (multi-line) Name cell */ +.resource-table .ant-table-cell { + vertical-align: middle; +} + +/* Header row: light, subtle, with uppercase gray labels */ +.resource-table .ant-table-thead > tr > th { + background: #fafafa; + border-bottom: 1px solid #f0f0f0; + padding-top: 14px; + padding-bottom: 14px; +} + +/* Sort-dropdown trigger / plain header label */ +.resource-table-th { + display: inline-flex; + align-items: center; + gap: 6px; + margin: 0; + padding: 0; + border: none; + background: none; + cursor: pointer; + color: #64748b; + font-size: 12px; + font-weight: 600; + letter-spacing: 0.05em; + text-transform: uppercase; +} + +.resource-table-th.static { + cursor: default; +} + +.resource-table-th.right { + width: 100%; + justify-content: flex-end; +} + +.resource-table-th.active { + color: #1677ff; +} + +/* Stacked up/down carets — the sortable indicator, blue when active */ +.resource-table-sort-icon { + display: inline-flex; + flex-direction: column; + font-size: 9px; + line-height: 0.6; + color: #bfbfbf; +} + +.resource-table-th.active .resource-table-sort-icon { + color: #1677ff; +} + +.resource-table-row-clickable { + cursor: pointer; +} + +/* Name column: icon + stacked title/description */ +.resource-table-name { + display: flex; + align-items: center; + gap: 10px; + min-width: 0; +} + +.resource-table-name-img { + width: 36px; + height: 36px; + max-width: 36px; + object-fit: contain; + flex-shrink: 0; +} + +.resource-table-name-emoji { + font-size: 20px; + line-height: 1; + flex-shrink: 0; +} + +/* The fixed-width Name column bounds this; min-width:0 lets the title/desc + ellipsize within the cell. */ +.resource-table-name-text { + display: flex; + flex-direction: column; + min-width: 0; +} + +.resource-table-name-title { + font-size: 15px; +} + +.resource-table-name-desc { + font-size: 12px; +} + +/* Owned By column: avatar + name/email, clickable to manage co-owners */ +.resource-table-owner { + min-width: 0; +} + +.resource-table-owner-avatar { + font-size: 12px; + flex-shrink: 0; +} + +.resource-table-owner-text { + display: flex; + flex-direction: column; + min-width: 0; + line-height: 1.3; +} + +.resource-table-owner-name { + font-weight: 500; + max-width: 190px; +} + +.resource-table-owner-email { + font-size: 12px; + max-width: 190px; +} + +.resource-table-owner-btn { + background: none; + border: none; + padding: 0; + margin: 0; + font: inherit; + color: inherit; + cursor: pointer; + text-align: left; + width: 100%; +} + +.resource-table-owner-btn:hover .resource-table-owner-name { + color: #1890ff; +} + +/* Actions column */ +.resource-table-actions { + justify-content: flex-end; +} + +.resource-table .disabled-icon { + opacity: 0.4; + cursor: not-allowed; +} + +/* Destructive action is red, per the design */ +.resource-table .delete-icon:not(.disabled-icon) { + color: #ff4d4f; +} diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx new file mode 100644 index 0000000000..23398a1f8f --- /dev/null +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -0,0 +1,389 @@ +import { + CaretDownOutlined, + CaretUpOutlined, + ClearOutlined, + DeleteOutlined, + EditOutlined, + QuestionCircleOutlined, + ShareAltOutlined, + SortAscendingOutlined, + SortDescendingOutlined, +} from "@ant-design/icons"; +import { + Avatar, + Dropdown, + Popconfirm, + Space, + Table, + Tooltip, + Typography, +} from "antd"; +import PropTypes from "prop-types"; +import { useNavigate } from "react-router-dom"; + +import "./ResourceTable.css"; + +// Stable, distinct avatar swatch per owner (seeded on email/name) like the design. +const AVATAR_COLORS = [ + "#f56a00", + "#7265e6", + "#00a2ae", + "#d48806", + "#1677ff", + "#eb2f96", + "#52c41a", + "#722ed1", +]; +const colorForSeed = (seed = "") => { + let hash = 0; + for (let i = 0; i < seed.length; i += 1) { + hash = seed.charCodeAt(i) + ((hash << 5) - hash); + } + return AVATAR_COLORS[Math.abs(hash) % AVATAR_COLORS.length]; +}; + +// Sort-menu wording differs for text vs date columns (per the design). +const SORT_OPTIONS = { + text: [ + { key: "asc", label: "A-Z", icon: }, + { key: "desc", label: "Z-A", icon: }, + ], + date: [ + { key: "asc", label: "Oldest First", icon: }, + { key: "desc", label: "Newest First", icon: }, + ], +}; + +const formatDate = (value) => { + if (!value) { + return "-"; + } + const date = new Date(value); + if (Number.isNaN(date.getTime())) { + return "-"; + } + return date.toLocaleDateString("en-US", { + month: "short", + day: "numeric", + year: "numeric", + }); +}; + +/** + * Column header with a sort dropdown (A-Z / Z-A / Clear Sort, or Oldest / + * Newest First for dates). Server-driven — picking an option refetches. + * @return {JSX.Element} Rendered sortable header + */ +function SortHeader({ label, sortKey, sortType = "text", sort, onSortChange }) { + const active = sort?.sortBy === sortKey; + const items = [ + ...SORT_OPTIONS[sortType], + { type: "divider" }, + { key: "clear", label: "Clear Sort", icon: }, + ]; + const onClick = ({ key, domEvent }) => { + domEvent?.stopPropagation(); + if (key === "clear") { + onSortChange?.("", "asc"); + } else { + onSortChange?.(sortKey, key); + } + }; + return ( + + + + ); +} + +SortHeader.propTypes = { + label: PropTypes.string.isRequired, + sortKey: PropTypes.string.isRequired, + sortType: PropTypes.oneOf(["text", "date"]), + sort: PropTypes.object, + onSortChange: PropTypes.func, +}; + +/** + * Sortable resource list table (Name / Owned By / Created Date / Actions). + * Sorting, search and pagination are server-driven — the parent owns the fetch. + * @return {JSX.Element} Rendered table + */ +function ResourceTable({ + dataSource, + loading, + pagination, + sort, + onPaginationChange, + onSortChange, + titleProp, + descriptionProp, + iconProp, + idProp, + dateProp = "created_at", + ownerEmailProp = "created_by_email", + handleEdit, + handleShare, + handleDelete, + handleCoOwner, + sessionDetails, + showOwner = true, + isClickable = true, + type, +}) { + const navigate = useNavigate(); + + const renderName = (item) => { + const icon = iconProp ? item?.[iconProp] : null; + const isImage = icon && icon.length > 4; + return ( +
+ {icon && + (isImage ? ( + + ) : ( + {icon} + ))} +
+ + {item?.[titleProp]} + + {descriptionProp && item?.[descriptionProp] && ( + + {item[descriptionProp]} + + )} +
+
+ ); + }; + + const renderOwner = (item) => { + const email = item?.[ownerEmailProp]; + const isMe = item?.is_owner || (email && email === sessionDetails?.email); + const name = isMe ? "Me" : email?.split("@")[0] || "Unknown"; + const extra = + item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; + const initials = (isMe ? email || "Me" : name).slice(0, 2).toUpperCase(); + + const cell = ( + + + {initials} + +
+ + {name} + {extra} + + {email && email !== name && ( + + {email} + + )} +
+ + ); + + if (!handleCoOwner) { + return cell; + } + return ( + + + + ); + }; + + const renderActions = (item) => { + const deprecated = item?.is_deprecated; + const disabledTitle = deprecated ? "This adapter is deprecated" : ""; + return ( + event.stopPropagation()} + role="none" + > + + !deprecated && handleEdit?.(event, item)} + /> + + {handleShare && ( + + !deprecated && handleShare(event, item, true)} + /> + + )} + } + onConfirm={(event) => handleDelete?.(event, item)} + > + + + + ); + }; + + const columns = [ + { + title: ( + + ), + key: "name", + width: "40%", + render: (_, item) => renderName(item), + }, + showOwner && { + title: ( + + ), + key: "owner", + width: "26%", + render: (_, item) => renderOwner(item), + }, + { + title: ( + + ), + key: "created", + width: "19%", + render: (_, item) => formatDate(item?.[dateProp]), + }, + { + title: Actions, + key: "actions", + width: "15%", + align: "right", + render: (_, item) => renderActions(item), + }, + ].filter(Boolean); + + // Sorting is handled by the header dropdowns, so onChange only carries the + // pager here. + const handleChange = (paginationConf) => { + onPaginationChange?.(paginationConf.current, paginationConf.pageSize); + }; + + return ( + ({ + onClick: isClickable ? () => navigate(`${item?.[idProp]}`) : undefined, + })} + pagination={{ + current: pagination?.current, + pageSize: pagination?.pageSize, + total: pagination?.total, + showSizeChanger: false, + showTotal: (total) => + `Page ${pagination?.current} of ${Math.max( + 1, + Math.ceil(total / (pagination?.pageSize || 1)), + )} · ${total} items`, + }} + /> + ); +} + +ResourceTable.propTypes = { + dataSource: PropTypes.array, + loading: PropTypes.bool, + pagination: PropTypes.object, + sort: PropTypes.object, + onPaginationChange: PropTypes.func, + onSortChange: PropTypes.func, + titleProp: PropTypes.string.isRequired, + descriptionProp: PropTypes.string, + iconProp: PropTypes.string, + idProp: PropTypes.string.isRequired, + dateProp: PropTypes.string, + ownerEmailProp: PropTypes.string, + handleEdit: PropTypes.func, + handleShare: PropTypes.func, + handleDelete: PropTypes.func, + handleCoOwner: PropTypes.func, + sessionDetails: PropTypes.object, + showOwner: PropTypes.bool, + isClickable: PropTypes.bool, + type: PropTypes.string, +}; + +export { ResourceTable }; diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 7d5567c36f..10f58ad047 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -1,5 +1,5 @@ import { PlusOutlined, UserOutlined } from "@ant-design/icons"; -import { Pagination, Typography } from "antd"; +import { Typography } from "antd"; import PropTypes from "prop-types"; import { useCallback, useEffect, useRef, useState } from "react"; import { useLocation, useNavigate } from "react-router-dom"; @@ -18,13 +18,13 @@ import { useSessionStore } from "../../../store/session-store"; import { useWorkflowStore } from "../../../store/workflow-store"; import { usePromptStudioService } from "../../api/prompt-studio-service"; import { PromptStudioModal } from "../../common/PromptStudioModal"; -import { ViewTools } from "../../custom-tools/view-tools/ViewTools.jsx"; import { groupsService } from "../../groups/groups-service.js"; import { ToolNavBar } from "../../navigations/tool-nav-bar/ToolNavBar.jsx"; import { CoOwnerManagement } from "../../widgets/co-owner-management/CoOwnerManagement.jsx"; import { CustomButton } from "../../widgets/custom-button/CustomButton.jsx"; import { EmptyState } from "../../widgets/empty-state/EmptyState.jsx"; import { LazyLoader } from "../../widgets/lazy-loader/LazyLoader.jsx"; +import { ResourceTable } from "../../widgets/resource-table/ResourceTable.jsx"; import { SharePermission } from "../../widgets/share-permission/SharePermission.jsx"; import { SpinnerLoader } from "../../widgets/spinner-loader/SpinnerLoader.jsx"; import { workflowService } from "./workflow-service"; @@ -69,22 +69,32 @@ function Workflows() { pagination, setPagination, searchTerm, + sort, handlePaginationChange, handleSearch, + handleSortChange, } = usePaginatedList({ fetchData: (...args) => fetchListRef.current?.(...args), defaultPageSize: DEFAULT_PAGE_SIZE, }); - // Refresh the current page (preserves page + active search) after mutations + // Refresh the current page (preserves page + active search/sort) after mutations const handleListRefresh = useCallback( () => fetchListRef.current?.( pagination.current, pagination.pageSize, searchTerm, + sort.sortBy, + sort.order, ), - [pagination.current, pagination.pageSize, searchTerm], + [ + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ], ); const { coOwnerOpen, @@ -115,12 +125,18 @@ function Workflows() { page = 1, pageSize = DEFAULT_PAGE_SIZE, search = "", + sortBy = "", + order = "asc", ) => { setLoading(true); const params = { page, page_size: pageSize }; if (search) { params.search = search; } + if (sortBy) { + params.sort_by = sortBy; + params.order = order; + } projectApiService .getProjectList(params) .then((res) => { @@ -131,7 +147,7 @@ function Workflows() { const total = data?.count ?? results.length; // Deleting the last row on a page leaves it empty; step back a page. if (results.length === 0 && page > 1 && total > 0) { - getProjectList(page - 1, pageSize, search); + getProjectList(page - 1, pageSize, search, sortBy, order); return; } setProjectList(results); @@ -423,33 +439,27 @@ function Workflows() { )} {projectList?.length > 0 && ( - <> - - {pagination.total > pagination.pageSize && ( -
- -
- )} - + )} {editingProject && ( { const newPage = pageSize === pagination.pageSize ? page : 1; - fetchRef.current?.(newPage, pageSize, searchTerm); + fetchRef.current?.(newPage, pageSize, searchTerm, sort.sortBy, sort.order); }; const handleSearch = (searchText) => { const term = searchText?.trim() || ""; setSearchTerm(term); - fetchRef.current?.(1, pagination.pageSize, term); + fetchRef.current?.(1, pagination.pageSize, term, sort.sortBy, sort.order); + }; + + // Table header sort click: reset to page 1 (the page a row sits on changes) + // and refetch with the new ordering. + const handleSortChange = (sortBy, order) => { + const nextSort = { sortBy: sortBy || "", order: order || "asc" }; + setSort(nextSort); + fetchRef.current?.( + 1, + pagination.pageSize, + searchTerm, + nextSort.sortBy, + nextSort.order, + ); }; return { @@ -36,8 +61,10 @@ function usePaginatedList({ fetchData, defaultPageSize = 10 }) { setPagination, searchTerm, setSearchTerm, + sort, handlePaginationChange, handleSearch, + handleSortChange, }; } diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index cacb50fc27..858f148822 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -1,22 +1,26 @@ import { PlusOutlined } from "@ant-design/icons"; import { Button } from "antd"; -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { ViewTools } from "../components/custom-tools/view-tools/ViewTools"; import { groupsService } from "../components/groups/groups-service.js"; import { AddSourceModal } from "../components/input-output/add-source-modal/AddSourceModal"; import { ToolNavBar } from "../components/navigations/tool-nav-bar/ToolNavBar"; import { CoOwnerManagement } from "../components/widgets/co-owner-management/CoOwnerManagement"; +import { EmptyState } from "../components/widgets/empty-state/EmptyState.jsx"; +import { ResourceTable } from "../components/widgets/resource-table/ResourceTable"; import { SharePermission } from "../components/widgets/share-permission/SharePermission"; +import { SpinnerLoader } from "../components/widgets/spinner-loader/SpinnerLoader.jsx"; import { useAxiosPrivate } from "../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../hooks/useExceptionHandler"; -import { useListSearch } from "../hooks/useListSearch"; +import { usePaginatedList } from "../hooks/usePaginatedList"; import useRequestUrl from "../hooks/useRequestUrl"; import { useAlertStore } from "../store/alert-store"; import { useSessionStore } from "../store/session-store"; import "./ConnectorsPage.css"; +const DEFAULT_PAGE_SIZE = 10; + function ConnectorsPage() { const [loading, setLoading] = useState(false); const [modalVisible, setModalVisible] = useState(false); @@ -27,6 +31,8 @@ function ConnectorsPage() { const [groupList, setGroupList] = useState([]); const [isPermissionEdit, setIsPermissionEdit] = useState(false); const [isShareLoading, setIsShareLoading] = useState(false); + // undefined = not fetched yet (spinner); [] = fetched-empty (empty state) + const [displayList, setDisplayList] = useState(); const groupsApi = groupsService(); const axiosPrivate = useAxiosPrivate(); @@ -34,6 +40,8 @@ function ConnectorsPage() { const { setAlertDetails } = useAlertStore(); const handleException = useExceptionHandler(); const { getUrl } = useRequestUrl(); + // Ref forwards the fetch fn to the pagination hook (avoids declaration order). + const fetchListRef = useRef(null); const connectorCoOwnerService = useMemo( () => ({ @@ -61,6 +69,38 @@ function ConnectorsPage() { [sessionDetails?.csrfToken], ); + const { + pagination, + setPagination, + searchTerm, + sort, + handlePaginationChange, + handleSearch, + handleSortChange, + } = usePaginatedList({ + fetchData: (...args) => fetchListRef.current?.(...args), + defaultPageSize: DEFAULT_PAGE_SIZE, + }); + + // Refresh the current page (preserves page + active search/sort) after mutations + const handleListRefresh = useCallback( + () => + fetchListRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ), + [ + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ], + ); + const { coOwnerOpen, setCoOwnerOpen, @@ -74,28 +114,66 @@ function ConnectorsPage() { } = useCoOwnerManagement({ service: connectorCoOwnerService, setAlertDetails, - onListRefresh: () => fetchConnectors(), + onListRefresh: handleListRefresh, }); - const { listRef, displayList, setDisplayList, setMasterList, onSearch } = - useListSearch("connector_name"); + + const getConnectors = useCallback( + ( + page = 1, + pageSize = DEFAULT_PAGE_SIZE, + search = "", + sortBy = "", + order = "asc", + ) => { + const params = { page, page_size: pageSize }; + if (search) { + params.search = search; + } + if (sortBy) { + params.sort_by = sortBy; + params.order = order; + } + setLoading(true); + axiosPrivate + .get(getUrl("connector/"), { params }) + .then((res) => { + const data = res?.data; + // Endpoint is opt-in paginated: envelope when we send ?page, else a + // bare array. Handle both so shared (dropdown) callers stay unaffected. + const results = data?.results ?? data ?? []; + const total = data?.count ?? results.length; + // Deleting the last row on a page leaves it empty; step back a page. + if (results.length === 0 && page > 1 && total > 0) { + getConnectors(page - 1, pageSize, search, sortBy, order); + return; + } + setDisplayList(results); + setPagination((prev) => ({ + ...prev, + current: page, + pageSize, + total, + })); + }) + .catch((err) => { + setAlertDetails(handleException(err, "Failed to load connectors")); + // Avoid an indefinite spinner when the first fetch fails. + setDisplayList((prev) => prev ?? []); + }) + .finally(() => { + setLoading(false); + }); + }, + [axiosPrivate, getUrl, setPagination, setAlertDetails, handleException], + ); + fetchListRef.current = getConnectors; useEffect(() => { - fetchConnectors(); + getConnectors(1, DEFAULT_PAGE_SIZE, "", "", "asc"); fetchUsers(); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - const fetchConnectors = async () => { - setLoading(true); - try { - const response = await axiosPrivate.get(getUrl("connector/")); - setMasterList(response.data || []); - } catch (error) { - setAlertDetails(handleException(error, "Failed to load connectors")); - } finally { - setLoading(false); - } - }; - const fetchUsers = async () => { try { const response = await axiosPrivate.get(getUrl("users/")); @@ -134,7 +212,7 @@ function ConnectorsPage() { type: "success", content: "Connector deleted successfully", }); - fetchConnectors(); + handleListRefresh(); } catch (error) { setAlertDetails(handleException(error, "Failed to delete connector")); } @@ -204,14 +282,18 @@ function ConnectorsPage() { }; const handleCoOwner = (_event, connector) => { - if (!connector?.id) return; + if (!connector?.id) { + return; + } handleCoOwnerAction(connector.id); }; const handleConnectorSaved = () => { setModalVisible(false); setEditingConnector(null); - fetchConnectors(); + // New/edited connectors land on some page under the active sort — refetch + // the current page to reflect server truth rather than splicing a stale array. + handleListRefresh(); setAlertDetails({ type: "success", content: editingConnector @@ -235,30 +317,45 @@ function ConnectorsPage() { handleSearch(value)} customButtons={newConnectorButton} />
- + {displayList === undefined && } + {displayList?.length === 0 && !searchTerm && ( + + )} + {displayList?.length === 0 && searchTerm && ( + + )} + {displayList?.length > 0 && ( + + )}
Date: Thu, 23 Jul 2026 19:25:38 +0530 Subject: [PATCH 02/11] UN-3769 [FIX] Dedupe list fetch into shared helpers; fix stale-response race Resolve SonarCloud/Greptile/CodeRabbit review on the resource-list rollout: - Extract buildPagedParams + applyPagedResponse into usePaginatedList so the four list pages stop copy-pasting the params/response blocks (clears the SonarCloud new-code duplication gate). - applyPagedResponse drops stale responses via a per-page sequence token so a slow older request can't overwrite a newer query, and returns the empty-page stepback refetch so loading isn't cleared before replacement data arrives. - ResourceTable detects image icons by URL/data scheme instead of length, so compound (ZWJ) emoji no longer render as a broken . - list_query lowercases sort_by before the dict lookup (matches order handling). - ToolSettings resets loading when a delete request fails. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/utils/list_query.py | 2 +- .../list-of-tools/ListOfTools.jsx | 56 +++++++-------- .../tool-settings/ToolSettings.jsx | 66 ++++++++--------- .../widgets/resource-table/ResourceTable.jsx | 6 +- .../workflows/workflow/Workflows.jsx | 50 ++++++------- frontend/src/hooks/usePaginatedList.js | 72 ++++++++++++++++++- frontend/src/pages/ConnectorsPage.jsx | 56 +++++++-------- 7 files changed, 189 insertions(+), 119 deletions(-) diff --git a/backend/utils/list_query.py b/backend/utils/list_query.py index 401eff498b..d86ece0673 100644 --- a/backend/utils/list_query.py +++ b/backend/utils/list_query.py @@ -61,7 +61,7 @@ def apply_search_and_sort( "name": name_field, "owner": OWNER_SORT_FIELD, "created": CREATED_SORT_FIELD, - }.get(params.get("sort_by") or default_sort_by, name_field) + }.get((params.get("sort_by") or default_sort_by).lower(), name_field) order_prefix = "-" if (params.get("order") or "asc").lower() == "desc" else "" # Ordering the source by ``pk`` keeps the DISTINCT ON (always the pk) valid diff --git a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx index 70be4167f6..19c2184047 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -6,7 +6,11 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { + applyPagedResponse, + buildPagedParams, + usePaginatedList, +} from "../../../hooks/usePaginatedList"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import { useAlertStore } from "../../../store/alert-store"; import { useSessionStore } from "../../../store/session-store"; @@ -81,6 +85,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const [allGroupList, setAllGroupList] = useState([]); // Ref forwards the fetch fn to the pagination hook (avoids declaration order). const fetchListRef = useRef(null); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const promptStudioCoOwnerService = useMemo( () => ({ @@ -172,40 +178,34 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { sortBy = "", order = "asc", ) => { - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - if (sortBy) { - params.sort_by = sortBy; - params.order = order; - } + const params = buildPagedParams({ + page, + pageSize, + search, + sortBy, + order, + }); + const seq = ++seqRef.current; setIsLoading(true); - axiosPrivate({ + return axiosPrivate({ method: "GET", url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, headers: { "X-CSRFToken": sessionDetails?.csrfToken }, params, }) - .then((res) => { - const data = res?.data; - // Endpoint is opt-in paginated: envelope when we send ?page, else a - // bare array. Handle both so any non-paginated caller stays unaffected. - const results = data?.results ?? data ?? []; - const total = data?.count ?? results.length; - // Deleting the last row on a page leaves it empty; step back a page. - if (results.length === 0 && page > 1 && total > 0) { - getListOfTools(page - 1, pageSize, search, sortBy, order); - return; - } - setDisplayList(results); - setPagination((prev) => ({ - ...prev, - current: page, + .then((res) => + applyPagedResponse({ + data: res?.data, + page, pageSize, - total, - })); - }) + seq, + latestSeqRef: seqRef, + setList: setDisplayList, + setPagination, + refetchPrevPage: () => + getListOfTools(page - 1, pageSize, search, sortBy, order), + }), + ) .catch((err) => { setAlertDetails( handleException(err, "Failed to get the list of tools"), diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index fb057c5d79..6c8cefa3d4 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -5,7 +5,11 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { + applyPagedResponse, + buildPagedParams, + usePaginatedList, +} from "../../../hooks/usePaginatedList"; import usePostHogEvents from "../../../hooks/usePostHogEvents"; import { IslandLayout } from "../../../layouts/island-layout/IslandLayout"; import { useAlertStore } from "../../../store/alert-store"; @@ -60,6 +64,8 @@ function ToolSettings({ type }) { const handleException = useExceptionHandler(); // Ref forwards the fetch fn to the pagination hook (avoids declaration order). const fetchListRef = useRef(null); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const adapterCoOwnerService = useMemo( () => ({ @@ -155,43 +161,34 @@ function ToolSettings({ type }) { if (!type) { return; } - const params = { - adapter_type: type.toUpperCase(), + const params = buildPagedParams({ page, - page_size: pageSize, - }; - if (search) { - params.search = search; - } - if (sortBy) { - params.sort_by = sortBy; - params.order = order; - } + pageSize, + search, + sortBy, + order, + }); + params.adapter_type = type.toUpperCase(); + const seq = ++seqRef.current; setIsLoading(true); - axiosPrivate({ + return axiosPrivate({ method: "GET", url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter`, params, }) - .then((res) => { - const data = res?.data; - // Endpoint is opt-in paginated: envelope when we send ?page, else a - // bare array. Handle both so shared (dropdown) callers stay unaffected. - const results = data?.results ?? data ?? []; - const total = data?.count ?? results.length; - // Deleting the last row on a page leaves it empty; step back a page. - if (results.length === 0 && page > 1 && total > 0) { - getAdapters(page - 1, pageSize, search, sortBy, order); - return; - } - setDisplayList(results); - setPagination((prev) => ({ - ...prev, - current: page, + .then((res) => + applyPagedResponse({ + data: res?.data, + page, pageSize, - total, - })); - }) + seq, + latestSeqRef: seqRef, + setList: setDisplayList, + setPagination, + refetchPrevPage: () => + getAdapters(page - 1, pageSize, search, sortBy, order), + }), + ) .catch((err) => { setAlertDetails(handleException(err)); // Avoid an indefinite spinner when the first fetch fails. @@ -234,7 +231,12 @@ function ToolSettings({ type }) { headers: { "X-CSRFToken": sessionDetails?.csrfToken }, }) .then(() => handleListRefresh()) - .catch((err) => setAlertDetails(handleException(err))); + .catch((err) => { + setAlertDetails(handleException(err)); + // Refresh only runs on success; clear loading here so a failed delete + // doesn't leave the table stuck under its spinner. + setIsLoading(false); + }); }; const handleEdit = (_event, item) => { diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 23398a1f8f..1d2c4d098c 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -147,7 +147,11 @@ function ResourceTable({ const renderName = (item) => { const icon = iconProp ? item?.[iconProp] : null; - const isImage = icon && icon.length > 4; + // Adapters/connectors pass image URLs; Prompt Studio passes emoji. Detect an + // actual URL/data source rather than a length heuristic — compound (ZWJ) + // emoji exceed 4 UTF-16 units and would otherwise render as a broken . + const isImage = + typeof icon === "string" && /^(https?:\/\/|\/|data:image\/)/.test(icon); return (
{icon && diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 10f58ad047..332b7016ec 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -6,7 +6,11 @@ import { useLocation, useNavigate } from "react-router-dom"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement.jsx"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler.jsx"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { + applyPagedResponse, + buildPagedParams, + usePaginatedList, +} from "../../../hooks/usePaginatedList"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import { useInitialFetchCount, @@ -55,6 +59,8 @@ function Workflows() { const [openModal, toggleModal] = useState(true); // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) const fetchListRef = useRef(null); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const [backendErrors, setBackendErrors] = useState(null); const [shareOpen, setShareOpen] = useState(false); const [selectedWorkflow, setSelectedWorkflow] = useState(); @@ -128,36 +134,24 @@ function Workflows() { sortBy = "", order = "asc", ) => { + const params = buildPagedParams({ page, pageSize, search, sortBy, order }); + const seq = ++seqRef.current; setLoading(true); - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - if (sortBy) { - params.sort_by = sortBy; - params.order = order; - } - projectApiService + return projectApiService .getProjectList(params) - .then((res) => { - const data = res?.data; - // Endpoint is opt-in paginated: envelope when we send ?page, else a - // bare array. Handle both so nothing breaks if the opt-in is dropped. - const results = data?.results ?? data ?? []; - const total = data?.count ?? results.length; - // Deleting the last row on a page leaves it empty; step back a page. - if (results.length === 0 && page > 1 && total > 0) { - getProjectList(page - 1, pageSize, search, sortBy, order); - return; - } - setProjectList(results); - setPagination((prev) => ({ - ...prev, - current: page, + .then((res) => + applyPagedResponse({ + data: res?.data, + page, pageSize, - total, - })); - }) + seq, + latestSeqRef: seqRef, + setList: setProjectList, + setPagination, + refetchPrevPage: () => + getProjectList(page - 1, pageSize, search, sortBy, order), + }), + ) .catch(() => { console.error("Unable to get project list"); // Avoid an indefinite spinner when the first fetch fails. diff --git a/frontend/src/hooks/usePaginatedList.js b/frontend/src/hooks/usePaginatedList.js index 78ca8939ba..b880769eaa 100644 --- a/frontend/src/hooks/usePaginatedList.js +++ b/frontend/src/hooks/usePaginatedList.js @@ -1,5 +1,75 @@ import { useRef, useState } from "react"; +/** + * Build the query params for a paginated list request, omitting empty + * search/sort so non-paginated callers stay unaffected. Callers add any + * resource-specific params (e.g. `adapter_type`) to the returned object. + * + * @param {Object} args + * @param {number} args.page - Requested page. + * @param {number} args.pageSize - Requested page size. + * @param {string} [args.search] - Search term. + * @param {string} [args.sortBy] - Sort column key. + * @param {string} [args.order] - Sort direction. + * @return {Object} Query params. + */ +function buildPagedParams({ page, pageSize, search, sortBy, order }) { + const params = { page, page_size: pageSize }; + if (search) { + params.search = search; + } + if (sortBy) { + params.sort_by = sortBy; + params.order = order; + } + return params; +} + +/** + * Commit a paginated list response, shared by every resource list page. + * + * Unwraps the opt-in envelope (`{results, count}` or a bare array), drops stale + * responses that a newer request already superseded, and steps back a page when + * a delete empties the last one — returning that refetch promise so the caller's + * `finally` waits for replacement data instead of clearing loading early. + * + * @param {Object} args + * @param {*} args.data - `res.data`: envelope or bare array. + * @param {number} args.page - Requested page. + * @param {number} args.pageSize - Requested page size. + * @param {number} args.seq - This request's sequence token. + * @param {{current: number}} args.latestSeqRef - Ref holding the newest token. + * @param {Function} args.setList - List state setter. + * @param {Function} args.setPagination - Pagination state setter. + * @param {Function} args.refetchPrevPage - Refetches `page - 1`; its promise is + * returned so the caller's `finally` waits for it. + * @return {Promise|undefined} + */ +function applyPagedResponse({ + data, + page, + pageSize, + seq, + latestSeqRef, + setList, + setPagination, + refetchPrevPage, +}) { + // A newer request already fired — ignore this superseded response. + if (seq !== latestSeqRef.current) { + return undefined; + } + const results = data?.results ?? data ?? []; + const total = data?.count ?? results.length; + // Deleting the last row on a page leaves it empty; step back a page. + if (results.length === 0 && page > 1 && total > 0) { + return refetchPrevPage(); + } + setList(results); + setPagination((prev) => ({ ...prev, current: page, pageSize, total })); + return undefined; +} + /** * Shared hook for paginated list state and handlers. * Uses a ref internally to avoid stale closure issues with fetchData. @@ -68,4 +138,4 @@ function usePaginatedList({ }; } -export { usePaginatedList }; +export { applyPagedResponse, buildPagedParams, usePaginatedList }; diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 858f148822..08dd62e495 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -13,7 +13,11 @@ import { SpinnerLoader } from "../components/widgets/spinner-loader/SpinnerLoade import { useAxiosPrivate } from "../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../hooks/useExceptionHandler"; -import { usePaginatedList } from "../hooks/usePaginatedList"; +import { + applyPagedResponse, + buildPagedParams, + usePaginatedList, +} from "../hooks/usePaginatedList"; import useRequestUrl from "../hooks/useRequestUrl"; import { useAlertStore } from "../store/alert-store"; import { useSessionStore } from "../store/session-store"; @@ -42,6 +46,8 @@ function ConnectorsPage() { const { getUrl } = useRequestUrl(); // Ref forwards the fetch fn to the pagination hook (avoids declaration order). const fetchListRef = useRef(null); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const connectorCoOwnerService = useMemo( () => ({ @@ -125,36 +131,30 @@ function ConnectorsPage() { sortBy = "", order = "asc", ) => { - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - if (sortBy) { - params.sort_by = sortBy; - params.order = order; - } + const params = buildPagedParams({ + page, + pageSize, + search, + sortBy, + order, + }); + const seq = ++seqRef.current; setLoading(true); - axiosPrivate + return axiosPrivate .get(getUrl("connector/"), { params }) - .then((res) => { - const data = res?.data; - // Endpoint is opt-in paginated: envelope when we send ?page, else a - // bare array. Handle both so shared (dropdown) callers stay unaffected. - const results = data?.results ?? data ?? []; - const total = data?.count ?? results.length; - // Deleting the last row on a page leaves it empty; step back a page. - if (results.length === 0 && page > 1 && total > 0) { - getConnectors(page - 1, pageSize, search, sortBy, order); - return; - } - setDisplayList(results); - setPagination((prev) => ({ - ...prev, - current: page, + .then((res) => + applyPagedResponse({ + data: res?.data, + page, pageSize, - total, - })); - }) + seq, + latestSeqRef: seqRef, + setList: setDisplayList, + setPagination, + refetchPrevPage: () => + getConnectors(page - 1, pageSize, search, sortBy, order), + }), + ) .catch((err) => { setAlertDetails(handleException(err, "Failed to load connectors")); // Avoid an indefinite spinner when the first fetch fails. From 4bfae22a22db95435ce8bcf0b43749430fe2ba91 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Thu, 23 Jul 2026 19:50:37 +0530 Subject: [PATCH 03/11] UN-3769 [FIX] Collapse duplicated list-page preamble to clear duplication gate The first review pass left SonarCloud at 8.4% new-code duplication; the real duplicated blocks were the per-page preamble and the co-owner modal JSX, not the fetch body. Fix both: - usePaginatedList now owns fetchRef (pages assign fetchRef.current) and returns handleListRefresh, so pages drop their local fetchListRef + identical handleListRefresh useCallback. - Add CoOwnerModal, a thin wrapper mapping a useCoOwnerManagement() bag + resourceType onto the CoOwnerManagement modal; the list pages now consume the hook as one object and render instead of repeating the 11-prop invocation. Net ~150 fewer lines; duplication drops well under the 3% gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../list-of-tools/ListOfTools.jsx | 59 +++-------------- .../tool-settings/ToolSettings.jsx | 59 +++-------------- .../co-owner-management/CoOwnerModal.jsx | 37 +++++++++++ .../workflows/workflow/Workflows.jsx | 63 +++---------------- frontend/src/hooks/usePaginatedList.js | 40 +++++++++--- frontend/src/pages/ConnectorsPage.jsx | 59 +++-------------- 6 files changed, 102 insertions(+), 215 deletions(-) create mode 100644 frontend/src/components/widgets/co-owner-management/CoOwnerModal.jsx diff --git a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx index 19c2184047..88bc6631f1 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -16,7 +16,7 @@ import { useAlertStore } from "../../../store/alert-store"; import { useSessionStore } from "../../../store/session-store"; import { groupsService } from "../../groups/groups-service.js"; import { ToolNavBar } from "../../navigations/tool-nav-bar/ToolNavBar"; -import { CoOwnerManagement } from "../../widgets/co-owner-management/CoOwnerManagement"; +import { CoOwnerModal } from "../../widgets/co-owner-management/CoOwnerModal"; import { CustomButton } from "../../widgets/custom-button/CustomButton"; import { EmptyState } from "../../widgets/empty-state/EmptyState.jsx"; import { ResourceTable } from "../../widgets/resource-table/ResourceTable"; @@ -83,8 +83,6 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const [isShareLoading, setIsShareLoading] = useState(false); const [allUserList, setAllUserList] = useState([]); const [allGroupList, setAllGroupList] = useState([]); - // Ref forwards the fetch fn to the pagination hook (avoids declaration order). - const fetchListRef = useRef(null); // Monotonic request token so a stale response can't overwrite a newer one. const seqRef = useRef(0); @@ -127,44 +125,14 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { searchTerm, setSearchTerm, sort, + fetchRef, handlePaginationChange, handleSearch, handleSortChange, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), - defaultPageSize: DEFAULT_PAGE_SIZE, - }); + handleListRefresh, + } = usePaginatedList({ defaultPageSize: DEFAULT_PAGE_SIZE }); - // Refresh the current page (preserves page + active search/sort) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ), - [ - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ], - ); - - const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + const coOwner = useCoOwnerManagement({ service: promptStudioCoOwnerService, setAlertDetails, onListRefresh: handleListRefresh, @@ -226,7 +194,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { handleException, ], ); - fetchListRef.current = getListOfTools; + fetchRef.current = getListOfTools; useEffect(() => { setSearchTerm(""); @@ -451,7 +419,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { }; const handleCoOwner = (_event, tool) => { - handleCoOwnerAction(tool.tool_id); + coOwner.handleCoOwner(tool.tool_id); }; const customButtonsElement = useMemo( @@ -542,18 +510,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { onApply={onShare} isSharableToOrg={true} /> - + ); } diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 6c8cefa3d4..61f93afbea 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -18,7 +18,7 @@ import { groupsService } from "../../groups/groups-service.js"; import { AddSourceModal } from "../../input-output/add-source-modal/AddSourceModal"; import "../../input-output/data-source-card/DataSourceCard.css"; import { ToolNavBar } from "../../navigations/tool-nav-bar/ToolNavBar"; -import { CoOwnerManagement } from "../../widgets/co-owner-management/CoOwnerManagement"; +import { CoOwnerModal } from "../../widgets/co-owner-management/CoOwnerModal"; import { CustomButton } from "../../widgets/custom-button/CustomButton"; import { EmptyState } from "../../widgets/empty-state/EmptyState.jsx"; import { ResourceTable } from "../../widgets/resource-table/ResourceTable"; @@ -62,8 +62,6 @@ function ToolSettings({ type }) { const { setAlertDetails } = useAlertStore(); const axiosPrivate = useAxiosPrivate(); const handleException = useExceptionHandler(); - // Ref forwards the fetch fn to the pagination hook (avoids declaration order). - const fetchListRef = useRef(null); // Monotonic request token so a stale response can't overwrite a newer one. const seqRef = useRef(0); @@ -106,44 +104,14 @@ function ToolSettings({ type }) { searchTerm, setSearchTerm, sort, + fetchRef, handlePaginationChange, handleSearch, handleSortChange, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), - defaultPageSize: DEFAULT_PAGE_SIZE, - }); + handleListRefresh, + } = usePaginatedList({ defaultPageSize: DEFAULT_PAGE_SIZE }); - // Refresh the current page (preserves page + active search/sort) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ), - [ - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ], - ); - - const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + const coOwner = useCoOwnerManagement({ service: adapterCoOwnerService, setAlertDetails, onListRefresh: handleListRefresh, @@ -207,7 +175,7 @@ function ToolSettings({ type }) { handleException, ], ); - fetchListRef.current = getAdapters; + fetchRef.current = getAdapters; useEffect(() => { setSearchTerm(""); @@ -351,7 +319,7 @@ function ToolSettings({ type }) { }); return; } - handleCoOwnerAction(adapter.id); + coOwner.handleCoOwner(adapter.id); }; const handleOpenAddSourceModal = () => { @@ -442,18 +410,7 @@ function ToolSettings({ type }) { onApply={onShare} isSharableToOrg={true} /> - +
); } diff --git a/frontend/src/components/widgets/co-owner-management/CoOwnerModal.jsx b/frontend/src/components/widgets/co-owner-management/CoOwnerModal.jsx new file mode 100644 index 0000000000..e66be15223 --- /dev/null +++ b/frontend/src/components/widgets/co-owner-management/CoOwnerModal.jsx @@ -0,0 +1,37 @@ +import PropTypes from "prop-types"; + +import { CoOwnerManagement } from "./CoOwnerManagement"; + +/** + * Thin wrapper that maps a `useCoOwnerManagement()` bag plus `resourceType` + * onto the CoOwnerManagement modal, so resource list pages don't each repeat + * the identical prop wiring. + * + * @param {Object} props + * @param {Object} props.coOwner - The `useCoOwnerManagement()` return value. + * @param {string} props.resourceType - Human-readable resource label. + * @return {JSX.Element} + */ +function CoOwnerModal({ coOwner, resourceType }) { + return ( + + ); +} + +CoOwnerModal.propTypes = { + coOwner: PropTypes.object.isRequired, + resourceType: PropTypes.string.isRequired, +}; + +export { CoOwnerModal }; diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 332b7016ec..ad77433a5d 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -1,7 +1,7 @@ import { PlusOutlined, UserOutlined } from "@ant-design/icons"; import { Typography } from "antd"; import PropTypes from "prop-types"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useLocation, useNavigate } from "react-router-dom"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement.jsx"; @@ -24,7 +24,7 @@ import { usePromptStudioService } from "../../api/prompt-studio-service"; import { PromptStudioModal } from "../../common/PromptStudioModal"; import { groupsService } from "../../groups/groups-service.js"; import { ToolNavBar } from "../../navigations/tool-nav-bar/ToolNavBar.jsx"; -import { CoOwnerManagement } from "../../widgets/co-owner-management/CoOwnerManagement.jsx"; +import { CoOwnerModal } from "../../widgets/co-owner-management/CoOwnerModal.jsx"; import { CustomButton } from "../../widgets/custom-button/CustomButton.jsx"; import { EmptyState } from "../../widgets/empty-state/EmptyState.jsx"; import { LazyLoader } from "../../widgets/lazy-loader/LazyLoader.jsx"; @@ -57,8 +57,6 @@ function Workflows() { const [editingProject, setEditProject] = useState(); const [loading, setLoading] = useState(false); const [openModal, toggleModal] = useState(true); - // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) - const fetchListRef = useRef(null); // Monotonic request token so a stale response can't overwrite a newer one. const seqRef = useRef(0); const [backendErrors, setBackendErrors] = useState(null); @@ -76,43 +74,13 @@ function Workflows() { setPagination, searchTerm, sort, + fetchRef, handlePaginationChange, handleSearch, handleSortChange, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), - defaultPageSize: DEFAULT_PAGE_SIZE, - }); - - // Refresh the current page (preserves page + active search/sort) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ), - [ - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ], - ); - const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + handleListRefresh, + } = usePaginatedList({ defaultPageSize: DEFAULT_PAGE_SIZE }); + const coOwner = useCoOwnerManagement({ service: projectApiService, setAlertDetails, onListRefresh: handleListRefresh, @@ -161,7 +129,7 @@ function Workflows() { setLoading(false); }); }; - fetchListRef.current = getProjectList; + fetchRef.current = getProjectList; function editProject(name, description) { setLoading(true); @@ -370,7 +338,7 @@ function Workflows() { const handleCoOwner = (event, workflow) => { event.stopPropagation(); - handleCoOwnerAction(workflow.id); + coOwner.handleCoOwner(workflow.id); }; const handleNewWorkflowBtnClick = () => { @@ -483,19 +451,8 @@ function Workflows() { isSharableToOrg={true} /> )} - {coOwnerOpen && ( - + {coOwner.coOwnerOpen && ( + )} diff --git a/frontend/src/hooks/usePaginatedList.js b/frontend/src/hooks/usePaginatedList.js index b880769eaa..9ffa25da1e 100644 --- a/frontend/src/hooks/usePaginatedList.js +++ b/frontend/src/hooks/usePaginatedList.js @@ -1,4 +1,4 @@ -import { useRef, useState } from "react"; +import { useCallback, useRef, useState } from "react"; /** * Build the query params for a paginated list request, omitting empty @@ -72,21 +72,23 @@ function applyPagedResponse({ /** * Shared hook for paginated list state and handlers. - * Uses a ref internally to avoid stale closure issues with fetchData. * - * @param {Object} options - * @param {Function} options.fetchData - fn(page, pageSize, search, sortBy, order) + * Owns `fetchRef` — the page assigns its fetch fn to `fetchRef.current`, and the + * hook's handlers (search/sort/paginate) plus `handleListRefresh` invoke the + * latest one, so pages don't re-derive that wiring. `handleListRefresh` refetches + * the current page preserving active search/sort — pass it to mutation callbacks. + * + * @param {Object} [options] * @param {number} [options.defaultPageSize=10] * @param {string} [options.defaultSortBy=""] - initial sort column key * @param {string} [options.defaultOrder="asc"] - initial sort direction - * @return {Object} Pagination/sort state and handlers + * @return {Object} Pagination/sort state, `fetchRef`, and handlers. */ function usePaginatedList({ - fetchData, defaultPageSize = 10, defaultSortBy = "", defaultOrder = "asc", -}) { +} = {}) { const [pagination, setPagination] = useState({ current: 1, pageSize: defaultPageSize, @@ -98,8 +100,8 @@ function usePaginatedList({ order: defaultOrder, }); - const fetchRef = useRef(fetchData); - fetchRef.current = fetchData; + // Page assigns its fetch fn here; handlers call the latest one via the ref. + const fetchRef = useRef(null); const handlePaginationChange = (page, pageSize) => { const newPage = pageSize === pagination.pageSize ? page : 1; @@ -126,15 +128,35 @@ function usePaginatedList({ ); }; + const handleListRefresh = useCallback( + () => + fetchRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ), + [ + pagination.current, + pagination.pageSize, + searchTerm, + sort.sortBy, + sort.order, + ], + ); + return { pagination, setPagination, searchTerm, setSearchTerm, sort, + fetchRef, handlePaginationChange, handleSearch, handleSortChange, + handleListRefresh, }; } diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 08dd62e495..ff17f99970 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -5,7 +5,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { groupsService } from "../components/groups/groups-service.js"; import { AddSourceModal } from "../components/input-output/add-source-modal/AddSourceModal"; import { ToolNavBar } from "../components/navigations/tool-nav-bar/ToolNavBar"; -import { CoOwnerManagement } from "../components/widgets/co-owner-management/CoOwnerManagement"; +import { CoOwnerModal } from "../components/widgets/co-owner-management/CoOwnerModal"; import { EmptyState } from "../components/widgets/empty-state/EmptyState.jsx"; import { ResourceTable } from "../components/widgets/resource-table/ResourceTable"; import { SharePermission } from "../components/widgets/share-permission/SharePermission"; @@ -44,8 +44,6 @@ function ConnectorsPage() { const { setAlertDetails } = useAlertStore(); const handleException = useExceptionHandler(); const { getUrl } = useRequestUrl(); - // Ref forwards the fetch fn to the pagination hook (avoids declaration order). - const fetchListRef = useRef(null); // Monotonic request token so a stale response can't overwrite a newer one. const seqRef = useRef(0); @@ -80,44 +78,14 @@ function ConnectorsPage() { setPagination, searchTerm, sort, + fetchRef, handlePaginationChange, handleSearch, handleSortChange, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), - defaultPageSize: DEFAULT_PAGE_SIZE, - }); - - // Refresh the current page (preserves page + active search/sort) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ), - [ - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ], - ); + handleListRefresh, + } = usePaginatedList({ defaultPageSize: DEFAULT_PAGE_SIZE }); - const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + const coOwner = useCoOwnerManagement({ service: connectorCoOwnerService, setAlertDetails, onListRefresh: handleListRefresh, @@ -166,7 +134,7 @@ function ConnectorsPage() { }, [axiosPrivate, getUrl, setPagination, setAlertDetails, handleException], ); - fetchListRef.current = getConnectors; + fetchRef.current = getConnectors; useEffect(() => { getConnectors(1, DEFAULT_PAGE_SIZE, "", "", "asc"); @@ -285,7 +253,7 @@ function ConnectorsPage() { if (!connector?.id) { return; } - handleCoOwnerAction(connector.id); + coOwner.handleCoOwner(connector.id); }; const handleConnectorSaved = () => { @@ -377,18 +345,7 @@ function ConnectorsPage() { loading={isShareLoading} isSharableToOrg={true} /> - + ); } From b76e2e297edd499ae0a948365dc10599753bfacc Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Thu, 23 Jul 2026 20:06:14 +0530 Subject: [PATCH 04/11] UN-3769 [FIX] Gate list-fetch catch/finally on the request sequence The seq guard only suppressed stale successful responses; each page's catch and finally still ran unconditionally, so a superseded request could clear loading while a newer one was pending, or surface an error for a query the user had already moved past. Gate both on seq === seqRef.current so only the newest request owns the loading state and error reporting. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../custom-tools/list-of-tools/ListOfTools.jsx | 9 ++++++++- .../tool-settings/tool-settings/ToolSettings.jsx | 9 ++++++++- frontend/src/components/workflows/workflow/Workflows.jsx | 9 ++++++++- frontend/src/pages/ConnectorsPage.jsx | 9 ++++++++- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx index 88bc6631f1..e0346a8176 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -175,6 +175,10 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { }), ) .catch((err) => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { + return; + } setAlertDetails( handleException(err, "Failed to get the list of tools"), ); @@ -182,7 +186,10 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { setDisplayList((prev) => prev ?? []); }) .finally(() => { - setIsLoading(false); + // Only the newest request owns the shared loading state. + if (seq === seqRef.current) { + setIsLoading(false); + } }); }, [ diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 61f93afbea..b4fce3656c 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -158,12 +158,19 @@ function ToolSettings({ type }) { }), ) .catch((err) => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { + return; + } setAlertDetails(handleException(err)); // Avoid an indefinite spinner when the first fetch fails. setDisplayList((prev) => prev ?? []); }) .finally(() => { - setIsLoading(false); + // Only the newest request owns the shared loading state. + if (seq === seqRef.current) { + setIsLoading(false); + } }); }, [ diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index ad77433a5d..5be8c2275b 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -121,12 +121,19 @@ function Workflows() { }), ) .catch(() => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { + return; + } console.error("Unable to get project list"); // Avoid an indefinite spinner when the first fetch fails. setProjectList((prev) => prev ?? []); }) .finally(() => { - setLoading(false); + // Only the newest request owns the shared loading state. + if (seq === seqRef.current) { + setLoading(false); + } }); }; fetchRef.current = getProjectList; diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index ff17f99970..530a3e3dc6 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -124,12 +124,19 @@ function ConnectorsPage() { }), ) .catch((err) => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { + return; + } setAlertDetails(handleException(err, "Failed to load connectors")); // Avoid an indefinite spinner when the first fetch fails. setDisplayList((prev) => prev ?? []); }) .finally(() => { - setLoading(false); + // Only the newest request owns the shared loading state. + if (seq === seqRef.current) { + setLoading(false); + } }); }, [axiosPrivate, getUrl, setPagination, setAlertDetails, handleException], From c435b06cf5807d3c07ddb172bc64f47d54c95c4c Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Thu, 23 Jul 2026 20:23:01 +0530 Subject: [PATCH 05/11] UN-3769 [FIX] Show a retryable error on list-fetch failure; use codePointAt - On fetch failure the list pages set displayList to [], so a failed initial load rendered a misleading "No X available" empty state. Track an explicit loadError instead and render a retryable error (Retry refetches the current page), so a failure is no longer shown as an empty success. - colorForSeed uses String#codePointAt over charCodeAt (SonarCloud S7758). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../custom-tools/list-of-tools/ListOfTools.jsx | 16 +++++++++++++--- .../tool-settings/tool-settings/ToolSettings.jsx | 16 +++++++++++++--- .../widgets/resource-table/ResourceTable.jsx | 2 +- .../components/workflows/workflow/Workflows.jsx | 16 +++++++++++++--- frontend/src/pages/ConnectorsPage.jsx | 16 +++++++++++++--- 5 files changed, 53 insertions(+), 13 deletions(-) diff --git a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx index e0346a8176..bcac14c568 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -75,6 +75,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { // undefined = not fetched yet (spinner); [] = fetched-empty (empty state) const [displayList, setDisplayList] = useState(); + // Fetch failure (vs. genuinely empty) — drives a retryable error state. + const [loadError, setLoadError] = useState(false); const [isEdit, setIsEdit] = useState(false); const [promptDetails, setPromptDetails] = useState(null); const [openSharePermissionModal, setOpenSharePermissionModal] = @@ -154,6 +156,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { order, }); const seq = ++seqRef.current; + setLoadError(false); setIsLoading(true); return axiosPrivate({ method: "GET", @@ -182,8 +185,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { setAlertDetails( handleException(err, "Failed to get the list of tools"), ); - // Avoid an indefinite spinner when the first fetch fails. - setDisplayList((prev) => prev ?? []); + // Surface a retryable error instead of a misleading empty state. + setLoadError(true); }) .finally(() => { // Only the newest request owns the shared loading state. @@ -454,7 +457,14 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) {
- {displayList === undefined && } + {displayList === undefined && !loadError && } + {displayList === undefined && loadError && ( + + )} {displayList?.length === 0 && !searchTerm && ( prev ?? []); + // Surface a retryable error instead of a misleading empty state. + setLoadError(true); }) .finally(() => { // Only the newest request owns the shared loading state. @@ -361,7 +364,14 @@ function ToolSettings({ type }) {
- {displayList === undefined && } + {displayList === undefined && !loadError && } + {displayList === undefined && loadError && ( + + )} {displayList?.length === 0 && !searchTerm && ( { let hash = 0; for (let i = 0; i < seed.length; i += 1) { - hash = seed.charCodeAt(i) + ((hash << 5) - hash); + hash = seed.codePointAt(i) + ((hash << 5) - hash); } return AVATAR_COLORS[Math.abs(hash) % AVATAR_COLORS.length]; }; diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 5be8c2275b..9fda83a3ac 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -54,6 +54,8 @@ function Workflows() { ); const [projectList, setProjectList] = useState(); + // Fetch failure (vs. genuinely empty) — drives a retryable error state. + const [loadError, setLoadError] = useState(false); const [editingProject, setEditProject] = useState(); const [loading, setLoading] = useState(false); const [openModal, toggleModal] = useState(true); @@ -104,6 +106,7 @@ function Workflows() { ) => { const params = buildPagedParams({ page, pageSize, search, sortBy, order }); const seq = ++seqRef.current; + setLoadError(false); setLoading(true); return projectApiService .getProjectList(params) @@ -126,8 +129,8 @@ function Workflows() { return; } console.error("Unable to get project list"); - // Avoid an indefinite spinner when the first fetch fails. - setProjectList((prev) => prev ?? []); + // Surface a retryable error instead of a misleading empty state. + setLoadError(true); }) .finally(() => { // Only the newest request owns the shared loading state. @@ -391,7 +394,14 @@ function Workflows() { />
- {projectList === undefined && } + {projectList === undefined && !loadError && } + {projectList === undefined && loadError && ( + + )} {projectList?.length === 0 && !searchTerm && (
prev ?? []); + // Surface a retryable error instead of a misleading empty state. + setLoadError(true); }) .finally(() => { // Only the newest request owns the shared loading state. @@ -297,7 +300,14 @@ function ConnectorsPage() { />
- {displayList === undefined && } + {displayList === undefined && !loadError && } + {displayList === undefined && loadError && ( + + )} {displayList?.length === 0 && !searchTerm && ( Date: Thu, 23 Jul 2026 21:41:32 +0530 Subject: [PATCH 06/11] UN-3769 [FIX] Track "Me" in the Owned By cell by displayed owner, not membership is_owner is true for any OWNER membership, so a co-owner viewing a resource they didn't create saw "Me" over the primary owner's avatar/email. Key the "Me" label on the displayed owner email instead; the creator viewing their own resource still reads "Me" via the email match. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/components/widgets/resource-table/ResourceTable.jsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 1d96576312..715c262cb5 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -184,7 +184,10 @@ function ResourceTable({ const renderOwner = (item) => { const email = item?.[ownerEmailProp]; - const isMe = item?.is_owner || (email && email === sessionDetails?.email); + // "Me" must track the DISPLAYED owner, not the viewer's own membership — + // else a co-owner sees "Me" over the primary owner's avatar/email. Match on + // the shown email so the creator viewing their own resource still reads "Me". + const isMe = Boolean(email) && email === sessionDetails?.email; const name = isMe ? "Me" : email?.split("@")[0] || "Unknown"; const extra = item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; From db2415152c9055af46bcf1c6fcbbf5f0fb4180b0 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Thu, 23 Jul 2026 21:47:57 +0530 Subject: [PATCH 07/11] UN-3769 [FIX] Gate delete-failure loading clear on the request sequence The adapter delete catch cleared isLoading unconditionally, so a failed delete could hide the spinner for a newer in-flight fetch (search/sort/paginate/refresh) and expose obsolete results. Snapshot the request token at delete start and clear loading only if no newer fetch has taken it over. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tool-settings/tool-settings/ToolSettings.jsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 254e5bd14a..69e818c717 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -202,6 +202,10 @@ function ToolSettings({ type }) { const addNewItem = () => handleListRefresh(); const handleDelete = (_event, adapter) => { + // Snapshot the request token: a fetch (search/sort/paginate/refresh) that + // starts while this delete is in flight bumps it, so we don't clear its + // loading on a failed delete. + const seq = seqRef.current; setIsLoading(true); axiosPrivate({ method: "DELETE", @@ -211,9 +215,11 @@ function ToolSettings({ type }) { .then(() => handleListRefresh()) .catch((err) => { setAlertDetails(handleException(err)); - // Refresh only runs on success; clear loading here so a failed delete - // doesn't leave the table stuck under its spinner. - setIsLoading(false); + // Refresh owns loading on success; on failure clear it only if no newer + // fetch has taken over (else that request still owns the spinner). + if (seq === seqRef.current) { + setIsLoading(false); + } }); }; From 52d9d11734100ce5e6eb520a4fb8d058c4a0bd89 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Thu, 23 Jul 2026 21:54:41 +0530 Subject: [PATCH 08/11] UN-3769 [FIX] Keep adapter delete out of the shared list-loading state ToolSettings was the only list driving the shared isLoading from a row delete, which produced a string of overlap races (stuck loading, clobbering a newer fetch, concurrent deletes clearing each other). Drop loading from the delete entirely, matching the other four lists: success refetches via handleListRefresh (which owns the spinner), failure just toasts. Removes the race class by construction rather than adding another guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tool-settings/ToolSettings.jsx | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 69e818c717..d0082f269a 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -202,25 +202,16 @@ function ToolSettings({ type }) { const addNewItem = () => handleListRefresh(); const handleDelete = (_event, adapter) => { - // Snapshot the request token: a fetch (search/sort/paginate/refresh) that - // starts while this delete is in flight bumps it, so we don't clear its - // loading on a failed delete. - const seq = seqRef.current; - setIsLoading(true); + // Don't drive the shared list-loading from a row delete (as the other lists + // avoid): success refetches via handleListRefresh, which owns the spinner; + // failure just surfaces a toast. Keeps deletes out of the loading races. axiosPrivate({ method: "DELETE", url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/${adapter?.id}/`, headers: { "X-CSRFToken": sessionDetails?.csrfToken }, }) .then(() => handleListRefresh()) - .catch((err) => { - setAlertDetails(handleException(err)); - // Refresh owns loading on success; on failure clear it only if no newer - // fetch has taken over (else that request still owns the spinner). - if (seq === seqRef.current) { - setIsLoading(false); - } - }); + .catch((err) => setAlertDetails(handleException(err))); }; const handleEdit = (_event, item) => { From 5947bcc64fffcf3e616875ffc3e45f8130804f60 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Thu, 23 Jul 2026 22:02:04 +0530 Subject: [PATCH 09/11] UN-3769 [FIX] Refresh the current list view, not the params captured earlier handleListRefresh closed over pagination/search/sort, so a refresh captured in a pending mutation's .then (e.g. a delete) would refetch the stale page/search/order and overwrite the view the user had since navigated to. Make it a stable callback that reads the latest params from a ref, so post-mutation refresh always targets the current view. Fixes it for every list's create/edit/import/delete. Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/src/hooks/usePaginatedList.js | 30 +++++++++++--------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/frontend/src/hooks/usePaginatedList.js b/frontend/src/hooks/usePaginatedList.js index 9ffa25da1e..7f1ad8cf9b 100644 --- a/frontend/src/hooks/usePaginatedList.js +++ b/frontend/src/hooks/usePaginatedList.js @@ -103,6 +103,12 @@ function usePaginatedList({ // Page assigns its fetch fn here; handlers call the latest one via the ref. const fetchRef = useRef(null); + // Mirror the latest list params so handleListRefresh always refetches the + // CURRENT view, even when captured earlier (e.g. a pending mutation's .then) + // before the user changed page/search/sort. + const listStateRef = useRef(); + listStateRef.current = { pagination, searchTerm, sort }; + const handlePaginationChange = (page, pageSize) => { const newPage = pageSize === pagination.pageSize ? page : 1; fetchRef.current?.(newPage, pageSize, searchTerm, sort.sortBy, sort.order); @@ -128,23 +134,13 @@ function usePaginatedList({ ); }; - const handleListRefresh = useCallback( - () => - fetchRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ), - [ - pagination.current, - pagination.pageSize, - searchTerm, - sort.sortBy, - sort.order, - ], - ); + // Stable identity, reads the latest params from the ref: a refresh captured + // before the user navigated still fetches the view they're on now instead of + // restoring the stale page/search/order. + const handleListRefresh = useCallback(() => { + const { pagination: p, searchTerm: term, sort: s } = listStateRef.current; + fetchRef.current?.(p.current, p.pageSize, term, s.sortBy, s.order); + }, []); return { pagination, From 83a460b1d165bd3c3d1341a0e68d60ef18f71e99 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Fri, 24 Jul 2026 07:50:06 +0530 Subject: [PATCH 10/11] UN-3769 [FIX] Fix list fetch state handling and dead pagination Pipelines and API deployments still passed the removed `fetchData` option, so the hook's `fetchRef` stayed null and their pagination and search were silent no-ops. Both now assign `fetchRef` directly and drop their local `fetchListRef`. Route every fetch (navigation, last-page stepback, adapter-type reset) through `requestList`, so the recorded request params always match what lands on screen and a post-mutation refresh replays the view the user actually asked for. Realign those params with the displayed view when the newest request fails, so a failed navigation can't leave a later refresh jumping to a page that never loaded. Give the workflow edit modal its own loading flag so saving no longer drives the shared list spinner. Co-Authored-By: Claude Opus 4.8 --- .../list-of-tools/ListOfTools.jsx | 6 +- .../api-deployment/ApiDeployment.jsx | 15 ++-- .../pipelines/Pipelines.jsx | 15 ++-- .../tool-settings/ToolSettings.jsx | 14 ++-- .../workflows/workflow/Workflows.jsx | 18 +++-- frontend/src/hooks/usePaginatedList.js | 70 +++++++++++++++---- frontend/src/pages/ConnectorsPage.jsx | 6 +- 7 files changed, 103 insertions(+), 41 deletions(-) diff --git a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx index bcac14c568..7e3025f4af 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -128,6 +128,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { setSearchTerm, sort, fetchRef, + requestList, + syncRequested, handlePaginationChange, handleSearch, handleSortChange, @@ -174,7 +176,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { setList: setDisplayList, setPagination, refetchPrevPage: () => - getListOfTools(page - 1, pageSize, search, sortBy, order), + requestList(page - 1, pageSize, search, sortBy, order), }), ) .catch((err) => { @@ -187,6 +189,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { ); // Surface a retryable error instead of a misleading empty state. setLoadError(true); + // Failed request — realign requestedRef with the still-shown view. + syncRequested(); }) .finally(() => { // Only the newest request owns the shared loading state. diff --git a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx index 29f8f17612..2bd277721a 100644 --- a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx +++ b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import { useLocation } from "react-router-dom"; import { deploymentApiTypes, displayURL } from "../../../helpers/GetStaticData"; @@ -71,26 +71,23 @@ function ApiDeployment() { const { count, isLoading, fetchCount } = usePromptStudioStore(); const { getPromptStudioCount } = usePromptStudioService(); - // Ref to forward the fetch function to hooks (avoids declaration ordering) - const fetchListRef = useRef(null); - const { pagination, setPagination, searchTerm, setSearchTerm, + // The hook owns the fetch ref; assigned below (avoids declaration ordering). + fetchRef, handlePaginationChange, handleSearch, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), - }); + } = usePaginatedList(); const { scrollRestoreId, activateScrollRestore, clearPendingScroll } = useScrollRestoration({ location, setSearchTerm, setPagination, - fetchData: (...args) => fetchListRef.current?.(...args), + fetchData: (...args) => fetchRef.current?.(...args), }); const { @@ -182,7 +179,7 @@ function ApiDeployment() { }); }; - fetchListRef.current = getApiDeploymentList; + fetchRef.current = getApiDeploymentList; const deleteApiDeployment = (item) => { const id = item?.id || selectedRow.id; diff --git a/frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx b/frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx index 32fdd4ae5b..f2688b6ea5 100644 --- a/frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx +++ b/frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx @@ -1,5 +1,5 @@ import PropTypes from "prop-types"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import { useLocation } from "react-router-dom"; import { @@ -75,26 +75,23 @@ function Pipelines({ type }) { const { count, isLoading, fetchCount } = usePromptStudioStore(); const { getPromptStudioCount } = usePromptStudioService(); - // Ref to forward the fetch function to hooks (avoids declaration ordering) - const fetchListRef = useRef(null); - const { pagination, setPagination, searchTerm, setSearchTerm, + // The hook owns the fetch ref; assigned below (avoids declaration ordering). + fetchRef, handlePaginationChange, handleSearch, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), - }); + } = usePaginatedList(); const { scrollRestoreId, activateScrollRestore, clearPendingScroll } = useScrollRestoration({ location, setSearchTerm, setPagination, - fetchData: (...args) => fetchListRef.current?.(...args), + fetchData: (...args) => fetchRef.current?.(...args), }); const { @@ -184,7 +181,7 @@ function Pipelines({ type }) { }); }; - fetchListRef.current = getPipelineList; + fetchRef.current = getPipelineList; const handleSync = (params) => { const body = { ...params, pipeline_type: type.toUpperCase() }; diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index d0082f269a..4f550b013e 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -104,9 +104,11 @@ function ToolSettings({ type }) { pagination, setPagination, searchTerm, - setSearchTerm, sort, fetchRef, + requestList, + resetList, + syncRequested, handlePaginationChange, handleSearch, handleSortChange, @@ -157,7 +159,7 @@ function ToolSettings({ type }) { setList: setDisplayList, setPagination, refetchPrevPage: () => - getAdapters(page - 1, pageSize, search, sortBy, order), + requestList(page - 1, pageSize, search, sortBy, order), }), ) .catch((err) => { @@ -168,6 +170,8 @@ function ToolSettings({ type }) { setAlertDetails(handleException(err)); // Surface a retryable error instead of a misleading empty state. setLoadError(true); + // Failed request — realign requestedRef with the still-shown view. + syncRequested(); }) .finally(() => { // Only the newest request owns the shared loading state. @@ -188,12 +192,14 @@ function ToolSettings({ type }) { fetchRef.current = getAdapters; useEffect(() => { - setSearchTerm(""); setDisplayList(undefined); if (!type) { return; } - getAdapters(1, DEFAULT_PAGE_SIZE, "", "", "asc"); + // Persistent instance across adapter types: reset search/sort/requestedRef + // together so the new type starts clean and later refreshes don't replay + // the previous type's view. + resetList(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [type]); diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 9fda83a3ac..43a1ccfe38 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -58,6 +58,9 @@ function Workflows() { const [loadError, setLoadError] = useState(false); const [editingProject, setEditProject] = useState(); const [loading, setLoading] = useState(false); + // Modal-local save spinner — kept off the shared list-loading so an edit can't + // race the post-edit refetch for the list's loading state. + const [editLoading, setEditLoading] = useState(false); const [openModal, toggleModal] = useState(true); // Monotonic request token so a stale response can't overwrite a newer one. const seqRef = useRef(0); @@ -77,6 +80,8 @@ function Workflows() { searchTerm, sort, fetchRef, + requestList, + syncRequested, handlePaginationChange, handleSearch, handleSortChange, @@ -120,7 +125,7 @@ function Workflows() { setList: setProjectList, setPagination, refetchPrevPage: () => - getProjectList(page - 1, pageSize, search, sortBy, order), + requestList(page - 1, pageSize, search, sortBy, order), }), ) .catch(() => { @@ -131,6 +136,8 @@ function Workflows() { console.error("Unable to get project list"); // Surface a retryable error instead of a misleading empty state. setLoadError(true); + // Failed request — realign requestedRef with the still-shown view. + syncRequested(); }) .finally(() => { // Only the newest request owns the shared loading state. @@ -142,7 +149,10 @@ function Workflows() { fetchRef.current = getProjectList; function editProject(name, description) { - setLoading(true); + // Drive the modal-local editLoading, not the shared list-loading: on success + // the edit path's handleListRefresh owns the list spinner (new path navigates + // away), so editProject can't clear a pending refetch's loading. + setEditLoading(true); projectApiService .editProject(name, description, editingProject?.id) .then((res) => { @@ -166,7 +176,7 @@ function Workflows() { ); }) .finally(() => { - setLoading(false); + setEditLoading(false); }); } @@ -448,7 +458,7 @@ function Workflows() { description={editingProject.description} onDone={editProject} onClose={closeNewProject} - loading={loading} + loading={editLoading} toggleModal={toggleModal} openModal={openModal} backendErrors={backendErrors} diff --git a/frontend/src/hooks/usePaginatedList.js b/frontend/src/hooks/usePaginatedList.js index 7f1ad8cf9b..2c7d778b87 100644 --- a/frontend/src/hooks/usePaginatedList.js +++ b/frontend/src/hooks/usePaginatedList.js @@ -103,21 +103,47 @@ function usePaginatedList({ // Page assigns its fetch fn here; handlers call the latest one via the ref. const fetchRef = useRef(null); - // Mirror the latest list params so handleListRefresh always refetches the - // CURRENT view, even when captured earlier (e.g. a pending mutation's .then) - // before the user changed page/search/sort. - const listStateRef = useRef(); - listStateRef.current = { pagination, searchTerm, sort }; + // Mirrors the on-screen view each render (pagination only advances when a + // response applies), so a FAILED request can realign requestedRef with what's + // actually displayed. See syncRequested. + const appliedRef = useRef(null); + appliedRef.current = { + page: pagination.current, + pageSize: pagination.pageSize, + search: searchTerm, + sortBy: sort.sortBy, + order: sort.order, + }; + + // Params of the LAST request, recorded at fetch time (not read from state, + // which lags — pagination only updates when a response applies). handleList- + // Refresh replays these, so a refresh captured during an in-flight navigation + // still targets the view the user asked for rather than snapping back. + const requestedRef = useRef({ + page: 1, + pageSize: defaultPageSize, + search: "", + sortBy: defaultSortBy, + order: defaultOrder, + }); + + // Single fetch entry: records the requested view before firing, so every + // path (navigation, stepback, reset) keeps requestedRef in sync with what + // ends up on screen. Pages route their stepback (refetchPrevPage) through it. + const requestList = (page, pageSize, search, sortBy, order) => { + requestedRef.current = { page, pageSize, search, sortBy, order }; + fetchRef.current?.(page, pageSize, search, sortBy, order); + }; const handlePaginationChange = (page, pageSize) => { const newPage = pageSize === pagination.pageSize ? page : 1; - fetchRef.current?.(newPage, pageSize, searchTerm, sort.sortBy, sort.order); + requestList(newPage, pageSize, searchTerm, sort.sortBy, sort.order); }; const handleSearch = (searchText) => { const term = searchText?.trim() || ""; setSearchTerm(term); - fetchRef.current?.(1, pagination.pageSize, term, sort.sortBy, sort.order); + requestList(1, pagination.pageSize, term, sort.sortBy, sort.order); }; // Table header sort click: reset to page 1 (the page a row sits on changes) @@ -125,7 +151,7 @@ function usePaginatedList({ const handleSortChange = (sortBy, order) => { const nextSort = { sortBy: sortBy || "", order: order || "asc" }; setSort(nextSort); - fetchRef.current?.( + requestList( 1, pagination.pageSize, searchTerm, @@ -134,12 +160,27 @@ function usePaginatedList({ ); }; - // Stable identity, reads the latest params from the ref: a refresh captured - // before the user navigated still fetches the view they're on now instead of - // restoring the stale page/search/order. + // Reset to a fresh default view (state + requestedRef) and fetch — used when + // a persistent instance changes what it lists (e.g. ToolSettings adapter type) + // so search box, sort header, and requestedRef all start clean. + const resetList = () => { + setSearchTerm(""); + setSort({ sortBy: defaultSortBy, order: defaultOrder }); + requestList(1, defaultPageSize, "", defaultSortBy, defaultOrder); + }; + + // Realign requestedRef with the displayed view — call from a page's catch when + // its newest request failed (setList was skipped, so the screen still shows the + // previous view). Stops the next handleListRefresh from replaying the failed + // target (e.g. jumping to a page whose fetch errored). + const syncRequested = () => { + requestedRef.current = { ...appliedRef.current }; + }; + + // Stable identity; replays the last requested params. const handleListRefresh = useCallback(() => { - const { pagination: p, searchTerm: term, sort: s } = listStateRef.current; - fetchRef.current?.(p.current, p.pageSize, term, s.sortBy, s.order); + const { page, pageSize, search, sortBy, order } = requestedRef.current; + fetchRef.current?.(page, pageSize, search, sortBy, order); }, []); return { @@ -149,6 +190,9 @@ function usePaginatedList({ setSearchTerm, sort, fetchRef, + requestList, + resetList, + syncRequested, handlePaginationChange, handleSearch, handleSortChange, diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 5c6ddc4306..55b7c51bda 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -81,6 +81,8 @@ function ConnectorsPage() { searchTerm, sort, fetchRef, + requestList, + syncRequested, handlePaginationChange, handleSearch, handleSortChange, @@ -123,7 +125,7 @@ function ConnectorsPage() { setList: setDisplayList, setPagination, refetchPrevPage: () => - getConnectors(page - 1, pageSize, search, sortBy, order), + requestList(page - 1, pageSize, search, sortBy, order), }), ) .catch((err) => { @@ -134,6 +136,8 @@ function ConnectorsPage() { setAlertDetails(handleException(err, "Failed to load connectors")); // Surface a retryable error instead of a misleading empty state. setLoadError(true); + // Failed request — realign requestedRef with the still-shown view. + syncRequested(); }) .finally(() => { // Only the newest request owns the shared loading state. From db133f5b5d0f58ac9b057bc65dcf91a6779f8f87 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Fri, 24 Jul 2026 13:29:13 +0530 Subject: [PATCH 11/11] UN-3769 [FIX] Address self-review on resource list views - Owned By names a live owner: owner_email() on HasMembersMixin + 4 list serializers, instead of created_by which can be a removed creator. - Retryable load error is reachable after the first load (gate loadError ahead of the length branches on all 4 pages). - Workflows list-fetch failure surfaces via handleException, not a bare console.error. - Correct usePaginatedList appliedRef comment; requestList returns the fetch promise so applyPagedResponse's documented stepback holds. - Fix stale prompt_count Subquery rationale comment. Co-Authored-By: Claude Opus 4.8 --- backend/adapter_processor_v2/serializers.py | 1 + backend/connector_v2/serializers.py | 1 + backend/permissions/models.py | 15 +++++++++++++++ .../prompt_studio_core_v2/serializers.py | 5 +++++ .../prompt_studio/prompt_studio_core_v2/views.py | 7 ++++--- .../workflow_manager/workflow_v2/serializers.py | 1 + .../custom-tools/list-of-tools/ListOfTools.jsx | 12 ++++++------ .../tool-settings/tool-settings/ToolSettings.jsx | 12 ++++++------ .../components/workflows/workflow/Workflows.jsx | 16 ++++++++-------- frontend/src/hooks/usePaginatedList.js | 12 ++++++++---- frontend/src/pages/ConnectorsPage.jsx | 12 ++++++------ 11 files changed, 61 insertions(+), 33 deletions(-) diff --git a/backend/adapter_processor_v2/serializers.py b/backend/adapter_processor_v2/serializers.py index a5f2c492d6..34aeccc5c1 100644 --- a/backend/adapter_processor_v2/serializers.py +++ b/backend/adapter_processor_v2/serializers.py @@ -215,6 +215,7 @@ def to_representation(self, instance: AdapterInstance) -> dict[str, str]: request = self.context.get("request") rep["is_owner"] = instance.is_owner(request.user) if request else False rep["co_owners_count"] = instance.co_owners_count() + rep["owner_email"] = instance.owner_email() return rep diff --git a/backend/connector_v2/serializers.py b/backend/connector_v2/serializers.py index 5c4c158333..7d15edb7f0 100644 --- a/backend/connector_v2/serializers.py +++ b/backend/connector_v2/serializers.py @@ -176,6 +176,7 @@ def to_representation(self, instance: ConnectorInstance) -> dict[str, str]: request = self.context.get("request") rep["is_owner"] = instance.is_owner(request.user) if request else False rep["co_owners_count"] = instance.co_owners_count() + rep["owner_email"] = instance.owner_email() return rep diff --git a/backend/permissions/models.py b/backend/permissions/models.py index 9c63804e16..d6c3eecc09 100644 --- a/backend/permissions/models.py +++ b/backend/permissions/models.py @@ -43,6 +43,21 @@ def co_owners_count(self) -> int: for m in self.memberships.all() # type: ignore[attr-defined] ) + def owner_email(self) -> str | None: + # Email for the "Owned By" label. ``created_by`` is audit-only (UN-2202) + # and the creator can be removed as owner, so it must not name the owner. + # Reads the prefetched ``memberships`` (list views set ``memberships__user``) + # to stay query-free; earliest live OWNER wins so the label names the same + # roster as ``co_owners_count()`` and is stable across page loads. + owners = [ + m + for m in self.memberships.all() # type: ignore[attr-defined] + if m.role == ResourceRole.OWNER and not m.user.is_service_account + ] + if not owners: + return None + return min(owners, key=lambda m: m.created_at).user.email + def is_owner(self, user: Any) -> bool: if user is None: return False diff --git a/backend/prompt_studio/prompt_studio_core_v2/serializers.py b/backend/prompt_studio/prompt_studio_core_v2/serializers.py index 245f2c0743..a09798b14a 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_core_v2/serializers.py @@ -48,6 +48,7 @@ class CustomToolListSerializer(serializers.ModelSerializer): prompt_count = serializers.SerializerMethodField() is_owner = serializers.SerializerMethodField() co_owners_count = serializers.SerializerMethodField() + owner_email = serializers.SerializerMethodField() class Meta: model = CustomTool @@ -65,6 +66,7 @@ class Meta: "prompt_count", "is_owner", "co_owners_count", + "owner_email", ] def get_created_by_email(self, instance): @@ -77,6 +79,9 @@ def get_is_owner(self, instance) -> bool: def get_co_owners_count(self, instance) -> int: return instance.co_owners_count() + def get_owner_email(self, instance) -> str | None: + return instance.owner_email() + def get_prompt_count(self, instance): if hasattr(instance, "_prompt_count"): return instance._prompt_count or 0 diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index f913ab261e..ea06e61dda 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -173,9 +173,10 @@ def get_queryset(self) -> QuerySet | None: select_related=("created_by",), prefetch_related=("memberships__user",), ) - # The pk__in re-wrap drops annotations, so re-apply prompt_count on - # the re-wrapped result. Subquery avoids conflict with the - # distinct("tool_id") that for_user() carries. + # apply_search_and_sort returns a fresh CustomTool.objects chain, so + # the prompt_count annotation goes on afterwards. Subquery keeps the + # count out of the outer GROUP BY, which would otherwise collide with + # the sort column. prompt_count_sq = ( ToolStudioPrompt.objects.filter(tool_id=OuterRef("pk")) .order_by() diff --git a/backend/workflow_manager/workflow_v2/serializers.py b/backend/workflow_manager/workflow_v2/serializers.py index c9715a9acb..c2f037570c 100644 --- a/backend/workflow_manager/workflow_v2/serializers.py +++ b/backend/workflow_manager/workflow_v2/serializers.py @@ -85,6 +85,7 @@ def to_representation(self, instance: Workflow) -> dict[str, str]: request = self.context.get("request") representation["is_owner"] = instance.is_owner(request.user) if request else False representation["co_owners_count"] = instance.co_owners_count() + representation["owner_email"] = instance.owner_email() return representation def create(self, validated_data: dict[str, Any]) -> Any: diff --git a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx index 7e3025f4af..0e07bc86bd 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -461,25 +461,25 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) {
- {displayList === undefined && !loadError && } - {displayList === undefined && loadError && ( + {loadError && ( )} - {displayList?.length === 0 && !searchTerm && ( + {!loadError && displayList === undefined && } + {!loadError && displayList?.length === 0 && !searchTerm && ( )} - {displayList?.length === 0 && searchTerm && ( + {!loadError && displayList?.length === 0 && searchTerm && ( )} - {displayList?.length > 0 && ( + {!loadError && displayList?.length > 0 && (
- {displayList === undefined && !loadError && } - {displayList === undefined && loadError && ( + {loadError && ( )} - {displayList?.length === 0 && !searchTerm && ( + {!loadError && displayList === undefined && } + {!loadError && displayList?.length === 0 && !searchTerm && ( )} - {displayList?.length === 0 && searchTerm && ( + {!loadError && displayList?.length === 0 && searchTerm && ( )} - {displayList?.length > 0 && ( + {!loadError && displayList?.length > 0 && ( { + .catch((err) => { // A newer request superseded this one — don't surface its error. if (seq !== seqRef.current) { return; } - console.error("Unable to get project list"); + setAlertDetails(handleException(err, "Unable to load workflows")); // Surface a retryable error instead of a misleading empty state. setLoadError(true); // Failed request — realign requestedRef with the still-shown view. @@ -404,15 +404,15 @@ function Workflows() { />
- {projectList === undefined && !loadError && } - {projectList === undefined && loadError && ( + {loadError && ( )} - {projectList?.length === 0 && !searchTerm && ( + {!loadError && projectList === undefined && } + {!loadError && projectList?.length === 0 && !searchTerm && (
)} - {projectList?.length === 0 && searchTerm && ( + {!loadError && projectList?.length === 0 && searchTerm && ( )} - {projectList?.length > 0 && ( + {!loadError && projectList?.length > 0 && ( { requestedRef.current = { page, pageSize, search, sortBy, order }; - fetchRef.current?.(page, pageSize, search, sortBy, order); + // Return the fetch promise so a stepback (refetchPrevPage) propagates up + // through applyPagedResponse, as its JSDoc documents. + return fetchRef.current?.(page, pageSize, search, sortBy, order); }; const handlePaginationChange = (page, pageSize) => { diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 55b7c51bda..3528c19866 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -304,25 +304,25 @@ function ConnectorsPage() { />
- {displayList === undefined && !loadError && } - {displayList === undefined && loadError && ( + {loadError && ( )} - {displayList?.length === 0 && !searchTerm && ( + {!loadError && displayList === undefined && } + {!loadError && displayList?.length === 0 && !searchTerm && ( )} - {displayList?.length === 0 && searchTerm && ( + {!loadError && displayList?.length === 0 && searchTerm && ( )} - {displayList?.length > 0 && ( + {!loadError && displayList?.length > 0 && (