[No QA] Add support for Pusher responses #9219

Gonals · 2022-05-30T13:53:00Z

Details

We haven't migrated any commands yet, so we need to add some changes in Web to test this.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/211236

Tests

In web-E, you'll need to add an Onyx response to a command to test this. I simply added this line here, so it would be triggered when a new comment is added on the chat:

$onyxUpdate[] = ['onyxMethod' => 'Report_AddCommentSet', 'key' =>'Nepal', 'value' => 'Test'];

You can just log the JSON to the console here to make sure it is received.
After that, open a chat in NewDot and send a message. In a second or two you should see the response showing up in the console.
Tail the logs.
Remove or change one of the keys in onyxUpdate and send a message to the chat. The response that shows up in the console should now be an empty array, and a warning indicating that the response was not correctly formatted should show in the logs.

marcaaron · 2022-05-30T16:20:09Z

src/libs/Network/SequentialQueue.js

+function getCurrentRequest() {
+    if (currentRequest === null) {
+        return Promise.resolve();
+    }


NAB, could also initialize/reset with a Promise.resolve() but this works.

marcaaron · 2022-05-30T16:23:08Z

src/libs/Pusher/EventType.js

@@ -11,4 +11,6 @@ export default {
    PREFERRED_LOCALE: 'preferredLocale',
    EXPENSIFY_CARD_UPDATE: 'expensifyCardUpdate',
    SCREEN_SHARE_REQUEST: 'screenshareRequest',
+    ONYX_API_UPDATE: 'onyxApiUpdate',
+    ONYX_API_UPDATE_CHUNK: 'chunked-onyxApiUpdate',


After these changes we won't need the prepend chunked to channel names.

cc @danieldoglas to confirm

Oh nice! That one seems fully approved, so it should be merging today, right @alex-mechler? I can update this then :)

Just self merged it, since all reviewers had approved!

That said, we still need the chunked- channel to be listened to for now, since Web-E will still be sending them until we make the changes to stop sending them. That is HELD on the linked PR getting deployed to production.

I guess we can move ahead with this PR as-is and remove that afterwards?

Yup, that sounds fine. I have to remove the other chunked- channels later from this repo anyways.

src/libs/Pusher/pusher.js

marcaaron · 2022-05-30T16:29:47Z

src/libs/Pusher/pusher.js

+    }
+
+    function onPusherResubscribeToPrivateUserChannel() {
+        NetworkConnection.triggerReconnectionCallbacks('Pusher re-subscribed to private user channel');


opinion (non-blocking): It stuck out to me that the Pusher library is now triggering and aware of the reconnection callbacks. Feels like it now has extra responsibility it should not have. I would maybe put this into something like a PusherUtils or just have it in User eventually (after the usages in Report.js are cleaned up). But not sure we need to do this now.

Hmmm. User seems like a weird place to me, but creating PusherUtils may be a good idea. Do we expect to create more stuff that would go in there?

Do we expect to create more stuff that would go in there?

Unsure, but I suspect that this subscription will eventually be moved into something like a "User Logged In" or "App Initialized" action since it will happen every time the app starts up.

I like putting it into PusherUtils 👍

src/libs/Pusher/pusher.js

yuwenmemon · 2022-06-01T22:21:41Z

src/libs/Pusher/pusher.js

+    }
+
+    function onPusherResubscribeToPrivateUserChannel() {
+        NetworkConnection.triggerReconnectionCallbacks('Pusher re-subscribed to private user channel');


I like putting it into PusherUtils 👍

Gonals · 2022-06-02T10:39:34Z

OFF HOLD and comments addressed!

yuwenmemon · 2022-06-02T17:56:39Z

@marcaaron all yours

marcaaron · 2022-06-02T17:59:17Z

🚀

OSBotify · 2022-06-02T18:08:03Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-06-06T18:29:47Z

🚀 Deployed to staging by @marcaaron in version: 1.1.72-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2022-06-08T16:30:45Z

🚀 Deployed to production by @yuwenmemon in version: 1.1.73-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

mountiny · 2022-06-09T18:35:03Z

src/libs/actions/Report.js

-        Log.hmmm('[Report] Failed to subscribe to Pusher channel', false, {error, pusherChannelName, eventName});
-    }
-
-    Pusher.subscribe(pusherChannelName, eventName, onEventPush, onPusherResubscribeToPrivateUserChannel)


Here there was not the isChunked but in the reportUtils it is back.

roryabraham · 2022-11-02T05:06:41Z

src/libs/actions/User.js

@@ -302,6 +304,18 @@ function subscribeToUserEvents() {

    const pusherChannelName = `private-encrypted-user-accountID-${currentUserAccountID}${CONFIG.PUSHER.SUFFIX}`;

+    // Receive any relevant Onyx updates from the server
+    PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.ONYX_API_UPDATE, currentUserAccountID, (pushJSON) => {
+        SequentialQueue.getCurrentRequest().then(() => {


Hey everyone 👋🏼 I know it's been a little while, but can someone explain why we needed to add SequentialQueue.getCurrentRequest() here? It's not clear to me what it's doing.

The first I heard about it you were the one proposing it 😄

https://expensify.slack.com/archives/C02HWMSMZEC/p1652731618392499?thread_ts=1652466984.820589&cid=C02HWMSMZEC

I think I understand why this was added now, I tried to summarize it as concisely as possible here

It was always a longer goal of mine to add a NETWORK.md and that explanation would be perfect for it (or added as a comment here).

Gonals added 7 commits May 27, 2022 10:53

Move subscribeToPrivateUserChannelEvent to Pusher

0658ff1

add type

5445a6a

subscribe to the event in User

729eeee

quotes

4689132

handle chunks

a3b3879

remove extra code

2ac4921

remove console line

66e9256

Gonals requested review from marcaaron and a team as code owners May 30, 2022 13:53

Gonals self-assigned this May 30, 2022

melvin-bot bot requested review from tylerkaraszewski and removed request for a team May 30, 2022 13:53

remove debug lines

a25bd56

Gonals changed the title ~~[No QA] Add support for Pusher responses~~ [HOLD][No QA] Add support for Pusher responses May 30, 2022

Gonals removed the request for review from tylerkaraszewski May 30, 2022 14:07

marcaaron previously approved these changes May 30, 2022

View reviewed changes

yuwenmemon previously approved these changes Jun 1, 2022

View reviewed changes

Gonals changed the title ~~[HOLD][No QA] Add support for Pusher responses~~ [No QA] Add support for Pusher responses Jun 2, 2022

Gonals added 2 commits June 2, 2022 10:13

merge main

ee589cf

Move function to PusherUtils

29ad302

Gonals dismissed stale reviews from yuwenmemon and marcaaron via 29ad302 June 2, 2022 08:50

Gonals added 2 commits June 2, 2022 11:12

add newline

089326d

add const

6977583

yuwenmemon approved these changes Jun 2, 2022

View reviewed changes

marcaaron approved these changes Jun 2, 2022

View reviewed changes

marcaaron merged commit afa9087 into main Jun 2, 2022

marcaaron deleted the alberto-onyxUpdate branch June 2, 2022 17:59

yuwenmemon mentioned this pull request Jun 6, 2022

Deploy Checklist: New Expensify 2022-06-06 #9326

Closed

47 tasks

mountiny reviewed Jun 9, 2022

View reviewed changes

roryabraham reviewed Nov 2, 2022

View reviewed changes

hellohublot mentioned this pull request Mar 22, 2023

[$1000] Chat - The reaction emoji left on the loading comment disappears #15998

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[No QA] Add support for Pusher responses #9219

[No QA] Add support for Pusher responses #9219

Gonals commented May 30, 2022 •

edited

Loading

marcaaron May 30, 2022

marcaaron May 30, 2022

Gonals Jun 1, 2022 •

edited

Loading

alex-mechler Jun 1, 2022

Gonals Jun 2, 2022

alex-mechler Jun 2, 2022

marcaaron May 30, 2022

Gonals Jun 1, 2022

marcaaron Jun 1, 2022

yuwenmemon Jun 1, 2022

Gonals Jun 2, 2022

yuwenmemon Jun 1, 2022

Gonals commented Jun 2, 2022

yuwenmemon commented Jun 2, 2022

marcaaron commented Jun 2, 2022

OSBotify commented Jun 2, 2022

OSBotify commented Jun 6, 2022

OSBotify commented Jun 8, 2022

mountiny Jun 9, 2022

roryabraham Nov 2, 2022

marcaaron Nov 2, 2022

roryabraham Apr 10, 2023

marcaaron Apr 10, 2023

[No QA] Add support for Pusher responses #9219

[No QA] Add support for Pusher responses #9219

Conversation

Gonals commented May 30, 2022 • edited Loading

Details

Fixed Issues

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gonals Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gonals commented Jun 2, 2022

yuwenmemon commented Jun 2, 2022

marcaaron commented Jun 2, 2022

OSBotify commented Jun 2, 2022

OSBotify commented Jun 6, 2022

OSBotify commented Jun 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gonals commented May 30, 2022 •

edited

Loading

Gonals Jun 1, 2022 •

edited

Loading