Skip to content

Commit 757daab

Browse files
turegjorupclaude
andcommitted
6871: Fixed unhandled promise rejections in CampaignsButton onClick
Inner promises in the onClick chain were not returned, so rejections from getAllScreenGroupCampaigns, getAllPages (screen campaigns), or getAllCampaigns never reached the outer .catch() handler — leaving the button stuck in a loading state permanently on network errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 679147a commit 757daab

2 files changed

Lines changed: 144 additions & 3 deletions

File tree

assets/admin/components/screen/util/campaigns-button.jsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ function CampaignsButton({ screen }) {
7171
.filter(({ campaignsLength }) => campaignsLength > 0)
7272
.map((group) => idFromUrl(group["@id"]));
7373

74-
getAllScreenGroupCampaigns(dispatch, screenGroupIds).then(
74+
return getAllScreenGroupCampaigns(dispatch, screenGroupIds).then(
7575
(screenGroupCampaigns) => {
76-
getAllPages(
76+
return getAllPages(
7777
dispatch,
7878
enhancedApi.endpoints.getV2ScreensByIdCampaigns,
7979
{ id: screen.id },
@@ -91,7 +91,7 @@ function CampaignsButton({ screen }) {
9191
!ids.has(campaign["@id"]) && ids.add(campaign["@id"]),
9292
);
9393

94-
getAllCampaigns(
94+
return getAllCampaigns(
9595
dispatch,
9696
uniqueCampaigns.map((campaign) => idFromUrl(campaign["@id"])),
9797
).then((allCampaigns) => {
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import { test, expect } from "@playwright/test";
2+
3+
/**
4+
* Regression tests for the CampaignsButton.onClick promise chain.
5+
*
6+
* The onClick handler chains nested .then() calls. Inner promises must be
7+
* returned so that rejections propagate to the outer .catch() and
8+
* setLoading(false) is always called. Without the returns, a rejection in
9+
* any inner call (getAllScreenGroupCampaigns, second getAllPages,
10+
* getAllCampaigns) leaves the button in a permanent loading state.
11+
*
12+
* These tests replicate the promise structure from campaigns-button.jsx
13+
* with controllable mocks.
14+
*/
15+
16+
/**
17+
* Replicates the onClick promise chain from CampaignsButton.
18+
* Inner promises are returned so rejections propagate to the outer .catch().
19+
*/
20+
function onClick({
21+
getAllPagesScreenGroups,
22+
getAllScreenGroupCampaigns,
23+
getAllPagesScreenCampaigns,
24+
getAllCampaigns,
25+
setLoading,
26+
setCampaigns,
27+
}) {
28+
setLoading(true);
29+
30+
getAllPagesScreenGroups()
31+
.then((screenGroups) => {
32+
const screenGroupIds = screenGroups
33+
.filter(({ campaignsLength }) => campaignsLength > 0)
34+
.map((group) => group.id);
35+
36+
return getAllScreenGroupCampaigns(screenGroupIds).then(
37+
(screenGroupCampaigns) => {
38+
return getAllPagesScreenCampaigns().then((screenCampaigns) => {
39+
const campaignIds = [
40+
...screenGroupCampaigns,
41+
...screenCampaigns,
42+
].map((c) => c.id);
43+
44+
return getAllCampaigns(campaignIds).then((allCampaigns) => {
45+
setCampaigns(allCampaigns);
46+
setLoading(false);
47+
});
48+
});
49+
},
50+
);
51+
})
52+
.catch(() => setLoading(false));
53+
}
54+
55+
function createMocks({ failAt } = {}) {
56+
const state = { loading: false, campaigns: [] };
57+
58+
return {
59+
state,
60+
setLoading: (v) => {
61+
state.loading = v;
62+
},
63+
setCampaigns: (v) => {
64+
state.campaigns = v;
65+
},
66+
getAllPagesScreenGroups:
67+
failAt === "screenGroups"
68+
? () => Promise.reject(new Error("screenGroups failed"))
69+
: () =>
70+
Promise.resolve([
71+
{ id: "group1", campaignsLength: 2, "@id": "/v2/groups/group1" },
72+
]),
73+
getAllScreenGroupCampaigns:
74+
failAt === "screenGroupCampaigns"
75+
? () => Promise.reject(new Error("screenGroupCampaigns failed"))
76+
: () =>
77+
Promise.resolve([
78+
{ id: "campaign1", campaign: { "@id": "/v2/playlists/c1" } },
79+
]),
80+
getAllPagesScreenCampaigns:
81+
failAt === "screenCampaigns"
82+
? () => Promise.reject(new Error("screenCampaigns failed"))
83+
: () =>
84+
Promise.resolve([
85+
{ id: "campaign2", campaign: { "@id": "/v2/playlists/c2" } },
86+
]),
87+
getAllCampaigns:
88+
failAt === "allCampaigns"
89+
? () => Promise.reject(new Error("allCampaigns failed"))
90+
: (ids) =>
91+
Promise.resolve(ids.map((id) => ({ "@id": id, title: id }))),
92+
};
93+
}
94+
95+
test.describe("CampaignsButton onClick promise chain", () => {
96+
test("happy path resolves and clears loading", async () => {
97+
const mocks = createMocks();
98+
onClick(mocks);
99+
100+
await new Promise((resolve) => setTimeout(resolve, 50));
101+
102+
expect(mocks.state.loading).toBe(false);
103+
expect(mocks.state.campaigns.length).toBeGreaterThan(0);
104+
});
105+
106+
test("rejection in getAllPagesScreenGroups clears loading", async () => {
107+
const mocks = createMocks({ failAt: "screenGroups" });
108+
onClick(mocks);
109+
110+
await new Promise((resolve) => setTimeout(resolve, 50));
111+
112+
expect(mocks.state.loading).toBe(false);
113+
});
114+
115+
test("rejection in getAllScreenGroupCampaigns clears loading", async () => {
116+
const mocks = createMocks({ failAt: "screenGroupCampaigns" });
117+
onClick(mocks);
118+
119+
await new Promise((resolve) => setTimeout(resolve, 50));
120+
121+
expect(mocks.state.loading).toBe(false);
122+
});
123+
124+
test("rejection in getAllPagesScreenCampaigns clears loading", async () => {
125+
const mocks = createMocks({ failAt: "screenCampaigns" });
126+
onClick(mocks);
127+
128+
await new Promise((resolve) => setTimeout(resolve, 50));
129+
130+
expect(mocks.state.loading).toBe(false);
131+
});
132+
133+
test("rejection in getAllCampaigns clears loading", async () => {
134+
const mocks = createMocks({ failAt: "allCampaigns" });
135+
onClick(mocks);
136+
137+
await new Promise((resolve) => setTimeout(resolve, 50));
138+
139+
expect(mocks.state.loading).toBe(false);
140+
});
141+
});

0 commit comments

Comments
 (0)