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

Jump to bottom

Update hold flow on the search page #49003

Closed
wants to merge 6 commits into from
Closed

Update hold flow on the search page #49003

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5dad945
change the API
cretadn22 Sep 11, 2024
ff85f75
update new main
cretadn22 Sep 17, 2024
7bf2c3b
add optimistic data
cretadn22 Sep 17, 2024
c1ee919
resolve lint, TS errors
cretadn22 Sep 17, 2024
b032b46
fix bug
cretadn22 Sep 18, 2024
6c3c9b9
fix route
cretadn22 Sep 18, 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
11 changes: 10 additions & 1 deletion src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,16 @@ const ROUTES = {
return `search/view/${reportID}` as const;
},
},
TRANSACTION_HOLD_REASON_RHP: 'search/hold',

TRANSACTION_HOLD_REASON_RHP: {
route: 'search/hold/:transactionID?',
getRoute: (transactionID?: string) => {
if (transactionID) {
return `search/hold/${transactionID}` as const;
}
return `search/hold` as const;
},
},

// This is a utility route used to go to the user's concierge chat, or the sign-in page if the user's not authenticated
CONCIERGE: 'concierge',
Expand Down
6 changes: 3 additions & 3 deletions src/components/PromotedActionsBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type PromotedActionsType = Record<BasePromotedActions, (report: OnyxReport) => P
reportID?: string;
isDelegateAccessRestricted: boolean;
setIsNoDelegateAccessMenuVisible: (isVisible: boolean) => void;
currentSearchHash: number;
}) => PromotedAction;
};

Expand Down Expand Up @@ -76,7 +77,7 @@ const PromotedActions = {
}
},
}),
hold: ({isTextHold, reportAction, reportID, isDelegateAccessRestricted, setIsNoDelegateAccessMenuVisible}) => ({
hold: ({isTextHold, reportAction, reportID, isDelegateAccessRestricted, setIsNoDelegateAccessMenuVisible, currentSearchHash}) => ({
key: CONST.PROMOTED_ACTIONS.HOLD,
icon: Expensicons.Stopwatch,
text: Localize.translateLocal(`iou.${isTextHold ? 'hold' : 'unhold'}`),
Expand All @@ -91,13 +92,12 @@ const PromotedActions = {
}
const targetedReportID = reportID ?? reportAction?.childReportID ?? '';
const topmostCentralPaneRoute = getTopmostCentralPaneRoute(navigationRef.getRootState() as State<RootStackParamList>);

if (topmostCentralPaneRoute?.name !== SCREENS.SEARCH.CENTRAL_PANE && isTextHold) {
ReportUtils.changeMoneyRequestHoldStatus(reportAction, ROUTES.REPORT_WITH_ID.getRoute(targetedReportID));
return;
}

ReportUtils.changeMoneyRequestHoldStatus(reportAction, ROUTES.SEARCH_REPORT.getRoute(targetedReportID));
ReportUtils.changeMoneyRequestHoldStatus(reportAction, ROUTES.SEARCH_REPORT.getRoute(targetedReportID), currentSearchHash);
},
}),
} satisfies PromotedActionsType;
Expand Down
2 changes: 1 addition & 1 deletion src/components/Search/SearchPageHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ function SearchPageHeader({queryJSON, hash, onSelectDeleteOption, setOfflineModa
return;
}

Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP);
Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP.getRoute());
},
});
}
Expand Down
1 change: 0 additions & 1 deletion src/components/Search/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ function Search({queryJSON}: SearchProps) {
const {type, status, sortBy, sortOrder, hash} = queryJSON;

const [currentSearchResults] = useOnyx(`${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`);

const canSelectMultiple = isSmallScreenWidth ? !!selectionMode?.isEnabled : true;

useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/linkingConfig/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,7 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
[SCREENS.RIGHT_MODAL.SEARCH_REPORT]: {
screens: {
[SCREENS.SEARCH.REPORT_RHP]: ROUTES.SEARCH_REPORT.route,
[SCREENS.SEARCH.TRANSACTION_HOLD_REASON_RHP]: ROUTES.TRANSACTION_HOLD_REASON_RHP,
[SCREENS.SEARCH.TRANSACTION_HOLD_REASON_RHP]: ROUTES.TRANSACTION_HOLD_REASON_RHP.route,
},
},
[SCREENS.RIGHT_MODAL.SEARCH_ADVANCED_FILTERS]: {
Expand Down
11 changes: 10 additions & 1 deletion src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import * as IOU from './actions/IOU';
import * as PolicyActions from './actions/Policy/Policy';
import * as store from './actions/ReimbursementAccount/store';
import * as SearchActions from './actions/Search';
import * as SessionUtils from './actions/Session';
import * as CurrencyUtils from './CurrencyUtils';
import DateUtils from './DateUtils';
Expand Down Expand Up @@ -1536,7 +1537,7 @@
*/
function isExpenseRequest(report: OnyxInputOrEntry<Report>): boolean {
if (isThread(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);

Check failure on line 1540 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
const parentReport = ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`];
return isExpenseReport(parentReport) && !isEmptyObject(parentReportAction) && ReportActionsUtils.isTransactionThread(parentReportAction);
}
Expand All @@ -1549,7 +1550,7 @@
*/
function isIOURequest(report: OnyxInputOrEntry<Report>): boolean {
if (isThread(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);

Check failure on line 1553 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
const parentReport = ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`];
return isIOUReport(parentReport) && !isEmptyObject(parentReportAction) && ReportActionsUtils.isTransactionThread(parentReportAction);
}
Expand All @@ -1562,7 +1563,7 @@
*/
function isTrackExpenseReport(report: OnyxInputOrEntry<Report>): boolean {
if (isThread(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);

Check failure on line 1566 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
return !isEmptyObject(parentReportAction) && ReportActionsUtils.isTrackExpenseAction(parentReportAction);
}
return false;
Expand Down Expand Up @@ -2211,7 +2212,7 @@
return [fallbackIcon];
}
if (isExpenseRequest(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);

Check failure on line 2215 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: personalDetails?.[parentReportAction?.actorAccountID ?? -1]?.avatar ?? FallbackAvatar,
Expand All @@ -2224,7 +2225,7 @@
return [memberIcon, workspaceIcon];
}
if (isChatThread(report)) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);

Check failure on line 2228 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead

const actorAccountID = getReportActionActorAccountID(parentReportAction, report);
const actorDisplayName = PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[actorAccountID ?? -1], '', false);
Expand Down Expand Up @@ -3105,7 +3106,7 @@
const transactionID = moneyRequestReport ? ReportActionsUtils.getOriginalMessage(reportAction)?.IOUTransactionID : 0;
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? ({} as Transaction);

const parentReportAction = ReportActionsUtils.getParentReportAction(moneyRequestReport);

Check failure on line 3109 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead

const isRequestIOU = isIOUReport(moneyRequestReport);
const isHoldActionCreator = isHoldCreator(transaction, reportAction.childReportID ?? '-1');
Expand Down Expand Up @@ -3134,7 +3135,7 @@
return {canHoldRequest, canUnholdRequest};
}

const changeMoneyRequestHoldStatus = (reportAction: OnyxEntry<ReportAction>, backTo?: string): void => {
const changeMoneyRequestHoldStatus = (reportAction: OnyxEntry<ReportAction>, backTo?: string, currentSearchHash?: number): void => {
if (!ReportActionsUtils.isMoneyRequestAction(reportAction)) {
return;
}
Expand All @@ -3151,8 +3152,16 @@
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${moneyRequestReport.policyID}`] ?? null;

if (isOnHold) {
if (currentSearchHash) {
SearchActions.unholdMoneyRequestOnSearch(currentSearchHash, [transactionID]);
return;
}
IOU.unholdRequest(transactionID, reportAction.childReportID ?? '');
} else {
if (currentSearchHash) {
Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP.getRoute(transactionID));
return;
}
const activeRoute = encodeURIComponent(Navigation.getActiveRouteWithoutParams());
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Navigation.navigate(ROUTES.MONEY_REQUEST_HOLD_REASON.getRoute(policy?.type ?? CONST.POLICY.TYPE.PERSONAL, transactionID, reportAction.childReportID ?? '', backTo || activeRoute));
Expand Down Expand Up @@ -3712,7 +3721,7 @@
}

let formattedName: string | undefined;
const parentReportAction = parentReportActionParam ?? ReportActionsUtils.getParentReportAction(report);

Check failure on line 3724 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
const parentReportActionMessage = ReportActionsUtils.getReportActionMessage(parentReportAction);

if (parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.SUBMITTED) {
Expand Down Expand Up @@ -4248,7 +4257,7 @@

// These parameters are not saved on the reportAction, but are used to display the task in the UI
// Added when we fetch the reportActions on a report
reportAction.reportAction.originalMessage = {

Check failure on line 4260 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'originalMessage' is deprecated. Used in old report actions before migration. Replaced by using getOriginalMessage function
html: ReportActionsUtils.getReportActionHtml(reportAction.reportAction),
taskReportID: ReportActionsUtils.getReportActionMessage(reportAction.reportAction)?.taskReportID,
whisperedTo: [],
Expand Down Expand Up @@ -5994,7 +6003,7 @@
if (
!report?.reportID ||
!report?.type ||
report?.reportName === undefined ||

Check failure on line 6006 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
report?.isHidden ||
(!report?.participants &&
Expand Down
22 changes: 22 additions & 0 deletions src/libs/actions/Search.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,34 @@ function createTransactionThread(hash: number, transactionID: string, reportID:
function holdMoneyRequestOnSearch(hash: number, transactionIDList: string[], comment: string) {
const {optimisticData, finallyData} = getOnyxLoadingData(hash);

optimisticData.push({
Copy link
Contributor

Choose a reason for hiding this comment

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

We need failureData too in case the API request doesn't succeed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should probably also:

  1. Add an explanatory comment here
  2. Only do these optimistic updates for the issue's case, e.g. via a new param isSingleTransaction
  3. Optimistically add the comment to IOU's report (this can probably be copied from HoldRequest as this is what currently happens on staging when (un/)holding whilst offline)
    image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjcoffee If we add a comment to the report optimistically, the backend returns the same comment, resulting in a duplicate. To do that, we need to update the backend to return the same optimistic reportActionID so that we no longer get a duplicate comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjcoffee I prefer not to add comments optimistically. Do you agree to remove optimistic report action?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah it doesn't seem useful if it gets duplicated. Let's revert for now whilst we decide on the approach.

onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDList[0]}`,
value: {
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
comment: {
hold: CONST.POLICY.ID_FAKE,
},
},
});

API.write(WRITE_COMMANDS.HOLD_MONEY_REQUEST_ON_SEARCH, {hash, transactionIDList, comment}, {optimisticData, finallyData});
}

function unholdMoneyRequestOnSearch(hash: number, transactionIDList: string[]) {
const {optimisticData, finallyData} = getOnyxLoadingData(hash);

optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionIDList[0]}`,
value: {
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
comment: {
hold: null,
},
},
});

API.write(WRITE_COMMANDS.UNHOLD_MONEY_REQUEST_ON_SEARCH, {hash, transactionIDList}, {optimisticData, finallyData});
}

Expand Down
15 changes: 14 additions & 1 deletion src/pages/ReportDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import PromotedActionsBar, {PromotedActions} from '@components/PromotedActionsBa
import RoomHeaderAvatars from '@components/RoomHeaderAvatars';
import ScreenWrapper from '@components/ScreenWrapper';
import ScrollView from '@components/ScrollView';
import {useSearchContext} from '@components/Search/SearchContext';
import Text from '@components/Text';
import useDelegateUserDetails from '@hooks/useDelegateUserDetails';
import useLocalize from '@hooks/useLocalize';
Expand Down Expand Up @@ -85,6 +86,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD
const {translate} = useLocalize();
const {isOffline} = useNetwork();
const styles = useThemeStyles();
const {currentSearchHash} = useSearchContext();

// The app would crash due to subscribing to the entire report collection if parentReportID is an empty string. So we should have a fallback ID here.
/* eslint-disable @typescript-eslint/prefer-nullish-coalescing */
Expand Down Expand Up @@ -560,6 +562,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD
reportID: transactionThreadReportID ? report.reportID : moneyRequestAction?.childReportID ?? '-1',
isDelegateAccessRestricted,
setIsNoDelegateAccessMenuVisible,
currentSearchHash,
}),
);
}
Expand All @@ -571,7 +574,17 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD
result.push(PromotedActions.share(report));

return result;
}, [report, moneyRequestAction, canJoin, isExpenseReport, shouldShowHoldAction, canHoldUnholdReportAction.canHoldRequest, transactionThreadReportID, isDelegateAccessRestricted]);
}, [
report,
moneyRequestAction,
canJoin,
isExpenseReport,
shouldShowHoldAction,
canHoldUnholdReportAction.canHoldRequest,
transactionThreadReportID,
isDelegateAccessRestricted,
currentSearchHash,
]);

const nameSectionExpenseIOU = (
<View style={[styles.reportDetailsRoomInfo, styles.mw100]}>
Expand Down
9 changes: 5 additions & 4 deletions src/pages/Search/SearchHoldReasonPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import INPUT_IDS from '@src/types/form/MoneyRequestHoldReasonForm';
type SearchHoldReasonPageRouteParams = {
/** Link to previous page */
backTo: Route;

transactionID: string;
};

type SearchHoldReasonPageProps = {
Expand All @@ -25,12 +27,11 @@ type SearchHoldReasonPageProps = {
function SearchHoldReasonPage({route}: SearchHoldReasonPageProps) {
const {translate} = useLocalize();

const {currentSearchHash, selectedTransactions} = useSearchContext();
const {backTo = ''} = route.params ?? {};
const {currentSearchHash} = useSearchContext();
const {backTo = '', transactionID = ''} = route.params ?? {};

const selectedTransactionIDs = Object.keys(selectedTransactions);
const onSubmit = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.MONEY_REQUEST_HOLD_FORM>) => {
SearchActions.holdMoneyRequestOnSearch(currentSearchHash, selectedTransactionIDs, values.comment);
SearchActions.holdMoneyRequestOnSearch(currentSearchHash, [transactionID], values.comment);
cretadn22 marked this conversation as resolved.
Show resolved Hide resolved
Navigation.goBack();
};

Expand Down
Loading