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] Default category set for distance expense in a policy does not auto populate when creating expense #34448

Closed
6 tasks done
m-natarajan opened this issue Jan 12, 2024 · 15 comments
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

@m-natarajan
Copy link

m-natarajan commented Jan 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.24-6
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: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1705010484215319

Action Performed:

  1. Set a default currency for distance expense in olddot
  2. Invite member B to the account in OD
  3. Open the workspace chat in ND as. a member B
  4. Create a distance expense
  5. Observe the confirmation page

Expected Result:

Default currency should be auto populated

Actual Result:

Not auto populated. Only after creating the expense category is set

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

Recording.2658.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0164f6cd9ebd57caa6
  • Upwork Job ID: 1745836097956413440
  • Last Price Increase: 2024-01-12
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 12, 2024
Copy link

melvin-bot bot commented Jan 12, 2024

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

@m-natarajan m-natarajan added the External Added to denote the issue can be worked on by a contributor label Jan 12, 2024
@melvin-bot melvin-bot bot changed the title Default category set for distance expense in a policy does not auto populate when creating expense [$500] Default category set for distance expense in a policy does not auto populate when creating expense Jan 12, 2024
Copy link

melvin-bot bot commented Jan 12, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0164f6cd9ebd57caa6

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

melvin-bot bot commented Jan 12, 2024

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

@FitseTLT
Copy link
Contributor

FitseTLT commented Jan 12, 2024

Proposal

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

Default category set for distance expense in a policy does not auto populate when creating expense

What is the root cause of that problem?

We are only displaying category if it exists in the transaction here

title={iouCategory}
description={translate('common.category')}

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

We should display the default category checking the workspace participant has default category set and distance request.

 title={iouCategory ||( isDistanceRequest && policy?.defaultCategory)||''} 

NOTE that there is also BE job required because now we are not receiving the defaultCategory prop from the server in the policy

What alternative solutions did you explore? (Optional)

@paultsimura
Copy link
Contributor

paultsimura commented Jan 12, 2024

Proposal

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

When a default Distance category is set for the workspace, it should be visible on the Confirmation step as well.

What is the root cause of that problem?

We have not yet implemented the defaultCategory handling for the Distance requests. We only show the transaction.category on the Category field, and for the distance requests it's not set.

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

First, we should fetch the distance-specific default category. It is located under the policy.customUnits[].defaultCategory.

We can add it as an util function to the DistanceRequestUtils similar to the getDefaultMileageRate:

function getDefaultCategory(policy: OnyxEntry<Policy>): string | null {
    if (!policy?.customUnits) {
        return null;
    }

    const distanceUnit = Object.values(policy.customUnits).find((unit) => unit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE);
    return distanceUnit?.defaultCategory;
}

Then, we should handle this default category similar to how we handle the tax rates: if it's not present on the transaction, use the default one on the Confirmation Step (I'll attach references to similar code blocks of TaxRates logic):

const shouldShowCategories = isPolicyExpenseChat && (iouCategory || OptionsListUtils.hasEnabledOptions(_.values(policyCategories)));
const categoryTitle = iouCategory || isDistanceRequest && DistanceRequestUtils.getDefaultCategory(policy);

const defaultTaxKey = policyTaxRates.defaultExternalID;
const defaultTaxName = (defaultTaxKey && `${policyTaxRates.taxes[defaultTaxKey].name} (${policyTaxRates.taxes[defaultTaxKey].value}) • ${translate('common.default')}`) || '';
const taxRateTitle = (transaction.taxRate && transaction.taxRate.text) || defaultTaxName;

And on the IOURequestStepCategory, mark the default category as selected:

const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction);
const defaultCategory = isDistanceRequest ? DistanceRequestUtils.getDefaultCategory(policy) : '';
const selectedCategory = transaction.category || defaultCategory;

const defaultTaxKey = policyTaxRates.defaultExternalID;
const defaultTaxName = (defaultTaxKey && `${policyTaxRates.taxes[defaultTaxKey].name} (${policyTaxRates.taxes[defaultTaxKey].value}) • ${translate('common.default')}`) || '';
const selectedTaxRate = (transaction.taxRate && transaction.taxRate.text) || defaultTaxName;

What alternative solutions did you explore? (Optional)

@Pujan92
Copy link
Contributor

Pujan92 commented Jan 12, 2024

Proposal

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

Default category isn't set for the distance transaction whereas it is set in the policy

What is the root cause of that problem?

Currenty, we are not fetching and setting the default category from the policy for the transaction in IOURequestStepConfirmation component.

function IOURequestStepConfirmation({

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

Consider fetching the customUnits from the policy for the distance unit(applicable when requestType is distance), if it is available take default category and set it to the transaction. We need to set it to the transaction so when user opens category picker the default option gets selected.

const distanceCustomUnit = _.find(lodashGet(policy, 'customUnits', {}), (unit) => unit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE);
if(!transaction.category && distanceCustomUnit) {
    IOU.setMoneyRequestCategory_temporaryForRefactor(transactionID, distanceCustomUnit.defaultCategory);
}
Result
Screen.Recording.2024-01-12.at.22.05.17.mp4

@dukenv0307
Copy link
Contributor

dukenv0307 commented Jan 12, 2024

Proposal

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

Not auto populated. Only after creating the expense category is set

What is the root cause of that problem?

We are not getting the default policy from the custom unit of the workspace to display if category doesn't exist from transaction

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

  1. Create a function to get the default category in DistanceRequestUtils
function getDefaultCategory(policy: Policy) {

  const distanceUnit = Object.values(policy.customUnits).find((unit) => unit.name === CONST.CUSTOM_UNITS.NAME_DISTANCE);
  return distanceUnit?.defaultCa.tegory;

}
  1. In IOURequestStepConfirmation, create a useEffect to update the category if policy.customUnits exists and category is undefined
useEffect(() => {
        if (!policy.customUnits) {
            return;
        }

        IOU.setMoneyRequestCategory_temporaryForRefactor(transactionID, DistanceRequestUtils.getDefaultCategory(policy))
    }, [policy.customUnits])

iouCategory={transaction.category}

*Note: We need to store it into transaction here because customUnits doesn't exist until the first time we call openDraftWorkspaceRequest API in IOURequestStepConfirmation and store it to make the optimistic data of category is correct when we create the transaction.

OPTIONAL: After we select a workspace or create a request from a policy expense chat we can update default category into transaction as well if exist.

What alternative solutions did you explore? (Optional)

@isabelastisser
Copy link
Contributor

Hey @c3024, can you please review the proposals above? Thanks!

@melvin-bot melvin-bot bot added the Overdue label Jan 14, 2024
@dylanexpensify dylanexpensify moved this to Release 3: Migration for All in [#whatsnext] Wave 05 - Deprecate Free Jan 15, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

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

@c3024
Copy link
Contributor

c3024 commented Jan 16, 2024

Will update today

@melvin-bot melvin-bot bot removed the Overdue label Jan 16, 2024
@isabelastisser
Copy link
Contributor

Hey @c3024, did you have the chance to review the proposals? Thanks!

@c3024
Copy link
Contributor

c3024 commented Jan 18, 2024

Thank you for all your proposals.

I think setting the category in the transaction is the best solution. It updates the category picker page as well correctly.

@Pujan92 's proposal here looks good to me.

🎀 👀 🎀 C+ Reviewed

Copy link

melvin-bot bot commented Jan 18, 2024

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

@dukenv0307
Copy link
Contributor

Just found that this will be fixed here #34012

@isabelastisser
Copy link
Contributor

Thanks for the heads up, @dukenv0307 ! Closing this issue then.

@github-project-automation github-project-automation bot moved this from Release 3: Migration for All to Done in [#whatsnext] Wave 05 - Deprecate Free Jan 18, 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
No open projects
Development

No branches or pull requests

8 participants