Skip to content

Commit

Permalink
Merge pull request Expensify#24407 from wildan-m/wildan/fix/21518/cli…
Browse files Browse the repository at this point in the history
…ent-pusher-method

Wildan/fix/21518/client pusher method
  • Loading branch information
luacmartins authored Sep 22, 2023
2 parents b50dfac + b2303b2 commit fa6a767
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 15 deletions.
2 changes: 2 additions & 0 deletions src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ const ONYXKEYS = {
REPORT_DRAFT_COMMENT_NUMBER_OF_LINES: 'reportDraftCommentNumberOfLines_',
REPORT_IS_COMPOSER_FULL_SIZE: 'reportIsComposerFullSize_',
REPORT_USER_IS_TYPING: 'reportUserIsTyping_',
REPORT_USER_IS_LEAVING_ROOM: 'reportUserIsLeavingRoom_',
SECURITY_GROUP: 'securityGroup_',
TRANSACTION: 'transactions_',

Expand Down Expand Up @@ -386,6 +387,7 @@ type OnyxValues = {
[ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT_NUMBER_OF_LINES]: number;
[ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE]: boolean;
[ONYXKEYS.COLLECTION.REPORT_USER_IS_TYPING]: boolean;
[ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM]: boolean;
[ONYXKEYS.COLLECTION.SECURITY_GROUP]: OnyxTypes.SecurityGroup;
[ONYXKEYS.COLLECTION.TRANSACTION]: OnyxTypes.Transaction;
[ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_TAGS]: OnyxTypes.RecentlyUsedTags;
Expand Down
1 change: 1 addition & 0 deletions src/libs/Pusher/EventType.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
export default {
REPORT_COMMENT: 'reportComment',
ONYX_API_UPDATE: 'onyxApiUpdate',
USER_IS_LEAVING_ROOM: 'client-userIsLeavingRoom',
USER_IS_TYPING: 'client-userIsTyping',
MULTIPLE_EVENTS: 'multipleEvents',
MULTIPLE_EVENT_TYPE: {
Expand Down
88 changes: 79 additions & 9 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,24 +103,24 @@ function getReportChannelName(reportID) {
}

/**
* There are 2 possibilities that we can receive via pusher for a user's typing status:
* There are 2 possibilities that we can receive via pusher for a user's typing/leaving status:
* 1. The "new" way from New Expensify is passed as {[login]: Boolean} (e.g. {yuwen@expensify.com: true}), where the value
* is whether the user with that login is typing on the report or not.
* is whether the user with that login is typing/leaving on the report or not.
* 2. The "old" way from e.com which is passed as {userLogin: login} (e.g. {userLogin: bstites@expensify.com})
*
* This method makes sure that no matter which we get, we return the "new" format
*
* @param {Object} typingStatus
* @param {Object} status
* @returns {Object}
*/
function getNormalizedTypingStatus(typingStatus) {
let normalizedTypingStatus = typingStatus;
function getNormalizedStatus(status) {
let normalizedStatus = status;

if (_.first(_.keys(typingStatus)) === 'userLogin') {
normalizedTypingStatus = {[typingStatus.userLogin]: true};
if (_.first(_.keys(status)) === 'userLogin') {
normalizedStatus = {[status.userLogin]: true};
}

return normalizedTypingStatus;
return normalizedStatus;
}

/**
Expand All @@ -141,7 +141,7 @@ function subscribeToReportTypingEvents(reportID) {
// If the pusher message comes from OldDot, we expect the typing status to be keyed by user
// login OR by 'Concierge'. If the pusher message comes from NewDot, it is keyed by accountID
// since personal details are keyed by accountID.
const normalizedTypingStatus = getNormalizedTypingStatus(typingStatus);
const normalizedTypingStatus = getNormalizedStatus(typingStatus);
const accountIDOrLogin = _.first(_.keys(normalizedTypingStatus));

if (!accountIDOrLogin) {
Expand Down Expand Up @@ -170,6 +170,41 @@ function subscribeToReportTypingEvents(reportID) {
});
}

/**
* Initialize our pusher subscriptions to listen for someone leaving a room.
*
* @param {String} reportID
*/
function subscribeToReportLeavingEvents(reportID) {
if (!reportID) {
return;
}

// Make sure we have a clean Leaving indicator before subscribing to leaving events
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${reportID}`, false);

const pusherChannelName = getReportChannelName(reportID);
Pusher.subscribe(pusherChannelName, Pusher.TYPE.USER_IS_LEAVING_ROOM, (leavingStatus) => {
// If the pusher message comes from OldDot, we expect the leaving status to be keyed by user
// login OR by 'Concierge'. If the pusher message comes from NewDot, it is keyed by accountID
// since personal details are keyed by accountID.
const normalizedLeavingStatus = getNormalizedStatus(leavingStatus);
const accountIDOrLogin = _.first(_.keys(normalizedLeavingStatus));

if (!accountIDOrLogin) {
return;
}

if (Number(accountIDOrLogin) !== currentUserAccountID) {
return;
}

Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${reportID}`, true);
}).catch((error) => {
Log.hmmm('[Report] Failed to initially subscribe to Pusher channel', false, {errorType: error.type, pusherChannelName});
});
}

/**
* Remove our pusher subscriptions to listen for someone typing in a report.
*
Expand All @@ -185,6 +220,21 @@ function unsubscribeFromReportChannel(reportID) {
Pusher.unsubscribe(pusherChannelName, Pusher.TYPE.USER_IS_TYPING);
}

/**
* Remove our pusher subscriptions to listen for someone leaving a report.
*
* @param {String} reportID
*/
function unsubscribeFromLeavingRoomReportChannel(reportID) {
if (!reportID) {
return;
}

const pusherChannelName = getReportChannelName(reportID);
Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${reportID}`, false);
Pusher.unsubscribe(pusherChannelName, Pusher.TYPE.USER_IS_LEAVING_ROOM);
}

// New action subscriber array for report pages
let newActionSubscribers = [];

Expand Down Expand Up @@ -865,6 +915,17 @@ function broadcastUserIsTyping(reportID) {
typingStatus[currentUserAccountID] = true;
Pusher.sendEvent(privateReportChannelName, Pusher.TYPE.USER_IS_TYPING, typingStatus);
}
/**
* Broadcasts to the report's private pusher channel whether a user is leaving a report
*
* @param {String} reportID
*/
function broadcastUserIsLeavingRoom(reportID) {
const privateReportChannelName = getReportChannelName(reportID);
const leavingStatus = {};
leavingStatus[currentUserAccountID] = true;
Pusher.sendEvent(privateReportChannelName, Pusher.TYPE.USER_IS_LEAVING_ROOM, leavingStatus);
}

/**
* When a report changes in Onyx, this fetches the report from the API if the report doesn't have a name
Expand Down Expand Up @@ -1781,6 +1842,12 @@ function getCurrentUserAccountID() {
function leaveRoom(reportID) {
const report = lodashGet(allReports, [reportID], {});
const reportKeys = _.keys(report);

// Pusher's leavingStatus should be sent earlier.
// Place the broadcast before calling the LeaveRoom API to prevent a race condition
// between Onyx report being null and Pusher's leavingStatus becoming true.
broadcastUserIsLeavingRoom(reportID);

API.write(
'LeaveRoom',
{
Expand Down Expand Up @@ -2071,10 +2138,13 @@ export {
updateWriteCapabilityAndNavigate,
updateNotificationPreferenceAndNavigate,
subscribeToReportTypingEvents,
subscribeToReportLeavingEvents,
unsubscribeFromReportChannel,
unsubscribeFromLeavingRoomReportChannel,
saveReportComment,
saveReportCommentNumberOfLines,
broadcastUserIsTyping,
broadcastUserIsLeavingRoom,
togglePinnedState,
editReportComment,
handleUserDeletedLinksInHtml,
Expand Down
57 changes: 51 additions & 6 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ const propTypes = {
/** All of the personal details for everyone */
personalDetails: PropTypes.objectOf(personalDetailsPropType),

/** Whether user is leaving the current report */
userLeavingStatus: PropTypes.bool,

...windowDimensionsPropTypes,
...viewportOffsetTopPropTypes,
...withCurrentReportIDPropTypes,
Expand All @@ -105,6 +108,7 @@ const defaultProps = {
betas: [],
policies: {},
accountManagerReportID: null,
userLeavingStatus: false,
personalDetails: {},
...withCurrentReportIDDefaultProps,
};
Expand Down Expand Up @@ -145,12 +149,14 @@ function ReportScreen({
viewportOffsetTop,
isComposerFullSize,
errors,
userLeavingStatus,
currentReportID,
}) {
const firstRenderRef = useRef(true);
const flatListRef = useRef();
const reactionListRef = useRef();
const prevReport = usePrevious(report);
const prevUserLeavingStatus = usePrevious(userLeavingStatus);

const [skeletonViewContainerHeight, setSkeletonViewContainerHeight] = useState(0);
const [isBannerVisible, setIsBannerVisible] = useState(true);
Expand All @@ -175,6 +181,7 @@ function ReportScreen({
const policy = policies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`];

const isTopMostReportId = currentReportID === getReportID(route);
const didSubscribeToReportLeavingEvents = useRef(false);

const isDefaultReport = checkDefaultReport(report);

Expand Down Expand Up @@ -234,7 +241,7 @@ function ReportScreen({
}

// It possible that we may not have the report object yet in Onyx yet e.g. we navigated to a URL for an accessible report that
// is not stored locally yet. If props.report.reportID exists, then the report has been stored locally and nothing more needs to be done.
// is not stored locally yet. If report.reportID exists, then the report has been stored locally and nothing more needs to be done.
// If it doesn't exist, then we fetch the report from the API.
if (report.reportID && report.reportID === getReportID(route)) {
return;
Expand Down Expand Up @@ -282,6 +289,14 @@ function ReportScreen({
useEffect(() => {
fetchReportIfNeeded();
ComposerActions.setShouldShowComposeInput(true);
return () => {
if (!didSubscribeToReportLeavingEvents) {
return;
}

Report.unsubscribeFromLeavingRoomReportChannel(report.reportID);
};

// I'm disabling the warning, as it expects to use exhaustive deps, even though we want this useEffect to run only on the first render.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand All @@ -292,24 +307,51 @@ function ReportScreen({
firstRenderRef.current = false;
return;
}

const onyxReportID = report.reportID;
const prevOnyxReportID = prevReport.reportID;
const routeReportID = getReportID(route);

// Navigate to the Concierge chat if the room was removed from another device (e.g. user leaving a room)
if (
// non-optimistic case
(!prevUserLeavingStatus && userLeavingStatus) ||
// optimistic case
(prevOnyxReportID && prevOnyxReportID === routeReportID && !onyxReportID && prevReport.statusNum === CONST.REPORT.STATUS.OPEN && report.statusNum === CONST.REPORT.STATUS.CLOSED)
) {
Navigation.goBack();
Report.navigateToConciergeChat();
return;
}

// If you already have a report open and are deeplinking to a new report on native,
// the ReportScreen never actually unmounts and the reportID in the route also doesn't change.
// Therefore, we need to compare if the existing reportID is the same as the one in the route
// before deciding that we shouldn't call OpenReport.
const onyxReportID = report.reportID;
const routeReportID = getReportID(route);
if (onyxReportID === prevReport.reportID && (!onyxReportID || onyxReportID === routeReportID)) {
return;
}

fetchReportIfNeeded();
ComposerActions.setShouldShowComposeInput(true);
}, [route, report, errors, fetchReportIfNeeded, prevReport.reportID]);
}, [route, report, errors, fetchReportIfNeeded, prevReport.reportID, prevUserLeavingStatus, userLeavingStatus, prevReport.statusNum]);

useEffect(() => {
// Ensures subscription event succeeds when the report/workspace room is created optimistically.
// Check if the optimistic `OpenReport` or `AddWorkspaceRoom` has succeeded by confirming
// any `pendingFields.createChat` or `pendingFields.addWorkspaceRoom` fields are set to null.
// Existing reports created will have empty fields for `pendingFields`.
const didCreateReportSuccessfully = !report.pendingFields || (!report.pendingFields.addWorkspaceRoom && !report.pendingFields.createChat);
if (!didSubscribeToReportLeavingEvents.current && didCreateReportSuccessfully) {
Report.subscribeToReportLeavingEvents(reportID);
didSubscribeToReportLeavingEvents.current = true;
}
}, [report, didSubscribeToReportLeavingEvents, reportID]);

// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = useMemo(
() => (!_.isEmpty(report) && !isDefaultReport && !report.reportID && !isOptimisticDelete && !report.isLoadingReportActions && !isLoading) || shouldHideReport,
[report, isLoading, shouldHideReport, isDefaultReport, isOptimisticDelete],
() => (!_.isEmpty(report) && !isDefaultReport && !report.reportID && !isOptimisticDelete && !report.isLoadingReportActions && !isLoading && !userLeavingStatus) || shouldHideReport,
[report, isLoading, shouldHideReport, isDefaultReport, isOptimisticDelete, userLeavingStatus],
);

return (
Expand Down Expand Up @@ -451,5 +493,8 @@ export default compose(
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
userLeavingStatus: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${getReportID(route)}`,
},
}),
)(ReportScreen);

0 comments on commit fa6a767

Please sign in to comment.