Skip to content

Commit 7a7f674

Browse files
Aryan-Verma-999Aryan-Verma-999
committed
refactor(api): address PR review comments
1 parent b592ffb commit 7a7f674

4 files changed

Lines changed: 50 additions & 79 deletions

File tree

‎packages/api/src/EmbeddedChatApi.ts‎

Lines changed: 50 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export default class EmbeddedChatApi {
2222
auth: RocketChatAuth;
2323
private _connectPromise: Promise<void> | null = null;
2424
private _activeSubscriptions: { stop: () => void }[] = [];
25+
private _authListener: ((user: any) => void) | null = null;
2526

2627
constructor(
2728
host: string,
@@ -46,12 +47,12 @@ export default class EmbeddedChatApi {
4647
getToken,
4748
saveToken,
4849
});
50+
this._registerAuthListener();
4951
}
5052

51-
private async _syncRestCredentials() {
52-
const user = (await this.auth.getCurrentUser()) || {};
53-
const userId = user.userId || user.data?.userId;
54-
const authToken = user.authToken || user.data?.authToken;
53+
private _applyCredentials(user: any) {
54+
const userId = user?.userId || user?.data?.userId;
55+
const authToken = user?.authToken || user?.data?.authToken;
5556
if (userId && authToken) {
5657
this.sdk.rest.setCredentials({
5758
"X-User-Id": userId,
@@ -60,12 +61,20 @@ export default class EmbeddedChatApi {
6061
}
6162
}
6263

64+
private _registerAuthListener() {
65+
this._authListener = (user: any) => {
66+
if (user) {
67+
this._applyCredentials(user);
68+
}
69+
};
70+
this.auth.onAuthChange(this._authListener);
71+
}
72+
6373
private async _restRequest(
6474
endpoint: string,
6575
method: "GET" | "POST" | "PUT" | "DELETE" = "GET",
6676
body?: any
6777
) {
68-
await this._syncRestCredentials();
6978
const options: RequestInit = {};
7079
if (body !== undefined) {
7180
options.headers = { "Content-Type": "application/json" };
@@ -76,15 +85,19 @@ export default class EmbeddedChatApi {
7685
}
7786

7887
private async _restUpload(endpoint: string, formData: FormData) {
79-
await this._syncRestCredentials();
8088
const response = await this.sdk.rest.send(endpoint, "POST", {
8189
body: formData,
8290
});
8391
return response.json();
8492
}
8593

8694
setAuth(auth: RocketChatAuth) {
95+
// Remove listener from the old auth instance before swapping.
96+
if (this._authListener) {
97+
this.auth.removeAuthListener(this._authListener);
98+
}
8799
this.auth = auth;
100+
this._registerAuthListener();
88101
}
89102

90103
getAuth() {
@@ -253,12 +266,6 @@ export default class EmbeddedChatApi {
253266
this.close(); // before connection, all previous subscriptions should be cancelled
254267
await this.sdk.connection.connect();
255268

256-
// Sync REST credentials first so HTTP headers are ready before any request fires
257-
await this._syncRestCredentials();
258-
259-
// Extract token from either flat or nested currentUser shape:
260-
// - auto-login (resume token): currentUser = { userId, authToken, me }
261-
// - password login: currentUser = { status, data: { userId, authToken, me } }
262269
const currentUser = (await this.auth.getCurrentUser()) as any;
263270
const token = currentUser?.authToken || currentUser?.data?.authToken;
264271
if (token) {
@@ -472,8 +479,7 @@ export default class EmbeddedChatApi {
472479
"/v1/users.getUsernameSuggestion"
473480
);
474481
if (suggestedUsername.success) {
475-
await this._syncRestCredentials();
476-
return await this.sdk.rest.post("/v1/users.update", {
482+
return await this._restRequest("/v1/users.update", "POST", {
477483
userId: userid,
478484
data: { username: suggestedUsername.result },
479485
});
@@ -490,12 +496,10 @@ export default class EmbeddedChatApi {
490496

491497
if (usernameRegExp.test(newUserName)) {
492498
try {
493-
await this._syncRestCredentials();
494-
const result = await this.sdk.rest.post("/v1/users.update", {
499+
return await this._restRequest("/v1/users.update", "POST", {
495500
userId: userid,
496501
data: { username: newUserName },
497502
});
498-
return result;
499503
} catch (err: any) {
500504
if (err?.errorType === "error-could-not-save-identity") {
501505
return await this.updateUserNameThroughSuggestion(userid);
@@ -509,8 +513,7 @@ export default class EmbeddedChatApi {
509513

510514
async channelInfo(): Promise<any> {
511515
try {
512-
await this._syncRestCredentials();
513-
return await this.sdk.rest.get("/v1/rooms.info", { roomId: this.rid });
516+
return await this._restRequest(`/v1/rooms.info?roomId=${this.rid}`);
514517
} catch (err: any) {
515518
console.error(err instanceof Error ? err.message : err);
516519
return err;
@@ -614,8 +617,7 @@ export default class EmbeddedChatApi {
614617
isChannelPrivate = false
615618
): Promise<any> {
616619
try {
617-
await this._syncRestCredentials();
618-
return await this.sdk.rest.get("/v1/chat.getThreadMessages", { tmid });
620+
return await this._restRequest(`/v1/chat.getThreadMessages?tmid=${tmid}`);
619621
} catch (err: any) {
620622
console.error(err instanceof Error ? err.message : String(err));
621623
return err;
@@ -682,8 +684,7 @@ export default class EmbeddedChatApi {
682684
messageObj.tmid = threadId;
683685
}
684686
try {
685-
await this._syncRestCredentials();
686-
return await this.sdk.rest.post("/v1/chat.sendMessage", {
687+
return await this._restRequest("/v1/chat.sendMessage", "POST", {
687688
message: messageObj,
688689
});
689690
} catch (err: any) {
@@ -694,8 +695,7 @@ export default class EmbeddedChatApi {
694695

695696
async deleteMessage(msgId: string): Promise<any> {
696697
try {
697-
await this._syncRestCredentials();
698-
return await this.sdk.rest.post("/v1/chat.delete", {
698+
return await this._restRequest("/v1/chat.delete", "POST", {
699699
roomId: this.rid,
700700
msgId,
701701
});
@@ -707,8 +707,7 @@ export default class EmbeddedChatApi {
707707

708708
async updateMessage(msgId: string, text: string): Promise<any> {
709709
try {
710-
await this._syncRestCredentials();
711-
return await this.sdk.rest.post("/v1/chat.update", {
710+
return await this._restRequest("/v1/chat.update", "POST", {
712711
roomId: this.rid,
713712
msgId,
714713
text,
@@ -735,8 +734,7 @@ export default class EmbeddedChatApi {
735734

736735
async getAllImages(): Promise<any> {
737736
try {
738-
await this._syncRestCredentials();
739-
return await this.sdk.rest.get("/v1/rooms.images", { roomId: this.rid });
737+
return await this._restRequest(`/v1/rooms.images?roomId=${this.rid}`);
740738
} catch (err: any) {
741739
console.error(err instanceof Error ? err.message : err);
742740
return err;
@@ -767,10 +765,9 @@ export default class EmbeddedChatApi {
767765

768766
async getStarredMessages(): Promise<any> {
769767
try {
770-
await this._syncRestCredentials();
771-
return await this.sdk.rest.get("/v1/chat.getStarredMessages", {
772-
roomId: this.rid,
773-
});
768+
return await this._restRequest(
769+
`/v1/chat.getStarredMessages?roomId=${this.rid}`
770+
);
774771
} catch (err: any) {
775772
console.error(err instanceof Error ? err.message : err);
776773
return err;
@@ -779,10 +776,9 @@ export default class EmbeddedChatApi {
779776

780777
async getPinnedMessages(): Promise<any> {
781778
try {
782-
await this._syncRestCredentials();
783-
return await this.sdk.rest.get("/v1/chat.getPinnedMessages", {
784-
roomId: this.rid,
785-
});
779+
return await this._restRequest(
780+
`/v1/chat.getPinnedMessages?roomId=${this.rid}`
781+
);
786782
} catch (err: any) {
787783
console.error(err instanceof Error ? err.message : err);
788784
return err;
@@ -791,10 +787,9 @@ export default class EmbeddedChatApi {
791787

792788
async getMentionedMessages(): Promise<any> {
793789
try {
794-
await this._syncRestCredentials();
795-
return await this.sdk.rest.get("/v1/chat.getMentionedMessages", {
796-
roomId: this.rid,
797-
});
790+
return await this._restRequest(
791+
`/v1/chat.getMentionedMessages?roomId=${this.rid}`
792+
);
798793
} catch (err: any) {
799794
console.error(err instanceof Error ? err.message : err);
800795
return err;
@@ -828,8 +823,7 @@ export default class EmbeddedChatApi {
828823
shouldReact: boolean | string
829824
): Promise<any> {
830825
try {
831-
await this._syncRestCredentials();
832-
return await this.sdk.rest.post("/v1/chat.react", {
826+
return await this._restRequest("/v1/chat.react", "POST", {
833827
messageId,
834828
emoji,
835829
shouldReact:
@@ -845,8 +839,7 @@ export default class EmbeddedChatApi {
845839

846840
async reportMessage(messageId: string, description: string): Promise<any> {
847841
try {
848-
await this._syncRestCredentials();
849-
return await this.sdk.rest.post("/v1/chat.reportMessage", {
842+
return await this._restRequest("/v1/chat.reportMessage", "POST", {
850843
messageId,
851844
description,
852845
});
@@ -858,8 +851,7 @@ export default class EmbeddedChatApi {
858851

859852
async findOrCreateInvite(): Promise<any> {
860853
try {
861-
await this._syncRestCredentials();
862-
return await this.sdk.rest.post("/v1/findOrCreateInvite", {
854+
return await this._restRequest("/v1/findOrCreateInvite", "POST", {
863855
rid: this.rid,
864856
days: 1,
865857
maxUses: 10,
@@ -910,8 +902,7 @@ export default class EmbeddedChatApi {
910902

911903
async me(): Promise<any> {
912904
try {
913-
await this._syncRestCredentials();
914-
return await this.sdk.rest.get("/v1/me");
905+
return await this._restRequest("/v1/me");
915906
} catch (err: any) {
916907
console.error(err instanceof Error ? err.message : err);
917908
return err;
@@ -932,11 +923,11 @@ export default class EmbeddedChatApi {
932923

933924
async getSearchMessages(text: string): Promise<any> {
934925
try {
935-
await this._syncRestCredentials();
936-
return await this.sdk.rest.get("/v1/chat.search", {
937-
roomId: this.rid,
938-
searchText: text,
939-
});
926+
return await this._restRequest(
927+
`/v1/chat.search?roomId=${this.rid}&searchText=${encodeURIComponent(
928+
text
929+
)}`
930+
);
940931
} catch (err: any) {
941932
console.error(err instanceof Error ? err.message : err);
942933
return err;
@@ -980,8 +971,7 @@ export default class EmbeddedChatApi {
980971
params: string;
981972
tmid?: string;
982973
}) {
983-
await this._syncRestCredentials();
984-
return await this.sdk.rest.post("/v1/commands.run", {
974+
return await this._restRequest("/v1/commands.run", "POST", {
985975
command,
986976
params,
987977
tmid,
@@ -995,16 +985,12 @@ export default class EmbeddedChatApi {
995985
}
996986

997987
async userInfo(reqUserId: string): Promise<any> {
998-
await this._syncRestCredentials();
999-
return await this.sdk.rest.get("/v1/users.info", {
1000-
userId: reqUserId,
1001-
});
988+
return await this._restRequest(`/v1/users.info?userId=${reqUserId}`);
1002989
}
1003990

1004991
async userData(username: string): Promise<any> {
1005-
await this._syncRestCredentials();
1006-
return await this.sdk.rest.get("/v1/users.info", {
1007-
username,
1008-
});
992+
return await this._restRequest(
993+
`/v1/users.info?username=${encodeURIComponent(username)}`
994+
);
1009995
}
1010996
}

‎packages/auth/src/RocketChatAuth.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,6 @@ class RocketChatAuth {
7474
}
7575
);
7676
this.setUser(response.data);
77-
// Note: setUser → save() already calls notifyAuthListeners().
78-
// Do NOT call it again here — a second notification would trigger
79-
// a second connect() after the first resolves, causing close() to
80-
// kill the active DDP session and all subscriptions.
8177
return this.currentUser;
8278
}
8379

@@ -98,7 +94,6 @@ class RocketChatAuth {
9894
credentials
9995
);
10096
this.setUser(response.data);
101-
// setUser → save() already calls notifyAuthListeners().
10297
return this.currentUser;
10398
}
10499

‎packages/react/src/hooks/useRCAuth.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,6 @@ export const useRCAuth = () => {
5050

5151
if (res.status === 'success') {
5252
setIsLoginModalOpen(false);
53-
// Do NOT call setIsUserAuthenticated(true) here.
54-
// EmbeddedChat.js's handleAuthChange listener fires once the auth
55-
// token is persisted and calls RCInstance.connect(), which syncs
56-
// REST credentials and re-establishes the DDP session before
57-
// setting isUserAuthenticated=true. Calling it here races with
58-
// connect() and causes all initial data fetches to fire with no
59-
// auth headers, resulting in 401s.
6053
setIsTotpModalOpen(false);
6154
setEmailorUser(null);
6255
setPassword(null);

‎packages/react/src/views/EmbeddedChat.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,6 @@ const EmbeddedChat = (props) => {
169169
RCInstance.connect()
170170
.then(() => {
171171
console.log(`Connected to RocketChat ${RCInstance.host}`);
172-
// currentUser shape differs by login method:
173-
// - resume/OAuth: { userId, authToken, me: {...} }
174-
// - password: { status, data: { userId, authToken, me: {...} } }
175172
const me = user.me || user.data?.me;
176173
if (me) {
177174
setAuthenticatedAvatarUrl(me.avatarUrl);

0 commit comments

Comments
 (0)