@@ -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 7584952f7..f6d5f4cdc 100644
--- a/src/components/signoff/SimpleReview/index.tsx
+++ b/src/components/signoff/SimpleReview/index.tsx
@@ -17,7 +17,7 @@ import { isReviewer } from "../SignoffToolBar";
import Spinner from "../../Spinner";
import CollectionTabs from "../../collection/CollectionTabs";
import { storageKeys, useLocalStorage } from "../../../hooks/storage";
-import { Redirect } from "react-router-dom";
+import { Redirect, useHistory } from "react-router-dom";
import { Shuffle } from "react-bootstrap-icons";
import { canEditCollection } from "../../../permission";
import { isMember } from "../utils";
@@ -57,6 +57,7 @@ export default function SimpleReview({
storageKeys.useSimpleReview,
true
);
+ const history = useHistory();
const signoffSource = signoff?.collectionsInfo?.source;
const sourceBid = signoffSource?.bid;
const sourceCid = signoffSource?.cid;
@@ -65,7 +66,10 @@ 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;
@@ -117,22 +121,31 @@ export default function SimpleReview({
}
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) {
- // TODO: use this to show/hide
- // also: {["to-review", "work-in-progress"].includes(signoffSource.status) && (
message = "This is not a collection that supports reviews.";
}
if (message) {
- return
{message}
;
+ return (
+
{message}
+ );
}
+ const handleRollback = (text: string) => {
+ rollbackChanges(text);
+ history.push(`/buckets/${bid}/collections/${cid}/records`);
+ };
+
return (
-
+
Review{" "}
@@ -154,25 +167,26 @@ export default function SimpleReview({
approveChanges={approveChanges}
declineChanges={declineChanges}
requestReview={requestReview}
- rollbackChanges={rollbackChanges}
+ rollbackChanges={handleRollback}
canReview={canReview}
canRequestReview={canRequestReview}
/>
)}
- {(records.loading && ) || (
-
- )}
+
diff --git a/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js b/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js
index e55fa99e2..3f6858749 100644
--- a/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js
+++ b/test/components/signoff/SimpleReview/SimpleReviewButtons_test.js
@@ -22,18 +22,26 @@ function renderButtons(props = null) {
}
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,
+ });
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" });
+ const { declineChanges, node } = renderButtons({
+ status: "to-review",
+ canReview: true,
+ });
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 () => {
@@ -43,10 +51,11 @@ describe("SimpleReviewHeader component", () => {
it("should display rollback button when status is wip and call rollbackChanges from modal", async () => {
const { rollbackChanges, node } = renderButtons({
status: "work-in-progress",
- canReview: true,
+ canRequestReview: true,
});
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 8d8b2b543..97e94190d 100644
--- a/test/components/signoff/SimpleReview/SimpleReview_test.js
+++ b/test/components/signoff/SimpleReview/SimpleReview_test.js
@@ -1,9 +1,9 @@
import * as React from "react";
-import { waitForElementToBeRemoved } from "@testing-library/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 } from "react-router-dom";
+import { Redirect, useHistory } from "react-router-dom";
jest.mock("../../../../src/hooks/storage", () => {
const originalModule = jest.requireActual("../../../../src/hooks/storage");
@@ -14,6 +14,38 @@ jest.mock("../../../../src/hooks/storage", () => {
};
});
+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 {
collectionsInfo: {
@@ -26,7 +58,7 @@ function signoffFactory() {
lastEditDate: 1622816256864,
lastEditorComment: undefined,
lastReviewBy: undefined,
- lastReviewDate: NaN,
+ lastReviewDate: 1622816256865,
lastReviewRequestBy: undefined,
lastReviewRequestDate: NaN,
lastReviewerComment: undefined,
@@ -68,6 +100,7 @@ function renderSimpleReview(props = null) {
return [];
},
...props,
+ rollbackChanges: jest.fn(),
};
return renderWithProvider();
}
@@ -78,18 +111,6 @@ const fakeLocation = {
hash: "",
};
-jest.mock("react-router", () => {
- const originalModule = jest.requireActual("react-router");
- return {
- __esModule: true,
- ...originalModule,
- useLocation: () => {
- return fakeLocation;
- },
- Redirect: jest.fn().mockReturnValue(foo
),
- };
-});
-
describe("SimpleTest component", () => {
it("should render spinner when authenticating", async () => {
const node = renderSimpleReview({
@@ -126,15 +147,21 @@ describe("SimpleTest component", () => {
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 () => {
@@ -144,6 +171,7 @@ describe("SimpleTest component", () => {
return [];
},
});
+ await waitForElementToBeRemoved(() => node.queryByTestId("spinner"));
expect(node.queryByText("Rollback")).toBeNull();
fakeLocation.search = "";
@@ -153,10 +181,9 @@ describe("SimpleTest component", () => {
useLocalStorage.mockReturnValue([false, jest.fn()]);
const session = sessionFactory();
session.serverInfo.user.principals = [];
- const node = renderSimpleReview({
+ renderSimpleReview({
session,
});
- console.log(node.container.innerHTML);
expect(Redirect).toHaveBeenCalled();
});
});