Skip to content

Commit c4eb920

Browse files
gismyalucaasffMathy
authored
fix: publishReply has incorrect Event in reply (#141)
* fix: publishReply has incorrect Event in reply * Corrected source preparation * Apply suggestions from code review Co-authored-by: Lucas Stålner Correia <lucas.correia@ftrack.com> * PR suggestions * feat(TS): Changed tests to TypeScript and updated typings (#136) * Changed tests to TypeScript and updated typings. * fix: review changes * feat(TS): Changed tests to TypeScript and updated typings (#136) * Changed tests to TypeScript and updated typings. * fix: review changes * Empty-merge * Event dropped out in merges --------- Co-authored-by: Lucas Stålner Correia <lucas.correia@ftrack.com> Co-authored-by: Mathias Lykkegaard Lorenzen <mathias.lykkegaard.lorenzen@lego.com>
1 parent 8dddf76 commit c4eb920

File tree

4 files changed

+141
-7
lines changed

4 files changed

+141
-7
lines changed

source/event.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export class Event {
2424
constructor(
2525
topic: string,
2626
data: object,
27-
options: { [key: string]: object } = {}
27+
options: { [key: string]: unknown } = {}
2828
) {
2929
this._data = {
3030
topic,
@@ -41,7 +41,15 @@ export class Event {
4141
return this._data;
4242
}
4343

44-
/** Add source to event data. */
44+
/** Add source to event data, keeping any already avalable source information */
45+
prepareSource(source: object): void {
46+
this._data.source = {
47+
...source,
48+
...this._data.source,
49+
};
50+
}
51+
52+
/** Add source to event data, replacing any previous data. */
4553
addSource(source: any): void {
4654
this._data.source = source;
4755
}

source/event_hub.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ export class EventHub {
271271
);
272272
}
273273

274-
event.addSource({
274+
event.prepareSource({
275275
id: this._id,
276276
applicationId: this._applicationId,
277277
user: {
@@ -613,13 +613,11 @@ export class EventHub {
613613
data: Data,
614614
source: Data | null = null
615615
): Promise<string> {
616-
const replyEvent = new Event("ftrack.meta.reply", {
617-
...data,
616+
const replyEvent = new Event("ftrack.meta.reply", data, {
618617
target: `id=${sourceEventPayload.source.id}`,
619618
inReplyToEvent: sourceEventPayload.id,
620-
source: source ?? data.source,
619+
source: source,
621620
});
622-
623621
return this.publish(replyEvent);
624622
}
625623
}

test/event.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// :copyright: Copyright (c) 2023 ftrack
2+
3+
import { Event } from "../source/event";
4+
import { describe, expect, it } from "vitest";
5+
6+
describe("Event class", () => {
7+
it("should initialize with correct topic and data", () => {
8+
const event = new Event("testTopic", { key: "value" });
9+
const data = event.getData();
10+
expect(data.topic).toBe("testTopic");
11+
expect(data.data).toEqual({ key: "value" });
12+
});
13+
14+
it("should have default properties", () => {
15+
const event = new Event("testTopic", { key: "value" });
16+
const data = event.getData();
17+
expect(data.target).toBe("");
18+
expect(data.inReplyToEvent).toBeNull();
19+
});
20+
21+
it("should set properties from options", () => {
22+
const event = new Event(
23+
"testTopic",
24+
{ key: "value" },
25+
{ target: "sampleTarget", customOption: "customValue" }
26+
);
27+
const data = event.getData();
28+
expect(data.target).toBe("sampleTarget");
29+
expect(data.customOption).toBe("customValue");
30+
});
31+
32+
it("should generate unique UUID", () => {
33+
const event1 = new Event("testTopic", { key: "value" });
34+
const event2 = new Event("testTopic", { key: "value" });
35+
const data1 = event1.getData();
36+
const data2 = event2.getData();
37+
const uuidRegexExp =
38+
/^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/gi;
39+
expect(data1.id).not.toBe(data2.id);
40+
expect(uuidRegexExp.test(data1.id)).toBe(true);
41+
});
42+
43+
it("should add source to event data", () => {
44+
const event = new Event("testTopic", { key: "value" });
45+
const source = {
46+
id: "testId",
47+
applicationId: "testApplicationId",
48+
user: {
49+
username: "testUser",
50+
},
51+
};
52+
event.addSource(source);
53+
const data = event.getData();
54+
expect(data.source).toBe(source);
55+
});
56+
describe("prepareSource Method", () => {
57+
it("should prepare and add source to event data", () => {
58+
const event = new Event("testTopic", { key: "value" });
59+
event.prepareSource({ newKey: "newValue" });
60+
const data = event.getData();
61+
expect(data.source).toEqual({ newKey: "newValue" });
62+
});
63+
64+
it("should not override existing source when preparing a new source", () => {
65+
const event = new Event(
66+
"testTopic",
67+
{ key: "value" },
68+
{ source: { oldKey: "oldValue" } }
69+
);
70+
event.prepareSource({ oldKey: "newValue", newKey: "newValue" });
71+
const data = event.getData();
72+
expect(data.source).toEqual({
73+
oldKey: "oldValue",
74+
newKey: "newValue",
75+
});
76+
});
77+
it("should handle source undefined", () => {
78+
const event = new Event(
79+
"testTopic",
80+
{ key: "value" },
81+
{ source: undefined }
82+
);
83+
event.prepareSource({ newKey: "newValue" });
84+
const data = event.getData();
85+
expect(data.source).toEqual({
86+
newKey: "newValue",
87+
});
88+
});
89+
it("Prepare should handle source null", () => {
90+
const event = new Event("testTopic", { key: "value" }, { source: null });
91+
event.prepareSource({ newKey: "newValue" });
92+
const data = event.getData();
93+
expect(data.source).toEqual({
94+
newKey: "newValue",
95+
});
96+
});
97+
});
98+
});

test/event_hub.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { EventHub } from "../source/event_hub";
2+
import { Event } from "../source/event";
23
import { vi, describe, expect, beforeEach, afterEach, test } from "vitest";
34

45
describe("EventHub", () => {
@@ -208,4 +209,33 @@ describe("EventHub", () => {
208209
);
209210
publishReplySpy.mockRestore();
210211
});
212+
test("publishReply published Event with correct shape", async () => {
213+
eventHub.publish = vi.fn();
214+
215+
const sourceEventPayload = {
216+
source: { id: "testId" },
217+
id: "anotherTestId",
218+
};
219+
220+
const data = {
221+
someData: "value",
222+
};
223+
224+
await eventHub.publishReply(sourceEventPayload, data);
225+
226+
const publishedEvent = eventHub.publish.mock.calls[0][0];
227+
expect(publishedEvent).toBeInstanceOf(Event);
228+
const EventData = publishedEvent.getData();
229+
// Ignoring the id field for comparison
230+
delete EventData.id;
231+
232+
const expectedEvent = {
233+
topic: "ftrack.meta.reply",
234+
data: { someData: "value" },
235+
source: null,
236+
target: "id=testId",
237+
inReplyToEvent: "anotherTestId",
238+
};
239+
expect(EventData).toEqual(expectedEvent);
240+
});
211241
});

0 commit comments

Comments
 (0)