[HOLD for payment 2023-04-12] [$4000] [Feature Request] Compose box should be accessible when opening a old chat

[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. Go to NewDot
2. Open any report that you have previously chatted with, but haven’t for quite some time

Expected Result:

The compose box is accessible and a message can be typed even while the skeleton UI loads

Actual Result:

The compose box is blocked until the skeleton UI loads. Clicking it does nothing, even though you can see it.

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.52-4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Recording.1261.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014ee521941ee126ec
• Upwork Job ID: 1617901674597654528
• Last Price Increase: 2023-03-28

[melvin-bot]

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

[techievivek]

We are already discussing this here. And will get fixed in this PR.

[mallenexpensify]

Thanks @techievivek !! Put on hold pending #13088

[yuwenmemon]

@techievivek I think we're dealing with a bit of a separate issue than you dealt with in the PR you've linked. This behavior is actually happening while online as well, and the chats are still loading.

[techievivek]

@yuwenmemon Agree, it seems to be happening in both offline and online cases.

[JmillsExpensify]

Ok yeah, that's right. This issue is about making sure that we don't block the user in an online case, though interesting that we also seem to be doing the same in the offline case. From the thread, it sounds like on offline we already had a non-blocking framework in place.

[techievivek]

it sounds like on offline we already had a non-blocking framework in place.

We have things already in place; it's just that the compose box is disabled, so users can't take optimistic actions in both online and offline cases.

[techievivek]

Passing it to contributors to work on it.

[melvin-bot]

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

[melvin-bot]

Current assignees @JmillsExpensify and @mallenexpensify are eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[MelvinBot]

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

• Make Compose box accessible before the chat loads #16705

If no regressions arise, payment will be issued on 2023-04-12. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• 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

[techievivek]

Hey, @mallenexpensify, it there anything else that needs to be done here, or are we good to close this one?

[mallenexpensify]

@techievivek @johnmlee101 , I see the below here

Possible Regression: #16908

Do either of you have any further insight or details before I pay?

Also.. for payment, below is the bonus timeline.

Hired - March 29

Merged March 31

[techievivek]

@mallenexpensify Yes, there was a regression from the original PR here #16908, which was also fixed by @getusha. The original PR was merged on April 1 and the regression was reported on April 4.

[getusha]

The regression was reported and fixed April 4 before hitting production.

[mallenexpensify]

Thanks @techievivek and @getusha. Per CONTRIBUTING.md.

If the PR causes a regression within 7 days of being deployed to production, contributors are not eligible for the 50% bonus.

So... if the regression was before production, that shouldn't affect the bonus for @getusha .

@sobitneupane , this is from the C+ process doc

If regressions are found that “should have”* been caught after the PR has been approved, the C+’s payment gets cut in half for each regression found (½ the first time, ½ the second, etc (ie. $1000 > $500 > $250)). This includes regressions on staging and production. Idea discussion slack here. Any bonus increases that might have been added for timeliness are rescinded for regressions.
*“Should have” = C+ should have caught the bug by fully following the checklist. If C+ skips a step or completed the checklist incompletely, payment will be cut in half.

If you don't think you "should have" caught the regression, can you provide reasoning?

[aimane-chnaif]

So... if the regression was before production, that shouldn't affect the bonus for @getusha .

Does that mean that if regression found on main or staging, bonus still applies to contributor?

[sobitneupane]

If you don't think you "should have" caught the regression, can you provide reasoning?

I think I should have caught it earlier though it is an edge case.

[techievivek]

Not overdue, we are just finalizing the payment here.

[mallenexpensify]

@aimane-chnaif , checking on here https://expensify.slack.com/archives/C02NK2DQWUX/p1681749639671849

So... if the regression was before production, that shouldn't affect the bonus for @getusha .

Does that mean that if regression found on main or staging, bonus still applies to contributor?

[getusha]

@mallenexpensify any updates?

[mallenexpensify]

Thanks for the ping @getusha , we're in the process of updating the language for CONTRIBUTING.md to denote that any regression cancels the bonus because there's no benefit for timeliness if there are regressions that require additional resources to address.

If the PR causes a regression within 7 days of being deployed to production, contributors are not eligible for the 50% bonus.

within 7 days of being deployed to production is the part we still need to honor here and the regression was not within that time period, it was before, so the bonus stands for now.

@sobitneupane paid $2000 and @getusha paid $6000. Anticipate CONTRIBUTING.md being updated very soon with the new language stating that timeliness bonuses won't be eligible if there are regressions.

[mallenexpensify]

For some reason this didn't get posted, @sobitneupane , can you complete the below then I'll create a GH to update TestRail?

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:

• [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:
• [@sobitneupane] 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:
• [@sobitneupane] 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:
• [@mallenexpensify] Determine if we should create a regression test for this bug.
• [@sobitneupane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[sobitneupane]

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:

• [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

#12505

• [@sobitneupane] 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:

https://github.com/Expensify/App/pull/12505/files#r1088840195

• [@sobitneupane] 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:

It is an improvement in the app rather than bug fix as discussed earlier in https://github.com/Expensify/App/pull/12505/files#r1088840195

[sobitneupane]

Regression Test Proposal

1. Open Expensify app
2. Disconnect the device from the internet
3. Open any old chat or a chat that is still loading
4. Type a message in the compose box.
5. Open EmojiPicker and insert an emoji
6. Send the message by clicking the send icon
7. Add any attachment and send the attachment
8. Confirm Sending, splitting, requesting money works as expected
9. Verify that the compose box is accessible and functioning properly(i.e, you are able to perform step 3, step 4, step 5, step 6, and step 7) as it would when the device is connected to the internet

Do we agree 👍 or 👎

[JmillsExpensify]

Thank you so much for getting this one out and in production! It's really great. ❤️

[mallenexpensify]

Thanks @sobitneupane (and @getusha for the 👍)
GH created to update TestRail with regression test steps
https://github.com/Expensify/Expensify/issues/277753