UN-3769 [FEAT] Sortable resource lists with co-owner ownership - #2200
Conversation
…arch & 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) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds shared backend search and sorting, extends the pagination hook with sort and refresh state, and migrates custom tools, adapters, connectors, and workflows to server-driven paginated ChangesResource listing modernization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ResourceTable
participant usePaginatedList
participant ResourcePage
participant BackendListEndpoint
ResourceTable->>usePaginatedList: change page, search, or sort
usePaginatedList->>ResourcePage: invoke fetchRef with list state
ResourcePage->>BackendListEndpoint: request paginated filtered resources
BackendListEndpoint-->>ResourcePage: return results and count
ResourcePage->>usePaginatedList: apply response with sequence token
usePaginatedList-->>ResourceTable: update rows and pagination
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| frontend/src/hooks/usePaginatedList.js | Centralizes pagination, search, sorting, refresh targeting, stale-response suppression, and empty-page fallback behavior. |
| frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx | Migrates adapter settings to the shared server-backed resource list and delegates mutation refreshes to the pagination hook. |
| frontend/src/components/widgets/resource-table/ResourceTable.jsx | Introduces the shared ownership-aware, sortable resource table used by the migrated list screens. |
| backend/permissions/models.py | Adds a shared helper that identifies the earliest live non-service-account owner for list serialization. |
| backend/utils/tests/test_list_pagination.py | Extends list endpoint contracts for name-only search, dropped owner ordering, and owner-email selection. |
Reviews (20): Last reviewed commit: "UN-3769 [FIX] Tests: pin name-only searc..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
kirtimanmishrazipstack
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
frontend/src/components/widgets/resource-table/ResourceTable.jsx (1)
238-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeprecation tooltip is hardcoded to "adapter".
ResourceTableis shared across Workflow/Connector/Prompt types, butdisabledTitlesays "This adapter is deprecated". Practically only adapters setis_deprecated, so it's rarely reachable elsewhere — still, usetypefor correctness if that ever changes.Proposed tweak
- const disabledTitle = deprecated ? "This adapter is deprecated" : ""; + const disabledTitle = deprecated ? `This ${type || "item"} is deprecated` : "";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/widgets/resource-table/ResourceTable.jsx` around lines 238 - 240, Update renderActions so the deprecation disabledTitle uses the ResourceTable type dynamically instead of hardcoding “adapter”. Preserve the existing empty title for non-deprecated items and ensure the resulting message remains grammatically correct for the supported resource types.frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx (1)
167-228: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the duplicated paginated-fetch flow into a shared helper. All four resource pages reimplement the identical params-building,
results ?? data ?? []/count ?? lengthenvelope parsing, empty-page step-back recursion, and error fallback. Consolidating into one helper (e.g.runPaginatedFetch({ request, page, pageSize, search, sortBy, order, setList, setPagination, onError })) removes ~4× drift risk as these evolve.
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx#L167-L228: replacegetListOfToolsbody with a call to the shared helper (Prompt Studio URL + params).frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx#L147-L212: replacegetAdaptersbody, keeping theadapter_type/typeguard as helper input.frontend/src/pages/ConnectorsPage.jsx#L120-L168: replacegetConnectorsbody with the shared helper.frontend/src/components/workflows/workflow/Workflows.jsx#L124-L169: replacegetProjectListbody with the shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx` around lines 167 - 228, Extract the duplicated paginated-fetch behavior into a shared helper such as runPaginatedFetch, centralizing parameter construction, envelope parsing, empty-page step-back recursion, loading/error fallback, and pagination updates. Replace getListOfTools in frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx:167-228 with the helper while preserving its Prompt Studio request; replace getAdapters in frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx:147-212 while passing through its adapter_type/type guard; replace getConnectors in frontend/src/pages/ConnectorsPage.jsx:120-168 and getProjectList in frontend/src/components/workflows/workflow/Workflows.jsx:124-169 similarly, preserving each request URL and list-specific state setters.backend/adapter_processor_v2/views.py (1)
182-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
select_related/prefetch_relatedbeforeapply_search_and_sort.apply_search_and_sortre-wraps the queryset viamodel.objects.filter(pk__in=queryset.order_by("pk").values("pk")), so relations attached to the queryset before the helper call are dropped — only theselect_related=/prefetch_related=kwargs passed into the helper actually take effect.workflow_v2/views.py'sget_queryset()already avoids this by only attaching relations via the helper call.
backend/adapter_processor_v2/views.py#L182-L186: drop the pre-helper.select_related("created_by").prefetch_related("memberships__user"), since lines 196-203 already pass these as helper kwargs.backend/connector_v2/views.py#L93-L97: drop the pre-helper.select_related("created_by").prefetch_related("memberships__user"), since lines 129-136 already pass these as helper kwargs.backend/prompt_studio/prompt_studio_core_v2/views.py#L158-L160: drop the pre-helper.prefetch_related("memberships__user"), since lines 165-172 already pass it as a helper kwarg.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/adapter_processor_v2/views.py` around lines 182 - 186, Remove the pre-helper relation loading from get_queryset in backend/adapter_processor_v2/views.py lines 182-186, backend/connector_v2/views.py lines 93-97, and backend/prompt_studio/prompt_studio_core_v2/views.py lines 158-160; retain the existing select_related and prefetch_related kwargs passed to apply_search_and_sort, which should be the only relation-loading configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/utils/list_query.py`:
- Around line 60-65: Normalize the `sort_by` value to lowercase before looking
it up in the mapping within the sort-field selection logic, while preserving the
existing default behavior when it is missing or unsupported. Keep the `order`
handling and the existing `name`, `owner`, and `created` mappings unchanged.
In `@frontend/src/components/widgets/resource-table/ResourceTable.jsx`:
- Around line 148-150: Update the isImage detection in renderName to identify
actual image URL or data schemes instead of using icon.length > 4. Ensure
compound and multi-codepoint emoji remain rendered as icons, while valid remote
or data image sources still use the image rendering path.
---
Nitpick comments:
In `@backend/adapter_processor_v2/views.py`:
- Around line 182-186: Remove the pre-helper relation loading from get_queryset
in backend/adapter_processor_v2/views.py lines 182-186,
backend/connector_v2/views.py lines 93-97, and
backend/prompt_studio/prompt_studio_core_v2/views.py lines 158-160; retain the
existing select_related and prefetch_related kwargs passed to
apply_search_and_sort, which should be the only relation-loading configuration.
In `@frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx`:
- Around line 167-228: Extract the duplicated paginated-fetch behavior into a
shared helper such as runPaginatedFetch, centralizing parameter construction,
envelope parsing, empty-page step-back recursion, loading/error fallback, and
pagination updates. Replace getListOfTools in
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx:167-228 with
the helper while preserving its Prompt Studio request; replace getAdapters in
frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx:147-212
while passing through its adapter_type/type guard; replace getConnectors in
frontend/src/pages/ConnectorsPage.jsx:120-168 and getProjectList in
frontend/src/components/workflows/workflow/Workflows.jsx:124-169 similarly,
preserving each request URL and list-specific state setters.
In `@frontend/src/components/widgets/resource-table/ResourceTable.jsx`:
- Around line 238-240: Update renderActions so the deprecation disabledTitle
uses the ResourceTable type dynamically instead of hardcoding “adapter”.
Preserve the existing empty title for non-deprecated items and ensure the
resulting message remains grammatically correct for the supported resource
types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0fb0d9b1-fc7b-4327-a330-c6b7dc079fe9
📒 Files selected for processing (16)
backend/adapter_processor_v2/views.pybackend/connector_v2/views.pybackend/prompt_studio/prompt_studio_core_v2/views.pybackend/utils/list_query.pybackend/workflow_manager/workflow_v2/views.pyfrontend/src/components/custom-tools/list-of-tools/ListOfTools.jsxfrontend/src/components/custom-tools/view-tools/ViewTools.cssfrontend/src/components/custom-tools/view-tools/ViewTools.jsxfrontend/src/components/tool-settings/tool-settings/ToolSettings.jsxfrontend/src/components/widgets/list-view/ListView.cssfrontend/src/components/widgets/list-view/ListView.jsxfrontend/src/components/widgets/resource-table/ResourceTable.cssfrontend/src/components/widgets/resource-table/ResourceTable.jsxfrontend/src/components/workflows/workflow/Workflows.jsxfrontend/src/hooks/usePaginatedList.jsfrontend/src/pages/ConnectorsPage.jsx
💤 Files with no reviewable changes (4)
- frontend/src/components/widgets/list-view/ListView.css
- frontend/src/components/custom-tools/view-tools/ViewTools.css
- frontend/src/components/custom-tools/view-tools/ViewTools.jsx
- frontend/src/components/widgets/list-view/ListView.jsx
Resolve the sole modify/delete conflict on ListView.css in favour of the deletion — the dead ListView component is removed by this branch, so main's CSS tweak to it is moot. prompt_studio_core_v2/views.py auto-merged cleanly (our get_queryset rewrite + main's PG task_status block occupy different regions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…se 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 <img>. - 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) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/hooks/usePaginatedList.js (1)
141-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse these shared helpers in the resource pages.
The supplied graph context still shows local copies of
buildPagedParamsandapplyPagedResponseinfrontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx,frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx,frontend/src/components/workflows/workflow/Workflows.jsx, andfrontend/src/pages/ConnectorsPage.jsx. Import the helpers from this module and remove those copies so fixes cannot diverge across listing surfaces.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/usePaginatedList.js` at line 141, Replace the local buildPagedParams and applyPagedResponse implementations in ListOfTools, ToolSettings, Workflows, and ConnectorsPage with imports from usePaginatedList.js, removing the duplicated helper definitions while preserving each page’s existing pagination behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/hooks/usePaginatedList.js`:
- Around line 28-71: Update the paginated-list request callers, including
ConnectorsPage and equivalent resource pages, so catch and finally handlers only
apply error or loading-state updates when their request sequence still matches
seqRef.current. Preserve the existing stale-response suppression and ensure a
refetch returned by applyPagedResponse lets the newest request retain ownership
of loading until replacement data completes.
---
Nitpick comments:
In `@frontend/src/hooks/usePaginatedList.js`:
- Line 141: Replace the local buildPagedParams and applyPagedResponse
implementations in ListOfTools, ToolSettings, Workflows, and ConnectorsPage with
imports from usePaginatedList.js, removing the duplicated helper definitions
while preserving each page’s existing pagination behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 217ff658-9d02-434f-b90e-d0be05fab4e7
📒 Files selected for processing (7)
backend/utils/list_query.pyfrontend/src/components/custom-tools/list-of-tools/ListOfTools.jsxfrontend/src/components/tool-settings/tool-settings/ToolSettings.jsxfrontend/src/components/widgets/resource-table/ResourceTable.jsxfrontend/src/components/workflows/workflow/Workflows.jsxfrontend/src/hooks/usePaginatedList.jsfrontend/src/pages/ConnectorsPage.jsx
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/src/pages/ConnectorsPage.jsx
- frontend/src/components/workflows/workflow/Workflows.jsx
- backend/utils/list_query.py
- frontend/src/components/widgets/resource-table/ResourceTable.jsx
- frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx
- frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx
…tion 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 <CoOwnerModal .../> 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsx`:
- Around line 177-187: Update the ListOfTools request state around the fetch
catch handler to track an explicit list error instead of setting displayList to
an empty array on failure. Clear the error when starting or successfully
completing a current request, and render a retryable error for initial or
empty-list failures; gate the normal empty-state branches around the existing
“No prompt projects available” and “No results found” rendering on !isLoading &&
!listError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 85878000-1314-4bfa-aa3c-e35f109d93d3
📒 Files selected for processing (4)
frontend/src/components/custom-tools/list-of-tools/ListOfTools.jsxfrontend/src/components/tool-settings/tool-settings/ToolSettings.jsxfrontend/src/components/workflows/workflow/Workflows.jsxfrontend/src/pages/ConnectorsPage.jsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/pages/ConnectorsPage.jsx
- frontend/src/components/tool-settings/tool-settings/ToolSettings.jsx
- frontend/src/components/workflows/workflow/Workflows.jsx
…ointAt - 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) <noreply@anthropic.com>
… 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
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 <noreply@anthropic.com>
kirtimanmishrazipstack
left a comment
kirtimanmishrazipstack
left a comment
There was a problem hiding this comment.
Second-pass review (self-review). Pass-1 findings are both closed — the is_owner fix at ResourceTable.jsx:190 holds and preserves the creator-sees-"Me" case. CI is fully green here.
This pass focused on the 5 commits reworking the list fetch state machine, plus a fresh look at the owner axis. 3 High + 3 Medium, all verified against the source rather than inferred from the diff.
- High 1 — the OSS "Owned By" column can name someone who is no longer an owner
- High 2 —
loadErroris unreachable after the first successful load, on all 5 pages - High 3 — the Workflows list-fetch catch reports nothing (compounds High 2 into total silence)
- Medium 1 —
syncRequestedcan't undo a failed search/sort; its comment claims otherwise - Medium 2 —
requestListvoids the stepback contractapplyPagedResponsedocuments - Medium 3 — stale rationale on the
_prompt_countSubquery
Also noted, not blocking: the ?order_by param on /workflow/ is now silently ignored (main honoured it for modified_at). No frontend senders remain, so it just needs a line in the breaking-change section rather than a code change.
Separately — the .distinct() calls in all five for_user() managers are no-ops (the sharing helpers return ValuesQuerySet subqueries used as IN, which can't multiply outer rows; they're leftovers from the pre-ResourceMembership join era). That does not make the pk__in re-wrap wrong — while those .distinct() calls stand, Postgres still requires ORDER BY to lead with the DISTINCT ON expression, so the re-wrap is load-bearing exactly as written. Worth a follow-up ticket that deletes the .distinct() calls, the re-wrap, both hint params, and the _prompt_count Subquery together.
- 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 <noreply@anthropic.com>
…resource-list-views
…resource-list-views
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 <noreply@anthropic.com>
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
Unstract test resultsPer-group results
Critical paths
|
Lighten avatar initial color (Ant -7 -> -6) and reduce initial size (12px -> 11px) per design review feedback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Reviewed this alongside #2208, which overlaps heavily. Summary plus a suggested path. tl;dr — the feature here (owner column, unified Why stackBoth PRs hit the same wall:
Stacked, Suggested order
Merging the two OSS PRs back-to-back rather than separately, so What the rebase looks likeMostly "take this PR's version":
Review points
Verified separately, both safe: |
…sions 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159NvRHFywvkNGji8ECQqeV
…esource-list sort to ?ordering= Resolve the 4 backend views.py conflicts by taking #2208's declarative ordering (?ordering= + DeterministicOrderingFilter) and folding owner sort/search back in (created_by__email in ordering_fields + name/owner search). Delete utils/list_query.py; frontend sort emits ?ordering=. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
…format, owner search-only Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Re-reviewed at current HEAD. The rebase onto Remaining concerns, roughly in order: 1. Search matches the creator, the column shows the owner. All four viewsets now filter on 2. owner_sq = (
ResourceMembership.objects.filter(
object_id=OuterRef("pk"),
content_type=ct,
role=ResourceRole.OWNER,
user__is_service_account=False,
)
.order_by("created_at")
.values("user__email")[:1]
)
3. No tests. Nothing added or touched across either PR. 4. Displayed sort and requested sort disagree. Two symptoms, one cause:
Both go away if "no sort" stops being a state. The viewset always applies an ordering, so clearing should restore the default rather than clear it: const nextSort = sortBy
? { sortBy, order: order || "asc" }
: { sortBy: defaultSortBy, order: defaultOrder };and mount should route through Minor
On comments — could you trim the new ones to be shorter and less situational? Several read as design notes and reference things that will drift. A few examples:
A single line on why is usually enough, and it survives refactors better than a paragraph on how. Same for the JSDoc — worth keeping where a param is non-obvious, but a lot of it currently narrates the implementation. Nothing above blocks the stack. (1) and (3) are the two I'd want resolved before merge. |
…t 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 <noreply@anthropic.com>
|
@chandrasekharan-zipstack thanks, went through all of it. Pushed 1 + 2 — Owned By is now display-only. Dropped 4 — sort-state fixed. Minors
Comments — trimmed the ones you flagged (the |
…lback, 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 <noreply@anthropic.com>
|
chandrasekharan-zipstack
left a comment
chandrasekharan-zipstack
left a comment
There was a problem hiding this comment.
Re-reviewed at 8401a95c. All the points from my last pass are addressed: owner search/sort divergence removed by scoping search to name-only and dropping created_by__email from ordering_fields across the four viewsets, clear-sort now restores the default ordering, list mounts request the seeded sort so header and rows agree, dead code (useListSearch.js, the initials branch, the unreferenced .listWrapper rule) gone, and the new comments trimmed. CI is green.
One thing left open, not blocking: there are still no tests for owner_email() (service-account filter, earliest-OWNER tiebreak, the None case) or the new ordering_fields. backend/utils/tests/test_list_pagination.py already covers the four endpoints and would take these cheaply — worth a follow-up.
Also noting search no longer matches owners at all. Consistent, and the right call for now, but if UN-3769 wanted owner search it's deferred with nothing tracking it.
Approving — merging into feat/list-pagination-consistency so the stack goes to main as one change.
|
@chandrasekharan-zipstack #3 (tests) is done — added in
All pass locally: |
fd40c16
into
feat/list-pagination-consistency
… endpoints (#2208) * 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 `<Pagination>` 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159NvRHFywvkNGji8ECQqeV * 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) <noreply@anthropic.com> * 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 <img>. - 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) <noreply@anthropic.com> * 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 <CoOwnerModal .../> 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * UN-3769 [FEAT] Resource list: relative Modified time, canonical date format, owner search-only Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * UN-3770 [FEAT] Search resource lists by owner too, not just name 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * UN-3770 [FEAT] List all co-owners in the Owned By tooltip 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * UN-3770 [REFACTOR] Collapse owner_email into owner_emails 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * UN-3770 [FEAT] ResourceTable: extraColumns + onRowClick (for lookups) (#2221) 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 <noreply@anthropic.com> * chore: re-trigger pre-commit.ci (transient mergeable-check error) * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Kirtiman Mishra <110175055+kirtimanmishrazipstack@users.noreply.github.com>
2 participants
What
ResourceTable(Name / Owned By / Created / Modified / Actions) replacesListView/ViewToolsacross Adapters, Workflows, Prompt Studio and Connectors.Prompts: Ncount; both Created and Modified columns are sortable.-modified_at) so the visible Modified column matches the sort order.Why
How
for_user()managers use plain.distinct()and each viewset is vanilla DRF (ordering = ["-modified_at", "pk"]+ordering_fields), filtered by the globalDeterministicOrderingFilter. Sort/search is standard?ordering=/?search=; Prompt Studio annotatesprompt_count.ResourceTableuses custom sort-dropdown headers;usePaginatedListowns sort state and pages fetch?ordering=/?search=server-side, defaulting tomodified_atdesc.ListViewandViewTools(0 remaining consumers).Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
for_user()-filtered. Endpoints keep the bare-array response unless?page/?page_sizeis sent, so existing API callers are unaffected.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
mainfor the sharedResourceTable).Dependencies Versions
Notes on Testing
-modified_at), instead ofname asc. This restores the-modified_atordering UN-3770 [FEAT] Opt-in pagination for list endpoints; wire Workflows page #2187 gave Workflows. Both Created and Modified columns are sortable; the default order shows as active on the Modified header on load.Screenshots
Add before merge.
Checklist
I have read and understood the Contribution Guidelines.