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

Expense - Track expense button displayed instead of Submit when creating expense in self DM #50826

Closed
2 of 6 tasks
IuliiaHerets opened this issue Oct 15, 2024 · 10 comments
Closed
2 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering

Comments

@IuliiaHerets
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.49.0
Reproducible in staging?: Y
Reproducible in production?: N/A - new feature, doesn't exist in prod
Issue was found when executing this PR: #50239
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to https://staging.new.expensify/
    and log in
  2. Enable combinedTrackSubmit beta
  3. Navigate to Self DM
  4. Click on + > Create expense > Manual
  5. Enter the amount and proceed to the next screen

Expected Result:

The "Submit" button is displayed on the confirmation screen

Actual Result:

The "Track expense" button displayed on the confirmation screen

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6635437_1729008557873.Recording__865.mp4

View all open jobs on GitHub

@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

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

Copy link

melvin-bot bot commented Oct 15, 2024

💬 A slack conversation has been started in #expensify-open-source

Copy link

melvin-bot bot commented Oct 15, 2024

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.

@melvin-bot melvin-bot bot added the Daily KSv2 label Oct 15, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Oct 15, 2024
Copy link
Contributor

👋 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.

@cead22
Copy link
Contributor

cead22 commented Oct 15, 2024

I get the same on staging and on production, so I don't think we should block on this
image

@cead22 cead22 added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Oct 15, 2024
@cead22
Copy link
Contributor

cead22 commented Oct 15, 2024

@grgia do you know what the behavior is supposed to be here? cc @trjExpensify

@nkdengineer
Copy link
Contributor

nkdengineer commented Oct 15, 2024

Edited by proposal-police: This proposal was edited at 2024-10-15 18:57:35 UTC.

Proposal

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

The "Track expense" button displayed on the confirmation screen

What is the root cause of that problem?

In here, we are display button text is translate('iou.trackExpense') for all cases

text = translate('iou.trackExpense');

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

We should display translate('iou.submitExpense') when canUseCombinedTrackSubmit = true

 const {canUseCombinedTrackSubmit} = usePermissions();

else if (isTypeTrackExpense) {
     text = canUseCombinedTrackSubmit ? translate('iou.submitExpense') : translate('iou.trackExpense');
}

Or we can use translate('iou.createExpense') like we did with other places

What alternative solutions did you explore? (Optional)

@mananjadhav
Copy link
Collaborator

mananjadhav commented Oct 15, 2024

Edited by proposal-police: This proposal was edited at 2024-10-15 19:03:42 UTC.

yeah doesn't look like a deploy blocker. We can wait for the confirmation on the behavior but I checked the code.

Proposal

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

In the Self DM, when we choose "Create Expense", we always end up with the "Track expense" flow. Check the URL:
/create/track/start/1/{reportId}/manual

What is the root cause of that problem?

In AttachmentPickerWithMenuItems we're updating the text text: canUseCombinedTrackSubmit ? translate('iou.createExpense') : translate('iou.trackExpense'), based on the canUseCombinedTrackSubmit. The button is still track expense and hence the track flow is triggered.

[CONST.IOU.TYPE.TRACK]: {
icon: canUseCombinedTrackSubmit ? getIconForAction(CONST.IOU.TYPE.CREATE) : getIconForAction(CONST.IOU.TYPE.TRACK),
text: canUseCombinedTrackSubmit ? translate('iou.createExpense') : translate('iou.trackExpense'),
onSelected: () => selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.TRACK, report?.reportID ?? '-1'), true),
},

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

In the ReportUtils.ts we should add the check for the permissions when we check for isSelfDM.


if (isSelfDM(report)) {
    if (canUseCombinedTrackSubmit) {
        options = [CONST.IOU.TYPE.SUBMIT];
    } else {
        options = [CONST.IOU.TYPE.TRACK];
    } 
}

App/src/libs/ReportUtils.ts

Lines 6784 to 6786 in e8a8c53

if (isSelfDM(report)) {
options = [CONST.IOU.TYPE.TRACK];
}

What alternative solutions did you explore? (Optional)

@paultsimura
Copy link
Contributor

paultsimura commented Oct 15, 2024

It's always been "Track" as you cannot submit an expense to yourself, only track it. IIRC, the self-dm was where we first implemented the "Track expense" functionality: #36362

Even the chat header says "Use + button to track an expense":
image

Therefore, this button title looks like expected behavior to me.

@cead22
Copy link
Contributor

cead22 commented Oct 15, 2024

Thanks @paultsimura. Closing for now

@cead22 cead22 closed this as completed Oct 15, 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 Engineering
Projects
None yet
Development

No branches or pull requests

6 participants