UN-3154 [FEAT] Card based layout to list Pipelines and API Deployments #1769
Conversation
- Add WorkflowExecution.get_last_run_statuses() method to fetch last N executions with PARTIAL_SUCCESS computed dynamically - Add last_5_run_statuses, run_count, last_run_time to API deployment serializer - Add last_5_run_statuses, next_run_time to pipeline serializer - Add source/destination instance names to pipeline representation - Enable pagination (CustomPagination) for both pipelines and deployments - Add search filter by name for both endpoints Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create CardGridView widget for displaying items in card/list layouts - Add CardItem component with expand/collapse, status badges, actions - Add LoadingSkeleton for loading states - Support pagination, search, scroll restoration, and card configuration - Add dateFormatter helper for consistent date formatting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add ApiDeploymentCardConfig for card layout configuration - Refactor ApiDeployment to use CardGridView with pagination - Update api-deployments-service with pagination and search support - Refactor Body component to use CardGridView instead of Table - Add search functionality to Header component - Update Layout to pass card configuration and pagination props Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add PipelineCardConfig with StatusPills component for execution status - Display source/destination connector icons and instance names - Show last 5 run statuses with clickable navigation to execution logs - Add schedule display, next run time, and run count - Support pagination and search functionality Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update MenuLayout to forward scrollToCardId state on back navigation - Pass scrollToCardId from workflow links in card configs - Add back route with state to ExecutionLogs for scroll restoration - Remove redundant back button from DetailedLogs header - Add previousRoute and previousRouteState props to ToolNavBar Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds server-driven pagination and search to deployment and pipeline endpoints, augments serializers with run-count and recent-run metadata, implements WorkflowExecution.get_last_run_statuses, and replaces table lists with a reusable CardGridView UI plus routing/state-preservation for navigation, search, and pagination. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
53-53: Avoid usingrequire()inside component body.Using
require()inside a React component is non-idiomatic and can cause issues with tree-shaking and bundling. This should be a static import at the top of the file.♻️ Proposed fix: Use static import
import { LogsModal } from "../log-modal/LogsModal.jsx"; + import { fetchExecutionLogs } from "../log-modal/fetchExecutionLogs.js"; import { EtlTaskDeploy } from "../etl-task-deploy/EtlTaskDeploy.jsx"; // ... inside component, remove: - const { fetchExecutionLogs } = require("../log-modal/fetchExecutionLogs.js");
🤖 Fix all issues with AI agents
In `@frontend/src/components/logging/execution-logs/ExecutionLogs.jsx`:
- Around line 29-30: In ExecutionLogs.jsx remove the unused "type" variable from
the destructuring of useParams() so only the needed "id" is extracted (i.e.
change the const { id, type } = useParams() usage to only extract id); this
keeps the component clean since the child component DetailedLogs already calls
useParams() to get type.
In
`@frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx`:
- Around line 199-201: The code uses cronstrue.toString(pipeline.cron_string)
directly which can throw on invalid cron expressions; wrap the conversion in a
try-catch inside the component (the logic that sets scheduleDisplay) so that if
cronstrue.toString throws, you fall back to "Not scheduled" or another safe
message and optionally log the error; update the assignment for scheduleDisplay
(the pipeline.cron_string -> cronstrue.toString usage) to handle exceptions
gracefully.
In `@frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx`:
- Around line 262-270: After enabling/disabling a pipeline the code calls
getPipelineList() with no args which resets pagination to page 1; update the
success handler of axiosPrivate in the enable/disable flow to call
getPipelineList with the current page state (the same pagination value used by
the delete handler) so the list refresh preserves the user's current page—locate
the axiosPrivate call and replace getPipelineList() with
getPipelineList(currentPage) (or the existing pagination variable name) while
keeping the existing failure rollback (handleLoaderInTableData) and error
handling (setAlertDetails(handleException(err))).
In `@frontend/src/components/widgets/card-grid-view/CardGridView.jsx`:
- Around line 125-126: The comparisons using strict equality between
forceExpandedId/scrollToId and item.id can fail if one side is a string and the
other a number; update the checks in CardGridView (where
forceExpanded={forceExpandedId === item.id} and scrollIntoView={scrollToId ===
item.id}) to perform a type-safe comparison (e.g., coerce both sides to string
before comparing: String(forceExpandedId) === String(item.id) and
String(scrollToId) === String(item.id)), and update the component PropTypes for
forceExpandedId and scrollToId (in the PropTypes block) to accept
oneOfType([PropTypes.string, PropTypes.number]) so callers and type-checking
align.
In `@frontend/src/helpers/GetStaticData.js`:
- Around line 548-554: shortenApiEndpoint currently uses split("/") which can
include query/hash or return the host for host-only URLs; update
shortenApiEndpoint to parse the input (using the URL constructor where possible,
falling back to treating it as a relative path), strip search and hash, trim
trailing slashes, then take only the last pathname segment as the suffix (with a
safe fallback to the entire sanitized path or the original input when parsing
fails) so you don't leak query tokens or produce host-only labels.
In `@frontend/src/layouts/menu-layout/MenuLayout.jsx`:
- Around line 38-44: The onClick handler in MenuLayout.jsx currently calls
navigate(location.state.from, { state: location.state?.scrollToCardId ? {
scrollToCardId: ... } : undefined }) which overwrites any existing state on
location.state.from; change the call so if location.state.from is a location
object with its own state you merge its state with { scrollToCardId } (e.g.,
build a mergedState = { ...(location.state.from.state || {}),
...(location.state?.scrollToCardId ? { scrollToCardId: ... } : {}) } and pass
that mergedState to navigate) and ensure you still pass the original location
object (location.state.from) to navigate so pathname/key/search are preserved.
🧹 Nitpick comments (19)
backend/workflow_manager/workflow_v2/models/execution.py (2)
401-406: Consider usingExecutionStatusenum values instead of hardcoded strings.The
ExecutionStatusenum is already imported in this file (line 18). Using the enum values would be more maintainable and type-safe.♻️ Suggested refactor
successful = WorkflowFileExecution.objects.filter( - workflow_execution_id=e.id, status="COMPLETED" + workflow_execution_id=e.id, status=ExecutionStatus.COMPLETED.value ).count() failed = WorkflowFileExecution.objects.filter( - workflow_execution_id=e.id, status="ERROR" + workflow_execution_id=e.id, status=ExecutionStatus.ERROR.value ).count()Also update the condition on line 410:
- if e.status == "COMPLETED" and failed > 0 and successful > 0: + if e.status == ExecutionStatus.COMPLETED.value and failed > 0 and successful > 0:
391-406: Acknowledged: N+1 query pattern documented for future optimization.The TODO comment appropriately documents the performance concern. Given the small default limit (5), this is acceptable for now. For future optimization, consider using Django's
annotate()with conditional aggregation:from django.db.models import Count, Q executions = cls.objects.filter(pipeline_id=pipeline_id).annotate( successful=Count('file_executions', filter=Q(file_executions__status='COMPLETED')), failed=Count('file_executions', filter=Q(file_executions__status='ERROR')) ).order_by("-created_at")[:limit]This would reduce 11 queries to 1.
backend/pipeline_v2/serializers/crud.py (1)
213-222: Consider logging the exception for debugging purposes.The bare
except Exceptioncatch is flagged by static analysis (BLE001). While catching a broad exception is acceptable here sincecronitercan raise various exception types for edge cases, logging the error would aid debugging whennext_run_timeunexpectedly returnsNone.♻️ Suggested improvement
def get_next_run_time(self, instance: Pipeline) -> str | None: """Calculate next scheduled run time from cron expression.""" if not instance.cron_string or not instance.active: return None try: cron = croniter(instance.cron_string, timezone.now()) next_run: datetime = cron.get_next(datetime) return next_run.isoformat() - except Exception: + except Exception as e: + logger.warning( + f"Failed to calculate next run time for pipeline {instance.id}: {e}" + ) return Nonefrontend/src/components/navigations/tool-nav-bar/ToolNavBar.jsx (1)
81-81: Consider usingPropTypes.shapefor stricter validation.The
previousRouteStateprop is typed as a genericPropTypes.object. Based on the usage inExecutionLogs.jsx, you could define a more specific shape for better documentation and validation.🔧 Optional: Add shape validation
- previousRouteState: PropTypes.object, + previousRouteState: PropTypes.shape({ + scrollToCardId: PropTypes.string, + cardExpanded: PropTypes.bool, + }),frontend/src/components/deployments/api-deployment/api-deployments-service.js (1)
4-4: Pre-existing: Shared mutableoptionsvariable could cause race conditions.The module-level
let options = {}is reassigned by every method, which could cause issues if multiple concurrent API calls are made (e.g., rapid pagination clicks). This is a pre-existing pattern, so not blocking, but worth noting for future improvement.♻️ Suggested pattern: Use local `const` in each method
-let options = {}; - function apiDeploymentsService() { // ... return { getApiDeploymentsList: (page = 1, pageSize = 10, search = "") => { const params = { page, page_size: pageSize }; if (search) params.search = search; - options = { + const options = { url: `${path}/api/deployment/`, method: "GET", params, }; return axiosPrivate(options); }, // Apply same pattern to other methods... }; }frontend/src/components/widgets/card-grid-view/CardGridView.css (1)
638-641: Consider removing the unused.connector-flow-arrowrule.The comment states this element is "no longer used in vertical layout" and the rule just hides it. If the arrow element has been removed from the JSX, this CSS rule can be deleted entirely. If it might be reintroduced, consider adding a TODO comment.
frontend/src/helpers/dateFormatter.js (2)
64-76: Future dates may produce unexpected relative formatting.When
dateis in the future,differenceInHours(now, date)anddifferenceInMinutes(now, date)return negative values. This means a date 5 minutes in the future would pass theminutesAgo < 1check (since -5 < 1) and display "Just now", which is misleading.If future dates are valid inputs (e.g.,
next_run_time), consider adding explicit handling:💡 Proposed fix to handle future dates
// Handle relative formatting for recent dates if (relative) { const hoursAgo = differenceInHours(now, date); const minutesAgo = differenceInMinutes(now, date); + // Skip relative formatting for future dates + if (minutesAgo < 0) { + return includeTime + ? format(date, "dd MMM yyyy HH:mm") + : format(date, "dd MMM yyyy"); + } + // "Just now" for < 1 minute if (minutesAgo < 1) { return "Just now"; }
51-59: Consider extracting shared date parsing logic.The try/catch parsing pattern is duplicated in
formatCompactDate,formatFullDate, andcheckIsToday. A small helper would reduce repetition.♻️ Optional: Extract parsing helper
/** * Safely parse a date value * `@param` {string|Date} value - Date string or Date object * `@return` {Date|null} Parsed Date or null if invalid */ function safeParse(value) { if (!value) return null; try { const date = typeof value === "string" ? parseISO(value) : new Date(value); return isValid(date) ? date : null; } catch { return null; } }Then usage becomes:
const date = safeParse(isoDate); if (!date) return isoDate; // or "-" or false depending on functionfrontend/src/components/deployments/api-deployment/ApiDeploymentCardConfig.jsx (2)
320-333: PropTypes on a factory function have no runtime effect.
createApiDeploymentCardConfig.propTypeswon't be validated by React since this is a plain function returning a config object, not a React component. This serves as documentation but provides no runtime type checking.Consider either:
- Using JSDoc
@paramtypes (already partially done) for documentation- Using TypeScript for actual type safety
- Moving PropTypes validation to where the config is consumed
257-261: Prop naming mismatch:pipelineIdused for deployment ID.
StatusPillsreceivespipelineId={deployment.id}but this is an API deployment, not a pipeline. Consider renaming the prop to a more genericentityIdinStatusPillsto avoid confusion when reused across different entity types.frontend/src/components/widgets/card-grid-view/CardGridView.jsx (1)
78-83: Index-based key fallback may cause reconciliation issues.Using
indexas the fallback key (line 82) can cause incorrect component reuse if items are reordered, inserted, or deleted. While this is a reasonable fallback, consider logging a dev warning when the fallback is used to help identify data without proper keys.frontend/src/components/deployments/api-deployment/ApiDeployment.jsx (3)
291-308: Missing dependencies inuseMemomay cause stale closures.The
useMemodependency array only includes[sessionDetails, location], but the config references multiple handler functions (handleEditDeployment,handleShareDeployment,handleDeleteDeployment, etc.) that are recreated on every render. Since these handlers reference state likepagination, they could become stale.Consider either:
- Adding all handlers to the dependency array
- Wrapping handlers in
useCallbackwith proper dependencies- Using refs to access current values within handlers
♻️ Option: Wrap handlers in useCallback
+ const handleEditDeployment = useCallback(() => { + openAddModal(true); + }, []); + const handleDeleteDeployment = useCallback(() => { + setOpenDeleteModal(true); + }, []); // ... similar for other handlers const apiDeploymentCardConfig = useMemo( () => createApiDeploymentCardConfig({ // ... }), - [sessionDetails, location] + [sessionDetails, location, handleEditDeployment, handleShareDeployment, handleDeleteDeployment, /* ... other handlers */] );
92-99: Consider combining effects or removing redundant state.
filteredDatais set directly fromtableDatawithout any transformation. Since search is now server-side (line 96-99), this intermediate state appears unnecessary. You could passtableDatadirectly to theLayoutcomponent.♻️ Simplify by removing filteredData
- const [filteredData, setFilteredData] = useState([]); - useEffect(() => { - setFilteredData(tableData); - }, [tableData]); // In the Layout component: - tableData={filteredData} + tableData={tableData}
115-138: Pagination total calculation may be incorrect when data is not paginated.On line 129, when the response is not paginated (
data.countis undefined), the fallback uses the current page's result length rather than the actual total. This would break pagination controls if the backend returns unpaginated data.total: data.count || (data.results ? data.results.length : data.length),If the API can return both paginated and unpaginated responses, the non-paginated case should set
totalto the full array length, which is correct. However, if the backend always returns paginated responses now, consider simplifying this logic.frontend/src/components/deployments/body/Body.jsx (1)
21-23: Redundant loading state handling.The
isTableLoadingcheck at line 21-23 shows aSpinnerLoaderand returns early, butloading={isTableLoading}is also passed toCardGridViewat line 42. Since the early return preventsCardGridViewfrom ever receivingloading={true}, theloadingprop is effectively alwaysfalse.Either remove the early return and let
CardGridViewhandle the loading state internally (viaLoadingSkeleton), or remove theloadingprop fromCardGridView.♻️ Option 1: Let CardGridView handle loading
- if (isTableLoading) { - return <SpinnerLoader />; - } if (!tableData?.length) { + if (isTableLoading) { + return <SpinnerLoader />; + } return ( <IslandLayout> <EmptyStateAlso applies to: 39-49
frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx (1)
302-320: Popconfirm delete flow may have stalepipelinereference.In the
onConfirmcallback (lines 305-308),setSelectedPorD(pipeline)is called beforeonDelete?.(pipeline). IfonDeletetriggers a re-render or async operation that depends onselectedPorD, there could be a race condition sincesetSelectedPorDis asynchronous.However, since
pipelineis passed directly toonDelete, this should work correctly. ThesetSelectedPorDcall might be redundant here ifonDeletedoesn't rely on it.frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (2)
465-490: SameuseMemodependency issue as ApiDeployment.jsx.The dependency array
[sessionDetails, isClearingFileHistory, location, type]is missing the handler functions that are referenced in the config. This could lead to stale closures when handlers depend on state that changes.
104-114: Scroll restoration timer works but doesn't clean up navigation state.The scroll restoration correctly clears
scrollRestoreIdafter 500ms, but thelocation.state.scrollToCardIdremains in the browser history. If the user navigates away and back, it may trigger again unexpectedly. Consider usingnavigatewithreplace: trueto clear the state after handling.frontend/src/components/widgets/card-grid-view/CardItem.jsx (1)
82-98: Helper functions recreated on every render.
resolveValue,isFieldVisible,renderField,renderSection, andrenderActionsare all defined inside the component and recreated on every render. For a card component rendered multiple times in a grid, this could have minor performance implications.However, since these functions depend on
itemandconfigfrom props, extracting them would require passing these as arguments. The current approach is acceptable for most use cases.
frontend/src/components/logging/execution-logs/ExecutionLogs.jsx
Outdated
Show resolved
Hide resolved
frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx
Outdated
Show resolved
Hide resolved
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx
Outdated
Show resolved
Hide resolved
frontend/src/components/widgets/card-grid-view/CardGridView.jsx
Outdated
Show resolved
Hide resolved
chandrasekharan-zipstack
requested review from
athul-rs,
jagadeeswaran-zipstack,
muhammad-ali-e and
vishnuszipstack
- Remove unused variables: type, filteredData, copyUrl, handleStatusRefresh, getPipelineData, sessionDetails - Wrap cronstrue.toString() in try-catch for invalid cron expressions - Use current pagination when refreshing after pipeline enable/disable - Use String() coercion for type-safe ID comparison in CardGridView - Update PropTypes to accept string or number for IDs - Improve shortenApiEndpoint to strip query/hash and handle edge cases - Merge state properly in MenuLayout back navigation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/deployments/api-deployment/ApiDeployment.jsx (1)
163-175: Direct object mutation inupdateStatusmay cause stale UI or React warnings.Line 165 directly mutates the
recordobject (record.is_active = !record?.is_active). This is an anti-pattern in React since:
- It mutates state directly instead of using setState
- The UI update relies on the component re-rendering from
setIsTableLoading, not from actual data changes- If the API call fails, there's no rollback mechanism
Consider using the optimistic update pattern like in
Pipelines.jsx:🔧 Proposed fix with optimistic update and rollback
const updateStatus = (record) => { - setIsTableLoading(true); - record.is_active = !record?.is_active; + const newStatus = !record?.is_active; + // Optimistic update + setTableData((prevData) => + prevData.map((item) => + item.id === record.id ? { ...item, is_active: newStatus } : item + ) + ); + apiDeploymentsApiService - .updateApiDeployment(record) - .then(() => {}) + .updateApiDeployment({ ...record, is_active: newStatus }) + .then(() => { + // Optionally refresh to sync with server + getApiDeploymentList(pagination.current, pagination.pageSize); + }) .catch((err) => { + // Rollback on failure + setTableData((prevData) => + prevData.map((item) => + item.id === record.id ? { ...item, is_active: !newStatus } : item + ) + ); setAlertDetails(handleException(err)); - }) - .finally(() => { - setIsTableLoading(false); }); };
🤖 Fix all issues with AI agents
In `@frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx`:
- Around line 98-100: The component calls setFilteredData but never declares the
corresponding state; add a useState declaration for filteredData and
setFilteredData (initialized from tableData, e.g., using useState(tableData ||
[])) near the top of the Pipelines component so the existing useEffect (which
calls setFilteredData(tableData)) and other calls to setFilteredData work
without runtime errors; also update any places that should read the derived
array to use filteredData instead of tableData (e.g., the render/logic that
currently references tableData on line ~464).
🧹 Nitpick comments (2)
frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx (1)
151-161: PropTypes forpipelineIdshould accept both string and number.For consistency with similar changes in
CardGridView.jsx,pipelineIdshould accept both types sincepipeline.idcould be either.🔧 Proposed fix
StatusPills.propTypes = { statuses: PropTypes.arrayOf( PropTypes.shape({ status: PropTypes.string, timestamp: PropTypes.string, execution_id: PropTypes.string, }) ), executionType: PropTypes.string, - pipelineId: PropTypes.string, + pipelineId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]), };frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
52-52: Usingrequire()inside a function component is non-standard and may cause bundling issues.Dynamic
require()calls inside React components are typically avoided as they:
- Bypass static analysis and tree-shaking
- May behave differently in ESM vs CommonJS environments
- Are evaluated at runtime rather than build time
Consider moving this to a standard import at the top of the file.
🔧 Proposed fix
At the top of the file, add:
import { SharePermission } from "../../widgets/share-permission/SharePermission"; import { createPipelineCardConfig } from "./PipelineCardConfig.jsx"; +import { fetchExecutionLogs } from "../log-modal/fetchExecutionLogs";Then remove line 52:
- const { fetchExecutionLogs } = require("../log-modal/fetchExecutionLogs");
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx
Outdated
Show resolved
Hide resolved
- Fix HIGH severity: Remove orphaned setFilteredData references in Pipelines.jsx
- Fix LOW severity: Add keyboard listeners to clickable spans for accessibility
- PipelineCardConfig.jsx: Add role, tabIndex, onKeyDown to status badge
- CardItem.jsx: Add keyboard handlers to action spans and expand chevron
- Reduce code duplication between ApiDeploymentCardConfig and PipelineCardConfig
- Extract shared components to CardFieldComponents.jsx:
CardActionBox, OwnerFieldRow, LastRunFieldRow, Last5RunsFieldRow,
WorkflowFieldRow, ApiEndpointSection, CardHeaderRow
- Net reduction: -289 lines in existing files
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace regex /\/+$/ with iterative string manipulation to strip trailing slashes, avoiding potential ReDoS concerns flagged by SonarCloud security scanner. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
265-272: Fix staleselectedPorDwhen clearing file history.
clearFileMarkersignores the passed workflow id and relies onselectedPorD, which may still be the previous pipeline due to async state updates. Use the workflow id argument directly.🐛 Proposed fix
- const clearFileMarkers = async () => { - const workflowId = selectedPorD?.workflow_id; - const success = await clearFileHistory(workflowId); + const clearFileMarkers = async (workflowId) => { + if (!workflowId) return; + const success = await clearFileHistory(workflowId); if (success && openDeleteModal) { setOpenDeleteModal(false); } }; @@ const handleClearFileHistoryPipeline = (pipeline) => { setSelectedPorD(pipeline); clearFileMarkers(pipeline.workflow_id); };Also applies to: 390-393
🤖 Fix all issues with AI agents
In `@frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx`:
- Around line 110-113: The server-side search term isn't persisted across
pagination/refresh; modify the component to store the query in state (e.g., add
a searchTerm state and setSearchTerm) and update handleSearch to set that state
instead of only calling getPipelineList; then ensure every call site that
invokes getPipelineList (including the pagination handlers, refresh/delete/share
callbacks referenced around lines where getPipelineList is called) passes the
current searchTerm state (trimmed or "") as the search parameter so paging and
subsequent refreshes keep the filter applied.
- Around line 418-444: The memoized pipelineCardConfig is missing pagination in
its dependency array which causes handlers that close over pagination
(handleEnablePipeline, handleSharePipeline, handleDeletePipeline) to use stale
pagination values; update the useMemo dependency array for
createPipelineCardConfig(...) to include pagination.current and
pagination.pageSize (or the pagination object) so pipelineCardConfig is
recomputed when pagination changes, ensuring those handlers refresh to the
correct page.
In `@frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx`:
- Around line 198-209: The Link target can be malformed if sessionDetails or
sessionDetails.orgName is undefined; update the Link rendering in
CardFieldComponents.jsx (the Link using sessionDetails?.orgName and workflowId)
to guard and provide a safe fallback (e.g., defaultOrgName or avoid rendering
the Link) by computing a safeOrg = sessionDetails?.orgName || '<fallback>'
before use, or conditionally render a non-navigating element when safeOrg is
missing, and ensure state (from, scrollToCardId) still uses location and itemId
correctly; adjust the Link's to prop to use safeOrg instead of
sessionDetails?.orgName so the route is never `/undefined/...`.
- Around line 38-86: These interactive icons (EditOutlined, ShareAltOutlined,
DeleteOutlined, MoreOutlined) need keyboard accessibility: add role="button" and
tabIndex={0} to each and implement onKeyDown handlers that listen for Enter and
Space to perform the same action as the onClick handlers (for EditOutlined call
setSelectedItem(item) then onEdit?.(item); for ShareAltOutlined call
setSelectedItem(item) then onShare?.(item); for DeleteOutlined call
setSelectedItem(item) then trigger the Popconfirm flow / onDelete?.(item); for
MoreOutlined ensure the kebabMenu dropdown is opened/activated), and ensure each
handler calls e.stopPropagation() like the click handlers.
🧹 Nitpick comments (2)
frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx (1)
165-169: Consider a more generic prop name for the StatusPillsComponent.The prop
pipelineId={itemId}uses pipeline-specific naming, but this component is reused for API Deployments as well. IfStatusPillsComponentcan accept a generic prop name likeitemIdorentityId, that would improve clarity.frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
52-52: Prefer a static ES module import forfetchExecutionLogs.
Keepingrequireinside the component re-resolves the module each render and can be awkward with ESM tooling. Consider moving it to a top-level import (please verify your bundler supports this import path).♻️ Proposed refactor
-import { useLocation } from "react-router-dom"; +import { useLocation } from "react-router-dom"; +import { fetchExecutionLogs } from "../log-modal/fetchExecutionLogs"; @@ - const { fetchExecutionLogs } = require("../log-modal/fetchExecutionLogs");
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx
Show resolved
Hide resolved
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx
Show resolved
Hide resolved
| <Tooltip title="Edit"> | ||
| <EditOutlined | ||
| className="action-icon-btn edit-icon" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| setSelectedItem(item); | ||
| onEdit?.(item); | ||
| }} | ||
| /> | ||
| </Tooltip> | ||
| <Tooltip title="Share"> | ||
| <ShareAltOutlined | ||
| className="action-icon-btn share-icon" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| setSelectedItem(item); | ||
| onShare?.(item); | ||
| }} | ||
| /> | ||
| </Tooltip> | ||
| <Popconfirm | ||
| title={deleteTitle} | ||
| description="This action cannot be undone." | ||
| onConfirm={() => { | ||
| setSelectedItem(item); | ||
| onDelete?.(item); | ||
| }} | ||
| onCancel={(e) => e?.stopPropagation()} | ||
| okText="Delete" | ||
| cancelText="Cancel" | ||
| okButtonProps={{ danger: true }} | ||
| > | ||
| <Tooltip title="Delete"> | ||
| <DeleteOutlined | ||
| className="action-icon-btn delete-icon" | ||
| onClick={(e) => e.stopPropagation()} | ||
| /> | ||
| </Tooltip> | ||
| </Popconfirm> | ||
| <Dropdown | ||
| menu={kebabMenuItems} | ||
| trigger={["click"]} | ||
| placement="bottomRight" | ||
| > | ||
| <MoreOutlined | ||
| className="card-kebab-menu" | ||
| onClick={(e) => e.stopPropagation()} | ||
| /> | ||
| </Dropdown> |
There was a problem hiding this comment.
Clickable icons missing keyboard accessibility attributes.
The EditOutlined, ShareAltOutlined, DeleteOutlined, and MoreOutlined icons are interactive but lack role="button", tabIndex={0}, and onKeyDown handlers. This prevents keyboard-only users from focusing and activating these actions.
The PR commit message mentions adding accessibility handlers, but these icons appear to be missing them. Consider wrapping each in an accessible element or adding the required attributes.
♿ Proposed fix for EditOutlined (apply similar pattern to others)
<Tooltip title="Edit">
<EditOutlined
className="action-icon-btn edit-icon"
+ role="button"
+ tabIndex={0}
onClick={(e) => {
e.stopPropagation();
setSelectedItem(item);
onEdit?.(item);
}}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ e.stopPropagation();
+ setSelectedItem(item);
+ onEdit?.(item);
+ }
+ }}
/>
</Tooltip>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Tooltip title="Edit"> | |
| <EditOutlined | |
| className="action-icon-btn edit-icon" | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| setSelectedItem(item); | |
| onEdit?.(item); | |
| }} | |
| /> | |
| </Tooltip> | |
| <Tooltip title="Share"> | |
| <ShareAltOutlined | |
| className="action-icon-btn share-icon" | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| setSelectedItem(item); | |
| onShare?.(item); | |
| }} | |
| /> | |
| </Tooltip> | |
| <Popconfirm | |
| title={deleteTitle} | |
| description="This action cannot be undone." | |
| onConfirm={() => { | |
| setSelectedItem(item); | |
| onDelete?.(item); | |
| }} | |
| onCancel={(e) => e?.stopPropagation()} | |
| okText="Delete" | |
| cancelText="Cancel" | |
| okButtonProps={{ danger: true }} | |
| > | |
| <Tooltip title="Delete"> | |
| <DeleteOutlined | |
| className="action-icon-btn delete-icon" | |
| onClick={(e) => e.stopPropagation()} | |
| /> | |
| </Tooltip> | |
| </Popconfirm> | |
| <Dropdown | |
| menu={kebabMenuItems} | |
| trigger={["click"]} | |
| placement="bottomRight" | |
| > | |
| <MoreOutlined | |
| className="card-kebab-menu" | |
| onClick={(e) => e.stopPropagation()} | |
| /> | |
| </Dropdown> | |
| <Tooltip title="Edit"> | |
| <EditOutlined | |
| className="action-icon-btn edit-icon" | |
| role="button" | |
| tabIndex={0} | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| setSelectedItem(item); | |
| onEdit?.(item); | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| setSelectedItem(item); | |
| onEdit?.(item); | |
| } | |
| }} | |
| /> | |
| </Tooltip> | |
| <Tooltip title="Share"> | |
| <ShareAltOutlined | |
| className="action-icon-btn share-icon" | |
| role="button" | |
| tabIndex={0} | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| setSelectedItem(item); | |
| onShare?.(item); | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| setSelectedItem(item); | |
| onShare?.(item); | |
| } | |
| }} | |
| /> | |
| </Tooltip> | |
| <Popconfirm | |
| title={deleteTitle} | |
| description="This action cannot be undone." | |
| onConfirm={() => { | |
| setSelectedItem(item); | |
| onDelete?.(item); | |
| }} | |
| onCancel={(e) => e?.stopPropagation()} | |
| okText="Delete" | |
| cancelText="Cancel" | |
| okButtonProps={{ danger: true }} | |
| > | |
| <Tooltip title="Delete"> | |
| <DeleteOutlined | |
| className="action-icon-btn delete-icon" | |
| role="button" | |
| tabIndex={0} | |
| onClick={(e) => e.stopPropagation()} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| } | |
| }} | |
| /> | |
| </Tooltip> | |
| </Popconfirm> | |
| <Dropdown | |
| menu={kebabMenuItems} | |
| trigger={["click"]} | |
| placement="bottomRight" | |
| > | |
| <MoreOutlined | |
| className="card-kebab-menu" | |
| role="button" | |
| tabIndex={0} | |
| onClick={(e) => e.stopPropagation()} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| } | |
| }} | |
| /> | |
| </Dropdown> |
🤖 Prompt for AI Agents
In `@frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx`
around lines 38 - 86, These interactive icons (EditOutlined, ShareAltOutlined,
DeleteOutlined, MoreOutlined) need keyboard accessibility: add role="button" and
tabIndex={0} to each and implement onKeyDown handlers that listen for Enter and
Space to perform the same action as the onClick handlers (for EditOutlined call
setSelectedItem(item) then onEdit?.(item); for ShareAltOutlined call
setSelectedItem(item) then onShare?.(item); for DeleteOutlined call
setSelectedItem(item) then trigger the Popconfirm flow / onDelete?.(item); for
MoreOutlined ensure the kebabMenu dropdown is opened/activated), and ensure each
handler calls e.stopPropagation() like the click handlers.
Update clearFileMarkers to accept optional workflowId parameter to match the call site that passes pipeline.workflow_id. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused useSessionStore import in DetailedLogs.jsx - Add search term persistence across pagination in Pipelines.jsx - Fix stale closure in pipelineCardConfig useMemo dependencies - Add keyboard accessibility (role, tabIndex, onKeyDown) to action icons - Guard against undefined orgName in WorkflowFieldRow link rendering Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx`:
- Around line 143-149: The pagination total calculation in the setPagination
callback uses the || operator which treats data.count === 0 as falsy and falls
back incorrectly; update the logic in the setPagination call (where
setPagination is invoked to update current/pageSize/total) to use a nullish
check (or explicit undefined check) for data.count — e.g., set total to
data.count if it is not null/undefined, otherwise fall back to
data.results?.length or data.length — ensuring zero counts are preserved.
🧹 Nitpick comments (2)
frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx (1)
195-199: Consider renamingpipelineIdprop for clarity.This component is used for both pipelines and API deployments per the PR scope. Passing
itemIdaspipelineIdworks functionally but may be confusing for API deployment contexts. IfStatusPillsComponentsupports a more generic prop name (e.g.,entityIdoritemId), consider using that for consistency.frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
53-53: Moverequire()to a top-level ES import.Using
require()inside a component body is an anti-pattern in modern React applications. It evaluates on each render, isn't tree-shakeable, and is inconsistent with the ES module imports used throughout this file.♻️ Proposed fix
Add at the top of the file with other imports:
+import { fetchExecutionLogs } from "../log-modal/fetchExecutionLogs";Then remove line 53:
- const { fetchExecutionLogs } = require("../log-modal/fetchExecutionLogs");
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx
Show resolved
Hide resolved
- Fix array index keys in PipelineCardConfig.jsx and LoadingSkeleton.jsx - Replace role="button" with native <button> elements for accessibility - Fix text contrast ratios to meet WCAG AA (4.5:1 minimum) - Fix unexpected negated conditions in pagination handlers - Use globalThis instead of window for better portability - Add proper button reset styles in CSS Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/deployments/api-deployment/ApiDeployment.jsx (1)
163-175: Direct mutation of state before API confirmation.Line 165 mutates
record.is_activedirectly. Sincerecordlikely references an object intableData, this mutates React state outside ofsetState, which can cause inconsistent UI and debugging difficulties. If the API call fails, the local state remains incorrectly toggled with no rollback.Proposed fix with optimistic update and rollback
const updateStatus = (record) => { setIsTableLoading(true); - record.is_active = !record?.is_active; + const newStatus = !record?.is_active; + const updatedRecord = { ...record, is_active: newStatus }; + + // Optimistically update the UI + setTableData((prev) => + prev.map((item) => (item.id === record.id ? updatedRecord : item)) + ); + apiDeploymentsApiService - .updateApiDeployment(record) - .then(() => {}) + .updateApiDeployment(updatedRecord) + .then(() => { + // Success - state already updated + }) .catch((err) => { + // Rollback on failure + setTableData((prev) => + prev.map((item) => (item.id === record.id ? record : item)) + ); setAlertDetails(handleException(err)); }) .finally(() => { setIsTableLoading(false); }); };frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
95-97: Search term not reset whentypechanges.When the pipeline type changes,
getPipelineList()is called with default parameters (empty search), butsearchTermstate is not reset. This could cause the search input to show a stale value that doesn't match the displayed data.🐛 Proposed fix
useEffect(() => { + setSearchTerm(""); + setPagination((prev) => ({ ...prev, current: 1 })); getPipelineList(); }, [type]);
🤖 Fix all issues with AI agents
In `@frontend/src/components/deployments/api-deployment/ApiDeployment.jsx`:
- Around line 120-129: The pagination total logic incorrectly treats data.count
=== 0 as falsy and falls back to array length; update the code that sets
pagination (the setPagination call that computes total) to use a nullish check
(e.g., use data.count ?? (data.results ? data.results.length : data.length) or
explicitly check data.count !== undefined) so a legitimate zero total is
preserved; adjust the same expression that sets setTableData/setPagination to
rely on data.results when present and only fallback to lengths when count is
undefined.
In
`@frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx`:
- Around line 89-94: The current key computation inside the statuses.map mapping
(where key is set using run.execution_id || run.timestamp || config.label) can
produce duplicate React keys when multiple runs lack execution_id and timestamp;
update the mapping logic in the statuses.map callback (the function that calls
getStatusConfig) to append the array index as a final fallback so each key is
unique (e.g., use run.execution_id || run.timestamp ||
`${config.label}-${index}` or similar), ensuring you reference the index
parameter of the map callback when constructing the key.
🧹 Nitpick comments (5)
frontend/src/components/deployments/api-deployment/ApiDeployment.jsx (2)
290-307:useMemohas incomplete dependencies—handlers may become stale.The memo depends only on
[sessionDetails, location], but it capturesupdateStatusand allhandle*callbacks which are recreated on every render. If any of these handlers reference state that changes (e.g.,paginationinupdateStatus's flow, orselectedRowinhandleShare), the memoized config will hold stale references.Consider either:
- Wrapping the handlers with
useCallbackand including them in deps, or- Removing
useMemoif re-creation on every render is acceptable.Option 1: Stabilize handlers with useCallback
const handleEditDeployment = useCallback(() => { openAddModal(true); }, []); const handleShareDeployment = useCallback(() => { handleShare(); }, [handleShare]); // handleShare would also need useCallback // ... similarly for other handlers const apiDeploymentCardConfig = useMemo( () => createApiDeploymentCardConfig({ setSelectedRow, updateStatus, sessionDetails, location, onEdit: handleEditDeployment, onShare: handleShareDeployment, // ... }), [ sessionDetails, location, updateStatus, handleEditDeployment, handleShareDeployment, // ... all handlers ] );
91-93:filteredDataappears redundant with server-side search.The
useEffectsyncsfilteredDatato matchtableDataon every update, andhandleSearchfetches from the server rather than filtering locally. PassingsetSearchList={setFilteredData}suggests client-side filtering capability, but it's not being used here.If client-side filtering is not needed, consider removing
filteredDatastate and passingtableDatadirectly to simplify the data flow.Also applies to: 323-323, 330-330
frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx (1)
142-152: PropTypes shape is incomplete.The component uses
run.successful_filesandrun.failed_files(lines 92, 100, 103), but these are not documented in the PropTypes shape.📝 Proposed fix
StatusPills.propTypes = { statuses: PropTypes.arrayOf( PropTypes.shape({ status: PropTypes.string, timestamp: PropTypes.string, execution_id: PropTypes.string, + successful_files: PropTypes.number, + failed_files: PropTypes.number, }) ), executionType: PropTypes.string, pipelineId: PropTypes.string, };frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
53-53: Avoidrequire()inside functional components.Using
require()inside a React component is unconventional and may cause issues with tree-shaking and module bundling. It also runs on every render. Consider moving this to a top-level import.♻️ Proposed fix
Move to top-level imports:
import { SharePermission } from "../../widgets/share-permission/SharePermission"; import { createPipelineCardConfig } from "./PipelineCardConfig.jsx"; +import { fetchExecutionLogs } from "../log-modal/fetchExecutionLogs"; function Pipelines({ type }) { // ... state declarations ... - const { fetchExecutionLogs } = require("../log-modal/fetchExecutionLogs");frontend/src/components/widgets/card-grid-view/CardGridView.css (1)
219-236: Consider addingprefers-reduced-motionsupport for accessibility.Users who prefer reduced motion (due to vestibular disorders) should have animations disabled or minimized. This is a WCAG 2.1 AAA consideration but increasingly expected.
♻️ Suggested enhancement
`@keyframes` cardExpandAnimation { from { opacity: 0; transform: translateY(-8px); } to { opacity: 1; transform: translateY(0); } } + +@media (prefers-reduced-motion: reduce) { + .card-expanded-content { + animation: none; + } + + .card-list-expanded { + transition: none; + } +}
| const data = res?.data; | ||
| // Handle paginated response | ||
| setTableData(data.results || data); | ||
| setPagination((prev) => ({ | ||
| ...prev, | ||
| current: page, | ||
| pageSize, | ||
| total: | ||
| data.count || (data.results ? data.results.length : data.length), | ||
| })); |
There was a problem hiding this comment.
Pagination total may be incorrect when data.count is 0.
When data.count is 0 (a valid count for empty results), the || operator treats it as falsy and falls back to the array length. This could misrepresent the total when server returns zero matching results.
Proposed fix
setPagination((prev) => ({
...prev,
current: page,
pageSize,
- total:
- data.count || (data.results ? data.results.length : data.length),
+ total:
+ data.count !== undefined
+ ? data.count
+ : data.results
+ ? data.results.length
+ : data.length,
}));🤖 Prompt for AI Agents
In `@frontend/src/components/deployments/api-deployment/ApiDeployment.jsx` around
lines 120 - 129, The pagination total logic incorrectly treats data.count === 0
as falsy and falls back to array length; update the code that sets pagination
(the setPagination call that computes total) to use a nullish check (e.g., use
data.count ?? (data.results ? data.results.length : data.length) or explicitly
check data.count !== undefined) so a legitimate zero total is preserved; adjust
the same expression that sets setTableData/setPagination to rely on data.results
when present and only fallback to lengths when count is undefined.
frontend/src/components/pipelines-or-deployments/pipelines/PipelineCardConfig.jsx
Outdated
Show resolved
Hide resolved
- Revert status badge colors to vibrant originals (pre-SonarCloud fix) - Fix pipeline list sorting with nulls_last for last_run_time - Add index fallback to React key in StatusPills to prevent duplicates - Simplify enableDisablePipeline by removing unnecessary .then() - Remove unused pagination deps from useMemo Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx`:
- Around line 99-109: The effect only restores scrollToCardId; update the
useEffect handling location.state in Pipelines.jsx to also read page, pageSize,
and searchTerm from location.state and apply them before triggering the scroll:
call setPage(location.state.page), setPageSize(location.state.pageSize),
setSearchTerm(location.state.searchTerm) (or the component's equivalents) and
then re-fetch the list via the existing load/fetch function (e.g.,
fetchPipelines/loadPipelines) so the correct page/filter is loaded before
calling setScrollRestoreId; keep the same timeout/cleanup logic that clears
setScrollRestoreId after 500ms. Also ensure navigation code that opens a card
(the navigate(...) invoked from StatusPills or similar) includes page, pageSize
and searchTerm in state when pushing the route so the effect can restore full
list context.
🧹 Nitpick comments (3)
frontend/src/components/widgets/card-grid-view/CardGridView.css (2)
16-24: Consider specifying explicit transition properties.Using
transition: allcan cause performance issues by transitioning unintended properties and may have unexpected side effects during future refactoring. Specifying exact properties is more predictable.♻️ Suggested improvement
.card-grid-item { height: 100%; - transition: all 0.2s ease; + transition: box-shadow 0.2s ease; border-radius: 8px; }.card-grid-item.card-list-mode { width: 100%; - transition: all 0.2s ease; + transition: box-shadow 0.2s ease; }Also applies to: 286-289
645-648: Remove dead CSS class.The comment indicates this class is no longer used. Consider removing it to keep the stylesheet clean.
🧹 Suggested cleanup
-/* Hide arrow - no longer used in vertical layout */ -.connector-flow-arrow { - display: none; -}frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
2-3: Avoidrequireinside an ES module component.
If this file is compiled as ESM (e.g., Vite),requireis undefined at runtime. Prefer a static import (or a lazyimport()if you intended to defer loading).♻️ Suggested change
-import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; +import { fetchExecutionLogs } from "../log-modal/fetchExecutionLogs"; @@ - const { fetchExecutionLogs } = require("../log-modal/fetchExecutionLogs");Also applies to: 32-32, 53-53
| // Handle scroll restoration from navigation | ||
| useEffect(() => { | ||
| if (location.state?.scrollToCardId) { | ||
| setScrollRestoreId(location.state.scrollToCardId); | ||
| // Clear after a short delay to prevent re-triggering | ||
| const timer = setTimeout(() => { | ||
| setScrollRestoreId(null); | ||
| }, 500); | ||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [location.state?.scrollToCardId]); |
There was a problem hiding this comment.
Scroll restoration should also restore pagination/search context.
Right now only scrollToCardId is restored; the list still reloads page 1 with an empty search term, so the originating card may not exist for paged/filtered lists. Pass {page, pageSize, searchTerm} in navigation state (from the StatusPills navigation) and re-fetch that page before scrolling.
🛠️ Suggested restoration logic
- useEffect(() => {
- if (location.state?.scrollToCardId) {
- setScrollRestoreId(location.state.scrollToCardId);
- // Clear after a short delay to prevent re-triggering
- const timer = setTimeout(() => {
- setScrollRestoreId(null);
- }, 500);
- return () => clearTimeout(timer);
- }
- }, [location.state?.scrollToCardId]);
+ useEffect(() => {
+ const state = location.state || {};
+ if (state.scrollToCardId) {
+ setScrollRestoreId(state.scrollToCardId);
+ const timer = setTimeout(() => setScrollRestoreId(null), 500);
+
+ const restoredSearch = state.searchTerm ?? "";
+ const restoredPageSize = state.pageSize ?? pagination.pageSize;
+ const restoredPage = state.page ?? 1;
+ setSearchTerm(restoredSearch);
+ getPipelineList(restoredPage, restoredPageSize, restoredSearch);
+
+ return () => clearTimeout(timer);
+ }
+ }, [location.state, pagination.pageSize]);🧭 Also pass list context when navigating from a card
navigate(`/${sessionDetails.orgName}/logs/${executionType}/${run.execution_id}`, {
state: {
from: location.pathname,
scrollToCardId: pipelineId,
page: pagination.current,
pageSize: pagination.pageSize,
searchTerm,
},
});🤖 Prompt for AI Agents
In `@frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx`
around lines 99 - 109, The effect only restores scrollToCardId; update the
useEffect handling location.state in Pipelines.jsx to also read page, pageSize,
and searchTerm from location.state and apply them before triggering the scroll:
call setPage(location.state.page), setPageSize(location.state.pageSize),
setSearchTerm(location.state.searchTerm) (or the component's equivalents) and
then re-fetch the list via the existing load/fetch function (e.g.,
fetchPipelines/loadPipelines) so the correct page/filter is loaded before
calling setScrollRestoreId; keep the same timeout/cleanup logic that clears
setScrollRestoreId after 500ms. Also ensure navigation code that opens a card
(the navigate(...) invoked from StatusPills or similar) includes page, pageSize
and searchTerm in state when pushing the route so the effect can restore full
list context.
What
Introduces a reusable
CardGridViewcomponent for displaying Pipelines and API Deployments in a modern card-based layout with execution status tracking, pagination, search, and scroll restoration.Why
The existing table-based view does not effectively display rich metadata like execution statuses, connector icons, and scheduling info. Cards provide better UX for scanning pipeline/deployment details at a glance.
How
Commits
61132896eec69cc3cbf938be528558dbb81dab0190ca14fddcda01Backend Changes
WorkflowExecution.get_last_run_statuses()method with dynamic PARTIAL_SUCCESS computationlast_5_run_statuses,run_count,last_run_time,next_run_timesource_instance_nameanddestination_instance_nameto pipeline representationCustomPaginationand search filters for both endpointsFrontend Changes
CardGridViewwidget with card/list modes, expand/collapse, status badgesPipelineCardConfigwith StatusPills showing clickable execution statusesApiDeploymentCardConfigwith similar card layoutCan this PR break any existing features? If yes, please list possible items. If no, please explain why.
No. The changes replace the table view with card view but maintain the same underlying data and functionality. All existing actions (edit, delete, share, toggle, etc.) are preserved.
Database Migrations
Not applicable.
Env Config
Not applicable.
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code