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.34k stars 2.77k forks source link

[$250] Distance - Distance edit system message only appears after refreshing the page #49000

Open izarutskaya opened 6 days ago

izarutskaya commented 6 days 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: 9.0.32-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: https://expensify.testrail.io/index.php?/tests/view/4945031 Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to workspace chat.
  3. Submit a distance expense.
  4. Go to transaction thread.
  5. Click Distance.
  6. Edit the distance and save it.

Expected Result:

Distance edit system message will appear after the new distance is saved.

Actual Result:

Distance edit system message does not appear after the new distance is saved. It only appears after refreshing the page.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/877c19ed-8042-4d86-bf73-c9592b0a28d4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021834276537749459128
  • Upwork Job ID: 1834276537749459128
  • Last Price Increase: 2024-09-12
Issue OwnerCurrent Issue Owner: @ZhenjaHorbach
melvin-bot[bot] commented 6 days ago

Triggered auto assignment to @miljakljajic (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[bot] commented 5 days ago

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

melvin-bot[bot] commented 5 days ago

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

melvin-bot[bot] commented 5 days ago

📣 @tryevertthhub! 📣 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>
tryevertthhub commented 5 days ago

I got it issue, and I have faced this problem before while I developing notification app. I resolved with Socket. Socket.IO can be an excellent solution to handle real-time updates in this case. Implementing Socket.IO would allow the frontend to receive an event immediately when the distance is edited and saved on the backend, eliminating the need for a manual page refresh to show the updated system message.

ZhenjaHorbach commented 4 days ago

I got it issue, and I have faced this problem before while I developing notification app. I resolved with Socket. Socket.IO can be an excellent solution to handle real-time updates in this case. Implementing Socket.IO would allow the frontend to receive an event immediately when the distance is edited and saved on the backend, eliminating the need for a manual page refresh to show the updated system message.

@tryevertthhub Thanks for your proposal ! But can you provide the proposal using PROPOSAL_TEMPLATE with code examples please?

klajdipaja commented 4 days ago

Proposal

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

When the distance is changed in a distance based report the message that "distance was changed" is not shown in the chat.

What is the root cause of that problem?

The changes of a report are derived from the reportActions_ entry in Onyx which when the distance is change is not updated. The screenshot below is the response of the BE when distance is changed, as you can see we do not have actions for the current report. image

In contrast you can see below another screenshot when the category is updated on the same report on which case the message is shown: image In the screenshots you can also see how the Changed category messages are shown but not the distance ones.

When we reopen the report the Open Report API brings all the reportActions_ that include the ones indicating the distance update. Another screenshot of that below for illustration:

image

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

I don't think we should change anything, this is a BE issue and should be solved there

What alternative solutions did you explore? (Optional)

We could fix this in the front end if that was necessary by calling the Open Report API again when the distance has changed. If that's a solution you would like to asses I can than update my proposal but I do not think that should be the way to go.

ZhenjaHorbach commented 1 day ago

@klajdipaja Thanks for your proposal ! I will review it today or tomorrow !

tryevertthhub commented 1 day ago

Edited by proposal-police: This proposal was edited at 2024-09-16 13:17:33 UTC.

Proposal

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

Distance edit system message does not appear after the new distance is saved. It only appears after refreshing the page.

What is the root cause of that problem?

The cause is asynchrony between the backend and the frontend.

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

I have faced this problem before while I developing notification app. I resolved with Socket. Socket.IO can be an excellent solution to handle real-time updates in this case. Implementing Socket.IO would allow the frontend to receive an event immediately when the distance is edited and saved on the backend, eliminating the need for a manual page refresh to show the updated system message.

What alternative solutions did you explore? (Optional)

You can solve this problem simply by using a socket. An example of this can be found in the notification example in the Upwork.

Using automatic page refresh is also a good idea. This is example code, in Next.js, I used router.fresh()

  const onSubmit: SubmitHandler<FieldValues> = (data) => {
      setIsLoading(true);
      signIn('credentials', {
        ...data,
        redirect: false
      })
      .then((callback) => {
        setIsLoading(false);

        if(callback?.ok) {
            toast.success('Logged in');
            router.refresh();
            loginModal.onClose();
        }

        if(callback?.error) {
            toast.error(callback.error);
        }
      })

    }

As klajdipaja already mentioned, using the Open Report API when the distance changes is one way. However, page refresh is a more flexible way.

ZhenjaHorbach commented 3 hours ago

@klajdipaja Thanks for your proposal ! But I'm not sure that this is a problem with BE I think that the problem is related to optimistic data

For example you can try to turn off the Internet And you will notice that after changing the receipt details Messages are added(only except changing Distance)

@tryevertthhub And thank you for your updates ! But I'm not sure we need to implement new libraries We just need to fix one bug using available code

tryevertthhub commented 3 minutes ago

@ZhenjaHorbach we don't need add new library, just add refresh function and I hope discuss more detail. this is my email: tryevertth@gmail.com this is my discord id: crazyolaf