Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Added warning message in response pane while prepared statement is on #36407

Merged
merged 6 commits into from
Sep 23, 2024
6 changes: 6 additions & 0 deletions app/client/src/ce/constants/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2503,3 +2503,9 @@ export const EMPTY_DATASOURCE_TOOLTIP_SIDEBUTTON = () =>
"Create a datasource to power your app with data.";

export const FIELD_REQUIRED_MESSAGE = () => `This field is required`;

export const PREPARED_STATEMENT_WARNING = {
MESSAGE: () =>
"Prepared Statements are currently enabled, which may be causing the query error. Turn them off and try running the query again",
LINK: () => "Open settings",
};
Comment on lines +2507 to +2511
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done on introducing the PREPARED_STATEMENT_WARNING constant

You've defined a new constant to display a warning message when prepared statements are enabled and an error occurs. This aligns perfectly with the PR objectives and enhances user awareness during error scenarios.

However, let's consider a small improvement to keep our code consistent and maintainable.

To maintain consistency with other message constants in this file, it would be better to define MESSAGE and LINK as constant strings rather than arrow functions, unless dynamic computation is required. Many existing constants use direct string assignments.

Here's how you might refactor the code:

-export const PREPARED_STATEMENT_WARNING = {
-  MESSAGE: () =>
-      "Prepared Statements are currently enabled, which may be causing the query error. Turn them off and try running the query again",
-  LINK: () => "Open settings",
+export const PREPARED_STATEMENT_WARNING_MESSAGE =
+  "Prepared Statements are currently enabled, which may be causing the query error. Turn them off and try running the query again.";
+export const PREPARED_STATEMENT_WARNING_LINK = "Open settings";

This change makes the constants consistent with the rest of the file and simplifies their usage.

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.

Suggested change
export const PREPARED_STATEMENT_WARNING = {
MESSAGE: () =>
"Prepared Statements are currently enabled, which may be causing the query error. Turn them off and try running the query again",
LINK: () => "Open settings",
};
export const PREPARED_STATEMENT_WARNING_MESSAGE =
"Prepared Statements are currently enabled, which may be causing the query error. Turn them off and try running the query again.";
export const PREPARED_STATEMENT_WARNING_LINK = "Open settings";

235 changes: 235 additions & 0 deletions app/client/src/pages/Editor/QueryEditor/QueryResponseTab.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
import React from "react";
import { render } from "@testing-library/react";
import { Provider } from "react-redux";
import configureStore from "redux-mock-store";
import QueryResponseTab from "./QueryResponseTab";
import { ENTITY_TYPE } from "ee/entities/AppsmithConsole/utils";
import type { Action } from "entities/Action";
import { unitTestBaseMockStore } from "layoutSystems/common/dropTarget/unitTestUtils";
import { EditorViewMode } from "ee/entities/IDE/constants";
import { lightTheme } from "selectors/themeSelectors";
import { ThemeProvider } from "styled-components";
import { BrowserRouter as Router } from "react-router-dom";

// Mock store
const mockStore = configureStore([]);

const defaultProps = {
actionName: "Test Action",
actionSource: {
name: "test source",
id: "test-source-id",
type: ENTITY_TYPE.ACTION,
},
currentActionConfig: {
id: "test-action-id",
name: "Test Action",
actionConfiguration: { pluginSpecifiedTemplates: [{ value: true }] },
userPermissions: ["execute"],
} as Action,
isRunning: false,
onRunClick: jest.fn(),
runErrorMessage: "",
};

const storeData = {
...unitTestBaseMockStore,
evaluations: {
tree: {},
},
entities: {
plugins: {
list: [],
},
datasources: {
structure: {},
},
},
ui: {
...unitTestBaseMockStore.ui,
users: {
featureFlag: {
data: {},
overriddenFlags: {},
},
},
ide: {
view: EditorViewMode.FullScreen,
},
debugger: {
context: {
errorCount: 0,
},
},
queryPane: {
debugger: {
open: true,
responseTabHeight: 200,
selectedTab: "response",
},
},
},
};

describe("QueryResponseTab", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let store: any;

beforeEach(() => {
store = mockStore(storeData);
});

/** Test use prepared statement warning **/
it("1. Prepared statement warning should not be showing", () => {
const { container } = render(
<Provider store={store}>
<ThemeProvider theme={lightTheme}>
<Router>
<QueryResponseTab {...defaultProps} />
</Router>
</ThemeProvider>
</Provider>,
);

// Check if the prepared statement warning is not showing
expect(
container.querySelector("[data-testid='t--prepared-statement-warning']"),
).toBeNull();
});

it("2. Check if prepared statement warning is not showing while running the query", () => {
const { container } = render(
<Provider store={store}>
<ThemeProvider theme={lightTheme}>
<Router>
<QueryResponseTab {...defaultProps} isRunning />
</Router>
</ThemeProvider>
</Provider>,
);

// Check if the prepared statement warning is showing
expect(
container.querySelector("[data-testid='t--prepared-statement-warning']"),
).toBeNull();
});

it("3. Check if prepared statement warning is not showing when run is successful", () => {
store = mockStore({
...storeData,
entities: {
...storeData.entities,
actions: [
{
config: {
id: "test-action-id",
name: "Test Action",
},
isLoading: false,
data: {
body: [{ key: "value" }],
isExecutionSuccess: true,
},
},
],
},
});

const { container } = render(
<Provider store={store}>
<ThemeProvider theme={lightTheme}>
<Router>
<QueryResponseTab {...defaultProps} />
</Router>
</ThemeProvider>
</Provider>,
);

// Check if the prepared statement warning is showing
expect(
container.querySelector("[data-testid='t--prepared-statement-warning']"),
).toBeNull();
});

it("4. Check if prepared statement warning is showing when run is failed", () => {
store = mockStore({
...storeData,
entities: {
...storeData.entities,
actions: [
{
config: {
id: "test-action-id",
name: "Test Action",
},
isLoading: false,
data: {
body: "ERROR: relation 'userssss' does not exist Position: 15",
isExecutionSuccess: false,
},
},
],
},
});

const { container } = render(
<Provider store={store}>
<ThemeProvider theme={lightTheme}>
<Router>
<QueryResponseTab {...defaultProps} />
</Router>
</ThemeProvider>
</Provider>,
);

// Check if the prepared statement warning is showing
expect(
container.querySelector("[data-testid='t--prepared-statement-warning']"),
).not.toBeNull();
});

it("5. Check if prepared statement warning is not showing when prepared statement is turned off", () => {
store = mockStore({
...storeData,
entities: {
...storeData.entities,
actions: [
{
config: {
id: "test-action-id",
name: "Test Action",
},
isLoading: false,
data: {
body: "ERROR: relation 'userssss' does not exist Position: 15",
isExecutionSuccess: false,
},
},
],
},
});

const props = {
...defaultProps,
currentActionConfig: {
...defaultProps.currentActionConfig,
actionConfiguration: { pluginSpecifiedTemplates: [{ value: false }] },
} as Action,
};

const { container } = render(
<Provider store={store}>
<ThemeProvider theme={lightTheme}>
<Router>
<QueryResponseTab {...props} />
</Router>
</ThemeProvider>
</Provider>,
);

// Check if the prepared statement warning is showing
expect(
container.querySelector("[data-testid='t--prepared-statement-warning']"),
).toBeNull();
});
});
48 changes: 46 additions & 2 deletions app/client/src/pages/Editor/QueryEditor/QueryResponseTab.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from "react";
import React, { useCallback, useState } from "react";
import { useDispatch, useSelector } from "react-redux";
import ReactJson from "react-json-view";
import {
Expand All @@ -13,7 +13,12 @@ import LogAdditionalInfo from "components/editorComponents/Debugger/ErrorLogs/co
import LogHelper from "components/editorComponents/Debugger/ErrorLogs/components/LogHelper";
import LOG_TYPE from "entities/AppsmithConsole/logtype";
import { JsonWrapper } from "components/editorComponents/Debugger/ErrorLogs/components/LogCollapseData";
import { Callout, Flex, SegmentedControl } from "@appsmith/ads";
import {
Callout,
Flex,
SegmentedControl,
type CalloutLinkProps,
} from "@appsmith/ads";
import styled from "styled-components";
import { DEBUGGER_TAB_KEYS } from "components/editorComponents/Debugger/helpers";
import AnalyticsUtil from "ee/utils/AnalyticsUtil";
Expand All @@ -32,6 +37,12 @@ import ActionExecutionInProgressView from "components/editorComponents/ActionExe
import { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig";
import BindDataButton from "./BindDataButton";
import { getQueryPaneDebuggerState } from "selectors/queryPaneSelectors";
import { setQueryPaneConfigSelectedTabIndex } from "actions/queryPaneActions";
import { EDITOR_TABS } from "constants/QueryEditorConstants";
import {
createMessage,
PREPARED_STATEMENT_WARNING,
} from "ee/constants/messages";

const HelpSection = styled.div``;

Expand Down Expand Up @@ -151,6 +162,7 @@ const QueryResponseTab = (props: Props) => {

let error = runErrorMessage;
let hintMessages: Array<string> = [];
let showPreparedStatementWarning = false;
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let output: Record<string, any>[] | null = null;
Expand Down Expand Up @@ -189,8 +201,31 @@ const QueryResponseTab = (props: Props) => {
error = "";
hintMessages = actionResponse.messages;
}

const { pluginSpecifiedTemplates } =
currentActionConfig.actionConfiguration;

if (
error &&
pluginSpecifiedTemplates &&
pluginSpecifiedTemplates.length > 0 &&
pluginSpecifiedTemplates[0].value === true
) {
showPreparedStatementWarning = true;
}
}

const navigateToSettings = useCallback(() => {
dispatch(setQueryPaneConfigSelectedTabIndex(EDITOR_TABS.SETTINGS));
}, []);

const preparedStatementCalloutLinks: CalloutLinkProps[] = [
{
onClick: navigateToSettings,
children: createMessage(PREPARED_STATEMENT_WARNING.LINK),
},
];

if (isRunning) {
return (
<ActionExecutionInProgressView
Expand All @@ -202,6 +237,15 @@ const QueryResponseTab = (props: Props) => {

return (
<ResponseContentWrapper isError={!!error}>
{showPreparedStatementWarning && (
<Callout
data-testid="t--prepared-statement-warning"
kind="warning"
links={preparedStatementCalloutLinks}
>
{createMessage(PREPARED_STATEMENT_WARNING.MESSAGE)}
</Callout>
)}
{error && (
<ResponseTabErrorContainer>
<ResponseTabErrorContent>
Expand Down
Loading