[Waiting on checklist] [$250] Saved search - Saved search name is blank in the list after clearing name field and saving it

[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. Go to staging.new.expensify.com
2. Go to Search.
3. Click 3-dot menu next to saved search.
4. Click Rename.
5. Clear the name field and save it.

Expected Result:

The saved search in the list will have the default name instead of blank name after saving empty name.

Actual Result:

The saved search in the list shows empty name.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6624129_1728030229123.20241004_162025.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021842161451688398163
• Upwork Job ID: 1842161451688398163
• Last Price Increase: 2024-10-04
• Automatic offers:
• adeel0202 | Contributor | 104543798

Issue Owner

Current Issue Owner: @mollfpr

[melvin-bot]

Triggered auto assignment to @twisterdotcom (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]

@twisterdotcom 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

[abzokhattab]

Edited by proposal-police: This proposal was edited at 2024-10-04 11:46:32 UTC.

Proposal

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

The saved search name appears as blank in the list after the name field is cleared and saved.

What is the root cause of that problem?

We currently allow saving a search with an empty name.

App/src/pages/Search/SavedSearchRenamePage.tsx

Lines 35 to 43 in f04c1e9

	const onSaveSearch = () => {
	const queryJSON = SearchUtils.buildSearchQueryJSON(q || SearchUtils.buildCannedSearchQuery()) ?? ({} as SearchQueryJSON);
	SearchActions.saveSearch({
	queryJSON,
	newName,
	});
	applyFiltersAndNavigate();

.

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

we have two solutions:

1. Either to validate input : We can add a validate function to the FormProvider used in SavedSearchRenamePage to ensure the name field is not empty before submitting. This will prevent saving an empty search name.

2. Or to handle empty input gracefully : If the input is empty, we can skip saving and simply close the modal. This can be implemented by modifying the onSaveSearch function as follows:

const onSaveSearch = () => {
    if (!newName) {
        return Navigation.dismissModal();
    }
    const queryJSON = SearchUtils.buildSearchQueryJSON(q || SearchUtils.buildCannedSearchQuery()) ?? ({} as SearchQueryJSON);

    SearchActions.saveSearch({
        queryJSON,
        newName
    });

    applyFiltersAndNavigate();
};

POC

Screen.Recording.2024-10-04.at.13.32.32.mov

optional: Additionally, we can dismiss the modal if the value has not been changed.

What alternative solutions did you explore? (Optional)

• instead of dismissing the modal and going back the prev page, we can navigate the filter url as we do usually however we need to change this to name: newName || name, this will use the default name if the new name is empty.
• then in the onSaveSearch we should call the saveSearch only if the newName is not empty:

    const onSaveSearch = () => {
        if (newName){
        const queryJSON = SearchUtils.buildSearchQueryJSON(q || SearchUtils.buildCannedSearchQuery()) ?? ({} as SearchQueryJSON);
        SearchActions.saveSearch({
            queryJSON,
            newName
        });
       }
        applyFiltersAndNavigate();
    };

POC

Screen.Recording.2024-10-04.at.13.50.09.mov

[melvin-bot]

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

[melvin-bot]

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

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-10-07 16:09:55 UTC.

Proposal

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

Saved search name is blank in the list after clearing name field and saving it

What is the root cause of that problem?

We currently don't have any implementation in our code to prevent users from saving an empty name.

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

We can return early if !newName

App/src/libs/actions/Search.ts

Line 54 in b0ef513

function saveSearch({queryJSON, newName}: {queryJSON: SearchQueryJSON; newName?: string}) {

  if(!newName){
        return;
    }

What alternative solutions did you explore? (Optional)

We can use isRequiredFulfilled utility function here to validate the new name.
Here we can add the validate function

App/src/pages/Search/SavedSearchRenamePage.tsx

Line 55 in 0c4b2d1

<FormProvider

And in validate function we can use ValidationUtils.isRequiredFulfilled(savedSearchNewName). We can do something like this (pseudo-code)

 const validate = useCallback(
        (values) => {
            const errors = {};
            const savedSearchNewName = values.savedSearchNewName.trim();

            if (!ValidationUtils.isRequiredFulfilled(savedSearchNewName)) {
                errors.savedSearchNewName = 'Required field' // here we can add any existing error from en.ts file or create a new variable
            } 


            return errors;
        },
        [translate],
    );

Optional: I have noticed we don't have any charchter limit on this field either we can add another check in Validate function

else if ([...savedSearchNewName].length > CONST.TITLE_CHARACTER_LIMIT) {
                // Uses the spread syntax to count the number of Unicode code points instead of the number of UTF-16
                // code units.
                ErrorUtils.addErrorMessage(errors, 'savedSearchNewName', translate('common.error.characterLimitExceedCounter', {length: [...savedSearchNewName].length, limit: CONST.TITLE_CHARACTER_LIMIT}));
            }

This is the standard character limit that we use in other pages too. We can use the limit of our choice. Alternatively we can also use maxlength prop to set the limit.

[abzokhattab]

Proposal Updated

• POC added
• Added an alternative proposal

[sher999]

Edited by proposal-police: This proposal was edited at 2024-10-04 20:29:15 UTC.

Proposal

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

Saved search name is blank in the list after clearing name field and saving it

What is the root cause of that problem?

We currently don't have any implementation in our code to prevent users from saving an empty name.

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

Disable the "Save" button when the newName is empty by adding isSubmitDisabled={!newName} in FormProvider this will not save empty newName

          <FormProvider
                formID={ONYXKEYS.FORMS.SEARCH_SAVED_SEARCH_RENAME_FORM}
                submitButtonText={translate('common.save')}
                onSubmit={onSaveSearch}
                style={[styles.mh5, styles.flex1]}
                enabledWhenOffline
                isSubmitDisabled={!newName}
            >

to the FormProvider

POC

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

---

What else we can do?

Validate the FormProvider and throw error using Highlight Fields and Inline Errors and Form Alerts

[sher999]

Proposal updated

Added what else can we do

[github-actions]

tbiplob Your proposal will be dismissed because you did not follow the proposal template.

[AlienistSolution]

Proposal

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

The saved search name appears as blank in the list after the name field is cleared and saved. Instead, you wish for the default name for a saved search, i.e. the query, to be stored as new name, rather than the empty string.

What is the root cause of that problem?

When saving an empty string, the empty string is passed on as is to the SearchActions.

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

It's a very easy fix. SavedSearchRenamePage.tsx already receives the query in the query params as q, regardless of what the name is. This is equal to the default name given to saved searches. All we have to do is add that value as a fallback for when newName turns out to be falsey, in this case an empty string.

[mollfpr]

The saved search in the list will have the default name instead of blank name after saving empty name.

@abzokhattab @Nodebrute @sher999 I don't think preventing saving the empty string is what we are looking for.

[Nodebrute]

@mollfpr Thank you for the review. I have updated my alternative solution can you review it again?

[mollfpr]

Our API seems already handled with the empty string value for newName and using the q value for the default, but this wasn't handled in our app for offline cases.

Screen.Recording.2024-10-07.at.23.03.37.mp4

The proposal from @AlienistSolution seems to have the correct solution. It implements the same logic as the API response and will handle the offline case on the ND side.

🎀 👀 🎀 C+ reviewed!

[melvin-bot]

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

[AlienistSolution]

@mollfpr awesome, happy to help. It's a miniscule change and this is the first time for me doing an Expensify bug. Are you guys gonna fix it yourself since it's so small or am I still supposed to do it? I'm fine either way, but just so I know how we continue from here :)

[chiragsalian]

Proposal LGTM. Feel free to create a PR @AlienistSolution. Let us know if you have any questions along the way 🙂

[melvin-bot]

📣 @AlienistSolution You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @adeel0202 🎉 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.56-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: saved search name is blank after clearing name field #51305

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

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

• @adeel0202 requires payment automatic offer (Contributor)
• @mollfpr requires payment through NewDot Manual Requests

[melvin-bot]

@mollfpr @twisterdotcom The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[twisterdotcom]

Payment Summary:

• @adeel0202 paid $250 here (Contributor)
• @mollfpr paid $250 via NewDot Manual Request

[mollfpr]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: https://github.com/Expensify/App/pull/48566/files#r1858632890

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: Not a critical issue.

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

1. Go to the Search page
2. Save a new search if there isn't any in the list
3. Click the 3-dot menu next to saved search
4. Click Rename
5. Clear the name field and save it
6. Verify that the saved search name is not empty

Do we agree 👍 or 👎

[garrettmknight]

@mollfpr can you fill that checklist out?

[mollfpr]

@garrettmknight Yup, sorry I forgot to update the checklist. I'm doing it now!

[mollfpr]

@garrettmknight The checklist is filled 🚀

[garrettmknight]

$250 approved for @mollfpr