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.55k stars 2.89k forks source link

[$500] Task – Task details page becomes unread in LHN when edit task Offline #35575

Closed lanitochka17 closed 8 months ago

lanitochka17 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.35-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/4262665 Email or phone of affected tester (no customers): applausetester+jp_e_category@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Log in
  3. Create a task in a group chat and open Task details
  4. Go Offline and edit task name, description, assignee
  5. Find Task details in the LHN and go back Online (don’t click on the Task details page)

Expected Result:

Task details page is read in LHN

Actual Result:

Task details page becomes unread in LHN

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/78819774/c420048e-c8b4-4151-8500-5259cd960625

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019a8f6449e90ae629
  • Upwork Job ID: 1753102600156340224
  • Last Price Increase: 2024-02-08
  • Automatic offers:
    • cubuspl42 | Reviewer | 0
melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

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

lanitochka17 commented 9 months ago

We think that this bug might be related to #vip-vsp CC @quinthar

melvin-bot[bot] commented 9 months ago

@cubuspl42, @greg-schroeder Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

greg-schroeder commented 9 months ago

Thoughts on this issue's potential connection to #vip-vsb @quinthar? Waiting for proposals either way

melvin-bot[bot] commented 9 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

greg-schroeder commented 9 months ago

Still awaiting proposals

melvin-bot[bot] commented 9 months ago

@cubuspl42, @greg-schroeder Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

greg-schroeder commented 9 months ago

Same as above

bi-kash commented 9 months ago

Proposal

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

After editing a task offline, returning online doesn't mark the Task details page as read in the LHN.

What is the root cause of that problem?

The issue stems from the fact that the lastReadTime (the time when the user reads the last message) for a task doesn't get updated when offline. The condition for determining unread status is the same for both task editing and general message chats.

function isUnread(report: OnyxEntry<Report>): boolean {
    if (!report) {
        return false;
    }

    // lastVisibleActionCreated and lastReadTime are both datetime strings and can be compared directly
    const lastVisibleActionCreated = report.lastVisibleActionCreated ?? '';
    const lastReadTime = report.lastReadTime ?? '';
    const lastMentionedTime = report.lastMentionedTime ?? '';

    // If the user was mentioned and the comment got deleted the lastMentionedTime will be more recent than the lastVisibleActionCreated
    return lastReadTime < lastVisibleActionCreated || lastReadTime < lastMentionedTime;
}

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

To address the issue, we can add a condition in the isUnread function: if lastActorAccountID for the task report is the same as currentUserAccountID, then return false. Because lastReadTime and lastMentionedTime have no meaning if the account for the last actor user and the current user is the same. I have checked it. It works

function isUnread(report: OnyxEntry<Report>): boolean {
    if (!report) {
        return false;
    }

    if (report.lastActorAccountID === currentUserAccountID) {
        return false;
    }
    // lastVisibleActionCreated and lastReadTime are both datetime strings and can be compared directly
    const lastVisibleActionCreated = report.lastVisibleActionCreated ?? '';
    const lastReadTime = report.lastReadTime ?? '';
    const lastMentionedTime = report.lastMentionedTime ?? '';

    // If the user was mentioned and the comment got deleted the lastMentionedTime will be more recent than the lastVisibleActionCreated
    return lastReadTime < lastVisibleActionCreated || lastReadTime < lastMentionedTime;
}

What alternative solutions did you explore? (Optional)

We can update lastReadTime even when offline. However, I think above solution is more robust..

melvin-bot[bot] commented 9 months ago

📣 @bi-kash! 📣 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>
bi-kash commented 9 months ago

Contributor details Your Expensify account email: bkash.timsina@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01113b8ddddd2dbada

melvin-bot[bot] commented 9 months ago

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

cubuspl42 commented 9 months ago

@bi-kash Interesting! We'd need to do some careful testing to ensure it works everywhere, but the idea that something can't be "unread" if I'm the last one to touch it sounds reasonable.

As this is the only proposal we have, I believe that it makes sense to give this a try.

C+ reviewed 🎀 👀 🎀

melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

@tylerkaraszewski @cubuspl42 @greg-schroeder this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

greg-schroeder commented 9 months ago

Bump @tylerkaraszewski to confirm the contributor assignment here, thanks!

melvin-bot[bot] commented 9 months ago

📣 @cubuspl42 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 9 months ago

📣 @bi-kash You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

bi-kash commented 9 months ago

The pull request (PR) is expected to be ready for review by this upcoming Monday.

bi-kash commented 8 months ago

PR is ready for review: https://github.com/Expensify/App/pull/36762

cubuspl42 commented 8 months ago

During the work on the PR, we found out that the issue is no longer reproducible, possibly because of the backend changes.

@lanitochka17 @greg-schroeder Would you confirm this?

mvtglobally commented 8 months ago

Issue not reproducible during KI retests. (First week)

greg-schroeder commented 8 months ago

Seems to be confirmed @cubuspl42

cubuspl42 commented 8 months ago

Alright, we should close this issue and the PR then.

Per the process, in such (rare) cases, "payment is due for C+ and the contributor".

bi-kash commented 8 months ago

Could we please finish the remaining steps for this issue and close it? I've noticed that the contribution guidelines specifically mention that new contributors should focus on one problem at a time. Will this open issue that is assigned to me prevent me from submitting proposals for other jobs?

cubuspl42 commented 8 months ago

@bi-kash In practice, we're not too strict about that rule. If your capacity is not affected (and here it's not), feel free to apply to another issue. It's good to remember about the common sense.

cubuspl42 commented 8 months ago

During the work on the PR, we found out that the issue is no longer reproducible, possibly because of the backend changes.

@lanitochka17 @greg-schroeder Would you confirm this?

Bump

greg-schroeder commented 8 months ago

Confirmed. Let me recap what happened here to consider potential payments.

greg-schroeder commented 8 months ago

@bi-kash you're free to take on something else, this issue is considered resolved for your purposes

greg-schroeder commented 8 months ago

I paid @cubuspl42 and sent an offer to @bi-kash - once you accept, I'll send you a payment. Thanks for your effort even though this didn't go forward. You can close the PR if we're finished there.

melvin-bot[bot] commented 8 months ago

@tylerkaraszewski @greg-schroeder Be sure to fill out the Contact List!