Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check if client is active before applying pusher updates #39990

Merged
merged 5 commits into from
Apr 10, 2024
Merged

Add check if client is active before applying pusher updates #39990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62f8e12
adding checks on pusher subscriptions
danieldoglas Apr 10, 2024
917d5e4
added check to urban airship notifications
danieldoglas Apr 10, 2024
45268af
adding log line
danieldoglas Apr 10, 2024
99eec24
adding log lines
danieldoglas Apr 10, 2024
7cd2871
prettier
danieldoglas Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/libs/Notification/PushNotification/subscribeToReportCommentPushNotifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Onyx from 'react-native-onyx';
import * as OnyxUpdates from '@libs/actions/OnyxUpdates';
import * as ActiveClientManager from '@libs/ActiveClientManager';
import Log from '@libs/Log';
import Navigation from '@libs/Navigation/Navigation';
import getPolicyMemberAccountIDs from '@libs/PolicyMembersUtils';
Expand Down Expand Up @@ -30,6 +31,10 @@ Onyx.connect({
*/
export default function subscribeToReportCommentPushNotifications() {
PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, reportActionID, onyxData, lastUpdateID, previousUpdateID}) => {
if (!ActiveClientManager.isClientTheLeader()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an FYI, mobile is always the leader (there's only one client) so we'll never hit this code block

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. This is not creating a problem, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope just noticed this while refactoring. I'll probably remove it.

Log.info('[PushNotification] received report comment notification, but ignoring it since this is not the active client');
return;
}
Log.info(`[PushNotification] received report comment notification in the ${Visibility.isVisible() ? 'foreground' : 'background'}`, false, {reportID, reportActionID});

if (onyxData && lastUpdateID && previousUpdateID) {
Expand Down
13 changes: 12 additions & 1 deletion src/libs/actions/User.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {isBefore} from 'date-fns';
import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import * as ActiveClientManager from '@libs/ActiveClientManager';
import * as API from '@libs/API';
import type {
AddNewContactMethodParams,
Expand Down Expand Up @@ -583,6 +584,11 @@ function subscribeToUserEvents() {
// 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)
// Example: {lastUpdateID: 1, previousUpdateID: 0, updates: [{onyxMethod: 'whatever', key: 'foo', value: 'bar'}]}
const updates = {
Expand All @@ -599,10 +605,15 @@ function subscribeToUserEvents() {
playSoundForMessageType(pushJSON);

return SequentialQueue.getCurrentRequest().then(() => {
// If we don't have the currentUserAccountID (user is logged out) we don't want to update Onyx with data from Pusher
// If we don't have the currentUserAccountID (user is logged out) or this is not the
// main client we don't want to update Onyx with data from Pusher
if (currentUserAccountID === -1) {
return;
}
if (!ActiveClientManager.isClientTheLeader()) {
Log.info('[Pusher] Received updates, but ignoring it since this is not the active client');
return;
}

const onyxUpdatePromise = Onyx.update(pushJSON).then(() => {
triggerNotifications(pushJSON);
Expand Down
Loading