Page MenuHomePhabricator
Differential D13741

[lib] Delete users for whom identity didnt return a device list for a long time
ClosedPublic
Actions

Authored by inka on Oct 17 2024, 4:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 3, 10:41 AM
Unknown Object (File)
Thu, Apr 3, 8:08 AM
Unknown Object (File)
Thu, Apr 3, 6:50 AM
Unknown Object (File)
Thu, Apr 3, 4:39 AM
Unknown Object (File)
Wed, Apr 2, 9:04 PM
Unknown Object (File)
Wed, Apr 2, 9:09 AM
Unknown Object (File)
Feb 28 2025, 6:06 AM
Unknown Object (File)
Feb 28 2025, 5:25 AM
View All 86 Files
Subscribers
ashoat

Details

Reviewers
tomek
kamil
Commits
rCOMMda9fc92ba039: [lib] Delete users for whom identity didnt return a device list for a long time
Summary

issue: ENG-9632
If we fail to fetch usres device list for a whole day, we can assume that this is not due to identity temporary data inconsistency. And if a user is a member of a DM, and their device list is not present on the identity, then we know for certain that they deleted their account.
There is only one catch here - if the request times out we shouldn't be deleting the user. Created ENG-9725 to address this

Test Plan

Tested that a user whose device list failed to be fetched for a long time gets deleted and they get removed from threads, users store and aux user store.
Checked that a user whose lastChecked was updated does not get removed from threads and other stores

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka created this revision.Oct 17 2024, 4:22 AM
Herald added a subscriber: ashoat. · View Herald TranscriptOct 17 2024, 4:22 AM
inka added a parent revision: D13740: [lib] Reduce removePeerUsersActionType in thread reducer.Oct 17 2024, 4:22 AM
Harbormaster completed remote builds in B32287: Diff 45245.Oct 17 2024, 4:40 AM
inka requested review of this revision.Oct 17 2024, 4:40 AM
inka updated this revision to Diff 45257.Oct 17 2024, 6:28 AM

Rebase

Harbormaster completed remote builds in B32295: Diff 45257.Oct 17 2024, 6:54 AM
inka added a child revision: D13750: [lib] Handle identity timeout when fetching device lists.Oct 18 2024, 3:36 AM
tomek accepted this revision.Oct 21 2024, 3:41 AM
tomek added inline comments.
lib/shared/dm-ops/dm-op-utils.js
178–202 ↗(On Diff #45257)

It might be a good idea to make the dependency between nonDeletedUsers and deletedUsers explicit by creating a function.

This revision is now accepted and ready to land.Oct 21 2024, 3:41 AM
inka updated this revision to Diff 45288.Oct 21 2024, 5:25 AM

Rebase, address review

Harbormaster completed remote builds in B32314: Diff 45288.Oct 21 2024, 5:44 AM
Closed by commit rCOMMda9fc92ba039: [lib] Delete users for whom identity didnt return a device list for a long time (authored by inka). · Explain WhyOct 21 2024, 6:42 AM
This revision was automatically updated to reflect the committed changes.
inka added a commit: rCOMMda9fc92ba039: [lib] Delete users for whom identity didnt return a device list for a long time.

Revision Contents
Changeset List

PathSize
lib/
shared/
dm-ops/
48 lines
2 lines

Diff 45288

View Options

lib/shared/dm-ops/dm-op-utils.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

lib/shared/thread-utils.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines