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

[$500] IOU - Missing Tag violation on IOU details page when tag was disabled in OD #36380

Closed
3 of 6 tasks
kbecciv opened this issue Feb 12, 2024 · 17 comments
Closed
3 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented Feb 12, 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.40.0
Reproducible in staging?: y
Reproducible in production?: y
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:

Issue found when executing PR #35048

Action Performed:

Precondition: set up Admin and Employee accounts and add only one tag to the workspace in OD

  1. Open app
  2. Sign in with Employee account
  3. Click on FAB
  4. Select Request money > manual
  5. Write any amount and click on Continue button
  6. Select the Workspace room
  7. Click on "Show more"
  8. Select the custom tag
  9. Click on merchant and write any name
  10. Click on Request button
  11. Navigate to OD and sign in as Admin account
  12. Disable the custom tag selected in step 8
  13. Navigate to app > IOS details page
  14. Refresh the page

Expected Result:

The tag violation should be displayed on the IOU details page

Actual Result:

The tag violation is missing on the IOU details page

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

Bug6377302_1707773969777.Screen_Recording_2024-02-12_at_19.35.23.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01da546d53cc21957d
  • Upwork Job ID: 1757158561545486336
  • Last Price Increase: 2024-02-12
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 12, 2024
@melvin-bot melvin-bot bot changed the title IOU - Missing Tag violation on IOU details page when tag was disabled in OD [$500] IOU - Missing Tag violation on IOU details page when tag was disabled in OD Feb 12, 2024
Copy link

melvin-bot bot commented Feb 12, 2024

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

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

melvin-bot bot commented Feb 12, 2024

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

Copy link

melvin-bot bot commented Feb 12, 2024

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

@brandonhenry
Copy link
Contributor

Proposal

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

The problem is that when a custom tag is selected for an IOU and then later disabled by an admin in Old Dot (OD), the expected tag violation does not appear on the IOU details page within the app. This leads to improper functionality due to a disabled tag being able to be used for an IOU.

What is the root cause of that problem?

The root cause seems to be a failure in the app's logic to check for tag status changes in OD after an IOU is created. There appears to be either a missing real-time update flow from OD to the app or a lack of validation check for tag status when displaying the IOU details.

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

  • Implement a mechanism to sync policy updates from OD in real-time or via periodic checks to ensure the app has the latest policy settings.
  • Introduce a validation check in the IOU details page logic to verify the status of the tag and display a violation if the tag has been disabled after the IOU was created.
  • Update the state management within the app to reflect any changes in tag status and trigger UI updates accordingly.
  • Perform thorough testing on all platforms to validate that tag violations appear as expected when tags are disabled post-IOU creation.

What alternative solutions did you explore? (Optional)

An alternative could be to prevent the disabling of tags in OD if they have been used in any outstanding IOUs, thus avoiding this issue altogether.

Copy link

melvin-bot bot commented Feb 16, 2024

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

@allroundexperts
Copy link
Contributor

Thanks for your proposal @brandonhenry. Your proposal seems too generic and does not provide a clear cut solution.

@melvin-bot melvin-bot bot removed the Overdue label Feb 17, 2024
@allroundexperts
Copy link
Contributor

Still looking for more proposals.

@brandonhenry
Copy link
Contributor

brandonhenry commented Feb 17, 2024

@allroundexperts i did some more digging on this one and I wonder if this issue is well defined? The video shows interaction with the OD workspace and the ND workspace. Are these supposed to be connected? I am currently having issues with there seemingly being two types of workspaces specific to each?

Maybe this one is a bit beyond me for my first contributor assist but I am curious why I am having the user experience I am.

Reproduceable steps:

  1. Login to admin account
  2. Go to OD
  3. Settings -> New Workspace
  4. Go to ND
  5. Notice you cannot see the workspace that was just made in OD

If you create the workspace in ND first, you cannot see that workspace inside of OD. In addition, I do not see any way to create a non-free workspace inside of ND, so i am unable to work with tags inside of the ND..? Not sure if this is the way its suppose to be

@brandonhenry
Copy link
Contributor

brandonhenry commented Feb 17, 2024

Going to still do some digging but was finding it hard to find the particularly screen shown in the original post without being able to access a paid workspace inside of ND..

@brandonhenry
Copy link
Contributor

brandonhenry commented Feb 17, 2024

@allroundexperts managed to find the hopeful culprit.

Updated Proposal

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

The problem is that when a custom tag is selected for an IOU and then later disabled by an admin in Old Dot (OD), the expected tag violation does not appear on the IOU details page within the app. This leads to improper functionality due to a disabled tag being able to be used for an IOU.

What is the root cause of that problem?

The root cause seems to be a failure in the app's logic to check for tag status changes in OD after an IOU is created. There appears to be either a missing real-time update flow from OD to the app or a lack of validation check for tag status when displaying the IOU details.

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

// Inside the MoneyRequestView component

In order for the error to properly show when a tag is disabled / not there, we need to update the inline error property for the MenuItem to check if the tag is available and enabled like so:


const isTagDisabled = policyTags?.[transactionTag]?.isEnabled === false;

// When rendering the tag field
{shouldShowTag && (
    <OfflineWithFeedback pendingAction={getPendingFieldAction('tag')}>
        <MenuItemWithTopDescription
            description={policyTag?.name ?? translate('common.tag')}
            title={PolicyUtils.getCleanedTagName(transactionTag ?? '')}
            interactive={canEdit}
            shouldShowRightIcon={canEdit}
            titleStyle={styles.flex1}
            onPress={() =>
                Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_TAG.getRoute(CONST.IOU.ACTION.EDIT, CONST.IOU.TYPE.REQUEST, transaction?.transactionID ?? '', report.reportID))
            }
            brickRoadIndicator={isTagDisabled ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
            error={isTagDisabled ? translate('common.error.tagDisabled') : ''}
        />
    </OfflineWithFeedback>
)}

@allroundexperts
Copy link
Contributor

@brandonhenry Thanks for the updated proposal. Your proposal is making sense now. However, I am unsure what exactly is the expected behaviour of this issue. Pulling in an internal engineer for further discussion 🎀 👀 🎀

Copy link

melvin-bot bot commented Feb 19, 2024

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

@allroundexperts
Copy link
Contributor

@neil-marcellini Shouldn't the backend be attaching a violation to the transaction object in the case where a used tag gets disabled in admin? If so, then this should be handled on the backend.

@cead22
Copy link
Contributor

cead22 commented Feb 19, 2024

I think we should put a hold on this issue, since this is a known issue that we need to holistic solution for, because updating tags is only one case where the violations displayed in the UI might be outdated.

The tag change on old dot should push the updated tag lists to the relevant new dot clients, but that alone doesn't trigger the re-render of the money request view (or money request preview, or report preview) to display violations

@neil-marcellini
Copy link
Contributor

Ok sounds good @cead22. What issue should we hold this on?

@cead22
Copy link
Contributor

cead22 commented Feb 20, 2024

We don't have one yet. Should we make this one internal, and I'll take it over, or do you think it's better that I create a new one?

@cead22
Copy link
Contributor

cead22 commented Feb 20, 2024

Actually, let's close this in favor of #34541 since it's a dupe

@cead22 cead22 closed this as completed Feb 20, 2024
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. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

6 participants