Skip to content

Commit

Permalink
fix: list only oncall users in plugin (#38)
Browse files Browse the repository at this point in the history
- add support for profile images
- refactor oncall user list behavior to only show users on escalation level 1
- remove duplicates from oncall user list
  • Loading branch information
t1agob authored Nov 15, 2023
1 parent 0e52c71 commit 2513204
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 7 deletions.
9 changes: 6 additions & 3 deletions dev/mockPagerDutyApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const mockPagerDutyApi: PagerDutyApi = {
email: 'email@email.com',
html_url: 'http://user',
name: 'some-user',
avatar_url: 'http://avatar',
},
},
},
Expand All @@ -61,6 +62,7 @@ export const mockPagerDutyApi: PagerDutyApi = {
email: 'email@email.com',
html_url: 'http://user',
name: 'some-user',
avatar_url: 'http://avatar',
},
},
},
Expand Down Expand Up @@ -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)],
};
},

Expand Down
1 change: 1 addition & 0 deletions src/api/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
1 change: 1 addition & 0 deletions src/components/EntityPagerDutyCard/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
136 changes: 135 additions & 1 deletion src/components/Escalation/Escalation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
],
}));
Expand All @@ -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(
<ApiProvider apis={apis}>
<EscalationPolicy policyId="abc" />
</ApiProvider>
)
);
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(
<ApiProvider apis={apis}>
<EscalationPolicy policyId="abc" />
</ApiProvider>
)
);
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(
<ApiProvider apis={apis}>
<EscalationPolicy policyId="abc" />
</ApiProvider>
)
);
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()
Expand Down
16 changes: 14 additions & 2 deletions src/components/Escalation/EscalationPolicy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down
2 changes: 1 addition & 1 deletion src/components/Escalation/EscalationUser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const EscalationUser = ({ user }: Props) => {
return (
<ListItem>
<ListItemIcon>
<Avatar alt="User" />
<Avatar alt="User" src={user.avatar_url} />
</ListItemIcon>
<ListItemText
primary={
Expand Down
1 change: 1 addition & 0 deletions src/components/PagerDutyCard/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,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 = {
Expand Down
1 change: 1 addition & 0 deletions src/components/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export type PagerDutyUser = {
email: string;
html_url: string;
name: string;
avatar_url: string;
};

/** @public */
Expand Down

0 comments on commit 2513204

Please sign in to comment.