From 25132049dd8d04a5d71c97d72a15925187e760aa Mon Sep 17 00:00:00 2001 From: Tiago Barbosa Date: Wed, 15 Nov 2023 17:41:43 -0600 Subject: [PATCH] fix: list only oncall users in plugin (#38) - add support for profile images - refactor oncall user list behavior to only show users on escalation level 1 - remove duplicates from oncall user list --- dev/mockPagerDutyApi.ts | 9 +- src/api/client.test.ts | 1 + .../EntityPagerDutyCard/index.test.tsx | 1 + src/components/Escalation/Escalation.test.tsx | 136 +++++++++++++++++- .../Escalation/EscalationPolicy.tsx | 16 ++- src/components/Escalation/EscalationUser.tsx | 2 +- src/components/PagerDutyCard/index.test.tsx | 1 + src/components/types.ts | 1 + 8 files changed, 160 insertions(+), 7 deletions(-) diff --git a/dev/mockPagerDutyApi.ts b/dev/mockPagerDutyApi.ts index f8836d7..270c490 100644 --- a/dev/mockPagerDutyApi.ts +++ b/dev/mockPagerDutyApi.ts @@ -39,6 +39,7 @@ export const mockPagerDutyApi: PagerDutyApi = { email: 'email@email.com', html_url: 'http://user', name: 'some-user', + avatar_url: 'http://avatar', }, }, }, @@ -61,6 +62,7 @@ export const mockPagerDutyApi: PagerDutyApi = { email: 'email@email.com', html_url: 'http://user', name: 'some-user', + avatar_url: 'http://avatar', }, }, }, @@ -122,21 +124,22 @@ export const mockPagerDutyApi: PagerDutyApi = { }, async getOnCallByPolicyId() { - const oncall = (name: string, escalation: number) => { + const oncall = (id: string, name: string, escalation: number) => { return { user: { - id: '123', + id: id, name: name, html_url: 'http://assignee', summary: 'summary', email: 'email@email.com', + avatar_url: 'http://avatar', }, escalation_level: escalation, }; }; return { - oncalls: [oncall('Jane Doe', 1), oncall('John Doe', 2)], + oncalls: [oncall('1', 'Jane Doe', 1), oncall('2', 'John Doe', 2), oncall('3', 'James Doe', 1)], }; }, diff --git a/src/api/client.test.ts b/src/api/client.test.ts index 84fabbb..51fa222 100644 --- a/src/api/client.test.ts +++ b/src/api/client.test.ts @@ -40,6 +40,7 @@ const user: PagerDutyUser = { summary: 'person1', email: 'person1@example.com', html_url: 'http://a.com/id1', + avatar_url: 'http://a.com/id1/avatar', }; const service: PagerDutyService = { diff --git a/src/components/EntityPagerDutyCard/index.test.tsx b/src/components/EntityPagerDutyCard/index.test.tsx index 051085c..1c82355 100644 --- a/src/components/EntityPagerDutyCard/index.test.tsx +++ b/src/components/EntityPagerDutyCard/index.test.tsx @@ -79,6 +79,7 @@ const user: PagerDutyUser = { summary: 'person1', email: 'person1@example.com', html_url: 'http://a.com/id1', + avatar_url: 'http://a.com/id1/avatar', }; const service: PagerDutyService = { diff --git a/src/components/Escalation/Escalation.test.tsx b/src/components/Escalation/Escalation.test.tsx index 3201bbc..7524c44 100644 --- a/src/components/Escalation/Escalation.test.tsx +++ b/src/components/Escalation/Escalation.test.tsx @@ -57,8 +57,9 @@ describe('Escalation', () => { id: 'p1', summary: 'person1', email: 'person1@example.com', - html_url: 'http://a.com/id1', + html_url: 'http://a.com/id1', } as PagerDutyUser, + escalation_level: 1, }, ], })); @@ -77,6 +78,139 @@ describe('Escalation', () => { expect(mockPagerDutyApi.getOnCallByPolicyId).toHaveBeenCalledWith('abc'); }); + it("Renders a list of users without duplicates", async () => { + mockPagerDutyApi.getOnCallByPolicyId = jest + .fn() + .mockImplementationOnce(async () => ({ + oncalls: [ + { + user: { + name: "person1", + id: "p1", + summary: "person1", + email: "person1@example.com", + html_url: "http://a.com/id1", + } as PagerDutyUser, + escalation_level: 1, + }, + { + user: { + name: "person2", + id: "p2", + summary: "person2", + email: "person2@example.com", + html_url: "http://a.com/id2", + } as PagerDutyUser, + escalation_level: 1, + }, + { + user: { + name: "person2", + id: "p2", + summary: "person2", + email: "person2@example.com", + html_url: "http://a.com/id2", + } as PagerDutyUser, + escalation_level: 1, + }, + ], + })); + + const { getByText, queryByTestId, queryAllByText } = render( + wrapInTestApp( + + + + ) + ); + await waitFor(() => !queryByTestId("progress")); + + expect(getByText("person1")).toBeInTheDocument(); + expect(getByText("person1@example.com")).toBeInTheDocument(); + expect(queryAllByText("person2").length).toBe(1); + expect(queryAllByText("person2@example.com").length).toBe(1); + expect(mockPagerDutyApi.getOnCallByPolicyId).toHaveBeenCalledWith("abc"); + }); + + it("Renders only user(s) in escalation level 1", async () => { + mockPagerDutyApi.getOnCallByPolicyId = jest + .fn() + .mockImplementationOnce(async () => ({ + oncalls: [ + { + user: { + name: "person1", + id: "p1", + summary: "person1", + email: "person1@example.com", + html_url: "http://a.com/id1", + } as PagerDutyUser, + escalation_level: 1, + }, + { + user: { + name: "person2", + id: "p2", + summary: "person2", + email: "person2@example.com", + html_url: "http://a.com/id2", + } as PagerDutyUser, + escalation_level: 2, + }, + ], + })); + + const { getByText, queryByTestId, queryByText } = render( + wrapInTestApp( + + + + ) + ); + await waitFor(() => !queryByTestId("progress")); + + expect(getByText("person1")).toBeInTheDocument(); + expect(getByText("person1@example.com")).toBeInTheDocument(); + expect(queryByText("person2")).not.toBeInTheDocument(); + expect(queryByText("person2@example.com")).not.toBeInTheDocument(); + expect(mockPagerDutyApi.getOnCallByPolicyId).toHaveBeenCalledWith("abc"); + }); + + it("Renders a user with profile picture", async () => { + mockPagerDutyApi.getOnCallByPolicyId = jest + .fn() + .mockImplementationOnce(async () => ({ + oncalls: [ + { + user: { + name: "person1", + id: "p1", + summary: "person1", + email: "person1@example.com", + html_url: "http://a.com/id1", + avatar_url: + "https://gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y", + } as PagerDutyUser, + escalation_level: 1, + }, + ], + })); + + const { getByText, queryByTestId, getByAltText } = render( + wrapInTestApp( + + + + ) + ); + await waitFor(() => !queryByTestId("progress")); + + expect(getByText("person1")).toBeInTheDocument(); + expect(getByText("person1@example.com")).toBeInTheDocument(); + expect(getByAltText("User")).toHaveAttribute("src", "https://gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y") + expect(mockPagerDutyApi.getOnCallByPolicyId).toHaveBeenCalledWith("abc"); + }); + it('Handles errors', async () => { mockPagerDutyApi.getOnCallByPolicyId = jest .fn() diff --git a/src/components/Escalation/EscalationPolicy.tsx b/src/components/Escalation/EscalationPolicy.tsx index f05b91b..21e41ca 100644 --- a/src/components/Escalation/EscalationPolicy.tsx +++ b/src/components/Escalation/EscalationPolicy.tsx @@ -39,8 +39,20 @@ export const EscalationPolicy = ({ policyId }: Props) => { } = useAsync(async () => { const { oncalls } = await api.getOnCallByPolicyId(policyId); const usersItem = oncalls - .sort((a, b) => a.escalation_level - b.escalation_level) - .map(oncall => oncall.user); + .filter((oncall) => oncall.escalation_level === 1) + .sort((a, b) => a.user.name > b.user.name ? 1 : -1) + .map((oncall) => oncall.user); + + // remove duplicates from usersItem + const uniqueUsers = new Map(); + usersItem.forEach((user) => { + uniqueUsers.set(user.id, user); + }); + + usersItem.length = 0; + uniqueUsers.forEach((user) => { + usersItem.push(user); + }); return usersItem; }); diff --git a/src/components/Escalation/EscalationUser.tsx b/src/components/Escalation/EscalationUser.tsx index 85542bf..105d6d7 100644 --- a/src/components/Escalation/EscalationUser.tsx +++ b/src/components/Escalation/EscalationUser.tsx @@ -46,7 +46,7 @@ export const EscalationUser = ({ user }: Props) => { return ( - +