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.

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

[$125] Web - Inconsistent behavior of tag disappearing and reappearing when requesting money #38744

Closed
1 of 6 tasks
kbecciv opened this issue Mar 21, 2024 · 69 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Mar 21, 2024

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?: y
Issue reported by: Applause - Internal team

Action Performed:

Preconditions:
In OldDot under admin, create a Collect group policy, enable tags and add the tag "Child: Tom", and it should be the only tag in the policy. Add the employee to the policy. Setup OldDot Collect Policy:
https://sites.google.com/applausemail.com/applause-expensifyproject/wiki-guides/newdot-categories?authuser=0

  1. Open the WS chat report
  2. Click on the plus icon inside the compose box > Request money > Enter an amount > Click on the tag
  3. Select the tag "Child: Tom" (Note that it is already selected by default)
  4. Observe the behavior of the "Tag" field after you selected the tag and went back
  5. Click on FAB > Request money > Enter an amount > Select the collect policy created in OD
  6. Click on the Tag field (note that the "Child: Tom" tag is already selected)
  7. Click on the "Child: Tom" tag again
  8. Observe the behavior of the tag field and compare it with the behavior seen on step 4

Expected Result:

The behavior seen on step 4 should be the same for step 8 as well

Actual Result:

The tag field first becomes empty and there is a noticeable delay before the selected tag "Child: Tom" shows up again. This behavior is different from the one seen on step 4, where there is no noticeable delay.

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

Bug6421398_1711004073820.bandicam_2024-03-21_09-47-07-173.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010aab18242acca3aa
  • Upwork Job ID: 1770985774762127360
  • Last Price Increase: 2024-04-08
  • Automatic offers:
    • Krishna2323 | Contributor | 0
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 21, 2024
Copy link

melvin-bot bot commented Mar 21, 2024

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

@kbecciv kbecciv changed the title Tag - Inconsistent behavior of tag disappearing and reappearing Web - Tag - Inconsistent behavior of tag disappearing and reappearing Mar 21, 2024
@kbecciv
Copy link
Author

kbecciv commented Mar 21, 2024

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

@kbecciv
Copy link
Author

kbecciv commented Mar 21, 2024

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

@Krishna2323
Copy link
Contributor

Krishna2323 commented Mar 21, 2024

Proposal

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

Web - Tag - Inconsistent behavior of tag disappearing and reappearing

What is the root cause of that problem?

The useEffect below is autoselecting the tag even when user deselects it.

// Auto select the tag if there is only one enabled tag and it is required
useEffect(() => {
let updatedTagsString = TransactionUtils.getTag(transaction);
policyTagLists.forEach((tagList, index) => {
const enabledTags = _.filter(tagList.tags, (tag) => tag.enabled);
const isTagListRequired = isUndefined(tagList.required) ? false : tagList.required && canUseViolations;
if (!isTagListRequired || enabledTags.length !== 1 || TransactionUtils.getTag(transaction, index)) {
return;
}
updatedTagsString = IOUUtils.insertTagIntoTransactionTagsString(updatedTagsString, enabledTags[0] ? enabledTags[0].name : '', index);
});
if (updatedTagsString !== TransactionUtils.getTag(transaction) && updatedTagsString) {
IOU.setMoneyRequestTag(transaction.transactionID, updatedTagsString);
}
}, [policyTagLists, transaction, policyTags, isTagRequired, canUseViolations]);

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

The useEffect should run only once. For that we can create a ref to track. We also need to do the same with other fields like category. We will create a ref isAutoSelectedTag/isAutoSelectedCategory and set it to true when we automatically select a tag and in useEffect we will check if sAutoSelectedTag/isAutoSelectedCategory.current is true or not and if true we will return without autoselecting the tag.

Result

auto_selected_tag.mp4

Alternatively

We can just remove the dependencies array of the useEffect. This will make sure that we autoselect the option only once (when the component mounts).

@Krishna2323
Copy link
Contributor

Proposal Updated

1 similar comment
@Krishna2323
Copy link
Contributor

Proposal Updated

@nkdengineer
Copy link
Contributor

nkdengineer commented Mar 21, 2024

Proposal

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

The tag field first becomes empty and there is a noticeable delay before the selected tag "Child: Tom" shows up again. This behavior is different from the one seen on step 4, where there is no noticeable delay.

What is the root cause of that problem?

We have a useEffect here that auto select the tag if the tag required and there's only one tag to select.

// Auto select the tag if there is only one enabled tag and it is required
useEffect(() => {
let updatedTagsString = TransactionUtils.getTag(transaction);
policyTagLists.forEach((tagList, index) => {
const enabledTags = _.filter(tagList.tags, (tag) => tag.enabled);
const isTagListRequired = isUndefined(tagList.required) ? false : tagList.required && canUseViolations;
if (!isTagListRequired || enabledTags.length !== 1 || TransactionUtils.getTag(transaction, index)) {
return;
}

But In IOURequestStepTag we have the logic to de-select the tag if we select the selected tag. So after going back to confirm page the tag will disappear and then reappear immediately

This bug is the same for category

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

Since we auto select the tag if it's required and there's only one tag to select, we should disable user go to IOURequestStepTag page in this case. That makes more seen because we can't edit this field, it's the same behavior the amount field in distance request flow. To do that we can create variable like isAutoSelectTag that will be true if the tag if it's required and there's only one tag to select

const [updatedTagsString, isAutoSelectTags] = useMemo(() => {
        let updatedTagsString = TransactionUtils.getTag(transaction);
        let isAutoSelectTags = {};
        policyTagLists.forEach((tagList, index) => {
            const enabledTags = _.filter(tagList.tags, (tag) => tag.enabled);
            const isTagListRequired = isUndefined(tagList.required) ? false : tagList.required && canUseViolations;
            if (!isTagListRequired || enabledTags.length !== 1 || TransactionUtils.getTag(transaction, index)) {
                isAutoSelectTags[tagList.name] = isTagListRequired && enabledTags.length === 1;
                return;
            }
            
            isAutoSelectTags[tagList.name] = true;
            updatedTagsString = IOUUtils.insertTagIntoTransactionTagsString(updatedTagsString, enabledTags[0] ? enabledTags[0].name : '', index);
        });
        return [updatedTagsString, isAutoSelectTags];
    }, [policyTagLists, transaction, policyTags, isTagRequired, canUseViolations]);

useEffect(() => {
    if (!isTagRequired) {
        return ;
    }
    if (updatedTagsString !== TransactionUtils.getTag(transaction) && updatedTagsString) {
        IOU.setMoneyRequestTag(transaction.transactionID, updatedTagsString);
    }
}, [isTagRequired, updatedTagsString]);

Update the condition to show the right icon here to !isReadOnly && !isAutoSelectTags

And update the onPress to do nothing if we auto select the tag

onPress={() => {
    if (isAutoSelectTags[name]) {
        return;
    }
    Navigation.navigate(
        ROUTES.MONEY_REQUEST_STEP_TAG.getRoute(CONST.IOU.ACTION.CREATE, iouType, index, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()),
    )
}}

Do the same way for category

What alternative solutions did you explore? (Optional)

NA

@luacmartins luacmartins changed the title Web - Tag - Inconsistent behavior of tag disappearing and reappearing [Simplified Collect][Tags] - Inconsistent behavior of tag disappearing and reappearing Mar 22, 2024
@luacmartins luacmartins added the External Added to denote the issue can be worked on by a contributor label Mar 22, 2024
@melvin-bot melvin-bot bot changed the title [Simplified Collect][Tags] - Inconsistent behavior of tag disappearing and reappearing [$500] [Simplified Collect][Tags] - Inconsistent behavior of tag disappearing and reappearing Mar 22, 2024
Copy link

melvin-bot bot commented Mar 22, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 22, 2024
Copy link

melvin-bot bot commented Mar 22, 2024

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

@joekaufmanexpensify
Copy link
Contributor

@luacmartins could you reproduce this? I'm not able to reproduce the behavior shown in OP at all. Maybe the performance is a bit slower on the second attempt to request, but nothing like the above video. If this is just impacting one tag, feels like pretty minor edge case, no? Not sure like it feels worth prioritizing.

2024-03-22_12-17-26.mp4

@Krishna2323
Copy link
Contributor

@joekaufmanexpensify, it's reproducible on slower machines I guess but I have reproduced it easily, also the important issue here is that the user is unable to deselect the tag which is possible when we have more than one tag.

@luacmartins
Copy link
Contributor

I haven't tried to reproduce, but it seems like it might happen on slower connections so maybe we can try to throttle it in the browser inspector. Also, I looked at the issue and this doesn't seem related to simplified collect, so I'm removing that from the project.

@luacmartins luacmartins changed the title [$500] [Simplified Collect][Tags] - Inconsistent behavior of tag disappearing and reappearing Inconsistent behavior of tag disappearing and reappearing when requesting money Mar 22, 2024
@Krishna2323
Copy link
Contributor

@luacmartins @joekaufmanexpensify, the problem lies in the code below, which automatically selects a tag when there is only one available and it is required. We need to disable the deselection of the tag on the tag selection page when there's only one tag available and it's required. The reason it disappears and reappears is because, on the tag selection page, we allow tag deselection. As a result, it gets deselected, and when we return to the confirmation list, the code below once again selects the tag. We might also want to show an error message when user deselects it.

// Auto select the tag if there is only one enabled tag and it is required
useEffect(() => {
let updatedTagsString = TransactionUtils.getTag(transaction);
policyTagLists.forEach((tagList, index) => {
const enabledTags = _.filter(tagList.tags, (tag) => tag.enabled);
const isTagListRequired = isUndefined(tagList.required) ? false : tagList.required && canUseViolations;
if (!isTagListRequired || enabledTags.length !== 1 || TransactionUtils.getTag(transaction, index)) {
return;
}
updatedTagsString = IOUUtils.insertTagIntoTransactionTagsString(updatedTagsString, enabledTags[0] ? enabledTags[0].name : '', index);
});
if (updatedTagsString !== TransactionUtils.getTag(transaction) && updatedTagsString) {
IOU.setMoneyRequestTag(transaction.transactionID, updatedTagsString);
}
}, [policyTagLists, transaction, policyTags, isTagRequired, canUseViolations]);

@luacmartins
Copy link
Contributor

So it seems like this is the expected behavior? Tags are required and we have only one tag, so it should be selected and user's can't deselect it. Maybe in that case we should disable the editing of tags altogether.

@Krishna2323
Copy link
Contributor

@luacmartins, yep, we need to make changes so that users can't deselect it on tags selection page.

@luacmartins
Copy link
Contributor

luacmartins commented Mar 22, 2024

I meant if tags are required and users only have a single tag, we auto select it and the MenuItem should be disabled, so users shouldn't even be able to see the tags selection page. @trjExpensify can you confirm?

@Krishna2323
Copy link
Contributor

@luacmartins, I guess disabling the option on categories options page makes more sense so users can get know they only have one category and that is the reason why they can't deselect it.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@sobitneupane
Copy link
Contributor

sobitneupane commented Apr 12, 2024

Alternative Proposal from @Krishna2323 looks good to me.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Apr 12, 2024

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

@amyevans
Copy link
Contributor

Great, let's just proceed with removing the dependencies array. Thanks for the convo here!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 12, 2024
Copy link

melvin-bot bot commented Apr 12, 2024

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

@Krishna2323
Copy link
Contributor

@sobitneupane, PR ready for review.

@joekaufmanexpensify
Copy link
Contributor

Great. TY!

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Third week)

@Krishna2323
Copy link
Contributor

@joekaufmanexpensify, PR was deployed to production on 22nd April, this is ready for payments process.

@joekaufmanexpensify
Copy link
Contributor

All set to issue payment! We need to issue the following payments:

@joekaufmanexpensify
Copy link
Contributor

@Krishna2323 $125 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

@sobitneupane please request $125 via NewDot, and confirm here once complete.

@joekaufmanexpensify
Copy link
Contributor

Bumped @sobitneupane in slack here.

@sobitneupane
Copy link
Contributor

Requested payment in newDot

@joekaufmanexpensify
Copy link
Contributor

Great. TY! All set. Payment summary is here.

@JmillsExpensify
Copy link

$125 approved for @sobitneupane

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

10 participants