Skip to content
Snippets Groups Projects
Unverified Commit 8e916de7 authored by Nick Fitzpatrick's avatar Nick Fitzpatrick Committed by GitHub
Browse files

Removing Question Edit Modal and adding tests (#23472)

parent df476c8f
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 77 additions and 323 deletions
enterprise/frontend/src/metabase-enterprise/caching/components/QuestionCacheSection/QuestionCacheSection.tsx
+ 1
− 0
View file @ 8e916de7
......@@ -62,6 +62,7 @@ export const QuestionCacheSection = ({
placeholder="24"
value={cacheTTL || ""}
onChange={handleChange}
data-testid="question-cache-ttl-input"
/>
{t`hours`}
</Text>
......
frontend/src/metabase/query_builder/components/QueryModals.jsx
+ 0
− 9
View file @ 8e916de7
......@@ -11,7 +11,6 @@ import Modal from "metabase/components/Modal";
import SaveQuestionModal from "metabase/containers/SaveQuestionModal";
import QuestionSavedModal from "metabase/components/QuestionSavedModal";
import AddToDashSelectDashModal from "metabase/containers/AddToDashSelectDashModal";
import EditQuestionInfoModal from "metabase/query_builder/components/view/EditQuestionInfoModal";
import CollectionMoveModal from "metabase/containers/CollectionMoveModal";
import ArchiveQuestionModal from "metabase/query_builder/containers/ArchiveQuestionModal";
......@@ -190,14 +189,6 @@ export default class QueryModals extends React.Component {
<Modal onClose={onCloseModal}>
<ArchiveQuestionModal question={question} onClose={onCloseModal} />
</Modal>
) : modal === MODAL_TYPES.EDIT ? (
<Modal onClose={onCloseModal}>
<EditQuestionInfoModal
question={question}
onClose={onCloseModal}
onSave={card => this.props.onSave(card, false)}
/>
</Modal>
) : modal === MODAL_TYPES.EMBED ? (
<Modal full onClose={onCloseModal}>
<QuestionEmbedWidget card={this.props.card} onClose={onCloseModal} />
......
frontend/src/metabase/query_builder/components/view/EditQuestionInfoModal.jsx deleted 100644 → 0
+ 0
− 86
View file @ df476c8f
import React, { useCallback } from "react";
import PropTypes from "prop-types";
import { t } from "ttag";
import _ from "underscore";
import Form from "metabase/containers/Form";
import ModalContent from "metabase/components/ModalContent";
import CollapseSection from "metabase/components/CollapseSection";
import { PLUGIN_CACHING } from "metabase/plugins";
import Questions from "metabase/entities/questions";
const COLLAPSED_FIELDS = ["cache_ttl"];
const propTypes = {
question: PropTypes.object.isRequired, // metabase-lib Question instance
onSave: PropTypes.func.isRequired,
onClose: PropTypes.func.isRequired,
};
function EditQuestionInfoModal({ question, onClose, onSave }) {
const modalTitle = question.isDataset() ? t`Edit model` : t`Edit question`;
const onSubmit = useCallback(
async card => {
await onSave({ ...question.card(), ...card });
onClose();
},
[question, onSave, onClose],
);
const isCachedImplicitly =
PLUGIN_CACHING.getQuestionsImplicitCacheTTL(question) > 0;
return (
<ModalContent title={modalTitle} onClose={onClose}>
<Form
initialValues={question.card()}
form={Questions.forms.edit}
onSubmit={onSubmit}
>
{({ Form, FormField, FormFooter, formFields }) => {
const [visibleFields, collapsedFields] = _.partition(
formFields,
field => !COLLAPSED_FIELDS.includes(field.name),
);
const cacheFieldExtraProps = {};
if (isCachedImplicitly) {
cacheFieldExtraProps.className = "mt1";
} else {
cacheFieldExtraProps.title = null;
}
return (
<Form>
{visibleFields.map(field => (
<FormField key={field.name} name={field.name} />
))}
{collapsedFields.length > 0 && (
<CollapseSection
header={t`More options`}
iconVariant="up-down"
iconPosition="right"
headerClass="text-bold text-medium text-brand-hover"
bodyClass="pt2"
>
<FormField
name="cache_ttl"
question={question}
{...cacheFieldExtraProps}
/>
</CollapseSection>
)}
<FormFooter submitTitle={t`Save`} onCancel={onClose} />
</Form>
);
}}
</Form>
</ModalContent>
);
}
EditQuestionInfoModal.propTypes = propTypes;
export default EditQuestionInfoModal;
frontend/src/metabase/query_builder/components/view/EditQuestionInfoModal.unit.spec.js deleted 100644 → 0
+ 0
− 226
View file @ df476c8f
import React from "react";
import { fireEvent, renderWithProviders, screen } from "__support__/ui";
import userEvent from "@testing-library/user-event";
import xhrMock from "xhr-mock";
import { setupEnterpriseTest } from "__support__/enterprise";
import MetabaseSettings from "metabase/lib/settings";
import EditQuestionInfoModal from "./EditQuestionInfoModal";
const QUESTION = {
id: 1,
name: "Question",
description: "I'm here for your unit tests",
collection_id: null,
cache_ttl: 0,
archived: false,
};
function mockCachingSettings({ enabled = true } = {}) {
const original = MetabaseSettings.get.bind(MetabaseSettings);
const spy = jest.spyOn(MetabaseSettings, "get");
spy.mockImplementation(key => {
if (key === "enable-query-caching") {
return enabled;
}
if (key === "query-caching-min-ttl") {
return 10000;
}
if (key === "application-name") {
return "Metabase Test";
}
if (key === "version") {
return { tag: "" };
}
if (key === "is-hosted?") {
return false;
}
if (key === "enable-enhancements?") {
return false;
}
return original(key);
});
}
function setup({ cachingEnabled = true } = {}) {
mockCachingSettings({
enabled: cachingEnabled,
});
const onSave = jest.fn();
const onClose = jest.fn();
const question = {
card: () => QUESTION,
database: () => ({
cache_ttl: null,
}),
isDataset: () => true,
};
renderWithProviders(
<EditQuestionInfoModal
question={question}
onSave={onSave}
onClose={onClose}
/>,
);
return {
onSave,
onClose,
};
}
function fillNumericInput(input, value) {
userEvent.clear(input);
userEvent.type(input, String(value));
input.blur();
}
function fillForm({ name, description, cache_ttl } = {}) {
if (name) {
const input = screen.getByLabelText("Name");
userEvent.clear(input);
userEvent.type(input, name);
}
if (description) {
const input = screen.getByLabelText("Description");
userEvent.clear(input);
userEvent.type(input, description);
}
if (cache_ttl) {
const input = screen.getByLabelText("Caching");
fillNumericInput(input, cache_ttl);
}
}
describe("EditQuestionInfoModal", () => {
beforeEach(() => {
xhrMock.setup();
xhrMock.get("/api/collection", {
body: JSON.stringify([
{
id: "root",
name: "Our analytics",
can_write: true,
},
]),
});
});
afterEach(() => {
xhrMock.teardown();
});
it("displays fields with filled values", () => {
setup();
expect(screen.queryByLabelText("Name")).toBeInTheDocument();
expect(screen.queryByLabelText("Name")).toHaveValue(QUESTION.name);
expect(screen.queryByLabelText("Description")).toBeInTheDocument();
expect(screen.queryByLabelText("Description")).toHaveValue(
QUESTION.description,
);
expect(screen.queryByRole("button", { name: "Save" })).toBeInTheDocument();
expect(
screen.queryByRole("button", { name: "Cancel" }),
).toBeInTheDocument();
});
it("calls onClose when Cancel button is clicked", () => {
const { onClose } = setup();
fireEvent.click(screen.queryByRole("button", { name: "Cancel" }));
expect(onClose).toHaveBeenCalledTimes(1);
});
it("can't submit if name is empty", () => {
setup();
fillForm({ name: "" });
fireEvent.click(screen.queryByRole("button", { name: "Save" }));
expect(screen.queryByRole("button", { name: "Save" })).toBeDisabled();
});
it("calls onSave callback on successful update", () => {
const UPDATES = {
name: "New fancy question name",
description: "Just testing if updates work correctly",
};
const { onSave } = setup();
fillForm(UPDATES);
fireEvent.click(screen.queryByRole("button", { name: "Save" }));
expect(onSave).toHaveBeenCalledTimes(1);
expect(onSave).toHaveBeenCalledWith({
...QUESTION,
...UPDATES,
});
});
describe("Cache TTL field", () => {
describe("OSS", () => {
it("is not shown", () => {
setup();
expect(screen.queryByText("More options")).not.toBeInTheDocument();
expect(
screen.queryByText("Cache all question results for"),
).not.toBeInTheDocument();
});
});
describe("EE", () => {
beforeEach(() => {
setupEnterpriseTest();
});
describe("caching enabled", () => {
it("is shown", () => {
setup();
fireEvent.click(screen.queryByText("More options"));
expect(screen.getByDisplayValue("0")).toBeInTheDocument();
});
it("can be changed", () => {
const { onSave } = setup();
fireEvent.click(screen.queryByText("More options"));
fillNumericInput(screen.getByDisplayValue("0"), 10);
fireEvent.click(screen.queryByRole("button", { name: "Save" }));
expect(onSave).toHaveBeenCalledWith({
...QUESTION,
cache_ttl: 10,
});
});
});
describe("caching disabled", () => {
it("is not shown if caching is disabled", () => {
setup({ cachingEnabled: false });
expect(screen.queryByText("More options")).not.toBeInTheDocument();
expect(
screen.queryByText("Cache all question results for"),
).not.toBeInTheDocument();
});
it("can still submit the form", () => {
const { onSave } = setup({
cachingEnabled: false,
});
fillForm({ name: "Test" });
fireEvent.click(screen.queryByRole("button", { name: "Save" }));
expect(onSave).toHaveBeenCalledWith({
...QUESTION,
name: "Test",
});
});
});
});
});
});
frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar.unit.spec.js
+ 76
− 1
View file @ 8e916de7
......@@ -5,6 +5,8 @@ import {
ORDERS,
metadata,
} from "__support__/sample_database_fixture";
import { setupEnterpriseTest } from "__support__/enterprise";
import MetabaseSettings from "metabase/lib/settings";
import Question from "metabase-lib/lib/Question";
import { QuestionInfoSidebar } from "./QuestionInfoSidebar";
......@@ -24,8 +26,32 @@ const BASE_QUESTION = {
"source-table": ORDERS.id,
},
},
moderation_reviews: [
{
status: "verified",
moderator_id: 1,
created_at: Date.now(),
most_recent: true,
},
],
};
function mockCachingSettings({ enabled = true } = {}) {
const original = MetabaseSettings.get.bind(MetabaseSettings);
const spy = jest.spyOn(MetabaseSettings, "get");
spy.mockImplementation(key => {
const settings = {
"enable-query-caching": enabled,
"query-caching-min-ttl": 10000,
"application-name": "Metabase Test",
version: { tag: "" },
"is-hosted?": false,
"enable-enhancements?": false,
};
return settings[key] ?? original(key);
});
}
function getQuestion(card) {
return new Question(
{
......@@ -47,7 +73,11 @@ function getDataset(card) {
);
}
function setup({ question } = {}) {
function setup({ question, cachingEnabled = true } = {}) {
mockCachingSettings({
enabled: cachingEnabled,
});
const onSave = jest.fn();
return renderWithProviders(
......@@ -74,4 +104,49 @@ describe("QuestionDetailsSidebarPanel", () => {
});
});
});
describe("cache ttl field", () => {
describe("oss", () => {
it("is not shown", () => {
setup({ question: getQuestion() });
expect(
screen.queryByText("Cache Configuration"),
).not.toBeInTheDocument();
});
});
describe("ee", () => {
beforeEach(() => {
setupEnterpriseTest();
});
it("is shown if caching is enabled", () => {
setup({ question: getQuestion({ cache_ttl: 2 }) });
expect(screen.queryByText("Cache Configuration")).toBeInTheDocument();
});
it("is hidden if caching is disabled", () => {
setup({ question: getQuestion(), cachingEnabled: false });
expect(
screen.queryByText("Cache Configuration"),
).not.toBeInTheDocument();
});
});
});
describe("moderation field", () => {
beforeEach(() => {
setupEnterpriseTest();
});
it("should not show verification badge if unverified", () => {
setup({ question: getQuestion({ moderation_reviews: [] }) });
expect(screen.queryByText(/verified this/)).not.toBeInTheDocument();
});
it("should show verification badge if verified", () => {
setup({ question: getQuestion() });
expect(screen.queryByText(/verified this/)).toBeInTheDocument();
});
});
});
frontend/src/metabase/query_builder/constants.js
+ 0
− 1
View file @ 8e916de7
export const MODAL_TYPES = {
SAVE: "save",
EDIT: "edit",
ADD_TO_DASHBOARD: "add-to-dashboard",
MOVE: "move",
CLONE: "clone",
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment