From 45bf27904197f95628ebaa63ce54d98551db2285 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Fri, 24 Jul 2026 17:22:00 +0530 Subject: [PATCH 01/17] UN-3770 [FIX] Make list pagination consistent across shared resource endpoints #2187 added opt-in pagination to workflows, prompt studio, adapters and connectors but only wired the Workflows page. The other three could not follow: their `for_user()` managers used `DISTINCT ON`, which forces Postgres to order by the distinct expression, so those viewsets were pinned to `order_by("id")` and could not order by `modified_at`. Backend - Swap `.distinct("id")` / `.distinct("tool_id")` for plain `.distinct()` in the adapter, connector and prompt studio managers. Every arm of the sharing predicate is a PK subquery, not a join, so no duplicate rows exist to collapse and the swap is behaviour-preserving. Workflows has shipped this way since #1462. - Replace the per-view `order_by()` calls with a declarative `ordering = ["-modified_at", "pk"]`. `OrderingFilter` is already in `DEFAULT_FILTER_BACKENDS`, so this needs no `filter_backends` override (which would drop `OrganizationFilterBackend`). - Drop Workflow's `?order_by=asc|desc`; it has no consumer, and `?ordering=` now covers it through the standard filter. Frontend - Add `unwrapList` / `fetchAllPages` helpers. Selectors page to exhaustion rather than silently showing only the first 50 rows. - Route all adapter, connector and workflow selectors through them. - Convert the Prompt Studio, adapters and connectors pages to server-side pagination and search via `usePaginatedList`, replacing the client-side `useListSearch` filter (now deleted). - Move `` into `ViewTools` so all four pages share one implementation: page size 10, size changer on, `["10","20","50"]`. Workflows had the changer disabled; that inconsistency goes away. - Stop assigning `fetchListRef.current` during render in Workflows. Endpoints stay opt-in paginated here, so every change is safe against both response shapes. Flipping them to unconditional `CustomPagination` is a separate change, after this has been validated on staging. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- backend/adapter_processor_v2/models.py | 2 +- backend/adapter_processor_v2/views.py | 8 +- backend/connector_v2/models.py | 2 +- backend/connector_v2/views.py | 8 +- .../prompt_studio_core_v2/models.py | 2 +- .../prompt_studio_core_v2/views.py | 10 +- backend/utils/tests/test_list_pagination.py | 186 ++++++++++++++++++ backend/workflow_manager/workflow_v2/views.py | 11 +- .../ConfigureConnectorModal.jsx | 15 +- .../AdapterSelectionModal.jsx | 13 +- .../add-llm-profile/AddLlmProfile.jsx | 14 +- .../combined-output/CombinedOutput.jsx | 4 +- .../list-of-tools/ListOfTools.jsx | 127 ++++++------ .../custom-tools/view-tools/ViewTools.css | 4 + .../custom-tools/view-tools/ViewTools.jsx | 62 ++++-- .../helpers/custom-tools/CustomToolsHelper.js | 14 +- .../etl-task-deploy/EtlTaskDeploy.jsx | 4 +- .../settings/default-triad/DefaultTriad.jsx | 12 +- .../tool-settings/ToolSettings.jsx | 122 +++++++----- .../workflows/workflow/Workflows.css | 6 - .../workflows/workflow/Workflows.jsx | 63 +++--- .../workflows/workflow/workflow-service.js | 14 +- frontend/src/helpers/pagination.js | 62 ++++++ frontend/src/helpers/pagination.test.js | 54 +++++ frontend/src/hooks/useListSearch.js | 61 ------ frontend/src/pages/ConnectorsPage.jsx | 79 ++++++-- 26 files changed, 650 insertions(+), 309 deletions(-) create mode 100644 backend/utils/tests/test_list_pagination.py create mode 100644 frontend/src/helpers/pagination.js create mode 100644 frontend/src/helpers/pagination.test.js delete mode 100644 frontend/src/hooks/useListSearch.js diff --git a/backend/adapter_processor_v2/models.py b/backend/adapter_processor_v2/models.py index adba66dbdb..f160e67538 100644 --- a/backend/adapter_processor_v2/models.py +++ b/backend/adapter_processor_v2/models.py @@ -59,7 +59,7 @@ def for_user(self, user: User) -> QuerySet[Any]: | models.Q(is_friction_less=True) | models.Q(pk__in=group_shared_ids) ) - .distinct("id") + .distinct() ) diff --git a/backend/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index 7dfacf4434..5222f751e9 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -148,6 +148,9 @@ class AdapterInstanceViewSet( ): serializer_class = AdapterInstanceSerializer pagination_class = OptionalPagination + # `pk` tiebreaker keeps paging deterministic when modified_at collides. + ordering = ["-modified_at", "pk"] + ordering_fields = ["adapter_name", "created_at", "modified_at"] notification_resource_name_field = "adapter_name" def get_notification_resource_type(self, resource: Any) -> str | None: @@ -194,10 +197,7 @@ def get_queryset(self) -> QuerySet | None: 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") + return queryset def get_serializer_class( self, diff --git a/backend/connector_v2/models.py b/backend/connector_v2/models.py index 499374d7ad..57ae5933c3 100644 --- a/backend/connector_v2/models.py +++ b/backend/connector_v2/models.py @@ -51,7 +51,7 @@ def for_user(self, user: User) -> models.QuerySet: | models.Q(shared_to_org=True) | models.Q(pk__in=group_shared_ids) ) - .distinct("id") + .distinct() ) diff --git a/backend/connector_v2/views.py b/backend/connector_v2/views.py index c4a3741f25..84f5ef7005 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -47,6 +47,9 @@ class ConnectorInstanceViewSet( versioning_class = URLPathVersioning serializer_class = ConnectorInstanceSerializer pagination_class = OptionalPagination + # `pk` tiebreaker keeps paging deterministic when modified_at collides. + ordering = ["-modified_at", "pk"] + ordering_fields = ["connector_name", "created_at", "modified_at"] notification_resource_name_field = "connector_name" def get_notification_resource_type(self, resource: Any) -> str | None: @@ -127,10 +130,7 @@ 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") + return queryset 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/models.py b/backend/prompt_studio/prompt_studio_core_v2/models.py index f09a468546..031d61b1e7 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/models.py +++ b/backend/prompt_studio/prompt_studio_core_v2/models.py @@ -47,7 +47,7 @@ def for_user(self, user: User) -> QuerySet[Any]: | models.Q(shared_to_org=True) | models.Q(pk__in=group_shared_ids) ) - .distinct("tool_id") + .distinct() ) diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index ae11da451a..71e15085ae 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -133,6 +133,9 @@ class PromptStudioCoreView( versioning_class = URLPathVersioning pagination_class = OptionalPagination + # `pk` tiebreaker keeps paging deterministic when modified_at collides. + ordering = ["-modified_at", "pk"] + ordering_fields = ["tool_name", "created_at", "modified_at"] serializer_class = CustomToolSerializer notification_resource_name_field = "tool_name" @@ -161,7 +164,8 @@ def get_queryset(self) -> QuerySet | None: "memberships__user" ) if self.action == "list": - # Subquery avoids conflict with distinct("tool_id") from for_user() + # Subquery rather than a join-aggregate: Count() over a join would + # need a GROUP BY that fights the queryset's distinct() prompt_count_sq = ( ToolStudioPrompt.objects.filter(tool_id=OuterRef("pk")) .order_by() @@ -180,9 +184,7 @@ def get_queryset(self) -> QuerySet | None: 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.order_by("tool_id") + return qs def get_object(self): """Override get_object to trigger lazy migration when accessing tools.""" diff --git a/backend/utils/tests/test_list_pagination.py b/backend/utils/tests/test_list_pagination.py new file mode 100644 index 0000000000..ca24f6f889 --- /dev/null +++ b/backend/utils/tests/test_list_pagination.py @@ -0,0 +1,186 @@ +"""Pagination contract for the four shared list endpoints (UN-3770). + +Workflows, Prompt Studio, adapters and connectors share one listing shape: +``for_user()`` sharing predicate -> ``.distinct()`` -> declarative ``ordering`` +-> ``OptionalPagination``. These tests pin the three ways that combination can +silently serve wrong rows — non-deterministic page boundaries, duplicate rows +from the sharing predicate, and a search applied after the count. + +DB-backed (Django ``TestCase``), so ``backend/conftest.py`` auto-marks these +``integration``. +""" + +from __future__ import annotations + +import json +from collections.abc import Callable +from functools import lru_cache +from typing import Any, NamedTuple + +from adapter_processor_v2.views import AdapterInstanceViewSet +from connector_v2.views import ConnectorInstanceViewSet +from django.test import TestCase +from permissions.roles import ResourceRole +from permissions.tests.base import ( + CoOwnerOrgTestMixin, + _build_adapter, + _build_connector, + _build_custom_tool, + _build_workflow, +) +from prompt_studio.prompt_studio_core_v2.views import PromptStudioCoreView +from rest_framework import status +from rest_framework.test import APIRequestFactory, force_authenticate +from workflow_manager.workflow_v2.views import WorkflowViewSet + + +@lru_cache(maxsize=1) +def _live_llm_adapter_id() -> str: + from unstract.sdk1.adapters.adapterkit import Adapterkit + + return next( + a["id"] for a in Adapterkit().get_adapters_list() if a["adapter_type"] == "LLM" + ) + + +@lru_cache(maxsize=1) +def _live_connector_id() -> str: + from unstract.connectors.connectorkit import Connectorkit + + return Connectorkit().get_connectors_list()[0]["id"] + + +def _prepare_adapter(obj: Any) -> None: + """Make the row serialisable: real registry id, real encrypted metadata.""" + from cryptography.fernet import Fernet + from django.conf import settings + + obj.adapter_id = _live_llm_adapter_id() + fernet = Fernet(settings.ENCRYPTION_KEY.encode("utf-8")) + obj.adapter_metadata_b = fernet.encrypt( + json.dumps({"model": "gpt-4o-mini"}).encode("utf-8") + ) + + +def _prepare_connector(obj: Any) -> None: + obj.connector_id = _live_connector_id() + + +class ListEndpoint(NamedTuple): + kind: str + viewset: Any + build: Any + name_field: str + # Adapter and connector rows are serialised against the live registry, so + # the shared builders' placeholder ids error before paging is reached. + prepare: Callable[[Any], None] | None = None + + +LIST_ENDPOINTS = [ + ListEndpoint("workflow", WorkflowViewSet, _build_workflow, "workflow_name"), + ListEndpoint("custom_tool", PromptStudioCoreView, _build_custom_tool, "tool_name"), + ListEndpoint( + "adapter", + AdapterInstanceViewSet, + _build_adapter, + "adapter_name", + prepare=_prepare_adapter, + ), + ListEndpoint( + "connector", + ConnectorInstanceViewSet, + _build_connector, + "connector_name", + prepare=_prepare_connector, + ), +] + + +class ListPaginationContractTests(CoOwnerOrgTestMixin, TestCase): + def setUp(self) -> None: + self._seed_org() + self.factory = APIRequestFactory() + + def _create(self, endpoint: ListEndpoint, name: str, owner=None) -> Any: + """A resource named ``name``, owned (membership) by ``owner``.""" + owner = owner or self.owner + obj = endpoint.build(self.org, owner) + setattr(obj, endpoint.name_field, name) + if endpoint.prepare: + endpoint.prepare(obj) + obj.save() + obj.memberships.create(user=owner, role=ResourceRole.OWNER) + return obj + + def _list(self, endpoint: ListEndpoint, user, **params: Any): + view = endpoint.viewset.as_view({"get": "list"}) + request = self.factory.get("/x/", params) + force_authenticate(request, user=user) + response = view(request) + assert response.status_code == status.HTTP_200_OK, response.data + return response + + def _names(self, endpoint: ListEndpoint, rows) -> list[str]: + return [row[endpoint.name_field] for row in rows] + + def test_pages_partition_the_result_set(self) -> None: + """Page 2 must not repeat or drop rows from page 1. + + Without a deterministic ``ordering`` the two requests run independent + queries, so rows can appear twice or vanish entirely between them. + """ + for endpoint in LIST_ENDPOINTS: + with self.subTest(kind=endpoint.kind): + expected = {f"{endpoint.kind}-page-{i}" for i in range(5)} + for name in sorted(expected): + self._create(endpoint, name) + + page1 = self._list(endpoint, self.owner, page=1, page_size=2) + page2 = self._list(endpoint, self.owner, page=2, page_size=2) + page3 = self._list(endpoint, self.owner, page=3, page_size=2) + + names1 = self._names(endpoint, page1.data["results"]) + names2 = self._names(endpoint, page2.data["results"]) + names3 = self._names(endpoint, page3.data["results"]) + + assert page1.data["count"] == len(expected) + assert not set(names1) & set(names2) + assert set(names1) | set(names2) | set(names3) == expected + + def test_multi_predicate_share_yields_one_row(self) -> None: + """A resource reachable by several sharing predicates lists once. + + ``for_user()`` ORs membership / org-share / group-share together; if any + arm ever becomes a join instead of a PK subquery, the row duplicates and + ``count`` overstates. Dedup is what ``.distinct()`` is there for. + """ + for endpoint in LIST_ENDPOINTS: + with self.subTest(kind=endpoint.kind): + name = f"{endpoint.kind}-multi-share" + obj = self._create(endpoint, name) + # Reachable via membership AND org-share simultaneously. + obj.memberships.create(user=self.viewer, role=ResourceRole.VIEWER) + obj.shared_to_org = True + obj.save() + + response = self._list(endpoint, self.viewer, page=1, page_size=10) + + assert self._names(endpoint, response.data["results"]).count(name) == 1 + assert response.data["count"] == 1 + + def test_search_narrows_rows_and_count(self) -> None: + """``?search=`` must filter before the count, not after paging.""" + for endpoint in LIST_ENDPOINTS: + with self.subTest(kind=endpoint.kind): + for i in range(3): + self._create(endpoint, f"{endpoint.kind}-alpha-{i}") + for i in range(2): + self._create(endpoint, f"{endpoint.kind}-beta-{i}") + + response = self._list( + endpoint, self.owner, page=1, page_size=10, search="alpha" + ) + + names = self._names(endpoint, response.data["results"]) + assert response.data["count"] == 3 + assert all("alpha" in name for name in names) diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index ef9f52f95f..74678a933b 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -77,6 +77,9 @@ class WorkflowViewSet( ): versioning_class = URLPathVersioning pagination_class = OptionalPagination + # `pk` tiebreaker keeps paging deterministic when modified_at collides. + ordering = ["-modified_at", "pk"] + ordering_fields = ["workflow_name", "created_at", "modified_at"] notification_resource_name_field = "workflow_name" def get_notification_resource_type(self, resource: Any) -> str | None: @@ -119,14 +122,6 @@ def get_queryset(self) -> QuerySet: 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 def get_serializer_class(self) -> serializers.Serializer: diff --git a/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx b/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx index 026c9919cb..f982e37221 100644 --- a/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx +++ b/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx @@ -14,6 +14,7 @@ import { cloneDeep, isEqual } from "lodash"; import PropTypes from "prop-types"; import { useCallback, useEffect, useRef, useState } from "react"; +import { fetchAllPages } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; import usePostHogEvents from "../../../hooks/usePostHogEvents"; @@ -141,15 +142,11 @@ function ConfigureConnectorModal({ setIsLoadingConnectors(true); - const requestOptions = { - method: "GET", - url: getUrl(`connector/?connector_mode=${connectionType}`), - }; - - axiosPrivate(requestOptions) - .then((response) => { - const connectors = response?.data || []; - + fetchAllPages(axiosPrivate, { + url: getUrl("connector/"), + params: { connector_mode: connectionType }, + }) + .then((connectors) => { // Separate regular connectors from "Add new connector" option const regularConnectors = connectors.map((conn) => ({ value: conn?.id, diff --git a/frontend/src/components/custom-tools/adapter-selection-modal/AdapterSelectionModal.jsx b/frontend/src/components/custom-tools/adapter-selection-modal/AdapterSelectionModal.jsx index 80a0bd67d2..af1edd9525 100644 --- a/frontend/src/components/custom-tools/adapter-selection-modal/AdapterSelectionModal.jsx +++ b/frontend/src/components/custom-tools/adapter-selection-modal/AdapterSelectionModal.jsx @@ -4,6 +4,7 @@ import { useEffect, useState } from "react"; import "./AdapterSelectionModal.css"; +import { fetchAllPages } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; import { useAlertStore } from "../../../store/alert-store"; @@ -45,8 +46,7 @@ function AdapterSelectionModal({ try { const adapterTypes = ["LLM", "EMBEDDING", "VECTOR_DB", "X2TEXT"]; const requests = adapterTypes.map((type) => - axiosPrivate({ - method: "GET", + fetchAllPages(axiosPrivate, { url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, headers: { "X-CSRFToken": sessionDetails?.csrfToken, @@ -57,14 +57,9 @@ function AdapterSelectionModal({ }), ); - const responses = await Promise.all(requests); + const [llm, embedding, vectorDb, x2text] = await Promise.all(requests); - setAdapters({ - llm: responses[0]?.data || [], - embedding: responses[1]?.data || [], - vectorDb: responses[2]?.data || [], - x2text: responses[3]?.data || [], - }); + setAdapters({ llm, embedding, vectorDb, x2text }); } catch (err) { setAlertDetails( handleException(err, "Failed to fetch available adapters"), diff --git a/frontend/src/components/custom-tools/add-llm-profile/AddLlmProfile.jsx b/frontend/src/components/custom-tools/add-llm-profile/AddLlmProfile.jsx index 80cf854c2d..c76e073e1b 100644 --- a/frontend/src/components/custom-tools/add-llm-profile/AddLlmProfile.jsx +++ b/frontend/src/components/custom-tools/add-llm-profile/AddLlmProfile.jsx @@ -20,6 +20,7 @@ import PropTypes from "prop-types"; import { useEffect, useState } from "react"; import { getBackendErrorDetail } from "../../../helpers/GetStaticData"; +import { fetchAllPages } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; import { useAlertStore } from "../../../store/alert-store"; @@ -195,15 +196,10 @@ function AddLlmProfile({ }; const setAdaptorProfilesDropdown = () => { - const requestOptions = { - method: "GET", - url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter`, - }; - - axiosPrivate(requestOptions) - .then((res) => { - const data = res?.data; - + fetchAllPages(axiosPrivate, { + url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, + }) + .then((data) => { const llm = []; const vectorDb = []; const embedding = []; diff --git a/frontend/src/components/custom-tools/combined-output/CombinedOutput.jsx b/frontend/src/components/custom-tools/combined-output/CombinedOutput.jsx index a80d0d145c..363aac078f 100644 --- a/frontend/src/components/custom-tools/combined-output/CombinedOutput.jsx +++ b/frontend/src/components/custom-tools/combined-output/CombinedOutput.jsx @@ -12,6 +12,7 @@ import { getLLMModelNamesForProfiles, promptType, } from "../../../helpers/GetStaticData"; +import { fetchAllPages } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useAlertStore } from "../../../store/alert-store"; import { useCustomToolStore } from "../../../store/custom-tool-store"; @@ -146,8 +147,7 @@ function CombinedOutput({ docId, setFilledFields, selectedPrompts }) { url = publicAdapterApi(id, "LLM"); } try { - const res = await axiosPrivate.get(url); - const adapterList = res?.data; + const adapterList = await fetchAllPages(axiosPrivate, { url }); setAdapterData(getLLMModelNamesForProfiles(llmProfiles, adapterList)); } catch (err) { setAlertDetails( 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..0698f30bd4 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,13 @@ 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 { unwrapList } from "../../../helpers/pagination"; 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"; @@ -51,6 +53,8 @@ DefaultCustomButtons.propTypes = { handleNewProjectBtnClick: PropTypes.func.isRequired, }; +const DEFAULT_PAGE_SIZE = 10; + function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const [isListLoading, setIsListLoading] = useState(false); const [openAddTool, setOpenAddTool] = useState(false); @@ -65,7 +69,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const groupsApi = groupsService(); const [listOfTools, setListOfTools] = useState([]); - const [filteredListOfTools, setFilteredListOfTools] = useState([]); + // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) + const fetchListRef = useRef(null); const [isEdit, setIsEdit] = useState(false); const [promptDetails, setPromptDetails] = useState(null); const [openSharePermissionModal, setOpenSharePermissionModal] = @@ -106,6 +111,28 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { [axiosPrivate, sessionDetails?.orgId, sessionDetails?.csrfToken], ); + const { + pagination, + setPagination, + searchTerm, + handlePaginationChange, + handleSearch, + } = usePaginatedList({ + fetchData: (...args) => fetchListRef.current?.(...args), + defaultPageSize: DEFAULT_PAGE_SIZE, + }); + + // Refresh the current page (preserves page + active search) after mutations + const handleListRefresh = useCallback( + () => + fetchListRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + ), + [pagination.current, pagination.pageSize, searchTerm], + ); + const { coOwnerOpen, setCoOwnerOpen, @@ -119,7 +146,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { } = useCoOwnerManagement({ service: promptStudioCoOwnerService, setAlertDetails, - onListRefresh: () => getListOfTools(), + onListRefresh: handleListRefresh, }); const [allGroupList, setAllGroupList] = useState([]); @@ -127,25 +154,35 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { getListOfTools(); }, []); - useEffect(() => { - setFilteredListOfTools(listOfTools); - }, [listOfTools]); + const getListOfTools = ( + page = 1, + pageSize = DEFAULT_PAGE_SIZE, + search = "", + ) => { + const params = { page, page_size: pageSize }; + if (search) { + params.search = search; + } - const getListOfTools = () => { - const requestOptions = { + setIsListLoading(true); + axiosPrivate({ method: "GET", url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, headers: { "X-CSRFToken": sessionDetails?.csrfToken, }, - }; - - setIsListLoading(true); - axiosPrivate(requestOptions) + params, + }) .then((res) => { - const data = res?.data; - setListOfTools(data); - setFilteredListOfTools(data); + const results = unwrapList(res); + const total = res?.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); + return; + } + setListOfTools(results); + setPagination((prev) => ({ ...prev, current: page, pageSize, total })); }) .catch((err) => { setAlertDetails( @@ -157,6 +194,12 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { }); }; + // Effect, not a render-time write: mutating a ref during render is unsafe + // under concurrent rendering, where a render can be discarded. + useEffect(() => { + fetchListRef.current = getListOfTools; + }); + const handleAddNewTool = (body) => { let method = "POST"; let url = `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`; @@ -178,8 +221,10 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { axiosPrivate(requestOptions) .then((res) => { - const tool = res?.data; - updateList(isEdit, tool); + if (isEdit) { + setEditItem(null); + } + handleListRefresh(); setOpenAddTool(false); resolve(res?.data); }) @@ -189,22 +234,6 @@ 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, @@ -228,28 +257,13 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { axiosPrivate(requestOptions) .then(() => { - const tools = [...listOfTools].filter( - (filterToll) => filterToll?.tool_id !== tool.tool_id, - ); - setListOfTools(tools); + 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 +329,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")); @@ -414,8 +428,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) {
); @@ -447,9 +466,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { handleSearch(value)} customButtons={customButtonsElement} segmentOptions={segmentOptions} segmentValue={segmentValue} diff --git a/frontend/src/components/custom-tools/view-tools/ViewTools.css b/frontend/src/components/custom-tools/view-tools/ViewTools.css index 329bdd21cf..63b6bfdfd4 100644 --- a/frontend/src/components/custom-tools/view-tools/ViewTools.css +++ b/frontend/src/components/custom-tools/view-tools/ViewTools.css @@ -1 +1,5 @@ /* Styles for ViewTools */ + +.view-tools-pagination { + padding: 12px 16px; +} diff --git a/frontend/src/components/custom-tools/view-tools/ViewTools.jsx b/frontend/src/components/custom-tools/view-tools/ViewTools.jsx index d5dca62837..dc4a044438 100644 --- a/frontend/src/components/custom-tools/view-tools/ViewTools.jsx +++ b/frontend/src/components/custom-tools/view-tools/ViewTools.jsx @@ -1,3 +1,4 @@ +import { Flex, Pagination } from "antd"; import PropTypes from "prop-types"; import { ListView } from "../../widgets/list-view/ListView"; @@ -23,6 +24,7 @@ function ViewTools({ showOwner, showModified, type, + pagination, }) { if (isLoading) { return ; @@ -48,23 +50,44 @@ function ViewTools({ return ; } + const showPagination = pagination && pagination.total > pagination.pageSize; + return ( - + <> + + {showPagination && ( + + + `${range[0]}-${range[1]} of ${total} ${ + pagination.itemLabel || "items" + }` + } + /> + + )} + ); } @@ -86,6 +109,13 @@ ViewTools.propTypes = { showOwner: PropTypes.bool, showModified: PropTypes.bool, type: PropTypes.string, + pagination: PropTypes.shape({ + current: PropTypes.number, + pageSize: PropTypes.number, + total: PropTypes.number, + onChange: PropTypes.func, + itemLabel: PropTypes.string, + }), }; export { ViewTools }; diff --git a/frontend/src/components/helpers/custom-tools/CustomToolsHelper.js b/frontend/src/components/helpers/custom-tools/CustomToolsHelper.js index 35c2854cb0..5a9dcb7574 100644 --- a/frontend/src/components/helpers/custom-tools/CustomToolsHelper.js +++ b/frontend/src/components/helpers/custom-tools/CustomToolsHelper.js @@ -1,6 +1,7 @@ import { useEffect, useState } from "react"; import { Outlet, useNavigate, useParams } from "react-router-dom"; +import { fetchAllPages } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; import { useAlertStore } from "../../../store/alert-store"; @@ -117,16 +118,13 @@ function CustomToolsHelper() { .then((res) => { const data = res?.data; updatedCusTool["shareId"] = data?.share_id; - const reqOpsLlmProfiles = { - method: "GET", - url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, - }; - return handleApiRequest(reqOpsLlmProfiles); + return fetchAllPages(axiosPrivate, { + url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, + }); }) - .then((res) => { - const data = res?.data; - updatedCusTool["adapters"] = data; + .then((adapters) => { + updatedCusTool["adapters"] = adapters; if (fetchLookupAssignments) { const toolId = updatedCusTool["details"]?.tool_id; diff --git a/frontend/src/components/pipelines-or-deployments/etl-task-deploy/EtlTaskDeploy.jsx b/frontend/src/components/pipelines-or-deployments/etl-task-deploy/EtlTaskDeploy.jsx index b3803ee79e..a6330bdb21 100644 --- a/frontend/src/components/pipelines-or-deployments/etl-task-deploy/EtlTaskDeploy.jsx +++ b/frontend/src/components/pipelines-or-deployments/etl-task-deploy/EtlTaskDeploy.jsx @@ -82,8 +82,8 @@ const EtlTaskDeploy = ({ const getWorkflowList = () => { workflowApiService .getWorkflowList() - .then((res) => { - setWorkflowList(res?.data); + .then((workflows) => { + setWorkflowList(workflows); }) .catch(() => { console.error("Unable to get workflow list"); diff --git a/frontend/src/components/settings/default-triad/DefaultTriad.jsx b/frontend/src/components/settings/default-triad/DefaultTriad.jsx index 141f630e4c..4496f3faf5 100644 --- a/frontend/src/components/settings/default-triad/DefaultTriad.jsx +++ b/frontend/src/components/settings/default-triad/DefaultTriad.jsx @@ -3,6 +3,7 @@ import { Button, Select, Typography } from "antd"; import { useEffect, useState } from "react"; import { useNavigate } from "react-router-dom"; +import { fetchAllPages } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler.jsx"; import { IslandLayout } from "../../../layouts/island-layout/IslandLayout.jsx"; @@ -61,14 +62,11 @@ function DefaultTriad() { }; const fetchData = () => { - const requestOptions = { - method: "GET", + fetchAllPages(axiosPrivate, { url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, - }; - axiosPrivate(requestOptions) - .then((res) => { - const data = res?.data; - setAdapterList(data); + }) + .then((adapters) => { + setAdapterList(adapters); }) .catch((err) => { setAlertDetails( diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 04f41d7694..7db9fdb338 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -1,11 +1,12 @@ 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 { unwrapList } from "../../../helpers/pagination"; 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"; @@ -36,8 +37,13 @@ const btnText = { ocr: "New OCR", }; +const DEFAULT_PAGE_SIZE = 10; + function ToolSettings({ type }) { const [isLoading, setIsLoading] = useState(false); + const [adapterList, setAdapterList] = useState([]); + // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) + const fetchListRef = useRef(null); const [isShareLoading, setIsShareLoading] = useState(false); const [adapterDetails, setAdapterDetails] = useState(null); const [userList, setUserList] = useState([]); @@ -86,6 +92,29 @@ function ToolSettings({ type }) { [sessionDetails?.orgId, sessionDetails?.csrfToken], ); + const { + pagination, + setPagination, + searchTerm, + setSearchTerm, + handlePaginationChange, + handleSearch, + } = usePaginatedList({ + fetchData: (...args) => fetchListRef.current?.(...args), + defaultPageSize: DEFAULT_PAGE_SIZE, + }); + + // Refresh the current page (preserves page + active search) after mutations + const handleListRefresh = useCallback( + () => + fetchListRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + ), + [pagination.current, pagination.pageSize, searchTerm], + ); + const { coOwnerOpen, setCoOwnerOpen, @@ -99,39 +128,45 @@ 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"); + // Adapter type is a separate listing; reset paging and search when it changes. useEffect(() => { - clearSearch(); - setMasterList([]); + setSearchTerm(""); + setAdapterList([]); if (!type) { return; } - getAdapters(); + getAdapters(1, DEFAULT_PAGE_SIZE, ""); }, [type]); - const getAdapters = () => { - const requestOptions = { - method: "GET", - url: `/api/v1/unstract/${ - sessionDetails?.orgId - }/adapter?adapter_type=${type.toUpperCase()}`, + const getAdapters = (page = 1, pageSize = DEFAULT_PAGE_SIZE, search = "") => { + const params = { + adapter_type: type.toUpperCase(), + page, + page_size: pageSize, }; + if (search) { + params.search = search; + } setIsLoading(true); - axiosPrivate(requestOptions) + axiosPrivate({ + method: "GET", + url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, + params, + }) .then((res) => { - setMasterList(res?.data || []); + const results = unwrapList(res); + const total = res?.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); + return; + } + setAdapterList(results); + setPagination((prev) => ({ ...prev, current: page, pageSize, total })); }) .catch((err) => { setAlertDetails(handleException(err)); @@ -141,26 +176,15 @@ function ToolSettings({ type }) { }); }; - 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]); - } - }; + // Effect, not a render-time write: mutating a ref during render is unsafe + // under concurrent rendering, where a render can be discarded. + useEffect(() => { + fetchListRef.current = getAdapters; + }); - const handleDeleteSuccess = (adapterId) => { - updateMasterList((currentList) => - currentList.filter((row) => row?.id !== adapterId), - ); - }; + const addNewItem = () => handleListRefresh(); + + const handleDeleteSuccess = () => handleListRefresh(); const handleDelete = (_event, adapter) => { const requestOptions = { @@ -173,7 +197,7 @@ function ToolSettings({ type }) { setIsLoading(true); axiosPrivate(requestOptions) - .then(() => handleDeleteSuccess(adapter?.id)) + .then(() => handleDeleteSuccess()) .catch((err) => setAlertDetails(handleException(err))) .finally(() => setIsLoading(false)); }; @@ -297,8 +321,7 @@ function ToolSettings({ type }) { title={titles[type]} enableSearch searchKey={type} - setSearchList={setDisplayList} - onSearch={onSearch} + onSearch={(value) => handleSearch(value)} customButtons={
diff --git a/frontend/src/components/workflows/workflow/Workflows.css b/frontend/src/components/workflows/workflow/Workflows.css index 7a407a2fec..22d6ec9e95 100644 --- a/frontend/src/components/workflows/workflow/Workflows.css +++ b/frontend/src/components/workflows/workflow/Workflows.css @@ -85,9 +85,3 @@ flex: 1; overflow-y: auto; } - -.workflows-pagination { - display: flex; - justify-content: flex-end; - padding: 12px 16px; -} diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 7d5567c36f..414736f469 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -1,9 +1,10 @@ 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"; +import { unwrapList } from "../../../helpers/pagination"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement.jsx"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler.jsx"; import { usePaginatedList } from "../../../hooks/usePaginatedList"; @@ -124,11 +125,8 @@ function Workflows() { 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; + const results = unwrapList(res); + const total = res?.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); @@ -151,7 +149,12 @@ function Workflows() { setLoading(false); }); }; - fetchListRef.current = getProjectList; + + // Effect, not a render-time write: mutating a ref during render is unsafe + // under concurrent rendering, where a render can be discarded. + useEffect(() => { + fetchListRef.current = getProjectList; + }); function editProject(name, description) { setLoading(true); @@ -423,33 +426,25 @@ function Workflows() { )} {projectList?.length > 0 && ( - <> - - {pagination.total > pagination.pageSize && ( -
- -
- )} - + )} {editingProject && ( { - options = { - url: `${path}/workflow/?is_active=True`, - method: "GET", - }; - return axiosPrivate(options); - }, + // Feeds selectors, so it resolves to every workflow rather than one page. + getWorkflowList: () => + fetchAllPages(axiosPrivate, { + url: `${path}/workflow/`, + params: { is_active: "True" }, + }), getWorkflowEndpointList: (endpointType, connectorType) => { options = { url: `${path}/workflow/endpoint/?endpoint_type=${endpointType}&connection_type=${connectorType}`, diff --git a/frontend/src/helpers/pagination.js b/frontend/src/helpers/pagination.js new file mode 100644 index 0000000000..2e0ee10bfb --- /dev/null +++ b/frontend/src/helpers/pagination.js @@ -0,0 +1,62 @@ +/** + * Helpers for list endpoints that may or may not be paginated. + * + * The shared resource endpoints (workflows, prompt studio, adapters, + * connectors) return a bare array unless the caller opts into pagination, and + * a `{count, next, previous, results}` envelope once it does. Callers should + * not care which they got. + */ + +/** + * Rows out of a list response, whichever shape it arrived in. + * + * @param {Object} res - axios response + * @return {Array} the rows, or [] when the payload is neither shape + */ +function unwrapList(res) { + const data = res?.data; + if (Array.isArray(data)) { + return data; + } + return Array.isArray(data?.results) ? data.results : []; +} + +/** + * Every row of a list endpoint, following pagination to exhaustion. + * + * For selectors and dropdowns, which must show the full set — reading only the + * first page would silently hide entries with no error. Pages are requested by + * number rather than by following `next`, so the call keeps going through the + * caller's axios instance (base URL, auth interceptors) instead of a + * server-built absolute URL. + * + * @param {Object} axiosInstance - an axios instance (e.g. from useAxiosPrivate) + * @param {Object} config - axios request config; `url` is required + * @return {Promise} all rows across all pages + */ +async function fetchAllPages(axiosInstance, config) { + const request = (params) => + axiosInstance({ ...config, method: "GET", ...(params ? { params } : {}) }); + + const first = await request(); + if (Array.isArray(first?.data)) { + return first.data; + } + + const rows = [...(first?.data?.results ?? [])]; + const total = first?.data?.count ?? rows.length; + let page = 1; + while (rows.length < total) { + page += 1; + const res = await request({ ...(config?.params ?? {}), page }); + const nextRows = res?.data?.results ?? []; + // A page that adds nothing would otherwise spin forever on a bad count. + if (!nextRows.length) { + break; + } + rows.push(...nextRows); + } + return rows; +} + +export { fetchAllPages, unwrapList }; diff --git a/frontend/src/helpers/pagination.test.js b/frontend/src/helpers/pagination.test.js new file mode 100644 index 0000000000..64810137e0 --- /dev/null +++ b/frontend/src/helpers/pagination.test.js @@ -0,0 +1,54 @@ +import { describe, expect, it, vi } from "vitest"; + +import { fetchAllPages, unwrapList } from "./pagination"; + +describe("unwrapList", () => { + it("returns a bare array unchanged", () => { + expect(unwrapList({ data: [1, 2] })).toEqual([1, 2]); + }); + + it("unwraps a paginated envelope", () => { + expect(unwrapList({ data: { count: 2, results: [1, 2] } })).toEqual([1, 2]); + }); + + it("returns [] for anything else", () => { + expect(unwrapList(undefined)).toEqual([]); + expect(unwrapList({ data: { detail: "nope" } })).toEqual([]); + }); +}); + +describe("fetchAllPages", () => { + it("makes a single request when the endpoint is not paginated", async () => { + const axiosInstance = vi.fn().mockResolvedValue({ data: [1, 2, 3] }); + await expect(fetchAllPages(axiosInstance, { url: "/x/" })).resolves.toEqual( + [1, 2, 3], + ); + expect(axiosInstance).toHaveBeenCalledTimes(1); + }); + + it("follows pages until every row is collected", async () => { + const axiosInstance = vi + .fn() + .mockResolvedValueOnce({ data: { count: 3, results: [1, 2] } }) + .mockResolvedValueOnce({ data: { count: 3, results: [3] } }); + + await expect( + fetchAllPages(axiosInstance, { url: "/x/", params: { type: "LLM" } }), + ).resolves.toEqual([1, 2, 3]); + // Later pages must keep the caller's own params alongside ?page. + expect(axiosInstance.mock.calls[1][0].params).toEqual({ + type: "LLM", + page: 2, + }); + }); + + it("stops instead of looping when a page adds nothing", async () => { + const axiosInstance = vi + .fn() + .mockResolvedValue({ data: { count: 99, results: [] } }); + await expect(fetchAllPages(axiosInstance, { url: "/x/" })).resolves.toEqual( + [], + ); + expect(axiosInstance).toHaveBeenCalledTimes(2); + }); +}); diff --git a/frontend/src/hooks/useListSearch.js b/frontend/src/hooks/useListSearch.js deleted file mode 100644 index f74d96718c..0000000000 --- a/frontend/src/hooks/useListSearch.js +++ /dev/null @@ -1,61 +0,0 @@ -import { useCallback, useRef, useState } from "react"; - -function useListSearch(searchField) { - const listRef = useRef([]); - const searchTextRef = useRef(""); - const [displayList, setDisplayList] = useState([]); - - const filterList = useCallback( - (list, searchText) => { - if (!searchText.trim()) { - return list; - } - return list.filter((item) => - item[searchField]?.toLowerCase().includes(searchText.toLowerCase()), - ); - }, - [searchField], - ); - - const setMasterList = useCallback( - (list) => { - listRef.current = list; - setDisplayList(filterList(list, searchTextRef.current)); - }, - [filterList], - ); - - const onSearch = useCallback( - (searchText, setSearchList) => { - searchTextRef.current = searchText; - setSearchList(filterList(listRef.current, searchText)); - }, - [filterList], - ); - - const clearSearch = useCallback(() => { - searchTextRef.current = ""; - setDisplayList(listRef.current); - }, []); - - const updateMasterList = useCallback( - (updateFn) => { - const updatedList = updateFn(listRef.current); - listRef.current = updatedList; - setDisplayList(filterList(updatedList, searchTextRef.current)); - }, - [filterList], - ); - - return { - listRef, - displayList, - setDisplayList, - setMasterList, - updateMasterList, - onSearch, - clearSearch, - }; -} - -export { useListSearch }; diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index cacb50fc27..2357263986 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -1,6 +1,6 @@ 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"; @@ -8,17 +8,23 @@ import { AddSourceModal } from "../components/input-output/add-source-modal/AddS import { ToolNavBar } from "../components/navigations/tool-nav-bar/ToolNavBar"; import { CoOwnerManagement } from "../components/widgets/co-owner-management/CoOwnerManagement"; import { SharePermission } from "../components/widgets/share-permission/SharePermission"; +import { unwrapList } from "../helpers/pagination"; 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 [connectorList, setConnectorList] = useState([]); + // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) + const fetchListRef = useRef(null); const [modalVisible, setModalVisible] = useState(false); const [editingConnector, setEditingConnector] = useState(null); const [shareModalVisible, setShareModalVisible] = useState(false); @@ -61,6 +67,28 @@ function ConnectorsPage() { [sessionDetails?.csrfToken], ); + const { + pagination, + setPagination, + searchTerm, + handlePaginationChange, + handleSearch, + } = usePaginatedList({ + fetchData: (...args) => fetchListRef.current?.(...args), + defaultPageSize: DEFAULT_PAGE_SIZE, + }); + + // Refresh the current page (preserves page + active search) after mutations + const handleListRefresh = useCallback( + () => + fetchListRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + ), + [pagination.current, pagination.pageSize, searchTerm], + ); + const { coOwnerOpen, setCoOwnerOpen, @@ -74,21 +102,34 @@ function ConnectorsPage() { } = useCoOwnerManagement({ service: connectorCoOwnerService, setAlertDetails, - onListRefresh: () => fetchConnectors(), + onListRefresh: handleListRefresh, }); - const { listRef, displayList, setDisplayList, setMasterList, onSearch } = - useListSearch("connector_name"); useEffect(() => { fetchConnectors(); fetchUsers(); }, []); - const fetchConnectors = async () => { + const fetchConnectors = async ( + page = 1, + pageSize = DEFAULT_PAGE_SIZE, + search = "", + ) => { + const params = { page, page_size: pageSize }; + if (search) { + params.search = search; + } setLoading(true); try { - const response = await axiosPrivate.get(getUrl("connector/")); - setMasterList(response.data || []); + const response = await axiosPrivate.get(getUrl("connector/"), { params }); + const results = unwrapList(response); + const total = response?.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 fetchConnectors(page - 1, pageSize, search); + } + setConnectorList(results); + setPagination((prev) => ({ ...prev, current: page, pageSize, total })); } catch (error) { setAlertDetails(handleException(error, "Failed to load connectors")); } finally { @@ -96,6 +137,12 @@ function ConnectorsPage() { } }; + // Effect, not a render-time write: mutating a ref during render is unsafe + // under concurrent rendering, where a render can be discarded. + useEffect(() => { + fetchListRef.current = fetchConnectors; + }); + const fetchUsers = async () => { try { const response = await axiosPrivate.get(getUrl("users/")); @@ -134,7 +181,7 @@ function ConnectorsPage() { type: "success", content: "Connector deleted successfully", }); - fetchConnectors(); + handleListRefresh(); } catch (error) { setAlertDetails(handleException(error, "Failed to delete connector")); } @@ -211,7 +258,7 @@ function ConnectorsPage() { const handleConnectorSaved = () => { setModalVisible(false); setEditingConnector(null); - fetchConnectors(); + handleListRefresh(); setAlertDetails({ type: "success", content: editingConnector @@ -235,14 +282,13 @@ function ConnectorsPage() { handleSearch(value)} customButtons={newConnectorButton} />
From c2ec0dcf30d375ba6d1660a6a59ae5aecc6b2aad Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Fri, 24 Jul 2026 18:25:23 +0530 Subject: [PATCH 02/17] UN-3770 [FIX] Address review: pk tie-breaker on client ordering, shared list hook Backend: - DeterministicOrderingFilter appends `pk` to whatever ordering is in effect. `?ordering=` replaces the view's `ordering` outright, so the tie-breaker has to be applied by the filter rather than declared on the view. Swapped into DEFAULT_FILTER_BACKENDS; a no-op for views that declare no ordering and receive no `?ordering=`. - Tests assert the returned sequence rather than set membership, and pin the tie-breaker with rows sharing one `modified_at`. Both fail without the filter. Frontend: - usePaginatedResource owns the request for all four list pages: params, unwrapping, the empty-page step back, and the loading flag. Replaces four copy-pasted fetch functions (the Sonar duplication) and the fetchListRef indirection. - Only the newest request may write state, so a slow response can no longer restore the previous page, search term or adapter type. - The loading flag is held by the superseding request, so it no longer clears while the step-back page is still in flight. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- backend/backend/settings/base.py | 2 +- backend/utils/filters/ordering_filter.py | 22 +++ backend/utils/tests/test_list_pagination.py | 94 ++++++++++--- .../list-of-tools/ListOfTools.jsx | 85 +++--------- .../tool-settings/ToolSettings.jsx | 81 +++--------- .../workflows/workflow/Workflows.jsx | 85 +++--------- frontend/src/hooks/usePaginatedResource.js | 125 ++++++++++++++++++ .../src/hooks/usePaginatedResource.test.js | 122 +++++++++++++++++ frontend/src/pages/ConnectorsPage.jsx | 66 ++------- 9 files changed, 403 insertions(+), 279 deletions(-) create mode 100644 backend/utils/filters/ordering_filter.py create mode 100644 frontend/src/hooks/usePaginatedResource.js create mode 100644 frontend/src/hooks/usePaginatedResource.test.js diff --git a/backend/backend/settings/base.py b/backend/backend/settings/base.py index 94071ac3b8..179e62593a 100644 --- a/backend/backend/settings/base.py +++ b/backend/backend/settings/base.py @@ -627,7 +627,7 @@ def filter(self, record): "DEFAULT_FILTER_BACKENDS": [ "utils.filters.organization_filter.OrganizationFilterBackend", "django_filters.rest_framework.DjangoFilterBackend", - "rest_framework.filters.OrderingFilter", + "utils.filters.ordering_filter.DeterministicOrderingFilter", ], # For API versioning "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.URLPathVersioning", diff --git a/backend/utils/filters/ordering_filter.py b/backend/utils/filters/ordering_filter.py new file mode 100644 index 0000000000..82a7e6e40c --- /dev/null +++ b/backend/utils/filters/ordering_filter.py @@ -0,0 +1,22 @@ +from rest_framework.filters import OrderingFilter + + +class DeterministicOrderingFilter(OrderingFilter): + """OrderingFilter that always ends the ordering with the primary key. + + Pagination runs one query per page, so an ordering with ties leaves the + tied rows in whatever order the DB returns that time — a row can repeat on + the next page or be skipped entirely. A trailing unique column removes the + ambiguity. `?ordering=` replaces the view's `ordering`, so the tie-breaker + has to be appended here rather than declared on the view. + """ + + def get_ordering(self, request, queryset, view): + ordering = super().get_ordering(request, queryset, view) + if not ordering: + return ordering + + pk_names = {"pk", queryset.model._meta.pk.name} + if any(term.lstrip("-") in pk_names for term in ordering): + return ordering + return [*ordering, "pk"] diff --git a/backend/utils/tests/test_list_pagination.py b/backend/utils/tests/test_list_pagination.py index ca24f6f889..cdb61677c0 100644 --- a/backend/utils/tests/test_list_pagination.py +++ b/backend/utils/tests/test_list_pagination.py @@ -2,9 +2,10 @@ Workflows, Prompt Studio, adapters and connectors share one listing shape: ``for_user()`` sharing predicate -> ``.distinct()`` -> declarative ``ordering`` --> ``OptionalPagination``. These tests pin the three ways that combination can -silently serve wrong rows — non-deterministic page boundaries, duplicate rows -from the sharing predicate, and a search applied after the count. +-> ``OptionalPagination``. These tests pin the ways that combination can +silently serve wrong rows — non-deterministic page boundaries, a client +``?ordering=`` that drops the pk tie-breaker, duplicate rows from the sharing +predicate, and a search applied after the count. DB-backed (Django ``TestCase``), so ``backend/conftest.py`` auto-marks these ``integration``. @@ -14,6 +15,7 @@ import json from collections.abc import Callable +from datetime import UTC, datetime, timedelta from functools import lru_cache from typing import Any, NamedTuple @@ -33,6 +35,9 @@ from rest_framework.test import APIRequestFactory, force_authenticate from workflow_manager.workflow_v2.views import WorkflowViewSet +# Fixed so ordering assertions never depend on wall-clock ties. +BASE_TIME = datetime(2026, 1, 1, tzinfo=UTC) + @lru_cache(maxsize=1) def _live_llm_adapter_id() -> str: @@ -123,29 +128,74 @@ def _list(self, endpoint: ListEndpoint, user, **params: Any): def _names(self, endpoint: ListEndpoint, rows) -> list[str]: return [row[endpoint.name_field] for row in rows] - def test_pages_partition_the_result_set(self) -> None: - """Page 2 must not repeat or drop rows from page 1. + def _stamp(self, obj: Any, when: datetime) -> None: + """Pin ``modified_at``, which ``auto_now`` would otherwise overwrite.""" + type(obj).objects.filter(pk=obj.pk).update(modified_at=when) + + def test_pages_partition_the_result_set_newest_first(self) -> None: + """Pages follow the declared ``-modified_at`` and together cover the set. - Without a deterministic ``ordering`` the two requests run independent - queries, so rows can appear twice or vanish entirely between them. + Asserting the sequence, not just membership: page boundaries are only + stable if every request evaluates the same ordering, so a lost or + arbitrary ordering has to fail here rather than pass on set equality. """ for endpoint in LIST_ENDPOINTS: with self.subTest(kind=endpoint.kind): - expected = {f"{endpoint.kind}-page-{i}" for i in range(5)} - for name in sorted(expected): - self._create(endpoint, name) - - page1 = self._list(endpoint, self.owner, page=1, page_size=2) - page2 = self._list(endpoint, self.owner, page=2, page_size=2) - page3 = self._list(endpoint, self.owner, page=3, page_size=2) - - names1 = self._names(endpoint, page1.data["results"]) - names2 = self._names(endpoint, page2.data["results"]) - names3 = self._names(endpoint, page3.data["results"]) - - assert page1.data["count"] == len(expected) - assert not set(names1) & set(names2) - assert set(names1) | set(names2) | set(names3) == expected + # Created oldest-first, so the expected listing is the reverse. + for i in range(5): + obj = self._create(endpoint, f"{endpoint.kind}-page-{i}") + self._stamp(obj, BASE_TIME + timedelta(minutes=i)) + expected = [f"{endpoint.kind}-page-{i}" for i in reversed(range(5))] + + pages = [ + self._list(endpoint, self.owner, page=n, page_size=2) + for n in (1, 2, 3) + ] + names = [ + n + for page in pages + for n in self._names(endpoint, page.data["results"]) + ] + + assert pages[0].data["count"] == len(expected) + assert names == expected + + def test_client_ordering_keeps_pk_tiebreaker(self) -> None: + """``?ordering=`` replaces the view default, so pk must still be appended. + + Every row here shares one ``modified_at``. Primary keys are random + UUIDs, so ordering by pk is unrelated to insertion order — the returned + sequence only matches if pk survived as the tie-breaker. + """ + for endpoint in LIST_ENDPOINTS: + with self.subTest(kind=endpoint.kind): + created = [] + for i in range(6): + obj = self._create(endpoint, f"{endpoint.kind}-tied-{i}") + self._stamp(obj, BASE_TIME) + created.append(obj) + expected = [ + getattr(obj, endpoint.name_field) + for obj in sorted(created, key=lambda o: str(o.pk)) + ] + + pages = [ + self._list( + endpoint, + self.owner, + page=n, + page_size=3, + ordering="modified_at", + ) + for n in (1, 2) + ] + names = [ + n + for page in pages + for n in self._names(endpoint, page.data["results"]) + ] + + assert names == expected def test_multi_predicate_share_yields_one_row(self) -> None: """A resource reachable by several sharing predicates lists once. 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 0698f30bd4..1485c312b4 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -1,13 +1,12 @@ import { ArrowDownOutlined, PlusOutlined } from "@ant-design/icons"; import { Space } from "antd"; import PropTypes from "prop-types"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; -import { unwrapList } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { usePaginatedResource } from "../../../hooks/usePaginatedResource"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import { useAlertStore } from "../../../store/alert-store"; import { useSessionStore } from "../../../store/session-store"; @@ -56,7 +55,6 @@ DefaultCustomButtons.propTypes = { const DEFAULT_PAGE_SIZE = 10; function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { - const [isListLoading, setIsListLoading] = useState(false); const [openAddTool, setOpenAddTool] = useState(false); const [openImportTool, setOpenImportTool] = useState(false); const [isImportLoading, setIsImportLoading] = useState(false); @@ -68,9 +66,6 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const handleException = useExceptionHandler(); const groupsApi = groupsService(); - const [listOfTools, setListOfTools] = useState([]); - // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) - const fetchListRef = useRef(null); const [isEdit, setIsEdit] = useState(false); const [promptDetails, setPromptDetails] = useState(null); const [openSharePermissionModal, setOpenSharePermissionModal] = @@ -112,27 +107,27 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { ); const { + items: listOfTools, + isLoading: isListLoading, pagination, - setPagination, searchTerm, + fetchPage, + refresh: handleListRefresh, handlePaginationChange, handleSearch, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), + } = usePaginatedResource({ + request: (params) => + axiosPrivate({ + method: "GET", + url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, + headers: { "X-CSRFToken": sessionDetails?.csrfToken }, + params, + }), + onError: (err) => + setAlertDetails(handleException(err, "Failed to get the list of tools")), defaultPageSize: DEFAULT_PAGE_SIZE, }); - // Refresh the current page (preserves page + active search) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - ), - [pagination.current, pagination.pageSize, searchTerm], - ); - const { coOwnerOpen, setCoOwnerOpen, @@ -151,55 +146,9 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const [allGroupList, setAllGroupList] = useState([]); useEffect(() => { - getListOfTools(); + fetchPage(); }, []); - const getListOfTools = ( - page = 1, - pageSize = DEFAULT_PAGE_SIZE, - search = "", - ) => { - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - - setIsListLoading(true); - axiosPrivate({ - method: "GET", - url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, - headers: { - "X-CSRFToken": sessionDetails?.csrfToken, - }, - params, - }) - .then((res) => { - const results = unwrapList(res); - const total = res?.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); - return; - } - setListOfTools(results); - setPagination((prev) => ({ ...prev, current: page, pageSize, total })); - }) - .catch((err) => { - setAlertDetails( - handleException(err, "Failed to get the list of tools"), - ); - }) - .finally(() => { - setIsListLoading(false); - }); - }; - - // Effect, not a render-time write: mutating a ref during render is unsafe - // under concurrent rendering, where a render can be discarded. - useEffect(() => { - fetchListRef.current = getListOfTools; - }); - const handleAddNewTool = (body) => { let method = "POST"; let url = `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`; diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 7db9fdb338..52228af3fd 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -1,12 +1,11 @@ import { PlusOutlined } from "@ant-design/icons"; import PropTypes from "prop-types"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; -import { unwrapList } from "../../../helpers/pagination"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { usePaginatedResource } from "../../../hooks/usePaginatedResource"; import usePostHogEvents from "../../../hooks/usePostHogEvents"; import { IslandLayout } from "../../../layouts/island-layout/IslandLayout"; import { useAlertStore } from "../../../store/alert-store"; @@ -40,10 +39,6 @@ const btnText = { const DEFAULT_PAGE_SIZE = 10; function ToolSettings({ type }) { - const [isLoading, setIsLoading] = useState(false); - const [adapterList, setAdapterList] = useState([]); - // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) - const fetchListRef = useRef(null); const [isShareLoading, setIsShareLoading] = useState(false); const [adapterDetails, setAdapterDetails] = useState(null); const [userList, setUserList] = useState([]); @@ -93,28 +88,28 @@ function ToolSettings({ type }) { ); const { - pagination, - setPagination, + items: adapterList, + setItems: setAdapterList, + isLoading, + setIsLoading, searchTerm, setSearchTerm, + pagination, + fetchPage, + refresh: handleListRefresh, handlePaginationChange, handleSearch, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), + } = usePaginatedResource({ + request: (params) => + axiosPrivate({ + method: "GET", + url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, + params: { ...params, adapter_type: type?.toUpperCase() }, + }), + onError: (err) => setAlertDetails(handleException(err)), defaultPageSize: DEFAULT_PAGE_SIZE, }); - // Refresh the current page (preserves page + active search) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - ), - [pagination.current, pagination.pageSize, searchTerm], - ); - const { coOwnerOpen, setCoOwnerOpen, @@ -139,49 +134,9 @@ function ToolSettings({ type }) { if (!type) { return; } - getAdapters(1, DEFAULT_PAGE_SIZE, ""); + fetchPage(1, DEFAULT_PAGE_SIZE, ""); }, [type]); - const getAdapters = (page = 1, pageSize = DEFAULT_PAGE_SIZE, search = "") => { - const params = { - adapter_type: type.toUpperCase(), - page, - page_size: pageSize, - }; - if (search) { - params.search = search; - } - setIsLoading(true); - axiosPrivate({ - method: "GET", - url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, - params, - }) - .then((res) => { - const results = unwrapList(res); - const total = res?.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); - return; - } - setAdapterList(results); - setPagination((prev) => ({ ...prev, current: page, pageSize, total })); - }) - .catch((err) => { - setAlertDetails(handleException(err)); - }) - .finally(() => { - setIsLoading(false); - }); - }; - - // Effect, not a render-time write: mutating a ref during render is unsafe - // under concurrent rendering, where a render can be discarded. - useEffect(() => { - fetchListRef.current = getAdapters; - }); - const addNewItem = () => handleListRefresh(); const handleDeleteSuccess = () => handleListRefresh(); diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 414736f469..39ccba6c8d 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -1,13 +1,12 @@ 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, useState } from "react"; import { useLocation, useNavigate } from "react-router-dom"; -import { unwrapList } from "../../../helpers/pagination"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement.jsx"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler.jsx"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { usePaginatedResource } from "../../../hooks/usePaginatedResource"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import { useInitialFetchCount, @@ -50,12 +49,8 @@ function Workflows() { getPromptStudioCount, ); - const [projectList, setProjectList] = useState(); 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); const [backendErrors, setBackendErrors] = useState(null); const [shareOpen, setShareOpen] = useState(false); const [selectedWorkflow, setSelectedWorkflow] = useState(); @@ -67,26 +62,20 @@ function Workflows() { const { setAlertDetails } = useAlertStore(); const { + items: projectList, + isLoading: loading, + setIsLoading: setLoading, pagination, - setPagination, searchTerm, + fetchPage, + refresh: handleListRefresh, handlePaginationChange, handleSearch, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), + } = usePaginatedResource({ + request: (params) => projectApiService.getProjectList(params), + onError: () => console.error("Unable to get project list"), defaultPageSize: DEFAULT_PAGE_SIZE, }); - - // Refresh the current page (preserves page + active search) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - ), - [pagination.current, pagination.pageSize, searchTerm], - ); const { coOwnerOpen, setCoOwnerOpen, @@ -108,54 +97,10 @@ function Workflows() { useEffect(() => { if (location.pathname === `/${orgName}/workflows`) { - getProjectList(); + fetchPage(); } }, [location.pathname]); - const getProjectList = ( - page = 1, - pageSize = DEFAULT_PAGE_SIZE, - search = "", - ) => { - setLoading(true); - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - projectApiService - .getProjectList(params) - .then((res) => { - const results = unwrapList(res); - const total = res?.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); - return; - } - setProjectList(results); - setPagination((prev) => ({ - ...prev, - current: page, - pageSize, - total, - })); - }) - .catch(() => { - console.error("Unable to get project list"); - // Avoid an indefinite spinner when the first fetch fails. - setProjectList((prev) => prev ?? []); - }) - .finally(() => { - setLoading(false); - }); - }; - - // Effect, not a render-time write: mutating a ref during render is unsafe - // under concurrent rendering, where a render can be discarded. - useEffect(() => { - fetchListRef.current = getProjectList; - }); - function editProject(name, description) { setLoading(true); projectApiService @@ -409,8 +354,8 @@ function Workflows() { />
- {projectList === undefined && } - {projectList?.length === 0 && !searchTerm && ( + {loading && !projectList.length && } + {!loading && projectList.length === 0 && !searchTerm && (
)} - {projectList?.length === 0 && searchTerm && ( + {!loading && projectList.length === 0 && searchTerm && ( )} - {projectList?.length > 0 && ( + {projectList.length > 0 && ( { + requestRef.current = request; + onErrorRef.current = onError; + }); + + // Only the newest request may write state. A slower earlier one would + // otherwise resurrect the previous page, search term or resource type. + const latestRequestId = useRef(0); + + const fetchPage = useCallback( + async (page = 1, pageSize = defaultPageSize, search = "") => { + const requestId = ++latestRequestId.current; + const isCurrent = () => requestId === latestRequestId.current; + + setIsLoading(true); + try { + const params = { page, page_size: pageSize }; + if (search) { + params.search = search; + } + const res = await requestRef.current?.(params); + if (!isCurrent()) { + return; + } + const results = unwrapList(res); + const total = res?.data?.count ?? results.length; + // Deleting the last row on a page leaves it empty; step back a page. + // Awaited so the loading flag outlives the replacement request. + if (!results.length && page > 1 && total > 0) { + return await fetchPage(page - 1, pageSize, search); + } + setItems(results); + setPagination((prev) => ({ ...prev, current: page, pageSize, total })); + } catch (err) { + if (isCurrent()) { + onErrorRef.current?.(err); + } + } finally { + // A superseding request owns the flag once it has started. + if (isCurrent()) { + setIsLoading(false); + } + } + }, + [defaultPageSize], + ); + + const handlePaginationChange = useCallback( + (page, pageSize) => { + // A changed page size invalidates the offset, so restart at page 1. + const nextPage = pageSize === pagination.pageSize ? page : 1; + fetchPage(nextPage, pageSize, searchTerm); + }, + [fetchPage, pagination.pageSize, searchTerm], + ); + + const handleSearch = useCallback( + (searchText) => { + const term = searchText?.trim() || ""; + setSearchTerm(term); + fetchPage(1, pagination.pageSize, term); + }, + [fetchPage, pagination.pageSize], + ); + + // Re-runs the current page, preserving the page and any active search. + const refresh = useCallback( + () => fetchPage(pagination.current, pagination.pageSize, searchTerm), + [fetchPage, pagination.current, pagination.pageSize, searchTerm], + ); + + return { + items, + setItems, + isLoading, + setIsLoading, + pagination, + setPagination, + searchTerm, + setSearchTerm, + fetchPage, + refresh, + handlePaginationChange, + handleSearch, + }; +} + +export { usePaginatedResource }; diff --git a/frontend/src/hooks/usePaginatedResource.test.js b/frontend/src/hooks/usePaginatedResource.test.js new file mode 100644 index 0000000000..b8afb0e180 --- /dev/null +++ b/frontend/src/hooks/usePaginatedResource.test.js @@ -0,0 +1,122 @@ +import { act, renderHook, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import { usePaginatedResource } from "./usePaginatedResource"; + +const page = (results, count) => ({ data: { count, results } }); + +const deferred = () => { + let resolve; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +}; + +describe("usePaginatedResource", () => { + it("loads the first page and records the total", async () => { + const request = vi.fn().mockResolvedValue(page(["a", "b"], 5)); + const { result } = renderHook(() => usePaginatedResource({ request })); + + await act(() => result.current.fetchPage(1, 2, "")); + + expect(request).toHaveBeenCalledWith({ page: 1, page_size: 2 }); + expect(result.current.items).toEqual(["a", "b"]); + expect(result.current.pagination).toMatchObject({ current: 1, total: 5 }); + expect(result.current.isLoading).toBe(false); + }); + + it("passes a search term through and omits it when blank", async () => { + const request = vi.fn().mockResolvedValue(page([], 0)); + const { result } = renderHook(() => usePaginatedResource({ request })); + + await act(() => result.current.fetchPage(1, 10, "abc")); + expect(request).toHaveBeenLastCalledWith({ + page: 1, + page_size: 10, + search: "abc", + }); + + await act(() => result.current.fetchPage(1, 10, "")); + expect(request).toHaveBeenLastCalledWith({ page: 1, page_size: 10 }); + }); + + it("steps back a page when the requested one came back empty", async () => { + const request = vi + .fn() + .mockResolvedValueOnce(page([], 2)) + .mockResolvedValueOnce(page(["a", "b"], 2)); + const { result } = renderHook(() => usePaginatedResource({ request })); + + await act(() => result.current.fetchPage(2, 2, "")); + + expect(request).toHaveBeenLastCalledWith({ page: 1, page_size: 2 }); + expect(result.current.items).toEqual(["a", "b"]); + expect(result.current.pagination).toMatchObject({ current: 1 }); + }); + + it("keeps loading true until the step-back request resolves", async () => { + const second = deferred(); + const request = vi + .fn() + .mockResolvedValueOnce(page([], 2)) + .mockReturnValueOnce(second.promise); + const { result } = renderHook(() => usePaginatedResource({ request })); + + let done; + act(() => { + done = result.current.fetchPage(2, 2, ""); + }); + await waitFor(() => expect(request).toHaveBeenCalledTimes(2)); + expect(result.current.isLoading).toBe(true); + + await act(async () => { + second.resolve(page(["a"], 2)); + await done; + }); + expect(result.current.isLoading).toBe(false); + // Awaiting the call must mean the replacement page is already in state, + // so callers that chain off refresh() see the rows they asked for. + expect(result.current.items).toEqual(["a"]); + }); + + it("ignores a superseded response that resolves late", async () => { + const slow = deferred(); + const request = vi + .fn() + .mockReturnValueOnce(slow.promise) + .mockResolvedValueOnce(page(["new"], 1)); + const { result } = renderHook(() => usePaginatedResource({ request })); + + let stale; + act(() => { + stale = result.current.fetchPage(1, 10, "old"); + }); + await act(() => result.current.fetchPage(1, 10, "new")); + expect(result.current.items).toEqual(["new"]); + + await act(async () => { + slow.resolve(page(["old"], 1)); + await stale; + }); + expect(result.current.items).toEqual(["new"]); + }); + + it("reports errors without clobbering the current rows", async () => { + const onError = vi.fn(); + const request = vi + .fn() + .mockResolvedValueOnce(page(["a"], 1)) + .mockRejectedValueOnce(new Error("boom")); + const { result } = renderHook(() => + usePaginatedResource({ request, onError }), + ); + + await act(() => result.current.fetchPage(1, 10, "")); + await act(() => result.current.fetchPage(2, 10, "")); + + expect(onError).toHaveBeenCalledTimes(1); + expect(result.current.items).toEqual(["a"]); + expect(result.current.isLoading).toBe(false); + }); +}); diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 2357263986..479d478979 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -1,6 +1,6 @@ import { PlusOutlined } from "@ant-design/icons"; import { Button } from "antd"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import { ViewTools } from "../components/custom-tools/view-tools/ViewTools"; import { groupsService } from "../components/groups/groups-service.js"; @@ -8,11 +8,10 @@ import { AddSourceModal } from "../components/input-output/add-source-modal/AddS import { ToolNavBar } from "../components/navigations/tool-nav-bar/ToolNavBar"; import { CoOwnerManagement } from "../components/widgets/co-owner-management/CoOwnerManagement"; import { SharePermission } from "../components/widgets/share-permission/SharePermission"; -import { unwrapList } from "../helpers/pagination"; import { useAxiosPrivate } from "../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../hooks/useExceptionHandler"; -import { usePaginatedList } from "../hooks/usePaginatedList"; +import { usePaginatedResource } from "../hooks/usePaginatedResource"; import useRequestUrl from "../hooks/useRequestUrl"; import { useAlertStore } from "../store/alert-store"; import { useSessionStore } from "../store/session-store"; @@ -21,10 +20,6 @@ import "./ConnectorsPage.css"; const DEFAULT_PAGE_SIZE = 10; function ConnectorsPage() { - const [loading, setLoading] = useState(false); - const [connectorList, setConnectorList] = useState([]); - // Ref forwards the fetch fn to the pagination hook (avoids declaration ordering) - const fetchListRef = useRef(null); const [modalVisible, setModalVisible] = useState(false); const [editingConnector, setEditingConnector] = useState(null); const [shareModalVisible, setShareModalVisible] = useState(false); @@ -68,27 +63,21 @@ function ConnectorsPage() { ); const { + items: connectorList, + isLoading: loading, pagination, - setPagination, searchTerm, + fetchPage, + refresh: handleListRefresh, handlePaginationChange, handleSearch, - } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), + } = usePaginatedResource({ + request: (params) => axiosPrivate.get(getUrl("connector/"), { params }), + onError: (error) => + setAlertDetails(handleException(error, "Failed to load connectors")), defaultPageSize: DEFAULT_PAGE_SIZE, }); - // Refresh the current page (preserves page + active search) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - ), - [pagination.current, pagination.pageSize, searchTerm], - ); - const { coOwnerOpen, setCoOwnerOpen, @@ -106,43 +95,10 @@ function ConnectorsPage() { }); useEffect(() => { - fetchConnectors(); + fetchPage(); fetchUsers(); }, []); - const fetchConnectors = async ( - page = 1, - pageSize = DEFAULT_PAGE_SIZE, - search = "", - ) => { - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - setLoading(true); - try { - const response = await axiosPrivate.get(getUrl("connector/"), { params }); - const results = unwrapList(response); - const total = response?.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 fetchConnectors(page - 1, pageSize, search); - } - setConnectorList(results); - setPagination((prev) => ({ ...prev, current: page, pageSize, total })); - } catch (error) { - setAlertDetails(handleException(error, "Failed to load connectors")); - } finally { - setLoading(false); - } - }; - - // Effect, not a render-time write: mutating a ref during render is unsafe - // under concurrent rendering, where a render can be discarded. - useEffect(() => { - fetchListRef.current = fetchConnectors; - }); - const fetchUsers = async () => { try { const response = await axiosPrivate.get(getUrl("users/")); From 45156b84b8178e3a4ea8ec21f8f3a11e00a4dd44 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Mon, 27 Jul 2026 18:26:32 +0530 Subject: [PATCH 03/17] UN-3770 [MISC] Scope to backend + selectors; drop listing-page conversions The Prompt Studio, adapters and connectors listing pages are being replaced wholesale by the ResourceTable in #2200. Converting them here to usePaginatedResource only to have that work overwritten created the entire conflict surface between the two PRs, plus a second pagination hook. Reverts the four listing-page conversions, the ViewTools pagination move and the useListSearch deletion, and drops usePaginatedResource. What remains is the part #2200 depends on and cannot do itself: the DISTINCT ON removal, declarative ordering with a pk tie-breaker, and the selector page-following that keeps dropdowns whole once pagination goes unconditional. usePaginatedResource.test.js goes with the hook; its coverage should be ported onto the surviving usePaginatedList in #2200. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0159NvRHFywvkNGji8ECQqeV --- .../list-of-tools/ListOfTools.jsx | 116 ++++++++++------ .../custom-tools/view-tools/ViewTools.css | 4 - .../custom-tools/view-tools/ViewTools.jsx | 62 +++------ .../tool-settings/ToolSettings.jsx | 101 ++++++++------ .../workflows/workflow/Workflows.css | 6 + .../workflows/workflow/Workflows.jsx | 130 +++++++++++++----- frontend/src/hooks/useListSearch.js | 61 ++++++++ frontend/src/hooks/usePaginatedResource.js | 125 ----------------- .../src/hooks/usePaginatedResource.test.js | 122 ---------------- frontend/src/pages/ConnectorsPage.jsx | 55 ++++---- 10 files changed, 336 insertions(+), 446 deletions(-) create mode 100644 frontend/src/hooks/useListSearch.js delete mode 100644 frontend/src/hooks/usePaginatedResource.js delete mode 100644 frontend/src/hooks/usePaginatedResource.test.js 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 1485c312b4..38fb925a9c 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,6 @@ import { useEffect, useMemo, useState } from "react"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; -import { usePaginatedResource } from "../../../hooks/usePaginatedResource"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import { useAlertStore } from "../../../store/alert-store"; import { useSessionStore } from "../../../store/session-store"; @@ -52,9 +51,8 @@ DefaultCustomButtons.propTypes = { handleNewProjectBtnClick: PropTypes.func.isRequired, }; -const DEFAULT_PAGE_SIZE = 10; - function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { + const [isListLoading, setIsListLoading] = useState(false); const [openAddTool, setOpenAddTool] = useState(false); const [openImportTool, setOpenImportTool] = useState(false); const [isImportLoading, setIsImportLoading] = useState(false); @@ -66,6 +64,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const handleException = useExceptionHandler(); const groupsApi = groupsService(); + const [listOfTools, setListOfTools] = useState([]); + const [filteredListOfTools, setFilteredListOfTools] = useState([]); const [isEdit, setIsEdit] = useState(false); const [promptDetails, setPromptDetails] = useState(null); const [openSharePermissionModal, setOpenSharePermissionModal] = @@ -106,28 +106,6 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { [axiosPrivate, sessionDetails?.orgId, sessionDetails?.csrfToken], ); - const { - items: listOfTools, - isLoading: isListLoading, - pagination, - searchTerm, - fetchPage, - refresh: handleListRefresh, - handlePaginationChange, - handleSearch, - } = usePaginatedResource({ - request: (params) => - axiosPrivate({ - method: "GET", - url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, - headers: { "X-CSRFToken": sessionDetails?.csrfToken }, - params, - }), - onError: (err) => - setAlertDetails(handleException(err, "Failed to get the list of tools")), - defaultPageSize: DEFAULT_PAGE_SIZE, - }); - const { coOwnerOpen, setCoOwnerOpen, @@ -141,14 +119,44 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { } = useCoOwnerManagement({ service: promptStudioCoOwnerService, setAlertDetails, - onListRefresh: handleListRefresh, + onListRefresh: () => getListOfTools(), }); const [allGroupList, setAllGroupList] = useState([]); useEffect(() => { - fetchPage(); + 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); + }); + }; + const handleAddNewTool = (body) => { let method = "POST"; let url = `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`; @@ -170,10 +178,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { axiosPrivate(requestOptions) .then((res) => { - if (isEdit) { - setEditItem(null); - } - handleListRefresh(); + const tool = res?.data; + updateList(isEdit, tool); setOpenAddTool(false); resolve(res?.data); }) @@ -183,6 +189,22 @@ 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, @@ -206,13 +228,28 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { axiosPrivate(requestOptions) .then(() => { - handleListRefresh(); + const tools = [...listOfTools].filter( + (filterToll) => filterToll?.tool_id !== tool.tool_id, + ); + setListOfTools(tools); }) .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); @@ -278,7 +315,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { setOpenImportTool(false); // Refresh the list of tools to show the new imported project - handleListRefresh(); + getListOfTools(); }) .catch((err) => { setAlertDetails(handleException(err, "Failed to import project")); @@ -377,8 +414,8 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) {
); @@ -415,7 +447,9 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { handleSearch(value)} + onSearch={onSearch} + searchList={listOfTools} + setSearchList={setFilteredListOfTools} customButtons={customButtonsElement} segmentOptions={segmentOptions} segmentValue={segmentValue} diff --git a/frontend/src/components/custom-tools/view-tools/ViewTools.css b/frontend/src/components/custom-tools/view-tools/ViewTools.css index 63b6bfdfd4..329bdd21cf 100644 --- a/frontend/src/components/custom-tools/view-tools/ViewTools.css +++ b/frontend/src/components/custom-tools/view-tools/ViewTools.css @@ -1,5 +1 @@ /* Styles for ViewTools */ - -.view-tools-pagination { - padding: 12px 16px; -} diff --git a/frontend/src/components/custom-tools/view-tools/ViewTools.jsx b/frontend/src/components/custom-tools/view-tools/ViewTools.jsx index dc4a044438..d5dca62837 100644 --- a/frontend/src/components/custom-tools/view-tools/ViewTools.jsx +++ b/frontend/src/components/custom-tools/view-tools/ViewTools.jsx @@ -1,4 +1,3 @@ -import { Flex, Pagination } from "antd"; import PropTypes from "prop-types"; import { ListView } from "../../widgets/list-view/ListView"; @@ -24,7 +23,6 @@ function ViewTools({ showOwner, showModified, type, - pagination, }) { if (isLoading) { return ; @@ -50,44 +48,23 @@ function ViewTools({ return ; } - const showPagination = pagination && pagination.total > pagination.pageSize; - return ( - <> - - {showPagination && ( - - - `${range[0]}-${range[1]} of ${total} ${ - pagination.itemLabel || "items" - }` - } - /> - - )} - + ); } @@ -109,13 +86,6 @@ ViewTools.propTypes = { showOwner: PropTypes.bool, showModified: PropTypes.bool, type: PropTypes.string, - pagination: PropTypes.shape({ - current: PropTypes.number, - pageSize: PropTypes.number, - total: PropTypes.number, - onChange: PropTypes.func, - itemLabel: 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 52228af3fd..04f41d7694 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -5,7 +5,7 @@ import { useEffect, useMemo, useState } from "react"; import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; -import { usePaginatedResource } from "../../../hooks/usePaginatedResource"; +import { useListSearch } from "../../../hooks/useListSearch"; import usePostHogEvents from "../../../hooks/usePostHogEvents"; import { IslandLayout } from "../../../layouts/island-layout/IslandLayout"; import { useAlertStore } from "../../../store/alert-store"; @@ -36,9 +36,8 @@ const btnText = { ocr: "New OCR", }; -const DEFAULT_PAGE_SIZE = 10; - function ToolSettings({ type }) { + const [isLoading, setIsLoading] = useState(false); const [isShareLoading, setIsShareLoading] = useState(false); const [adapterDetails, setAdapterDetails] = useState(null); const [userList, setUserList] = useState([]); @@ -87,29 +86,6 @@ function ToolSettings({ type }) { [sessionDetails?.orgId, sessionDetails?.csrfToken], ); - const { - items: adapterList, - setItems: setAdapterList, - isLoading, - setIsLoading, - searchTerm, - setSearchTerm, - pagination, - fetchPage, - refresh: handleListRefresh, - handlePaginationChange, - handleSearch, - } = usePaginatedResource({ - request: (params) => - axiosPrivate({ - method: "GET", - url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter/`, - params: { ...params, adapter_type: type?.toUpperCase() }, - }), - onError: (err) => setAlertDetails(handleException(err)), - defaultPageSize: DEFAULT_PAGE_SIZE, - }); - const { coOwnerOpen, setCoOwnerOpen, @@ -123,23 +99,68 @@ function ToolSettings({ type }) { } = useCoOwnerManagement({ service: adapterCoOwnerService, setAlertDetails, - onListRefresh: handleListRefresh, + onListRefresh: () => getAdapters(), }); const { posthogEventText, setPostHogCustomEvent } = usePostHogEvents(); + const { + listRef, + displayList, + setDisplayList, + setMasterList, + updateMasterList, + onSearch, + clearSearch, + } = useListSearch("adapter_name"); - // Adapter type is a separate listing; reset paging and search when it changes. useEffect(() => { - setSearchTerm(""); - setAdapterList([]); + clearSearch(); + setMasterList([]); if (!type) { return; } - fetchPage(1, DEFAULT_PAGE_SIZE, ""); + getAdapters(); }, [type]); - const addNewItem = () => handleListRefresh(); + 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 = () => handleListRefresh(); + const handleDeleteSuccess = (adapterId) => { + updateMasterList((currentList) => + currentList.filter((row) => row?.id !== adapterId), + ); + }; const handleDelete = (_event, adapter) => { const requestOptions = { @@ -152,7 +173,7 @@ function ToolSettings({ type }) { setIsLoading(true); axiosPrivate(requestOptions) - .then(() => handleDeleteSuccess()) + .then(() => handleDeleteSuccess(adapter?.id)) .catch((err) => setAlertDetails(handleException(err))) .finally(() => setIsLoading(false)); }; @@ -276,7 +297,8 @@ function ToolSettings({ type }) { title={titles[type]} enableSearch searchKey={type} - onSearch={(value) => handleSearch(value)} + setSearchList={setDisplayList} + onSearch={onSearch} customButtons={
diff --git a/frontend/src/components/workflows/workflow/Workflows.css b/frontend/src/components/workflows/workflow/Workflows.css index 22d6ec9e95..7a407a2fec 100644 --- a/frontend/src/components/workflows/workflow/Workflows.css +++ b/frontend/src/components/workflows/workflow/Workflows.css @@ -85,3 +85,9 @@ flex: 1; overflow-y: auto; } + +.workflows-pagination { + display: flex; + justify-content: flex-end; + padding: 12px 16px; +} diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 39ccba6c8d..7d5567c36f 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -1,12 +1,12 @@ import { PlusOutlined, UserOutlined } from "@ant-design/icons"; -import { Typography } from "antd"; +import { Pagination, Typography } from "antd"; import PropTypes from "prop-types"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { useLocation, useNavigate } from "react-router-dom"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement.jsx"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler.jsx"; -import { usePaginatedResource } from "../../../hooks/usePaginatedResource"; +import { usePaginatedList } from "../../../hooks/usePaginatedList"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import { useInitialFetchCount, @@ -49,8 +49,12 @@ function Workflows() { getPromptStudioCount, ); + const [projectList, setProjectList] = useState(); 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); const [backendErrors, setBackendErrors] = useState(null); const [shareOpen, setShareOpen] = useState(false); const [selectedWorkflow, setSelectedWorkflow] = useState(); @@ -62,20 +66,26 @@ function Workflows() { const { setAlertDetails } = useAlertStore(); const { - items: projectList, - isLoading: loading, - setIsLoading: setLoading, pagination, + setPagination, searchTerm, - fetchPage, - refresh: handleListRefresh, handlePaginationChange, handleSearch, - } = usePaginatedResource({ - request: (params) => projectApiService.getProjectList(params), - onError: () => console.error("Unable to get project list"), + } = usePaginatedList({ + fetchData: (...args) => fetchListRef.current?.(...args), defaultPageSize: DEFAULT_PAGE_SIZE, }); + + // Refresh the current page (preserves page + active search) after mutations + const handleListRefresh = useCallback( + () => + fetchListRef.current?.( + pagination.current, + pagination.pageSize, + searchTerm, + ), + [pagination.current, pagination.pageSize, searchTerm], + ); const { coOwnerOpen, setCoOwnerOpen, @@ -97,10 +107,52 @@ function Workflows() { useEffect(() => { if (location.pathname === `/${orgName}/workflows`) { - fetchPage(); + getProjectList(); } }, [location.pathname]); + const getProjectList = ( + page = 1, + pageSize = DEFAULT_PAGE_SIZE, + search = "", + ) => { + setLoading(true); + const params = { page, page_size: pageSize }; + if (search) { + params.search = search; + } + 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); + return; + } + setProjectList(results); + setPagination((prev) => ({ + ...prev, + current: page, + pageSize, + total, + })); + }) + .catch(() => { + console.error("Unable to get project list"); + // Avoid an indefinite spinner when the first fetch fails. + setProjectList((prev) => prev ?? []); + }) + .finally(() => { + setLoading(false); + }); + }; + fetchListRef.current = getProjectList; + function editProject(name, description) { setLoading(true); projectApiService @@ -354,8 +406,8 @@ function Workflows() { />
- {loading && !projectList.length && } - {!loading && projectList.length === 0 && !searchTerm && ( + {projectList === undefined && } + {projectList?.length === 0 && !searchTerm && (
)} - {!loading && projectList.length === 0 && searchTerm && ( + {projectList?.length === 0 && searchTerm && ( )} - {projectList.length > 0 && ( - + {projectList?.length > 0 && ( + <> + + {pagination.total > pagination.pageSize && ( +
+ +
+ )} + )} {editingProject && ( { + if (!searchText.trim()) { + return list; + } + return list.filter((item) => + item[searchField]?.toLowerCase().includes(searchText.toLowerCase()), + ); + }, + [searchField], + ); + + const setMasterList = useCallback( + (list) => { + listRef.current = list; + setDisplayList(filterList(list, searchTextRef.current)); + }, + [filterList], + ); + + const onSearch = useCallback( + (searchText, setSearchList) => { + searchTextRef.current = searchText; + setSearchList(filterList(listRef.current, searchText)); + }, + [filterList], + ); + + const clearSearch = useCallback(() => { + searchTextRef.current = ""; + setDisplayList(listRef.current); + }, []); + + const updateMasterList = useCallback( + (updateFn) => { + const updatedList = updateFn(listRef.current); + listRef.current = updatedList; + setDisplayList(filterList(updatedList, searchTextRef.current)); + }, + [filterList], + ); + + return { + listRef, + displayList, + setDisplayList, + setMasterList, + updateMasterList, + onSearch, + clearSearch, + }; +} + +export { useListSearch }; diff --git a/frontend/src/hooks/usePaginatedResource.js b/frontend/src/hooks/usePaginatedResource.js deleted file mode 100644 index e1378d2ee8..0000000000 --- a/frontend/src/hooks/usePaginatedResource.js +++ /dev/null @@ -1,125 +0,0 @@ -import { useCallback, useEffect, useRef, useState } from "react"; - -import { unwrapList } from "../helpers/pagination"; - -const DEFAULT_PAGE_SIZE = 10; - -/** - * Server-side paginated list state for the shared resource endpoints. - * - * Owns the request so every list page behaves the same way: page/page_size/ - * search params, response unwrapping, the empty-page step back, in-flight - * response ordering, and the loading flag. - * - * @param {Object} options - * @param {Function} options.request - fn(params) resolving to the axios response - * @param {Function} [options.onError] - called with the request error - * @param {number} [options.defaultPageSize=10] - * @return {Object} List state and handlers - */ -function usePaginatedResource({ - request, - onError, - defaultPageSize = DEFAULT_PAGE_SIZE, -}) { - const [items, setItems] = useState([]); - const [isLoading, setIsLoading] = useState(false); - const [pagination, setPagination] = useState({ - current: 1, - pageSize: defaultPageSize, - total: 0, - }); - const [searchTerm, setSearchTerm] = useState(""); - - // Effect, not a render-time write: mutating a ref during render is unsafe - // under concurrent rendering, where a render can be discarded. - const requestRef = useRef(request); - const onErrorRef = useRef(onError); - useEffect(() => { - requestRef.current = request; - onErrorRef.current = onError; - }); - - // Only the newest request may write state. A slower earlier one would - // otherwise resurrect the previous page, search term or resource type. - const latestRequestId = useRef(0); - - const fetchPage = useCallback( - async (page = 1, pageSize = defaultPageSize, search = "") => { - const requestId = ++latestRequestId.current; - const isCurrent = () => requestId === latestRequestId.current; - - setIsLoading(true); - try { - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - const res = await requestRef.current?.(params); - if (!isCurrent()) { - return; - } - const results = unwrapList(res); - const total = res?.data?.count ?? results.length; - // Deleting the last row on a page leaves it empty; step back a page. - // Awaited so the loading flag outlives the replacement request. - if (!results.length && page > 1 && total > 0) { - return await fetchPage(page - 1, pageSize, search); - } - setItems(results); - setPagination((prev) => ({ ...prev, current: page, pageSize, total })); - } catch (err) { - if (isCurrent()) { - onErrorRef.current?.(err); - } - } finally { - // A superseding request owns the flag once it has started. - if (isCurrent()) { - setIsLoading(false); - } - } - }, - [defaultPageSize], - ); - - const handlePaginationChange = useCallback( - (page, pageSize) => { - // A changed page size invalidates the offset, so restart at page 1. - const nextPage = pageSize === pagination.pageSize ? page : 1; - fetchPage(nextPage, pageSize, searchTerm); - }, - [fetchPage, pagination.pageSize, searchTerm], - ); - - const handleSearch = useCallback( - (searchText) => { - const term = searchText?.trim() || ""; - setSearchTerm(term); - fetchPage(1, pagination.pageSize, term); - }, - [fetchPage, pagination.pageSize], - ); - - // Re-runs the current page, preserving the page and any active search. - const refresh = useCallback( - () => fetchPage(pagination.current, pagination.pageSize, searchTerm), - [fetchPage, pagination.current, pagination.pageSize, searchTerm], - ); - - return { - items, - setItems, - isLoading, - setIsLoading, - pagination, - setPagination, - searchTerm, - setSearchTerm, - fetchPage, - refresh, - handlePaginationChange, - handleSearch, - }; -} - -export { usePaginatedResource }; diff --git a/frontend/src/hooks/usePaginatedResource.test.js b/frontend/src/hooks/usePaginatedResource.test.js deleted file mode 100644 index b8afb0e180..0000000000 --- a/frontend/src/hooks/usePaginatedResource.test.js +++ /dev/null @@ -1,122 +0,0 @@ -import { act, renderHook, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; - -import { usePaginatedResource } from "./usePaginatedResource"; - -const page = (results, count) => ({ data: { count, results } }); - -const deferred = () => { - let resolve; - const promise = new Promise((r) => { - resolve = r; - }); - return { promise, resolve }; -}; - -describe("usePaginatedResource", () => { - it("loads the first page and records the total", async () => { - const request = vi.fn().mockResolvedValue(page(["a", "b"], 5)); - const { result } = renderHook(() => usePaginatedResource({ request })); - - await act(() => result.current.fetchPage(1, 2, "")); - - expect(request).toHaveBeenCalledWith({ page: 1, page_size: 2 }); - expect(result.current.items).toEqual(["a", "b"]); - expect(result.current.pagination).toMatchObject({ current: 1, total: 5 }); - expect(result.current.isLoading).toBe(false); - }); - - it("passes a search term through and omits it when blank", async () => { - const request = vi.fn().mockResolvedValue(page([], 0)); - const { result } = renderHook(() => usePaginatedResource({ request })); - - await act(() => result.current.fetchPage(1, 10, "abc")); - expect(request).toHaveBeenLastCalledWith({ - page: 1, - page_size: 10, - search: "abc", - }); - - await act(() => result.current.fetchPage(1, 10, "")); - expect(request).toHaveBeenLastCalledWith({ page: 1, page_size: 10 }); - }); - - it("steps back a page when the requested one came back empty", async () => { - const request = vi - .fn() - .mockResolvedValueOnce(page([], 2)) - .mockResolvedValueOnce(page(["a", "b"], 2)); - const { result } = renderHook(() => usePaginatedResource({ request })); - - await act(() => result.current.fetchPage(2, 2, "")); - - expect(request).toHaveBeenLastCalledWith({ page: 1, page_size: 2 }); - expect(result.current.items).toEqual(["a", "b"]); - expect(result.current.pagination).toMatchObject({ current: 1 }); - }); - - it("keeps loading true until the step-back request resolves", async () => { - const second = deferred(); - const request = vi - .fn() - .mockResolvedValueOnce(page([], 2)) - .mockReturnValueOnce(second.promise); - const { result } = renderHook(() => usePaginatedResource({ request })); - - let done; - act(() => { - done = result.current.fetchPage(2, 2, ""); - }); - await waitFor(() => expect(request).toHaveBeenCalledTimes(2)); - expect(result.current.isLoading).toBe(true); - - await act(async () => { - second.resolve(page(["a"], 2)); - await done; - }); - expect(result.current.isLoading).toBe(false); - // Awaiting the call must mean the replacement page is already in state, - // so callers that chain off refresh() see the rows they asked for. - expect(result.current.items).toEqual(["a"]); - }); - - it("ignores a superseded response that resolves late", async () => { - const slow = deferred(); - const request = vi - .fn() - .mockReturnValueOnce(slow.promise) - .mockResolvedValueOnce(page(["new"], 1)); - const { result } = renderHook(() => usePaginatedResource({ request })); - - let stale; - act(() => { - stale = result.current.fetchPage(1, 10, "old"); - }); - await act(() => result.current.fetchPage(1, 10, "new")); - expect(result.current.items).toEqual(["new"]); - - await act(async () => { - slow.resolve(page(["old"], 1)); - await stale; - }); - expect(result.current.items).toEqual(["new"]); - }); - - it("reports errors without clobbering the current rows", async () => { - const onError = vi.fn(); - const request = vi - .fn() - .mockResolvedValueOnce(page(["a"], 1)) - .mockRejectedValueOnce(new Error("boom")); - const { result } = renderHook(() => - usePaginatedResource({ request, onError }), - ); - - await act(() => result.current.fetchPage(1, 10, "")); - await act(() => result.current.fetchPage(2, 10, "")); - - expect(onError).toHaveBeenCalledTimes(1); - expect(result.current.items).toEqual(["a"]); - expect(result.current.isLoading).toBe(false); - }); -}); diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 479d478979..cacb50fc27 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -11,15 +11,14 @@ import { SharePermission } from "../components/widgets/share-permission/SharePer import { useAxiosPrivate } from "../hooks/useAxiosPrivate"; import { useCoOwnerManagement } from "../hooks/useCoOwnerManagement"; import { useExceptionHandler } from "../hooks/useExceptionHandler"; -import { usePaginatedResource } from "../hooks/usePaginatedResource"; +import { useListSearch } from "../hooks/useListSearch"; 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); const [editingConnector, setEditingConnector] = useState(null); const [shareModalVisible, setShareModalVisible] = useState(false); @@ -62,22 +61,6 @@ function ConnectorsPage() { [sessionDetails?.csrfToken], ); - const { - items: connectorList, - isLoading: loading, - pagination, - searchTerm, - fetchPage, - refresh: handleListRefresh, - handlePaginationChange, - handleSearch, - } = usePaginatedResource({ - request: (params) => axiosPrivate.get(getUrl("connector/"), { params }), - onError: (error) => - setAlertDetails(handleException(error, "Failed to load connectors")), - defaultPageSize: DEFAULT_PAGE_SIZE, - }); - const { coOwnerOpen, setCoOwnerOpen, @@ -91,14 +74,28 @@ function ConnectorsPage() { } = useCoOwnerManagement({ service: connectorCoOwnerService, setAlertDetails, - onListRefresh: handleListRefresh, + onListRefresh: () => fetchConnectors(), }); + const { listRef, displayList, setDisplayList, setMasterList, onSearch } = + useListSearch("connector_name"); useEffect(() => { - fetchPage(); + fetchConnectors(); fetchUsers(); }, []); + 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/")); @@ -137,7 +134,7 @@ function ConnectorsPage() { type: "success", content: "Connector deleted successfully", }); - handleListRefresh(); + fetchConnectors(); } catch (error) { setAlertDetails(handleException(error, "Failed to delete connector")); } @@ -214,7 +211,7 @@ function ConnectorsPage() { const handleConnectorSaved = () => { setModalVisible(false); setEditingConnector(null); - handleListRefresh(); + fetchConnectors(); setAlertDetails({ type: "success", content: editingConnector @@ -238,13 +235,14 @@ function ConnectorsPage() { handleSearch(value)} + setSearchList={setDisplayList} + onSearch={onSearch} customButtons={newConnectorButton} />
From fd40c16f5cc4064c0bf9fb4c2ba22598aaa451da Mon Sep 17 00:00:00 2001 From: Kirtiman Mishra <110175055+kirtimanmishrazipstack@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:37:42 +0530 Subject: [PATCH 04/17] UN-3769 [FEAT] Sortable resource lists with co-owner ownership (#2200) * 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) * 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) * 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) * 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) * 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) * 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) * 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) * 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) * 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) * 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 * 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 * UN-3769 [FIX] Match resource-table owner avatars to Figma pastel palette Swap the saturated avatar swatches for light pastel fills paired with a darker same-hue initial, matching the design. Applies to all resource list views via the shared ResourceTable. Co-Authored-By: Claude Opus 4.8 * UN-3769 [FIX] Lighten resource-table owner-avatar initials per design Lighten avatar initial color (Ant -7 -> -6) and reduce initial size (12px -> 11px) per design review feedback. Co-Authored-By: Claude Opus 4.8 * UN-3769 [FEAT] Resource list: add Modified column + PS prompt count, default modified-desc sort Add a sortable Modified column and rename Created Date -> Created so both dates are visible. Surface the already-serialized prompt_count as "Prompts: N" on the Prompt Studio list. Default all resource lists to modified-desc so the visible Modified column matches the sort (restores #2187 Workflows ordering). Frontend-only; backend already served both dates and prompt_count. Co-Authored-By: Claude Opus 4.8 * UN-3769 [FEAT] Resource list: relative Modified time, canonical date format, owner search-only Co-Authored-By: Claude Opus 4.8 * UN-3769 [FIX] Resource list review fixes: name-only search, clear-sort restores default Address Chandru's re-review on #2200: - Owned By is display-only: drop created_by__email from ordering_fields and the search Q-filter across the 4 viewsets so search matches the shown owner (name-only) instead of the creator, and no dead owner-sort surface remains. - usePaginatedList: "Clear Sort" restores the default ordering (not an empty one) and list mounts request the seeded sort, so the header and rows agree. - Remove dead code: orphaned useListSearch.js, the dead avatar-initials branch, and the unreferenced .listWrapper rule; trim over-narrated comments. Co-Authored-By: Claude Opus 4.8 * UN-3769 [FIX] Tests: pin name-only search, dropped owner-ordering fallback, owner_email Cover the review changes in the shared list-pagination contract test: - ?search= matches the resource name only, not the owner's email. - ?ordering=created_by__email is a dropped field -> ignored, list stays newest-first (had it survived, same-creator rows would be pk-ordered). - owner_email() names the earliest live OWNER, skips service accounts, None with no owner (shared mixin, pinned once). Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 (1M context) --- backend/adapter_processor_v2/serializers.py | 1 + backend/adapter_processor_v2/views.py | 1 + backend/connector_v2/serializers.py | 1 + backend/connector_v2/views.py | 9 +- backend/permissions/models.py | 13 + .../prompt_studio_core_v2/serializers.py | 5 + .../prompt_studio_core_v2/views.py | 7 +- backend/utils/tests/test_list_pagination.py | 68 +++ .../workflow_v2/serializers.py | 1 + backend/workflow_manager/workflow_v2/views.py | 13 +- .../list-of-tools/ListOfTools.jsx | 309 +++++++------ .../custom-tools/view-tools/ViewTools.css | 1 - .../custom-tools/view-tools/ViewTools.jsx | 91 ---- .../api-deployment/ApiDeployment.jsx | 15 +- .../pipelines/Pipelines.jsx | 15 +- .../tool-settings/ToolSettings.jsx | 305 +++++++------ .../co-owner-management/CoOwnerModal.jsx | 37 ++ .../components/widgets/list-view/ListView.css | 180 -------- .../components/widgets/list-view/ListView.jsx | 336 -------------- .../widgets/resource-table/ResourceTable.css | 176 ++++++++ .../widgets/resource-table/ResourceTable.jsx | 412 ++++++++++++++++++ .../workflows/workflow/Workflows.css | 7 - .../workflows/workflow/Workflows.jsx | 206 +++++---- frontend/src/hooks/useListSearch.js | 61 --- frontend/src/hooks/usePaginatedList.js | 182 +++++++- frontend/src/pages/ConnectorsPage.jsx | 211 ++++++--- 26 files changed, 1513 insertions(+), 1150 deletions(-) delete mode 100644 frontend/src/components/custom-tools/view-tools/ViewTools.css delete mode 100644 frontend/src/components/custom-tools/view-tools/ViewTools.jsx create mode 100644 frontend/src/components/widgets/co-owner-management/CoOwnerModal.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 delete mode 100644 frontend/src/hooks/useListSearch.js 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/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index 5222f751e9..94b2d02d1d 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -193,6 +193,7 @@ def get_queryset(self) -> QuerySet | None: ): queryset = queryset.filter(**filter_args) + # Name search. search = self.request.query_params.get("search") if search: queryset = queryset.filter(adapter_name__icontains=search) 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/connector_v2/views.py b/backend/connector_v2/views.py index 84f5ef7005..90092a5846 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -108,10 +108,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: @@ -130,6 +126,11 @@ def get_queryset(self) -> QuerySet | None: ) queryset = queryset.none() + # Name search. + search = self.request.query_params.get("search") + if search: + queryset = queryset.filter(connector_name__icontains=search) + return queryset def _get_connector_metadata(self, connector_id: str) -> dict[str, str] | None: diff --git a/backend/permissions/models.py b/backend/permissions/models.py index 9c63804e16..e800847b5f 100644 --- a/backend/permissions/models.py +++ b/backend/permissions/models.py @@ -43,6 +43,19 @@ def co_owners_count(self) -> int: for m in self.memberships.all() # type: ignore[attr-defined] ) + def owner_email(self) -> str | None: + # "Owned By" email: earliest live OWNER. ``created_by`` is audit-only + # (UN-2202) and may differ from the owner. Reads prefetched + # ``memberships`` to stay query-free. + 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 71e15085ae..7617b63ae3 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -173,11 +173,8 @@ 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 + # modified_at stays a plain orderable field: prompt writes bump the + # parent row, so no annotation is needed. qs = qs.select_related("created_by").annotate( _prompt_count=Subquery(prompt_count_sq), ) diff --git a/backend/utils/tests/test_list_pagination.py b/backend/utils/tests/test_list_pagination.py index cdb61677c0..7984f2ff2b 100644 --- a/backend/utils/tests/test_list_pagination.py +++ b/backend/utils/tests/test_list_pagination.py @@ -29,6 +29,7 @@ _build_connector, _build_custom_tool, _build_workflow, + make_user, ) from prompt_studio.prompt_studio_core_v2.views import PromptStudioCoreView from rest_framework import status @@ -234,3 +235,70 @@ def test_search_narrows_rows_and_count(self) -> None: names = self._names(endpoint, response.data["results"]) assert response.data["count"] == 3 assert all("alpha" in name for name in names) + + def test_search_matches_name_not_owner_email(self) -> None: + """``?search=`` matches the resource name only, not the owner's email. + + UN-3769 narrowed search from owner-inclusive (``created_by__email``) to + name-only so the search box and the Owned By column agree; the owner's + email substring must no longer return their rows. + """ + for endpoint in LIST_ENDPOINTS: + with self.subTest(kind=endpoint.kind): + self._create(endpoint, f"{endpoint.kind}-searchable", owner=self.owner) + + by_email = self._list( + endpoint, self.owner, page=1, page_size=10, search="owner" + ) + by_name = self._list( + endpoint, self.owner, page=1, page_size=10, search="searchable" + ) + + assert by_email.data["count"] == 0 + assert by_name.data["count"] == 1 + + def test_dropped_owner_ordering_field_is_ignored(self) -> None: + """``?ordering=created_by__email`` is a dropped field, so it's ignored. + + UN-3769 removed ``created_by__email`` from ``ordering_fields``. DRF drops + an unknown ordering key and falls back to the view default + (``-modified_at, pk``) rather than 400ing, so the rows stay newest-first. + Every row shares one creator, so had the field survived the response + would be pk-ordered, not the newest-first sequence asserted here. + """ + for endpoint in LIST_ENDPOINTS: + with self.subTest(kind=endpoint.kind): + for i in range(5): + obj = self._create(endpoint, f"{endpoint.kind}-ord-{i}") + self._stamp(obj, BASE_TIME + timedelta(minutes=i)) + expected = [f"{endpoint.kind}-ord-{i}" for i in reversed(range(5))] + + response = self._list( + endpoint, + self.owner, + page=1, + page_size=10, + ordering="created_by__email", + ) + + assert self._names(endpoint, response.data["results"]) == expected + + def test_owner_email_is_earliest_live_owner(self) -> None: + """``owner_email()`` (the Owned By label) names the earliest live OWNER, + skips service accounts, and is ``None`` with no owner. Shared-mixin + behaviour, so one endpoint pins it for all four. + """ + svc = make_user("svc@example.com", is_service_account=True) + wf = _build_workflow(self.org, self.owner) + # Service account owns earliest (must be skipped); coowner then owns + # before owner, so coowner is the earliest live owner. + for user, minute in ((svc, 0), (self.coowner, 1), (self.owner, 2)): + membership = wf.memberships.create(user=user, role=ResourceRole.OWNER) + type(membership).objects.filter(pk=membership.pk).update( + created_at=BASE_TIME + timedelta(minutes=minute) + ) + + assert type(wf).objects.get(pk=wf.pk).owner_email() == self.coowner.email + + wf.memberships.all().delete() + assert type(wf).objects.get(pk=wf.pk).owner_email() is None 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/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index 74678a933b..002996ee19 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -107,17 +107,16 @@ def get_queryset(self) -> QuerySet: WorkflowKey.WF_IS_ACTIVE, WorkflowKey.WF_NAME, ) - # Use for_user method to include shared workflows - queryset = ( - Workflow.objects.for_user(self.request.user).filter(**filter_args) - if filter_args - else Workflow.objects.for_user(self.request.user) - ) - # Avoid per-row queries for owner/co-owner + creator fields in list views + # Use for_user to include shared workflows; prefetch owner/co-owner + # joins to avoid per-row queries in the Owned By column. + queryset = Workflow.objects.for_user(self.request.user) + if filter_args: + queryset = queryset.filter(**filter_args) queryset = queryset.select_related("created_by").prefetch_related( "memberships__user" ) + # Name search. search = self.request.query_params.get("search") if search: queryset = queryset.filter(workflow_name__icontains=search) 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..ba3c0f1a4b 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -1,24 +1,33 @@ 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 { + applyPagedResponse, + buildPagedParams, + usePaginatedList, +} from "../../../hooks/usePaginatedList"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; 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"; 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 +61,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 +73,10 @@ 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(); + // 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] = @@ -73,6 +84,10 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { const [isPermissionEdit, setIsPermissionEdit] = useState(false); const [isShareLoading, setIsShareLoading] = useState(false); const [allUserList, setAllUserList] = useState([]); + const [allGroupList, setAllGroupList] = useState([]); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); + const promptStudioCoOwnerService = useMemo( () => ({ getAllUsers: () => @@ -107,61 +122,110 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { ); const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + pagination, + setPagination, + searchTerm, + setSearchTerm, + sort, + fetchRef, + requestList, + syncRequested, + handlePaginationChange, + handleSearch, + handleSortChange, + handleListRefresh, + } = usePaginatedList({ + defaultPageSize: DEFAULT_PAGE_SIZE, + defaultSortBy: "modified_at", + defaultOrder: "desc", + }); + + const coOwner = 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 = buildPagedParams({ + page, + pageSize, + search, + sortBy, + order, + }); + const seq = ++seqRef.current; + setLoadError(false); + setIsLoading(true); + return axiosPrivate({ + method: "GET", + url: `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/`, + headers: { "X-CSRFToken": sessionDetails?.csrfToken }, + params, + }) + .then((res) => + applyPagedResponse({ + data: res?.data, + page, + pageSize, + seq, + latestSeqRef: seqRef, + setList: setDisplayList, + setPagination, + refetchPrevPage: () => + requestList(page - 1, pageSize, search, sortBy, order), + }), + ) + .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"), + ); + // 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. + if (seq === seqRef.current) { + setIsLoading(false); + } + }); + }, + [ + sessionDetails?.orgId, + sessionDetails?.csrfToken, + axiosPrivate, + setPagination, + setAlertDetails, + handleException, + ], + ); + fetchRef.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); + requestList(1, DEFAULT_PAGE_SIZE, "", sort.sortBy, sort.order); + // 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 +242,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 +255,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 +274,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 +345,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 +355,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 +376,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { .then((res) => { setOpenSharePermissionModal(true); setPromptDetails(res?.data); - setIsPermissionEdit(isEdit); + setIsPermissionEdit(isEditShare); }) .catch((err) => { setAlertDetails(handleException(err)); @@ -407,30 +437,9 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { }; const handleCoOwner = (_event, tool) => { - handleCoOwnerAction(tool.tool_id); + coOwner.handleCoOwner(tool.tool_id); }; - const defaultContent = ( -
- -
- ); - const customButtonsElement = useMemo( () => ( handleSearch(value)} customButtons={customButtonsElement} segmentOptions={segmentOptions} segmentValue={segmentValue} segmentFilter={onSegmentChange} />
-
{defaultContent}
+
+
+ {loadError && ( + + )} + {!loadError && displayList === undefined && } + {!loadError && displayList?.length === 0 && !searchTerm && ( + + )} + {!loadError && displayList?.length === 0 && searchTerm && ( + + )} + {!loadError && displayList?.length > 0 && ( + + )} +
+
{openAddTool && ( - + ); } diff --git a/frontend/src/components/custom-tools/view-tools/ViewTools.css b/frontend/src/components/custom-tools/view-tools/ViewTools.css deleted file mode 100644 index 329bdd21cf..0000000000 --- a/frontend/src/components/custom-tools/view-tools/ViewTools.css +++ /dev/null @@ -1 +0,0 @@ -/* Styles for ViewTools */ diff --git a/frontend/src/components/custom-tools/view-tools/ViewTools.jsx b/frontend/src/components/custom-tools/view-tools/ViewTools.jsx deleted file mode 100644 index d5dca62837..0000000000 --- a/frontend/src/components/custom-tools/view-tools/ViewTools.jsx +++ /dev/null @@ -1,91 +0,0 @@ -import PropTypes from "prop-types"; - -import { ListView } from "../../widgets/list-view/ListView"; -import { SpinnerLoader } from "../../widgets/spinner-loader/SpinnerLoader.jsx"; -import "./ViewTools.css"; -import { EmptyState } from "../../widgets/empty-state/EmptyState.jsx"; - -function ViewTools({ - isLoading, - isEmpty, - listOfTools, - setOpenAddTool, - handleEdit, - handleDelete, - titleProp, - descriptionProp, - iconProp, - idProp, - centered, - isClickable = true, - handleShare, - handleCoOwner, - showOwner, - showModified, - type, -}) { - if (isLoading) { - return ; - } - - 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/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 04f41d7694..baccbd4980 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -1,23 +1,29 @@ 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 { + applyPagedResponse, + buildPagedParams, + 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 { 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"; import { SharePermission } from "../../widgets/share-permission/SharePermission"; +import { SpinnerLoader } from "../../widgets/spinner-loader/SpinnerLoader.jsx"; import "./ToolSettings.css"; const titles = { @@ -36,6 +42,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 +56,16 @@ 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(); + // Fetch failure (vs. genuinely empty) — drives a retryable error state. + const [loadError, setLoadError] = useState(false); const { sessionDetails } = useSessionStore(); const { setAlertDetails } = useAlertStore(); const axiosPrivate = useAxiosPrivate(); const handleException = useExceptionHandler(); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const adapterCoOwnerService = useMemo( () => ({ @@ -87,95 +101,139 @@ function ToolSettings({ type }) { ); const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + pagination, + setPagination, + searchTerm, + sort, + fetchRef, + requestList, + resetList, + syncRequested, + handlePaginationChange, + handleSearch, + handleSortChange, + handleListRefresh, + } = usePaginatedList({ + defaultPageSize: DEFAULT_PAGE_SIZE, + defaultSortBy: "modified_at", + defaultOrder: "desc", + }); + + const coOwner = 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 = buildPagedParams({ + page, + pageSize, + search, + sortBy, + order, + }); + params.adapter_type = type.toUpperCase(); + const seq = ++seqRef.current; + setLoadError(false); + setIsLoading(true); + return axiosPrivate({ + method: "GET", + url: `/api/v1/unstract/${sessionDetails?.orgId}/adapter`, + params, + }) + .then((res) => + applyPagedResponse({ + data: res?.data, + page, + pageSize, + seq, + latestSeqRef: seqRef, + setList: setDisplayList, + setPagination, + refetchPrevPage: () => + requestList(page - 1, pageSize, search, sortBy, order), + }), + ) + .catch((err) => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { + return; + } + 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. + if (seq === seqRef.current) { + setIsLoading(false); + } + }); + }, + [ + type, + sessionDetails?.orgId, + axiosPrivate, + setPagination, + setAlertDetails, + handleException, + ], + ); + fetchRef.current = getAdapters; useEffect(() => { - clearSearch(); - setMasterList([]); + setDisplayList(undefined); if (!type) { return; } - getAdapters(); + // 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]); - 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 = { + // 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, - }, - }; + 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 +326,9 @@ function ToolSettings({ type }) { }; const handleCoOwner = (_event, adapter) => { - if (!adapter?.id) return; + if (!adapter?.id) { + return; + } if (adapter?.is_deprecated) { setAlertDetails({ type: "error", @@ -276,7 +336,7 @@ function ToolSettings({ type }) { }); return; } - handleCoOwnerAction(adapter.id); + coOwner.handleCoOwner(adapter.id); }; const handleOpenAddSourceModal = () => { @@ -297,8 +357,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" - /> + {loadError && ( + + )} + {!loadError && displayList === undefined && } + {!loadError && displayList?.length === 0 && !searchTerm && ( + + )} + {!loadError && displayList?.length === 0 && searchTerm && ( + + )} + {!loadError && displayList?.length > 0 && ( + + )}
@@ -364,18 +434,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/widgets/list-view/ListView.css b/frontend/src/components/widgets/list-view/ListView.css deleted file mode 100644 index 7ea9b43bb6..0000000000 --- a/frontend/src/components/widgets/list-view/ListView.css +++ /dev/null @@ -1,180 +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; - /* must not shrink: its columns are fixed below, so a shrinking box would - let them spill over the action icons. The left column absorbs the - shortfall instead — it ellipsizes, this doesn't. */ - flex: 0 0 auto; - gap: 20px; -} - -/* The meta cluster is anchored on its right edge (the left column grows to - absorb the row's slack), so any column that sizes to its content pushes - the ones before it sideways, row by row. Fixed widths keep every row's - columns at the same x. */ -.list-view-meta .adapters-list-profile-container { - flex: 0 0 240px; - justify-content: flex-start; -} - -.list-view-meta .list-view-modified-container { - flex: 0 0 170px; -} - -.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; - /* match the agentic Prompt Studio project list */ - font-weight: 600; -} - -.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..5424317cc8 --- /dev/null +++ b/frontend/src/components/widgets/resource-table/ResourceTable.css @@ -0,0 +1,176 @@ +/* Styles for the sortable resource list table + (Name / Owned By / Created / Modified / 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; +} + +/* PS-only "Prompts: N" meta line under the project name */ +.resource-table-name-meta { + 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: 11px; + font-weight: 600; + 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..3aa79849cc --- /dev/null +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -0,0 +1,412 @@ +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 { formattedDateTime, timeAgo } from "../../../helpers/GetStaticData"; +import "./ResourceTable.css"; + +// Stable, distinct avatar swatch per owner (seeded on email/name) like the +// design: a light pastel fill paired with a matching darker initial. +const AVATAR_COLORS = [ + { bg: "#ffccc7", fg: "#f5222d" }, + { bg: "#ffe7ba", fg: "#fa8c16" }, + { bg: "#fff1b8", fg: "#faad14" }, + { bg: "#d9f7be", fg: "#52c41a" }, + { bg: "#b5f5ec", fg: "#13c2c2" }, + { bg: "#bae0ff", fg: "#1677ff" }, + { bg: "#efdbff", fg: "#722ed1" }, + { bg: "#ffd6e7", fg: "#eb2f96" }, +]; +const colorForSeed = (seed = "") => { + let hash = 0; + for (let i = 0; i < seed.length; i += 1) { + hash = seed.codePointAt(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: }, + ], +}; + +/** + * 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 / Modified / 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", + modifiedProp = "modified_at", + ownerEmailProp = "created_by_email", + countProp, + countLabel, + handleEdit, + handleShare, + handleDelete, + handleCoOwner, + sessionDetails, + showOwner = true, + isClickable = true, + type, +}) { + const navigate = useNavigate(); + + const renderName = (item) => { + const icon = iconProp ? item?.[iconProp] : null; + // 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 && + (isImage ? ( + + ) : ( + {icon} + ))} +
+ + {item?.[titleProp]} + + {descriptionProp && item?.[descriptionProp] && ( + + {item[descriptionProp]} + + )} + {countProp && item?.[countProp] != null && ( + + {countLabel}: {item[countProp]} + + )} +
+
+ ); + }; + + const renderOwner = (item) => { + const email = item?.[ownerEmailProp]; + // "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}` : ""; + const initials = (email || name).slice(0, 2).toUpperCase(); + const swatch = colorForSeed(email || name); + + 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: "34%", + render: (_, item) => renderName(item), + }, + showOwner && { + title: Owned By, + key: "owner", + width: "22%", + render: (_, item) => renderOwner(item), + }, + { + title: ( + + ), + key: "created", + width: "15%", + render: (_, item) => formattedDateTime(item?.[dateProp]) || "-", + }, + { + title: ( + + ), + key: "modified", + width: "15%", + render: (_, item) => { + const iso = item?.[modifiedProp]; + const rel = timeAgo(iso); + return rel ? ( + {rel} + ) : ( + "-" + ); + }, + }, + { + title: Actions, + key: "actions", + width: "14%", + 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, + modifiedProp: PropTypes.string, + ownerEmailProp: PropTypes.string, + countProp: PropTypes.string, + countLabel: 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.css b/frontend/src/components/workflows/workflow/Workflows.css index 7a407a2fec..f72d8001d1 100644 --- a/frontend/src/components/workflows/workflow/Workflows.css +++ b/frontend/src/components/workflows/workflow/Workflows.css @@ -67,13 +67,6 @@ width: fit-content; } -.listWrapper { - height: 92%; - overflow: hidden auto; - margin-block-start: 12px; - margin-inline: 16px; -} - .listItem { display: grid; grid-auto-flow: row; diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 7d5567c36f..40d104a856 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -1,12 +1,16 @@ 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 { useEffect, useRef, useState } from "react"; 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, @@ -18,13 +22,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 { 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"; +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"; @@ -50,11 +54,16 @@ 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); + // 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); - // 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(); @@ -69,34 +78,20 @@ function Workflows() { pagination, setPagination, searchTerm, + sort, + fetchRef, + requestList, + syncRequested, handlePaginationChange, handleSearch, + handleSortChange, + handleListRefresh, } = usePaginatedList({ - fetchData: (...args) => fetchListRef.current?.(...args), defaultPageSize: DEFAULT_PAGE_SIZE, + defaultSortBy: "modified_at", + defaultOrder: "desc", }); - - // Refresh the current page (preserves page + active search) after mutations - const handleListRefresh = useCallback( - () => - fetchListRef.current?.( - pagination.current, - pagination.pageSize, - searchTerm, - ), - [pagination.current, pagination.pageSize, searchTerm], - ); - const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + const coOwner = useCoOwnerManagement({ service: projectApiService, setAlertDetails, onListRefresh: handleListRefresh, @@ -107,7 +102,7 @@ function Workflows() { useEffect(() => { if (location.pathname === `/${orgName}/workflows`) { - getProjectList(); + requestList(1, DEFAULT_PAGE_SIZE, "", sort.sortBy, sort.order); } }, [location.pathname]); @@ -115,46 +110,53 @@ function Workflows() { page = 1, pageSize = DEFAULT_PAGE_SIZE, search = "", + sortBy = "", + order = "asc", ) => { + const params = buildPagedParams({ page, pageSize, search, sortBy, order }); + const seq = ++seqRef.current; + setLoadError(false); setLoading(true); - const params = { page, page_size: pageSize }; - if (search) { - params.search = search; - } - 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); + .then((res) => + applyPagedResponse({ + data: res?.data, + page, + pageSize, + seq, + latestSeqRef: seqRef, + setList: setProjectList, + setPagination, + refetchPrevPage: () => + requestList(page - 1, pageSize, search, sortBy, order), + }), + ) + .catch((err) => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { return; } - setProjectList(results); - setPagination((prev) => ({ - ...prev, - current: page, - pageSize, - total, - })); - }) - .catch(() => { - console.error("Unable to get project list"); - // Avoid an indefinite spinner when the first fetch fails. - setProjectList((prev) => prev ?? []); + 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. + syncRequested(); }) .finally(() => { - setLoading(false); + // Only the newest request owns the shared loading state. + if (seq === seqRef.current) { + setLoading(false); + } }); }; - fetchListRef.current = getProjectList; + 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) => { @@ -178,7 +180,7 @@ function Workflows() { ); }) .finally(() => { - setLoading(false); + setEditLoading(false); }); } @@ -360,7 +362,7 @@ function Workflows() { const handleCoOwner = (event, workflow) => { event.stopPropagation(); - handleCoOwnerAction(workflow.id); + coOwner.handleCoOwner(workflow.id); }; const handleNewWorkflowBtnClick = () => { @@ -406,8 +408,15 @@ function Workflows() { />
- {projectList === undefined && } - {projectList?.length === 0 && !searchTerm && ( + {loadError && ( + + )} + {!loadError && projectList === undefined && } + {!loadError && projectList?.length === 0 && !searchTerm && (
)} - {projectList?.length === 0 && searchTerm && ( + {!loadError && projectList?.length === 0 && searchTerm && ( )} - {projectList?.length > 0 && ( - <> - - {pagination.total > pagination.pageSize && ( -
- -
- )} - + {!loadError && projectList?.length > 0 && ( + )} {editingProject && ( )} - {coOwnerOpen && ( - + {coOwner.coOwnerOpen && ( + )}
diff --git a/frontend/src/hooks/useListSearch.js b/frontend/src/hooks/useListSearch.js deleted file mode 100644 index f74d96718c..0000000000 --- a/frontend/src/hooks/useListSearch.js +++ /dev/null @@ -1,61 +0,0 @@ -import { useCallback, useRef, useState } from "react"; - -function useListSearch(searchField) { - const listRef = useRef([]); - const searchTextRef = useRef(""); - const [displayList, setDisplayList] = useState([]); - - const filterList = useCallback( - (list, searchText) => { - if (!searchText.trim()) { - return list; - } - return list.filter((item) => - item[searchField]?.toLowerCase().includes(searchText.toLowerCase()), - ); - }, - [searchField], - ); - - const setMasterList = useCallback( - (list) => { - listRef.current = list; - setDisplayList(filterList(list, searchTextRef.current)); - }, - [filterList], - ); - - const onSearch = useCallback( - (searchText, setSearchList) => { - searchTextRef.current = searchText; - setSearchList(filterList(listRef.current, searchText)); - }, - [filterList], - ); - - const clearSearch = useCallback(() => { - searchTextRef.current = ""; - setDisplayList(listRef.current); - }, []); - - const updateMasterList = useCallback( - (updateFn) => { - const updatedList = updateFn(listRef.current); - listRef.current = updatedList; - setDisplayList(filterList(updatedList, searchTextRef.current)); - }, - [filterList], - ); - - return { - listRef, - displayList, - setDisplayList, - setMasterList, - updateMasterList, - onSearch, - clearSearch, - }; -} - -export { useListSearch }; diff --git a/frontend/src/hooks/usePaginatedList.js b/frontend/src/hooks/usePaginatedList.js index ce45ffa3f4..316b0f70e0 100644 --- a/frontend/src/hooks/usePaginatedList.js +++ b/frontend/src/hooks/usePaginatedList.js @@ -1,44 +1,204 @@ -import { useRef, useState } from "react"; +import { useCallback, 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) { + // DRF OrderingFilter: `?ordering=field`, a leading `-` sorts descending. + params.ordering = (order === "desc" ? "-" : "") + sortBy; + } + 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. * - * @param {Object} options - * @param {Function} options.fetchData - fn(page, pageSize, search) to fetch data + * 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] - * @return {Object} Pagination state and handlers + * @param {string} [options.defaultSortBy=""] - initial sort column key + * @param {string} [options.defaultOrder="asc"] - initial sort direction + * @return {Object} Pagination/sort state, `fetchRef`, and handlers. */ -function usePaginatedList({ fetchData, defaultPageSize = 10 }) { +function usePaginatedList({ + defaultPageSize = 10, + defaultSortBy = "", + defaultOrder = "asc", +} = {}) { const [pagination, setPagination] = useState({ current: 1, pageSize: defaultPageSize, total: 0, }); const [searchTerm, setSearchTerm] = useState(""); + const [sort, setSort] = useState({ + sortBy: defaultSortBy, + 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); + + // page/pageSize mirror the applied view; search/sort are last-REQUESTED + // (handlers commit before firing). syncRequested snapshots this for refresh. + 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 (state lags applies). + // handleListRefresh replays these so a refresh targets the asked-for view. + 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. Returns the fetch + // promise so a stepback propagates up through applyPagedResponse. + const requestList = (page, pageSize, search, sortBy, order) => { + requestedRef.current = { page, pageSize, search, sortBy, order }; + return fetchRef.current?.(page, pageSize, search, sortBy, order); + }; const handlePaginationChange = (page, pageSize) => { const newPage = pageSize === pagination.pageSize ? page : 1; - fetchRef.current?.(newPage, pageSize, searchTerm); + requestList(newPage, pageSize, searchTerm, sort.sortBy, sort.order); }; const handleSearch = (searchText) => { const term = searchText?.trim() || ""; setSearchTerm(term); - fetchRef.current?.(1, pagination.pageSize, term); + requestList(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) => { + // "Clear Sort" (empty sortBy) restores the default view, not an empty + // ordering: the viewset always applies one, so header and rows stay in sync. + const nextSort = sortBy + ? { sortBy, order: order || "asc" } + : { sortBy: defaultSortBy, order: defaultOrder }; + setSort(nextSort); + requestList( + 1, + pagination.pageSize, + searchTerm, + nextSort.sortBy, + nextSort.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 { page, pageSize, search, sortBy, order } = requestedRef.current; + fetchRef.current?.(page, pageSize, search, sortBy, order); + }, []); + return { pagination, setPagination, searchTerm, setSearchTerm, + sort, + fetchRef, + requestList, + resetList, + syncRequested, handlePaginationChange, handleSearch, + handleSortChange, + handleListRefresh, }; } -export { usePaginatedList }; +export { applyPagedResponse, buildPagedParams, usePaginatedList }; diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index cacb50fc27..3a28487997 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -1,22 +1,30 @@ 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 { 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"; +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 { + applyPagedResponse, + buildPagedParams, + 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 +35,10 @@ 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(); + // Fetch failure (vs. genuinely empty) — drives a retryable error state. + const [loadError, setLoadError] = useState(false); const groupsApi = groupsService(); const axiosPrivate = useAxiosPrivate(); @@ -34,6 +46,8 @@ function ConnectorsPage() { const { setAlertDetails } = useAlertStore(); const handleException = useExceptionHandler(); const { getUrl } = useRequestUrl(); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const connectorCoOwnerService = useMemo( () => ({ @@ -62,40 +76,90 @@ function ConnectorsPage() { ); const { - coOwnerOpen, - setCoOwnerOpen, - coOwnerData, - coOwnerLoading, - coOwnerAllUsers, - coOwnerResourceId, - handleCoOwner: handleCoOwnerAction, - onAddCoOwner, - onRemoveCoOwner, - } = useCoOwnerManagement({ + pagination, + setPagination, + searchTerm, + sort, + fetchRef, + requestList, + syncRequested, + handlePaginationChange, + handleSearch, + handleSortChange, + handleListRefresh, + } = usePaginatedList({ + defaultPageSize: DEFAULT_PAGE_SIZE, + defaultSortBy: "modified_at", + defaultOrder: "desc", + }); + + const coOwner = 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 = buildPagedParams({ + page, + pageSize, + search, + sortBy, + order, + }); + const seq = ++seqRef.current; + setLoadError(false); + setLoading(true); + return axiosPrivate + .get(getUrl("connector/"), { params }) + .then((res) => + applyPagedResponse({ + data: res?.data, + page, + pageSize, + seq, + latestSeqRef: seqRef, + setList: setDisplayList, + setPagination, + refetchPrevPage: () => + requestList(page - 1, pageSize, search, sortBy, order), + }), + ) + .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")); + // 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. + if (seq === seqRef.current) { + setLoading(false); + } + }); + }, + [axiosPrivate, getUrl, setPagination, setAlertDetails, handleException], + ); + fetchRef.current = getConnectors; useEffect(() => { - fetchConnectors(); + requestList(1, DEFAULT_PAGE_SIZE, "", sort.sortBy, sort.order); 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 +198,7 @@ function ConnectorsPage() { type: "success", content: "Connector deleted successfully", }); - fetchConnectors(); + handleListRefresh(); } catch (error) { setAlertDetails(handleException(error, "Failed to delete connector")); } @@ -204,14 +268,18 @@ function ConnectorsPage() { }; const handleCoOwner = (_event, connector) => { - if (!connector?.id) return; - handleCoOwnerAction(connector.id); + if (!connector?.id) { + return; + } + coOwner.handleCoOwner(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 +303,52 @@ function ConnectorsPage() { handleSearch(value)} customButtons={newConnectorButton} />
- + {loadError && ( + + )} + {!loadError && displayList === undefined && } + {!loadError && displayList?.length === 0 && !searchTerm && ( + + )} + {!loadError && displayList?.length === 0 && searchTerm && ( + + )} + {!loadError && displayList?.length > 0 && ( + + )}
- + ); } From d28123aa7247910025ceb6d090fdbebce58fb2f3 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 14:08:28 +0530 Subject: [PATCH 05/17] UN-3770 [FIX] Address review: deterministic owner, keyboard row actions, payload guard - owner_email(): break created_at ties by pk so the "Owned By" label is stable across requests; cover equal timestamps in the shared test. - ResourceTable row actions rendered as non-focusable icon spans; wrap in real buttons so edit/share/delete are keyboard reachable and Popconfirm gets a focusable trigger. - applyPagedResponse: guard non-array payloads (204 body, stray object) before they reach antd Table dataSource. - Client-ordering test: two modified_at groups so honored ascending order is distinguishable from the -modified_at default, not just the pk tie. - Drop redundant "# Name search." comments on the name-only search blocks. Co-Authored-By: Claude Opus 4.8 --- backend/adapter_processor_v2/views.py | 1 - backend/connector_v2/views.py | 1 - backend/permissions/models.py | 3 +- backend/utils/tests/test_list_pagination.py | 31 ++++++++++++----- backend/workflow_manager/workflow_v2/views.py | 1 - .../widgets/resource-table/ResourceTable.css | 15 ++++++-- .../widgets/resource-table/ResourceTable.jsx | 34 +++++++++++++------ frontend/src/hooks/usePaginatedList.js | 4 ++- 8 files changed, 64 insertions(+), 26 deletions(-) diff --git a/backend/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index 94b2d02d1d..5222f751e9 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -193,7 +193,6 @@ def get_queryset(self) -> QuerySet | None: ): queryset = queryset.filter(**filter_args) - # Name search. search = self.request.query_params.get("search") if search: queryset = queryset.filter(adapter_name__icontains=search) diff --git a/backend/connector_v2/views.py b/backend/connector_v2/views.py index 90092a5846..3f5e308045 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -126,7 +126,6 @@ def get_queryset(self) -> QuerySet | None: ) queryset = queryset.none() - # Name search. search = self.request.query_params.get("search") if search: queryset = queryset.filter(connector_name__icontains=search) diff --git a/backend/permissions/models.py b/backend/permissions/models.py index e800847b5f..0ae0d87892 100644 --- a/backend/permissions/models.py +++ b/backend/permissions/models.py @@ -54,7 +54,8 @@ def owner_email(self) -> str | None: ] if not owners: return None - return min(owners, key=lambda m: m.created_at).user.email + # pk breaks created_at ties so the label is stable across requests. + return min(owners, key=lambda m: (m.created_at, m.pk)).user.email def is_owner(self, user: Any) -> bool: if user is None: diff --git a/backend/utils/tests/test_list_pagination.py b/backend/utils/tests/test_list_pagination.py index 7984f2ff2b..d633760dca 100644 --- a/backend/utils/tests/test_list_pagination.py +++ b/backend/utils/tests/test_list_pagination.py @@ -164,20 +164,23 @@ def test_pages_partition_the_result_set_newest_first(self) -> None: def test_client_ordering_keeps_pk_tiebreaker(self) -> None: """``?ordering=`` replaces the view default, so pk must still be appended. - Every row here shares one ``modified_at``. Primary keys are random - UUIDs, so ordering by pk is unrelated to insertion order — the returned - sequence only matches if pk survived as the tie-breaker. + Two ``modified_at`` groups with ties in each: an ascending client order + must list the older group first — distinguishing it from the + ``-modified_at`` default — and break each in-group tie by pk. Primary + keys are random UUIDs unrelated to insertion order, so the sequence only + matches if pk survived as the tie-breaker. """ for endpoint in LIST_ENDPOINTS: with self.subTest(kind=endpoint.kind): created = [] - for i in range(6): - obj = self._create(endpoint, f"{endpoint.kind}-tied-{i}") - self._stamp(obj, BASE_TIME) - created.append(obj) + for group in range(2): + for i in range(3): + obj = self._create(endpoint, f"{endpoint.kind}-g{group}-{i}") + self._stamp(obj, BASE_TIME + timedelta(minutes=group)) + created.append((group, obj)) expected = [ getattr(obj, endpoint.name_field) - for obj in sorted(created, key=lambda o: str(o.pk)) + for _, obj in sorted(created, key=lambda t: (t[0], str(t[1].pk))) ] pages = [ @@ -300,5 +303,17 @@ def test_owner_email_is_earliest_live_owner(self) -> None: assert type(wf).objects.get(pk=wf.pk).owner_email() == self.coowner.email + # Tied created_at: pk decides, so the label never flips between requests. + wf.memberships.all().delete() + tied = [ + wf.memberships.create(user=user, role=ResourceRole.OWNER) + for user in (self.owner, self.coowner) + ] + type(tied[0]).objects.filter(pk__in=[m.pk for m in tied]).update( + created_at=BASE_TIME + ) + earliest = min(tied, key=lambda m: m.pk) + assert type(wf).objects.get(pk=wf.pk).owner_email() == earliest.user.email + wf.memberships.all().delete() assert type(wf).objects.get(pk=wf.pk).owner_email() is None diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index 002996ee19..ada8d4f426 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -116,7 +116,6 @@ def get_queryset(self) -> QuerySet: "memberships__user" ) - # Name search. search = self.request.query_params.get("search") if search: queryset = queryset.filter(workflow_name__icontains=search) diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.css b/frontend/src/components/widgets/resource-table/ResourceTable.css index 5424317cc8..ae8563bafb 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.css +++ b/frontend/src/components/widgets/resource-table/ResourceTable.css @@ -165,12 +165,23 @@ justify-content: flex-end; } -.resource-table .disabled-icon { +.resource-table .action-icon-btn { + background: none; + border: none; + padding: 0; + margin: 0; + font: inherit; + color: inherit; + cursor: pointer; + display: inline-flex; +} + +.resource-table .action-icon-btn:disabled { opacity: 0.4; cursor: not-allowed; } /* Destructive action is red, per the design */ -.resource-table .delete-icon:not(.disabled-icon) { +.resource-table .delete-icon { color: #ff4d4f; } diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 3aa79849cc..e70e82b196 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -252,21 +252,27 @@ function ResourceTable({ role="none" > - !deprecated && handleEdit?.(event, item)} - /> + > + + {handleShare && ( - !deprecated && handleShare(event, item, true)} - /> + > + + )} } onConfirm={(event) => handleDelete?.(event, item)} > - + ); diff --git a/frontend/src/hooks/usePaginatedList.js b/frontend/src/hooks/usePaginatedList.js index 316b0f70e0..bd5d26b9cb 100644 --- a/frontend/src/hooks/usePaginatedList.js +++ b/frontend/src/hooks/usePaginatedList.js @@ -59,7 +59,9 @@ function applyPagedResponse({ if (seq !== latestSeqRef.current) { return undefined; } - const results = data?.results ?? data ?? []; + // A 204 body or an unexpected object must not reach antd Table as dataSource. + const raw = data?.results ?? data; + const results = Array.isArray(raw) ? raw : []; 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) { From 8fc79162d7d7615f2257c25b4eeb69a14f68bc8c Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 14:11:08 +0530 Subject: [PATCH 06/17] UN-3770 [FIX] Use aria-disabled on row actions so the deprecated tooltip survives A native disabled button suppresses hover, hiding the "deprecated" tooltip. aria-disabled keeps the control focusable and hoverable; the onClick guard already no-ops when deprecated. Co-Authored-By: Claude Opus 4.8 --- .../src/components/widgets/resource-table/ResourceTable.css | 2 +- .../src/components/widgets/resource-table/ResourceTable.jsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.css b/frontend/src/components/widgets/resource-table/ResourceTable.css index ae8563bafb..cbbe1a0af0 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.css +++ b/frontend/src/components/widgets/resource-table/ResourceTable.css @@ -176,7 +176,7 @@ display: inline-flex; } -.resource-table .action-icon-btn:disabled { +.resource-table .action-icon-btn[aria-disabled="true"] { opacity: 0.4; cursor: not-allowed; } diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index e70e82b196..ab8846dbdc 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -256,7 +256,7 @@ function ResourceTable({ type="button" className="action-icon-btn" aria-label={`Edit ${type}`} - disabled={deprecated} + aria-disabled={deprecated} onClick={(event) => !deprecated && handleEdit?.(event, item)} > @@ -268,7 +268,7 @@ function ResourceTable({ type="button" className="action-icon-btn" aria-label={`Share ${type}`} - disabled={deprecated} + aria-disabled={deprecated} onClick={(event) => !deprecated && handleShare(event, item, true)} > From 3550f421a3bb9efa0ef84665575505cb4b7d8bc0 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 16:16:25 +0530 Subject: [PATCH 07/17] UN-3770 [FIX] Polish resource list table: sort affordance + layout - Don't highlight the default sort column on load; the header lights up only once the user explicitly picks a sort. usePaginatedList tracks a `userSorted` flag threaded to ResourceTable/SortHeader. - Name column absorbs the slack while Owned By/Created/Modified/Actions share one compact fixed width, so they read as an evenly-spaced group and Name stays dominant. - Owner name/email ellipsize within their cell (drop the 190px cap, let the Space item shrink), removing the trailing-gap skew. - Table scrolls inside its own container below its min-width instead of crushing columns on narrow screens. - Created timestamp gets ellipsis+tooltip as a safety for long values. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- .../list-of-tools/ListOfTools.jsx | 2 + .../tool-settings/ToolSettings.jsx | 2 + .../widgets/resource-table/ResourceTable.css | 20 ++++++--- .../widgets/resource-table/ResourceTable.jsx | 43 +++++++++++++++---- .../workflows/workflow/Workflows.jsx | 2 + frontend/src/hooks/usePaginatedList.js | 7 +++ frontend/src/pages/ConnectorsPage.jsx | 2 + 7 files changed, 65 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 ba3c0f1a4b..86ffc741e3 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -127,6 +127,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { searchTerm, setSearchTerm, sort, + userSorted, fetchRef, requestList, syncRequested, @@ -489,6 +490,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { loading={isLoading} pagination={pagination} sort={sort} + userSorted={userSorted} onPaginationChange={handlePaginationChange} onSortChange={handleSortChange} titleProp="tool_name" diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index baccbd4980..10e2cc850a 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -105,6 +105,7 @@ function ToolSettings({ type }) { setPagination, searchTerm, sort, + userSorted, fetchRef, requestList, resetList, @@ -395,6 +396,7 @@ function ToolSettings({ type }) { loading={isLoading} pagination={pagination} sort={sort} + userSorted={userSorted} onPaginationChange={handlePaginationChange} onSortChange={handleSortChange} titleProp="adapter_name" diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.css b/frontend/src/components/widgets/resource-table/ResourceTable.css index cbbe1a0af0..ae913d3bf7 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.css +++ b/frontend/src/components/widgets/resource-table/ResourceTable.css @@ -2,13 +2,19 @@ (Name / Owned By / Created / Modified / 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. */ + /* Name has no fixed width and absorbs the slack; the other columns are a + fixed compact width (set in the JSX) so they stay evenly spaced. */ width: 100%; padding: 0 4px 24px 4px; } -/* Let wide content scroll inside the table instead of the page body */ +/* Below this the fixed columns would crush, so the table scrolls inside its + own container instead. overflow-x on the content is the scroll boundary, so + the min-width never pushes the page wider. */ +.resource-table table { + min-width: 1040px; +} + .resource-table .ant-table-content { overflow-x: auto; } @@ -119,6 +125,12 @@ /* Owned By column: avatar + name/email, clickable to manage co-owners */ .resource-table-owner { min-width: 0; + max-width: 100%; +} + +/* Let the name/email item shrink so it ellipsizes within the cell */ +.resource-table-owner .ant-space-item { + min-width: 0; } .resource-table-owner-avatar { @@ -136,12 +148,10 @@ .resource-table-owner-name { font-weight: 500; - max-width: 190px; } .resource-table-owner-email { font-size: 12px; - max-width: 190px; } .resource-table-owner-btn { diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index ab8846dbdc..ac04cd7a01 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -44,6 +44,11 @@ const colorForSeed = (seed = "") => { return AVATAR_COLORS[Math.abs(hash) % AVATAR_COLORS.length]; }; +// Owner/Created/Modified/Actions share one compact width so they read as an +// evenly-spaced group; Name has no width and absorbs the remaining space. Sized +// to fit a full timestamp on one line. +const COMPACT_COL_WIDTH = 210; + // Sort-menu wording differs for text vs date columns (per the design). const SORT_OPTIONS = { text: [ @@ -61,8 +66,16 @@ const SORT_OPTIONS = { * 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; +function SortHeader({ + label, + sortKey, + sortType = "text", + sort, + userSorted, + onSortChange, +}) { + // Don't light up the default sort column on load — only once the user picks. + const active = userSorted && sort?.sortBy === sortKey; const items = [ ...SORT_OPTIONS[sortType], { type: "divider" }, @@ -100,6 +113,7 @@ SortHeader.propTypes = { sortKey: PropTypes.string.isRequired, sortType: PropTypes.oneOf(["text", "date"]), sort: PropTypes.object, + userSorted: PropTypes.bool, onSortChange: PropTypes.func, }; @@ -113,6 +127,7 @@ function ResourceTable({ loading, pagination, sort, + userSorted, onPaginationChange, onSortChange, titleProp, @@ -303,17 +318,17 @@ function ResourceTable({ sortKey={titleProp} sortType="text" sort={sort} + userSorted={userSorted} onSortChange={onSortChange} /> ), key: "name", - width: "34%", render: (_, item) => renderName(item), }, showOwner && { title: Owned By, key: "owner", - width: "22%", + width: COMPACT_COL_WIDTH, render: (_, item) => renderOwner(item), }, { @@ -323,12 +338,22 @@ function ResourceTable({ sortKey={dateProp} sortType="date" sort={sort} + userSorted={userSorted} onSortChange={onSortChange} /> ), key: "created", - width: "15%", - render: (_, item) => formattedDateTime(item?.[dateProp]) || "-", + width: COMPACT_COL_WIDTH, + render: (_, item) => { + const formatted = formattedDateTime(item?.[dateProp]); + return formatted ? ( + + {formatted} + + ) : ( + "-" + ); + }, }, { title: ( @@ -337,11 +362,12 @@ function ResourceTable({ sortKey={modifiedProp} sortType="date" sort={sort} + userSorted={userSorted} onSortChange={onSortChange} /> ), key: "modified", - width: "15%", + width: COMPACT_COL_WIDTH, render: (_, item) => { const iso = item?.[modifiedProp]; const rel = timeAgo(iso); @@ -355,7 +381,7 @@ function ResourceTable({ { title: Actions, key: "actions", - width: "14%", + width: COMPACT_COL_WIDTH, align: "right", render: (_, item) => renderActions(item), }, @@ -400,6 +426,7 @@ ResourceTable.propTypes = { loading: PropTypes.bool, pagination: PropTypes.object, sort: PropTypes.object, + userSorted: PropTypes.bool, onPaginationChange: PropTypes.func, onSortChange: PropTypes.func, titleProp: PropTypes.string.isRequired, diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 40d104a856..1d5afeac24 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -79,6 +79,7 @@ function Workflows() { setPagination, searchTerm, sort, + userSorted, fetchRef, requestList, syncRequested, @@ -437,6 +438,7 @@ function Workflows() { loading={loading} pagination={pagination} sort={sort} + userSorted={userSorted} onPaginationChange={handlePaginationChange} onSortChange={handleSortChange} titleProp="workflow_name" diff --git a/frontend/src/hooks/usePaginatedList.js b/frontend/src/hooks/usePaginatedList.js index bd5d26b9cb..f90653ab81 100644 --- a/frontend/src/hooks/usePaginatedList.js +++ b/frontend/src/hooks/usePaginatedList.js @@ -101,6 +101,10 @@ function usePaginatedList({ sortBy: defaultSortBy, order: defaultOrder, }); + // The default sort still drives the server; this only gates the header's + // "active" styling so the default column isn't highlighted until the user + // explicitly picks a sort. + const [userSorted, setUserSorted] = useState(false); // Page assigns its fetch fn here; handlers call the latest one via the ref. const fetchRef = useRef(null); @@ -154,6 +158,7 @@ function usePaginatedList({ ? { sortBy, order: order || "asc" } : { sortBy: defaultSortBy, order: defaultOrder }; setSort(nextSort); + setUserSorted(Boolean(sortBy)); requestList( 1, pagination.pageSize, @@ -169,6 +174,7 @@ function usePaginatedList({ const resetList = () => { setSearchTerm(""); setSort({ sortBy: defaultSortBy, order: defaultOrder }); + setUserSorted(false); requestList(1, defaultPageSize, "", defaultSortBy, defaultOrder); }; @@ -192,6 +198,7 @@ function usePaginatedList({ searchTerm, setSearchTerm, sort, + userSorted, fetchRef, requestList, resetList, diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 3a28487997..e841cd8b9a 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -80,6 +80,7 @@ function ConnectorsPage() { setPagination, searchTerm, sort, + userSorted, fetchRef, requestList, syncRequested, @@ -332,6 +333,7 @@ function ConnectorsPage() { loading={loading} pagination={pagination} sort={sort} + userSorted={userSorted} onPaginationChange={handlePaginationChange} onSortChange={handleSortChange} titleProp="connector_name" From c1cc997f9973a36cf04cf5545a43dfa5ca4126aa Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 16:31:09 +0530 Subject: [PATCH 08/17] UN-3770 [FEAT] Search resource lists by owner too, not just name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner search matches the displayed owner — the OWNER membership that backs the Owned By column — via a search-time subquery (name OR owner email), across all four shared list endpoints. Reuses the sharing_helpers varchar/UUID object_id cast. `created_by` stays audit-only (UN-2202); service accounts and non-owner (VIEWER) members are excluded. This intentionally reverses the name-only narrowing from UN-3769: search now agrees with what the Owned By column shows, which was that change's goal. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- backend/adapter_processor_v2/views.py | 10 ++++++- backend/connector_v2/views.py | 10 ++++++- .../prompt_studio_core_v2/views.py | 13 +++++++-- backend/tenant_account_v2/sharing_helpers.py | 24 ++++++++++++++++ backend/utils/tests/test_list_pagination.py | 28 ++++++++++++------- backend/workflow_manager/workflow_v2/views.py | 10 ++++++- 6 files changed, 79 insertions(+), 16 deletions(-) diff --git a/backend/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index 5222f751e9..f2aef82b0c 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -195,7 +195,15 @@ def get_queryset(self) -> QuerySet | None: search = self.request.query_params.get("search") if search: - queryset = queryset.filter(adapter_name__icontains=search) + from django.db.models import Q + from tenant_account_v2.sharing_helpers import ( + resources_matching_owner_search, + ) + + queryset = queryset.filter( + Q(adapter_name__icontains=search) + | Q(pk__in=resources_matching_owner_search(queryset.model, search)) + ) return queryset diff --git a/backend/connector_v2/views.py b/backend/connector_v2/views.py index 3f5e308045..fd75b749db 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -128,7 +128,15 @@ def get_queryset(self) -> QuerySet | None: search = self.request.query_params.get("search") if search: - queryset = queryset.filter(connector_name__icontains=search) + from django.db.models import Q + from tenant_account_v2.sharing_helpers import ( + resources_matching_owner_search, + ) + + queryset = queryset.filter( + Q(connector_name__icontains=search) + | Q(pk__in=resources_matching_owner_search(queryset.model, search)) + ) return queryset diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index 7617b63ae3..cd65f2de77 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -180,7 +180,15 @@ def get_queryset(self) -> QuerySet | None: ) search = self.request.query_params.get("search") if search: - qs = qs.filter(tool_name__icontains=search) + from django.db.models import Q + from tenant_account_v2.sharing_helpers import ( + resources_matching_owner_search, + ) + + qs = qs.filter( + Q(tool_name__icontains=search) + | Q(pk__in=resources_matching_owner_search(qs.model, search)) + ) return qs def get_object(self): @@ -942,8 +950,7 @@ def task_status( # than a bare 500 that a status-code-keyed client would misread as a # terminal task failure. logger.exception( - "task_status: pg_task_result read failed for %s; treating as " - "pending", + "task_status: pg_task_result read failed for %s; treating as pending", task_id, ) row = None diff --git a/backend/tenant_account_v2/sharing_helpers.py b/backend/tenant_account_v2/sharing_helpers.py index 7420af0cfd..32c30d69ef 100644 --- a/backend/tenant_account_v2/sharing_helpers.py +++ b/backend/tenant_account_v2/sharing_helpers.py @@ -36,6 +36,7 @@ from django.db import models, transaction from django.db.models import Model, QuerySet from django.db.models.functions import Cast +from permissions.roles import ResourceRole from rest_framework.exceptions import ValidationError from utils.user_context import UserContext @@ -221,6 +222,29 @@ def resources_visible_via_memberships( return _object_id_subquery(qs, model) +def resources_matching_owner_search( + model: type[Model], term: str, organization: Organization | None = None +) -> QuerySet[Any]: + """Subquery of ``model`` PKs whose displayed owner matches ``term``. + + Owner search hits the same OWNER memberships that back the "Owned By" + column, so the search box agrees with what is shown. Matches the owner's + email (its local part is the shown name), skips service accounts, and is + org-scoped like :func:`resources_visible_via_memberships`. Any OWNER counts, + so a co-owner's email surfaces the resource too, not just the earliest one. + """ + organization = organization or UserContext.get_organization() + qs = ResourceMembership.objects.filter( + content_type=ContentType.objects.get_for_model(model), + role=ResourceRole.OWNER, + user__is_service_account=False, + user__email__icontains=term, + ) + if organization is not None: + qs = qs.filter(organization=organization) + return _object_id_subquery(qs, model) + + def serialize_group_refs(resource_obj: Any) -> list[dict[str, Any]]: """Return a compact ``[{id, name}]`` listing for share modals.""" return list(get_resource_share_groups(resource_obj).values("id", "name")) diff --git a/backend/utils/tests/test_list_pagination.py b/backend/utils/tests/test_list_pagination.py index d633760dca..ddba6d07cb 100644 --- a/backend/utils/tests/test_list_pagination.py +++ b/backend/utils/tests/test_list_pagination.py @@ -239,26 +239,34 @@ def test_search_narrows_rows_and_count(self) -> None: assert response.data["count"] == 3 assert all("alpha" in name for name in names) - def test_search_matches_name_not_owner_email(self) -> None: - """``?search=`` matches the resource name only, not the owner's email. + def test_search_matches_name_and_owner_email(self) -> None: + """``?search=`` matches the resource name or the displayed owner's email. - UN-3769 narrowed search from owner-inclusive (``created_by__email``) to - name-only so the search box and the Owned By column agree; the owner's - email substring must no longer return their rows. + UN-3770 restored owner search, but against the OWNER membership that + backs the Owned By column — not the audit-only ``created_by`` (UN-2202). + A viewer's email must not surface the row; only owners match. """ for endpoint in LIST_ENDPOINTS: with self.subTest(kind=endpoint.kind): - self._create(endpoint, f"{endpoint.kind}-searchable", owner=self.owner) - - by_email = self._list( - endpoint, self.owner, page=1, page_size=10, search="owner" + obj = self._create( + endpoint, f"{endpoint.kind}-searchable", owner=self.owner ) + # Viewer shares the row but is not an owner. + obj.memberships.create(user=self.viewer, role=ResourceRole.VIEWER) + by_name = self._list( endpoint, self.owner, page=1, page_size=10, search="searchable" ) + by_owner = self._list( + endpoint, self.owner, page=1, page_size=10, search="owner@example" + ) + by_viewer = self._list( + endpoint, self.owner, page=1, page_size=10, search="viewer@example" + ) - assert by_email.data["count"] == 0 assert by_name.data["count"] == 1 + assert by_owner.data["count"] == 1 + assert by_viewer.data["count"] == 0 def test_dropped_owner_ordering_field_is_ignored(self) -> None: """``?ordering=created_by__email`` is a dropped field, so it's ignored. diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index ada8d4f426..fefba8c21a 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -118,7 +118,15 @@ def get_queryset(self) -> QuerySet: search = self.request.query_params.get("search") if search: - queryset = queryset.filter(workflow_name__icontains=search) + from django.db.models import Q + from tenant_account_v2.sharing_helpers import ( + resources_matching_owner_search, + ) + + queryset = queryset.filter( + Q(workflow_name__icontains=search) + | Q(pk__in=resources_matching_owner_search(queryset.model, search)) + ) return queryset From 368a143d5736b08461197020a7c0557b0fbf367b Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 16:33:56 +0530 Subject: [PATCH 09/17] UN-3770 [FEAT] Update search placeholder to "Search by name or owner" The four owner-searchable list pages now advertise owner search via the placeholder; ToolNavBar's other consumers keep the "Search by name" default. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- .../src/components/custom-tools/list-of-tools/ListOfTools.jsx | 1 + .../src/components/navigations/tool-nav-bar/ToolNavBar.jsx | 4 +++- .../components/tool-settings/tool-settings/ToolSettings.jsx | 1 + frontend/src/components/workflows/workflow/Workflows.jsx | 1 + frontend/src/pages/ConnectorsPage.jsx | 1 + 5 files changed, 7 insertions(+), 1 deletion(-) 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 86ffc741e3..a3368c0c77 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -457,6 +457,7 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { handleSearch(value)} customButtons={customButtonsElement} segmentOptions={segmentOptions} diff --git a/frontend/src/components/navigations/tool-nav-bar/ToolNavBar.jsx b/frontend/src/components/navigations/tool-nav-bar/ToolNavBar.jsx index e39c50ff1e..1a979eb6ca 100644 --- a/frontend/src/components/navigations/tool-nav-bar/ToolNavBar.jsx +++ b/frontend/src/components/navigations/tool-nav-bar/ToolNavBar.jsx @@ -23,6 +23,7 @@ function ToolNavBar({ segmentValue, onSearch, searchKey, + searchPlaceholder = "Search by name", }) { const navigate = useNavigate(); const onSearchDebounce = debounce(({ target: { value } }) => { @@ -93,7 +94,7 @@ function ToolNavBar({ @@ -120,6 +121,7 @@ ToolNavBar.propTypes = { segmentFilter: PropTypes.func, onSearch: PropTypes.func, searchKey: PropTypes.string, + searchPlaceholder: PropTypes.string, }; export { ToolNavBar }; diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 10e2cc850a..68eb8bf923 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -357,6 +357,7 @@ function ToolSettings({ type }) { handleSearch(value)} customButtons={ diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 1d5afeac24..6bc9d22969 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -404,6 +404,7 @@ function Workflows() { handleSearch(value)} /> diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index e841cd8b9a..30eadb3e38 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -304,6 +304,7 @@ function ConnectorsPage() { handleSearch(value)} customButtons={newConnectorButton} /> From f992b3f4a0e5698dd709c40fcdd5e38e55be7513 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 16:40:17 +0530 Subject: [PATCH 10/17] UN-3770 [FEAT] List all co-owners in the Owned By tooltip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds owner_emails() (all live OWNER emails, earliest-first) on HasMembersMixin, exposed by the four list serializers. The Owned By cell still shows the primary owner + `+N` inline, but its tooltip now names every co-owner — so a search that matched a co-owner hidden behind `+N` is explainable on hover. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- backend/adapter_processor_v2/serializers.py | 1 + backend/connector_v2/serializers.py | 1 + backend/permissions/models.py | 12 ++++++++++++ .../prompt_studio_core_v2/serializers.py | 5 +++++ backend/utils/tests/test_list_pagination.py | 15 +++++++++++---- .../workflow_manager/workflow_v2/serializers.py | 1 + .../widgets/resource-table/ResourceTable.jsx | 11 ++++++++++- 7 files changed, 41 insertions(+), 5 deletions(-) diff --git a/backend/adapter_processor_v2/serializers.py b/backend/adapter_processor_v2/serializers.py index 34aeccc5c1..edb63c7608 100644 --- a/backend/adapter_processor_v2/serializers.py +++ b/backend/adapter_processor_v2/serializers.py @@ -216,6 +216,7 @@ def to_representation(self, instance: AdapterInstance) -> dict[str, str]: 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() + rep["owner_emails"] = instance.owner_emails() return rep diff --git a/backend/connector_v2/serializers.py b/backend/connector_v2/serializers.py index 7d15edb7f0..985600447c 100644 --- a/backend/connector_v2/serializers.py +++ b/backend/connector_v2/serializers.py @@ -177,6 +177,7 @@ def to_representation(self, instance: ConnectorInstance) -> dict[str, str]: 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() + rep["owner_emails"] = instance.owner_emails() return rep diff --git a/backend/permissions/models.py b/backend/permissions/models.py index 0ae0d87892..7f5ad12087 100644 --- a/backend/permissions/models.py +++ b/backend/permissions/models.py @@ -57,6 +57,18 @@ def owner_email(self) -> str | None: # pk breaks created_at ties so the label is stable across requests. return min(owners, key=lambda m: (m.created_at, m.pk)).user.email + def owner_emails(self) -> list[str]: + # Every live OWNER email, earliest-first (``owner_email`` is just the + # head). Backs the Owned By tooltip so all co-owners are named, not only + # the primary + a ``+N`` count. Reads prefetched ``memberships``. + owners = [ + m + for m in self.memberships.all() # type: ignore[attr-defined] + if m.role == ResourceRole.OWNER and not m.user.is_service_account + ] + owners.sort(key=lambda m: (m.created_at, m.pk)) + return [m.user.email for m in owners] + 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 a09798b14a..96396936af 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_core_v2/serializers.py @@ -49,6 +49,7 @@ class CustomToolListSerializer(serializers.ModelSerializer): is_owner = serializers.SerializerMethodField() co_owners_count = serializers.SerializerMethodField() owner_email = serializers.SerializerMethodField() + owner_emails = serializers.SerializerMethodField() class Meta: model = CustomTool @@ -67,6 +68,7 @@ class Meta: "is_owner", "co_owners_count", "owner_email", + "owner_emails", ] def get_created_by_email(self, instance): @@ -82,6 +84,9 @@ def get_co_owners_count(self, instance) -> int: def get_owner_email(self, instance) -> str | None: return instance.owner_email() + def get_owner_emails(self, instance) -> list[str]: + return instance.owner_emails() + def get_prompt_count(self, instance): if hasattr(instance, "_prompt_count"): return instance._prompt_count or 0 diff --git a/backend/utils/tests/test_list_pagination.py b/backend/utils/tests/test_list_pagination.py index ddba6d07cb..d68a3e6abc 100644 --- a/backend/utils/tests/test_list_pagination.py +++ b/backend/utils/tests/test_list_pagination.py @@ -309,7 +309,10 @@ def test_owner_email_is_earliest_live_owner(self) -> None: created_at=BASE_TIME + timedelta(minutes=minute) ) - assert type(wf).objects.get(pk=wf.pk).owner_email() == self.coowner.email + fresh = type(wf).objects.get(pk=wf.pk) + assert fresh.owner_email() == self.coowner.email + # owner_emails() is the full roster, earliest-first, svc still skipped. + assert fresh.owner_emails() == [self.coowner.email, self.owner.email] # Tied created_at: pk decides, so the label never flips between requests. wf.memberships.all().delete() @@ -320,8 +323,12 @@ def test_owner_email_is_earliest_live_owner(self) -> None: type(tied[0]).objects.filter(pk__in=[m.pk for m in tied]).update( created_at=BASE_TIME ) - earliest = min(tied, key=lambda m: m.pk) - assert type(wf).objects.get(pk=wf.pk).owner_email() == earliest.user.email + ordered = sorted(tied, key=lambda m: m.pk) + fresh = type(wf).objects.get(pk=wf.pk) + assert fresh.owner_email() == ordered[0].user.email + assert fresh.owner_emails() == [m.user.email for m in ordered] wf.memberships.all().delete() - assert type(wf).objects.get(pk=wf.pk).owner_email() is None + fresh = type(wf).objects.get(pk=wf.pk) + assert fresh.owner_email() is None + assert fresh.owner_emails() == [] diff --git a/backend/workflow_manager/workflow_v2/serializers.py b/backend/workflow_manager/workflow_v2/serializers.py index c2f037570c..a25abfbd56 100644 --- a/backend/workflow_manager/workflow_v2/serializers.py +++ b/backend/workflow_manager/workflow_v2/serializers.py @@ -86,6 +86,7 @@ def to_representation(self, instance: Workflow) -> dict[str, str]: 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() + representation["owner_emails"] = instance.owner_emails() return representation def create(self, validated_data: dict[str, Any]) -> Any: diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index ac04cd7a01..50f1f8ec45 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -137,6 +137,7 @@ function ResourceTable({ dateProp = "created_at", modifiedProp = "modified_at", ownerEmailProp = "created_by_email", + ownerEmailsProp = "owner_emails", countProp, countLabel, handleEdit, @@ -206,6 +207,13 @@ function ResourceTable({ item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; const initials = (email || name).slice(0, 2).toUpperCase(); const swatch = colorForSeed(email || name); + // With co-owners, name only the primary inline (+N); the tooltip lists all + // owners so search hitting a hidden co-owner is still explainable. + const ownerEmails = item?.[ownerEmailsProp]; + const ownerTooltip = + Array.isArray(ownerEmails) && ownerEmails.length > 1 + ? ownerEmails.join(", ") + : `${name}${extra}`; const cell = ( @@ -219,7 +227,7 @@ function ResourceTable({
{name} {extra} @@ -436,6 +444,7 @@ ResourceTable.propTypes = { dateProp: PropTypes.string, modifiedProp: PropTypes.string, ownerEmailProp: PropTypes.string, + ownerEmailsProp: PropTypes.string, countProp: PropTypes.string, countLabel: PropTypes.string, handleEdit: PropTypes.func, From 9fed8311ff89b63db0210f04d5287736d4a8e0ba Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 16:48:27 +0530 Subject: [PATCH 11/17] UN-3770 [REFACTOR] Collapse owner_email into owner_emails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit owner_email was owner_emails[0] — one derivable scalar of redundancy. Drop it from the four list serializers and read owner_emails[0] on the frontend instead (ResourceTable, and the cloud Projects card in the companion cloud PR). The model's owner_email() accessor stays for callers/tests that want just the head. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- backend/adapter_processor_v2/serializers.py | 1 - backend/connector_v2/serializers.py | 1 - backend/prompt_studio/prompt_studio_core_v2/serializers.py | 5 ----- backend/workflow_manager/workflow_v2/serializers.py | 1 - .../components/custom-tools/list-of-tools/ListOfTools.jsx | 1 - .../tool-settings/tool-settings/ToolSettings.jsx | 1 - .../components/widgets/resource-table/ResourceTable.jsx | 7 +++---- frontend/src/components/workflows/workflow/Workflows.jsx | 1 - frontend/src/pages/ConnectorsPage.jsx | 1 - 9 files changed, 3 insertions(+), 16 deletions(-) diff --git a/backend/adapter_processor_v2/serializers.py b/backend/adapter_processor_v2/serializers.py index edb63c7608..1680849836 100644 --- a/backend/adapter_processor_v2/serializers.py +++ b/backend/adapter_processor_v2/serializers.py @@ -215,7 +215,6 @@ 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() rep["owner_emails"] = instance.owner_emails() return rep diff --git a/backend/connector_v2/serializers.py b/backend/connector_v2/serializers.py index 985600447c..8e5583e889 100644 --- a/backend/connector_v2/serializers.py +++ b/backend/connector_v2/serializers.py @@ -176,7 +176,6 @@ 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() rep["owner_emails"] = instance.owner_emails() return rep diff --git a/backend/prompt_studio/prompt_studio_core_v2/serializers.py b/backend/prompt_studio/prompt_studio_core_v2/serializers.py index 96396936af..acb3a243d7 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_core_v2/serializers.py @@ -48,7 +48,6 @@ class CustomToolListSerializer(serializers.ModelSerializer): prompt_count = serializers.SerializerMethodField() is_owner = serializers.SerializerMethodField() co_owners_count = serializers.SerializerMethodField() - owner_email = serializers.SerializerMethodField() owner_emails = serializers.SerializerMethodField() class Meta: @@ -67,7 +66,6 @@ class Meta: "prompt_count", "is_owner", "co_owners_count", - "owner_email", "owner_emails", ] @@ -81,9 +79,6 @@ 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_owner_emails(self, instance) -> list[str]: return instance.owner_emails() diff --git a/backend/workflow_manager/workflow_v2/serializers.py b/backend/workflow_manager/workflow_v2/serializers.py index a25abfbd56..f96c3bd70a 100644 --- a/backend/workflow_manager/workflow_v2/serializers.py +++ b/backend/workflow_manager/workflow_v2/serializers.py @@ -85,7 +85,6 @@ 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() representation["owner_emails"] = instance.owner_emails() return representation 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 a3368c0c77..9f2d403007 100644 --- a/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx +++ b/frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx @@ -500,7 +500,6 @@ function ListOfTools({ segmentOptions, segmentValue, onSegmentChange }) { idProp="tool_id" dateProp="created_at" modifiedProp="modified_at" - ownerEmailProp="owner_email" countProp="prompt_count" countLabel="Prompts" handleEdit={handleEdit} diff --git a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx index 68eb8bf923..67c256388c 100644 --- a/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx +++ b/frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx @@ -405,7 +405,6 @@ function ToolSettings({ type }) { iconProp="icon" idProp="id" dateProp="created_at" - ownerEmailProp="owner_email" handleEdit={handleEdit} handleShare={handleShare} handleDelete={handleDelete} diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 50f1f8ec45..91f327ad13 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -136,7 +136,6 @@ function ResourceTable({ idProp, dateProp = "created_at", modifiedProp = "modified_at", - ownerEmailProp = "created_by_email", ownerEmailsProp = "owner_emails", countProp, countLabel, @@ -197,7 +196,9 @@ function ResourceTable({ }; const renderOwner = (item) => { - const email = item?.[ownerEmailProp]; + // owner_emails is earliest-first; [0] is the primary shown owner. + const ownerEmails = item?.[ownerEmailsProp]; + const email = Array.isArray(ownerEmails) ? ownerEmails[0] : undefined; // "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". @@ -209,7 +210,6 @@ function ResourceTable({ const swatch = colorForSeed(email || name); // With co-owners, name only the primary inline (+N); the tooltip lists all // owners so search hitting a hidden co-owner is still explainable. - const ownerEmails = item?.[ownerEmailsProp]; const ownerTooltip = Array.isArray(ownerEmails) && ownerEmails.length > 1 ? ownerEmails.join(", ") @@ -443,7 +443,6 @@ ResourceTable.propTypes = { idProp: PropTypes.string.isRequired, dateProp: PropTypes.string, modifiedProp: PropTypes.string, - ownerEmailProp: PropTypes.string, ownerEmailsProp: PropTypes.string, countProp: PropTypes.string, countLabel: PropTypes.string, diff --git a/frontend/src/components/workflows/workflow/Workflows.jsx b/frontend/src/components/workflows/workflow/Workflows.jsx index 6bc9d22969..4cc41b86c0 100644 --- a/frontend/src/components/workflows/workflow/Workflows.jsx +++ b/frontend/src/components/workflows/workflow/Workflows.jsx @@ -446,7 +446,6 @@ function Workflows() { descriptionProp="description" idProp="id" dateProp="created_at" - ownerEmailProp="owner_email" handleEdit={updateProject} handleShare={handleShare} handleDelete={deleteProject} diff --git a/frontend/src/pages/ConnectorsPage.jsx b/frontend/src/pages/ConnectorsPage.jsx index 30eadb3e38..5e95351823 100644 --- a/frontend/src/pages/ConnectorsPage.jsx +++ b/frontend/src/pages/ConnectorsPage.jsx @@ -342,7 +342,6 @@ function ConnectorsPage() { iconProp="icon" idProp="id" dateProp="created_at" - ownerEmailProp="owner_email" handleEdit={handleEditConnector} handleShare={handleShareConnector} handleDelete={handleDeleteConnector} From dfc903715cc82e135aac66aa31f581ef23db2732 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Wed, 29 Jul 2026 18:15:17 +0530 Subject: [PATCH 12/17] UN-3770 [REVERT] Drop the table column-width/layout tweaks for now Revert the layout half of the earlier "sort affordance + layout" change: restore the proportional column widths (34/22/15/15/14%), the 190px owner name/email cap, and drop the min-width scroll container + Created ellipsis. The sort-affordance (userSorted) fix stays. Layout to be revisited later. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- .../widgets/resource-table/ResourceTable.css | 20 ++++----------- .../widgets/resource-table/ResourceTable.jsx | 25 +++++-------------- 2 files changed, 11 insertions(+), 34 deletions(-) diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.css b/frontend/src/components/widgets/resource-table/ResourceTable.css index ae913d3bf7..cbbe1a0af0 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.css +++ b/frontend/src/components/widgets/resource-table/ResourceTable.css @@ -2,19 +2,13 @@ (Name / Owned By / Created / Modified / Actions) */ .resource-table { - /* Name has no fixed width and absorbs the slack; the other columns are a - fixed compact width (set in the JSX) so they stay evenly spaced. */ + /* 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; } -/* Below this the fixed columns would crush, so the table scrolls inside its - own container instead. overflow-x on the content is the scroll boundary, so - the min-width never pushes the page wider. */ -.resource-table table { - min-width: 1040px; -} - +/* Let wide content scroll inside the table instead of the page body */ .resource-table .ant-table-content { overflow-x: auto; } @@ -125,12 +119,6 @@ /* Owned By column: avatar + name/email, clickable to manage co-owners */ .resource-table-owner { min-width: 0; - max-width: 100%; -} - -/* Let the name/email item shrink so it ellipsizes within the cell */ -.resource-table-owner .ant-space-item { - min-width: 0; } .resource-table-owner-avatar { @@ -148,10 +136,12 @@ .resource-table-owner-name { font-weight: 500; + max-width: 190px; } .resource-table-owner-email { font-size: 12px; + max-width: 190px; } .resource-table-owner-btn { diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 91f327ad13..5cb3d3e3a4 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -44,11 +44,6 @@ const colorForSeed = (seed = "") => { return AVATAR_COLORS[Math.abs(hash) % AVATAR_COLORS.length]; }; -// Owner/Created/Modified/Actions share one compact width so they read as an -// evenly-spaced group; Name has no width and absorbs the remaining space. Sized -// to fit a full timestamp on one line. -const COMPACT_COL_WIDTH = 210; - // Sort-menu wording differs for text vs date columns (per the design). const SORT_OPTIONS = { text: [ @@ -331,12 +326,13 @@ function ResourceTable({ /> ), key: "name", + width: "34%", render: (_, item) => renderName(item), }, showOwner && { title: Owned By, key: "owner", - width: COMPACT_COL_WIDTH, + width: "22%", render: (_, item) => renderOwner(item), }, { @@ -351,17 +347,8 @@ function ResourceTable({ /> ), key: "created", - width: COMPACT_COL_WIDTH, - render: (_, item) => { - const formatted = formattedDateTime(item?.[dateProp]); - return formatted ? ( - - {formatted} - - ) : ( - "-" - ); - }, + width: "15%", + render: (_, item) => formattedDateTime(item?.[dateProp]) || "-", }, { title: ( @@ -375,7 +362,7 @@ function ResourceTable({ /> ), key: "modified", - width: COMPACT_COL_WIDTH, + width: "15%", render: (_, item) => { const iso = item?.[modifiedProp]; const rel = timeAgo(iso); @@ -389,7 +376,7 @@ function ResourceTable({ { title: Actions, key: "actions", - width: COMPACT_COL_WIDTH, + width: "14%", align: "right", render: (_, item) => renderActions(item), }, From 3853476dc59b3597464308a2dd6a46e81e66d0af Mon Sep 17 00:00:00 2001 From: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:03:29 +0530 Subject: [PATCH 13/17] UN-3770 [FEAT] ResourceTable: extraColumns + onRowClick (for lookups) (#2221) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UN-3770 [FEAT] ResourceTable: extraColumns + onRowClick override Let callers inject resource-specific columns (inserted before Actions) and override the default relative row-click nav — needed so the Prompt Studio lookups table can reuse this widget while keeping its Files/Latest Version columns and its absolute, stateful navigation. Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ Co-authored-by: Claude Opus 4.8 --- .../widgets/resource-table/ResourceTable.jsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 5cb3d3e3a4..8e4ca6e8ef 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -134,10 +134,12 @@ function ResourceTable({ ownerEmailsProp = "owner_emails", countProp, countLabel, + extraColumns = [], handleEdit, handleShare, handleDelete, handleCoOwner, + onRowClick, sessionDetails, showOwner = true, isClickable = true, @@ -373,6 +375,9 @@ function ResourceTable({ ); }, }, + // Resource-specific columns (e.g. Files, Latest Version) sit between the + // shared date columns and Actions. + ...extraColumns, { title: Actions, key: "actions", @@ -399,7 +404,12 @@ function ResourceTable({ onChange={handleChange} rowClassName={isClickable ? "resource-table-row-clickable" : ""} onRow={(item) => ({ - onClick: isClickable ? () => navigate(`${item?.[idProp]}`) : undefined, + // onRowClick lets callers override the default relative nav (e.g. an + // absolute org-scoped path with router state). + onClick: isClickable + ? () => + onRowClick ? onRowClick(item) : navigate(`${item?.[idProp]}`) + : undefined, })} pagination={{ current: pagination?.current, @@ -433,10 +443,12 @@ ResourceTable.propTypes = { ownerEmailsProp: PropTypes.string, countProp: PropTypes.string, countLabel: PropTypes.string, + extraColumns: PropTypes.array, handleEdit: PropTypes.func, handleShare: PropTypes.func, handleDelete: PropTypes.func, handleCoOwner: PropTypes.func, + onRowClick: PropTypes.func, sessionDetails: PropTypes.object, showOwner: PropTypes.bool, isClickable: PropTypes.bool, From 8d05a7b136937f6dcf26609062fd23f701cf0929 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Thu, 30 Jul 2026 13:45:36 +0530 Subject: [PATCH 14/17] chore: re-trigger pre-commit.ci (transient mergeable-check error) From 7c66465d0eb99d9b90ac432ae7f15295d4ee4152 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Thu, 30 Jul 2026 14:49:33 +0530 Subject: [PATCH 15/17] UN-3770 [FIX] Address review: owner fallback+mask, scoped tiebreaker, index, prefix search - ResourceTable: fall back to created_by_email so rows with no live OWNER membership render the creator instead of "Unknown". - Adapter serializer: mask owner_emails to ["Unstract"] for frictionless adapters so the Owned By column keeps the org-wide mask. - Drop redundant .distinct() from adapter/connector/prompt_studio for_user (every arm is a PK subquery, no join, nothing to collapse). - Add (organization, -modified_at) index to the 4 resource models backing the default list ordering. - fetchAllPages: request MAX_PAGE_SIZE up front so the common case is one round-trip; the loop stays as the tail guard. - Pin plain OrderingFilter on the two high-volume execution-log endpoints so they don't inherit the pk tiebreaker (unindexed Sort) from the global default. - Owner search matches the email prefix (local part) so a bare domain fragment doesn't return every row in a single-domain org. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- ...dapterinstance_adapter_org_modified_idx.py | 18 ++++++++++++++++ backend/adapter_processor_v2/models.py | 21 +++++++++++-------- backend/adapter_processor_v2/serializers.py | 5 ++++- ...ctorinstance_connector_org_modified_idx.py | 18 ++++++++++++++++ backend/connector_v2/models.py | 19 ++++++++++------- ...10_customtool_custtool_org_modified_idx.py | 18 ++++++++++++++++ .../prompt_studio_core_v2/models.py | 19 ++++++++++------- backend/tenant_account_v2/sharing_helpers.py | 11 +++++----- .../workflow_manager/file_execution/views.py | 7 +++++++ .../workflow_v2/execution_log_view.py | 7 +++++++ ...0025_workflow_workflow_org_modified_idx.py | 18 ++++++++++++++++ .../workflow_v2/models/workflow.py | 7 +++++++ .../widgets/resource-table/ResourceTable.jsx | 6 +++++- frontend/src/helpers/pagination.js | 16 ++++++++++++-- 14 files changed, 156 insertions(+), 34 deletions(-) create mode 100644 backend/adapter_processor_v2/migrations/0006_adapterinstance_adapter_org_modified_idx.py create mode 100644 backend/connector_v2/migrations/0008_connectorinstance_connector_org_modified_idx.py create mode 100644 backend/prompt_studio/prompt_studio_core_v2/migrations/0010_customtool_custtool_org_modified_idx.py create mode 100644 backend/workflow_manager/workflow_v2/migrations/0025_workflow_workflow_org_modified_idx.py diff --git a/backend/adapter_processor_v2/migrations/0006_adapterinstance_adapter_org_modified_idx.py b/backend/adapter_processor_v2/migrations/0006_adapterinstance_adapter_org_modified_idx.py new file mode 100644 index 0000000000..a9e3f7df1e --- /dev/null +++ b/backend/adapter_processor_v2/migrations/0006_adapterinstance_adapter_org_modified_idx.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.30 on 2026-07-30 09:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("adapter_processor_v2", "0005_absorb_shared_users"), + ] + + operations = [ + migrations.AddIndex( + model_name="adapterinstance", + index=models.Index( + fields=["organization", "-modified_at"], name="adapter_org_modified_idx" + ), + ), + ] diff --git a/backend/adapter_processor_v2/models.py b/backend/adapter_processor_v2/models.py index f160e67538..a2287aa4a3 100644 --- a/backend/adapter_processor_v2/models.py +++ b/backend/adapter_processor_v2/models.py @@ -51,15 +51,11 @@ def for_user(self, user: User) -> QuerySet[Any]: group_shared_ids = resources_visible_via_groups(self.model, user_group_ids) member_ids = resources_visible_via_memberships(self.model, user) - return ( - self.get_queryset() - .filter( - models.Q(pk__in=member_ids) - | models.Q(shared_to_org=True) - | models.Q(is_friction_less=True) - | models.Q(pk__in=group_shared_ids) - ) - .distinct() + return self.get_queryset().filter( + models.Q(pk__in=member_ids) + | models.Q(shared_to_org=True) + | models.Q(is_friction_less=True) + | models.Q(pk__in=group_shared_ids) ) @@ -172,6 +168,13 @@ class Meta: name="unique_organization_adapter", ), ] + # Backs the default org-scoped `-modified_at, pk` list ordering. + indexes = [ + models.Index( + fields=["organization", "-modified_at"], + name="adapter_org_modified_idx", + ), + ] def create_adapter(self) -> None: encryption_secret: str = settings.ENCRYPTION_KEY diff --git a/backend/adapter_processor_v2/serializers.py b/backend/adapter_processor_v2/serializers.py index 1680849836..c8545223bd 100644 --- a/backend/adapter_processor_v2/serializers.py +++ b/backend/adapter_processor_v2/serializers.py @@ -207,15 +207,18 @@ def to_representation(self, instance: AdapterInstance) -> dict[str, str]: if model: rep["model"] = model + # Frictionless (Unstract-provisioned) adapters mask the owner org-wide; + # mask owner_emails too, else the Owned By column leaks the real owner. if instance.is_friction_less: rep["created_by_email"] = "Unstract" + rep["owner_emails"] = ["Unstract"] else: rep["created_by_email"] = instance.created_by.email + rep["owner_emails"] = instance.owner_emails() 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_emails"] = instance.owner_emails() return rep diff --git a/backend/connector_v2/migrations/0008_connectorinstance_connector_org_modified_idx.py b/backend/connector_v2/migrations/0008_connectorinstance_connector_org_modified_idx.py new file mode 100644 index 0000000000..adf8efb4b0 --- /dev/null +++ b/backend/connector_v2/migrations/0008_connectorinstance_connector_org_modified_idx.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.30 on 2026-07-30 09:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("connector_v2", "0007_absorb_shared_users"), + ] + + operations = [ + migrations.AddIndex( + model_name="connectorinstance", + index=models.Index( + fields=["organization", "-modified_at"], name="connector_org_modified_idx" + ), + ), + ] diff --git a/backend/connector_v2/models.py b/backend/connector_v2/models.py index 57ae5933c3..e0790c052c 100644 --- a/backend/connector_v2/models.py +++ b/backend/connector_v2/models.py @@ -44,14 +44,10 @@ def for_user(self, user: User) -> models.QuerySet: group_shared_ids = resources_visible_via_groups(self.model, user_group_ids) member_ids = resources_visible_via_memberships(self.model, user) - return ( - self.get_queryset() - .filter( - models.Q(pk__in=member_ids) - | models.Q(shared_to_org=True) - | models.Q(pk__in=group_shared_ids) - ) - .distinct() + return self.get_queryset().filter( + models.Q(pk__in=member_ids) + | models.Q(shared_to_org=True) + | models.Q(pk__in=group_shared_ids) ) @@ -173,3 +169,10 @@ class Meta: name="unique_organization_connector", ), ] + # Backs the default org-scoped `-modified_at, pk` list ordering. + indexes = [ + models.Index( + fields=["organization", "-modified_at"], + name="connector_org_modified_idx", + ), + ] diff --git a/backend/prompt_studio/prompt_studio_core_v2/migrations/0010_customtool_custtool_org_modified_idx.py b/backend/prompt_studio/prompt_studio_core_v2/migrations/0010_customtool_custtool_org_modified_idx.py new file mode 100644 index 0000000000..363cb2ef3e --- /dev/null +++ b/backend/prompt_studio/prompt_studio_core_v2/migrations/0010_customtool_custtool_org_modified_idx.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.30 on 2026-07-30 09:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("prompt_studio_core_v2", "0009_absorb_shared_users"), + ] + + operations = [ + migrations.AddIndex( + model_name="customtool", + index=models.Index( + fields=["organization", "-modified_at"], name="custtool_org_modified_idx" + ), + ), + ] diff --git a/backend/prompt_studio/prompt_studio_core_v2/models.py b/backend/prompt_studio/prompt_studio_core_v2/models.py index 031d61b1e7..4403de75db 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/models.py +++ b/backend/prompt_studio/prompt_studio_core_v2/models.py @@ -40,14 +40,10 @@ def for_user(self, user: User) -> QuerySet[Any]: group_shared_ids = resources_visible_via_groups(self.model, user_group_ids) member_ids = resources_visible_via_memberships(self.model, user) - return ( - self.get_queryset() - .filter( - models.Q(pk__in=member_ids) - | models.Q(shared_to_org=True) - | models.Q(pk__in=group_shared_ids) - ) - .distinct() + return self.get_queryset().filter( + models.Q(pk__in=member_ids) + | models.Q(shared_to_org=True) + | models.Q(pk__in=group_shared_ids) ) @@ -226,3 +222,10 @@ class Meta: name="unique_tool_name", ), ] + # Backs the default org-scoped `-modified_at, pk` list ordering. + indexes = [ + models.Index( + fields=["organization", "-modified_at"], + name="custtool_org_modified_idx", + ), + ] diff --git a/backend/tenant_account_v2/sharing_helpers.py b/backend/tenant_account_v2/sharing_helpers.py index 32c30d69ef..36887b1331 100644 --- a/backend/tenant_account_v2/sharing_helpers.py +++ b/backend/tenant_account_v2/sharing_helpers.py @@ -228,17 +228,18 @@ def resources_matching_owner_search( """Subquery of ``model`` PKs whose displayed owner matches ``term``. Owner search hits the same OWNER memberships that back the "Owned By" - column, so the search box agrees with what is shown. Matches the owner's - email (its local part is the shown name), skips service accounts, and is - org-scoped like :func:`resources_visible_via_memberships`. Any OWNER counts, - so a co-owner's email surfaces the resource too, not just the earliest one. + column, so the search box agrees with what is shown. Matches on the email + prefix (the local part is the shown name) so a bare domain fragment like + "com" doesn't return every row in a single-domain org. Skips service + accounts, org-scoped like :func:`resources_visible_via_memberships`. Any + OWNER counts, so a co-owner's email surfaces the resource too. """ organization = organization or UserContext.get_organization() qs = ResourceMembership.objects.filter( content_type=ContentType.objects.get_for_model(model), role=ResourceRole.OWNER, user__is_service_account=False, - user__email__icontains=term, + user__email__istartswith=term, ) if organization is not None: qs = qs.filter(organization=organization) diff --git a/backend/workflow_manager/file_execution/views.py b/backend/workflow_manager/file_execution/views.py index cadfa25706..46fe5ee2d5 100644 --- a/backend/workflow_manager/file_execution/views.py +++ b/backend/workflow_manager/file_execution/views.py @@ -1,6 +1,9 @@ from django.db.models import OuterRef, Subquery +from django_filters.rest_framework import DjangoFilterBackend from rest_framework import viewsets +from rest_framework.filters import OrderingFilter from rest_framework.permissions import IsAuthenticated +from utils.filters.organization_filter import OrganizationFilterBackend from utils.pagination import CustomPagination from workflow_manager.file_execution.filter import FileExecutionFilter @@ -18,6 +21,10 @@ class FileCentricExecutionViewSet(viewsets.ReadOnlyModelViewSet): ordering_fields = ["created_at", "execution_time", "file_size"] ordering = ["created_at"] filterset_class = FileExecutionFilter + # Plain OrderingFilter (not the global deterministic one): this is a + # high-volume table and appending pk to the ordering can't be served by an + # ordered index scan. Opted out explicitly rather than via the global default. + filter_backends = [OrganizationFilterBackend, DjangoFilterBackend, OrderingFilter] def get_queryset(self): execution_id = self.kwargs.get("pk") diff --git a/backend/workflow_manager/workflow_v2/execution_log_view.py b/backend/workflow_manager/workflow_v2/execution_log_view.py index 2d8575080d..f808b25f58 100644 --- a/backend/workflow_manager/workflow_v2/execution_log_view.py +++ b/backend/workflow_manager/workflow_v2/execution_log_view.py @@ -7,11 +7,14 @@ from django.db.models.query import QuerySet from django.http import HttpResponse from django.utils import timezone +from django_filters.rest_framework import DjangoFilterBackend from permissions.permission import IsOwner from rest_framework import status, viewsets +from rest_framework.filters import OrderingFilter from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.versioning import URLPathVersioning +from utils.filters.organization_filter import OrganizationFilterBackend from utils.pagination import CustomPagination from workflow_manager.workflow_v2.filters import ExecutionLogFilter @@ -34,6 +37,10 @@ class WorkflowExecutionLogViewSet(viewsets.ModelViewSet): ordering_fields = ["event_time"] ordering = ["event_time"] filterset_class = ExecutionLogFilter + # Plain OrderingFilter (not the global deterministic one): ExecutionLog is + # high-volume and its (…, event_time) indexes don't include pk, so appending + # pk forces a Sort node. Opted out explicitly rather than via the global default. + filter_backends = [OrganizationFilterBackend, DjangoFilterBackend, OrderingFilter] def get_queryset(self) -> QuerySet: execution_id = self.kwargs.get("pk") diff --git a/backend/workflow_manager/workflow_v2/migrations/0025_workflow_workflow_org_modified_idx.py b/backend/workflow_manager/workflow_v2/migrations/0025_workflow_workflow_org_modified_idx.py new file mode 100644 index 0000000000..af4150024f --- /dev/null +++ b/backend/workflow_manager/workflow_v2/migrations/0025_workflow_workflow_org_modified_idx.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.30 on 2026-07-30 09:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("workflow_v2", "0024_merge_coowner_and_active_idx"), + ] + + operations = [ + migrations.AddIndex( + model_name="workflow", + index=models.Index( + fields=["organization", "-modified_at"], name="workflow_org_modified_idx" + ), + ), + ] diff --git a/backend/workflow_manager/workflow_v2/models/workflow.py b/backend/workflow_manager/workflow_v2/models/workflow.py index 461e63689b..0fd5039603 100644 --- a/backend/workflow_manager/workflow_v2/models/workflow.py +++ b/backend/workflow_manager/workflow_v2/models/workflow.py @@ -159,3 +159,10 @@ class Meta: name="unique_workflow_name", ), ] + # Backs the default org-scoped `-modified_at, pk` list ordering. + indexes = [ + models.Index( + fields=["organization", "-modified_at"], + name="workflow_org_modified_idx", + ), + ] diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 8e4ca6e8ef..f2180f939d 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -194,8 +194,12 @@ function ResourceTable({ const renderOwner = (item) => { // owner_emails is earliest-first; [0] is the primary shown owner. + // Fall back to created_by_email so rows with no live OWNER membership + // (platform API-key sessions, pre-backfill rows) don't render "Unknown". const ownerEmails = item?.[ownerEmailsProp]; - const email = Array.isArray(ownerEmails) ? ownerEmails[0] : undefined; + const email = + (Array.isArray(ownerEmails) ? ownerEmails[0] : undefined) ?? + item?.created_by_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". diff --git a/frontend/src/helpers/pagination.js b/frontend/src/helpers/pagination.js index 2e0ee10bfb..939e55dff0 100644 --- a/frontend/src/helpers/pagination.js +++ b/frontend/src/helpers/pagination.js @@ -34,11 +34,19 @@ function unwrapList(res) { * @param {Object} config - axios request config; `url` is required * @return {Promise} all rows across all pages */ +// Mirrors backend Pagination.MAX_PAGE_SIZE. Requesting the max up front keeps +// the common case (an org below this many rows) to a single round-trip once the +// endpoints paginate by default; the loop stays as the tail guard past it. +const MAX_PAGE_SIZE = 1000; + async function fetchAllPages(axiosInstance, config) { const request = (params) => axiosInstance({ ...config, method: "GET", ...(params ? { params } : {}) }); - const first = await request(); + const first = await request({ + ...(config?.params ?? {}), + page_size: MAX_PAGE_SIZE, + }); if (Array.isArray(first?.data)) { return first.data; } @@ -48,7 +56,11 @@ async function fetchAllPages(axiosInstance, config) { let page = 1; while (rows.length < total) { page += 1; - const res = await request({ ...(config?.params ?? {}), page }); + const res = await request({ + ...(config?.params ?? {}), + page, + page_size: MAX_PAGE_SIZE, + }); const nextRows = res?.data?.results ?? []; // A page that adds nothing would otherwise spin forever on a bad count. if (!nextRows.length) { From 8d0f4bc7b1c1ccf58ff43b34289c8710033d9fe4 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Thu, 30 Jul 2026 15:08:27 +0530 Subject: [PATCH 16/17] UN-3770 [FIX] Address Greptile: stable log pagination + drop stale list responses Two P1 findings from Greptile: - Execution-log endpoints (file_execution, execution_log_view) dropped their plain-OrderingFilter override and now inherit the global deterministic filter, so tied created_at/event_time rows can't repeat or omit across pages. Each request is already scoped to a single execution_id, so the pk tie-breaker sorts a narrow set, not the whole table. - Pipelines and ApiDeployment list fetches adopt the monotonic seq guard via applyPagedResponse, so a slow superseded response can no longer overwrite a newer search/page/type selection. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- .../workflow_manager/file_execution/views.py | 7 --- .../workflow_v2/execution_log_view.py | 7 --- .../api-deployment/ApiDeployment.jsx | 45 +++++++++++++------ .../pipelines/Pipelines.jsx | 44 ++++++++++++------ 4 files changed, 61 insertions(+), 42 deletions(-) diff --git a/backend/workflow_manager/file_execution/views.py b/backend/workflow_manager/file_execution/views.py index 46fe5ee2d5..cadfa25706 100644 --- a/backend/workflow_manager/file_execution/views.py +++ b/backend/workflow_manager/file_execution/views.py @@ -1,9 +1,6 @@ from django.db.models import OuterRef, Subquery -from django_filters.rest_framework import DjangoFilterBackend from rest_framework import viewsets -from rest_framework.filters import OrderingFilter from rest_framework.permissions import IsAuthenticated -from utils.filters.organization_filter import OrganizationFilterBackend from utils.pagination import CustomPagination from workflow_manager.file_execution.filter import FileExecutionFilter @@ -21,10 +18,6 @@ class FileCentricExecutionViewSet(viewsets.ReadOnlyModelViewSet): ordering_fields = ["created_at", "execution_time", "file_size"] ordering = ["created_at"] filterset_class = FileExecutionFilter - # Plain OrderingFilter (not the global deterministic one): this is a - # high-volume table and appending pk to the ordering can't be served by an - # ordered index scan. Opted out explicitly rather than via the global default. - filter_backends = [OrganizationFilterBackend, DjangoFilterBackend, OrderingFilter] def get_queryset(self): execution_id = self.kwargs.get("pk") diff --git a/backend/workflow_manager/workflow_v2/execution_log_view.py b/backend/workflow_manager/workflow_v2/execution_log_view.py index f808b25f58..2d8575080d 100644 --- a/backend/workflow_manager/workflow_v2/execution_log_view.py +++ b/backend/workflow_manager/workflow_v2/execution_log_view.py @@ -7,14 +7,11 @@ from django.db.models.query import QuerySet from django.http import HttpResponse from django.utils import timezone -from django_filters.rest_framework import DjangoFilterBackend from permissions.permission import IsOwner from rest_framework import status, viewsets -from rest_framework.filters import OrderingFilter from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.versioning import URLPathVersioning -from utils.filters.organization_filter import OrganizationFilterBackend from utils.pagination import CustomPagination from workflow_manager.workflow_v2.filters import ExecutionLogFilter @@ -37,10 +34,6 @@ class WorkflowExecutionLogViewSet(viewsets.ModelViewSet): ordering_fields = ["event_time"] ordering = ["event_time"] filterset_class = ExecutionLogFilter - # Plain OrderingFilter (not the global deterministic one): ExecutionLog is - # high-volume and its (…, event_time) indexes don't include pk, so appending - # pk forces a Sort node. Opted out explicitly rather than via the global default. - filter_backends = [OrganizationFilterBackend, DjangoFilterBackend, OrderingFilter] def get_queryset(self) -> QuerySet: execution_id = self.kwargs.get("pk") diff --git a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx index 2bd277721a..b1e5bd0708 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, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useLocation } from "react-router-dom"; import { deploymentApiTypes, displayURL } from "../../../helpers/GetStaticData"; @@ -6,7 +6,10 @@ import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate.js"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement.jsx"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler.jsx"; import { useExecutionLogs } from "../../../hooks/useExecutionLogs"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { + applyPagedResponse, + usePaginatedList, +} from "../../../hooks/usePaginatedList"; import usePipelineHelper from "../../../hooks/usePipelineHelper.js"; import { useInitialFetchCount, @@ -44,6 +47,8 @@ function ApiDeployment() { const [openManageKeysModal, setOpenManageKeysModal] = useState(false); const [selectedRow, setSelectedRow] = useState({}); const [tableData, setTableData] = useState([]); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const [filteredData, setFilteredData] = useState([]); const [apiKeys, setApiKeys] = useState([]); const [isEdit, setIsEdit] = useState(false); @@ -155,27 +160,39 @@ function ApiDeployment() { const getApiDeploymentList = (page = 1, pageSize = 10, search = "") => { setIsTableLoading(true); - apiDeploymentsApiService + const seq = ++seqRef.current; + return apiDeploymentsApiService .getApiDeploymentsList(page, pageSize, search) .then((res) => { - const data = res?.data; - const results = data.results || data; - setTableData(results); - setPagination((prev) => ({ - ...prev, - current: page, + const stepback = applyPagedResponse({ + data: res?.data, + page, pageSize, - total: data.count ?? data.results?.length ?? data.length, - })); - - activateScrollRestore(); + seq, + latestSeqRef: seqRef, + setList: setTableData, + setPagination, + refetchPrevPage: () => + getApiDeploymentList(page - 1, pageSize, search), + }); + if (seq === seqRef.current) { + activateScrollRestore(); + } + return stepback; }) .catch((err) => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { + return; + } setAlertDetails(handleException(err)); clearPendingScroll(); }) .finally(() => { - setIsTableLoading(false); + // Only the newest request owns the shared loading state. + if (seq === seqRef.current) { + setIsTableLoading(false); + } }); }; diff --git a/frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx b/frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx index f2688b6ea5..edde00911e 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, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useLocation } from "react-router-dom"; import { @@ -11,7 +11,10 @@ import useClearFileHistory from "../../../hooks/useClearFileHistory"; import { useCoOwnerManagement } from "../../../hooks/useCoOwnerManagement.jsx"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler.jsx"; import { useExecutionLogs } from "../../../hooks/useExecutionLogs"; -import { usePaginatedList } from "../../../hooks/usePaginatedList"; +import { + applyPagedResponse, + usePaginatedList, +} from "../../../hooks/usePaginatedList"; import usePipelineHelper from "../../../hooks/usePipelineHelper.js"; import { useInitialFetchCount, @@ -39,6 +42,8 @@ import "./Pipelines.css"; function Pipelines({ type }) { const [tableData, setTableData] = useState([]); + // Monotonic request token so a stale response can't overwrite a newer one. + const seqRef = useRef(0); const [openEtlOrTaskModal, setOpenEtlOrTaskModal] = useState(false); const [selectedPorD, setSelectedPorD] = useState({}); const [tableLoading, setTableLoading] = useState(true); @@ -158,26 +163,37 @@ function Pipelines({ type }) { params, }; - axiosPrivate(requestOptions) + const seq = ++seqRef.current; + return axiosPrivate(requestOptions) .then((res) => { - const data = res?.data; - // Handle paginated response - setTableData(data.results || data); - setPagination((prev) => ({ - ...prev, - current: page, + const stepback = applyPagedResponse({ + data: res?.data, + page, pageSize, - total: data.count ?? data.results?.length ?? data.length ?? 0, - })); - - activateScrollRestore(); + seq, + latestSeqRef: seqRef, + setList: setTableData, + setPagination, + refetchPrevPage: () => getPipelineList(page - 1, pageSize, search), + }); + if (seq === seqRef.current) { + activateScrollRestore(); + } + return stepback; }) .catch((err) => { + // A newer request superseded this one — don't surface its error. + if (seq !== seqRef.current) { + return; + } setAlertDetails(handleException(err)); clearPendingScroll(); }) .finally(() => { - setTableLoading(false); + // Only the newest request owns the shared loading state. + if (seq === seqRef.current) { + setTableLoading(false); + } }); }; From d6562f4c0e17a96db3795ebdb7ce119381d14f37 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Thu, 30 Jul 2026 17:47:17 +0530 Subject: [PATCH 17/17] UN-3770 [FIX] Route remaining paginated viewsets through deterministic ordering Greptile P1: ExecutionViewSet still bypassed the global DeterministicOrderingFilter with a plain OrderingFilter, so tied created_at rows could repeat or omit across pages. Four other pre-existing paginated viewsets (tags, usage_v2, dashboard_metrics, pipeline_v2) had the same override. Swapped each to DeterministicOrderingFilter, appending the pk tie-breaker while keeping their existing backends unchanged. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --- backend/dashboard_metrics/views.py | 4 ++-- backend/pipeline_v2/views.py | 4 ++-- backend/tags/views.py | 4 ++-- backend/usage_v2/views.py | 4 ++-- backend/workflow_manager/execution/views/execution.py | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/backend/dashboard_metrics/views.py b/backend/dashboard_metrics/views.py index 394b34d5b9..4c59d1536c 100644 --- a/backend/dashboard_metrics/views.py +++ b/backend/dashboard_metrics/views.py @@ -14,11 +14,11 @@ from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied -from rest_framework.filters import OrderingFilter from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response from rest_framework.throttling import UserRateThrottle +from utils.filters.ordering_filter import DeterministicOrderingFilter from utils.user_context import UserContext from .cache import ( @@ -156,7 +156,7 @@ class DashboardMetricsViewSet(viewsets.ReadOnlyModelViewSet): permission_classes = [IsAuthenticated, IsOrganizationMember] throttle_classes = [MetricsRateThrottle] serializer_class = EventMetricsHourlySerializer - filter_backends = [DjangoFilterBackend, OrderingFilter] + filter_backends = [DjangoFilterBackend, DeterministicOrderingFilter] ordering_fields = ["timestamp", "metric_name", "metric_value"] ordering = ["-timestamp"] diff --git a/backend/pipeline_v2/views.py b/backend/pipeline_v2/views.py index 41024dd409..ec7e7720f3 100644 --- a/backend/pipeline_v2/views.py +++ b/backend/pipeline_v2/views.py @@ -16,11 +16,11 @@ from plugins import get_plugin from rest_framework import serializers, status, viewsets from rest_framework.decorators import action -from rest_framework.filters import OrderingFilter from rest_framework.request import Request from rest_framework.response import Response from rest_framework.versioning import URLPathVersioning from scheduler.helper import SchedulerHelper +from utils.filters.ordering_filter import DeterministicOrderingFilter from utils.pagination import CustomPagination from pipeline_v2.constants import ( @@ -51,7 +51,7 @@ class PipelineViewSet( versioning_class = URLPathVersioning queryset = Pipeline.objects.all() pagination_class = CustomPagination - filter_backends = [OrderingFilter] + filter_backends = [DeterministicOrderingFilter] ordering_fields = ["created_at", "last_run_time", "pipeline_name", "run_count"] # Note: Default ordering with nulls_last is applied in get_queryset() # DRF's ordering attribute doesn't support nulls_last natively diff --git a/backend/tags/views.py b/backend/tags/views.py index d8fcff5a84..b7d3bf43b2 100644 --- a/backend/tags/views.py +++ b/backend/tags/views.py @@ -2,9 +2,9 @@ from permissions.permission import IsOrganizationMember from rest_framework import status, viewsets from rest_framework.decorators import action -from rest_framework.filters import OrderingFilter from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from utils.filters.ordering_filter import DeterministicOrderingFilter from utils.pagination import CustomPagination from workflow_manager.file_execution.serializers import WorkflowFileExecutionSerializer from workflow_manager.workflow_v2.serializers import WorkflowExecutionSerializer @@ -19,7 +19,7 @@ class TagViewSet(viewsets.ModelViewSet): serializer_class = TagSerializer pagination_class = CustomPagination ordering_fields = ["created_at"] - filter_backends = [DjangoFilterBackend, OrderingFilter] + filter_backends = [DjangoFilterBackend, DeterministicOrderingFilter] filterset_fields = ["name"] def get_queryset(self): diff --git a/backend/usage_v2/views.py b/backend/usage_v2/views.py index 414814d80b..f374e0197f 100644 --- a/backend/usage_v2/views.py +++ b/backend/usage_v2/views.py @@ -6,10 +6,10 @@ from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.exceptions import ValidationError -from rest_framework.filters import OrderingFilter from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from utils.date import DateTimeProcessor +from utils.filters.ordering_filter import DeterministicOrderingFilter from utils.pagination import CustomPagination from utils.user_context import UserContext @@ -29,7 +29,7 @@ class UsageView(viewsets.ModelViewSet): permission_classes = [IsAuthenticated, IsOrganizationMember] serializer_class = UsageSerializer pagination_class = CustomPagination - filter_backends = [DjangoFilterBackend, OrderingFilter] + filter_backends = [DjangoFilterBackend, DeterministicOrderingFilter] filterset_class = UsageFilter ordering_fields = ["created_at"] diff --git a/backend/workflow_manager/execution/views/execution.py b/backend/workflow_manager/execution/views/execution.py index 5363ef5082..a80a1e354f 100644 --- a/backend/workflow_manager/execution/views/execution.py +++ b/backend/workflow_manager/execution/views/execution.py @@ -2,8 +2,8 @@ from django_filters.rest_framework import DjangoFilterBackend from rest_framework import viewsets -from rest_framework.filters import OrderingFilter from rest_framework.permissions import BasePermission, IsAuthenticated +from utils.filters.ordering_filter import DeterministicOrderingFilter from utils.pagination import CustomPagination from workflow_manager.execution.filter import ExecutionFilter @@ -34,7 +34,7 @@ class ExecutionViewSet(viewsets.ReadOnlyModelViewSet): permission_classes = [IsAuthenticated, UserWorkflowExecutionPermission] serializer_class = ExecutionSerializer pagination_class = CustomPagination - filter_backends = [DjangoFilterBackend, OrderingFilter] + filter_backends = [DjangoFilterBackend, DeterministicOrderingFilter] ordering_fields = ["created_at", "execution_time"] ordering = ["-created_at"] filterset_class = ExecutionFilter