-
Notifications
You must be signed in to change notification settings - Fork 424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TAA-136: MVP of the Approval Request and Approval Request List page functionality #576
Changes from all commits
c979b49
548391f
e1e3c55
a378675
aa08970
bd686d0
c44cbc5
e0b910c
6e2bf42
0c77487
350abd6
b828012
3968b2c
e574128
9ae1976
f9fd58b
a2fb5d7
7d1ddab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,168 @@ | ||
import { screen, render, waitFor } from "@testing-library/react"; | ||
import userEvent from "@testing-library/user-event"; | ||
import type { ReactElement } from "react"; | ||
import { ThemeProvider } from "@zendeskgarden/react-theming"; | ||
import ApprovalRequestListPage from "./ApprovalRequestListPage"; | ||
import { useSearchApprovalRequests } from "./hooks/useSearchApprovalRequests"; | ||
|
||
jest.mock( | ||
"@zendeskgarden/svg-icons/src/16/search-stroke.svg", | ||
() => "svg-mock" | ||
); | ||
|
||
jest.mock("./hooks/useSearchApprovalRequests"); | ||
const mockUseSearchApprovalRequests = useSearchApprovalRequests as jest.Mock; | ||
|
||
const renderWithTheme = (ui: ReactElement) => { | ||
return render(<ThemeProvider>{ui}</ThemeProvider>); | ||
}; | ||
|
||
const mockApprovalRequests = [ | ||
{ | ||
id: 123, | ||
subject: "Hardware request", | ||
requester_name: "Jane Doe", | ||
created_by_name: "John Doe", | ||
created_at: "2024-02-20T10:00:00Z", | ||
status: "active", | ||
}, | ||
{ | ||
id: 456, | ||
subject: "Software license", | ||
requester_name: "Jane Smith", | ||
created_by_name: "Bob Smith", | ||
created_at: "2024-02-19T15:00:00Z", | ||
status: "approved", | ||
}, | ||
]; | ||
|
||
describe("ApprovalRequestListPage", () => { | ||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it("renders the loading state initially", () => { | ||
mockUseSearchApprovalRequests.mockReturnValue({ | ||
approvalRequests: [], | ||
errorFetchingApprovalRequests: null, | ||
approvalRequestStatus: "any", | ||
setApprovalRequestStatus: jest.fn(), | ||
isLoading: true, | ||
}); | ||
|
||
renderWithTheme( | ||
<ApprovalRequestListPage baseLocale="en-US" helpCenterPath="/hc/en-us" /> | ||
); | ||
|
||
expect(screen.getByRole("progressbar")).toBeInTheDocument(); | ||
}); | ||
|
||
it("renders the approval requests list page with data", () => { | ||
mockUseSearchApprovalRequests.mockReturnValue({ | ||
approvalRequests: mockApprovalRequests, | ||
errorFetchingApprovalRequests: null, | ||
approvalRequestStatus: "any", | ||
setApprovalRequestStatus: jest.fn(), | ||
isLoading: false, | ||
}); | ||
|
||
renderWithTheme( | ||
<ApprovalRequestListPage baseLocale="en-US" helpCenterPath="/hc/en-us" /> | ||
); | ||
|
||
expect(screen.getByText("Approval requests")).toBeInTheDocument(); | ||
expect(screen.getByText("Hardware request")).toBeInTheDocument(); | ||
expect(screen.getByText("Software license")).toBeInTheDocument(); | ||
}); | ||
|
||
it("renders the empty state when there are no approval requests", () => { | ||
mockUseSearchApprovalRequests.mockReturnValue({ | ||
approvalRequests: [], | ||
errorFetchingApprovalRequests: null, | ||
approvalRequestStatus: "any", | ||
setApprovalRequestStatus: jest.fn(), | ||
isLoading: false, | ||
}); | ||
|
||
renderWithTheme( | ||
<ApprovalRequestListPage baseLocale="en-US" helpCenterPath="/hc/en-us" /> | ||
); | ||
|
||
expect(screen.getByText("No approval requests found.")).toBeInTheDocument(); | ||
}); | ||
|
||
it("filters the approval requests by search term", async () => { | ||
const user = userEvent.setup(); | ||
mockUseSearchApprovalRequests.mockReturnValue({ | ||
approvalRequests: mockApprovalRequests, | ||
errorFetchingApprovalRequests: null, | ||
approvalRequestStatus: "any", | ||
setApprovalRequestStatus: jest.fn(), | ||
isLoading: false, | ||
}); | ||
|
||
renderWithTheme( | ||
<ApprovalRequestListPage baseLocale="en-US" helpCenterPath="/hc/en-us" /> | ||
); | ||
|
||
const searchInput = screen.getByPlaceholderText( | ||
/search approval requests/i | ||
); | ||
await user.type(searchInput, "Hardware"); | ||
|
||
waitFor(() => { | ||
expect(screen.getByText("Hardware request")).toBeInTheDocument(); | ||
expect(screen.queryByText("Software license")).not.toBeInTheDocument(); | ||
}); | ||
}); | ||
|
||
it("sorts the approval requests by the sent on date", async () => { | ||
const user = userEvent.setup(); | ||
mockUseSearchApprovalRequests.mockReturnValue({ | ||
approvalRequests: mockApprovalRequests, | ||
errorFetchingApprovalRequests: null, | ||
approvalRequestStatus: "any", | ||
setApprovalRequestStatus: jest.fn(), | ||
isLoading: false, | ||
}); | ||
|
||
renderWithTheme( | ||
<ApprovalRequestListPage baseLocale="en-US" helpCenterPath="/hc/en-us" /> | ||
); | ||
|
||
const createdHeader = screen.getByText("Sent on"); | ||
await user.click(createdHeader); | ||
|
||
// Wait for re-render after sort | ||
await waitFor(() => { | ||
const rows = screen.getAllByRole("row").slice(1); // Skip header row | ||
expect(rows[0]).toHaveTextContent("Software license"); | ||
expect(rows[1]).toHaveTextContent("Hardware request"); | ||
}); | ||
}); | ||
|
||
it("throws an error when the request fails", () => { | ||
const consoleSpy = jest | ||
.spyOn(console, "error") | ||
.mockImplementation(() => {}); | ||
const error = new Error("Failed to fetch"); | ||
mockUseSearchApprovalRequests.mockReturnValue({ | ||
approvalRequests: [], | ||
errorFetchingApprovalRequests: error, | ||
approvalRequestStatus: "any", | ||
setApprovalRequestStatus: jest.fn(), | ||
isLoading: false, | ||
}); | ||
|
||
expect(() => | ||
renderWithTheme( | ||
<ApprovalRequestListPage | ||
baseLocale="en-US" | ||
helpCenterPath="/hc/en-us" | ||
/> | ||
) | ||
).toThrow("Failed to fetch"); | ||
|
||
consoleSpy.mockRestore(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
import { memo, useState, useMemo } from "react"; | ||
import { useTranslation } from "react-i18next"; | ||
import styled from "styled-components"; | ||
import { MD, XXL } from "@zendeskgarden/react-typography"; | ||
import { getColorV8 } from "@zendeskgarden/react-theming"; | ||
import { Spinner } from "@zendeskgarden/react-loaders"; | ||
import { useSearchApprovalRequests } from "./hooks/useSearchApprovalRequests"; | ||
import ApprovalRequestListFilters from "./components/approval-request-list/ApprovalRequestListFilters"; | ||
import ApprovalRequestListTable from "./components/approval-request-list/ApprovalRequestListTable"; | ||
|
||
const Container = styled.div` | ||
display: flex; | ||
flex-direction: column; | ||
gap: ${(props) => props.theme.space.lg}; | ||
margin-top: ${(props) => props.theme.space.xl}; /* 40px */ | ||
`; | ||
|
||
const LoadingContainer = styled.div` | ||
display: flex; | ||
justify-content: center; | ||
align-items: center; | ||
`; | ||
|
||
const NoApprovalRequestsText = styled(MD)` | ||
color: ${(props) => getColorV8("grey", 600, props.theme)}; | ||
`; | ||
|
||
export interface ApprovalRequestListPageProps { | ||
baseLocale: string; | ||
helpCenterPath: string; | ||
} | ||
|
||
type SortDirection = "asc" | "desc" | undefined; | ||
|
||
function ApprovalRequestListPage({ | ||
baseLocale, | ||
helpCenterPath, | ||
}: ApprovalRequestListPageProps) { | ||
const { t } = useTranslation(); | ||
const [searchTerm, setSearchTerm] = useState(""); | ||
const [sortDirection, setSortDirection] = useState<SortDirection>(undefined); | ||
const { | ||
approvalRequests, | ||
errorFetchingApprovalRequests: error, | ||
approvalRequestStatus, | ||
setApprovalRequestStatus, | ||
isLoading, | ||
} = useSearchApprovalRequests(); | ||
|
||
const sortedAndFilteredApprovalRequests = useMemo(() => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't filtering and sorting be handled via the api? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, ideally the filtering and sorting is both handled API side but that has not been implemented yet and we agreed to do this client side for the EAP. The results are currently unpaginated as well and we do have plans to sort/filter on the API long term |
||
let results = [...approvalRequests]; | ||
|
||
// Apply search filter | ||
if (searchTerm) { | ||
const term = searchTerm.toLowerCase(); | ||
results = results.filter((request) => | ||
request.subject.toLowerCase().includes(term) | ||
); | ||
} | ||
|
||
// Apply sorting | ||
if (sortDirection) { | ||
results.sort((a, b) => { | ||
const dateA = new Date(a.created_at).getTime(); | ||
const dateB = new Date(b.created_at).getTime(); | ||
return sortDirection === "asc" ? dateA - dateB : dateB - dateA; | ||
}); | ||
} | ||
|
||
return results; | ||
}, [approvalRequests, searchTerm, sortDirection]); | ||
|
||
if (error) { | ||
throw error; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: i know this is still MVP but we could note to handle errors gracefully in the future like a growl or something with better UX There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe similar to using notify below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good callout, I followed the pattern used in the Service Catalog: https://github.com/zendesk/copenhagen_theme/blob/beta/src/modules/service-catalog/ServiceCatalogItemPage.tsx#L49-L51 This relies on the |
||
} | ||
|
||
if (isLoading) { | ||
return ( | ||
<LoadingContainer> | ||
<Spinner size="64" /> | ||
</LoadingContainer> | ||
); | ||
} | ||
|
||
return ( | ||
<Container> | ||
<XXL isBold> | ||
{t("approval-requests.list.header", "Approval requests")} | ||
</XXL> | ||
<ApprovalRequestListFilters | ||
approvalRequestStatus={approvalRequestStatus} | ||
setApprovalRequestStatus={setApprovalRequestStatus} | ||
setSearchTerm={setSearchTerm} | ||
/> | ||
{approvalRequests.length === 0 ? ( | ||
<NoApprovalRequestsText> | ||
{t( | ||
"approval-requests.list.no-requests", | ||
"No approval requests found." | ||
)} | ||
</NoApprovalRequestsText> | ||
) : ( | ||
<ApprovalRequestListTable | ||
approvalRequests={sortedAndFilteredApprovalRequests} | ||
baseLocale={baseLocale} | ||
helpCenterPath={helpCenterPath} | ||
sortDirection={sortDirection} | ||
onSortChange={setSortDirection} | ||
/> | ||
)} | ||
</Container> | ||
); | ||
} | ||
|
||
export default memo(ApprovalRequestListPage); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious why do we need to memoize here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a habit/pre-optimization. In |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only using
testing-library/react
version 12 here because that is what is compatible with react 17 which is the current version of react within the repo