[HOLD for payment 2024-10-24] [$250] Saved search- Default search name in dropdown button shows policy ID when reopening search

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.44-7
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+pso@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Launch New Expensify app.
2. Go to Search.
3. Open workspace switcher and select a workspace.
4. Tap Filters icon.
5. Add any filter.
6. Tap Save search.
7. Note that the search name in the dropdown button and list does not show policy ID.
8. Open dropdown list.
9. Tap on the same saved search again.

Expected Result:

The search name in the dropdown button will not show policy ID.

Actual Result:

The search name in the dropdown button shows policy ID, while the search name in the list does not have policy ID.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6624120_1728029543489.ScreenRecording_10-04-2024_16-03-38_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021843666631417893773
• Upwork Job ID: 1843666631417893773
• Last Price Increase: 2024-10-08
• Automatic offers:
• suneox | Reviewer | 104344570
• Nodebrute | Contributor | 104344571

Issue Owner

Current Issue Owner: @OfstadC

[melvin-bot]

Triggered auto assignment to @OfstadC (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

We think that this bug might be related to #wave-control

[IuliiaHerets]

@OfstadC FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-10-04 16:00:16 UTC.

Proposal

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

Default search name in dropdown button shows policy ID when reopening search

What is the root cause of that problem?

Here we use searchName for title

App/src/pages/Search/SearchTypeMenu.tsx

Line 223 in 7d66ba7

const title = searchName ?? (isCannedQuery ? undefined : SearchUtils.getSearchHeaderTitle(queryJSON, personalDetails, cardList, reports, taxRates));

and we get this searchName from here.

App/src/pages/Search/SearchPageBottomTab.tsx

Line 64 in 7d66ba7

const searchName = searchParams?.name;

It works fine when we use the rename feature. Otherwise, the searchName will be:

type:expense status:all policyID:F038A7F0AD070880 merchant:ok to:18429985

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

We should only use searchName when the user has used the rename feature. We can check if searchParams?.name === searchParams?.q, and if so, we shouldn't pass searchName.

pseudocode

   const isUnchanged =  searchParams?.name === searchParams?.q
    const searchName = isUnchanged ? undefined : searchParams?.name;

Result

Screen.Recording.2024-10-04.at.9.02.49.PM.mov

What alternative solutions did you explore? (Optional)

We can also do this check inside SearchTypeMenu.tsx.

[bernhardoj]

Proposal

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

Saved search name shows policyID when selecting it from the popover.

What is the root cause of that problem?

I think there are 2 problems here.

When we save a new search filter, the query and name will be the same, and if we switch to a workspace before creating the filter, the policyID will be included as the query and name.

App/src/pages/Search/AdvancedSearchFilters.tsx

Lines 261 to 263 in 99f280b

	SearchActions.saveSearch({
	queryJSON,
	});

App/src/libs/actions/Search.ts

Lines 54 to 65 in 99f280b

	function saveSearch({queryJSON, newName}: {queryJSON: SearchQueryJSON; newName?: string}) {
	const saveSearchName = newName ?? queryJSON?.inputQuery ?? '';
	const jsonQuery = JSON.stringify(queryJSON);
	const optimisticData: OnyxUpdate[] = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.SAVED_SEARCHES}`,
	value: {
	[queryJSON.hash]: {
	pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
	name: saveSearchName,

Selecting the saved search from the popover will update the route params, query and name, from the saved search query and name, in our case, it will be the same.

The first problem is the inconsistent name logic between the search type button text,

and the saved search text in the popover.

The search type button text always uses the name if it exists,

App/src/pages/Search/SearchTypeMenu.tsx

Lines 222 to 223 in 99f280b

	if (shouldUseNarrowLayout) {
	const title = searchName ?? (isCannedQuery ? undefined : SearchUtils.getSearchHeaderTitle(queryJSON, personalDetails, cardList, reports, taxRates));

while the saved search text uses the name if the value is different than the query. If it's the same, it will build the name using getSearchHeaderTitle.

App/src/pages/Search/SearchTypeMenu.tsx

Lines 115 to 119 in 99f280b

	let title = item.name;
	if (title === item.query) {
	const jsonQuery = SearchUtils.buildSearchQueryJSON(item.query) ?? ({} as SearchQueryJSON);
	title = SearchUtils.getSearchHeaderTitle(jsonQuery, personalDetails, cardList, reports, taxRates);
	}

The second issue is getSearchHeaderTitle doesn't include the policyID attribute.

App/src/libs/SearchUtils.ts

Lines 766 to 769 in 99f280b

	const {type, status} = queryJSON;
	const filters = queryJSON.flatFilters ?? {};
	let title = `type:${type} status:${status}`;

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

We can update the getSearchHeaderTitle logic to include the policyID and it will make it consistent by showing the policyID.

const {type, status, policyID} = queryJSON;

let title = policyID ? `type:${type} status:${status} policyID:${policyID}` : `type:${type} status:${status}`;

But we can also update the name logic to make it consistent.

App/src/pages/Search/SearchTypeMenu.tsx

Lines 222 to 223 in 99f280b

	if (shouldUseNarrowLayout) {
	const title = searchName ?? (isCannedQuery ? undefined : SearchUtils.getSearchHeaderTitle(queryJSON, personalDetails, cardList, reports, taxRates));

let title: string | undefined = undefined;
if (!isCannedQuery) {
    title = searchName && searchName !== queryJSON.inputQuery 
        ? searchName 
        : SearchUtils.getSearchHeaderTitle(queryJSON, personalDetails, cardList, reports, taxRates);
}

[melvin-bot]

@OfstadC Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[OfstadC]

Added this to Wave Control - posted in Slack for confirmation

[melvin-bot]

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

[melvin-bot]

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

[suneox]

Can anyone reproduce this issue on the latest main?

[bernhardoj]

Still reproducible.

a.mp4

[suneox]

Thanks for all the proposals. Based on the expected behavior for this issue is that The search name in the dropdown button will not show policy ID. and this issue only occurs on small devices.
Therefore, @bernhardoj proposal does not meet the expected behavior.
@Nodebrute proposal LGTM

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @tgolen, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @Nodebrute 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.49-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix search param #50583

If no regressions arise, payment will be issued on 2024-10-24. 🎊

For reference, here are some details about the assignees on this issue:

• @suneox requires payment automatic offer (Reviewer)
• @Nodebrute requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@suneox] The PR that introduced the bug has been identified. Link to the PR:
• [@suneox] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@suneox] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@suneox] Determine if we should create a regression test for this bug.
• [@suneox] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@OfstadC] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[OfstadC]

@suneox Could you please complete the BZ checklist? Thank you ! 😃

[suneox]

Checklist

• [@suneox] The PR that introduced the bug has been identified. Link to the PR: fix: dropdown title does not follow search name #49446
• [@suneox] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment
• [@suneox] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• [@suneox] Determine if we should create a regression test for this bug: N/A
• [@suneox] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. N/A

[OfstadC]

Payment Summary

• @suneox paid $250 via Upwork
• @Nodebrute paid $250 via Upwork