From e76204c67769f2e3396ae67a7c3fa683438583ae Mon Sep 17 00:00:00 2001 From: Nick Fitzpatrick <nick@metabase.com> Date: Thu, 2 Jun 2022 16:21:36 -0300 Subject: [PATCH] Adding action button to Question Page (#23004) Adding action button to Question Page --- .../QuestionModerationButton.tsx | 96 ++++++++ .../metabase-enterprise/moderation/index.js | 2 + .../PluginPlaceholder/PluginPlaceholder.tsx | 9 + frontend/src/metabase/plugins/index.ts | 1 + .../components/QuestionActionButtons.jsx | 113 +-------- .../QuestionActionButtons.styled.jsx | 30 --- .../components/QuestionActions.styled.tsx | 62 +++++ .../components/QuestionActions.tsx | 231 ++++++++++++++++++ .../components/view/ViewHeader.jsx | 17 ++ .../components/view/ViewHeader.unit.spec.js | 14 ++ ...atasetMetadataStrengthIndicator.styled.tsx | 21 +- .../DatasetMetadataStrengthIndicator.tsx | 12 +- .../sidebars/QuestionDetailsSidebarPanel.jsx | 22 +- .../QuestionDetailsSidebarPanel.unit.spec.js | 40 --- .../e2e/helpers/e2e-ui-elements-helpers.js | 10 + .../QuestionActionButtons.unit.spec.js | 90 +++---- .../models/helpers/e2e-models-helpers.js | 16 +- .../models/models-metadata.cy.spec.js | 26 +- .../models/models-query-editor.cy.spec.js | 34 ++- .../models/models-revision-history.cy.spec.js | 12 +- .../scenarios/models/models.cy.spec.js | 21 +- ...question-with-snippets-to-model.cy.spec.js | 14 +- .../bookmarks-question.cy.spec.js | 4 +- .../moderation-question.cy.spec.js | 46 +++- .../question/question-management.cy.spec.js | 23 +- .../scenarios/question/saved.cy.spec.js | 7 +- 26 files changed, 625 insertions(+), 348 deletions(-) create mode 100644 enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationButton/QuestionModerationButton.tsx create mode 100644 frontend/src/metabase/plugins/components/PluginPlaceholder/PluginPlaceholder.tsx create mode 100644 frontend/src/metabase/query_builder/components/QuestionActions.styled.tsx create mode 100644 frontend/src/metabase/query_builder/components/QuestionActions.tsx diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationButton/QuestionModerationButton.tsx b/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationButton/QuestionModerationButton.tsx new file mode 100644 index 00000000000..c1bb955d4e8 --- /dev/null +++ b/enterprise/frontend/src/metabase-enterprise/moderation/components/QuestionModerationButton/QuestionModerationButton.tsx @@ -0,0 +1,96 @@ +import React from "react"; +import { t } from "ttag"; +import { connect } from "react-redux"; + +import { + MODERATION_STATUS, + getLatestModerationReview, + getStatusIcon, + isItemVerified, +} from "metabase-enterprise/moderation/service"; +import { getIsModerator } from "metabase-enterprise/moderation/selectors"; +import { + verifyCard, + removeCardReview, +} from "metabase-enterprise/moderation/actions"; + +import { VerifyButton as DefaultVerifyButton } from "../QuestionModerationSection/QuestionModerationSection.styled"; + +import { State } from "metabase-types/store"; +import Question from "metabase-lib/lib/Question"; + +interface Props { + question: Question; + verifyCard: (id: number) => void; + removeCardReview: (id: number) => void; + isModerator: boolean; + VerifyButton: React.FC; + verifyButtonProps: any; +} + +const mapStateToProps = (state: State, props: Props) => ({ + isModerator: getIsModerator(state, props), +}); + +const mapDispatchToProps = { + verifyCard, + removeCardReview, +}; + +export default connect( + mapStateToProps, + mapDispatchToProps, +)(QuestionModerationButton); + +const { name: verifiedIconName } = getStatusIcon(MODERATION_STATUS.verified); + +function QuestionModerationButton({ + question, + verifyCard, + removeCardReview, + isModerator, + VerifyButton = DefaultVerifyButton, + verifyButtonProps = {}, +}: Props) { + const latestModerationReview = getLatestModerationReview( + question.getModerationReviews(), + ); + const isVerified = isItemVerified(latestModerationReview); + + const onVerify = () => { + const id = question.id(); + verifyCard(id); + }; + + const onRemoveModerationReview = () => { + const id = question.id(); + removeCardReview(id); + }; + + return ( + <React.Fragment> + {isModerator && !isVerified && ( + <VerifyButton + icon={verifiedIconName} + onClick={onVerify} + data-testid="moderation-verify-action" + {...verifyButtonProps} + > + {question.isDataset() + ? t`Verify this model` + : t`Verify this question`} + </VerifyButton> + )} + {isModerator && isVerified && ( + <VerifyButton + icon="close" + onClick={isModerator && onRemoveModerationReview} + data-testid="moderation-remove-verification-action" + {...verifyButtonProps} + > + {t`Remove verification`} + </VerifyButton> + )} + </React.Fragment> + ); +} diff --git a/enterprise/frontend/src/metabase-enterprise/moderation/index.js b/enterprise/frontend/src/metabase-enterprise/moderation/index.js index 24ea01da56a..0b2bbd1d66b 100644 --- a/enterprise/frontend/src/metabase-enterprise/moderation/index.js +++ b/enterprise/frontend/src/metabase-enterprise/moderation/index.js @@ -2,6 +2,7 @@ import { PLUGIN_MODERATION } from "metabase/plugins"; import { hasPremiumFeature } from "metabase-enterprise/settings"; import QuestionModerationSection from "./components/QuestionModerationSection/QuestionModerationSection"; +import QuestionModerationButton from "./components/QuestionModerationButton/QuestionModerationButton"; import ModerationStatusIcon from "./components/ModerationStatusIcon/ModerationStatusIcon"; import { @@ -14,6 +15,7 @@ if (hasPremiumFeature("content_management")) { Object.assign(PLUGIN_MODERATION, { isEnabled: () => true, QuestionModerationSection, + QuestionModerationButton, ModerationStatusIcon, getStatusIconForQuestion, getStatusIcon, diff --git a/frontend/src/metabase/plugins/components/PluginPlaceholder/PluginPlaceholder.tsx b/frontend/src/metabase/plugins/components/PluginPlaceholder/PluginPlaceholder.tsx new file mode 100644 index 00000000000..9b1d5cf2d6e --- /dev/null +++ b/frontend/src/metabase/plugins/components/PluginPlaceholder/PluginPlaceholder.tsx @@ -0,0 +1,9 @@ +interface Props { + [key: string]: any; +} + +function PluginPlaceholder(props: Props) { + return null; +} + +export default PluginPlaceholder; diff --git a/frontend/src/metabase/plugins/index.ts b/frontend/src/metabase/plugins/index.ts index 0735a40fd40..2c71c7bc8a5 100644 --- a/frontend/src/metabase/plugins/index.ts +++ b/frontend/src/metabase/plugins/index.ts @@ -113,6 +113,7 @@ export const PLUGIN_COLLECTION_COMPONENTS = { export const PLUGIN_MODERATION = { isEnabled: () => false, QuestionModerationSection: PluginPlaceholder, + QuestionModerationButton: PluginPlaceholder, ModerationStatusIcon: PluginPlaceholder, getStatusIconForQuestion: object, getStatusIcon: object, diff --git a/frontend/src/metabase/query_builder/components/QuestionActionButtons.jsx b/frontend/src/metabase/query_builder/components/QuestionActionButtons.jsx index de8472656fe..454682c8b40 100644 --- a/frontend/src/metabase/query_builder/components/QuestionActionButtons.jsx +++ b/frontend/src/metabase/query_builder/components/QuestionActionButtons.jsx @@ -1,34 +1,21 @@ -import React, { useState } from "react"; +import React from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; import { connect } from "react-redux"; -import { color } from "metabase/lib/colors"; - -import { - checkDatabaseSupportsModels, - checkCanBeModel, - checkDatabaseCanPersistDatasets, -} from "metabase/lib/data-modeling/utils"; - +import { checkDatabaseCanPersistDatasets } from "metabase/lib/data-modeling/utils"; import { onModelPersistenceChange } from "metabase/query_builder/actions"; import { MODAL_TYPES } from "metabase/query_builder/constants"; import { getNestedQueriesEnabled } from "metabase/selectors/settings"; import { PLUGIN_MODEL_PERSISTENCE } from "metabase/plugins"; - import Button from "metabase/core/components/Button"; import Tooltip from "metabase/components/Tooltip"; -import { BookmarkButton, Container } from "./QuestionActionButtons.styled"; +import { Container } from "./QuestionActionButtons.styled"; export const EDIT_TESTID = "edit-details-button"; -export const ADD_TO_DASH_TESTID = "add-to-dashboard-button"; -export const MOVE_TESTID = "move-button"; -export const TURN_INTO_DATASET_TESTID = "turn-into-dataset"; export const TOGGLE_MODEL_PERSISTENCE_TESTID = "toggle-persistence"; -export const CLONE_TESTID = "clone-button"; -export const ARCHIVE_TESTID = "archive-button"; const ICON_SIZE = 18; @@ -55,42 +42,17 @@ const mapDispatchToProps = { function QuestionActionButtons({ question, canWrite, - areNestedQueriesEnabled, onOpenModal, - isBookmarked, - toggleBookmark, onModelPersistenceChange, }) { - const [animation, setAnimation] = useState(null); - - const handleClickBookmark = () => { - toggleBookmark(); - setAnimation(isBookmarked ? "shrink" : "expand"); - }; - const isSaved = question.isSaved(); const isDataset = question.isDataset(); - - const duplicateTooltip = isDataset - ? t`Duplicate this model` - : t`Duplicate this question`; - - const canTurnIntoModel = - canWrite && - !isDataset && - areNestedQueriesEnabled && - checkDatabaseSupportsModels(question.query().database()); - const canPersistDataset = PLUGIN_MODEL_PERSISTENCE.isModelLevelPersistenceEnabled() && canWrite && isSaved && isDataset && checkDatabaseCanPersistDatasets(question.query().database()); - - const bookmarkButtonColor = isBookmarked ? color("brand") : ""; - const bookmarkTooltip = isBookmarked ? t`Remove from bookmarks` : t`Bookmark`; - return ( <Container data-testid="question-action-buttons"> {canWrite && ( @@ -104,42 +66,6 @@ function QuestionActionButtons({ /> </Tooltip> )} - <Tooltip tooltip={t`Add to dashboard`}> - <Button - onlyIcon - icon="add_to_dash" - iconSize={ICON_SIZE} - onClick={() => onOpenModal(MODAL_TYPES.ADD_TO_DASHBOARD)} - data-testid={ADD_TO_DASH_TESTID} - /> - </Tooltip> - {canWrite && ( - <Tooltip tooltip={t`Move`}> - <Button - onlyIcon - icon="move" - iconSize={ICON_SIZE} - onClick={() => onOpenModal(MODAL_TYPES.MOVE)} - data-testid={MOVE_TESTID} - /> - </Tooltip> - )} - {canTurnIntoModel && ( - <Tooltip tooltip={t`Turn this into a model`}> - <Button - onlyIcon - icon="model" - iconSize={ICON_SIZE} - onClick={() => { - const modal = checkCanBeModel(question) - ? MODAL_TYPES.TURN_INTO_DATASET - : MODAL_TYPES.CAN_NOT_CREATE_MODEL; - onOpenModal(modal); - }} - data-testid={TURN_INTO_DATASET_TESTID} - /> - </Tooltip> - )} {canPersistDataset && ( <PLUGIN_MODEL_PERSISTENCE.ModelCacheControl model={question} @@ -148,39 +74,6 @@ function QuestionActionButtons({ data-testid={TOGGLE_MODEL_PERSISTENCE_TESTID} /> )} - {canWrite && ( - <Tooltip tooltip={duplicateTooltip}> - <Button - onlyIcon - icon="segment" - iconSize={ICON_SIZE} - onClick={() => onOpenModal(MODAL_TYPES.CLONE)} - data-testid={CLONE_TESTID} - /> - </Tooltip> - )} - {canWrite && ( - <Tooltip tooltip={t`Archive`}> - <Button - onlyIcon - icon="archive" - iconSize={ICON_SIZE} - onClick={() => onOpenModal(MODAL_TYPES.ARCHIVE)} - data-testid={ARCHIVE_TESTID} - /> - </Tooltip> - )} - <Tooltip tooltip={bookmarkTooltip}> - <BookmarkButton - onlyIcon - animation={animation} - icon="bookmark" - iconSize={ICON_SIZE} - isBookmarked={isBookmarked} - onClick={handleClickBookmark} - color={bookmarkButtonColor} - /> - </Tooltip> </Container> ); } diff --git a/frontend/src/metabase/query_builder/components/QuestionActionButtons.styled.jsx b/frontend/src/metabase/query_builder/components/QuestionActionButtons.styled.jsx index 8c71c517603..b1172aeeab2 100644 --- a/frontend/src/metabase/query_builder/components/QuestionActionButtons.styled.jsx +++ b/frontend/src/metabase/query_builder/components/QuestionActionButtons.styled.jsx @@ -1,34 +1,4 @@ import styled from "@emotion/styled"; -import { css } from "@emotion/react"; - -import { color } from "metabase/lib/colors"; -import Button from "metabase/core/components/Button"; -import { - shrinkOrExpandOnClick, - shrinkOrExpandDuration, -} from "metabase/styled-components/theme/button.ts"; - -export const BookmarkButton = styled(Button)` - ${shrinkOrExpandOnClick} - - ${props => - props.animation === "expand" && - css` - animation: expand linear ${shrinkOrExpandDuration}; - `} - - ${props => - props.animation === "shrink" && - css` - animation: shrink linear ${shrinkOrExpandDuration}; - `} - - &:hover { - color: ${props => - props.isBookmarked ? color("brand") : color("text-dark")}; - } - } -`; export const Container = styled.div` display: flex; diff --git a/frontend/src/metabase/query_builder/components/QuestionActions.styled.tsx b/frontend/src/metabase/query_builder/components/QuestionActions.styled.tsx new file mode 100644 index 00000000000..1994a12f4cd --- /dev/null +++ b/frontend/src/metabase/query_builder/components/QuestionActions.styled.tsx @@ -0,0 +1,62 @@ +import styled from "@emotion/styled"; +import { css } from "@emotion/react"; +import { color } from "metabase/lib/colors"; +import Button from "metabase/core/components/Button"; + +import { + shrinkOrExpandOnClick, + shrinkOrExpandDuration, +} from "metabase/styled-components/theme/button"; + +export const QuestionActionsContainer = styled.div` + border-left: 1px solid ${color("border")}; + margin-left: 1rem; + padding-left: 1rem; +`; + +export const PopoverContainer = styled.div` + padding: 1rem; + min-width: 260px; +`; + +export const PopoverButton = styled(Button)` + width: 100%; + ${Button.Content} { + justify-content: flex-start; + } + + ${Button.TextContainer} { + width: 100%; + display: flex; + justify-content: space-between; + } +`; + +export type AnimationStates = "expand" | "shrink" | null; + +interface BookmarkButtonProps { + animation: AnimationStates; + isBookmarked: boolean; +} + +export const BookmarkButton = styled(Button)<BookmarkButtonProps>` + ${shrinkOrExpandOnClick} + + ${props => + props.animation === "expand" && + css` + animation: expand linear ${shrinkOrExpandDuration}; + `} + + ${props => + props.animation === "shrink" && + css` + animation: shrink linear ${shrinkOrExpandDuration}; + `} + + &:hover { + color: ${props => + props.isBookmarked ? color("brand") : color("text-dark")}; + } + } +`; diff --git a/frontend/src/metabase/query_builder/components/QuestionActions.tsx b/frontend/src/metabase/query_builder/components/QuestionActions.tsx new file mode 100644 index 00000000000..cf132fdf50e --- /dev/null +++ b/frontend/src/metabase/query_builder/components/QuestionActions.tsx @@ -0,0 +1,231 @@ +import React, { useCallback, useState } from "react"; +import { t } from "ttag"; + +import Button from "metabase/core/components/Button"; +import Tooltip from "metabase/components/Tooltip"; +import TippyPopoverWithTrigger from "metabase/components/PopoverWithTrigger/TippyPopoverWithTrigger"; + +import DatasetMetadataStrengthIndicator from "./view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator"; + +import { PLUGIN_MODERATION } from "metabase/plugins"; + +import { MODAL_TYPES } from "metabase/query_builder/constants"; + +import { color } from "metabase/lib/colors"; +import { checkCanBeModel } from "metabase/lib/data-modeling/utils"; + +import Question from "metabase-lib/lib/Question"; + +import { + QuestionActionsContainer, + PopoverContainer, + PopoverButton, + BookmarkButton, + AnimationStates, +} from "./QuestionActions.styled"; + +const ICON_SIZE = 18; + +const ADD_TO_DASH_TESTID = "add-to-dashboard-button"; +const MOVE_TESTID = "move-button"; +const TURN_INTO_DATASET_TESTID = "turn-into-dataset"; +const TOGGLE_MODEL_PERSISTENCE_TESTID = "toggle-persistence"; +const CLONE_TESTID = "clone-button"; +const ARCHIVE_TESTID = "archive-button"; + +interface Props { + isBookmarked: boolean; + handleBookmark: () => void; + onOpenModal: (modalType: string) => void; + question: Question; + setQueryBuilderMode: ( + mode: string, + opt: { datasetEditorTab: string }, + ) => void; + turnDatasetIntoQuestion: () => void; +} + +const buttonProps = { + iconSize: ICON_SIZE, + borderless: true, + color: color("text-dark"), +}; + +const QuestionActions = ({ + isBookmarked, + handleBookmark, + onOpenModal, + question, + setQueryBuilderMode, + turnDatasetIntoQuestion, +}: Props) => { + const [animation, setAnimation] = useState<AnimationStates>(null); + + const handleClickBookmark = () => { + handleBookmark(); + setAnimation(isBookmarked ? "shrink" : "expand"); + }; + const bookmarkButtonColor = isBookmarked ? color("brand") : ""; + const bookmarkTooltip = isBookmarked ? t`Remove from bookmarks` : t`Bookmark`; + + const isDataset = question.isDataset(); + const canWrite = question.canWrite(); + + const handleEditQuery = useCallback(() => { + setQueryBuilderMode("dataset", { + datasetEditorTab: "query", + }); + }, [setQueryBuilderMode]); + + const handleEditMetadata = useCallback(() => { + setQueryBuilderMode("dataset", { + datasetEditorTab: "metadata", + }); + }, [setQueryBuilderMode]); + + const handleTurnToModel = useCallback(() => { + const modal = checkCanBeModel(question) + ? MODAL_TYPES.TURN_INTO_DATASET + : MODAL_TYPES.CAN_NOT_CREATE_MODEL; + onOpenModal(modal); + }, [onOpenModal, question]); + + return ( + <QuestionActionsContainer data-testid="question-action-buttons-container"> + <Tooltip tooltip={bookmarkTooltip}> + <BookmarkButton + animation={animation} + isBookmarked={isBookmarked} + onlyIcon + icon="bookmark" + iconSize={ICON_SIZE} + onClick={handleClickBookmark} + color={bookmarkButtonColor} + /> + </Tooltip> + + <TippyPopoverWithTrigger + key="extra-actions-menu" + placement="bottom-end" + renderTrigger={({ onClick }) => ( + <Button + onClick={onClick} + onlyIcon + icon="ellipsis" + iconSize={ICON_SIZE} + /> + )} + popoverContent={ + <PopoverContainer> + <div> + <PLUGIN_MODERATION.QuestionModerationButton + question={question} + VerifyButton={PopoverButton} + verifyButtonProps={buttonProps} + /> + </div> + {isDataset && ( + <div> + <PopoverButton + icon="notebook" + onClick={handleEditQuery} + data-testid={ADD_TO_DASH_TESTID} + {...buttonProps} + > + {t`Edit query definition`} + </PopoverButton> + </div> + )} + {isDataset && ( + <div> + <PopoverButton + icon="label" + onClick={handleEditMetadata} + data-testid={ADD_TO_DASH_TESTID} + {...buttonProps} + > + {t`Edit metadata`} + <DatasetMetadataStrengthIndicator dataset={question} /> + </PopoverButton> + </div> + )} + {!isDataset && ( + <div> + <PopoverButton + icon="dashboard" + onClick={() => onOpenModal(MODAL_TYPES.ADD_TO_DASHBOARD)} + data-testid={ADD_TO_DASH_TESTID} + {...buttonProps} + > + {t`Add to dashboard`} + </PopoverButton> + </div> + )} + {canWrite && ( + <div> + <PopoverButton + icon="move" + onClick={() => onOpenModal(MODAL_TYPES.MOVE)} + data-testid={MOVE_TESTID} + {...buttonProps} + > + {t`Move`} + </PopoverButton> + </div> + )} + {!isDataset && canWrite && ( + <div> + <PopoverButton + icon="model" + onClick={handleTurnToModel} + data-testid={TURN_INTO_DATASET_TESTID} + {...buttonProps} + > + {t`Turn into a model`} + </PopoverButton> + </div> + )} + {isDataset && canWrite && ( + <div> + <PopoverButton + icon="model_framed" + onClick={turnDatasetIntoQuestion} + data-testid="" + {...buttonProps} + > + {t`Turn back to saved question`} + </PopoverButton> + </div> + )} + {canWrite && ( + <div> + <PopoverButton + icon="segment" + onClick={() => onOpenModal(MODAL_TYPES.CLONE)} + data-testid={CLONE_TESTID} + {...buttonProps} + > + {t`Duplicate`} + </PopoverButton> + </div> + )} + {canWrite && ( + <div> + <PopoverButton + icon="archive" + onClick={() => onOpenModal(MODAL_TYPES.ARCHIVE)} + data-testid={ARCHIVE_TESTID} + {...buttonProps} + > + {t`Archive`} + </PopoverButton> + </div> + )} + </PopoverContainer> + } + /> + </QuestionActionsContainer> + ); +}; + +export default QuestionActions; diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx b/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx index 59aac594946..146d65b8fb5 100644 --- a/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx +++ b/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx @@ -32,6 +32,7 @@ import QuestionFilters, { QuestionFilterWidget, } from "./QuestionFilters"; import { QuestionSummarizeWidget } from "./QuestionSummaries"; +import QuestionActions from "../QuestionActions"; import NativeQueryButton from "./NativeQueryButton"; import { AdHocViewHeading, @@ -378,9 +379,12 @@ ViewTitleHeaderRightSide.propTypes = { onEditSummary: PropTypes.func, onCloseSummary: PropTypes.func, setQueryBuilderMode: PropTypes.func, + turnDatasetIntoQuestion: PropTypes.func, areFiltersExpanded: PropTypes.bool, onExpandFilters: PropTypes.func, onCollapseFilters: PropTypes.func, + isBookmarked: PropTypes.bool, + toggleBookmark: PropTypes.func, }; function ViewTitleHeaderRightSide(props) { @@ -388,6 +392,8 @@ function ViewTitleHeaderRightSide(props) { question, result, queryBuilderMode, + isBookmarked, + toggleBookmark, isSaved, isDataset, isNative, @@ -409,6 +415,7 @@ function ViewTitleHeaderRightSide(props) { onEditSummary, onCloseSummary, setQueryBuilderMode, + turnDatasetIntoQuestion, areFiltersExpanded, onExpandFilters, onCollapseFilters, @@ -538,6 +545,16 @@ function ViewTitleHeaderRightSide(props) { onCancel={cancelQuery} /> )} + {isSaved && ( + <QuestionActions + isBookmarked={isBookmarked} + handleBookmark={toggleBookmark} + onOpenModal={onOpenModal} + question={question} + setQueryBuilderMode={setQueryBuilderMode} + turnDatasetIntoQuestion={turnDatasetIntoQuestion} + /> + )} </div> ); } diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader.unit.spec.js b/frontend/src/metabase/query_builder/components/view/ViewHeader.unit.spec.js index b0c489d5faa..34c7d3aa95b 100644 --- a/frontend/src/metabase/query_builder/components/view/ViewHeader.unit.spec.js +++ b/frontend/src/metabase/query_builder/components/view/ViewHeader.unit.spec.js @@ -362,6 +362,13 @@ describe("ViewHeader", () => { fireEvent.click(screen.getByText(question.displayName())); expect(onOpenQuestionDetails).toHaveBeenCalled(); }); + + it("shows bookmark and action buttons", () => { + setup({ question }); + expect( + screen.queryByTestId("question-action-buttons-container"), + ).toBeInTheDocument(); + }); }); }); }); @@ -387,6 +394,13 @@ describe("ViewHeader | Ad-hoc GUI question", () => { ).toBeInTheDocument(); }); + it("does not render bookmark and action buttons", () => { + setupAdHoc(); + expect( + screen.queryByTestId("question-action-buttons-container"), + ).not.toBeInTheDocument(); + }); + describe("filters", () => { const question = getAdHocQuestion(FILTERED_GUI_QUESTION); diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.styled.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.styled.tsx index f83d14d1c60..818291b80d6 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.styled.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.styled.tsx @@ -1,34 +1,17 @@ import styled from "@emotion/styled"; export const PercentageLabel = styled.span` - position: absolute; - - top: -1rem; - left: 50%; - transform: translate(-50%, 60%); - color: ${props => props.color}; font-size: 0.8rem; font-weight: bold; user-select: none; - opacity: 0; - transition: all 0.4s; `; export const Root = styled.div` - display: flex; - flex: 1; - position: relative; - flex-direction: column; - - &:hover { - ${PercentageLabel} { - opacity: 1; - transform: translate(-50%, 0); - } - } + display: inline-block; + float: right; `; export const TooltipParagraph = styled.p` diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.tsx index 892e0be58b7..be7b6b3cfb5 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/DatasetManagementSection/DatasetMetadataStrengthIndicator/DatasetMetadataStrengthIndicator.tsx @@ -22,7 +22,7 @@ function getIndicationColor(percentage: number, isHovered: boolean): string { return color("danger"); } if (!isHovered) { - return color("bg-medium"); + return color("text-medium"); } return percentage >= 0.9 ? color("success") : color("warning"); } @@ -75,14 +75,12 @@ function DatasetMetadataStrengthIndicator({ dataset, ...props }: Props) { delay={TOOLTIP_DELAY} placement="bottom" > - <PercentageLabel color={indicationColor}> + <PercentageLabel + color={indicationColor} + data-testid="tooltip-component-wrapper" + > {formatPercentage(percentage)} </PercentageLabel> - <ProgressBar - percentage={percentage} - color={indicationColor} - height="8px" - /> </Tooltip> </Root> ); diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx index 973ca774341..0650e282467 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.jsx @@ -5,15 +5,12 @@ import QuestionActionButtons from "metabase/query_builder/components/QuestionAct import { ClampedDescription } from "metabase/query_builder/components/ClampedDescription"; import QuestionActivityTimeline from "metabase/query_builder/components/QuestionActivityTimeline"; -import { PLUGIN_MODERATION, PLUGIN_MODEL_PERSISTENCE } from "metabase/plugins"; +import { PLUGIN_MODEL_PERSISTENCE } from "metabase/plugins"; import { Container, - BorderedSectionContainer, SidebarPaddedContent, - ModerationSectionContainer, } from "./QuestionDetailsSidebarPanel.styled"; -import DatasetManagementSection from "./DatasetManagementSection"; QuestionDetailsSidebarPanel.propTypes = { question: PropTypes.object.isRequired, @@ -38,9 +35,6 @@ function QuestionDetailsSidebarPanel({ } : undefined; - const hasSecondarySection = - (isDataset && canWrite) || (!isDataset && PLUGIN_MODERATION.isEnabled()); - return ( <Container> <SidebarPaddedContent> @@ -61,20 +55,6 @@ function QuestionDetailsSidebarPanel({ model={question} /> )} - {hasSecondarySection && ( - <BorderedSectionContainer> - {isDataset && canWrite && ( - <DatasetManagementSection dataset={question} /> - )} - {!isDataset && ( - <ModerationSectionContainer> - <PLUGIN_MODERATION.QuestionModerationSection - question={question} - /> - </ModerationSectionContainer> - )} - </BorderedSectionContainer> - )} </SidebarPaddedContent> <QuestionActivityTimeline question={question} /> </Container> diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.unit.spec.js b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.unit.spec.js index 1a3cfc4aebf..3ddbb91c1d5 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.unit.spec.js +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionDetailsSidebarPanel.unit.spec.js @@ -1,6 +1,5 @@ import React from "react"; import { renderWithProviders, screen } from "__support__/ui"; -import { setupEnterpriseTest } from "__support__/enterprise"; import { SAMPLE_DATABASE, ORDERS, @@ -90,43 +89,4 @@ describe("QuestionDetailsSidebarPanel", () => { }); }); }); - - describe("models", () => { - it("displays model management section", () => { - setup({ question: getDataset() }); - expect(screen.queryByText("Model management")).toBeInTheDocument(); - expect( - screen.queryByText("Turn back into a saved question"), - ).toBeInTheDocument(); - }); - - it("does not display model management section with read-only-access", () => { - setup({ question: getDataset({ can_write: false }) }); - expect(screen.queryByText("Model management")).not.toBeInTheDocument(); - expect( - screen.queryByText("Turn back into a saved question"), - ).not.toBeInTheDocument(); - }); - }); - - describe("saved questions", () => { - it("does not display model management section", () => { - setup({ question: getQuestion() }); - expect(screen.queryByText("Model management")).not.toBeInTheDocument(); - expect( - screen.queryByText("Turn back into a saved question"), - ).not.toBeInTheDocument(); - }); - - describe("content moderation", () => { - beforeEach(() => { - setupEnterpriseTest(); - }); - - it("offers to verify a question", () => { - setup({ question: getQuestion() }); - expect(screen.queryByText("Verify this question")).toBeInTheDocument(); - }); - }); - }); }); diff --git a/frontend/test/__support__/e2e/helpers/e2e-ui-elements-helpers.js b/frontend/test/__support__/e2e/helpers/e2e-ui-elements-helpers.js index 66998e1c6c8..ba9feaa6549 100644 --- a/frontend/test/__support__/e2e/helpers/e2e-ui-elements-helpers.js +++ b/frontend/test/__support__/e2e/helpers/e2e-ui-elements-helpers.js @@ -61,3 +61,13 @@ export function browse() { export function filterWidget() { return cy.get("fieldset"); } + +export const openQuestionActions = () => { + cy.findByTestId("question-action-buttons-container").within(() => { + cy.icon("ellipsis").click(); + }); +}; + +export const closeQuestionActions = () => { + cy.findByTestId("qb-header").click(); +}; diff --git a/frontend/test/metabase/query_builder/components/QuestionActionButtons.unit.spec.js b/frontend/test/metabase/query_builder/components/QuestionActionButtons.unit.spec.js index 3753b00ebf0..88b74975f26 100644 --- a/frontend/test/metabase/query_builder/components/QuestionActionButtons.unit.spec.js +++ b/frontend/test/metabase/query_builder/components/QuestionActionButtons.unit.spec.js @@ -5,19 +5,9 @@ import { MODAL_TYPES } from "metabase/query_builder/constants"; import QuestionActionButtons, { EDIT_TESTID, - ADD_TO_DASH_TESTID, - MOVE_TESTID, - CLONE_TESTID, - ARCHIVE_TESTID, } from "metabase/query_builder/components/QuestionActionButtons"; -const testIdActionPairs = [ - [EDIT_TESTID, MODAL_TYPES.EDIT], - [ADD_TO_DASH_TESTID, MODAL_TYPES.ADD_TO_DASHBOARD], - [MOVE_TESTID, MODAL_TYPES.MOVE], - [CLONE_TESTID, MODAL_TYPES.CLONE], - [ARCHIVE_TESTID, MODAL_TYPES.ARCHIVE], -]; +const testIdActionPairs = [[EDIT_TESTID, MODAL_TYPES.EDIT]]; function setup({ canWrite = true, @@ -67,21 +57,8 @@ describe("QuestionActionButtons", () => { describe("when `canWrite` is falsy", () => { it("only renders the 'add to dashboard' and 'bookmark' buttons", () => { setup({ canWrite: false }); - const buttons = screen.getAllByRole("button"); - - screen.getByTestId(ADD_TO_DASH_TESTID); - expect(buttons.length).toBe(2); - }); - - it("should pass the correct action to the `onOpenModal` prop", () => { - const { onOpenModal } = setup({ canWrite: false }); - screen.getByTestId(ADD_TO_DASH_TESTID).click(); - expect(onOpenModal).toHaveBeenCalledWith(MODAL_TYPES.ADD_TO_DASHBOARD); - }); - - it("shouldn't show the control for turning question into model", () => { - setup({ canWrite: false }); - expect(screen.queryByLabelText("model icon")).not.toBeInTheDocument(); + const buttons = screen.queryAllByRole("button"); + expect(buttons.length).toBe(0); }); }); @@ -89,7 +66,7 @@ describe("QuestionActionButtons", () => { it("should show all buttons", () => { setup(); const buttons = screen.getAllByRole("button"); - expect(buttons.length).toBe(7); + expect(buttons.length).toBe(1); }); it("should pass the correct action to the `onOpenModal`", () => { @@ -102,38 +79,33 @@ describe("QuestionActionButtons", () => { onOpenModal.mockClear(); }); }); - - it("should show the control for turning question into model", () => { - setup(); - expect(screen.getByLabelText("model icon")).toBeInTheDocument(); - }); - }); - - describe("when database supports models", () => { - it("should show the control for turning question into model", () => { - setup(); - expect(screen.getByLabelText("model icon")).toBeInTheDocument(); - }); - }); - - describe("when database doesn't support models", () => { - it("shouldn't show the control for turning question into model", () => { - setup({ questionDatabaseSupportsModels: false }); - expect(screen.queryByLabelText("model icon")).not.toBeInTheDocument(); - }); }); - describe("when nested queries are disabled", () => { - it("shouldn't show the control for turning question into model", () => { - setup({ areNestedQueriesEnabled: false }); - expect(screen.queryByLabelText("model icon")).not.toBeInTheDocument(); - }); - }); - - describe("when displaying model actions", () => { - it("shouldn't show the control for turning question into model", () => { - setup({ isDataModel: true }); - expect(screen.queryByLabelText("model icon")).not.toBeInTheDocument(); - }); - }); + // describe("when database supports models", () => { + // it("should show the control for turning question into model", () => { + // setup(); + // expect(screen.getByLabelText("model icon")).toBeInTheDocument(); + // }); + // }); + + // describe("when database doesn't support models", () => { + // it("shouldn't show the control for turning question into model", () => { + // setup({ questionDatabaseSupportsModels: false }); + // expect(screen.queryByLabelText("model icon")).not.toBeInTheDocument(); + // }); + // }); + + // describe("when nested queries are disabled", () => { + // it("shouldn't show the control for turning question into model", () => { + // setup({ areNestedQueriesEnabled: false }); + // expect(screen.queryByLabelText("model icon")).not.toBeInTheDocument(); + // }); + // }); + + // describe("when displaying model actions", () => { + // it("shouldn't show the control for turning question into model", () => { + // setup({ isDataModel: true }); + // expect(screen.queryByLabelText("model icon")).not.toBeInTheDocument(); + // }); + // }); }); diff --git a/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js b/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js index cb24b795ccd..e8ba3e92a34 100644 --- a/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js +++ b/frontend/test/metabase/scenarios/models/helpers/e2e-models-helpers.js @@ -1,4 +1,4 @@ -import { popover, modal } from "__support__/e2e/cypress"; +import { popover, modal, openQuestionActions } from "__support__/e2e/cypress"; export function assertQuestionIsBasedOnModel({ questionName, @@ -63,12 +63,11 @@ export function getDetailsSidebarActions() { return cy.findByTestId("question-action-buttons"); } -// Requires model details sidebar to be open +// Requires model actions to be open export function assertIsModel() { - getDetailsSidebarActions().within(() => { + popover().within(() => { cy.icon("model").should("not.exist"); }); - cy.findByText("Model management"); cy.findByText("Sample Database").should("not.exist"); // For native @@ -76,18 +75,17 @@ export function assertIsModel() { cy.get("ace_content").should("not.exist"); } -// Requires question details sidebar to be open +// Requires question actions to be open export function assertIsQuestion() { - getDetailsSidebarActions().within(() => { + popover().within(() => { cy.icon("model"); }); - cy.findByText("Model management").should("not.exist"); cy.findByText("Sample Database"); } export function turnIntoModel() { - openDetailsSidebar(); - getDetailsSidebarActions().within(() => { + openQuestionActions(); + popover().within(() => { cy.icon("model").click(); }); modal().within(() => { diff --git a/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js b/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js index 4f1579b9811..06aca8f6ddc 100644 --- a/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js +++ b/frontend/test/metabase/scenarios/models/models-metadata.cy.spec.js @@ -4,6 +4,7 @@ import { visualize, visitDashboard, popover, + openQuestionActions, } from "__support__/e2e/cypress"; import { @@ -35,10 +36,12 @@ describe("scenarios > models metadata", () => { cy.visit("/model/1"); - openDetailsSidebar(); + openQuestionActions(); - sidebar().within(() => { - cy.findByTestId("tooltip-component-wrapper").realHover(); + popover().within(() => { + cy.findByTestId("tooltip-component-wrapper") + .parent() + .realHover(); cy.findByText("89%"); }); @@ -49,7 +52,7 @@ describe("scenarios > models metadata", () => { "Adding metadata makes it easier for your team to explore this data.", ); - cy.findByText("Customize metadata").click(); + cy.findByText("Edit metadata").click(); cy.wait(["@cardQuery", "@cardQuery"]); cy.url().should("include", "/metadata"); @@ -79,10 +82,12 @@ describe("scenarios > models metadata", () => { { visitQuestion: true }, ); - openDetailsSidebar(); + openQuestionActions(); - sidebar().within(() => { - cy.findByTestId("tooltip-component-wrapper").realHover(); + popover().within(() => { + cy.findByTestId("tooltip-component-wrapper") + .parent() + .realHover(); cy.findByText("37%"); }); @@ -93,7 +98,7 @@ describe("scenarios > models metadata", () => { "Adding metadata makes it easier for your team to explore this data.", ); - cy.findByText("Customize metadata").click(); + cy.findByText("Edit metadata").click(); cy.wait(["@cardQuery", "@cardQuery"]); cy.url().should("include", "/metadata"); @@ -138,8 +143,9 @@ describe("scenarios > models metadata", () => { // Revision 1 cy.findByText("Subtotal ($)"); cy.findByText("Tax ($)").should("not.exist"); - openDetailsSidebar(); - cy.findByText("Customize metadata").click(); + + openQuestionActions(); + cy.findByText("Edit metadata").click(); cy.wait(["@cardQuery", "@cardQuery"]); cy.findByTextEnsureVisible("TAX"); diff --git a/frontend/test/metabase/scenarios/models/models-query-editor.cy.spec.js b/frontend/test/metabase/scenarios/models/models-query-editor.cy.spec.js index bfff9119855..1918c26f267 100644 --- a/frontend/test/metabase/scenarios/models/models-query-editor.cy.spec.js +++ b/frontend/test/metabase/scenarios/models/models-query-editor.cy.spec.js @@ -1,9 +1,12 @@ -import { restore, runNativeQuery, summarize } from "__support__/e2e/cypress"; - import { - selectFromDropdown, - openDetailsSidebar, -} from "./helpers/e2e-models-helpers"; + restore, + runNativeQuery, + summarize, + popover, + openQuestionActions, +} from "__support__/e2e/cypress"; + +import { selectFromDropdown } from "./helpers/e2e-models-helpers"; describe("scenarios > models query editor", () => { beforeEach(() => { @@ -27,8 +30,12 @@ describe("scenarios > models query editor", () => { .should("contain", "37.65") .and("contain", "109.22"); - openDetailsSidebar(); - cy.findByText("Edit query definition").click(); + openQuestionActions(); + + popover().within(() => { + cy.findByText("Edit query definition").click(); + }); + cy.button("Save changes").should("be.disabled"); cy.findByText("Row limit").click(); @@ -87,8 +94,11 @@ describe("scenarios > models query editor", () => { .should("contain", "37.65") .and("contain", "109.22"); - openDetailsSidebar(); - cy.findByText("Edit query definition").click(); + openQuestionActions(); + + popover().within(() => { + cy.findByText("Edit query definition").click(); + }); cy.url().should("include", "/query"); cy.button("Save changes").should("be.disabled"); @@ -124,9 +134,11 @@ describe("scenarios > models query editor", () => { { visitQuestion: true }, ); - openDetailsSidebar(); + openQuestionActions(); - cy.findByText("Customize metadata").click(); + popover().within(() => { + cy.findByText("Edit metadata").click(); + }); cy.wait("@cardQuery"); cy.findByText(/Syntax error in SQL/).should("be.visible"); diff --git a/frontend/test/metabase/scenarios/models/models-revision-history.cy.spec.js b/frontend/test/metabase/scenarios/models/models-revision-history.cy.spec.js index 5c53b32b2bb..d37e9d2e2aa 100644 --- a/frontend/test/metabase/scenarios/models/models-revision-history.cy.spec.js +++ b/frontend/test/metabase/scenarios/models/models-revision-history.cy.spec.js @@ -1,4 +1,10 @@ -import { restore, modal, filter, visitQuestion } from "__support__/e2e/cypress"; +import { + restore, + modal, + filter, + visitQuestion, + openQuestionActions, +} from "__support__/e2e/cypress"; import { assertIsModel, @@ -28,12 +34,14 @@ describe("scenarios > models > revision history", () => { it("should allow reverting to a saved question state", () => { cy.visit("/model/3"); openDetailsSidebar(); + openQuestionActions(); assertIsModel(); cy.findByText("History").click(); cy.button("Revert").click(); cy.wait("@revertToRevision"); + openQuestionActions(); assertIsQuestion(); cy.get(".LineAreaBarChart"); @@ -54,6 +62,7 @@ describe("scenarios > models > revision history", () => { visitQuestion(3); openDetailsSidebar(); + openQuestionActions(); assertIsQuestion(); cy.findByText("History").click(); @@ -64,6 +73,7 @@ describe("scenarios > models > revision history", () => { }); cy.wait("@revertToRevision"); + openQuestionActions(); assertIsModel(); cy.get(".LineAreaBarChart").should("not.exist"); diff --git a/frontend/test/metabase/scenarios/models/models.cy.spec.js b/frontend/test/metabase/scenarios/models/models.cy.spec.js index e20a3104ebe..d49c277419e 100644 --- a/frontend/test/metabase/scenarios/models/models.cy.spec.js +++ b/frontend/test/metabase/scenarios/models/models.cy.spec.js @@ -13,6 +13,8 @@ import { visitQuestion, visitDashboard, startNewQuestion, + openQuestionActions, + closeQuestionActions, } from "__support__/e2e/cypress"; import { SAMPLE_DATABASE } from "__support__/e2e/cypress_sample_database"; @@ -43,6 +45,7 @@ describe("scenarios > models", () => { visitQuestion(1); turnIntoModel(); + openQuestionActions(); assertIsModel(); filter(); @@ -92,6 +95,7 @@ describe("scenarios > models", () => { ); turnIntoModel(); + openQuestionActions(); assertIsModel(); filter(); @@ -150,6 +154,7 @@ describe("scenarios > models", () => { cy.findByText("Undo").click(); cy.get(".LineAreaBarChart"); + openQuestionActions(); assertIsQuestion(); }); @@ -159,14 +164,20 @@ describe("scenarios > models", () => { cy.visit("/model/1"); openDetailsSidebar(); - cy.findByText("Turn back into a saved question").click(); + openQuestionActions(); + popover().within(() => { + cy.findByText("Turn back to saved question").click(); + }); + cy.wait("@cardUpdate"); cy.findByText("This is a question now."); + openQuestionActions(); assertIsQuestion(); cy.findByText("Undo").click(); cy.wait("@cardUpdate"); + openQuestionActions(); assertIsModel(); }); @@ -180,7 +191,7 @@ describe("scenarios > models", () => { // Important - do not use visitQuestion(1) here! cy.visit("/question/1"); cy.wait("@dataset"); - openDetailsSidebar(); + openQuestionActions(); assertIsModel(); cy.url().should("include", "/model"); }); @@ -485,7 +496,8 @@ describe("scenarios > models", () => { ); openDetailsSidebar(); - getDetailsSidebarActions().within(() => { + openQuestionActions(); + popover().within(() => { cy.icon("model").click(); }); modal().within(() => { @@ -493,7 +505,9 @@ describe("scenarios > models", () => { cy.button("Turn this into a model").should("not.exist"); cy.icon("close").click(); }); + openQuestionActions(); assertIsQuestion(); + closeQuestionActions(); cy.findByText(/Open editor/i).click(); cy.get(".ace_content").type( @@ -509,6 +523,7 @@ describe("scenarios > models", () => { .click(); turnIntoModel(); + openQuestionActions(); assertIsModel(); }); diff --git a/frontend/test/metabase/scenarios/models/reproductions/20963-can-not-convert-question-with-snippets-to-model.cy.spec.js b/frontend/test/metabase/scenarios/models/reproductions/20963-can-not-convert-question-with-snippets-to-model.cy.spec.js index 084a75683ea..ccd062e6ea8 100644 --- a/frontend/test/metabase/scenarios/models/reproductions/20963-can-not-convert-question-with-snippets-to-model.cy.spec.js +++ b/frontend/test/metabase/scenarios/models/reproductions/20963-can-not-convert-question-with-snippets-to-model.cy.spec.js @@ -1,8 +1,10 @@ -import { restore, modal, openNativeEditor } from "__support__/e2e/cypress"; import { - openDetailsSidebar, - getDetailsSidebarActions, -} from "../helpers/e2e-models-helpers"; + restore, + modal, + openNativeEditor, + popover, + openQuestionActions, +} from "__support__/e2e/cypress"; const snippetName = `string 'test'`; const questionName = "Converting questions with snippets to models"; @@ -45,8 +47,8 @@ describe("issue 20963", () => { cy.findByText("Not now").click(); // Convert into to a model - openDetailsSidebar(); - getDetailsSidebarActions().within(() => { + openQuestionActions(); + popover().within(() => { cy.icon("model").click(); }); diff --git a/frontend/test/metabase/scenarios/organization/bookmarks-question.cy.spec.js b/frontend/test/metabase/scenarios/organization/bookmarks-question.cy.spec.js index 69fad1eb593..bbaf427d0a9 100644 --- a/frontend/test/metabase/scenarios/organization/bookmarks-question.cy.spec.js +++ b/frontend/test/metabase/scenarios/organization/bookmarks-question.cy.spec.js @@ -2,7 +2,6 @@ import { restore, navigationSidebar, openNavigationSidebar, - sidebar, visitQuestion, } from "__support__/e2e/cypress"; import { getSidebarSectionTitle as getSectionTitle } from "__support__/e2e/helpers/e2e-collection-helpers"; @@ -16,7 +15,6 @@ describe("scenarios > question > bookmarks", () => { it("should add then remove bookmark from question page", () => { visitQuestion(1); - cy.findByTestId("saved-question-header-button").click(); toggleBookmark(); openNavigationSidebar(); @@ -36,7 +34,7 @@ describe("scenarios > question > bookmarks", () => { }); function toggleBookmark() { - sidebar().within(() => { + cy.findByTestId("question-action-buttons-container").within(() => { cy.icon("bookmark").click(); }); cy.wait("@toggleBookmark"); diff --git a/frontend/test/metabase/scenarios/organization/moderation-question.cy.spec.js b/frontend/test/metabase/scenarios/organization/moderation-question.cy.spec.js index b92be0def27..975aed0593c 100644 --- a/frontend/test/metabase/scenarios/organization/moderation-question.cy.spec.js +++ b/frontend/test/metabase/scenarios/organization/moderation-question.cy.spec.js @@ -1,4 +1,10 @@ -import { describeEE, restore, visitQuestion } from "__support__/e2e/cypress"; +import { + describeEE, + restore, + visitQuestion, + popover, + openQuestionActions, +} from "__support__/e2e/cypress"; describeEE("scenarios > saved question moderation", () => { describe("as an admin", () => { @@ -10,11 +16,12 @@ describeEE("scenarios > saved question moderation", () => { }); it("should be able to verify a saved question", () => { - cy.findByTestId("saved-question-header-button").click(); - - cy.findByTestId("moderation-verify-action").click(); + openQuestionActions(); - cy.findAllByText("You verified this"); + popover().within(() => { + cy.findByTestId("moderation-verify-action").click(); + cy.findByText("Remove verification"); + }); cy.findByPlaceholderText("Search…").type("orders{enter}"); cy.findByText("Orders, Count").icon("verified"); @@ -25,15 +32,18 @@ describeEE("scenarios > saved question moderation", () => { }); it("should be able to unverify a verified saved question", () => { - cy.findByTestId("saved-question-header-button").click(); + openQuestionActions(); - cy.findByTestId("moderation-verify-action").click(); - cy.findByTestId("moderation-remove-review-action").click(); + popover().within(() => { + cy.findByTestId("moderation-verify-action").click(); + cy.findByTestId("moderation-remove-verification-action").click(); + }); - cy.findByText("You verified this").should("not.be.visible"); - cy.findByTestId("saved-question-header-button").click(); + cy.findByText("Verify this question").should("be.visible"); - cy.icon("verified").should("not.exist"); + cy.findByTestId("saved-question-header-button").within(() => { + cy.icon("verified").should("not.exist"); + }); cy.findByPlaceholderText("Search…").type("orders{enter}"); cy.findByText("Orders, Count") @@ -48,13 +58,23 @@ describeEE("scenarios > saved question moderation", () => { }); it("should be able to see evidence of verification/unverification in the question's timeline", () => { + openQuestionActions(); + + popover().within(() => { + cy.findByTestId("moderation-verify-action").click(); + }); + cy.findByTestId("saved-question-header-button").click(); cy.findByText("History").click(); - cy.findByTestId("moderation-verify-action").click(); cy.findAllByText("You verified this").should("be.visible"); - cy.findByTestId("moderation-remove-review-action").click(); + openQuestionActions(); + + popover().within(() => { + cy.findByTestId("moderation-remove-verification-action").click(); + }); + cy.findByText("You removed verification").should("be.visible"); }); }); diff --git a/frontend/test/metabase/scenarios/question/question-management.cy.spec.js b/frontend/test/metabase/scenarios/question/question-management.cy.spec.js index 42c21d83a0a..c2d29ae3197 100644 --- a/frontend/test/metabase/scenarios/question/question-management.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/question-management.cy.spec.js @@ -1,4 +1,10 @@ -import { restore, visitQuestion, saveDashboard } from "__support__/e2e/cypress"; +import { + restore, + visitQuestion, + saveDashboard, + popover, + openQuestionActions, +} from "__support__/e2e/cypress"; import { onlyOn } from "@cypress/skip-test"; @@ -61,6 +67,7 @@ describe("managing question from the question's details sidebar", () => { it("should be able to move the question (metabase#11719-2)", () => { // cy.skipOn(user === "nodata"); + openQuestionActions(); cy.findByTestId("move-button").click(); cy.findByText("My personal collection").click(); clickButton("Move"); @@ -72,6 +79,7 @@ describe("managing question from the question's details sidebar", () => { cy.intercept("GET", "/api/collection/root/items**").as( "getItems", ); + openQuestionActions(); cy.findByTestId("archive-button").click(); clickButton("Archive"); assertOnRequest("updateQuestion"); @@ -82,6 +90,7 @@ describe("managing question from the question's details sidebar", () => { }); it("should be able to add question to dashboard", () => { + openQuestionActions(); cy.findByTestId("add-to-dashboard-button").click(); cy.get(".Modal") @@ -107,6 +116,7 @@ describe("managing question from the question's details sidebar", () => { }); it("should not be offered to add question to dashboard inside a collection they have `read` access to", () => { + openQuestionActions(); cy.findByTestId("add-to-dashboard-button").click(); cy.get(".Modal").within(() => { @@ -122,6 +132,7 @@ describe("managing question from the question's details sidebar", () => { it("should offer personal collection as a save destination for a new dashboard", () => { const { first_name, last_name } = USERS[user]; const personalCollection = `${first_name} ${last_name}'s Personal Collection`; + openQuestionActions(); cy.findByTestId("add-to-dashboard-button").click(); cy.get(".Modal").within(() => { @@ -141,9 +152,13 @@ describe("managing question from the question's details sidebar", () => { "not.exist", ); - cy.findByTestId("move-button").should("not.exist"); - cy.findByTestId("clone-button").should("not.exist"); - cy.findByTestId("archive-button").should("not.exist"); + openQuestionActions(); + + popover().within(() => { + cy.findByTestId("move-button").should("not.exist"); + cy.findByTestId("clone-button").should("not.exist"); + cy.findByTestId("archive-button").should("not.exist"); + }); cy.findByText("Revert").should("not.exist"); }); diff --git a/frontend/test/metabase/scenarios/question/saved.cy.spec.js b/frontend/test/metabase/scenarios/question/saved.cy.spec.js index 531510e53a5..40948dad8ce 100644 --- a/frontend/test/metabase/scenarios/question/saved.cy.spec.js +++ b/frontend/test/metabase/scenarios/question/saved.cy.spec.js @@ -7,6 +7,7 @@ import { visitQuestion, startNewQuestion, visualize, + openQuestionActions, } from "__support__/e2e/cypress"; describe("scenarios > question > saved", () => { @@ -104,8 +105,10 @@ describe("scenarios > question > saved", () => { visitQuestion(1); cy.wait("@query"); - cy.findByTestId("saved-question-header-button").click(); - cy.icon("segment").click(); + openQuestionActions(); + popover().within(() => { + cy.icon("segment").click(); + }); modal().within(() => { cy.findByLabelText("Name").should("have.value", "Orders - Duplicate"); -- GitLab