Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[$500] SplitBill date is incorrect when a date other than today is selected #36285

Closed m-natarajan closed 9 months ago

m-natarajan commented 9 months ago

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.39-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: @tgolen Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707500020195589

Action Performed:

  1. Create a split bill
  2. On the confirmation screen, set the date to a date in the future
  3. After creating the bill, click on the preview to view the details

    Expected Result:

    The date should show as date that is not today

    Actual Result:

    The date shows today's date because it's showing the "created" date and not the date the user specified.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/06fe85ce-3a30-490d-a5a9-fd81e9c22296

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d790dc8609cd540a
  • Upwork Job ID: 1757674654685200384
  • Last Price Increase: 2024-02-14
melvin-bot[bot] commented 9 months ago

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

vishalsanghai commented 9 months ago

Should we allow entries with future date? If not we can disable selecting any future date on UI.

melvin-bot[bot] commented 9 months ago

📣 @vishalsanghai! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
vishalsanghai commented 9 months ago

Contributor details Your Expensify account email: vcoolsanghai@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~0121fccdaccc2abdc1

melvin-bot[bot] commented 9 months ago

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

brunovjk commented 9 months ago

Proposal

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

The SplitBill date displays incorrectly when a date other than today is selected.

What is the root cause of that problem?

1 - While making an API call, the created field is not passed as a param. Consequently, the backend consistently returns today's date. 2 - The addition of the created parameter to SplitBillParams improves the optimistic transaction data. However, the SplitBill response still reflects today's date.

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

To address this issue, two essential changes are required: 1 - Include the created parameter in SplitBillParams and other relevant split params types. 2 - Coordinate with the backend team to implement the necessary updates.

What alternative solutions did you explore? (Optional)

brunovjk commented 9 months ago

Interested in help, despite the absence of the "Help Wanted" tag. Noticed this issue while working on another PR and @DylanDylann helped with the solution. However, I believe we still need an internal team member to check and implement the required backend adjustments.

DylanDylann commented 9 months ago

Could I take this issue as C+ contributor because I have context and discuss here

tgolen commented 9 months ago

Should we allow entries with future date?

I noticed this question and wanted to respond. This should be allowed, yes. Please use these constants for the date.

tgolen commented 9 months ago

Assigned to you @DylanDylann to work on this.

melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

Current assignee @DylanDylann is eligible for the External assigner, not assigning anyone new.

stephanieelliott commented 9 months ago

Seems like the labels didn't apply correctly here. Thanks for noticing @brunovjk -- giving you a heads up if you want to make a proposal 😊

jonxzsh commented 9 months ago

It seems like "created" is passed to the RequestMoney command with a value similar to "2024-02-17", but the same doesn't happen on SplitBill. I don't have my environment setup so just posting to anybody who's looking. Seems like you might have to make API changes aswell to implement the "created" parameter and do something with it on the server.

melvin-bot[bot] commented 9 months ago

📣 @jonsystems! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
brunovjk commented 9 months ago

Very good, thank you @stephanieelliott, I already have a proposal. However, today I will re-investigate and update the proposal if necessary.

brunovjk commented 9 months ago

Proposal

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

SplitBill date displays incorrectly when a date other than today is selected.

What is the root cause of that problem?

Missing created parameters at SplitBillParams.

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

[!NOTE] With these changes, transaction.created is correct, but we must also update BE to use the created parameter when we call SPLIT_BILL and SPLIT_BILL_AND_OPEN_REPORT, as we do in COMPLETE_SPLIT_BILL, which returns the correct date.

POC: Create a splitBillAndOpenReport offline and than go online:

Proposed changes https://github.com/Expensify/App/assets/95647348/24665cde-8444-479f-8421-0b85d6528a3e
Latest main https://github.com/Expensify/App/assets/95647348/42186470-e7d6-489a-9537-a96cc31b0c8a

What alternative solutions did you explore? (Optional)

DylanDylann commented 9 months ago

@brunovjk Your proposal looks good to me

Note that: We need to update splitBillAndOpenReport API and splitBill API from the BE side to fix this issue completely. Don't need to update StartSplitBill API, because this flow is used when splitting bill with receipt and date field is hidden in this case

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 9 months ago

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

brunovjk commented 9 months ago

Don't need to update StartSplitBill API

Exactly! Thanks for the review @DylanDylann :D

francoisl commented 9 months ago

Hi @brunovjk, looks like the backend was just modified to support the created param for the commands SplitBill and SplitBillAndOpenReport a few days ago. Can you try your changes again and see if the backend returns the right date in the response now?

francoisl commented 9 months ago

Ah shoot, actually this might be the same issue as https://github.com/Expensify/App/issues/34526, which just had this fix merged: https://github.com/Expensify/App/pull/35141. Right?

DylanDylann commented 9 months ago

Thank you for pointing out that. Can't reproduce anymore

stephanieelliott commented 9 months ago

I can't repro it anymore either -- gonna close this as resolved.