Skip to content

Commit 94a9295

Browse files
authored
fix: separate draft event subscriptions and make it a public api (#1565)
## CLA - [ ] I have signed the [Stream CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform) (required). - [ ] Code changes are tested ## Description of the changes, What, Why and How? ## Changelog -
1 parent 8e226fb commit 94a9295

File tree

2 files changed

+68
-30
lines changed

2 files changed

+68
-30
lines changed

src/messageComposer/messageComposer.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,16 @@ export class MessageComposer extends WithSubscriptions {
364364
});
365365
}
366366

367+
public registerDraftEventSubscriptions = () => {
368+
const unsubscribeDraftUpdated = this.subscribeDraftUpdated();
369+
const unsubscribeDraftDeleted = this.subscribeDraftDeleted();
370+
371+
return () => {
372+
unsubscribeDraftUpdated();
373+
unsubscribeDraftDeleted();
374+
};
375+
};
376+
367377
public registerSubscriptions = (): UnregisterSubscriptions => {
368378
if (!this.hasSubscriptions) {
369379
this.addUnsubscribeFunction(this.subscribeMessageComposerSetupStateChange());
@@ -552,7 +562,7 @@ export class MessageComposer extends WithSubscriptions {
552562
});
553563

554564
private subscribeMessageComposerConfigStateChanged = () => {
555-
let draftUnsubscribeFunctions: Unsubscribe[] | null;
565+
let draftUnsubscribeFunction: Unsubscribe | null;
556566

557567
const unsubscribe = this.configState.subscribeWithSelector(
558568
(currentValue) => ({
@@ -567,20 +577,17 @@ export class MessageComposer extends WithSubscriptions {
567577
});
568578
}
569579

570-
if (draftsEnabled && !draftUnsubscribeFunctions) {
571-
draftUnsubscribeFunctions = [
572-
this.subscribeDraftUpdated(),
573-
this.subscribeDraftDeleted(),
574-
];
575-
} else if (!draftsEnabled && draftUnsubscribeFunctions) {
576-
draftUnsubscribeFunctions.forEach((fn) => fn());
577-
draftUnsubscribeFunctions = null;
580+
if (draftsEnabled && !draftUnsubscribeFunction) {
581+
draftUnsubscribeFunction = this.registerDraftEventSubscriptions();
582+
} else if (!draftsEnabled && draftUnsubscribeFunction) {
583+
draftUnsubscribeFunction();
584+
draftUnsubscribeFunction = null;
578585
}
579586
},
580587
);
581588

582589
return () => {
583-
draftUnsubscribeFunctions?.forEach((unsubscribe) => unsubscribe());
590+
draftUnsubscribeFunction?.();
584591
unsubscribe();
585592
};
586593
};

test/unit/MessageComposer/messageComposer.test.ts

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,48 @@ describe('MessageComposer', () => {
528528
expect(messageComposer.hasSubscriptions).toBe(false);
529529
});
530530

531+
it('should register draft subscriptions only once if done through registerSubscriptions', () => {
532+
const { messageComposer } = setup({
533+
config: { drafts: { enabled: true } },
534+
});
535+
536+
const draftEventSubscriptionsSpy = vi.spyOn(
537+
messageComposer,
538+
'registerDraftEventSubscriptions',
539+
);
540+
541+
messageComposer.registerSubscriptions();
542+
messageComposer.registerSubscriptions();
543+
544+
expect(draftEventSubscriptionsSpy).toHaveBeenCalledOnce();
545+
});
546+
547+
it('should allow multiple registrations of draft ws events if done through registerDraftEventSubscriptions', () => {
548+
const { messageComposer } = setup({
549+
config: { drafts: { enabled: true } },
550+
});
551+
552+
const subscribeDraftUpdatedSpy = vi
553+
// @ts-expect-error - we are testing private properties
554+
.spyOn(messageComposer, 'subscribeDraftUpdated');
555+
const subscribeDraftDeletedSpy = vi
556+
// @ts-expect-error - we are testing private properties
557+
.spyOn(messageComposer, 'subscribeDraftDeleted');
558+
559+
messageComposer.registerSubscriptions();
560+
561+
expect(subscribeDraftUpdatedSpy).toHaveBeenCalledOnce();
562+
expect(subscribeDraftDeletedSpy).toHaveBeenCalledOnce();
563+
564+
subscribeDraftUpdatedSpy.mockClear();
565+
subscribeDraftDeletedSpy.mockClear();
566+
567+
messageComposer.registerDraftEventSubscriptions();
568+
569+
expect(subscribeDraftUpdatedSpy).toHaveBeenCalledOnce();
570+
expect(subscribeDraftDeletedSpy).toHaveBeenCalledOnce();
571+
});
572+
531573
it('should set quoted message', () => {
532574
const { messageComposer } = setup();
533575
const quotedMessage = {
@@ -1302,37 +1344,26 @@ describe('MessageComposer', () => {
13021344
config: { drafts: { enabled: false } },
13031345
});
13041346

1305-
const unsubscribeDraftUpdated = vi.fn();
1306-
const unsubscribeDraftDeleted = vi.fn();
1347+
const unsubscribeDraftEvents = vi.fn();
13071348

1308-
// @ts-expect-error - we are testing private properties
1309-
const subscribeDraftUpdatedSpy = vi
1310-
.spyOn(messageComposer, 'subscribeDraftUpdated')
1311-
.mockImplementation(() => unsubscribeDraftUpdated);
1312-
// @ts-expect-error - we are testing private properties
1313-
const subscribeDraftDeletedSpy = vi
1314-
.spyOn(messageComposer, 'subscribeDraftDeleted')
1315-
.mockImplementation(() => unsubscribeDraftDeleted);
1349+
const registerDraftEventSubscriptionsSpy = vi
1350+
.spyOn(messageComposer, 'registerDraftEventSubscriptions')
1351+
.mockImplementation(() => unsubscribeDraftEvents);
13161352

13171353
messageComposer.registerSubscriptions();
13181354

1319-
expect(subscribeDraftUpdatedSpy).not.toHaveBeenCalled();
1320-
expect(subscribeDraftDeletedSpy).not.toHaveBeenCalled();
1355+
expect(registerDraftEventSubscriptionsSpy).not.toHaveBeenCalled();
13211356

13221357
messageComposer.updateConfig({ drafts: { enabled: true } });
13231358

1324-
expect(subscribeDraftUpdatedSpy).toHaveBeenCalledTimes(1);
1325-
expect(subscribeDraftDeletedSpy).toHaveBeenCalledTimes(1);
1359+
expect(registerDraftEventSubscriptionsSpy).toHaveBeenCalledTimes(1);
13261360

1327-
subscribeDraftUpdatedSpy.mockClear();
1328-
subscribeDraftDeletedSpy.mockClear();
1361+
registerDraftEventSubscriptionsSpy.mockClear();
13291362

13301363
messageComposer.updateConfig({ drafts: { enabled: false } });
13311364

1332-
expect(unsubscribeDraftUpdated).toHaveBeenCalledTimes(1);
1333-
expect(unsubscribeDraftDeleted).toHaveBeenCalledTimes(1);
1334-
expect(subscribeDraftUpdatedSpy).not.toHaveBeenCalled();
1335-
expect(subscribeDraftDeletedSpy).not.toHaveBeenCalled();
1365+
expect(unsubscribeDraftEvents).toHaveBeenCalledTimes(1);
1366+
expect(registerDraftEventSubscriptionsSpy).not.toHaveBeenCalled();
13361367
});
13371368
});
13381369
});

0 commit comments

Comments
 (0)