feat: update response tab UI #37640
feat: update response tab UI #37640
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
alex-golovanov
requested review from
ApekshaBhosale and
sagar-qa007
as code owners
WalkthroughThe pull request introduces modifications across multiple Cypress test files, primarily focusing on standardizing method names and enhancing the structure of test cases related to various data sources. Key changes include renaming the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant API
participant DB
User->>UI: Trigger Query
UI->>API: Run Query
API->>DB: Execute Query
DB-->>API: Return Results
API-->>UI: Send Results
UI-->>User: Display Results
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
IDE Navigation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (46)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/styles.ts (2)
18-18: Consider usingoverflow: hiddeninstead ofclipThe
overflow: clipproperty has limited browser support. Consider usingoverflow: hiddenfor better compatibility.- overflow: clip; + overflow: hidden;
44-54: Consider simplifying the selectorThe double ampersand (
&&) might be unnecessary unless you're specifically overriding other styles. Consider simplifying if possible.- && { + & {app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx (2)
Line range hint
41-71: Test coverage needs expansion for new UI features.The test suite only verifies text selection functionality, while the PR introduces multiple UI changes that should be tested:
- Query name prefix in header
- New header component with record count
- Tooltips for metadata
- Content type icons
- FAB with dropdown menu
- Status bar for error states
Would you like me to help generate additional test cases for these features? Here's a sample test case structure we could implement:
it("should display query name prefix and metadata tooltips", () => { const { getByTestId, getByText } = render( <Provider store={store}> <ThemeProvider theme={lightTheme}> <Router> <QueryDebuggerTabs actionName="Query1" actionSource={{ id: "ID1", name: "Query1", type: ENTITY_TYPE.ACTION, }} isRunning={false} onRunClick={() => {}} /> </Router> </ThemeProvider> </Provider>, ); // Add assertions for new UI elements });
Line range hint
72-95: Consider adding error scenarios and loading state tests.The PR objectives mention changes to loading states and error scenarios, but these aren't covered in tests.
Add test cases for:
- Loading state with spinner
- Error state with status bar
- Empty state text and button alignment
it("should display loading state with spinner when isRunning is true", () => { const { getByTestId } = render( <Provider store={store}> <ThemeProvider theme={lightTheme}> <Router> <QueryDebuggerTabs actionName="Query1" actionSource={{ id: "ID1", name: "Query1", type: ENTITY_TYPE.ACTION, }} isRunning={true} onRunClick={() => {}} /> </Router> </ThemeProvider> </Provider>, ); // Add assertions for spinner });app/client/cypress/e2e/Sanity/Datasources/MockDBs_Spec.ts (3)
Line range hint
83-84: Remove agHelper.Sleep() callReplace the explicit sleep with proper Cypress wait conditions. Consider using
cy.intercept()and waiting for the network request to complete.- agHelper.Sleep(500); + cy.intercept('GET', '**/api/v1/datasources/*/structure').as('structureFetch'); + cy.wait('@structureFetch');
Line range hint
57-60: Replace cy.wait with interceptAvoid using
cy.waitwith static timing. Instead, usecy.intercept()to wait for specific network requests.- cy.wait("@createNewApi").then((interception) => { + cy.intercept('POST', '**/api/v1/actions').as('createNewApi'); + cy.wait('@createNewApi').then((interception) => {
45-48: Consider enhancing runQueryAndVerifyResponseViews parametersThe method signature could be improved to better handle response validation:
- Add TypeScript interface for the parameters
- Make responseTypes a required parameter with default value
interface QueryValidationParams { count: number; operator: 'eq' | 'gt' | 'gte' | 'lt' | 'lte'; responseTypes?: string[]; }Also applies to: 64-67, 93-97
app/client/src/IDE/Components/BottomView.tsx (2)
72-72: LGTM! Consider adding responsive positioningThe button position adjustment aligns with the UI update objectives. However, consider using CSS custom properties for consistent spacing across different viewport sizes.
- top: 2px; + top: var(--ads-v2-spaces-2);
Line range hint
1-180: Consider implementing missing PR objectivesThe component could be enhanced to address these PR objectives:
- Ensure consistent scrollbar visibility in ViewWrapper
- Add loading state handling
Consider these improvements:
const ViewWrapper = styled.div` height: 100%; + scrollbar-gutter: stable; + overflow-y: auto; &&& { ul.ads-v2-tabs__list {Also, consider adding a loading state prop to handle loading indicators:
interface Props { // ... existing props + isLoading?: boolean; }app/client/cypress/e2e/Regression/ServerSide/Datasources/Redis_Basic_Spec.ts (4)
Line range hint
13-17: Replace cy.get with data- attribute selector.*According to the coding guidelines, we should avoid using plain CSS selectors and prefer data-* attributes.
- cy.get("@dsName").then(($dsName) => { + cy.get('[data-cy="dsName"]').then(($dsName) => {
Line range hint
59-67: Improve assertion structure for dynamic responses.The current assertion using
.to.be.oneOfcould be flaky as it depends on the order of responses. Consider using a more robust assertion approach.dataSources.ReadQueryTableResponse(0).then(($cellData: any) => { const expectedKeys = new Set([ "name", "ingredients", "instructions", "difficulty" ]); expect(expectedKeys.has($cellData)).to.be.true; });
Line range hint
79-87: Similar assertion structure needs improvement.The same assertion pattern is repeated. Consider extracting this into a reusable helper function.
function assertKeyExists(response: any, expectedKeys: string[]) { expect(expectedKeys).to.include(response); } // Usage dataSources.ReadQueryTableResponse(0).then(($cellData: any) => { assertKeyExists($cellData, [ "name", "ingredients", "instructions", "difficulty", "prep_time" ]); });
Add after() hook for test cleanup resilience
The test currently relies on cleanup within the test case using
DeleteDatasourceFromWithinDS, but this won't execute if the test fails before reaching that point. Add an after() hook to ensure cleanup runs regardless of test outcome:after(() => { dataSources.DeleteDatasourceFromWithinDS(dsName); });🔗 Analysis chain
Line range hint
1-112: Verify test isolation and cleanup.The test uses
beforehook but doesn't have a correspondingafterhook to clean up the Redis database state. While the test does clean up its own data, we should ensure complete cleanup in case of test failures.
Let me check for more specific cleanup patterns in the Redis test files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for cleanup patterns in test files rg -l 'after\(' app/client/cypress/e2e/Regression/ServerSide/Datasources/ # Check for potential cleanup methods ast-grep --pattern 'after($$$) { $$$DeleteDatasource$$$}'Length of output: 373
Script:
#!/bin/bash # Search for Redis test files and their cleanup patterns rg -l "Redis.*Spec" app/client/cypress/e2e/ rg -C 5 "after\(" app/client/cypress/e2e/Regression/ServerSide/Datasources/Redis_Basic_Spec.ts # Check for DeleteDatasource usage patterns rg -C 3 "DeleteDatasource" app/client/cypress/e2e/Regression/ServerSide/Datasources/Redis_Basic_Spec.ts # Look for similar Redis test files fd "Redis.*Spec"Length of output: 552
app/client/src/components/editorComponents/JSResponseView.test.tsx (1)
Line range hint
89-90: Consider addressing the TODO comment.The TODO comment about fixing the store type should be addressed to improve type safety.
Would you like me to help implement proper typing for the store?
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Table.tsx (1)
Line range hint
1-400: Consider improving type safety and addressing TODOsThe file contains multiple TODOs and type-safety suppressions. Consider:
- Adding proper TypeScript types instead of using
any- Addressing the TODOs, especially the platform-widget coupling mentioned in the imports
app/client/cypress/e2e/Sanity/Datasources/Arango_Basic_Spec.ts (4)
Line range hint
34-38: Replace cy.get alias with proper data-cy attributeUsing cy.get with aliases is not recommended. Instead, use data-cy attributes for better maintainability.
- cy.get("@dsName").then(($dsName) => { - dsName = $dsName; - }); + cy.get('[data-cy="datasource-name"]').then(($dsName) => { + dsName = $dsName; + });
Line range hint
303-307: Group related assertions using cy.shouldMultiple separate assertions should be grouped using cy.should for better readability and performance.
- agHelper - .GetText(dataSources._noRecordFound) - .then(($noRecMsg) => - expect($noRecMsg).to.eq("No data records to show"), - ); + agHelper + .GetText(dataSources._noRecordFound) + .should(($noRecMsg) => { + expect($noRecMsg).to.eq("No data records to show") + });
Line range hint
315-318: Combine multiple assertions for table responsesMultiple table response assertions should be combined into a single chain for better performance.
- dataSources.AssertQueryTableResponse(0, "France"); - dataSources.AssertQueryTableResponse(1, "USA"); - dataSources.AssertQueryTableResponse(2, "Brazil"); + dataSources.AssertQueryTableResponses([ + { row: 0, value: "France" }, + { row: 1, value: "USA" }, + { row: 2, value: "Brazil" } + ]);
Line range hint
386-389: Remove commented codeThe commented code
//entityExplorer.CreateNewDsQuery("New cURL import", false);should be removed as it's not providing any value and the comment indicates it's failing.app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL1_Spec.ts (2)
Line range hint
63-63: Replace agHelper.Sleep with proper wait conditionsUsing explicit sleep times makes tests flaky. Instead, wait for specific elements or network requests to complete.
Replace with a more reliable wait condition:
- agHelper.Sleep(3000); + agHelper.WaitUntilAllToastsDisappear(); + cy.wait("@updateLayout").its("response.statusCode").should("eq", 200);
Line range hint
25-31: Remove commented code blocksCommented-out code reduces readability and may become outdated. If the MySQL interception logic is no longer needed, remove it. If it might be needed later, track it in version control history.
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts (2)
Line range hint
8-10: Remove usage of Sleep() callsAccording to the coding guidelines, using
agHelper.Sleep()should be avoided. Instead, use Cypress's built-in waiting mechanisms.Replace sleep calls with appropriate Cypress wait commands:
- agHelper.Sleep(2000); + cy.wait('@postExecute').its('response.statusCode').should('eq', 200);Also applies to: 391-393
Line range hint
33-35: Refactor hardcoded selectors to use data- attributes*According to the coding guidelines, selectors should use data-* attributes instead of classes or other attributes.
Replace hardcoded selectors with data attributes:
- locators._buttonByText("Run InsertQuery") + '[data-cy="insert-query-button"]'- locators._modal + '[data-cy="modal"]'Also applies to: 71-73
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js (4)
74-74: Consistent update to record count validationThe changes to
validateRecordCountnow properly handle the comparison logic with the new parameter structure. However, consider extracting the common validation object into a constant to maintain DRY principles.const defaultValidation = { count: 10, operator: "lte" }; // Then use: BottomPane.response.validateRecordCount(defaultValidation);Also applies to: 96-96, 104-104, 112-112, 134-134, 148-148, 436-436
Line range hint
30-36: Remove commented-out afterEach blocksThese commented blocks contain outdated error handling logic. Either implement proper error handling or remove the comments entirely.
- // afterEach(function() { - // if (this.currentTest.state === "failed") { - // Cypress.runner.stop(); - // } - // }); - - // afterEach(() => { - // if (queryName) - // cy.actionContextMenuByEntityName(queryName); - // });
Line range hint
126-133: Replace cy.wait with proper assertionsUsing
cy.wait('@postExecute')violates the coding guidelines. Consider using Cypress's built-in retry-ability with assertions instead.cy.get('@postExecute').should((response) => { expect(response.body.data.body[0].description).to.contains( "The ideal apartment to visit...", "Response is not as expected for Find command with multiple conditions" ); });Also applies to: 140-147
Line range hint
3-11: Consider using TypeScript for better type safetyThe file imports multiple JSON files and uses require statements. Converting to TypeScript would provide better type safety and maintainability.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/S3_1_spec.js (2)
Line range hint
1-600: Replace cy.wait with better alternatives.The test file extensively uses
cy.wait('@postExecute')which is against Cypress best practices. Consider using Cypress route aliases withcy.intercept()and proper assertions instead.Example refactor:
-cy.wait("@postExecute").then(({ response }) => { +cy.intercept('POST', '**/execute').as('postExecute'); +cy.get('@postExecute').should(({ response }) => { expect(response.body.data.isExecutionSuccess).to.eq(true); });Also, consider using
cy.waitUntil()for checking UI states instead of arbitrary waits likecy.wait(3000).
Line range hint
1-600: Use data- attributes for selectors.*Several selectors in the test file use attributes or complex CSS paths. Replace them with data-* attributes for better maintainability.
Examples to fix:
-cy.get(formControls.s3BucketName) +cy.get('[data-cy="s3-bucket-name"]') -cy.get(".t--widget-textwidget span:contains('" + fileName + "')") +cy.get('[data-cy="file-name-text"]')app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts (2)
82-82: Consider adding type definition for the count parameter.While the method signature update to accept an object parameter is good, adding TypeScript interface/type for the parameter would improve code maintainability and type safety.
- dataSources.runQueryAndVerifyResponseViews({ count: 10 }); + interface QueryVerificationOptions { + count: number; + } + dataSources.runQueryAndVerifyResponseViews({ count: 10 } as QueryVerificationOptions);
Line range hint
1-500: Review test structure and practices.Several areas in the test file could be improved:
- The test uses
agHelper.Sleep()in multiple places which is against Cypress best practices.- Some selectors use XPath which should be avoided in favor of data-* attributes.
- The test uses
it.skipwhich should be avoided unless there's a linked issue.Consider these improvements:
- Replace
agHelper.Sleep()with Cypress's built-in retry-ability andcy.wait('@alias')for network requests- Use data-* attributes instead of XPath selectors
- Either fix the skipped test or remove it if it's no longer relevant
Example for point 1:
- agHelper.Sleep(2000); //for update to reflect! + cy.wait('@postExecute'); + cy.wait('@postExecute');app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts (3)
Line range hint
1-11: Consider adding JSDoc documentation for the test suite.Adding documentation for the test suite's purpose and requirements would improve maintainability.
+/** + * Test suite for MongoDB query functionality validation + * Covers CRUD operations and various MongoDB-specific commands + * @requires MongoDB datasource + */ import { INTERCEPT } from "../../../../fixtures/variables"; import { agHelper, appSettings, assertHelper,
Line range hint
969-1007: Consider extracting helper function to a shared utility.The
GenerateCRUDNValidateDeployPagefunction could be useful in other test files. Consider moving it to a shared utility file.
MongoDB connection handling is inconsistent across test files
The conditional intercept setup (
INTERCEPT.MONGO) is only implemented in two files:
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.tsapp/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Mongo_Spec.tsOther MongoDB test files either directly create connections or use different patterns, which could lead to flaky tests when MongoDB is unavailable.
🔗 Analysis chain
Line range hint
18-31: Verify MongoDB connection handling.The beforeEach block contains conditional intercept setup. Let's verify if this is consistently used across all MongoDB test files.
Let me check if there are any other MongoDB test files that might have different patterns.
Let me check the connection handling patterns in these MongoDB test files.
Let me check one more set of files to ensure we have a complete picture of the MongoDB test patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if other MongoDB test files follow the same connection handling pattern # Test: Search for MongoDB connection handling patterns in test files rg -A 5 "INTERCEPT.MONGO" "app/client/cypress"Length of output: 1325
Script:
#!/bin/bash # Find all MongoDB-related test files fd -e ts -e js mongo "app/client/cypress"Length of output: 956
Script:
#!/bin/bash # Search for beforeEach blocks and connection handling in MongoDB test files rg -B 2 -A 5 "beforeEach|before\(" "app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js" "app/client/cypress/e2e/Sanity/Datasources/MongoDatasourceURI_spec.ts" "app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MongoURI_Spec.ts"Length of output: 1142
Script:
#!/bin/bash # Check connection handling in client-side MongoDB test files rg -B 2 -A 5 "beforeEach|before\(" "app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/Table_MongoDB_spec.ts" "app/client/cypress/e2e/Regression/ClientSide/FormNativeToRawTests/MongoConversion_spec.ts"Length of output: 1221
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts (1)
Line range hint
1-620: Consider enhancing test reliability and maintainability.The test file contains several instances of hard-coded waits using
agHelper.Sleep(). This can make tests flaky and harder to maintain.Consider replacing these with Cypress's built-in retry-ability and assertions:
-agHelper.Sleep(2000); +cy.get(locators._jsonFormWidget).should('not.exist');Also, consider adding data-testid attributes to elements for more reliable selection:
-cy.xpath(locators._buttonByText("Update")) +cy.get('[data-testid="update-button"]')app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx (1)
Line range hint
1100-1379: Consider optimizing the ICON_LOOKUP structure.The current flat structure of ICON_LOOKUP could be reorganized into categories for better maintainability. Consider grouping related icons:
-const ICON_LOOKUP = { +const ICON_LOOKUP = { + contentTypes: { + table: ContentTypeTable, + json: ContentTypeJson, + raw: ContentTypeRaw, + }, + actions: { + add: AddLineIcon, + remove: RemoveIcon, + // ... other action icons + }, + // ... other categories };This would make the code more maintainable and easier to navigate as we add more icons.
app/client/cypress/support/Pages/DataSources.ts (2)
Line range hint
1136-1153: Method marked as deprecated - consider removing in future releasesThis method is now deprecated in favor of the new
runQueryAndVerifyResponseViewsmethod. Consider adding a deprecation notice with migration instructions in the JSDoc comment./** * @deprecated Use runQueryAndVerifyResponseViews instead. * @example * // Instead of: * RunQueryNVerifyResponseViews(1, true) * // Use: * runQueryAndVerifyResponseViews({ count: 1, responseTypes: ["TABLE", "JSON", "RAW"] }) */ public RunQueryNVerifyResponseViews(
1155-1181: LGTM! Well-structured implementation with proper TypeScript typesThe new method implementation is clean and provides better type safety with proper parameter destructuring. The use of optional parameters with defaults is a good practice.
Consider adding JSDoc comments to document the parameters and their purpose:
+/** + * Runs a query and verifies the response views + * @param options - Configuration options + * @param options.count - Expected record count (default: 1) + * @param options.operator - Comparison operator for record count validation + * @param options.responseTypes - Array of response types to verify (default: ["TABLE", "JSON", "RAW"]) + */ public runQueryAndVerifyResponseViews({app/client/src/ce/constants/messages.ts (1)
572-572: Consider consolidating empty response messagesThe new constant
EMPTY_RESPONSE_RUNappears to serve the same purpose as the combination ofEMPTY_RESPONSE_FIRST_HALFandEMPTY_RESPONSE_LAST_HALF. Consider deprecating the split messages to reduce duplication and improve maintainability.app/client/cypress/support/Pages/IDE/BottomPane/Response.ts (1)
43-46: Simplify method name toselectResponseTypeFromMenuThe method
selectResponseResponseTypeFromMenuis verbose and repetitive. Simplifying it toselectResponseTypeFromMenuenhances readability.Apply this change:
-public selectResponseResponseTypeFromMenu(type: string): void { +public selectResponseTypeFromMenu(type: string): void {app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx (6)
143-145: Simplify condition with optional chainingYou can simplify the conditional check by using optional chaining.
Apply this diff to streamline the code:
- if (actionResponse.messages && actionResponse.messages.length) { + if (actionResponse.messages?.length) {
163-177: Simplify mapping with optional chainingUse optional chaining to simplify the assignment.
Apply this diff:
- responseBodyTabs = - responseDataTypes && - responseDataTypes.map((dataType, index) => { + responseBodyTabs = + responseDataTypes?.map((dataType, index) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 163-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
180-183: Simplify mapping with optional chainingUse optional chaining to simplify the assignment.
Apply this diff:
- contentTypeOptions = - responseBodyTabs && - responseBodyTabs.map((item) => { + contentTypeOptions = + responseBodyTabs?.map((item) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 180-183: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
197-200: Simplify findIndex with optional chainingUse optional chaining to simplify the assignment.
Apply this diff:
- selectedTabIndex = - responseDataTypes && - responseDataTypes.findIndex( + selectedTabIndex = + responseDataTypes?.findIndex(🧰 Tools
🪛 Biome (1.9.4)
[error] 197-200: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
368-370: Simplify property access with optional chainingUse optional chaining to simplify the property access.
Apply this diff:
- pluginErrorDetails={ - actionResponse && actionResponse.pluginErrorDetails - } + pluginErrorDetails={actionResponse?.pluginErrorDetails}🧰 Tools
🪛 Biome (1.9.4)
[error] 368-368: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
373-373: Simplify condition with optional chainingUse optional chaining to simplify the condition.
Apply this diff:
- {actionResponse && actionResponse.request && ( + {actionResponse?.request && (🧰 Tools
🪛 Biome (1.9.4)
[error] 373-373: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (4)
app/client/packages/design-system/ads/src/__assets__/icons/ads/content-type-json.svgis excluded by!**/*.svgapp/client/packages/design-system/ads/src/__assets__/icons/ads/content-type-raw.svgis excluded by!**/*.svgapp/client/packages/design-system/ads/src/__assets__/icons/ads/content-type-table.svgis excluded by!**/*.svgapp/client/src/assets/images/no-response.svgis excluded by!**/*.svg
📒 Files selected for processing (30)
app/client/cypress/e2e/Regression/ClientSide/BugTests/Binding_Bug28287_Spec.ts(1 hunks)app/client/cypress/e2e/Regression/ServerSide/Datasources/Redis_Basic_Spec.ts(3 hunks)app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL1_Spec.ts(2 hunks)app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts(3 hunks)app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts(3 hunks)app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts(3 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts(13 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js(5 hunks)app/client/cypress/e2e/Regression/ServerSide/QueryPane/S3_1_spec.js(2 hunks)app/client/cypress/e2e/Sanity/Datasources/Arango_Basic_Spec.ts(5 hunks)app/client/cypress/e2e/Sanity/Datasources/MockDBs_Spec.ts(3 hunks)app/client/cypress/support/Pages/DataSources.ts(2 hunks)app/client/cypress/support/Pages/IDE/BottomPane/Response.ts(1 hunks)app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx(5 hunks)app/client/src/IDE/Components/BottomView.tsx(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/BindDataButton.tsx(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/NoResponse.tsx(4 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab.tsx(0 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/constants.ts(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/index.ts(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/styles.ts(1 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/Table.tsx(4 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/constants.ts(1 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/components/editorComponents/ActionExecutionInProgressView.tsx(2 hunks)app/client/src/components/editorComponents/EntityBottomTabs.tsx(1 hunks)app/client/src/components/editorComponents/JSResponseView.test.tsx(3 hunks)app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx(1 hunks)app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab.tsx
- app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx
✅ Files skipped from review due to trivial changes (5)
- app/client/src/PluginActionEditor/components/PluginActionResponse/components/BindDataButton.tsx
- app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/constants.ts
- app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/index.ts
- app/client/src/PluginActionEditor/components/PluginActionResponse/components/constants.ts
- app/client/src/components/editorComponents/EntityBottomTabs.tsx
🧰 Additional context used
📓 Path-based instructions (13)
app/client/cypress/e2e/Regression/ClientSide/BugTests/Binding_Bug28287_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/Datasources/Redis_Basic_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL1_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Querypane_Mongo_Spec.js (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/S3_1_spec.js (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Sanity/Datasources/Arango_Basic_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Sanity/Datasources/MockDBs_Spec.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/support/Pages/DataSources.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/support/Pages/IDE/BottomPane/Response.ts (1)
Pattern app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
🪛 Biome (1.9.4)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx
[error] 142-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 163-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 180-183: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 197-200: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 368-368: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 373-373: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (30)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/NoResponse.tsx (3)
17-17: LGTM! Improved layout structure
The styling changes improve the component's layout:
- Proper height calculation accounting for the tab bar
- Consistent spacing with gap properties
- Better organization with the new RunGroup component
Also applies to: 22-22, 25-29
Line range hint 45-55: LGTM! Enhanced empty state presentation
The changes align well with the PR objectives:
- Updated empty state text
- Improved button alignment
- Better visual grouping of related elements
2-3: Consider using the new ADS components
The component is using @appsmith/ads-old. Consider migrating to the newer ADS components for consistency with the design system.
app/client/cypress/e2e/Regression/ClientSide/BugTests/Binding_Bug28287_Spec.ts (1)
48-51: LGTM! Proper usage of response type menu interaction
The changes correctly implement the response tab UI updates mentioned in the PR objectives, using proper locators and menu interaction methods.
app/client/src/components/editorComponents/ActionExecutionInProgressView.tsx (2)
24-24: LGTM: Improved spacing between loading elements
The increased gap provides better visual separation between the spinner, message, and button.
58-58: LGTM: Enhanced loading state visual hierarchy
The larger spinner (lg) combined with the smaller cancel button (sm) creates a better visual hierarchy, making the loading state more prominent while keeping the cancel action accessible but not distracting.
Also applies to: 66-66
app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/styles.ts (3)
27-42: LGTM! Well-structured status bar implementation
The status bar styling aligns perfectly with the PR objectives, using modern CSS features appropriately.
60-73: LGTM! Well-implemented text styling with props
The StatusBarText component effectively implements the required tooltip decoration and text styling variations using props.
5-5: Consider removing unused styled component
The HelpSection component appears to be empty and potentially unused.
app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx (1)
Line range hint 16-40: Verify mock store configuration completeness.
The mock store setup might need updates to support new features mentioned in PR objectives:
- Query duration and HTTP status
- Request execution time in Redux
Let's verify the required Redux state structure:
app/client/cypress/e2e/Regression/ServerSide/Datasources/Redis_Basic_Spec.ts (1)
33-34: Method naming and parameters have been standardized.
The renaming of RunQueryNVerifyResponseViews to runQueryAndVerifyResponseViews follows camelCase convention and improves readability. The addition of the count parameter for multi-response assertions is a good practice.
Also applies to: 38-39, 43-44, 49-50, 53-54, 58-59, 73-74, 77-78, 90-91, 94-95
app/client/src/components/editorComponents/JSResponseView.test.tsx (2)
119-119: LGTM! Function call syntax is correct.
The assertion properly calls EMPTY_RESPONSE_RUN as a function, matching its new implementation.
147-147: LGTM! Negative test case is properly updated.
The assertion correctly uses queryByText with the new function call.
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Table.tsx (4)
46-46: LGTM! Good UX improvement
Changed overflow to 'auto' which shows scrollbars only when needed.
72-72: LGTM! Improved table row separation
Good use of design system color tokens for consistent styling.
337-337: LGTM! Consistent prop naming
Correctly updated to match the defaultColumn configuration.
249-249: Verify the impact of changing width to minWidth
This change in column behavior could affect existing table layouts. Please ensure all query responses render correctly with varying data lengths.
✅ Verification successful
Let me search for any other instances of minWidth in react-table configurations.
Let me check the specific Table component implementation to understand the context of this change.
Change from width to minWidth is safe and aligned with react-table's column sizing behavior
The change is appropriate because:
- This is a react-table implementation where
minWidthis the recommended property for column sizing - The table already uses
useResizeColumnshook which works withminWidth - The value 170px is consistent with other table implementations in the codebase
- The component has proper resize handlers and column width management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other table width configurations that might need similar updates
rg -g '*.{ts,tsx}' 'width:|width=' --type ts
Length of output: 104041
Script:
#!/bin/bash
# Search for react-table minWidth configurations
rg -g '*.{ts,tsx}' 'minWidth:' --type ts
Length of output: 39035
Script:
#!/bin/bash
# Get the Table component implementation
cat app/client/src/PluginActionEditor/components/PluginActionResponse/components/Table.tsx
Length of output: 11509
app/client/cypress/e2e/Sanity/Datasources/Arango_Basic_Spec.ts (1)
222-222: Method renaming follows camelCase convention
The renaming of RunQueryNVerifyResponseViews to runQueryAndVerifyResponseViews improves consistency with JavaScript/TypeScript naming conventions. The change to use an object parameter for count also enhances maintainability.
Also applies to: 235-235, 268-268, 279-279, 290-290
app/client/cypress/e2e/Regression/ServerSide/Postgres_DataTypes/UUID_Spec.ts (1)
262-262: LGTM! Method renaming follows consistent pattern
The renaming of RunQuery to runQueryAndVerifyResponseViews improves code consistency and readability.
Also applies to: 300-300, 317-317
app/client/cypress/e2e/Regression/ServerSide/QueryPane/S3_1_spec.js (3)
80-83: LGTM! Method signature change improves readability.
The updated method signature using an object parameter makes the code more maintainable and easier to understand.
198-198: LGTM! Response type selection method follows naming conventions.
The new method name selectResponseResponseTypeFromMenu clearly describes its purpose.
Line range hint 1-600: Verify test stability with API-based setup/teardown.
According to the coding guidelines, login/logout/signup operations should be performed via API. Consider refactoring the before/after hooks to use API calls.
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL2_Spec.ts (2)
71-71: LGTM! Method rename follows camelCase convention.
The renaming of RunQueryNVerifyResponseViews to runQueryAndVerifyResponseViews improves consistency with JavaScript/TypeScript naming conventions.
378-378: LGTM! Consistent method naming.
The method rename is consistently applied throughout the file.
app/client/cypress/e2e/Regression/ServerSide/QueryPane/Mongo1_spec.ts (1)
335-339: LGTM! Method signature improvement enhances test maintainability.
The changes consistently update the method calls across all test cases, improving the API by:
- Using camelCase naming convention
- Accepting an object parameter for better extensibility
- Adding response type verification
Also applies to: 358-362, 441-445, 479-483, 527-531, 569-574, 602-606, 636-640, 674-678, 700-704, 738-742, 758-761, 924-927
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/Postgres2_Spec.ts (3)
64-64: Method name updated for better clarity and consistency.
The renaming from RunQueryNVerifyResponseViews to runQueryAndVerifyResponseViews follows camelCase convention and improves readability.
73-73: Improved parameter passing with object destructuring.
The update to accept an object parameter with count property follows best practices for parameter handling in JavaScript/TypeScript.
620-620: Consistent method naming applied.
The method rename is consistently applied across the file.
app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx (2)
1075-1085: LGTM! New content type icons are properly imported.
The imports follow the established pattern and align with the PR objectives to add content type icons.
1165-1167: LGTM! Content type icons are properly added to ICON_LOOKUP.
The new entries maintain alphabetical ordering and follow the established naming convention.
app/client/cypress/e2e/Regression/ClientSide/BugTests/Binding_Bug28287_Spec.ts
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ServerSide/GenerateCRUD/MySQL1_Spec.ts
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/11962635810. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx (3)
60-68: Consider improving Props interface typingThe
isRunDisabledprop could be more strictly typed to prevent accidental undefined values.interface Props { actionSource: SourceEntity; - isRunDisabled?: boolean; + isRunDisabled: boolean; isRunning: boolean; onRunClick: () => void; currentActionConfig: Action; runErrorMessage?: string; actionName: string; }
163-177: Improve null safety with optional chainingMultiple instances where optional chaining could prevent potential runtime errors.
- const responseBodyTabs = - responseDataTypes && - responseDataTypes.map((dataType, index) => { + const responseBodyTabs = responseDataTypes?.map((dataType, index) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 163-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
394-477: Extract response viewer into a separate componentThe response data container logic is complex and could be extracted into a dedicated component.
Consider creating a
ResponseViewercomponent to handle the response display logic, which would improve maintainability and testing.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx
[error] 142-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 163-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 180-183: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 197-200: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 359-359: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 364-364: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
...ctionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx
Show resolved
Hide resolved
...ctionEditor/components/PluginActionResponse/components/QueryResponseTab/QueryResponseTab.tsx
Show resolved
Hide resolved
|
Deploy-Preview-URL: https://ce-37640.dp.appsmith.com |
|
@sagar-qa007 @ApekshaBhosale would you please take a look at this PR as it is in its final state (at least for Cypress tests). |
app/client/src/PluginActionEditor/components/PluginActionResponse/components/NoResponse.tsx
Outdated
Show resolved
Hide resolved
## Description Query response tab UI update. Fixes appsmithorg#35290 ## Automation /ok-to-test tags="@tag.All" ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new `QueryResponseTab` component for managing query responses in the plugin action editor. - Added new icons for better visual representation in the application. - Enhanced the `NoResponse` component for improved layout and messaging. - Added a new constant for clearer user instructions on obtaining responses. - **Improvements** - Updated button labels and component structures for clarity and usability. - Refined validation logic and error handling across various components. - Enhanced styling and layout behavior in the `Table` and `ActionExecutionInProgressView` components. - Streamlined the `EntityBottomTabs` component's styling for consistency. - Adjusted the height of the response container during action execution. - **Bug Fixes** - Adjusted test cases to ensure accurate validation of response handling and UI elements. - **Chores** - Deprecated older methods and constants to streamline code and enhance maintainability. - Removed unused components to simplify the codebase. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Description
Query response tab UI update.
Fixes #35290
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12045663742
Commit: 9387d0e
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ServerSide/ApiTests/API_Bugs_Spec.js
List of identified flaky tests.Wed, 27 Nov 2024 15:24:30 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
QueryResponseTabcomponent for managing query responses in the plugin action editor.NoResponsecomponent for improved layout and messaging.Improvements
TableandActionExecutionInProgressViewcomponents.EntityBottomTabscomponent's styling for consistency.Bug Fixes
Chores