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

[HOLD for regression test steps][$1000] Web - Changing room name offline doesn't reflect in LHN until another chat/room opened #13457

Closed
kbecciv opened this issue Dec 9, 2022 · 30 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Dec 9, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue found when executing PR #13189

Action Performed:

  1. Go to https://staging.new.expensify.com/
    and login
  2. Create a room
  3. Go offline
  4. Go to room settings
  5. Change room name
  6. Click Save

Expected Result:

Room name should be updated through out the app

Actual Result:

Room name is not updated in LHN until another chat/room opened

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web

Version Number: 1.2.37.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5855586_video_21.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b4df137e42b2ccf6
  • Upwork Job ID: 1601251791178326016
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 9, 2022

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

@tienifr
Copy link
Contributor

tienifr commented Dec 9, 2022

Proposal

Problem

In App/src/pages/home/sidebar/SidebarLinks.js
Because the chatReportSelector function doesn't update reportName and then when a reportName of the report is updated the old data and the updated data is similar so the SidebarLinks component isn't rendered

Solution

In https://github.com/tienifr/App/blob/216bbe9ff3995264e08a542d618bcc1db12d946f/src/pages/home/sidebar/SidebarLinks.js#L205

we will update the reportName and then when reportName is updated, this component will rendered again and updated the lastest information

const chatReportSelector = (report) => {
    if (ReportUtils.isIOUReport(report)) {
        return null;
    }
    return report && ({
        reportID: report.reportID,
        participants: report.participants,
        hasDraft: report.hasDraft,
        isPinned: report.isPinned,
        errorFields: {
            addWorkspaceRoom: report.errorFields && report.errorFields.addWorkspaceRoom,
        },
        maxSequenceNumber: report.maxSequenceNumber,
        lastReadSequenceNumber: report.lastReadSequenceNumber,
        lastMessageText: report.lastMessageText,
        lastActionCreated: report.lastActionCreated,
        iouReportID: report.iouReportID,
        hasOutstandingIOU: report.hasOutstandingIOU,
        statusNum: report.statusNum,
        stateNum: report.stateNum,
        chatType: report.chatType,
        policyID: report.policyID,
+        reportName: report.reportName,
    });
};

Result

Screen.Recording.2022-12-09.at.15.07.30.mov

@zanyrenney zanyrenney added the External Added to denote the issue can be worked on by a contributor label Dec 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 9, 2022

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

@melvin-bot melvin-bot bot changed the title Web - Changing room name offline doesn't reflect in LHN until another chat/room opened [$1000] Web - Changing room name offline doesn't reflect in LHN until another chat/room opened Dec 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 9, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Dec 9, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 9, 2022

Triggered auto assignment to @neil-marcellini (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@tgolen
Copy link
Contributor

tgolen commented Dec 9, 2022

🟢 on the proposal from @tienifr. Without the report name being returned there, the LHN will never get updated when the name changes.

@mananjadhav
Copy link
Collaborator

Good with @tienifr's proposal.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 9, 2022

📣 @tienifr You have been assigned to this job by @neil-marcellini!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@neil-marcellini
Copy link
Contributor

neil-marcellini commented Dec 9, 2022

It looks like this regression was introduced here d9da250. I checked out out the parent commit 15e57cd and verified that the bug did not exist at that point. I think we already have a similar regression test, so we should add this specific case.

Screen.Recording.2022-12-09.at.10.32.16.AM.mov

@tienifr
Copy link
Contributor

tienifr commented Dec 11, 2022

Thanks, I've applied to the job, the PR will be ready by Dec 12

@mananjadhav
Copy link
Collaborator

Thanks for the update @tienifr. Let's target to get it closed by Dec 12 itself to be eligible for 50% bonus.

@neil-marcellini
Copy link
Contributor

We're waiting on the fix to be deployed now.

@melvin-bot melvin-bot bot removed the Daily KSv2 label Dec 15, 2022
@mallenexpensify
Copy link
Contributor

Hired @tienifr and @mananjadhav can you please accept the job and reply here once you have?

@mananjadhav
Copy link
Collaborator

Accepted @mallenexpensify

@tienifr
Copy link
Contributor

tienifr commented Dec 24, 2022

accepted, thanks @mallenexpensify

@melvin-bot melvin-bot bot added the Overdue label Dec 26, 2022
@mallenexpensify
Copy link
Contributor

@tienifr and @mananjadhav paid $1500/each. Thanks for the quick fix
Leaving open cuz @zanyrenney needs to add the regression test steps, I'll try to get to this week too.

@neil-marcellini
Copy link
Contributor

^ we're waiting on the regression tests. I'm commenting to remove the overdue label.

@melvin-bot melvin-bot bot removed the Overdue label Dec 27, 2022
@mallenexpensify mallenexpensify changed the title [HOLD for payment 2022-12-22] [$1000] Web - Changing room name offline doesn't reflect in LHN until another chat/room opened [HOLD for regression test steps][$1000] Web - Changing room name offline doesn't reflect in LHN until another chat/room opened Dec 27, 2022
@melvin-bot melvin-bot bot added the Overdue label Dec 29, 2022
@neil-marcellini
Copy link
Contributor

Same

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 29, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jan 2, 2023

@mananjadhav, @neil-marcellini, @zanyrenney, @tienifr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels Jan 3, 2023
@melvin-bot melvin-bot bot removed the Overdue label Jan 3, 2023
@mananjadhav
Copy link
Collaborator

@zanyrenney Did you get a chance to look at the above comments on pending regression steps?

@zanyrenney
Copy link
Contributor

zanyrenney commented Jan 10, 2023

The call for the regression steps came in whilst I was OOO for the holidays (26th December). Working on this now.

@zanyrenney
Copy link
Contributor

@mananjadhav
Copy link
Collaborator

thanks for picking this up @zanyrenney.

@melvin-bot melvin-bot bot added the Overdue label Jan 20, 2023
@zanyrenney
Copy link
Contributor

Going to bump the QA channel for the regression test.

@melvin-bot melvin-bot bot removed the Overdue label Jan 20, 2023
@melvin-bot melvin-bot bot added the Overdue label Jan 30, 2023
@zanyrenney
Copy link
Contributor

Bumped the issue again. https://github.com/Expensify/Expensify/issues/254153

@melvin-bot melvin-bot bot removed the Overdue label Jan 30, 2023
@melvin-bot melvin-bot bot added the Overdue label Feb 7, 2023
@neil-marcellini
Copy link
Contributor

Looks like the regression tests were added. Have we paid everyone? If so I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants