[WAITING FOR PAYMENT MAY 29] [$250] Categories & Tags - App closes RHP instead of returning to previous RHP after saving name

[m-natarajan]

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: 1.4.68-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings.
3. Go to Taxes.
4. Click on the tax rate > Name.
5. Edit the name and save it.
6. Note that RHP still opens after saving the name. The same behavior is seen with distance rates.
7. Go to Categories.
8. Click on the category > Category name.
9. Edit the category name and save it.
10. Repeat Step 7-9 with tags.

Expected Result:

App will return to previous RHP (with toggle, three-dot menu and name) after saving the category name, which is the behavior with taxes and distance rates.

Actual Result:

RHP closes after saving category and tag name.

Workaround:
Unknown

Platforms:
Which of our officially supported platforms is this issue occurring on?

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence

Bug6466672_1714486645846.20240430_221313.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @abekkala (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.

[m-natarajan]

@abekkala 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.

[m-natarajan]

We think this bug might be related to #wave-collect - Release 1

[neonbhai]

Proposal

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

App closes RHP instead of returning to previous RHP after saving name

What is the root cause of that problem?

We dismiss the RHP explicitly when editing Category in CategoryForm

App/src/pages/workspace/categories/CategoryForm.tsx

Line 63 in 35772b2

Navigation.dismissModal();

This is called after submitting the form.

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

We should call Navigation.goBack() instead of Navigation.dismissModal() here:

App/src/pages/workspace/categories/CategoryForm.tsx

Line 63 in 35772b2

Navigation.dismissModal();

Alternatively:

We can remove Navigation.dismissModal() from CategoryForm and handle the navigation in EditCategoryPage:
add here:

Navigation.goBack(ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(policyID, categoryName));

[Nodebrute]

Proposal

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

Categories & Tags - App closes RHP instead of returning to previous RHP after saving name

What is the root cause of that problem?

We are dismissing modal instead of goBack

App/src/pages/workspace/categories/CategoryForm.tsx

Line 63 in 35772b2

Navigation.dismissModal();

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

Remove Navigation.dismissModal()

App/src/pages/workspace/categories/CategoryForm.tsx

Line 63 in 35772b2

Navigation.dismissModal();

App/src/pages/workspace/categories/EditCategoryPage.tsx

Lines 53 to 55 in 05793d1

	if (currentCategoryName !== newCategoryName) {
	Policy.renamePolicyCategory(route.params.policyID, {oldName: currentCategoryName, newName: values.categoryName});
	}

In the code block above, use Navigation.goBack with true as the second argument. If we just use goBack(), the URL will change due to the name change, causing the user to be redirected to a 'not found' page. We can do something like this

Navigation.goBack(ROUTES.WORKSPACE_CATEGORY_SETTINGS.getRoute(route.params.policyID, newCategoryName),true);

Optional
Instead of adding navigation logic in EditCategoryPage.tsx we can add it in the place of Navigation.dismissModal().

App/src/pages/workspace/categories/CategoryForm.tsx

Line 63 in 35772b2

Navigation.dismissModal();

We should do the same thing for tags

App/src/pages/workspace/tags/EditTagPage.tsx

Lines 56 to 67 in 35772b2

	const editTag = useCallback(
	(values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_TAG_FORM>) => {
	const tagName = values.tagName.trim();
	// Do not call the API if the edited tag name is the same as the current tag name
	if (currentTagName !== tagName) {
	Policy.renamePolicyTag(route.params.policyID, {oldName: currentTagName, newName: values.tagName.trim()});
	}
	Keyboard.dismiss();
	Navigation.dismissModal();
	},
	[route.params.policyID, currentTagName],
	);

Note

We should use new category name or new tag name instead of categoryName/currentTagName

What alternative solutions did you explore?

We can use Navigation.navigate here.

[melvin-bot]

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

[melvin-bot]

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

[abekkala]

@akinwale we've already received some proposals! 🎉

[nkdengineer]

Proposal

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

Categories & Tags - App closes RHP instead of returning to previous RHP after saving name

What is the root cause of that problem?

We dismiss the modal here so after editing category/tag name, App closes RHP

App/src/pages/workspace/categories/CategoryForm.tsx

Line 63 in 0ae357e

Navigation.dismissModal();

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

1. We should use goBack method here so we can navigate to the previous RHP

App/src/pages/workspace/categories/CategoryForm.tsx

Line 63 in 0ae357e

Navigation.dismissModal();

2. But if we do that, the not found page will appear because the old category name is removed. To fix this we can update a new field in optimisticData like previousCategoryName here

App/src/libs/actions/Policy.ts

Line 3232 in 0ae357e

[policyCategory.newName]: {

and we will use this to get the new category in CategorySettingPage

const policyCategory = policyCategories?.[route.params.categoryName] ?? Object.values(policyCategories ?? {}).find((category) => category.previousCategoryName === route.params.categoryName);

App/src/pages/workspace/categories/CategorySettingsPage.tsx

Line 43 in 0ae357e

const policyCategory = policyCategories?.[route.params.categoryName];

OPTIONAL: In CategorySettingsPage, we can also change the categoryName param if the new category name is updated

useEffect(() => {
    if (policyCategory?.name === route.params.categoryName || !policyCategory) {
        return;
    }
    navigation.setParams({categoryName: policyCategory?.name});
}, [route.params.categoryName, policyCategory?.name])

Do the same way for Tag page

Note: We should not fallback goBack to the CategorySettingsPage with new category name param because if we do that when we click on back button again after going back to CategorySettingsPage not found page will appear.

What alternative solutions did you explore? (Optional)

For point 2: we can change the struct of policyCategories and policyTags the same as taxRates and then when we edit the name or other fields we will edit the value of the category and not change the id. But it will be a big refactor and require the backend change.

[category_id]: Category

[akinwale]

@Nodebrute's proposal makes sense here.

🎀👀🎀 C+ reviewed.

[melvin-bot]

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

[nkdengineer]

@akinwale The selected proposal will cause the regression as this video. When we edit tag and back to tag setting page and click on the back button, not found page will appear instead of go back to tags page.

Can you please check my proposal #41339 (comment)? It can cover this case as well.

Screen.Recording.2024-05-05.at.19.57.23.mov

[akinwale]

@akinwale The selected proposal will cause the regression as this video. When we edit tag and back to tag setting page and click on the back button, not found page will appear instead of go back to tags page.

I tested the proposal with the second parameter for goBack set to true, and it doesn't navigate to the not here page. However, there is another regression with that approach where the "Not here" page flickers for a bit before it actually displays the previous screen.

Considering this, @nkdengineer's proposal has the correct approach to solving the issue.

[blimpich]

👍 Sounds good. Feel free to raise a PR @nkdengineer. For some reason the automation for commenting on this issue with a confirmation of the contract offer didn't work, but we can proceed business as usual and make sure that everything gets paid out as normal.

[nkdengineer]

The PR is here.

[abekkala]

Deployed to Production May 22
The payment date would be May 29, pending no regressions

[abekkala]

PAYMENT SUMMARY:

• Fix: @nkdengineer [$250] Offer Link
• PR Review: @akinwale [$250] Offer Link

[abekkala]

@akinwale payment sent and contract ended - thank you! 🎉

[nkdengineer]

@abekkala I accepted the offer, thanks!

[abekkala]

@nkdengineer payment sent and contract ended - thank you! 🎉