From 3d690981cb25288feb797ea02fae9a144b2fb9ca Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 18 Apr 2024 17:23:41 +0200 Subject: [PATCH 1/6] Show spinner in create ticket button while ticket generation is ongoing --- src/components/incident/IncidentDetails.tsx | 22 +++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/components/incident/IncidentDetails.tsx b/src/components/incident/IncidentDetails.tsx index 9e29244f..9beb4419 100644 --- a/src/components/incident/IncidentDetails.tsx +++ b/src/components/incident/IncidentDetails.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo } from "react"; +import React, {useEffect, useMemo, useState} from "react"; import "react-table/react-table.css"; import Grid from "@material-ui/core/Grid"; @@ -48,6 +48,7 @@ import { globalConfig } from "../../config"; import "./IncidentDetails.css"; import {Hidden} from "@material-ui/core"; import {ModifyTicketButton, TicketModifiableField} from "./ModifyTicketAction"; +import Spinning from "../spinning"; type IncidentDetailsListItemPropsType = { title: string; detail: string | React.ReactNode; @@ -174,6 +175,7 @@ const IncidentDetails: React.FC = ({ const [{ result: acks, isLoading: isAcksLoading }, setAcksPromise] = useApiIncidentAcks(); const [{ result: events, isLoading: isEventsLoading }, setEventsPromise] = useApiIncidentEvents(); + const [isGenerateTicketLoading, setIsGenerateTicketLoading] = useState(false); useEffect(() => { setAcksPromise(api.getIncidentAcks(incident.pk)); @@ -242,15 +244,19 @@ const IncidentDetails: React.FC = ({ }; const handleCreateTicket = () => { + setIsGenerateTicketLoading(true); api .putCreateTicketEvent(incident.pk) // eslint-disable-next-line @typescript-eslint/no-unused-vars .then(({ ticket_url }: IncidentTicketUrlBody) => { displayAlert(`Created ticket from incident ${incident.pk}`, "success"); onIncidentChange({ ...incident, ticket_url }); + setIsGenerateTicketLoading(false); window.open(ticket_url, '_blank', 'noopener,noreferrer'); }) - .catch((error) => {}); + .catch((error) => { + setIsGenerateTicketLoading(false); + }); }; const handleSaveTicket = (url?: string) => { @@ -401,7 +407,11 @@ const IncidentDetails: React.FC = ({ onCreateTicket={handleCreateTicket} onSaveTicket={handleSaveTicket} ticketUrl={incident.ticket_url} - isBulk={false}> + isBulk={false} + modifyTicketButtonProps={{ + startIcon: isGenerateTicketLoading ? : null, + }} + > @@ -566,7 +576,11 @@ const IncidentDetails: React.FC = ({ onCreateTicket={handleCreateTicket} onSaveTicket={handleSaveTicket} ticketUrl={incident.ticket_url} - isBulk={false}> + isBulk={false} + modifyTicketButtonProps={{ + startIcon: isGenerateTicketLoading ? : null, + }} + > From 24a2dff11a49f57b92fbba51738c127fa4bc8733 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 18 Apr 2024 17:39:40 +0200 Subject: [PATCH 2/6] Make it possible to center several components at once --- src/components/centercontainer/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/centercontainer/index.tsx b/src/components/centercontainer/index.tsx index 24760f96..0b856974 100644 --- a/src/components/centercontainer/index.tsx +++ b/src/components/centercontainer/index.tsx @@ -6,7 +6,7 @@ type CenterContainerPropsType = { children: React.Props<{}>["children"] }; const CenterContainer: React.FC = ({ children }: CenterContainerPropsType) => { return ( - {children} + {children} ); }; From 968893a7156e50749a269bdeaceb825384b83993 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 18 Apr 2024 17:53:47 +0200 Subject: [PATCH 3/6] Fix bug with color definitions in buttons With previous definition the whole button would become white, not just the contents color --- src/components/incident/styles.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/incident/styles.tsx b/src/components/incident/styles.tsx index 6443d1de..dd330711 100644 --- a/src/components/incident/styles.tsx +++ b/src/components/incident/styles.tsx @@ -30,16 +30,16 @@ export const useStyles = makeStyles((theme: Theme) => "flex-grow": 0, }, dangerousButton: { - background: theme.palette.warning.main, - color: "#FFFFFF !important", + background: `${theme.palette.warning.main} !important`, + color: `${WHITE} !important`, '&:hover': { backgroundColor: `${theme.palette.warning.main} !important`, opacity: 0.7 }, }, safeButton: { - background: theme.palette.primary.main, - color: "#FFFFFF !important", + background: `${theme.palette.primary.main} !important`, + color: `${WHITE} !important`, '&:hover': { backgroundColor: `${theme.palette.primary.main} !important`, opacity: 0.7 From 2045f28df0c4084c0c2807505586c0d80c48d760 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Thu, 18 Apr 2024 17:42:17 +0200 Subject: [PATCH 4/6] Fix styling and spacing when spinner is rendered --- src/components/incident/IncidentDetails.tsx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/components/incident/IncidentDetails.tsx b/src/components/incident/IncidentDetails.tsx index 9beb4419..5def17f3 100644 --- a/src/components/incident/IncidentDetails.tsx +++ b/src/components/incident/IncidentDetails.tsx @@ -401,16 +401,16 @@ const IncidentDetails: React.FC = ({ onManualOpen={handleManualOpen} isBulk={false} /> - - + : null, - }} + onCreateTicket={handleCreateTicket} + onSaveTicket={handleSaveTicket} + ticketUrl={incident.ticket_url} + isBulk={false} + modifyTicketButtonProps={{ + startIcon: isGenerateTicketLoading ? : null, + variant: "contained", + }} > @@ -579,6 +579,7 @@ const IncidentDetails: React.FC = ({ isBulk={false} modifyTicketButtonProps={{ startIcon: isGenerateTicketLoading ? : null, + variant: "contained", }} > From d0793a719350a0f6ddcb0d0bdeba53342f196401 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 19 Apr 2024 10:42:36 +0200 Subject: [PATCH 5/6] Write tests --- .../incident/test/IncidentDetails.test.tsx | 132 +++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/src/components/incident/test/IncidentDetails.test.tsx b/src/components/incident/test/IncidentDetails.test.tsx index 7fc001d7..90760034 100644 --- a/src/components/incident/test/IncidentDetails.test.tsx +++ b/src/components/incident/test/IncidentDetails.test.tsx @@ -1,7 +1,7 @@ /** * @jest-environment jsdom */ import React from "react"; -import {render, screen, waitFor, within} from "@testing-library/react"; +import {render, screen, waitFor, waitForElementToBeRemoved, within} from "@testing-library/react"; import IncidentDetails from "../IncidentDetails"; import {Incident, IncidentTag, IncidentTicketUrlBody, SourceSystem} from "../../../api/types"; @@ -319,6 +319,136 @@ describe('Incident Details: Create Ticket button',() => { // Create ticket endpoint was not called expect(createTicketSpy).toHaveBeenCalledTimes(0); }); + + it("should not display spinner within create ticket button by default in the incident details view", async () => { + await waitFor(() => { + render( + + ) + }); + + // Create ticket button is rendered + const createTicketButton = screen.getByRole('button', {name: /create ticket/i}); + expect(createTicketButton).toBeInTheDocument(); + + // Spinner is not present within create ticket button + expect(within(createTicketButton).queryByRole('progressbar')).not.toBeInTheDocument(); + }); + + it("should display spinner within create ticket button after user confirms create ticket action in the dialog", async () => { + await waitFor(() => { + render( + + ) + }); + + // User clicks on the Create Ticket button + const createTicketButton = screen.getByRole('button', {name: /create ticket/i}); + userEvent.click(createTicketButton); + + // User confirms create ticket action in the dialog + const dialogSubmitButton = screen.getByRole('button', {name: /yes/i}); + expect(dialogSubmitButton).toBeInTheDocument(); + userEvent.click(dialogSubmitButton); + + // Spinner appears within create ticket button + const createTicketProgress = within(createTicketButton).getByRole('progressbar'); + expect(createTicketProgress).toBeInTheDocument(); + }); + + it("should not display spinner within create ticket button after user cancels create ticket action in the dialog", async () => { + await waitFor(() => { + render( + + ) + }); + + // User clicks on the Create Ticket button + const createTicketButton = screen.getByRole('button', {name: /create ticket/i}); + userEvent.click(createTicketButton); + + // User cancels create ticket action in the dialog + const dialogCancelButton = screen.getByRole('button', {name: /no/i}); + expect(dialogCancelButton).toBeInTheDocument(); + userEvent.click(dialogCancelButton); + + // Spinner is not present within create ticket button + expect(within(createTicketButton).queryByRole('progressbar')).not.toBeInTheDocument(); + }); + + it("should stop displaying spinner within create ticket button after error on create ticket operation", async () => { + createTicketSpy.mockRejectedValue(new Error("Create ticket test error")); + + await waitFor(() => { + render( + + ) + }); + + // User clicks on the Create Ticket button + const createTicketButton = screen.getByRole('button', {name: /create ticket/i}); + userEvent.click(createTicketButton); + + // User confirms create ticket action + const dialogSubmitButton = screen.getByRole('button', {name: /yes/i}); + expect(dialogSubmitButton).toBeInTheDocument(); + userEvent.click(dialogSubmitButton); + + // Spinner appears within create ticket button + const createTicketProgress = within(createTicketButton).getByRole('progressbar'); + expect(createTicketProgress).toBeInTheDocument(); + + // Create ticket endpoint was called with correct incident pk + expect(createTicketSpy).toHaveBeenCalledTimes(1) + expect(createTicketSpy).toHaveBeenCalledWith(4000) + + // Spinner within create ticket button disappears + await waitForElementToBeRemoved(createTicketProgress); + }); + + it("should stop displaying spinner within create ticket button after success on create ticket operation", async () => { + await waitFor(() => { + render( + + ) + }); + + // User clicks on the Create Ticket button + const createTicketButton = screen.getByRole('button', {name: /create ticket/i}); + userEvent.click(createTicketButton); + + // User confirms create ticket action + const dialogSubmitButton = screen.getByRole('button', {name: /yes/i}); + expect(dialogSubmitButton).toBeInTheDocument(); + userEvent.click(dialogSubmitButton); + + // Spinner appears within create ticket button + const createTicketProgress = within(createTicketButton).getByRole('progressbar'); + expect(createTicketProgress).toBeInTheDocument(); + + // Create ticket endpoint was called with correct incident pk + expect(createTicketSpy).toHaveBeenCalledTimes(1) + expect(createTicketSpy).toHaveBeenCalledWith(4000) + expect(createTicketSpy).toHaveReturnedWith(Promise.resolve({ticket_url: generatedTicketUrlMockValue})) + + // Spinner within create ticket button disappears + await waitForElementToBeRemoved(createTicketProgress); + }); }); describe('Primary Details section: Source internal incident ID',() => { From cc1088b57edc24802d8d46e2a9d92cf308e72409 Mon Sep 17 00:00:00 2001 From: Ilona Podliashanyk Date: Fri, 19 Apr 2024 11:03:51 +0200 Subject: [PATCH 6/6] Update changelog --- changelog.d/560.added.md | 1 + changelog.d/560.fixed.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog.d/560.added.md create mode 100644 changelog.d/560.fixed.md diff --git a/changelog.d/560.added.md b/changelog.d/560.added.md new file mode 100644 index 00000000..e067451e --- /dev/null +++ b/changelog.d/560.added.md @@ -0,0 +1 @@ +Visual feedback via spinner in the _Create ticket_-button that ticket is being automatically generated. \ No newline at end of file diff --git a/changelog.d/560.fixed.md b/changelog.d/560.fixed.md new file mode 100644 index 00000000..0044f93e --- /dev/null +++ b/changelog.d/560.fixed.md @@ -0,0 +1 @@ +Bug with inconsistent text and background coloring of the custom made buttons on the _Incidents_ page. \ No newline at end of file