Skip to content

Don't set custom profile fields to null when deletion is meant. #34844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
reivilibre wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Don't set custom profile fields to null when deletion is meant. #34844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions apps/web/src/utils/userStatus.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,15 @@ describe("userStatus utils", () => {

await clearAllUserStatus(client);

expect(client.setExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status", null);
expect(client.deleteExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status");
});

it("clears the call status", async () => {
vi.mocked(client.getExtendedProfileProperty).mockResolvedValue({ call_joined_ts: 12345 });

await clearAllUserStatus(client);

expect(client.setExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.call", null);
expect(client.deleteExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.call");
});
});

Expand All @@ -151,7 +151,7 @@ describe("userStatus utils", () => {
it("clears the call status if onCall is false", async () => {
await setUserOnCall(client, false);

expect(client.setExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.call", null);
expect(client.deleteExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.call");
});
});
});
15 changes: 6 additions & 9 deletions apps/web/src/utils/userStatus.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export function setUserStatus(client: MatrixClient, userStatus: UserStatus): Pro
export async function clearAllUserStatus(client: MatrixClient): Promise<void> {
const rawUserStatus = await client.getExtendedProfileProperty(client.getSafeUserId(), "org.matrix.msc4426.status");
if (rawUserStatus) {
await client.setExtendedProfileProperty("org.matrix.msc4426.status", null);
await client.deleteExtendedProfileProperty("org.matrix.msc4426.status");
}

const rawCallStatus = await client.getExtendedProfileProperty(client.getSafeUserId(), "org.matrix.msc4426.call");
Expand All @@ -165,12 +165,9 @@ export async function clearAllUserStatus(client: MatrixClient): Promise<void> {
* @param onCall Whether the user is currently on a call.
*/
export function setUserOnCall(client: MatrixClient, onCall: boolean): Promise<void> {
return client.setExtendedProfileProperty(
"org.matrix.msc4426.call",
onCall
? {
call_joined_ts: Date.now(),
}
: null,
);
if (!onCall) {
return client.deleteExtendedProfileProperty("org.matrix.msc4426.call");
}

return client.setExtendedProfileProperty("org.matrix.msc4426.call", { call_joined_ts: Date.now() });
}
3 changes: 2 additions & 1 deletion apps/web/src/viewmodels/menus/UserMenuViewModel.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe("UserMenuViewModel", () => {
getAuthMetadata: vi.fn().mockRejectedValue(new MatrixError({ errcode: "M_UNRECOGNIZED" }, 404)),
getExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
setExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
deleteExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
});
sdkContext = new TestSDKContext();
// @ts-ignore UserMenuViewModel uses SDKContext in the constructor
Expand Down Expand Up @@ -187,7 +188,7 @@ describe("UserMenuViewModel", () => {
vm.setOpen(true);
vm.clearStatus();
await waitFor(() =>
expect(client.setExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status", null),
expect(client.deleteExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status"),
);
});

Expand Down
10 changes: 6 additions & 4 deletions apps/web/src/viewmodels/status/SetStatusViewModel.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe("SetStatusViewModel", () => {
...mockClientMethodsServer(),
getExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
setExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
deleteExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
});
vi.mocked(mockOwnProfileStoreInstance).userStatus = undefined;
vi.mocked(recent.get).mockReturnValue([]);
Expand Down Expand Up @@ -159,12 +160,12 @@ describe("SetStatusViewModel", () => {
expect(vm.getSnapshot().userStatus).toBeUndefined();
});

it("calls setExtendedProfileProperty with null", async () => {
it("calls deleteExtendedProfileProperty", async () => {
client.getExtendedProfileProperty.mockResolvedValue(STATUS);
const vm = new SetStatusViewModel({ client, ownProfileStore: mockOwnProfileStoreInstance });
vm.clearStatus();
await waitFor(() =>
expect(client.setExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status", null),
expect(client.deleteExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status"),
);
});

Expand All @@ -180,7 +181,7 @@ describe("SetStatusViewModel", () => {
it("rolls back the snapshot on failure", async () => {
vi.mocked(mockOwnProfileStoreInstance).userStatus = STATUS;
client.getExtendedProfileProperty.mockResolvedValue(STATUS);
client.setExtendedProfileProperty.mockRejectedValue(new Error("network error"));
client.deleteExtendedProfileProperty.mockRejectedValue(new Error("network error"));
const vm = new SetStatusViewModel({ client, ownProfileStore: mockOwnProfileStoreInstance });
vm.clearStatus();
expect(vm.getSnapshot().userStatus).toBeUndefined();
Expand All @@ -206,6 +207,7 @@ describe("UserMenuSetStatusViewModel", () => {
...mockClientMethodsServer(),
getExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
setExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
deleteExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
});
vi.mocked(mockOwnProfileStoreInstance).userStatus = undefined;
dispatchSpy = vi.spyOn(dis, "dispatch").mockImplementation(() => {});
Expand Down Expand Up @@ -244,7 +246,7 @@ describe("UserMenuSetStatusViewModel", () => {
const vm = new UserMenuSetStatusViewModel({ client, ownProfileStore: mockOwnProfileStoreInstance });
vm.clearStatus();
await waitFor(() =>
expect(client.setExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status", null),
expect(client.deleteExtendedProfileProperty).toHaveBeenCalledWith("org.matrix.msc4426.status"),
);
});
});
1 change: 1 addition & 0 deletions apps/web/test/test-utils/test-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ export function createTestClient(): MatrixClient {
setRoomTag: vi.fn().mockResolvedValue({}),
getExtendedProfileProperty: vi.fn(),
setExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
deleteExtendedProfileProperty: vi.fn().mockResolvedValue(undefined),
doesServerSupportExtendedProfiles: vi.fn(),
} as unknown as MatrixClient;

Expand Down
Loading