Skip to content

Commit 7038e2f

Browse files
AmarTrebinjacAmarTrebinjac
authored
fix: syncSubscriptions should no longer malform notificationFlags on user (#2975)
1 parent 4805eb4 commit 7038e2f

29 files changed

+1857
-244
lines changed

‎__tests__/__snapshots__/users.ts.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ Object {
5050
"infoConfirmed": false,
5151
"language": null,
5252
"name": "Ido",
53-
"notificationEmail": true,
5453
"permalink": "http://localhost:5002/aaa1",
5554
"timezone": "Europe/London",
5655
"twitter": null,

‎__tests__/boot.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,9 @@ const LOGGED_IN_BODY = {
129129
timezone: DEFAULT_TIMEZONE,
130130
reputation: 10,
131131
portfolio: null,
132-
notificationEmail: true,
133132
acceptedMarketing: false,
134133
company: null,
135134
experienceLevel: null,
136-
followNotifications: true,
137-
followingEmail: true,
138-
awardEmail: true,
139-
awardNotifications: true,
140135
isTeamMember: false,
141136
bluesky: null,
142137
roadmap: null,

‎__tests__/common/mailing.ts

Lines changed: 266 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@ import {
99
UserPersonalizedDigestType,
1010
} from '../../src/entity';
1111
import { usersFixture } from '../fixture/user';
12+
import { syncSubscription, updateFlagsStatement } from '../../src/common';
13+
import { CioUnsubscribeTopic } from '../../src/cio';
14+
import * as cio from '../../src/cio';
1215
import {
13-
CioUnsubscribeTopic,
14-
syncSubscription,
15-
updateFlagsStatement,
16-
} from '../../src/common';
16+
NotificationType,
17+
DEFAULT_NOTIFICATION_SETTINGS,
18+
NotificationPreferenceStatus,
19+
} from '../../src/notifications/common';
20+
import type { UserNotificationFlags } from '../../src/entity/user/User';
21+
import { logger } from '../../src/logger';
1722

1823
let con: DataSource;
1924

@@ -95,13 +100,10 @@ describe('mailing', () => {
95100

96101
users.forEach((user, index) => {
97102
expect(user.acceptedMarketing).toBe(false);
98-
expect(user.notificationEmail).toBe(true);
99103
expect(digests[index]).toMatchObject({
100104
userId: user.id,
101105
type: UserPersonalizedDigestType.Digest,
102106
});
103-
expect(user.followingEmail).toBe(false);
104-
expect(user.awardEmail).toBe(false);
105107
});
106108
});
107109

@@ -174,17 +176,271 @@ describe('mailing', () => {
174176

175177
users.forEach((user, index) => {
176178
expect(user.acceptedMarketing).toBe(false);
177-
expect(user.notificationEmail).toBe(true);
178179
expect(digests[index]).toMatchObject({
179180
userId: user.id,
180181
type: UserPersonalizedDigestType.Brief,
181182
flags: {
182183
sendType: UserPersonalizedDigestSendType.daily,
183184
},
184185
});
185-
expect(user.followingEmail).toBe(false);
186-
expect(user.awardEmail).toBe(false);
187186
});
188187
});
188+
189+
it('should preserve user-customized notification flags during CIO sync', async () => {
190+
const customNotificationFlags: UserNotificationFlags = {
191+
...DEFAULT_NOTIFICATION_SETTINGS,
192+
[NotificationType.ArticleNewComment]: {
193+
email: NotificationPreferenceStatus.Subscribed,
194+
inApp: NotificationPreferenceStatus.Muted,
195+
},
196+
[NotificationType.CommentReply]: {
197+
email: NotificationPreferenceStatus.Muted,
198+
inApp: NotificationPreferenceStatus.Subscribed,
199+
},
200+
[NotificationType.UserReceivedAward]: {
201+
email: NotificationPreferenceStatus.Subscribed,
202+
inApp: NotificationPreferenceStatus.Muted,
203+
},
204+
[NotificationType.DevCardUnlocked]: {
205+
email: NotificationPreferenceStatus.Muted,
206+
inApp: NotificationPreferenceStatus.Muted,
207+
},
208+
};
209+
210+
const testUserIds = usersFixture.map((user) => `mss-${user.id}`);
211+
await con
212+
.getRepository(User)
213+
.update(
214+
{ id: In(testUserIds) },
215+
{ notificationFlags: customNotificationFlags },
216+
);
217+
218+
let users = await con.getRepository(User).find({
219+
where: { id: In(testUserIds) },
220+
select: ['id', 'notificationFlags'],
221+
});
222+
223+
nock(`https://api.customer.io`)
224+
.post('/v1/customers/attributes', {
225+
ids: testUserIds,
226+
})
227+
.reply(200, {
228+
customers: users.map((user) => ({
229+
id: user.id,
230+
attributes: {
231+
cio_subscription_preferences: JSON.stringify({
232+
topics: {
233+
[`topic_${CioUnsubscribeTopic.CommentsOnPost}`]: false, // article_new_comment email -> muted
234+
[`topic_${CioUnsubscribeTopic.CommentReply}`]: true, // comment_reply email -> subscribed
235+
[`topic_${CioUnsubscribeTopic.UserReceivedAward}`]: false, // user_received_award email -> muted
236+
[`topic_${CioUnsubscribeTopic.Marketing}`]: true,
237+
[`topic_${CioUnsubscribeTopic.Digest}`]: true,
238+
[`topic_${CioUnsubscribeTopic.Notifications}`]: true,
239+
[`topic_${CioUnsubscribeTopic.Follow}`]: true,
240+
[`topic_${CioUnsubscribeTopic.Award}`]: true,
241+
},
242+
}),
243+
},
244+
unsubscribed: false,
245+
})),
246+
});
247+
248+
await syncSubscription(testUserIds, con);
249+
250+
users = await con.getRepository(User).find({
251+
where: { id: In(testUserIds) },
252+
select: ['id', 'notificationFlags'],
253+
order: { id: 'ASC' },
254+
});
255+
256+
users.forEach((user) => {
257+
const flags = user.notificationFlags;
258+
259+
expect(flags[NotificationType.ArticleNewComment].email).toBe(
260+
NotificationPreferenceStatus.Muted,
261+
);
262+
expect(flags[NotificationType.CommentReply].email).toBe(
263+
NotificationPreferenceStatus.Subscribed,
264+
);
265+
expect(flags[NotificationType.UserReceivedAward].email).toBe(
266+
NotificationPreferenceStatus.Muted,
267+
);
268+
269+
expect(flags[NotificationType.ArticleNewComment].inApp).toBe(
270+
NotificationPreferenceStatus.Muted,
271+
);
272+
expect(flags[NotificationType.CommentReply].inApp).toBe(
273+
NotificationPreferenceStatus.Subscribed,
274+
);
275+
expect(flags[NotificationType.UserReceivedAward].inApp).toBe(
276+
NotificationPreferenceStatus.Muted,
277+
);
278+
279+
expect(flags[NotificationType.DevCardUnlocked]).toEqual({
280+
email: NotificationPreferenceStatus.Muted,
281+
inApp: NotificationPreferenceStatus.Muted,
282+
});
283+
284+
// All notification types should have both email and inApp properties.
285+
Object.entries(flags).forEach(([, preferences]) => {
286+
expect(preferences).toHaveProperty('email');
287+
expect(preferences).toHaveProperty('inApp');
288+
expect([
289+
NotificationPreferenceStatus.Muted,
290+
NotificationPreferenceStatus.Subscribed,
291+
]).toContain(preferences.email);
292+
expect([
293+
NotificationPreferenceStatus.Muted,
294+
NotificationPreferenceStatus.Subscribed,
295+
]).toContain(preferences.inApp);
296+
});
297+
});
298+
});
299+
300+
it('should handle validation failures gracefully during CIO sync', async () => {
301+
const testUserId = `mss-${usersFixture[0].id}`;
302+
await con.getRepository(User).update(
303+
{ id: testUserId },
304+
{
305+
notificationFlags: DEFAULT_NOTIFICATION_SETTINGS,
306+
acceptedMarketing: false,
307+
},
308+
);
309+
310+
const loggerSpy = jest
311+
.spyOn(logger, 'error')
312+
.mockImplementation(() => {});
313+
314+
const mockGetCioTopicsToNotificationFlags = jest
315+
.spyOn(cio, 'getCioTopicsToNotificationFlags')
316+
.mockReturnValue({
317+
// Return invalid data that will fail Zod validation
318+
[NotificationType.ArticleNewComment]: {
319+
email: 'muted',
320+
// Missing inApp field
321+
},
322+
// Missing other required notification types
323+
});
324+
325+
nock(`https://api.customer.io`)
326+
.post('/v1/customers/attributes', {
327+
ids: [testUserId],
328+
})
329+
.reply(200, {
330+
customers: [
331+
{
332+
id: testUserId,
333+
attributes: {
334+
cio_subscription_preferences: JSON.stringify({
335+
topics: {
336+
[`topic_${CioUnsubscribeTopic.CommentsOnPost}`]: false,
337+
},
338+
}),
339+
},
340+
unsubscribed: false,
341+
},
342+
],
343+
});
344+
345+
await syncSubscription([testUserId], con);
346+
347+
expect(loggerSpy).toHaveBeenCalledWith(
348+
expect.objectContaining({
349+
userId: testUserId,
350+
errors: expect.any(Array),
351+
flags: expect.any(Object),
352+
}),
353+
'Failed to validate merged notification flags from CIO sync, skipping notification flags update',
354+
);
355+
356+
const user = await con.getRepository(User).findOne({
357+
where: { id: testUserId },
358+
select: ['notificationFlags', 'acceptedMarketing'],
359+
});
360+
361+
// notificationFlags should remain unchanged due to validation failure
362+
expect(user.notificationFlags).toEqual(DEFAULT_NOTIFICATION_SETTINGS);
363+
364+
expect(user.acceptedMarketing).toBe(true); // Updated from CIO
365+
366+
mockGetCioTopicsToNotificationFlags.mockRestore();
367+
loggerSpy.mockRestore();
368+
});
369+
370+
it('should handle users with incomplete notification flags during CIO sync', async () => {
371+
const incompleteFlags: Partial<UserNotificationFlags> = {
372+
[NotificationType.ArticleNewComment]: {
373+
email: NotificationPreferenceStatus.Subscribed,
374+
inApp: NotificationPreferenceStatus.Subscribed,
375+
},
376+
[NotificationType.CommentReply]: {
377+
email: NotificationPreferenceStatus.Muted,
378+
inApp: NotificationPreferenceStatus.Subscribed,
379+
},
380+
// Missing many other notification types that exist in DEFAULT_NOTIFICATION_SETTINGS
381+
};
382+
383+
const testUserId = `mss-${usersFixture[0].id}`;
384+
await con
385+
.getRepository(User)
386+
.update(
387+
{ id: testUserId },
388+
{ notificationFlags: incompleteFlags as UserNotificationFlags },
389+
);
390+
391+
nock(`https://api.customer.io`)
392+
.post('/v1/customers/attributes', {
393+
ids: [testUserId],
394+
})
395+
.reply(200, {
396+
customers: [
397+
{
398+
id: testUserId,
399+
attributes: {
400+
cio_subscription_preferences: JSON.stringify({
401+
topics: {
402+
[`topic_${CioUnsubscribeTopic.CommentsOnPost}`]: false,
403+
[`topic_${CioUnsubscribeTopic.CommentReply}`]: true,
404+
[`topic_${CioUnsubscribeTopic.UserReceivedAward}`]: false,
405+
},
406+
}),
407+
},
408+
unsubscribed: false,
409+
},
410+
],
411+
});
412+
413+
await syncSubscription([testUserId], con);
414+
415+
const user = await con.getRepository(User).findOne({
416+
where: { id: testUserId },
417+
select: ['notificationFlags'],
418+
});
419+
420+
const flags = user.notificationFlags;
421+
422+
expect(flags[NotificationType.ArticleNewComment].email).toBe(
423+
NotificationPreferenceStatus.Muted,
424+
);
425+
expect(flags[NotificationType.ArticleNewComment].inApp).toBe(
426+
NotificationPreferenceStatus.Subscribed,
427+
);
428+
expect(flags[NotificationType.CommentReply].email).toBe(
429+
NotificationPreferenceStatus.Subscribed,
430+
);
431+
expect(flags[NotificationType.CommentReply].inApp).toBe(
432+
NotificationPreferenceStatus.Subscribed,
433+
);
434+
435+
expect(flags[NotificationType.UserReceivedAward]).toEqual({
436+
email: NotificationPreferenceStatus.Muted,
437+
inApp: NotificationPreferenceStatus.Subscribed,
438+
});
439+
440+
expect(Object.keys(flags).length).toBeGreaterThan(2);
441+
expect(flags).toHaveProperty(NotificationType.ArticleNewComment);
442+
expect(flags).toHaveProperty(NotificationType.CommentReply);
443+
expect(flags).toHaveProperty(NotificationType.UserReceivedAward);
444+
});
189445
});
190446
});

0 commit comments

Comments
 (0)