Paths

Table of Contentst

-
lib/reducers/
-
reducers/
1
message-reducer.js

[lib] Fix messages disappearing from thick sidebars
ClosedPublic
Actions

Authored by inka on Oct 1 2024, 2:45 AM.

Details

Reviewers

tomek
kamil

Commits

rCOMMe33a11e5af95: [lib] Fix messages disappearing from thick sidebars

Summary

issue: ENG-9403
I considered using threadWatcher, but it seems like it has a different purpose:
Overall threadWatcher represents threads that are bing watched by the user. For thick sidebars we want to always receive all updates.
More datailed explanation:
threadWatcher is currently only used in the context of keyservers. It is used to listen for chat updates when the user is either

viewing a chat they are not a member of (this is not possible in thick threads, other than sidebars. But for sidebars with this code we will be getting all updates anyway)
viewing a list of sidebars / subchannels of a chat they are not a mamber of (this is not possible in thick threads)
viewing settings of a chat they are not a member of (this is not possible in thick threads)

Then, keyserver specific actions, keyserver specific selector, and this reducer use the ids from thread watcher.

Test Plan

Tested that if a user is a member of a thick thread but not a member of its sidebar, they receive this sidebars messages. Tested they receive updates (tested name update)
Tested that if a user is mamber of a thick sidebar and leaves it, the messages are not removed from the store.
Tested that if the user leaves the parent chat, the messages from the thick sidebar ARE removed (in the function, !!threadInfo condition is not satisfied)

Checked all places where threadWatcher is used and verified that none of them require changes for thick sidebars.

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D13554

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Oct 1 2024, 2:45 AM

Herald added a subscriber: ashoat. · View Herald TranscriptOct 1 2024, 2:45 AM

Harbormaster completed remote builds in B31975: Diff 44740.Oct 1 2024, 5:27 AM

inka requested review of this revision.Oct 1 2024, 5:27 AM

tomek accepted this revision.Oct 1 2024, 5:36 AM

tomek added inline comments.

lib/reducers/message-reducer.js
148 ↗	(On Diff #44740)	Can we make this more generic and test whether `threadTypeIsThick`?

This revision is now accepted and ready to land.Oct 1 2024, 5:36 AM

threadTypeIsThick

This revision was landed with ongoing or failed builds.Oct 1 2024, 12:49 PM

Closed by commit rCOMMe33a11e5af95: [lib] Fix messages disappearing from thick sidebars (authored by inka, committed by ashoat). · Explain Why

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMMe33a11e5af95: [lib] Fix messages disappearing from thick sidebars.

Harbormaster completed remote builds in B31997: Diff 44797.Oct 1 2024, 1:00 PM

Revision Contents
Changeset List

Path

Size

lib/

reducers/

message-reducer.js

4 lines

Diff 44797

View Options

lib/reducers/message-reducer.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

[lib] Fix messages disappearing from thick sidebarsClosedPublicActions