diff --git a/css/styles.css b/css/styles.css index 62d3064a5..6cf77fef0 100644 --- a/css/styles.css +++ b/css/styles.css @@ -197,6 +197,22 @@ table.record-list td.load-more { text-decoration: none; } +.card-body > .spinner { + position: absolute; + left: 0; + right: 0; + top: 0; + bottom: 0; + background-color: rgba(0, 0, 0, 0.2); + z-index: 1; + display: flex; + justify-content: center; + align-items: center; + width: auto; + margin: 0; + border-radius: 3px; +} + .load-more .spinner { margin-top: 0.25em; } diff --git a/package-lock.json b/package-lock.json index 71b66a5ab..089a71689 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,6 +15,7 @@ "@rjsf/bootstrap-4": "^5.13.2", "@rjsf/core": "^5.13.2", "@rjsf/validator-ajv8": "^5.13.2", + "@testing-library/react-hooks": "^8.0.1", "atob": "^2.0.3", "bootstrap": "^4.6.2", "bootstrap-icons": "^1.11.1", @@ -4033,6 +4034,35 @@ "react-dom": "<18.0.0" } }, + "node_modules/@testing-library/react-hooks": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/@testing-library/react-hooks/-/react-hooks-8.0.1.tgz", + "integrity": "sha512-Aqhl2IVmLt8IovEVarNDFuJDVWVvhnr9/GCU6UUnrYXwgDFF9h2L2o2P9KBni1AST5sT6riAyoukFLyjQUgD/g==", + "dependencies": { + "@babel/runtime": "^7.12.5", + "react-error-boundary": "^3.1.0" + }, + "engines": { + "node": ">=12" + }, + "peerDependencies": { + "@types/react": "^16.9.0 || ^17.0.0", + "react": "^16.9.0 || ^17.0.0", + "react-dom": "^16.9.0 || ^17.0.0", + "react-test-renderer": "^16.9.0 || ^17.0.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "react-dom": { + "optional": true + }, + "react-test-renderer": { + "optional": true + } + } + }, "node_modules/@tootallnate/once": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@tootallnate/once/-/once-2.0.0.tgz", @@ -13301,6 +13331,21 @@ "object-assign": "^4.1.1" } }, + "node_modules/react-error-boundary": { + "version": "3.1.4", + "resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-3.1.4.tgz", + "integrity": "sha512-uM9uPzZJTF6wRQORmSrvOIgt4lJ9MC1sNgEOj2XGsDTRE4kmpWxg7ENK9EWNKJRMAOY9z0MuF4yIfl6gp4sotA==", + "dependencies": { + "@babel/runtime": "^7.12.5" + }, + "engines": { + "node": ">=10", + "npm": ">=6" + }, + "peerDependencies": { + "react": ">=16.13.1" + } + }, "node_modules/react-is": { "version": "16.13.1", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz", @@ -18976,6 +19021,15 @@ "@types/react-dom": "<18.0.0" } }, + "@testing-library/react-hooks": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/@testing-library/react-hooks/-/react-hooks-8.0.1.tgz", + "integrity": "sha512-Aqhl2IVmLt8IovEVarNDFuJDVWVvhnr9/GCU6UUnrYXwgDFF9h2L2o2P9KBni1AST5sT6riAyoukFLyjQUgD/g==", + "requires": { + "@babel/runtime": "^7.12.5", + "react-error-boundary": "^3.1.0" + } + }, "@tootallnate/once": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/@tootallnate/once/-/once-2.0.0.tgz", @@ -25930,6 +25984,14 @@ } } }, + "react-error-boundary": { + "version": "3.1.4", + "resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-3.1.4.tgz", + "integrity": "sha512-uM9uPzZJTF6wRQORmSrvOIgt4lJ9MC1sNgEOj2XGsDTRE4kmpWxg7ENK9EWNKJRMAOY9z0MuF4yIfl6gp4sotA==", + "requires": { + "@babel/runtime": "^7.12.5" + } + }, "react-is": { "version": "16.13.1", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz", diff --git a/package.json b/package.json index a93916f7f..0ace0d416 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "kinto-admin", - "version": "2.0.0", + "version": "2.1.0", "description": "Kinto Web Administration Console in React.js", "scripts": { "build": "rimraf build && cross-env NODE_ENV=production webpack --progress --config webpack.prod.js", @@ -27,6 +27,7 @@ "@rjsf/bootstrap-4": "^5.13.2", "@rjsf/core": "^5.13.2", "@rjsf/validator-ajv8": "^5.13.2", + "@testing-library/react-hooks": "^8.0.1", "atob": "^2.0.3", "bootstrap": "^4.6.2", "bootstrap-icons": "^1.11.1", diff --git a/src/components/HomePage.tsx b/src/components/HomePage.tsx index 3d700c706..1b413686c 100644 --- a/src/components/HomePage.tsx +++ b/src/components/HomePage.tsx @@ -9,7 +9,7 @@ import Spinner from "./Spinner"; import AuthForm from "./AuthForm"; import { getServerByPriority, isObject } from "../utils"; import { loadSession } from "../store/localStore"; -import { useAppDispatch, useAppSelector } from "../hooks"; +import { useAppDispatch, useAppSelector } from "../hooks/app"; import { useParams } from "react-router"; function ServerProps({ node }: { node: any }) { diff --git a/src/components/Layout.tsx b/src/components/Layout.tsx index 948c9d417..0755f41d2 100644 --- a/src/components/Layout.tsx +++ b/src/components/Layout.tsx @@ -30,7 +30,7 @@ import { Sidebar } from "../components/Sidebar"; import Notifications from "../containers/Notifications"; import { SessionInfoBar } from "./SessionInfoBar"; -import { useAppSelector } from "../hooks"; +import { useAppSelector } from "../hooks/app"; export function Layout() { const authenticated = useAppSelector(store => store.session.authenticated); diff --git a/src/components/PermissionsForm.tsx b/src/components/PermissionsForm.tsx index b96b26eeb..d5d204851 100644 --- a/src/components/PermissionsForm.tsx +++ b/src/components/PermissionsForm.tsx @@ -9,7 +9,7 @@ import { preparePermissionsForm, } from "../permission"; import { useParams } from "react-router"; -import { useAppSelector } from "../hooks"; +import { useAppSelector } from "../hooks/app"; import { RJSFSchema } from "@rjsf/utils"; diff --git a/src/components/SessionInfoBar.tsx b/src/components/SessionInfoBar.tsx index d3dccf2cb..2d8d5706f 100644 --- a/src/components/SessionInfoBar.tsx +++ b/src/components/SessionInfoBar.tsx @@ -3,7 +3,7 @@ import { BoxArrowRight } from "react-bootstrap-icons"; import { QuestionCircleFill } from "react-bootstrap-icons"; import { Clipboard } from "react-bootstrap-icons"; -import { useAppSelector, useAppDispatch } from "../hooks"; +import { useAppSelector, useAppDispatch } from "../hooks/app"; import * as SessionActions from "../actions/session"; export function SessionInfoBar() { diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index 5fdbc5fdf..c52bce712 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -19,7 +19,7 @@ import AdminLink from "./AdminLink"; import url from "../url"; import { canCreateBucket } from "../permission"; import { SIDEBAR_MAX_LISTED_COLLECTIONS } from "../constants"; -import { useAppSelector, useAppDispatch } from "../hooks"; +import { useAppSelector, useAppDispatch } from "../hooks/app"; type SideBarLinkProps = { currentPath: string; diff --git a/src/components/bucket/BucketForm.tsx b/src/components/bucket/BucketForm.tsx index f06313b79..ddf5fe764 100644 --- a/src/components/bucket/BucketForm.tsx +++ b/src/components/bucket/BucketForm.tsx @@ -53,7 +53,7 @@ export default function BucketForm({ onSubmit, }: Props) { const creation = !formData.id; - const hasWriteAccess = canEditBucket(session, bucket); + const hasWriteAccess = canEditBucket(session, bid); const formIsEditable = creation || hasWriteAccess; const showDeleteForm = !creation && hasWriteAccess; diff --git a/src/components/bucket/BucketPermissions.tsx b/src/components/bucket/BucketPermissions.tsx index 7bb301f45..d181af312 100644 --- a/src/components/bucket/BucketPermissions.tsx +++ b/src/components/bucket/BucketPermissions.tsx @@ -9,7 +9,7 @@ import { PermissionsForm } from "../PermissionsForm"; import { canEditBucket } from "../../permission"; import { useParams } from "react-router"; -import { useAppSelector, useAppDispatch } from "../../hooks"; +import { useAppSelector, useAppDispatch } from "../../hooks/app"; export function BucketPermissions() { const dispatch = useAppDispatch(); @@ -38,7 +38,7 @@ export function BucketPermissions() { diff --git a/src/components/bucket/CollectionDataList.tsx b/src/components/bucket/CollectionDataList.tsx index dfac3b1ef..b53344e66 100644 --- a/src/components/bucket/CollectionDataList.tsx +++ b/src/components/bucket/CollectionDataList.tsx @@ -9,7 +9,7 @@ import { canCreateCollection } from "../../permission"; export function ListActions(props) { const { bid, session, bucket } = props; - if (session.busy || bucket.busy || !canCreateCollection(session, bucket)) { + if (session.busy || bucket.busy || !canCreateCollection(session, bid)) { return null; } return ( diff --git a/src/components/bucket/GroupDataList.tsx b/src/components/bucket/GroupDataList.tsx index d02422cef..9c6a017fa 100644 --- a/src/components/bucket/GroupDataList.tsx +++ b/src/components/bucket/GroupDataList.tsx @@ -67,7 +67,7 @@ export function DataList(props) { } export function ListActions({ bid, session, bucket }) { - if (session.busy || bucket.busy || !canCreateGroup(session, bucket)) { + if (session.busy || bucket.busy || !canCreateGroup(session, bid)) { return null; } return ( diff --git a/src/components/collection/CollectionForm.tsx b/src/components/collection/CollectionForm.tsx index 07890d577..3e109f016 100644 --- a/src/components/collection/CollectionForm.tsx +++ b/src/components/collection/CollectionForm.tsx @@ -247,8 +247,8 @@ export default function CollectionForm({ }: Props) { const [asJSON, setAsJSON] = useState(false); const allowEditing = propFormData - ? canEditCollection(session, bucket, collection) - : canCreateCollection(session, bucket); + ? canEditCollection(session, bucket?.data?.id, collection) + : canCreateCollection(session, bucket?.data?.id); const toggleJSON = event => { event.preventDefault(); diff --git a/src/components/collection/CollectionPermissions.tsx b/src/components/collection/CollectionPermissions.tsx index 2c9e7179c..92a43e03c 100644 --- a/src/components/collection/CollectionPermissions.tsx +++ b/src/components/collection/CollectionPermissions.tsx @@ -8,7 +8,7 @@ import CollectionTabs from "./CollectionTabs"; import { PermissionsForm } from "../PermissionsForm"; import { canEditCollection } from "../../permission"; import { useParams } from "react-router"; -import { useAppSelector, useAppDispatch } from "../../hooks"; +import { useAppSelector, useAppDispatch } from "../../hooks/app"; interface RouteParams { bid: string; cid: string; @@ -17,7 +17,6 @@ interface RouteParams { export function CollectionPermissions() { const { bid, cid } = useParams(); const session = useAppSelector(state => state.session); - const bucket = useAppSelector(state => state.bucket); const collection = useAppSelector(state => state.collection); const { busy, permissions } = collection; const acls = ["read", "write", "record:create"]; @@ -50,7 +49,7 @@ export function CollectionPermissions() { diff --git a/src/components/collection/CollectionRecords.tsx b/src/components/collection/CollectionRecords.tsx index 7581e192a..610381ef4 100644 --- a/src/components/collection/CollectionRecords.tsx +++ b/src/components/collection/CollectionRecords.tsx @@ -15,6 +15,9 @@ import { CommonProps, CommonStateProps } from "./commonPropTypes"; import * as CollectionActions from "../../actions/collection"; import * as RouteActions from "../../actions/route"; +import AdminLink from "../AdminLink"; +import { storageKeys, useLocalStorage } from "../../hooks/storage"; +import { Shuffle } from "react-bootstrap-icons"; type OwnProps = { match: CollectionRouteMatch; @@ -71,6 +74,10 @@ export default function CollectionRecords(props: Props) { } = collection; const { schema, displayFields } = data; + const [useSimpleReview, setUseSimpleReview] = useLocalStorage( + storageKeys.useSimpleReview, + true + ); useEffect(() => { if (!session.authenticated || !data?.last_modified) { return; @@ -127,6 +134,22 @@ export default function CollectionRecords(props: Props) { /> )} {listActions} + {capabilities.signer && !useSimpleReview && ( + { + setUseSimpleReview(true); + }} + style={{ + float: "right", + marginTop: "1.5em", + }} + > + Switch to Default Review UI + + )} ); diff --git a/src/components/collection/CollectionTabs.tsx b/src/components/collection/CollectionTabs.tsx index 688849708..858d4cf5f 100644 --- a/src/components/collection/CollectionTabs.tsx +++ b/src/components/collection/CollectionTabs.tsx @@ -1,13 +1,25 @@ import React from "react"; -import { Gear, Lock, Justify, ClockHistory } from "react-bootstrap-icons"; +import { + Gear, + Lock, + Justify, + ClockHistory, + Braces, +} from "react-bootstrap-icons"; import AdminLink from "../AdminLink"; import type { Capabilities } from "../../types"; +import { storageKeys, useLocalStorage } from "../../hooks/storage"; type Props = { bid: string; cid: string; - selected: "records" | "attributes" | "permissions" | "history"; + selected: + | "records" + | "attributes" + | "permissions" + | "history" + | "simple-review"; capabilities: Capabilities; children?: React.ReactNode; totalRecords?: number | null; @@ -21,6 +33,8 @@ export default function CollectionTabs({ children, totalRecords, }: Props) { + const [useSimpleReview] = useLocalStorage(storageKeys.useSimpleReview, true); + return (
@@ -37,6 +51,20 @@ export default function CollectionTabs({ Records {totalRecords ? `(${totalRecords})` : null} + {capabilities.signer && useSimpleReview && ( +
  • + + + Review + +
  • + )}
  • - {canCreateRecord(session, bucket, collection) && ( + {canCreateRecord(session, bid, collection) && ( <> state.bucket); const group = useAppSelector(state => state.group); const { busy, permissions } = group; const session = useAppSelector(state => state.session); @@ -45,7 +44,7 @@ export function GroupPermissions() { diff --git a/src/components/record/RecordForm.tsx b/src/components/record/RecordForm.tsx index b7a0adabb..4618327f5 100644 --- a/src/components/record/RecordForm.tsx +++ b/src/components/record/RecordForm.tsx @@ -1,7 +1,6 @@ import React, { useState } from "react"; import type { SessionState, - BucketState, CollectionState, RecordState, Capabilities, @@ -52,7 +51,6 @@ type Props = { cid: string; rid?: string; session: SessionState; - bucket: BucketState; collection: CollectionState; record?: RecordState; deleteRecord?: typeof CollectionActions.deleteRecord; @@ -68,7 +66,6 @@ export default function RecordForm(props: Props) { bid, cid, session, - bucket, collection, record, deleteRecord, @@ -77,8 +74,8 @@ export default function RecordForm(props: Props) { } = props; const allowEditing = record - ? canEditRecord(session, bucket, collection, record) - : canCreateRecord(session, bucket, collection); + ? canEditRecord(session, bid, collection, record) + : canCreateRecord(session, bid, collection); const handleDeleteRecord = () => { const { rid } = props; diff --git a/src/components/record/RecordPermissions.tsx b/src/components/record/RecordPermissions.tsx index ca115d13e..fc14d6e7a 100644 --- a/src/components/record/RecordPermissions.tsx +++ b/src/components/record/RecordPermissions.tsx @@ -6,7 +6,7 @@ import RecordTabs from "./RecordTabs"; import { PermissionsForm } from "../PermissionsForm"; import { canEditRecord } from "../../permission"; import { useParams } from "react-router"; -import { useAppSelector, useAppDispatch } from "../../hooks"; +import { useAppSelector, useAppDispatch } from "../../hooks/app"; interface RouteParams { bid: string; @@ -14,7 +14,6 @@ interface RouteParams { rid: string; } export function RecordPermissions() { - const bucket = useAppSelector(state => state.bucket); const session = useAppSelector(state => state.session); const collection = useAppSelector(state => state.collection); const record = useAppSelector(state => state.record); @@ -50,7 +49,7 @@ export function RecordPermissions() { diff --git a/src/components/signoff/Review.tsx b/src/components/signoff/Review.tsx index 44d1eb5bf..490adb522 100644 --- a/src/components/signoff/Review.tsx +++ b/src/components/signoff/Review.tsx @@ -3,12 +3,12 @@ import type { SignoffSourceInfo, PreviewInfo, ChangesList } from "../../types"; import React from "react"; import { Comment } from "./Comment"; -import { ChatLeft } from "react-bootstrap-icons"; -import { Check2 } from "react-bootstrap-icons"; +import { Braces, ChatLeft, Check2 } from "react-bootstrap-icons"; import AdminLink from "../AdminLink"; import { ProgressStep } from "./ProgressBar"; import HumanDate from "./HumanDate"; +import { storageKeys, useLocalStorage } from "../../hooks/storage"; type ReviewProps = { label: string; @@ -24,20 +24,20 @@ type ReviewProps = { changes: ChangesList | null; }; -export function Review(props: ReviewProps) { - const { - label, - canEdit, - hasHistory, - currentStep, - step, - isCurrentUrl, - approveChanges, - confirmDeclineChanges, - source, - preview, - changes, - } = props; +export function Review({ + label, + canEdit, + hasHistory, + currentStep, + step, + isCurrentUrl, + approveChanges, + confirmDeclineChanges, + source, + preview, + changes, +}: ReviewProps) { + const [useSimpleReview] = useLocalStorage(storageKeys.useSimpleReview, true); const isCurrentStep = step == currentStep; // If preview disabled, the preview object is empty. @@ -63,12 +63,21 @@ export function Review(props: ReviewProps) { changes={changes} /> )} - {isCurrentStep && canEdit && ( + {isCurrentStep && canEdit && !useSimpleReview && ( )} + {isCurrentStep && canEdit && useSimpleReview && ( + + Review Changes + + )} ); } diff --git a/src/components/signoff/SignoffToolBar.tsx b/src/components/signoff/SignoffToolBar.tsx index 0fafe6fa5..17d2c4071 100644 --- a/src/components/signoff/SignoffToolBar.tsx +++ b/src/components/signoff/SignoffToolBar.tsx @@ -19,24 +19,7 @@ import HumanDate from "./HumanDate"; import { Signed } from "./Signed"; import { Review, DiffInfo } from "./Review"; import Spinner from "../Spinner"; - -function isMember(groupKey, source, sessionState) { - const { - serverInfo: { user = {}, capabilities }, - } = sessionState; - if (!source || !user.principals) { - return false; - } - const { principals } = user; - const { bid, cid } = source; - const { signer = {} } = capabilities; - const { [groupKey]: defaultGroupName } = signer; - const { [groupKey]: groupName = defaultGroupName } = source; - const expectedGroup = groupName.replace("{collection_id}", cid); - const expectedPrincipal = `/buckets/${bid}/groups/${expectedGroup}`; - - return principals.includes(expectedPrincipal); -} +import { isMember } from "./utils"; function isEditor(source, sessionState) { return isMember("editors_group", source, sessionState); @@ -99,12 +82,12 @@ export default function SignoffToolBar({ } }, [collectionState.data]); - const canEdit = canEditCollection(sessionState, bucketState, collectionState); - const { data: { id: bid }, } = bucketState; + const canEdit = canEditCollection(sessionState, bid, collectionState); + const { data: { id: cid }, } = collectionState; @@ -187,7 +170,6 @@ export default function SignoffToolBar({ {canRollback && status != "signed" && ( )} - {pendingConfirmReviewRequest && ( void }) { function RollbackChangesButton(props: { onClick: () => void }) { const { onClick } = props; return ( - ); diff --git a/src/components/signoff/SimpleReview/PerRecordDiffView.tsx b/src/components/signoff/SimpleReview/PerRecordDiffView.tsx index c617947c6..d4279287c 100644 --- a/src/components/signoff/SimpleReview/PerRecordDiffView.tsx +++ b/src/components/signoff/SimpleReview/PerRecordDiffView.tsx @@ -76,10 +76,10 @@ export default function PerRecordDiffView({ ); default: return ( - <> +
    No changes to review, collection status is{" "} {collectionData.status}. - +
    ); } } @@ -179,25 +179,30 @@ export function findChangeTypes( let result: Array = []; - diffArrays( + const differences = diffArrays( oldItems.map(r => r.id).sort(), newItems.map(r => r.id).sort() - ).forEach(part => { - const id = part.value[0]; - if (part.added) { - result.push({ - id, - target: newItems.find(r => r.id === id), - changeType: ChangeType.ADD, - }); - } else if (part.removed) { - result.push({ - id, - source: oldItems.find(r => r.id === id), - changeType: ChangeType.REMOVE, - }); + ); + + for (let d of differences) { + if (d.added) { + for (let id of d.value) { + result.push({ + id, + target: newItems.find(r => r.id === id), + changeType: ChangeType.ADD, + }); + } + } else if (d.removed) { + for (let id of d.value) { + result.push({ + id, + source: oldItems.find(r => r.id === id), + changeType: ChangeType.REMOVE, + }); + } } - }); + } newItems.forEach(item => { const matchingOldItem = oldItems.find(r => r.id === item.id); diff --git a/src/components/signoff/SimpleReview/SimpleReviewButtons.tsx b/src/components/signoff/SimpleReview/SimpleReviewButtons.tsx index 936d3c599..753e75d96 100644 --- a/src/components/signoff/SimpleReview/SimpleReviewButtons.tsx +++ b/src/components/signoff/SimpleReview/SimpleReviewButtons.tsx @@ -4,56 +4,85 @@ import { SignoffCollectionStatus } from "../../../types"; import { useLocation } from "react-router"; import { parseSearchString } from "../../../locationUtils"; +import { ChatLeft, Check2, XCircleFill } from "react-bootstrap-icons"; +import Spinner from "../../Spinner"; export interface SimpleReviewButtonsProps { status: SignoffCollectionStatus; + canRequestReview: boolean; + canReview: boolean; approveChanges(): void; declineChanges(text: string): void; + requestReview(text: string): void; rollbackChanges(text: string): void; } export default function SimpleReviewButtons({ status, + canRequestReview, + canReview, approveChanges, declineChanges, + requestReview, rollbackChanges, }: SimpleReviewButtonsProps) { - const [dialog, setDialog] = useState<"reject" | "rollback" | "">(""); + const [dialog, setDialog] = useState<"reject" | "rollback" | "review" | "">( + "" + ); const searchParams = parseSearchString(useLocation().search); + const [showSpinner, setShowSpinner] = useState(false); return ( <>
    - {status === "to-review" && ( + {status === "to-review" && canReview && ( <> {" "} )} - {status === "work-in-progress" && !searchParams.hideRollback && ( + {status === "work-in-progress" && canRequestReview && ( - )} + )}{" "} + {canRequestReview && + ["work-in-progress", "to-review"].includes(status) && + !searchParams.hideRollback && ( + + )}
    {dialog === "reject" && ( { + setShowSpinner(true); + declineChanges(val); + }} onClose={() => setDialog("")} /> )} @@ -61,10 +90,25 @@ export default function SimpleReviewButtons({ { + setShowSpinner(true); + rollbackChanges(val); + }} + onClose={() => setDialog("")} + /> + )} + {dialog === "review" && ( + { + setShowSpinner(true); + requestReview(val); + }} onClose={() => setDialog("")} /> )} + {showSpinner && } ); } diff --git a/src/components/signoff/SimpleReview/SimpleReviewHeader.tsx b/src/components/signoff/SimpleReview/SimpleReviewHeader.tsx index f730e6837..4cdeb0ee6 100644 --- a/src/components/signoff/SimpleReview/SimpleReviewHeader.tsx +++ b/src/components/signoff/SimpleReview/SimpleReviewHeader.tsx @@ -7,6 +7,8 @@ export default function SimpleReviewHeader({ lastReviewRequestBy, lastReviewerComment, children, + lastEditDate, + lastReviewDate, }: SignoffSourceInfo & { children?: React.ReactElement }) { return (
    @@ -20,7 +22,9 @@ export default function SimpleReviewHeader({ {status === "work-in-progress" && (
    Status is {status}.{" "} - {lastReviewerComment && "Most recent reviewer comment was:"} + {lastReviewerComment && + lastEditDate < lastReviewDate && + "Most recent reviewer comment was:"}
    )}
    @@ -29,7 +33,7 @@ export default function SimpleReviewHeader({ {lastEditorComment || "(No comment was left by the author)"}

    )} - {status === "work-in-progress" && ( + {status === "work-in-progress" && lastEditDate < lastReviewDate && (

    {lastReviewerComment || "(No comment was left by a reviewer)"}

    diff --git a/src/components/signoff/SimpleReview/index.tsx b/src/components/signoff/SimpleReview/index.tsx index d2d22ea20..f6d5f4cdc 100644 --- a/src/components/signoff/SimpleReview/index.tsx +++ b/src/components/signoff/SimpleReview/index.tsx @@ -4,6 +4,8 @@ import type { ValidRecord, CollectionRouteMatch, SignoffState, + CollectionState, + Capabilities, } from "../../../types"; import * as SignoffActions from "../../../actions/signoff"; @@ -13,16 +15,25 @@ import SimpleReviewHeader from "./SimpleReviewHeader"; import PerRecordDiffView from "./PerRecordDiffView"; import { isReviewer } from "../SignoffToolBar"; import Spinner from "../../Spinner"; +import CollectionTabs from "../../collection/CollectionTabs"; +import { storageKeys, useLocalStorage } from "../../../hooks/storage"; +import { Redirect, useHistory } from "react-router-dom"; +import { Shuffle } from "react-bootstrap-icons"; +import { canEditCollection } from "../../../permission"; +import { isMember } from "../utils"; export type StateProps = { signoff?: SignoffState; session: SessionState; + capabilities: Capabilities; + collection: CollectionState; fetchRecords(bid: string, cid: string): Promise>; }; export type SimpleReviewProps = StateProps & { approveChanges: typeof SignoffActions.approveChanges; declineChanges: typeof SignoffActions.declineChanges; + requestReview: typeof SignoffActions.requestReview; rollbackChanges: typeof SignoffActions.rollbackChanges; listRecords: typeof CollectionActions.listRecords; match: CollectionRouteMatch; @@ -35,10 +46,18 @@ export default function SimpleReview({ fetchRecords, approveChanges, declineChanges, + requestReview, rollbackChanges, listRecords, match, + collection, + capabilities, }: SimpleReviewProps) { + const [useSimpleReview, setUseSimpleReview] = useLocalStorage( + storageKeys.useSimpleReview, + true + ); + const history = useHistory(); const signoffSource = signoff?.collectionsInfo?.source; const sourceBid = signoffSource?.bid; const sourceCid = signoffSource?.cid; @@ -47,34 +66,42 @@ export default function SimpleReview({ const destBid = signoffDest?.bid; const destCid = signoffDest?.cid; - const canReview = signoffSource ? isReviewer(signoffSource, session) : false; + const canReview = signoffSource + ? isReviewer(signoffSource, session) && + session.serverInfo?.user?.id !== signoffSource.lastReviewRequestBy + : false; + const [records, setRecords] = useState<{ loading: boolean; newRecords: ValidRecord[]; oldRecords: ValidRecord[]; }>({ - loading: false, + loading: true, newRecords: [], oldRecords: [], }); + const { + params: { bid, cid }, + } = match; + const canRequestReview = + canEditCollection(session, bid, collection) && + isMember("editors_group", signoffSource, session); + useEffect(() => { async function load() { - const { - params: { bid, cid }, - } = match; if (bid && cid) { // For signoff listRecords(bid, cid, "id"); } } load(); - }, [match.params.bid, match.params.cid]); + }, [bid, cid]); useEffect(() => { + // get collection data async function getRecords() { if (destCid && destBid && sourceBid && sourceCid) { - setRecords({ oldRecords: [], newRecords: [], loading: true }); const newRecords = await fetchRecords(sourceBid, sourceCid); const oldRecords = await fetchRecords(destBid, destCid); setRecords({ oldRecords, newRecords, loading: false }); @@ -83,41 +110,87 @@ export default function SimpleReview({ getRecords(); }, [destBid, destCid, sourceBid, sourceCid]); + if (!useSimpleReview) { + return ( + + ); + } + let message = ""; - if (session.authenticating) { - return ; - } else if (!session.authenticated) { + if (!session.authenticated) { message = "Not authenticated"; + } else if ( + session.authenticating || + session.busy || + (records.loading && signoffSource && signoffDest) + ) { + return ; } else if (!signoffSource || !signoffSource?.status) { message = "This is not a collection that supports reviews."; - } else if (session.authenticated && !canReview) { - message = `You do not have review permissions for the ${sourceCid} collection. Please contact an authorized reviewer.`; - } else if (records.loading) { - return ; } if (message) { - return
    {message}
    ; + return ( +
    {message}
    + ); } - return ( -
    - {["to-review", "work-in-progress"].includes(signoffSource.status) && ( - - - - )} + const handleRollback = (text: string) => { + rollbackChanges(text); + history.push(`/buckets/${bid}/collections/${cid}/records`); + }; - + return ( +
    +

    + Review{" "} + + {bid}/{cid} + {" "} + Changes +

    + + {signoffSource.status !== "signed" && ( + + + + )} + + +
    ); } diff --git a/src/components/signoff/utils.ts b/src/components/signoff/utils.ts new file mode 100644 index 000000000..25439ea26 --- /dev/null +++ b/src/components/signoff/utils.ts @@ -0,0 +1,25 @@ +import { SessionState, SignoffSourceInfo } from "../../types"; + +export function isMember( + groupKey: string, + source: SignoffSourceInfo, + sessionState: SessionState +) { + const { + serverInfo: { user, capabilities }, + } = sessionState; + if (!source || !user?.principals) { + return false; + } + const { principals } = user; + const { bid, cid } = source; + const { signer = {} } = capabilities; + // @ts-ignore + const { [groupKey]: defaultGroupName } = signer; + // @ts-ignore + const { [groupKey]: groupName = defaultGroupName } = source; + const expectedGroup = groupName.replace("{collection_id}", cid); + const expectedPrincipal = `/buckets/${bid}/groups/${expectedGroup}`; + + return principals.includes(expectedPrincipal); +} diff --git a/src/containers/signoff/SimpleReviewPage.ts b/src/containers/signoff/SimpleReviewPage.ts index ac78b5326..e60ec87f1 100644 --- a/src/containers/signoff/SimpleReviewPage.ts +++ b/src/containers/signoff/SimpleReviewPage.ts @@ -25,6 +25,8 @@ function mapStateToProps( return { session: state.session, signoff: state.signoff, + collection: state.collection, + capabilities: state.session.serverInfo.capabilities, fetchRecords, }; } diff --git a/src/hooks.ts b/src/hooks/app.ts similarity index 79% rename from src/hooks.ts rename to src/hooks/app.ts index c6458e6ce..de4bf39d9 100644 --- a/src/hooks.ts +++ b/src/hooks/app.ts @@ -1,5 +1,5 @@ import { TypedUseSelectorHook, useSelector, useDispatch } from "react-redux"; -import type { AppDispatch, AppState } from "./types"; +import type { AppDispatch, AppState } from "../types"; export const useAppSelector: TypedUseSelectorHook = useSelector; export const useAppDispatch = () => useDispatch(); diff --git a/src/hooks/storage.ts b/src/hooks/storage.ts new file mode 100644 index 000000000..e5d65a151 --- /dev/null +++ b/src/hooks/storage.ts @@ -0,0 +1,40 @@ +import { useState, useEffect } from "react"; + +export const storageKeys = { + useSimpleReview: "useSimpleReview", +}; + +export function useLocalStorage(key: string, initialValue: any) { + const [val, setVal] = useState(() => { + try { + return localStorage[key] ? JSON.parse(localStorage[key]) : initialValue; + } catch (ex) { + console.error("Error retrieving value from localStorage", ex); + return initialValue; + } + }); + + const setStoredVal = val => { + try { + localStorage[key] = JSON.stringify(val); + setVal(val); + } catch (ex) { + console.error("Error setting value in localStorage", ex); + } + }; + + useEffect(() => { + // will fire if localStorage is touched in another browser tab/window + const handleStorageChange = (evt: StorageEvent) => { + if (evt.key !== key) { + return; + } + setVal(evt.newValue ? JSON.parse(evt.newValue) : undefined); + }; + window.addEventListener("storage", handleStorageChange); + + return () => window.removeEventListener("storage", handleStorageChange); + }, [key]); + + return [val, setStoredVal]; +} diff --git a/src/permission.tsx b/src/permission.tsx index d9881e586..e28c5f588 100644 --- a/src/permission.tsx +++ b/src/permission.tsx @@ -1,6 +1,5 @@ import type { SessionState, - BucketState, CollectionState, GroupState, GroupData, @@ -37,12 +36,12 @@ export function canCreateBucket(session: SessionState): boolean { export function canEditBucket( session: SessionState, - bucket: BucketState + bucketId: string ): boolean { return can(session, (perm: PermissionsListEntry) => { return ( perm.resource_name == "bucket" && - perm.bucket_id == bucket.data.id && + perm.bucket_id == bucketId && perm.permissions.includes("write") ); }); @@ -50,12 +49,12 @@ export function canEditBucket( export function canCreateCollection( session: SessionState, - bucket: BucketState + bucketId: string ): boolean { return can(session, (perm: PermissionsListEntry) => { return ( perm.resource_name == "bucket" && - perm.bucket_id == bucket.data.id && + perm.bucket_id == bucketId && perm.permissions.includes("collection:create") ); }); @@ -63,7 +62,7 @@ export function canCreateCollection( export function canEditCollection( session: SessionState, - bucket: BucketState, + bucketId: string, collection: CollectionState ): boolean { return can(session, (perm: PermissionsListEntry) => { @@ -71,7 +70,7 @@ export function canEditCollection( (perm.resource_name == "bucket" || (perm.resource_name == "collection" && perm.collection_id == collection.data.id)) && - perm.bucket_id == bucket.data.id && + perm.bucket_id == bucketId && perm.permissions.includes("write") ); }); @@ -79,12 +78,12 @@ export function canEditCollection( export function canCreateGroup( session: SessionState, - bucket: BucketState + bucketId: string ): boolean { return can(session, (perm: PermissionsListEntry) => { return ( perm.resource_name == "bucket" && - perm.bucket_id == bucket.data.id && + perm.bucket_id == bucketId && perm.permissions.includes("group:create") ); }); @@ -92,7 +91,7 @@ export function canCreateGroup( export function canEditGroup( session: SessionState, - bucket: BucketState, + bucketId: string, group: GroupState ): boolean { return can(session, (perm: PermissionsListEntry) => { @@ -102,7 +101,7 @@ export function canEditGroup( return ( (perm.resource_name == "bucket" || (perm.resource_name == "group" && perm.group_id == group.data.id)) && - perm.bucket_id == bucket.data.id && + perm.bucket_id == bucketId && perm.permissions.includes("write") ); }); @@ -110,7 +109,7 @@ export function canEditGroup( export function canCreateRecord( session: SessionState, - bucket: BucketState, + bucketId: string, collection: CollectionState ): boolean { return can(session, (perm: PermissionsListEntry) => { @@ -119,14 +118,14 @@ export function canCreateRecord( (perm.resource_name == "collection" && perm.collection_id == collection.data.id && perm.permissions.includes("record:create"))) && - perm.bucket_id == bucket.data.id + perm.bucket_id == bucketId ); }); } export function canEditRecord( session: SessionState, - bucket: BucketState, + bucketId: string, collection: CollectionState, record: RecordState ): boolean { @@ -139,7 +138,7 @@ export function canEditRecord( perm.collection_id == collection.data.id && perm.record_id == record.data.id)) && perm.permissions.includes("write") && - perm.bucket_id == bucket.data.id + perm.bucket_id == bucketId ); }); } diff --git a/src/url.ts b/src/url.ts index b343ce3c9..f10b543b0 100644 --- a/src/url.ts +++ b/src/url.ts @@ -30,6 +30,8 @@ const URLS = { `/buckets/${bid}/collections/${cid}/history`, "collection:permissions": ({ bid, cid }) => `/buckets/${bid}/collections/${cid}/permissions`, + "collection:simple-review": ({ bid, cid }) => + `/buckets/${bid}/collections/${cid}/simple-review`, "collection:records": ({ bid, cid }) => `/buckets/${bid}/collections/${cid}/records`, diff --git a/test/components/signoff/SimpleReview/PerRecordDiffView_test.js b/test/components/signoff/SimpleReview/PerRecordDiffView_test.js index 7d2f324a7..c5b075501 100644 --- a/test/components/signoff/SimpleReview/PerRecordDiffView_test.js +++ b/test/components/signoff/SimpleReview/PerRecordDiffView_test.js @@ -65,6 +65,19 @@ describe("findChangeTypes", () => { ]); }); + it("should find multiple removed items", () => { + expect( + findChangeTypes( + [{ id: "a" }, { id: "b" }, { id: "c" }, { id: "d" }], + [{ id: "a" }] + ) + ).eql([ + { id: "b", changeType: ChangeType.REMOVE, source: { id: "b" } }, + { id: "c", changeType: ChangeType.REMOVE, source: { id: "c" } }, + { id: "d", changeType: ChangeType.REMOVE, source: { id: "d" } }, + ]); + }); + it("should find updated items", () => { expect( findChangeTypes( diff --git a/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js b/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js index 36fa4e3de..3f6858749 100644 --- a/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js +++ b/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js @@ -1,15 +1,13 @@ -import { expect } from "chai"; -import { createComponent } from "../../../test_utils"; -import ReactDomTestUtils from "react-dom/test-utils"; -import sinon from "sinon"; +import { renderWithProvider } from "../../../test_utils"; +import { fireEvent } from "@testing-library/react"; import * as React from "react"; import SimpleReviewButtons from "../../../../src/components/signoff/SimpleReview/SimpleReviewButtons"; function renderButtons(props = null) { - const approveChanges = sinon.stub(); - const declineChanges = sinon.stub(); - const rollbackChanges = sinon.stub(); + const approveChanges = jest.fn(); + const declineChanges = jest.fn(); + const rollbackChanges = jest.fn(); const mergedProps = { status: "to-review", @@ -19,45 +17,45 @@ function renderButtons(props = null) { ...props, }; - const node = createComponent(); + const node = renderWithProvider(); return { approveChanges, declineChanges, rollbackChanges, node }; } describe("SimpleReviewHeader component", () => { - it("should call approveChanges when approve button is clicked", () => { - const { approveChanges, node } = renderButtons({ status: "to-review" }); + it("should call approveChanges when approve button is clicked", async () => { + const { approveChanges, node } = renderButtons({ + status: "to-review", + canReview: true, + }); - ReactDomTestUtils.Simulate.click(node.querySelector(".btn-success")); - expect(approveChanges.firstCall).to.be.ok; + fireEvent.click(node.getByText(/Approve/)); + expect(approveChanges).toHaveBeenCalled(); + expect(await node.findByTestId("spinner")).toBeDefined(); }); it("should open CommentDialog when reject is clicked call declineChanges from modal", async () => { - const { declineChanges, node } = renderButtons({ status: "to-review" }); - ReactDomTestUtils.act(() => { - ReactDomTestUtils.Simulate.click(node.querySelector(".btn-danger")); - }); - expect(node.querySelector(".modal")).to.be.ok; - ReactDomTestUtils.act(() => { - ReactDomTestUtils.Simulate.click( - node.querySelector(".modal .btn-primary") - ); + const { declineChanges, node } = renderButtons({ + status: "to-review", + canReview: true, }); - expect(declineChanges.firstCall).to.be.ok; + fireEvent.click(node.getByText(/Decline/)); + fireEvent.click(await node.findByText("Reject changes")); + expect(declineChanges).toHaveBeenCalled(); + expect(await node.findByTestId("spinner")).toBeDefined(); + }); + + it("should open CommentDialog when request review is clicked and call requestReview from modal", async () => { + const {} = renderButtons({ status: "work-in-progress" }); }); it("should display rollback button when status is wip and call rollbackChanges from modal", async () => { const { rollbackChanges, node } = renderButtons({ status: "work-in-progress", + canRequestReview: true, }); - ReactDomTestUtils.act(() => { - ReactDomTestUtils.Simulate.click(node.querySelector(".btn-danger")); - }); - expect(node.querySelector(".modal")).to.be.ok; - ReactDomTestUtils.act(() => { - ReactDomTestUtils.Simulate.click( - node.querySelector(".modal .btn-primary") - ); - }); - expect(rollbackChanges.firstCall).to.be.ok; + fireEvent.click(node.getByText(/Rollback changes/)); + fireEvent.click(await node.findByText("Rollback")); + expect(rollbackChanges).toHaveBeenCalled(); + expect(await node.findByTestId("spinner")).toBeDefined(); }); }); diff --git a/test/components/signoff/SimpleReview/SimpleReviewHeader_test.js b/test/components/signoff/SimpleReview/SimpleReviewHeader_test.js index 20077b18a..d0893932f 100644 --- a/test/components/signoff/SimpleReview/SimpleReviewHeader_test.js +++ b/test/components/signoff/SimpleReview/SimpleReviewHeader_test.js @@ -1,6 +1,4 @@ -import { expect } from "chai"; import { createComponent } from "../../../test_utils"; - import SimpleReviewHeader from "../../../../src/components/signoff/SimpleReview/SimpleReviewHeader"; import * as React from "react"; @@ -22,24 +20,25 @@ const wipProps = { describe("SimpleReviewHeader component", () => { it("should render title when component is to-review", () => { const node = createComponent(); - expect(node.querySelector(".card-header").textContent).to.equal( + expect(node.querySelector(".card-header").textContent).toBe( "Review requested by ana:" ); }); it("should render an editor comment when component is to-review", () => { const node = createComponent(); - expect(node.querySelector(".card-text").textContent).to.equal( - "please review" - ); + expect(node.querySelector(".card-text").textContent).toBe("please review"); }); - it("should render a wip header", () => { - const node = createComponent(); - expect(node.querySelector(".card-header").textContent).to.equal( + it("should render a reviewer comments as expected when component is wip", () => { + const node = createComponent( + + ); + expect(node.querySelector(".card-text").textContent).toBe("no thanks"); + expect(node.querySelector(".card-header").textContent).toBe( "Status is work-in-progress. Most recent reviewer comment was:" ); }); - it("should render a reviewer comment when component is wip", () => { - const node = createComponent(); - expect(node.querySelector(".card-text").textContent).to.equal("no thanks"); - }); }); diff --git a/test/components/signoff/SimpleReview/SimpleReview_test.js b/test/components/signoff/SimpleReview/SimpleReview_test.js index 7f71a60b1..97e94190d 100644 --- a/test/components/signoff/SimpleReview/SimpleReview_test.js +++ b/test/components/signoff/SimpleReview/SimpleReview_test.js @@ -1,6 +1,50 @@ import * as React from "react"; +import { fireEvent, waitForElementToBeRemoved } from "@testing-library/react"; import { renderWithProvider, sessionFactory } from "../../../test_utils"; import SimpleReview from "../../../../src/components/signoff/SimpleReview"; +import { useLocalStorage } from "../../../../src/hooks/storage"; +import { Redirect, useHistory } from "react-router-dom"; + +jest.mock("../../../../src/hooks/storage", () => { + const originalModule = jest.requireActual("../../../../src/hooks/storage"); + return { + __esModule: true, + ...originalModule, + useLocalStorage: jest.fn().mockReturnValue([true, jest.fn()]), + }; +}); + +jest.mock("react-router-dom", () => { + const originalModule = jest.requireActual("react-router-dom"); + const pushMock = jest.fn(); + const useHistory = () => { + return { + push: pushMock, + }; + }; + return { + __esModule: true, + ...originalModule, + useHistory, + Redirect: jest.fn().mockReturnValue(
    foo
    ), + }; +}); + +jest.mock("../../../../src/permission", () => { + return { + canEditCollection: () => { + return true; + }, + }; +}); + +jest.mock("../../../../src/components/signoff/utils", () => { + return { + isMember: () => { + return true; + }, + }; +}); function signoffFactory() { return { @@ -14,7 +58,7 @@ function signoffFactory() { lastEditDate: 1622816256864, lastEditorComment: undefined, lastReviewBy: undefined, - lastReviewDate: NaN, + lastReviewDate: 1622816256865, lastReviewRequestBy: undefined, lastReviewRequestDate: NaN, lastReviewerComment: undefined, @@ -34,6 +78,10 @@ function signoffFactory() { pendingConfirmReviewRequest: false, pendingConfirmDeclineChanges: false, pendingConfirmRollbackChanges: false, + capabilities: {}, + collection: { + totalRecords: 3, + }, }; } @@ -48,8 +96,11 @@ function renderSimpleReview(props = null) { listRecords() {}, session: sessionFactory(), signoff: signoffFactory(), - async fetchRecords() {}, + async fetchRecords() { + return []; + }, ...props, + rollbackChanges: jest.fn(), }; return renderWithProvider(); } @@ -60,17 +111,6 @@ const fakeLocation = { hash: "", }; -jest.mock("react-router", () => { - const originalModule = jest.requireActual("react-router"); - return { - __esModule: true, - ...originalModule, - useLocation: () => { - return fakeLocation; - }, - }; -}); - describe("SimpleTest component", () => { it("should render spinner when authenticating", async () => { const node = renderSimpleReview({ @@ -86,13 +126,6 @@ describe("SimpleTest component", () => { expect(node.container.textContent).toBe("Not authenticated"); }); - it("should render not authenticated", async () => { - const node = renderSimpleReview({ - session: sessionFactory({ authenticated: false, authenticating: false }), - }); - expect(node.container.textContent).toBe("Not authenticated"); - }); - it("should render not reviewable", async () => { const node = renderSimpleReview({ signoff: undefined }); expect(node.container.textContent).toBe( @@ -100,26 +133,35 @@ describe("SimpleTest component", () => { ); }); - it("should render not a reviewer", async () => { + it("should render a diff form for not a reviewer", async () => { const session = sessionFactory(); session.serverInfo.user.principals = []; const node = renderSimpleReview({ session, }); - expect(node.container.textContent).toMatch( - /You do not have review permissions/ + await waitForElementToBeRemoved(() => node.queryByTestId("spinner")); + expect(node.getByText(/Status is/).textContent).toBe( + "Status is work-in-progress. " ); + expect(node.getByText("(No comment was left by a reviewer)")).toBeDefined(); + expect(node.getByText("Show all lines")).toBeDefined(); }); - it("should render a review component after records are fetched", async () => { + it("should render a review component after records are fetched and allow for a rollback", async () => { let node = renderSimpleReview({ async fetchRecords() { return []; }, }); + await waitForElementToBeRemoved(() => node.queryByTestId("spinner")); expect(node.queryByText("Rollback")).toBeDefined(); expect(node.container.querySelector(".simple-review-header")).toBeDefined(); + + // also check rollback calls history.push while we're here + fireEvent.click(node.queryByText(/Rollback changes/)); + fireEvent.click(node.queryByText("Rollback")); + expect(useHistory().push).toHaveBeenCalled(); }); it("should hide the rollback button if the hideRollback query parameter is provided", async () => { @@ -129,8 +171,19 @@ describe("SimpleTest component", () => { return []; }, }); + await waitForElementToBeRemoved(() => node.queryByTestId("spinner")); expect(node.queryByText("Rollback")).toBeNull(); fakeLocation.search = ""; }); + + it("should redirect the user if the legacy review process is enabled", async () => { + useLocalStorage.mockReturnValue([false, jest.fn()]); + const session = sessionFactory(); + session.serverInfo.user.principals = []; + renderSimpleReview({ + session, + }); + expect(Redirect).toHaveBeenCalled(); + }); }); diff --git a/test/components/signoff/components_test.js b/test/components/signoff/components_test.js index 0c9d5e2ad..ff8f120f7 100644 --- a/test/components/signoff/components_test.js +++ b/test/components/signoff/components_test.js @@ -2,6 +2,25 @@ import SignoffToolBar from "../../../src/components/signoff/SignoffToolBar"; import * as React from "react"; import { render, fireEvent } from "@testing-library/react"; +// to avoid rendering a router around everything, allows for more focused testing +jest.mock("react-router-dom", () => { + const originalModule = jest.requireActual("react-router-dom"); + return { + __esModule: true, + ...originalModule, + Link: "a", + }; +}); + +jest.mock("../../../src/hooks/storage", () => { + const originalModule = jest.requireActual("../../../src/hooks/storage"); + return { + __esModule: true, + ...originalModule, + useLocalStorage: jest.fn().mockReturnValue([false, jest.fn()]), + }; +}); + describe("SignoffToolBar component", () => { const props = { sessionState: { @@ -121,7 +140,7 @@ describe("SignoffToolBar component", () => { expect(node.queryByTestId("spinner")).toBeNull(); expect(propsOverride.approveChanges).toHaveBeenCalledTimes(0); fireEvent.click(await node.findByText("Approve")); - expect(node.queryByTestId("spinner")).toBeDefined(); + expect(await node.findByTestId("spinner")).toBeDefined(); expect(propsOverride.approveChanges).toHaveBeenCalledTimes(1); }); }); diff --git a/test/hooks/storage_test.js b/test/hooks/storage_test.js new file mode 100644 index 000000000..b814c86f7 --- /dev/null +++ b/test/hooks/storage_test.js @@ -0,0 +1,76 @@ +import { renderHook, act } from "@testing-library/react-hooks"; +import { useLocalStorage } from "../../src/hooks/storage"; + +class localStorageClass { + isConstructor() { + this.store = {}; + } + + clear() { + this.store = {}; + } + + getItem(key) { + return this.store[key]; + } + + setItem(key, value) { + this.store[key] = String(value); + } + + removeItem(key) { + delete this.store[key]; + } +} + +global.localStorage = new localStorageClass(); + +describe("useLocalStorage", () => { + it("should return default value if a value isn't set yet", () => { + const { result } = renderHook(() => + useLocalStorage("testKey", "defaultValue") + ); + + expect(result.current[0]).toBe("defaultValue"); + }); + + it("should get an existing value from localStorage if one exists", () => { + localStorage.setItem("testKey", JSON.stringify("storedValue")); + const { result } = renderHook(() => + useLocalStorage("testKey", "defaultValue") + ); + + expect(result.current[0]).toBe("storedValue"); + }); + + it("should set a new value in localStorage", () => { + const { result } = renderHook(() => + useLocalStorage("testKey", "defaultValue") + ); + + act(() => { + result.current[1]("newValue"); + }); + + expect(result.current[0]).toBe("newValue"); + expect(localStorage.getItem("testKey")).toBe(JSON.stringify("newValue")); + }); + + it("should update the value when localStorage changes from another session", () => { + const { result } = renderHook(() => + useLocalStorage("testKey", "defaultValue") + ); + + act(() => { + localStorage.setItem("testKey", JSON.stringify("externalChange")); + const event = new StorageEvent("storage", { + oldValue: JSON.stringify("defaultValue"), + newValue: JSON.stringify("externalChange"), + key: "testKey", + }); + window.dispatchEvent(event); + }); + + expect(result.current[0]).toBe("externalChange"); + }); +}); diff --git a/test/permission_test.js b/test/permission_test.js index 85a50d106..29513c09f 100644 --- a/test/permission_test.js +++ b/test/permission_test.js @@ -44,14 +44,12 @@ describe("canCreateBucket", () => { describe("canEditBucket", () => { it("should always return true if no permissions list", () => { const session = { permissions: null }; - const bucket = {}; - expect(canEditBucket(session, bucket)).eql(true); + expect(canEditBucket(session, "")).eql(true); }); it("should return false if object is not listed", () => { const session = { permissions: [{ bucket_id: "xyz" }] }; - const bucket = { data: { id: "abc" } }; - expect(canEditBucket(session, bucket)).eql(false); + expect(canEditBucket(session, "abc")).eql(false); }); it("should return false if permission is not listed", () => { @@ -64,8 +62,7 @@ describe("canEditBucket", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; - expect(canEditBucket(session, bucket)).eql(false); + expect(canEditBucket(session, "xyz")).eql(false); }); it("should return true if permission is listed", () => { @@ -78,22 +75,19 @@ describe("canEditBucket", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; - expect(canEditBucket(session, bucket)).eql(true); + expect(canEditBucket(session, "xyz")).eql(true); }); }); describe("canCreateCollection", () => { it("should always return true if no permisssions list", () => { const session = { permissions: null }; - const bucket = {}; - expect(canCreateCollection(session, bucket)).eql(true); + expect(canCreateCollection(session, "")).eql(true); }); it("should return false if object is not listed", () => { const session = { permissions: [{ bucket_id: "xyz" }] }; - const bucket = { data: { id: "abc" } }; - expect(canCreateCollection(session, bucket)).eql(false); + expect(canCreateCollection(session, "abc")).eql(false); }); it("should return false if permission is not listed", () => { @@ -106,8 +100,7 @@ describe("canCreateCollection", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; - expect(canCreateCollection(session, bucket)).eql(false); + expect(canCreateCollection(session, "xyz")).eql(false); }); it("should return true if permission is listed", () => { @@ -120,22 +113,19 @@ describe("canCreateCollection", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; - expect(canCreateCollection(session, bucket)).eql(true); + expect(canCreateCollection(session, "xyz")).eql(true); }); }); describe("canCreateGroup", () => { it("should always return true if no permisssions list", () => { const session = { permissions: null }; - const bucket = {}; - expect(canCreateGroup(session, bucket)).eql(true); + expect(canCreateGroup(session, "")).eql(true); }); it("should return false if object is not listed", () => { const session = { permissions: [{ bucket_id: "xyz" }] }; - const bucket = { data: { id: "abc" } }; - expect(canCreateGroup(session, bucket)).eql(false); + expect(canCreateGroup(session, "abc")).eql(false); }); it("should return false if permission is not listed", () => { @@ -148,8 +138,7 @@ describe("canCreateGroup", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; - expect(canCreateGroup(session, bucket)).eql(false); + expect(canCreateGroup(session, "xyz")).eql(false); }); it("should return true if permission is listed", () => { @@ -162,26 +151,23 @@ describe("canCreateGroup", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; - expect(canCreateGroup(session, bucket)).eql(true); + expect(canCreateGroup(session, "xyz")).eql(true); }); }); describe("canEditCollection", () => { it("should always return true if no permisssions list", () => { const session = { permissions: null }; - const bucket = {}; const collection = {}; - expect(canEditCollection(session, bucket, collection)).eql(true); + expect(canEditCollection(session, "", collection)).eql(true); }); it("should return false if object is not listed", () => { const session = { permissions: [{ bucket_id: "abc", collection_id: "foo" }], }; - const bucket = { data: { id: "abc" } }; const collection = { data: { id: "bar" } }; - expect(canEditCollection(session, bucket, collection)).eql(false); + expect(canEditCollection(session, "abc", collection)).eql(false); }); it("should return false if permission is not listed", () => { @@ -195,9 +181,8 @@ describe("canEditCollection", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; - expect(canEditCollection(session, bucket, collection)).eql(false); + expect(canEditCollection(session, "xyz", collection)).eql(false); }); it("should return true if permission is listed", () => { @@ -211,9 +196,8 @@ describe("canEditCollection", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; - expect(canEditCollection(session, bucket, collection)).eql(true); + expect(canEditCollection(session, "xyz", collection)).eql(true); }); it("should return true if permission on bucket is listed", () => { @@ -226,25 +210,22 @@ describe("canEditCollection", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; - expect(canEditCollection(session, bucket, collection)).eql(true); + expect(canEditCollection(session, "xyz", collection)).eql(true); }); }); describe("canEditGroup", () => { it("should always return true if no permisssions list", () => { const session = { permissions: null }; - const bucket = {}; const group = {}; - expect(canEditGroup(session, bucket, group)).eql(true); + expect(canEditGroup(session, "", group)).eql(true); }); it("should return false if object is not listed", () => { const session = { permissions: [{ bucket_id: "abc", group_id: "foo" }] }; - const bucket = { data: { id: "abc" } }; const group = { data: { id: "bar" } }; - expect(canEditGroup(session, bucket, group)).eql(false); + expect(canEditGroup(session, "abc", group)).eql(false); }); it("should return false if permission is not listed", () => { @@ -258,9 +239,8 @@ describe("canEditGroup", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const group = { data: { id: "foo" } }; - expect(canEditGroup(session, bucket, group)).eql(false); + expect(canEditGroup(session, "xyz", group)).eql(false); }); it("should return true if permission is listed", () => { @@ -274,9 +254,8 @@ describe("canEditGroup", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const group = { data: { id: "foo" } }; - expect(canEditGroup(session, bucket, group)).eql(true); + expect(canEditGroup(session, "xyz", group)).eql(true); }); it("should return true if permission on bucket is listed", () => { @@ -289,27 +268,24 @@ describe("canEditGroup", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const group = { data: { id: "foo" } }; - expect(canEditGroup(session, bucket, group)).eql(true); + expect(canEditGroup(session, "xyz", group)).eql(true); }); }); describe("canCreateRecord", () => { it("should always return true if no permisssions list", () => { const session = { permissions: null }; - const bucket = {}; const collection = {}; - expect(canCreateRecord(session, bucket, collection)).eql(true); + expect(canCreateRecord(session, "", collection)).eql(true); }); it("should return false if object is not listed", () => { const session = { permissions: [{ bucket_id: "abc", collection_id: "foo" }], }; - const bucket = { data: { id: "abc" } }; const collection = { data: { id: "bar" } }; - expect(canCreateRecord(session, bucket, collection)).eql(false); + expect(canCreateRecord(session, "abc", collection)).eql(false); }); it("should return false if permission is not listed", () => { @@ -323,9 +299,8 @@ describe("canCreateRecord", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; - expect(canCreateRecord(session, bucket, collection)).eql(false); + expect(canCreateRecord(session, "xyz", collection)).eql(false); }); it("should return true if permission is listed", () => { @@ -339,9 +314,8 @@ describe("canCreateRecord", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; - expect(canCreateRecord(session, bucket, collection)).eql(true); + expect(canCreateRecord(session, "xyz", collection)).eql(true); }); it("should return true if permission on bucket is listed", () => { @@ -354,19 +328,17 @@ describe("canCreateRecord", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; - expect(canCreateRecord(session, bucket, collection)).eql(true); + expect(canCreateRecord(session, "xyz", collection)).eql(true); }); }); describe("canEditRecord", () => { it("should always return true if no permisssions list", () => { const session = { permissions: null }; - const bucket = {}; const collection = {}; const record = {}; - expect(canEditRecord(session, bucket, collection, record)).eql(true); + expect(canEditRecord(session, "", collection, record)).eql(true); }); it("should return false if object is not listed", () => { @@ -379,10 +351,9 @@ describe("canEditRecord", () => { }, ], }; - const bucket = { data: { id: "abc" } }; const collection = { data: { id: "bar" } }; const record = { data: { id: "blah" } }; - expect(canEditRecord(session, bucket, collection, record)).eql(false); + expect(canEditRecord(session, "abc", collection, record)).eql(false); }); it("should return false if permission is not listed", () => { @@ -397,10 +368,9 @@ describe("canEditRecord", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; const record = { data: { id: "blah" } }; - expect(canEditRecord(session, bucket, collection, record)).eql(false); + expect(canEditRecord(session, "xyz", collection, record)).eql(false); }); it("should return true if permission is listed", () => { @@ -415,10 +385,9 @@ describe("canEditRecord", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; const record = { data: { id: "blah" } }; - expect(canEditRecord(session, bucket, collection, record)).eql(true); + expect(canEditRecord(session, "xyz", collection, record)).eql(true); }); it("should return true if permission on bucket is listed", () => { @@ -431,10 +400,9 @@ describe("canEditRecord", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; const record = { data: { id: "blah" } }; - expect(canEditRecord(session, bucket, collection, record)).eql(true); + expect(canEditRecord(session, "xyz", collection, record)).eql(true); }); it("should return true if permission on collection is listed", () => { @@ -448,10 +416,9 @@ describe("canEditRecord", () => { }, ], }; - const bucket = { data: { id: "xyz" } }; const collection = { data: { id: "foo" } }; const record = { data: { id: "blah" } }; - expect(canEditRecord(session, bucket, collection, record)).eql(true); + expect(canEditRecord(session, "xyz", collection, record)).eql(true); }); });