[Payment Due Oct 1][$250] [#fast-apis] Subscribe guides to a new presence pusher channel #47888

MonilBhavsar · 2024-08-22T22:52:03Z

Context:

Guides are agents who help new customers with onboarding.

We use Pusher to send real time updates.
We have a private user channel here

Line 1154 in f7d8e48

PRIVATE_USER_CHANNEL_PREFIX: 'private-encrypted-user-accountID-',

private-encrypted-user-accountID-<> that is used to receive various events in real time like onyx updates, user typing and more.

Problem

Pusher private channel is dynamic, or unique for each user as it contains an accountID.
For example: A channel name could look like private-encrypted-user-accountID-5-b8645604fe014e82b765ca84b7df1f2e

At the server side, if we want to know how many guides are online at the current time, then we need to make an API call to each pusher channel and see if it is occupied or not.
If we have 20 guides, then we make 20 API calls to check guide's presence, which is not efficient.

Solution

Make guides subscribe to a presence channel - activeGuides
https://pusher.com/docs/channels/using_channels/presence-channels/

With this, a server only will have to make one API call to find all active guides or users subscribed to this channel.

If a user is a guide - i.e. have a team.expensify.com domain and belongs to a specific policyID, then we additionally subscribe guide to the the guides presence channel.

Like the private user channel, a subscription to the channel should indicate guide's presence in the app - They should be subscribed if they login and are active, and should be unsubscribed if they sign out.

We probably have most of the code and logic for the private user channel. We need to similarly subscribe guides to the new channel.

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01fe75f988a6386f2e
Upwork Job ID: 1826754279960805798
Last Price Increase: 2024-08-29
Automatic offers:

situchan | Reviewer | 103739965

Issue Owner

Current Issue Owner: @trjExpensify

The text was updated successfully, but these errors were encountered:

melvin-bot · 2024-08-22T22:52:16Z

Job added to Upwork: https://www.upwork.com/jobs/~01fe75f988a6386f2e

melvin-bot · 2024-08-22T22:52:21Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

Krishna2323 · 2024-08-22T23:47:02Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

[#fast-apis] Subscribe guides to a new presence pusher channel

What is the root cause of that problem?

Improvement.

What changes do you think we should make in order to solve the problem?

We already have subscribeToPrivateUserChannelEvent function which subscribes to to private user channel events.

App/src/libs/PusherUtils.ts

Lines 26 to 49 in 194f197

    
           /** 
        
            * Abstraction around subscribing to private user channel events. Handles all logs and errors automatically. 
        
            */ 
        
           function subscribeToPrivateUserChannelEvent(eventName: string, accountID: string, onEvent: (pushJSON: OnyxUpdatesFromServer) => void) { 
        
               const pusherChannelName = `${CONST.PUSHER.PRIVATE_USER_CHANNEL_PREFIX}${accountID}${CONFIG.PUSHER.SUFFIX}` as const; 
        
               function logPusherEvent(pushJSON: OnyxUpdatesFromServer) { 
        
                   Log.info(`[Report] Handled ${eventName} event sent by Pusher`, false, pushJSON); 
        
               } 
        
               function onPusherResubscribeToPrivateUserChannel() { 
        
                   NetworkConnection.triggerReconnectionCallbacks('Pusher re-subscribed to private user channel'); 
        
               } 
        
               function onEventPush(pushJSON: OnyxUpdatesFromServer) { 
        
                   logPusherEvent(pushJSON); 
        
                   onEvent(pushJSON); 
        
               } 
        
               function onSubscriptionFailed(error: Error) { 
        
                   Log.hmmm('Failed to subscribe to Pusher channel', {error, pusherChannelName, eventName}); 
        
               } 
        
               Pusher.subscribe(pusherChannelName, eventName, onEventPush, onPusherResubscribeToPrivateUserChannel).catch(onSubscriptionFailed); 
        
           }

We can created a similar function to subscribeToPrivateUserChannelEvent which will subscribe to the new presence pusher channel.

When calling User.subscribeToUserEvents(); we can check if the user is a guide or not and then after we can subscribe to the new presence pusher channel using the newly created function.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 112 to 128 in 194f197

    
           Onyx.connect({ 
        
               key: ONYXKEYS.SESSION, 
        
               callback: (value) => { 
        
                   // When signed out, val hasn't accountID 
        
                   if (!(value && 'accountID' in value)) { 
        
                       timezone = null; 
        
                       return; 
        
                   } 
        
                   currentAccountID = value.accountID ?? -1; 
        
                   if (Navigation.isActiveRoute(ROUTES.SIGN_IN_MODAL)) { 
        
                       // This means sign in in RHP was successful, so we can subscribe to user events 
        
                       User.subscribeToUserEvents(); 
        
                   } 
        
               }, 
        
           });

We should also create and call unsubscribe when user sign's out, for that we subscribe to ONYX as in the above code snipped or can unsubscribe in the above code.

What alternative solutions did you explore? (Optional) 

Result

daledah · 2024-08-23T01:40:01Z

Edited by proposal-police: This proposal was edited at 2024-08-23 01:43:52 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

New feature: Subscribe guides to a new presence pusher channel

What is the root cause of that problem?

It is new feature, so we can skip this part.

What changes do you think we should make in order to solve the problem?

Create a subscribe function, same as what we did in subscribeToReportTypingEvents, subscribeToReportLeavingEvents or subscribeToPrivateUserChannelEvent:

function subscribeToActiveGuides() {
    Pusher.subscribe(`activeGuides`).catch((error: ReportError) => {
        Log.hmmm('[Report] Failed to initially subscribe to Pusher channel');
    });
}

function unsubscribeToActiveGuides() {
    Pusher.unsubscribe(`activeGuides`);
}

the eventName can be provided later. And in this case, we can skip the eventCallback because BE just needs to know if the guide is online or not.

Create a subscribe component.

This component is named ActiveGuidesEventListener, its responsibility is to subscribe to a active guides channel if the current user is a guide and belongs to a specific policyID. Same as what we did in component UserTypingEventListener.

function ActiveGuidesEventListener({session, allPoliciesEmployeeList}) {
    const flatternPoliciesEmployee = flatten(Object.values(allPoliciesEmployeeList));
    const didSubscribeToActiveGuides = useRef(false);

    useEffect(
        () => () => {
            const sessionEmail = session.email;
            const emailDomain = Str.extractEmailDomain(sessionEmail ?? '');
            if (didSubscribeToActiveGuides.current) {
                return;
            }
            if (emailDomain !== CONST.EMAIL.GUIDES_DOMAIN) {
                return;
            }
            if (some(flatternPoliciesEmployee, (user) => user.email === sessionEmail)) {
                didSubscribeToActiveGuides.current = true;
                subscribeToActiveGuides();
            }
        },

        [session, flatternPoliciesEmployee],
    );
    return null;
}

export default withOnyx({
    session: {
        key: ONYXKEYS.SESSION,
    },
    allPoliciesEmployeeList: {
        key: ONYXKEYS.COLLECTION.POLICY,
        selector: (policy) => policy.employeeList,
    },
})(ActiveGuidesEventListener);

Use the above component in AuthScreens:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 526 in 194f197

</View>

            </View>
           {didPusherInit && <ActiveGuidesEventListener />}

with didPusherInit is state:

    const [didPusherInit, setDidPusherInit] = useState(false);

and we set it to true in here.

We don't need to cleanup function because this line:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 344 in 194f197

Session.cleanupSession();

already did that.

What alternative solutions did you explore? (Optional)

Based on comment, we can modify the ActiveGuidesEventListener in main solution to:

function ActiveGuidesEventListener({user}) {
    const didSubscribeToActiveGuides = useRef(false);
    useEffect(
        () => () => {
            if (didSubscribeToActiveGuides.current) {
                return;
            }
            if (user.isGuide) {
                didSubscribeToActiveGuides.current = true;
                subscribeToActiveGuides();
            }
        },

        [user],
    );
    return null;
}

export default withOnyx({
    user: {
        key: ONYXKEYS.USER,
    },
})(ActiveGuidesEventListener);

We still need to keep using didPusherInit && <ActiveGuidesEventListener /> as in main solution to make sure the Pusher.subscribe is called after the Pusher is init.

daledah · 2024-08-23T01:54:21Z

@MonilBhavsar In the issue's description you said:

have a team.expensify.com domain and belong to a specific policyID

if user has team.expensify.com domain but does not belongs to specific workspace, should we subscribe to the presence pusher channel?

daledah · 2024-08-23T01:57:05Z

Proposal updated

MonilBhavsar · 2024-08-23T14:07:34Z

if user has team.expensify.com domain but does not belongs to specific workspace, should we subscribe to the presence pusher channel?

No, both conditions are required

MonilBhavsar · 2024-08-23T14:08:37Z

Thanks for the proposals! 🚀
@situchan could you please take a look.

mariapeever · 2024-08-24T00:21:46Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

[#fast-apis] Subscribe guides to a new presence pusher channel #47888

What is the root cause of that problem?

Guide users are subscribed to a private encrypted user channel only and not subscribed to a presence channel ('presence-activeGuides').

What changes do you think we should make in order to solve the problem?

If a user is a guide, i.e. their email is an expensify team email, add a Pusher subscription to a presence channel request (PusherUtils.subscribeToPresenceChannelEvent) to the subscribeToUserEvents function in User (called in AuthScreens after Pusher init).

Add a subscribeToPresenceChannelEvent function to Pusher Utils with a Pusher subscribe request. All guides are subscribed to the same presence channel. Subscription to a presence channel adds a members property to the pusher socket, which includes a members count.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 254 to 262 in a29977b

    
           PusherConnectionManager.init(); 
        
           Pusher.init({ 
        
               appKey: CONFIG.PUSHER.APP_KEY, 
        
               cluster: CONFIG.PUSHER.CLUSTER, 
        
               authEndpoint: `${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/AuthenticatePusher?`, 
        
           }).then(() => { 
        
               User.subscribeToUserEvents(); 
        
           });

Add a PusherUtils.subscribeToPresenceChannelEvent request just after PusherUtils.subscribeToPrivateUserChannelEvent.

App/src/libs/actions/User.ts

Lines 660 to 674 in a29977b

    
           function subscribeToUserEvents() { 
        
               // If we don't have the user's accountID yet (because the app isn't fully setup yet) we can't subscribe so return early 
        
               if (currentUserAccountID === -1) { 
        
                   return; 
        
               } 
        
               // Handles the mega multipleEvents from Pusher which contains an array of single events. 
        
               // Each single event is passed to PusherUtils in order to trigger the callbacks for that event 
        
               PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.MULTIPLE_EVENTS, currentUserAccountID.toString(), (pushJSON) => { 
        
                   // If this is not the main client, we shouldn't process any data received from pusher. 
        
                   if (!ActiveClientManager.isClientTheLeader()) { 
        
                       Log.info('[Pusher] Received updates, but ignoring it since this is not the active client'); 
        
                       return; 
        
                   } 
        
                   // The data for the update is an object, containing updateIDs from the server and an array of onyx updates (this array is the same format as the original format above)

Add a subscribeToPresenceChannelEvent function just after subscribeToPrivateUserChannelEvent.

App/src/libs/PusherUtils.ts

Lines 29 to 31 in a29977b

    
           function subscribeToPrivateUserChannelEvent(eventName: string, accountID: string, onEvent: (pushJSON: OnyxUpdatesFromServer) => void) { 
        
               const pusherChannelName = `${CONST.PUSHER.PRIVATE_USER_CHANNEL_PREFIX}${accountID}${CONFIG.PUSHER.SUFFIX}` as const;

Include a Pusher.subscribe request in the subscribeToPresenceChannelEvent function with channel name 'presence-activeGuides'.

App/src/libs/PusherUtils.ts

Line 48 in a29977b

    
           Pusher.subscribe(pusherChannelName, eventName, onEventPush, onPusherResubscribeToPrivateUserChannel).catch(onSubscriptionFailed);

What alternative solutions did you explore? (Optional)

N/A

melvin-bot · 2024-08-26T18:37:44Z

@MonilBhavsar, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

situchan · 2024-08-26T19:09:35Z

Thanks for the proposals.

I think @Krishna2323's solution is the most simple, though the logic to check if user is guide or not is missing. And unsubscribe logic is also not clear.

@daledah introducing new component is fine but I doubt subscribeToActiveGuides() is called before pusher init is complete.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 254 to 262 in a29977b

    
           PusherConnectionManager.init(); 
        
           Pusher.init({ 
        
               appKey: CONFIG.PUSHER.APP_KEY, 
        
               cluster: CONFIG.PUSHER.CLUSTER, 
        
               authEndpoint: `${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/AuthenticatePusher?`, 
        
           }).then(() => { 
        
               User.subscribeToUserEvents(); 
        
           });

@mariapeever I don't see any meaningful difference from @Krishna2323's proposal.

daledah · 2024-08-26T19:41:12Z

@situchan

I updated the proposal to address your feedback:

I doubt subscribeToActiveGuides() is called before pusher init is complete

Also, pls note the condition to subscribe to a new presence pusher:

have a team.expensify.com domain and belongs to a specific policyID

situchan · 2024-08-26T19:46:41Z

@daledah thanks. What is the benefit of introducing new ActiveGuidesEventListener? Can't we just subscribe after subscribeToUserEvents()?

daledah · 2024-08-26T19:50:50Z

Can't we just subscribe after subscribeToUserEvents()

As mentioned in the OP's description, we just want to subscribe to a new presence pusher channel if they have a team.expensify.com domain and belong to a specific policyID. If we subscribe after subscribeToUserEvents(), it will subscribe to presence channel even if user is not in any policy.

situchan · 2024-08-26T19:52:06Z

it will subscribe to presence channel even if user is not in any policy.

Ofc we'll check if user belongs to specific policy

daledah · 2024-08-26T19:55:09Z

Ofc we'll check if user belongs to specific policy

Yeah. If we subscribe after subscribeToUserEvents(), we cannot check if user belongs to specific policy, right?

Krishna2323 · 2024-08-26T19:58:08Z

@situchan, I think we are already disconnecting from the socket when the AuthScreen is unmounted, If we want we can explicitly from the channel. For checking if the user is a guide or not, we can use the email and compare it with CONST.EMAIL.GUIDES_DOMAIN and we will also make sure that the user belongs to a policy by getting the policy using PolicyUtils.getPolicy(policyID) and finding if the member is in the participants list or not.

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 340 in d4d1c0c

Session.cleanupSession();

App/src/libs/actions/Session/index.ts

Lines 698 to 701 in d4d1c0c

    
           function cleanupSession() { 
        
               Pusher.disconnect(); 
        
               Timers.clearAll(); 
        
               Welcome.resetAllChecks();

situchan · 2024-08-26T20:00:00Z

Ofc we'll check if user belongs to specific policy

Yeah. If we subscribe after subscribeToUserEvents(), we cannot check if user belongs to specific policy, right?

Makes sense. We may need to connect ONYXKEYS.COLLECTION.POLICY in AuthScreens

@Krishna2323 what about this concern?

Krishna2323 · 2024-08-26T20:20:04Z

@Krishna2323 what about #47888 (comment)?

I'm not sure why we can't check if user belongs to specific policy, can you please help me understand? Can't we get the policy using useOnyx or usePolicy hook?

situchan · 2024-08-26T20:21:12Z

@Krishna2323 pusher init is called on AuthScreens component mount

daledah · 2024-09-10T10:28:14Z

@MonilBhavsar PR is up.

MonilBhavsar · 2024-09-24T19:29:37Z

Okay, this PR was deployed to production. I'll add a label to assign bug-zero team member to issue payment next week

melvin-bot · 2024-09-24T19:29:55Z

Triggered auto assignment to @trjExpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

melvin-bot · 2024-09-24T19:29:57Z

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

melvin-bot · 2024-09-24T19:30:06Z

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

MonilBhavsar · 2024-09-24T19:31:28Z

@dannymcclain sorry for the ping. This was just to issue the payment :D

trjExpensify · 2024-09-25T01:04:02Z

Wonderful!

melvin-bot · 2024-10-04T18:06:20Z

@trjExpensify, @MonilBhavsar, @situchan, @daledah Whoops! This issue is 2 days overdue. Let's get this updated quick!

trjExpensify · 2024-10-04T18:07:28Z

Payment summary as follows:

$250 to @situchan for the C+ review
$250 to @daledah for the fix

@situchan I've paid you. @daledah can you send me your upwork profile, please?

trjExpensify · 2024-10-04T18:07:38Z

(Great timing, Melv).

daledah · 2024-10-05T03:53:38Z

@trjExpensify Here it is https://www.upwork.com/freelancers/~0138d999529f34d33f

situchan · 2024-10-07T06:08:50Z

@trjExpensify looks like ~~you forgot to release payment~~ there's bug in upwork. Contract ended but I don't see any payment 😕

trjExpensify · 2024-10-07T23:42:58Z

@situchan paid a bonus for the amount that should have been paid due to the Upwork bug.
@daledah sent you an offer.

daledah · 2024-10-08T02:23:49Z

@trjExpensify Thank you I accepted the offer

trjExpensify · 2024-10-08T12:30:33Z

@daledah paid, closing!

MonilBhavsar self-assigned this Aug 22, 2024

MonilBhavsar added the External Added to denote the issue can be worked on by a contributor label Aug 22, 2024

melvin-bot bot changed the title ~~[#fast-apis] Subscribe guides to a new presence pusher channel~~ [$250] [#fast-apis] Subscribe guides to a new presence pusher channel Aug 22, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 22, 2024

melvin-bot bot assigned situchan Aug 22, 2024

melvin-bot bot added the Daily KSv2 label Aug 22, 2024

MonilBhavsar added the Improvement Item broken or needs improvement. label Aug 22, 2024

This comment was marked as duplicate.

Sign in to view

melvin-bot bot added the Overdue label Aug 26, 2024

melvin-bot bot removed the Overdue label Aug 26, 2024

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 10, 2024

MonilBhavsar added the NewFeature Something to build that is a new item. label Sep 24, 2024

melvin-bot bot assigned trjExpensify Sep 24, 2024

melvin-bot bot assigned dannymcclain Sep 24, 2024

MonilBhavsar unassigned dannymcclain Sep 24, 2024

MonilBhavsar changed the title ~~[$250] [#fast-apis] Subscribe guides to a new presence pusher channel~~ [Payment Due Oct 1][$250] [#fast-apis] Subscribe guides to a new presence pusher channel Sep 24, 2024

trjExpensify added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Sep 25, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 1, 2024

melvin-bot bot added the Overdue label Oct 4, 2024

melvin-bot bot removed the Overdue label Oct 7, 2024

trjExpensify closed this as completed Oct 8, 2024

[Payment Due Oct 1][$250] [#fast-apis] Subscribe guides to a new presence pusher channel #47888

[Payment Due Oct 1][$250] [#fast-apis] Subscribe guides to a new presence pusher channel #47888

Comments

MonilBhavsar commented Aug 22, 2024 • edited by trjExpensify Loading

Context:

Problem

Solution

melvin-bot bot commented Aug 22, 2024

melvin-bot bot commented Aug 22, 2024

Krishna2323 commented Aug 22, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

Result

daledah commented Aug 23, 2024 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

This comment was marked as duplicate.

daledah commented Aug 23, 2024 • edited Loading

daledah commented Aug 23, 2024

Proposal updated

MonilBhavsar commented Aug 23, 2024

MonilBhavsar commented Aug 23, 2024

mariapeever commented Aug 24, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Aug 26, 2024

situchan commented Aug 26, 2024

daledah commented Aug 26, 2024

situchan commented Aug 26, 2024

daledah commented Aug 26, 2024

situchan commented Aug 26, 2024

daledah commented Aug 26, 2024

Krishna2323 commented Aug 26, 2024

situchan commented Aug 26, 2024

Krishna2323 commented Aug 26, 2024

situchan commented Aug 26, 2024

daledah commented Sep 10, 2024

MonilBhavsar commented Sep 24, 2024

melvin-bot bot commented Sep 24, 2024

melvin-bot bot commented Sep 24, 2024

melvin-bot bot commented Sep 24, 2024

MonilBhavsar commented Sep 24, 2024

trjExpensify commented Sep 25, 2024

melvin-bot bot commented Oct 4, 2024

trjExpensify commented Oct 4, 2024

trjExpensify commented Oct 4, 2024

daledah commented Oct 5, 2024

situchan commented Oct 7, 2024 • edited Loading

trjExpensify commented Oct 7, 2024

daledah commented Oct 8, 2024

trjExpensify commented Oct 8, 2024

MonilBhavsar commented Aug 22, 2024 •

edited by trjExpensify

Loading

What alternative solutions did you explore? (Optional) 

daledah commented Aug 23, 2024 •

edited

Loading

daledah commented Aug 23, 2024 •

edited

Loading

situchan commented Oct 7, 2024 •

edited

Loading