Text on Delete Comment are not changing to new selected language

[kavimuru]

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

---

Action Performed:

1. Open two NewDot sessions
2. Press Delete Comment in a chat in one of the sessions (but leave the confirmation dialog open).
3. Observe that the message in the confirmation dialog is in the current language selected at that time.
4. Navigate to another NewDot session.
5. Click Settings.
6. Click Preferences.
7. Change the language setting to a different language.
8. Navigate back to the first session.
9. Notice that the language in the confirmation dialog to delete the chat has (incorrectly) not changed.
10. Press Cancel Button, notice that the language in the confirmation dialog flashes to the newly Language.

Expected Result:

Text on Delete Comment should be changed to new selected language

Actual Result:

Text on Delete Comment are not changing to new selected language

Workaround:

unknown

Platforms:

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

• Windows OS / Chrome / Safari

Version Number: 1.2.42-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Screen.Recording.2022-12-17.at.11.37.12.PM.mov

Recording.1137.mp4

Expensify/Expensify Issue URL:
Issue reported by: @varshamb
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1671301099745939

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01aa3d54fc6a28d4ff
• Upwork Job ID: 1608148498934513664
• Last Price Increase: 2022-12-28

[sketchydroide]

this does seem like a bug I will take a look at it.
Seems it is not updating, and only when the pop up is refreshed (py taping cancel) does it change the language
I'll check if Onyx is correctly setup

[joekaufmanexpensify]

Triaging:

From this SO: https://stackoverflow.com/c/expensify/questions/14418

• The "bug" is actually a bug: Yes, this is unexpected behavior.
• The bug is not a duplicate report of an existing GH (close the GH and add any novel details to the original GH instead): There is a similar bug, but we decided to move this to a new issue here, as the other issue's PR is up.
• The bug is reproducible, following the reproduction steps: Yes.
• If you're unable to reproduce the bug, add the Needs reproduction label. Comment on the issue outlining the steps you took to try to reproduce the bug, your results and tag the issue reporter and the Applause QA member who created the issue. Ask them to clarify reproduction steps and/or to check the reproduction steps again. Close issue.: N/A
• The GH template is filled out as fully as possible -- this means the GH body and title are clear (ie. could an external contributor understand it and work on it?): Yes.
• The GH was created by an Expensify employee or a QA tester: Yes.
• If the bug is an OldDot issue, you should decide whether it is widespread enough to justify fixing or it is better to wait for reunification. If it's better to wait, close the GH & provide this justification: N/A
• If there's a link to Slack, check the discussion to see if we decided not to fix it: Confirmed.
• Decide if the GH should be resolved by an External contributor or Internal engineer, add the appropriate label: Confirming.

[joekaufmanexpensify]

Reproduced on web:

[joekaufmanexpensify]

@sketchydroide Are you planning to keep this one internally? If so, mind adding the internal label? Otherwise, mind adding external?

[joekaufmanexpensify]

Also, I'm going to be OOO from tomorrow - January 2nd. Given this bug is in the early stages, I'm not going to re-assign another BZ as there should be no urgent work for BZ to do during that time period (even if this is external, no payment will be issued prior to January 2nd because we still need to write the PR, have it be merged/deployed, and then wait for 7 day regression period).

With that in mind, going to change to weekly for now. I'll change back to daily once I'm back. If at any point another BZ is needed while I'm out, feel free to re-assign.

[sketchydroide]

Internal I'll add the label later

[sketchydroide]

setting it as daily as I still think this is a bug for WAQ, and at the moment I don't think this needs a BZ, just to pay any reviews or bug find fees.

[sketchydroide]

the PR is pretty much done, I think we can have this reviewing yes, althoug I don't know why melvin keeps adding the Reviewing label to draft PRs

[sketchydroide]

#13859 has been aproved internally awaiting for C+ to review as well

[sketchydroide]

bumped the C+ review, it's a slow time of the year

[sketchydroide]

Merged, this should be in staging hopefully tomorrow

[joekaufmanexpensify]

Great! Looks like it was deployed to staging yesterday.

[joekaufmanexpensify]

Okay, this was merged to prod yesterday. Looks like there's an issue with the BZ and payment checklists. I raised this here.

In the meantime, going to manually create both.

[joekaufmanexpensify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.47-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

#13859

If no regressions arise, payment will be issued on 2023-01-10. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

[joekaufmanexpensify]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@sketchydroide / @eVoloshchak] The PR that introduced the bug has been identified. Link to the PR:
• [@sketchydroide / @eVoloshchak ] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@sketchydroide / @eVoloshchak] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion
• [@joekaufmanexpensify] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: https://github.com/Expensify/Expensify/issues/255482

[sketchydroide]

@joekaufmanexpensify I don't think this needs all of this since this was internal, it just needs to be payed for the PR review and the bug reported

[joekaufmanexpensify]

My understanding is we should be doing the bug zero checklist on all app issues, even if we fix it internally. As the bug still has a root cause, and we can still add a new regression test to catch it again in the future.

[joekaufmanexpensify]

Since we fixed this internally, we need to pay:

• @eVoloshchak - $1,000 for C+ PR review.
• @varshamb - $250 for reporting.

[joekaufmanexpensify]

@eVoloshchak offer for $1,000 sent!

[joekaufmanexpensify]

@varshamb offer for $250 sent!

[joekaufmanexpensify]

Going to add to weekly for now, as payment is pending on 2023-01-10 (if there are no regressions). Besides that, we just need to complete BZ checklist.

[joekaufmanexpensify]

All set to issue payment.

[joekaufmanexpensify]

I actually see there is some discussion here about whether the PR linked to this issue caused a regression. It sounds like the result of that discussion was that this PR did not cause any regressions, is that correct? I want to be sure before I pay out the review payment.

cc @sketchydroide @deetergp @jasperhuangg as you were all involved in the discussion.

[joekaufmanexpensify]

@varshamb $250 reporting payment sent, and contract ended!

[joekaufmanexpensify]

Proposed new regression test here.

[deetergp]

I would not consider that a regression. Implementing dark mode everywhere pointed out the fact that a component we use didn't natively support dark mode.

[joekaufmanexpensify]

Got it. So just to be 100% sure, you don't think the PR linked to this issue introduced a bug/regression? I just want to be sure before issuing payment as it seemed like you previously thought it might have based on this.

[joekaufmanexpensify]

Still discussing this potential regression piece which affects C+ pay out. Also BZ checklist is still pending.

[joekaufmanexpensify]

Discussed this with @deetergp 1:1 and confirmed that the PR linked to this issue did not cause any regressions. Proceeding with full payment for C+ review!

[joekaufmanexpensify]

@eVoloshchak $1,000 sent and contract ended!

[joekaufmanexpensify]

Upwork job closed.

[joekaufmanexpensify]

Added regression test above. All that's left here are the remaining engineer/C+ BZ checklist steps. cc @sketchydroide @eVoloshchak