Skip to content
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

Merged
merged 18 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c979b49
feat(TAA-163): create approval requests module and placeholder templates
KlimekM Jan 7, 2025
548391f
feat(TAA-163): build out initial Approval Request skeleton UI with mo…
KlimekM Jan 8, 2025
e1e3c55
feat(TAA-163): build out initial Approval Request List skeleton UI wi…
KlimekM Jan 10, 2025
a378675
feat(TAA-163): wire up individual approval request to first API endpoint
KlimekM Jan 24, 2025
aa08970
feat(TAA-163): begin integrating with the decision API
KlimekM Jan 29, 2025
bd686d0
feat(TAA-163): wire up the Approval Request List to the REST API and…
KlimekM Feb 4, 2025
c44cbc5
fix(TAA-136): addresses type error, wires up now returning decision n…
KlimekM Feb 5, 2025
e0b910c
fix(TAA-136): commit assets/ changes, without the custom_theme was th…
KlimekM Feb 5, 2025
6e2bf42
feat(TAA-163): add Breadcrumbs to the Approval Request page
KlimekM Feb 5, 2025
0c77487
test(TAA-163): begin adding tests for the individual approval-request…
KlimekM Feb 5, 2025
350abd6
feat(TAA-163): add hardcoded link in dropdown for Approval requests
KlimekM Feb 6, 2025
b828012
fix(TAA-364): update decision textarea to match design and include av…
KlimekM Feb 10, 2025
3968b2c
fix(TAA-370): remove border radius, add sortable sent on cell, and ad…
KlimekM Feb 10, 2025
e574128
fix: commit generated bundle file updates
KlimekM Feb 10, 2025
9ae1976
feat: update to use translation function with English fallbacks
KlimekM Feb 11, 2025
f9fd58b
test: finish writing unit tests for ApprovalRequests
KlimekM Feb 11, 2025
a2fb5d7
feat(TAA-136): address PR feedback re: using constant and missed tran…
KlimekM Feb 18, 2025
7d1ddab
chore(TAA-136): colocate tests based on PR feedback
KlimekM Feb 19, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ module.exports = {
"@typescript-eslint/consistent-type-imports": "error",
"check-file/folder-naming-convention": [
"error",
{ "src/**/": "KEBAB_CASE" },
{
"src/**/!(__tests__)/": "KEBAB_CASE",
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luis-almeida I ended up adding a __tests__ directory within the approval-requests module and this was throwing an eslint error on those files. Let me know if you want to handle this differently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to colocate the tests in 7d1ddab which removed the need for the eslint updates

],
"check-file/filename-naming-convention": [
"error",
Expand Down
479 changes: 479 additions & 0 deletions assets/approval-requests-bundle.js

Large diffs are not rendered by default.

131 changes: 130 additions & 1 deletion assets/flash-notifications-bundle.js

Large diffs are not rendered by default.

1,162 changes: 1,127 additions & 35 deletions assets/new-request-form-bundle.js

Large diffs are not rendered by default.

3,656 changes: 3,655 additions & 1 deletion assets/new-request-form-translations-bundle.js

Large diffs are not rendered by default.

1,115 changes: 1,028 additions & 87 deletions assets/service-catalog-bundle.js

Large diffs are not rendered by default.

2,003 changes: 2,002 additions & 1 deletion assets/service-catalog-translations-bundle.js

Large diffs are not rendered by default.

38,944 changes: 38,909 additions & 35 deletions assets/shared-bundle.js

Large diffs are not rendered by default.

1,671 changes: 1,663 additions & 8 deletions assets/ticket-fields-bundle.js

Large diffs are not rendered by default.

35 changes: 34 additions & 1 deletion assets/wysiwyg-bundle.js

Large diffs are not rendered by default.

7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@
"@zendeskgarden/container-grid": "^3.0.14",
"@zendeskgarden/container-utilities": "^2.0.2",
"@zendeskgarden/react-accordions": "8.76.9",
"@zendeskgarden/react-breadcrumbs": "8.76.9",
"@zendeskgarden/react-buttons": "8.76.9",
"@zendeskgarden/react-datepickers": "8.76.9",
"@zendeskgarden/react-dropdowns.next": "8.76.9",
"@zendeskgarden/react-grid": "8.76.9",
"@zendeskgarden/react-forms": "8.76.9",
"@zendeskgarden/react-grid": "^8.76.9",
"@zendeskgarden/react-loaders": "8.76.9",
"@zendeskgarden/react-modals": "8.76.9",
"@zendeskgarden/react-notifications": "8.76.9",
"@zendeskgarden/react-pagination": "8.76.9",
"@zendeskgarden/react-tables": "8.76.9",
"@zendeskgarden/react-tags": "8.76.9",
"@zendeskgarden/react-theming": "8.76.9",
"@zendeskgarden/react-tooltips": "8.76.9",
Expand Down Expand Up @@ -66,7 +68,8 @@
"@svgr/rollup": "^8.1.0",
"@testing-library/dom": "^9.3.1",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/user-event": "^14.4.3",
"@testing-library/react": "^12.1.5",
Copy link
Contributor Author

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

"@testing-library/user-event": "^14.6.1",
"@types/lodash.debounce": "^4.0.9",
"@types/react": "^17.0.62",
"@types/react-dom": "^17.0.20",
Expand Down
1 change: 1 addition & 0 deletions rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export default defineConfig([
"new-request-form": "src/modules/new-request-form/index.tsx",
"flash-notifications": "src/modules/flash-notifications/index.ts",
"service-catalog": "src/modules/service-catalog/index.tsx",
"approval-requests": "src/modules/approval-requests/index.tsx",
},
output: {
dir: "assets",
Expand Down
89 changes: 89 additions & 0 deletions src/modules/approval-requests/ApprovalRequestListPage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { memo, useState, useMemo } from "react";
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;
}

function ApprovalRequestListPage({
baseLocale,
helpCenterPath,
}: ApprovalRequestListPageProps) {
const [searchTerm, setSearchTerm] = useState("");
const {
approvalRequests,
errorFetchingApprovalRequests: error,
approvalRequestStatus,
setApprovalRequestStatus,
isLoading,
} = useSearchApprovalRequests();

const filteredRequests = useMemo(() => {
if (!searchTerm) return approvalRequests;

const term = searchTerm.toLowerCase();
return approvalRequests.filter((request) =>
request.subject.toLowerCase().includes(term)
);
}, [approvalRequests, searchTerm]);

if (error) {
throw error;

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe similar to using notify below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 ErrorBoundary in the src/modules/approval-requests/renderApprovalRequestList.tsx file which will display this standard error screen:
Screenshot 2025-02-05 at 11 09 08 AM

}

if (isLoading) {
return (
<LoadingContainer>
<Spinner size="64" />
</LoadingContainer>
);
}

return (
<Container>
<XXL isBold>Approval requests</XXL>

Check warning on line 68 in src/modules/approval-requests/ApprovalRequestListPage.tsx

View workflow job for this annotation

GitHub Actions / Lint JS files

Do not use hardcoded content as the children of the XXL component
<ApprovalRequestListFilters
approvalRequestStatus={approvalRequestStatus}
setApprovalRequestStatus={setApprovalRequestStatus}
setSearchTerm={setSearchTerm}
/>
{approvalRequests.length === 0 ? (
<NoApprovalRequestsText>

Check warning on line 75 in src/modules/approval-requests/ApprovalRequestListPage.tsx

View workflow job for this annotation

GitHub Actions / Lint JS files

Do not use hardcoded content as the children of the NoApprovalRequestsText component
No approval requests found.
</NoApprovalRequestsText>
) : (
<ApprovalRequestListTable
requests={filteredRequests}
baseLocale={baseLocale}
helpCenterPath={helpCenterPath}
/>
)}
</Container>
);
}

export default memo(ApprovalRequestListPage);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious why do we need to memoize here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a habit/pre-optimization. In zendesk_console we wrap each component in memo by default to to ensure that the component does not re-render if it's parent component re-renders unless it's props have also changed.

138 changes: 138 additions & 0 deletions src/modules/approval-requests/ApprovalRequestPage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { memo } from "react";
import styled from "styled-components";
import { MD, XXL } from "@zendeskgarden/react-typography";
import { Spinner } from "@zendeskgarden/react-loaders";
import ApprovalRequestDetails from "./components/approval-request/ApprovalRequestDetails";
import ApprovalTicketDetails from "./components/approval-request/ApprovalTicketDetails";
import ApproverActions from "./components/approval-request/ApproverActions";
import { useApprovalRequest } from "./hooks/useApprovalRequest";
import type { Organization } from "../ticket-fields";
import ApprovalRequestBreadcrumbs from "./components/approval-request/ApprovalRequestBreadcrumbs";

const Container = styled.div`
display: flex;
flex-direction: row;
margin-top: ${(props) => props.theme.space.xl}; /* 40px */
margin-bottom: ${(props) => props.theme.space.lg}; /* 32px */

@media (max-width: ${(props) => props.theme.breakpoints.md}) {
flex-direction: column;
margin-bottom: ${(props) => props.theme.space.xl}; /* 40px */
}
`;

const LoadingContainer = styled.div`
display: flex;
justify-content: center;
align-items: center;
`;

const LeftColumn = styled.div`
flex: 2;
display: grid;
grid-template-rows: auto;
margin-right: ${(props) => props.theme.space.xl};

& > *:first-child {
margin-bottom: ${(props) => props.theme.space.base * 4}px; /* 16px */
}

& > *:not(:first-child) {
margin-bottom: ${(props) => props.theme.space.lg}; /* 32px */
}

& > *:last-child {
margin-bottom: 0;
}

@media (max-width: ${(props) => props.theme.breakpoints.md}) {
margin-right: 0;
margin-bottom: ${(props) => props.theme.space.lg};
}
`;

const RightColumn = styled.div`
flex: 1;
margin-left: ${(props) => props.theme.space.xl};

@media (max-width: ${(props) => props.theme.breakpoints.md}) {
margin-left: 0;
}
`;

export interface ApprovalRequestPageProps {
approvalWorkflowInstanceId: string;
approvalRequestId: string;
baseLocale: string;
helpCenterPath: string;
organizations: Array<Organization>;
userId: number;
}

function ApprovalRequestPage({
approvalWorkflowInstanceId,
approvalRequestId,
baseLocale,
helpCenterPath,
organizations,
userId,
}: ApprovalRequestPageProps) {
const {
approvalRequest,
setApprovalRequest,
errorFetchingApprovalRequest: error,
isLoading,
} = useApprovalRequest(approvalWorkflowInstanceId, approvalRequestId);

if (error) {
throw error;
}

if (isLoading) {
return (
<LoadingContainer>
<Spinner size="64" />
</LoadingContainer>
);
}

const showApproverActions =
userId === approvalRequest?.assignee_user?.id &&
approvalRequest?.status === "active";

return (
<>
<ApprovalRequestBreadcrumbs
helpCenterPath={helpCenterPath}
organizations={organizations}
isApprovalRequestPage
/>
<Container>
<LeftColumn>
<XXL isBold>{approvalRequest?.subject}</XXL>
<MD>{approvalRequest?.message}</MD>
{approvalRequest?.ticket_details && (
<ApprovalTicketDetails ticket={approvalRequest.ticket_details} />
)}
</LeftColumn>
<RightColumn>
{approvalRequest && (
<ApprovalRequestDetails
approvalRequest={approvalRequest}
baseLocale={baseLocale}
/>
)}
</RightColumn>
</Container>
{showApproverActions && (
<ApproverActions
approvalWorkflowInstanceId={approvalWorkflowInstanceId}
approvalRequestId={approvalRequestId}
setApprovalRequest={setApprovalRequest}
/>
)}
</>
);
}

export default memo(ApprovalRequestPage);
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import {
memo,
useCallback,
type Dispatch,
type SetStateAction,
useMemo,
} from "react";
import styled from "styled-components";
import debounce from "lodash.debounce";
import SearchIcon from "@zendeskgarden/svg-icons/src/16/search-stroke.svg";
import {
Field,
Label,
Combobox,
Option,
} from "@zendeskgarden/react-dropdowns.next";
import { MediaInput } from "@zendeskgarden/react-forms";
import type { IComboboxProps } from "@zendeskgarden/react-dropdowns.next";
import type { ApprovalRequestDropdownStatus } from "../../types";

const FiltersContainer = styled.div`
display: flex;
gap: ${(props) => props.theme.space.base * 17}px; /* 68px */
align-items: flex-end;

@media (max-width: ${(props) => props.theme.breakpoints.md}) {
flex-direction: column;
align-items: normal;
gap: ${(props) => props.theme.space.base * 4}px; /* 16px */
}
`;

const SearchField = styled(Field)`
flex: 3;
`;

const StyledMediaInput = styled(MediaInput)`
border-radius: 25px;
`;

const DropdownFilterField = styled(Field)`
flex: 1;
`;

const ApprovalRequestStatusInputMap = {
any: "Any",
active: "Decision pending",
approved: "Approved",
rejected: "Denied",
withdrawn: "Withdrawn",
};

interface ApprovalRequestListFiltersProps {
approvalRequestStatus: ApprovalRequestDropdownStatus;
setApprovalRequestStatus: Dispatch<
SetStateAction<ApprovalRequestDropdownStatus>
>;
setSearchTerm: Dispatch<SetStateAction<string>>;
}

function ApprovalRequestListFilters({
approvalRequestStatus,
setApprovalRequestStatus,
setSearchTerm,
}: ApprovalRequestListFiltersProps) {
const handleChange = useCallback<NonNullable<IComboboxProps["onChange"]>>(
(changes) => {
if (!changes.selectionValue) {
return;
}

setApprovalRequestStatus(
changes.selectionValue as ApprovalRequestDropdownStatus
);
setSearchTerm(""); // Reset search term when changing status
},
[setApprovalRequestStatus]

Check warning on line 77 in src/modules/approval-requests/components/approval-request-list/ApprovalRequestListFilters.tsx

View workflow job for this annotation

GitHub Actions / Lint JS files

React Hook useCallback has a missing dependency: 'setSearchTerm'. Either include it or remove the dependency array. If 'setSearchTerm' changes too often, find the parent component that defines it and wrap that definition in useCallback
);

const debouncedSetSearchTerm = useMemo(
() => debounce((value: string) => setSearchTerm(value), 300),
[setSearchTerm]
);

const handleSearch = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
debouncedSetSearchTerm(event.target.value);
},
[debouncedSetSearchTerm]
);

return (
<FiltersContainer>
<SearchField>
<Label hidden>Search approval requests</Label>

Check warning on line 95 in src/modules/approval-requests/components/approval-request-list/ApprovalRequestListFilters.tsx

View workflow job for this annotation

GitHub Actions / Lint JS files

Do not use hardcoded content as the children of the Label component
<StyledMediaInput
start={<SearchIcon />}
placeholder="Search approval requests"
onChange={handleSearch}
/>
</SearchField>
<DropdownFilterField>
<Label>Status:</Label>

Check warning on line 103 in src/modules/approval-requests/components/approval-request-list/ApprovalRequestListFilters.tsx

View workflow job for this annotation

GitHub Actions / Lint JS files

Do not use hardcoded content as the children of the Label component
<Combobox
isEditable={false}
onChange={handleChange}
selectionValue={approvalRequestStatus}
inputValue={ApprovalRequestStatusInputMap[approvalRequestStatus]}
>
<Option value="any" label="Any" />
<Option value="active" label="Decision pending" />
{/* <Option value="clarification_requested" label="Info needed" /> */}
<Option value="approved" label="Approved" />
<Option value="rejected" label="Denied" />
<Option value="withdrawn" label="Withdrawn" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will these options always be hardcoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values will since they correspond with the accepted statuses in the API. The labels will be updated. I merged the translations earlier today and will follow up and update the text throughout

</Combobox>
</DropdownFilterField>
</FiltersContainer>
);
}

export default memo(ApprovalRequestListFilters);
Loading
Loading