[HOLD for payment 2024-04-05] [$250] Web - Inconsistent behavior when saving existing name without edit

[kbecciv]

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.55-0
Reproducible in staging?: y
Reproducible in production?: new feature
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• User is admin of Collect workspace.
• The Collect workspace has categories and tags.

1. Go to staging.new.expensify.com.
2. Go to Profile > Workspaces > Collect workspace.
3. Go to Categories.
4. Click on any category.
5. Click Category name.
6. Save the name without editing.
7. Note that an error shows up.
8. Go to Tags.
9. Click on any tag.
10. Click Tag name.
11. Save the name without editing.

Expected Result:

When saving category and tag name without editing, the behavior should be consistent. If error is meant to show up, it should show up for both. Otherwise, there should be no error for both.

For any feature - Category, tag, distance or tax, when the name is being changed but no change has been done, do nothing and just close the modal (skip the API call too)

Actual Result:

In Step 6, error shows up when saving category name without editing it.
In Step 11, no error shows up when saving tag name without editing it.

Workaround:

n/a

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

Bug6420400_1710936083255.20240320_194746.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017d373a5e049d77c6
• Upwork Job ID: 1770442815835451392
• Last Price Increase: 2024-03-20
• Automatic offers:
• GandalfGwaihir | Contributor | 0
• shubham1206agra | Contributor | 0

Issue Owner

Current Issue Owner: @shubham1206agra

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @cristipaval (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[mountiny]

This is not a blocker, its a new feature, I can take over.

[allgandalf]

Proposal

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

Categories & Tags - Inconsistent behavior when saving existing name without edit

What is the root cause of that problem?

We have duplicate tag check but it is wrong:

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

Lines 41 to 50 in 0fa4d0f

	const validate = useCallback(
	(values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_TAG_FORM>) => {
	const errors: FormInputErrors<typeof ONYXKEYS.FORMS.WORKSPACE_TAG_FORM> = {};
	const tagName = values.tagName.trim();
	const {tags} = PolicyUtils.getTagList(policyTags, 0);
	if (!ValidationUtils.isRequiredFulfilled(tagName)) {
	errors.tagName = 'workspace.tags.tagRequiredError';
	} else if (tags?.[tagName] && currentTagName !== tagName) {
	errors.tagName = 'workspace.tags.existingTagError';
	}

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

Update duplicate category check similar to tag check:

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

Lines 42 to 43 in 0fa4d0f

	} else if (policyCategories?.[newCategoryName]) {
	errors.categoryName = 'workspace.categories.existingCategoryError';

This check should be updated to:

} else if (policyCategories?.[newCategoryName] && currentCategoryName !== newCategoryName) { 
     errors.categoryName = 'workspace.categories.existingCategoryError'; 

Now we also shouldn't make a API call if we have made no changes, so update the following:

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

Lines 57 to 64 in 0fa4d0f

	const editTag = useCallback(
	(values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_TAG_FORM>) => {
	Policy.renamePolicyTag(route.params.policyID, {oldName: currentTagName, newName: values.tagName.trim()});
	Keyboard.dismiss();
	Navigation.dismissModal();
	},
	[route.params.policyID, currentTagName],
	);

TO:

 const editTag = useCallback( 
     (values: FormOnyxValues<typeof ONYXKEYS.FORMS.WORKSPACE_TAG_FORM>) => { 
        if(!(policyCategories?.[newCategoryName] && currentCategoryName !== newCategoryName)){
         Policy.renamePolicyTag(route.params.policyID, {oldName: currentTagName, newName: values.tagName.trim()}); 
         }
         Keyboard.dismiss(); 
         Navigation.dismissModal(); 
     }, 
     [route.params.policyID, currentTagName], 
 ); 

Similar would be done on category page.

What alternative solutions did you explore? (Optional)

N/A

[allgandalf]

I think we should update duplicate category check from the edit category page , what about you @mountiny ?

[cristipaval]

This is not a blocker, its a new feature, I can take over.

Thanks @mountiny ! 🙏

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

⚠️ This issue has had its price increased by 4x or more. Please review the issue and ensure the price is correct.

[melvin-bot]

Upwork job price has been updated to $500

[Krishna2323]

Proposal

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

Web - Inconsistent behavior when saving existing name without edit

What is the root cause of that problem?

We don't check if the value is changed or not before validating here:

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

Line 42 in 0fa4d0f

} else if (policyCategories?.[newCategoryName]) {

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

Update the line to use the categoryName passed from EditCategoryPage to check if the new name is the same or not:
else if (policyCategories?.[newCategoryName] && categoryName !== newCategoryName)

Result

[allgandalf]

Updated proposal

Updated my proposal to also not call the api if we make no edits

[allgandalf]

PR ready for review c.c. @shubham1206agra

[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 1.4.57-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [FIX]: Inconsistent behavior when saving existing name without edit and unnecessary API calls #38757

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

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

• @GandalfGwaihir requires payment automatic offer (Contributor)

[shubham1206agra]

@mountiny Please assign me here as C+

[melvin-bot]

📣 @shubham1206agra 🎉 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 📖

[mountiny]

$250 to @GandalfGwaihir and $250 to @shubham1206agra

[melvin-bot]

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[slafortune]

@shubham1206agra please accept this offer - https://www.upwork.com/nx/wm/offer/101650388
@GandalfGwaihir - Paid

[slafortune]

@shubham1206agra please accept this offer - https://www.upwork.com/nx/wm/offer/101650388

[slafortune]

I'll be out of the office until 4/22 and will check on this then.

[shubham1206agra]

@slafortune Accepted the offer.

[allgandalf]

Bump to @slafortune to close this one once @shubham1206agra gets paid :)

[slafortune]

Yep, just back today! Paid and closing ✔️