[PAY-1032][PAY-892] Mobile DMs unread indicator, prefetch chats by dharit-tan · Pull Request #3352 · AudiusProject/audius-client

dharit-tan · 2023-05-10T00:27:38Z

Description

Purple unread circle to indicate unread chats.
To enable this, also fetching chats + connecting to websockets at root screen load, and moved fetching chat messages for the first 10 messages on ChatListScreen load.
Changed takeLatest to takeEvery so multiple fetchMoreMessages calls in a row would all go through.

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Local ios stage

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

…t never came back?

audius-infra · 2023-05-10T00:47:54Z

Preview this change https://demo.audius.co/rt-unread-purple

audius-infra · 2023-05-16T15:58:21Z

Preview this change https://demo.audius.co/rt-unread-purple

…in left nav bar

audius-infra · 2023-05-16T16:53:35Z

Preview this change https://demo.audius.co/rt-unread-purple

audius-infra · 2023-05-16T20:22:30Z

Preview this change https://demo.audius.co/rt-unread-purple

rickyrombo · 2023-05-16T20:28:56Z

+        return
+      }
+      const existingSummary = state.chats.entities[chatId]?.messagesSummary
+      if (summary.next_count > (existingSummary?.next_count ?? -1)) {


I think we really want to take the max of the next_cursor and the min of the prev_cursor (or vice versa? can't remember which way they go)?

Wouldn't we want to take whichever summary is latest? And to get that, we could either check max of next_cursor or min of prev_cursor. And wouldn't a summary with a higher next_count necessarily have a later next_cursor, and vice versa?

Chatted with @rickyrombo on slack, his way is the way to go. "the summary we get back is for that query but I want the store to have the summary for all the stuff itself."

Turns out we want min of both i believe.

rickyrombo · 2023-05-16T22:20:06Z

      action: PayloadAction<{ unreadMessagesCount: number }>
    ) => {
      state.unreadMessagesCount = action.payload.unreadMessagesCount
+      state.optimisticUnreadMessagesCount = action.payload.unreadMessagesCount


maybe don't do this actually... this will clear any pending optimistic changes. Perhaps instead of an "optimisticUnreadMessagesCount" we should keep track of "optimisticUnreadMessageCountChanges"?

UI shows 10 unreads

start fetching unread count

start mark chat as read that has 4 unreads

optimistic unread count decremented 4 to 6

UI shows 6 unreads

fetch unread count succeeds with 10 since the write hasn't completed yet

optimistic unread count set to unread count, 10

UI shows 10 unreads

mark chat as read succeeds

unread count updated to optimistic unread count (10)

optimistic unread count deleted

UI shows 10 unreads

When should we reset optimisticUnreadMessageCountChanges? We need to reconcile it with the actual unread count eventually, right? Eg:

On first load we fetch unread count, it's 4.

We mark a chat as read that had all 4 unread messages, optimisticUnreadCountChanges = -4.

2 new messages come in on a different chat, optimisticUnreadCountChanges = -2

Open left nav bar which re-fetches unreadCount, it is now 2.

We would incorrectly show no unread messages bc unreadCount + optimisticUnreadCountChanges = 0.
Not sure of solution here, just pointing out broken case, but if we don't reconcile the optimistic changes ever... this approach may not work.

Hmm yeah this is quite tricky... Before I say anything, I think we could delete state.optimisticUnreadMessagesCount here and merge this as v1 and revisit before launch

Wonder if we could get away with only fetching the unread message count once, maybe on initial app load. The websocket is always listening, right? It'll "reconcile" on app kill + reopen. It sounds a little risky but it might not be...

Might have to sleep on this and think more

audius-infra · 2023-05-16T22:21:37Z

Preview this change https://demo.audius.co/rt-unread-purple

audius-infra · 2023-05-17T22:55:38Z

Preview this change https://demo.audius.co/rt-unread-purple

[9855fd2] Update locks (#3397) Dylan Jeffers [aecef5f] [PAY-1144] [PAY-1182] [PAY-1147] DMs: Delete chat, message permissions (#3390) Marcus Pasell [64ccd9d] Add stylelint to ci (#3373) Dylan Jeffers [b642a16] [C-2518, C-2523, C-2611] Improve playlist create sagas (#3378) Dylan Jeffers [5620d53] [PAY-1197] Mobile inbox unavailable modal from profile screen (#3376) Reed [2ebaef5] [PAY-1248] Initial changes to get ready for upcoming PRs that include track and playlist tiles in DMs (#3391) Saliou Diallo [c2fabf6] [PAY-1218] Mobile block dms drawer switches block/unblock (#3387) Reed [0acad29] [C-2575] Match length of related artists user list to preview (#3395) Andrew Mendelsohn [40525ae] [C-2615] Fix favorite tracks error due to empty track entries on web mobile (#3386) Kyle Shanks [93a8077] [PAY-1145] DMs: Add InboxUnavailableModal (#3369) Marcus Pasell [633484d] Update edit playlist flow and components (#3361) Kyle Shanks [334ea0d] Special case ios safari for stem download (#3385) Raymond Jacobson [d38238d] [C-2607] Pagination wrapper hooks for audius-query (#3375) Andrew Mendelsohn [0a332d8] Hotfix: Fix stems downloads on mobile web (#3382) Marcus Pasell [81da65e] Clean up artist_pick_track_id in APIUser (#3381) Michelle Brier [64d89ef] Fix lint (#3380) Raymond Jacobson [59129b4] [C-2614] Fix download stems mobile web (#3379) Raymond Jacobson [df68727] [PAY-1032][PAY-892] Mobile DMs unread indicator, prefetch chats (#3352) Reed [4a00fad] [PAY-1151] Handle chat reactions near top of screen on mobile (#3370) Reed [bff316f] [PAY-1139] Throttle calls to fetchMessages on web scroll (#3372) Michael Piazza [e723cec] C-2483 Fix queue overshot empty track player bug (#3353) nicoback2 [472a41d] [C-2596] Add disabled option to audius-query hooks (#3367) Andrew Mendelsohn [f518962] [C-2602] Improve playlist library layout (#3364) Dylan Jeffers [b3db8fa] Fix debounce on notif reaction (#3362) Raymond Jacobson [2104b2b] [PAY-1183] Make clicking ChatUser handle/displayname lead to profile (#3368) Michael Piazza [21cb6fc] DMs: Fix click handler in search user list for users you can't chat (#3358) Marcus Pasell [24e7d8a] DMs: Update copy, scroll inbox-settings modal (#3359) Marcus Pasell [e5e8d90] [PAY-941] Fix "1 new messages" unread tag (web) (#3366) Michael Piazza [cfa1b2a] [C-2603] Fix readonly object error in audius-query reducer (#3365) Andrew Mendelsohn [978c993] Fix invite reward claimable state on mobile (#3363) Reed [a174fcc] [C-2556, C-2557] Address AI Attribution QA (#3349) Dylan Jeffers [3c9b0f1] [PAY-1202] Refactor saved collections fetching (#3337) Randy Schott [a0bdad5] Get call to action for chat permissions (#3325) Marcus Pasell [03a2721] DMs: Use the optimistic unread count if applicable (#3354) Marcus Pasell [8b99c2b] [C-2550] Left-nav fixes and improvements (#3357) Dylan Jeffers [e7b0aab] [PAY-1215] Fix create new message crash (#3356) Reed [6e7ece9] [PAY-1196] Mobile dms search users empty state (#3355) Reed [315ae4f] [PAY-1219] Fix mobile chat reactions popup message getting cut off (#3342) Reed

dharit-tan added 4 commits May 9, 2023 17:02

[PAY-1032][PAY-892] Mobile DMs unread indicator, prefetch chats

2e86145

fetching msgs only results in 1 success, rest of requests went out bu…

278d926

…t never came back?

remove fetches elsewhere

ec856a4

WORKING

bd3b5b4

dharit-tan requested a review from rickyrombo May 10, 2023 00:27

dharit-tan assigned schottra, piazzatron and sddioulde May 10, 2023

pull-request-size Bot added the size/M label May 10, 2023

rickyrombo reviewed May 15, 2023

View reviewed changes

Comment thread packages/common/src/store/pages/chat/sagas.ts

Comment thread packages/common/src/store/pages/chat/selectors.ts Outdated

Comment thread packages/mobile/src/screens/chat-screen/ChatScreen.tsx

use unread endpoint

72fa511

pull-request-size Bot added size/L and removed size/M labels May 16, 2023

schottra reviewed May 16, 2023

View reviewed changes

Comment thread packages/mobile/src/screens/chat-screen/ChatListScreen.tsx

fetch unread messages count

7b244ad

dharit-tan added 2 commits May 16, 2023 12:20

update fetchMoreMessagesSucceeded logic, foreach, fetch unread count …

db69f23

…in left nav bar

increment global unread count

52cf04e

dharit-tan requested review from rickyrombo and schottra May 16, 2023 16:30

dharit-tan added 3 commits May 16, 2023 15:55

Include optimistic reads in getHasUnreadMessages

ca2aff2

comment

aebaa5a

Merge remote-tracking branch 'origin/main' into rt-unread-purple

676d508

rickyrombo approved these changes May 16, 2023

View reviewed changes

rickyrombo reviewed May 16, 2023

View reviewed changes

Comment thread packages/common/src/store/pages/chat/selectors.ts Outdated

rickyrombo reviewed May 16, 2023

View reviewed changes

Comment thread packages/common/src/store/pages/chat/slice.ts Outdated

Optimistic unread count

eef45f0

dharit-tan added 2 commits May 16, 2023 17:41

Use max/min of next_count/prev_count

0411c92

min of both

6e06ba7

dharit-tan requested a review from rickyrombo May 16, 2023 21:58

rickyrombo reviewed May 16, 2023

View reviewed changes

rickyrombo approved these changes May 16, 2023

View reviewed changes

rickyrombo reviewed May 16, 2023

View reviewed changes

Comment thread packages/common/src/store/pages/chat/slice.ts

PR comments

5745d36

dharit-tan merged commit df68727 into main May 18, 2023

dharit-tan deleted the rt-unread-purple branch May 18, 2023 00:05

AudiusProject deleted a comment from linear Bot Sep 11, 2023

Uh oh!

Conversation

dharit-tan commented May 10, 2023

Description

Dragons

How Has This Been Tested?

How will this change be monitored?

Feature Flags

Uh oh!

audius-infra commented May 10, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

audius-infra commented May 16, 2023

Uh oh!

audius-infra commented May 16, 2023

Uh oh!

audius-infra commented May 16, 2023

Uh oh!

rickyrombo May 16, 2023

Choose a reason for hiding this comment

Uh oh!

dharit-tan May 16, 2023

Choose a reason for hiding this comment

Uh oh!

dharit-tan May 16, 2023

Choose a reason for hiding this comment

Uh oh!

dharit-tan May 16, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rickyrombo May 16, 2023

Choose a reason for hiding this comment

Uh oh!

rickyrombo May 16, 2023

Choose a reason for hiding this comment

Uh oh!

dharit-tan May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rickyrombo May 17, 2023

Choose a reason for hiding this comment

Uh oh!

audius-infra commented May 16, 2023

Uh oh!

Uh oh!

audius-infra commented May 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dharit-tan May 17, 2023 •

edited

Loading