Skip to content

Commit

Permalink
chore: Revert "feat: Handle deleted user names appropriately (WPB-144…
Browse files Browse the repository at this point in the history
…92) (#18521)" (#18537)

This reverts commit bcdcebb.
  • Loading branch information
thisisamir98 authored Dec 19, 2024
1 parent b83d4ab commit 1c7d05f
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 66 deletions.
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

0 comments on commit 1c7d05f

Please sign in to comment.