UN-3097 [FIX] Fix logs screen pagination and other issues in card-based deployment layout#1821
Merged
Deepak-Kesavan merged 4 commits intomainfrom Mar 3, 2026
Merged
Conversation
- Fix null reference in CreateApiDeploymentModal (unable to create API deployment) - Replace clickable endpoint link with copy button to prevent 404 redirects - Add immediate table update on manual ETL sync with API response - Remove strict is_active filter for API deployment banner visibility - Remove redundant DeleteModal (delete now triggers directly from card) - Cleanup Azure connector error message (remove redundant prefix) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Summary by CodeRabbit
WalkthroughRemoval of modal-driven deletion in pipelines and API deployments, safer handling of possibly paginated API responses and undefined lists, UI simplifications for endpoint rendering and pagination labels, and standardized Azure error message formatting. Changes
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 |
chandrasekharan-zipstack
requested review from
Deepak-Kesavan,
athul-rs,
jaags-dev and
vishnuszipstack
chandrasekharan-zipstack
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/create-api-deployment-modal/CreateApiDeploymentModal.jsx (1)
292-292:⚠️ Potential issue | 🟡 MinorPropTypes mismatch:
workflowEndpointListshould bePropTypes.array, notPropTypes.object.The prop is initialized as an array (
useState([])inApiDeployment.jsxline 46), defaulted to an array on line 28, and iterated with.map()on line 265. The PropTypes declaration should reflect this.Proposed fix
- workflowEndpointList: PropTypes.object, + workflowEndpointList: PropTypes.array,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx` at line 292, PropTypes for workflowEndpointList is incorrect: update the CreateApiDeploymentModal.jsx PropTypes declaration for workflowEndpointList from PropTypes.object to PropTypes.array (or PropTypes.arrayOf(PropTypes.object) if the items are objects) so it matches the useState([]) initialization and the .map() usage; ensure any defaultProps remain an array and run a quick search for workflowEndpointList in CreateApiDeploymentModal to update all related prop type expectations consistently.
🧹 Nitpick comments (1)
unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py (1)
45-45: Use explicit f-string conversion on Line 45.Ruff RUF010 (
explicit-f-string-type-conversion) applies here. Replace{str(e)}with the conversion flag{e!s}for consistency with modern f-string conventions.Proposed fix
- error_message += f"Error from Azure Cloud Storage. \n```\n{str(e)}\n```" + error_message += f"Error from Azure Cloud Storage. \n```\n{e!s}\n```"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py` at line 45, Replace the explicit str() conversion in the f-string that builds error_message: change the interpolation from {str(e)} to the explicit f-string conversion {e!s} in the code that appends "Error from Azure Cloud Storage" (the block that updates error_message using variable e); this satisfies Ruff RUF010 and keeps the message content the same while using {e!s} for correct f-string conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx`:
- Line 292: PropTypes for workflowEndpointList is incorrect: update the
CreateApiDeploymentModal.jsx PropTypes declaration for workflowEndpointList from
PropTypes.object to PropTypes.array (or PropTypes.arrayOf(PropTypes.object) if
the items are objects) so it matches the useState([]) initialization and the
.map() usage; ensure any defaultProps remain an array and run a quick search for
workflowEndpointList in CreateApiDeploymentModal to update all related prop type
expectations consistently.
---
Nitpick comments:
In
`@unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py`:
- Line 45: Replace the explicit str() conversion in the f-string that builds
error_message: change the interpolation from {str(e)} to the explicit f-string
conversion {e!s} in the code that appends "Error from Azure Cloud Storage" (the
block that updates error_message using variable e); this satisfies Ruff RUF010
and keeps the message content the same while using {e!s} for correct f-string
conversion.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
frontend/src/components/agency/agency/Agency.jsxfrontend/src/components/deployments/api-deployment/ApiDeployment.jsxfrontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsxfrontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsxfrontend/src/components/widgets/card-grid-view/CardFieldComponents.jsxunstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py
Use "APIs" and "pipelines" instead of generic "items" in pagination text. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
athul-rs
approved these changes
Mar 3, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/deployments/api-deployment/ApiDeployment.jsx`:
- Around line 165-169: The deleteApiDeployment handler may call
apiDeploymentsApiService.deleteApiDeployment with an undefined or empty id
(using item?.id || selectedRow.id); add a guard in deleteApiDeployment to
validate the resolved id before calling the service, e.g., compute const id =
item?.id ?? selectedRow?.id and if (!id) return early (or show a user-facing
error/toast) so apiDeploymentsApiService.deleteApiDeployment is never invoked
with an invalid id.
In `@frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx`:
- Around line 233-237: The deletePipeline function can compute an empty id from
item?.id || selectedPorD.id which may trigger a DELETE to an invalid endpoint;
add a guard at the top of deletePipeline (referencing deletePipeline, item,
selectedPorD, and id) that checks if id is falsy and returns early (or shows an
error/notification) before constructing requestOptions and issuing the DELETE
request to avoid sending requests with an empty id.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/src/components/deployments/api-deployment/ApiDeployment.jsxfrontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsxfrontend/src/components/widgets/card-grid-view/CardGridView.jsx
frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx
Show resolved
Hide resolved
jaags-dev
approved these changes
Mar 3, 2026
Deepak-Kesavan
requested changes
Mar 3, 2026
unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py
Outdated
Show resolved
Hide resolved
Reset pagination to page 1 when switching execution log tabs to avoid stale page state. Merge implicit f-string concatenation in Azure exceptions to resolve SonarQube S5799. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
chandrasekharan-zipstack
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py`:
- Line 45: Replace the explicit str() call inside the f-string with Python's
conversion flag: change the interpolation from {str(e)} to {e!s} where the
exception is appended to error_message (the f-string that builds "Error from
Azure Cloud Storage..." in the exceptions module) so Ruff RUF010 is satisfied
and the same stringified exception is produced.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
frontend/src/components/logging/execution-logs/ExecutionLogs.jsxunstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py
unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/exceptions.py
Show resolved
Hide resolved
Deepak-Kesavan
approved these changes
Mar 3, 2026
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
4 participants
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
https://zipstack.atlassian.net/browse/UN-3097
Why
How
CreateApiDeploymentModalcrashed on nullworkflowEndpointList— added default value[]and optional chaining on.find()/.map()handleSyncnow updates table data immediately with the API response instead of setting a hardcoded "processing" state before the responseis_active === truefilter — banner now shows for any API deployment linked to the workflow (prefers active, falls back to first)DeleteModal— delete now triggers directly from the card's built-in confirm actionCan 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)
Notes on Testing
Screenshots
Checklist
I have read and understood the Contribution Guidelines.