UN-3770 [FEAT] ResourceTable: extraColumns + onRowClick (for lookups) - #2221
Merged
chandrasekharan-zipstack merged 1 commit intoJul 30, 2026
Conversation
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. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ
Contributor
|
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:
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 |
|
chandrasekharan-zipstack
marked this pull request as ready for review
July 30, 2026 05:06
Contributor
|
| Filename | Overview |
|---|---|
| frontend/src/components/widgets/resource-table/ResourceTable.jsx | Adds optional custom columns and row-click handling without changing the four existing consumers. |
Reviews (1): Last reviewed commit: "UN-3770 [FEAT] ResourceTable: extraColum..." | Re-trigger Greptile
chandrasekharan-zipstack
merged commit Jul 30, 2026
3853476
into
feat/list-pagination-consistency
6 checks passed
chandrasekharan-zipstack
added a commit
that referenced
this pull request
Jul 31, 2026
… 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two small additions to the shared
ResourceTablewidget so the Prompt Studio lookups table can reuse it:extraColumns— resource-specific antd column defs, injected before the Actions column. Lookups uses this for its Files and Latest Version columns.onRowClick(item)— overrides the default relative row-click nav, so callers can navigate to an absolute, org-scoped path with router state (lookups needs this).No behavior change for the existing 4 list pages (both props are optional).
Stacking
Based on
feat/list-pagination-consistency(#2208) — merge that first. The cloud consumer is Zipstack/unstract-cloudfeat/lookups-table-consistency.🤖 Generated with Claude Code
https://claude.ai/code/session_018HXYFVCZGi7YN9qjDGyJCQ