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

[$250] [HOLD for payment 2024-08-01] Group name - RBR doesn't disappear from LHN after removing error message #42765

Closed
1 of 6 tasks
lanitochka17 opened this issue May 29, 2024 · 45 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering 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

@lanitochka17
Copy link

lanitochka17 commented May 29, 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.77-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #41826

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Create a group chat
  3. Go offline
  4. Rename the group to something invalid for example "adoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoajdoajadoadoadoajdoajdoadoaỏ"
  5. Go online
  6. Follow the RBR and remove the error message

Expected Result:

RBR on LHN gets removed from LHN as well

Actual Result:

RBR stays on LHN even after removing error message

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

Bug6495078_1716985565573.bandicam_2024-05-29_15-20-11-422.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f34a445311dae423
  • Upwork Job ID: 1821299949503498320
  • Last Price Increase: 2024-08-07
Issue OwnerCurrent Issue Owner: @rojiphil
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels May 29, 2024
Copy link

melvin-bot bot commented May 29, 2024

Triggered auto assignment to @danieldoglas (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@lanitochka17
Copy link
Author

@danieldoglas FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@lanitochka17
Copy link
Author

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

@Nodebrute
Copy link
Contributor

Nodebrute commented May 29, 2024

Proposal

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

RBR doesn't disappear from LHN after removing error message

What is the root cause of that problem?

Here we call clearPolicyRoomNameErrors

onClose={() => ReportActions.clearPolicyRoomNameErrors(reportID)}

and in clearPolicyRoomNameErrors we only clear errorFields but not errors

App/src/libs/actions/Report.ts

Lines 2220 to 2229 in c244fa1

function clearPolicyRoomNameErrors(reportID: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
errorFields: {
reportName: null,
},
pendingFields: {
reportName: null,
},
});
}

and here when we fail we set errors to common.genericErrorMessage
errors: {
reportName: Localize.translateLocal('common.genericErrorMessage'),
},

So the RCA is when "x" is pressed we clear errorFields but not errors

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

Solution 1
Instead of setting errors here set "errorFields" error to invalid Name

errors: {
reportName: Localize.translateLocal('common.genericErrorMessage'),
},

Note:we can keep errors here too.

What alternative solutions did you explore? (Optional)

In clearPolicyRoomNameErrors also clear errors or create a new function to use here

App/src/libs/actions/Report.ts

Lines 2220 to 2229 in c244fa1

function clearPolicyRoomNameErrors(reportID: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
errorFields: {
reportName: null,
},
pendingFields: {
reportName: null,
},
});
}

        errors: {
                    reportName: 'null',
                },

@luacmartins luacmartins removed the DeployBlocker Indicates it should block deploying the API label May 29, 2024
@puneetlath
Copy link
Contributor

Doesn't seem blocker-worthy to me. Let's treat as a regular bug.

@puneetlath puneetlath removed the DeployBlockerCash This issue or pull request should block deployment label May 29, 2024
@danieldoglas danieldoglas added Daily KSv2 External Added to denote the issue can be worked on by a contributor Bug Something is broken. Auto assigns a BugZero manager. and removed Hourly KSv2 labels May 30, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 2, 2024
Copy link

melvin-bot bot commented Jun 2, 2024

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

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

melvin-bot bot commented Jun 2, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 2, 2024
Copy link

melvin-bot bot commented Jun 2, 2024

Triggered auto assignment to @muttmuure (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.

@muttmuure
Copy link
Contributor

muttmuure commented Jun 5, 2024

Catching up from OOO

@melvin-bot melvin-bot bot added the Overdue label Jun 5, 2024
@muttmuure
Copy link
Contributor

Not overdue

@Nodebrute
Copy link
Contributor

@rojiphil bump

@rojiphil
Copy link
Contributor

rojiphil commented Aug 4, 2024

@muttmuure Completed BZ checklist.
I am not finding the upwork offer for this. Can you please create an offer if not present somewhere? Thanks.

Copy link

melvin-bot bot commented Aug 5, 2024

@rojiphil, @danieldoglas, @mountiny, @muttmuure, @Nodebrute Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot removed the Overdue label Aug 6, 2024
@danieldoglas danieldoglas added Overdue Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Aug 6, 2024
Copy link

melvin-bot bot commented Aug 6, 2024

Triggered auto assignment to @mallenexpensify (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 melvin-bot bot removed the Overdue label Aug 6, 2024
@danieldoglas
Copy link
Contributor

@mallenexpensify can you please take care of the payment on this one? Thanks!

@mallenexpensify mallenexpensify added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Aug 7, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-08-01] Group name - RBR doesn't disappear from LHN after removing error message [$250] [HOLD for payment 2024-08-01] Group name - RBR doesn't disappear from LHN after removing error message Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

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

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

melvin-bot bot commented Aug 7, 2024

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

@mallenexpensify
Copy link
Contributor

@rojiphil @Nodebrute , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01f34a445311dae423

@Nodebrute
Copy link
Contributor

@mallenexpensify I haven’t received the Upwork offer yet. Can you send the contract manually when you get a chance?

@mallenexpensify
Copy link
Contributor

Musta forgot to invite ¯_(ツ)_/¯, plz accept meow 🐱
https://www.upwork.com/jobs/~01f34a445311dae423

@Nodebrute
Copy link
Contributor

@mallenexpensify thank you, accepted.

@rojiphil
Copy link
Contributor

@mallenexpensify Accepted offer. Thanks.

@melvin-bot melvin-bot bot added the Overdue label Aug 12, 2024
@danieldoglas
Copy link
Contributor

payment in progress

@melvin-bot melvin-bot bot removed the Overdue label Aug 12, 2024
@mallenexpensify
Copy link
Contributor

Contributor: @Nodebrute paid $250 via Upwork
Contributor+: @rojiphil paid $250 via NewDot

@rojiphil do we want a regression test for this? If not, why not? Thx

@rojiphil
Copy link
Contributor

Contributor: @Nodebrute paid $250 via Upwork Contributor+: @rojiphil paid $250 via NewDot

@rojiphil do we want a regression test for this? If not, why not? Thx

@mallenexpensify As mentioned in BZ checklist here, I think we can skip the regression test as this is an edge case.

@mallenexpensify
Copy link
Contributor

BZ checklist #42765 (comment),

@rojiphil , it looks like you just checked the box there. We also want reasoning if we don't create a test case. We want to err on the side of creating more test cases because the QA will know if it's needed. They also have a separate monthly test for edge cases.

I created a test case using the steps in the PR

@rojiphil
Copy link
Contributor

We want to err on the side of creating more test cases because the QA will know if it's needed. They also have a separate monthly test for edge cases.

@mallenexpensify Ok. Got it and noted too. Thanks.

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. Daily KSv2 Engineering 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
None yet
Development

No branches or pull requests

9 participants