Skip to content
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.

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

chore: Revert "feat: Handle deleted user names appropriately (WPB-14492)" #18537

Merged
merged 1 commit into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion src/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,6 @@
"dataSharingModalDecline": "Decline",
"dataSharingModalDescription": "Help to improve Wire by sharing your usage data via a pseudonymous ID. The data is neither linked to your personal information nor shared with third parties besides Wire Group. It includes, for example, when you use a feature, your app version, device type, or your operating system. This data will be deleted at the latest after 365 days. <br /> Find further details in our [link]Privacy Policy[/link]. You can revoke your consent at any time.",
"dataSharingModalTitle": "Consent to share user data",
"deletedUser": "Deleted User",
"downloadLatestMLS": "Download the latest MLS Wire version",
"enumerationAnd": ", and ",
"ephemeralRemaining": "remaining",
Expand Down
16 changes: 4 additions & 12 deletions src/script/user/UserRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ describe('UserRepository', () => {
beforeEach(async () => {
[userRepository, {userState, userService}] = await buildUserRepository();
jest.resetAllMocks();
jest.spyOn(userService, 'loadUsersFromDb').mockResolvedValue(localUsers);
jest.spyOn(userService, 'loadUserFromDb').mockResolvedValue(localUsers);
const selfUser = new User('self');
selfUser.isMe = true;
userState.self(selfUser);
Expand Down Expand Up @@ -290,19 +290,11 @@ describe('UserRepository', () => {
const userIds = localUsers.map(user => user.qualified_id!);
const connections = createConnections(localUsers);
const partialUsers = [
{
id: userIds[0].id,
availability: Availability.Type.AVAILABLE,
qualified_id: userIds[0],
},
{
id: userIds[1].id,
availability: Availability.Type.BUSY,
qualified_id: userIds[1],
},
{id: userIds[0].id, availability: Availability.Type.AVAILABLE},
{id: userIds[1].id, availability: Availability.Type.BUSY},
];

jest.spyOn(userRepository['userService'], 'loadUsersFromDb').mockResolvedValue(partialUsers as any);
jest.spyOn(userRepository['userService'], 'loadUserFromDb').mockResolvedValue(partialUsers as any);
const fetchUserSpy = jest.spyOn(userService, 'getUsers').mockResolvedValue({found: localUsers});

await userRepository.loadUsers(new User('self'), connections, [], []);
Expand Down
67 changes: 27 additions & 40 deletions src/script/user/UserRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,36 +216,26 @@ export class UserRepository extends TypedEventEmitter<Events> {
// the entries we get back will be used to feed the availabilities of those users
const nonQualifiedUsers = await this.userService.clearNonQualifiedUsers();

const dbUsers = await this.userService.loadUsersFromDb();
const dbUsers = await this.userService.loadUserFromDb();

// The self user doesn't need to be re-fetched
const usersToFetch = users.filter(user => !matchQualifiedIds(selfUser.qualifiedId, user));

const {found, failed} = await this.fetchRawUsers(usersToFetch, selfUser.domain);

const usersWithAvailability = found.map(user => {
const localUser = dbUsers.find(
dbUser => dbUser.id === user.id || matchQualifiedIds(dbUser.qualified_id, user.qualified_id),
);

const userWithAvailability = [...dbUsers, ...nonQualifiedUsers].find(userRecord => userRecord.id === user.id);

const userWithEscapedDefaultName = this.replaceDeletedUserNameWithNameInDb(user, localUser);
const userWithAvailability = found.map(user => {
const availability = [...dbUsers, ...nonQualifiedUsers].find(userRecord => userRecord.id === user.id);

if (userWithAvailability) {
return {
availability: userWithAvailability.availability,
...userWithEscapedDefaultName,
};
if (availability) {
return {availability: availability.availability, ...user};
}

return userWithEscapedDefaultName;
return user;
});

// Save all new users to the database
await Promise.all(usersWithAvailability.map(user => this.saveUserInDb(user)));
await Promise.all(userWithAvailability.map(user => this.saveUserInDb(user)));

const mappedUsers = this.mapUserResponse(usersWithAvailability, failed, dbUsers);
const mappedUsers = this.mapUserResponse(userWithAvailability, failed, dbUsers);

// Assign connections to users
mappedUsers.forEach(user => {
Expand Down Expand Up @@ -333,14 +323,6 @@ export class UserRepository extends TypedEventEmitter<Events> {
* Will update the user both in database and in memory.
*/
private async updateUser(userId: QualifiedId, user: Partial<UserRecord>, isWebSocket = false): Promise<User> {
if (user.deleted && user.name) {
const dbUser = await this.userService.loadUserFromDb(userId);

if (dbUser && dbUser.name) {
user.name = dbUser.name;
}
}

const selfUser = this.userState.self();
const isSelfUser = matchQualifiedIds(userId, selfUser.qualifiedId);
const userEntity = isSelfUser ? selfUser : await this.getUserById(userId);
Expand Down Expand Up @@ -593,19 +575,23 @@ export class UserRepository extends TypedEventEmitter<Events> {
}

// Replaces a deleted user name ("default") with the name from the local database.
private replaceDeletedUserNameWithNameInDb(user: APIClientUser, localUser?: UserRecord): UserRecord {
if (!user.deleted) {
return user;
}
private restoreDeletedUserNames(apiUsers: APIClientUser[], dbUsers: UserRecord[]): UserRecord[] {
return apiUsers.map(user => {
if (!user.deleted) {
return user;
}

if (localUser && localUser.name) {
return {
...user,
name: localUser.name,
};
}
const localUser = dbUsers.find(dbUser => dbUser.id === user.id);

if (localUser && localUser.name) {
return {
...user,
name: localUser.name,
};
}

return {...user, name: t('deletedUser')};
return user;
});
}

private mapUserResponse(found: APIClientUser[], failed: QualifiedId[], dbUsers: UserRecord[]): User[] {
Expand All @@ -629,7 +615,9 @@ export class UserRepository extends TypedEventEmitter<Events> {
return new User(userId.id, userId.domain);
});

const mappedUsers = this.userMapper.mapUsersFromJson(found, selfDomain).concat(failedToLoad);
const users = this.restoreDeletedUserNames(found, dbUsers);

const mappedUsers = this.userMapper.mapUsersFromJson(users, selfDomain).concat(failedToLoad);

if (this.teamState.isTeam()) {
this.mapGuestStatus(mappedUsers);
Expand All @@ -645,7 +633,7 @@ export class UserRepository extends TypedEventEmitter<Events> {
*/
private async fetchUsers(userIds: QualifiedId[]): Promise<User[]> {
const {found, failed} = await this.fetchRawUsers(userIds, this.userState.self().domain);
const dbUsers = await this.userService.loadUsersFromDb();
const dbUsers = await this.userService.loadUserFromDb();
const users = this.mapUserResponse(found, failed, dbUsers);

let fetchedUserEntities = this.saveUsers(users);
Expand Down Expand Up @@ -725,7 +713,6 @@ export class UserRepository extends TypedEventEmitter<Events> {
if (localOnly) {
const deletedUser = new User(userId.id, userId.domain);
deletedUser.isDeleted = true;
deletedUser.name(t('deletedUser'));
return deletedUser;
}
try {
Expand Down
12 changes: 1 addition & 11 deletions src/script/user/UserService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,10 @@ export class UserService {
* @todo There might be more keys which are returned by this function
* @returns Resolves with all the stored user states
*/
loadUsersFromDb(): Promise<UserRecord[]> {
loadUserFromDb(): Promise<UserRecord[]> {
return this.storageService.getAll<UserRecord>(this.USER_STORE_NAME);
}

/**
* Loads user from the local database
* @returns Resolves to the stored user, if it does not exists resolves to undefined
*/
loadUserFromDb(qualifiedId: QualifiedId): Promise<UserRecord | undefined> {
const primaryKey = constructUserPrimaryKey(qualifiedId);

return this.storageService.load<UserRecord>(this.USER_STORE_NAME, primaryKey);
}

/**
* Will remove all the non-qualified entries in the DB
*/
Expand Down
3 changes: 1 addition & 2 deletions src/types/i18n.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,11 +443,11 @@ declare module 'I18n/en-US.json' {
'conversationDetailsActionConversationParticipants': `Show all ({number})`;
'conversationDetailsActionCreateGroup': `Create group`;
'conversationDetailsActionDelete': `Delete group`;
'conversationDetailsActionDeleteForMe': `Delete for Me`;
'conversationDetailsActionDevices': `Devices`;
'conversationDetailsActionGuestOptions': `Guests`;
'conversationDetailsActionLeave': `Leave group`;
'conversationDetailsActionNotifications': `Notifications`;
'conversationDetailsActionDeleteForMe': `Delete for Me`;
'conversationDetailsActionServicesOptions': `Services`;
'conversationDetailsActionTimedMessages': `Self-deleting messages`;
'conversationDetailsActionUnblock': `Unblock`;
Expand Down Expand Up @@ -654,7 +654,6 @@ declare module 'I18n/en-US.json' {
'dataSharingModalDecline': `Decline`;
'dataSharingModalDescription': `Help to improve Wire by sharing your usage data via a pseudonymous ID. The data is neither linked to your personal information nor shared with third parties besides Wire Group. It includes, for example, when you use a feature, your app version, device type, or your operating system. This data will be deleted at the latest after 365 days. <br /> Find further details in our [link]Privacy Policy[/link]. You can revoke your consent at any time.`;
'dataSharingModalTitle': `Consent to share user data`;
'deletedUser': `Deleted User`;
'downloadLatestMLS': `Download the latest MLS Wire version`;
'enumerationAnd': `, and `;
'ephemeralRemaining': `remaining`;
Expand Down
Loading