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 payment 2023-08-10] [$1000] Android - Emoji - App crashes after selecting Emoji icon that is above the "Frequently used label" #23716

Closed
1 of 6 tasks
kbecciv opened this issue Jul 27, 2023 · 32 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Jul 27, 2023

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. Access a conversation chat with another user.
  2. Long press on any of your messages.
  3. Tap the "emoji reaction" icon.
  4. Tap the "emoji" icon that is above the "Frequently used label".

Expected Result:

The app should not crash.

Actual Result:

The app crashes.

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: v1.3.46-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Bug6143286_App_crashes_after_selecting_Emoji_icon_that_is_above_the_Frequently_used_label.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/~0165450f4a10937496
  • Upwork Job ID: 1684677319871397888
  • Last Price Increase: 2023-07-27
@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Jul 27, 2023
@OSBotify
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.

@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

Triggered auto assignment to @Gonals (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@hungvu193
Copy link
Contributor

Proposal

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

Android - Emoji - App crashes after selecting Emoji icon that is above the "Frequently used label"

What is the root cause of that problem?

Problem came from this line:

import Animated, {runOnUI, _scrollTo} from 'react-native-reanimated';

We're importing the wrong function which didn't exist.

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

Remove usage of _scrollTo

 import Animated, {runOnUI} from 'react-native-reanimated'; 

We can use this.emojiList.scrollToOffset({ offset: calculatedOffset, animated: true }) instead

this.emojiList.scrollToOffset({ offset: calculatedOffset, animated: true })

What alternative solutions did you explore? (Optional)

N/A

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Jul 27, 2023
@shubham1206agra
Copy link
Contributor

shubham1206agra commented Jul 27, 2023

Proposal

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

Android - Emoji - App crashes after selecting Emoji icon that is above the "Frequently used label"

What is the root cause of that problem?

In this line:

import Animated, {runOnUI, _scrollTo} from 'react-native-reanimated';

Since the react-native-reanimated is bumped to v3.4.0 in #23550. The function _scrollTo was removed, and we are forced to use scrollTo function instead. Due to this, the function not found was shown.

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

Since scrollTo requires the use of useAnimatedRef for the first argument. We will first quickly migrate this component to a functional component. And replace with scrollTo since this is the function recommended by react-native-reanimated.

What alternative solutions did you explore? (Optional)

N/A

@shubham1206agra
Copy link
Contributor

@Gonals BTW, this issue is also reproducible on iOS.

And I have tested my solution. Works without any problem :)

@stitesExpensify stitesExpensify self-assigned this Jul 27, 2023
@marcaaron marcaaron removed the DeployBlockerCash This issue or pull request should block deployment label Jul 27, 2023
@marcaaron
Copy link
Contributor

revert fix tests well removing the blocker.

@stitesExpensify
Copy link
Contributor

We should still switch this to a functional component so that we can upgrade our version of reanimated. Marking this external to get a C+

@stitesExpensify stitesExpensify added 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 labels Jul 27, 2023
@melvin-bot melvin-bot bot changed the title Android - Emoji - App crashes after selecting Emoji icon that is above the "Frequently used label" [$1000] Android - Emoji - App crashes after selecting Emoji icon that is above the "Frequently used label" Jul 27, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0165450f4a10937496

@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

Triggered auto assignment to @jliexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

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

@stitesExpensify
Copy link
Contributor

@mananjadhav @shubham1206agra's proposal looks good to me! What do you think?

@aimane-chnaif
Copy link
Contributor

aimane-chnaif commented Jul 28, 2023

Coming from #18507 (comment):
This issue is now high priority, which blocks upgrading RN 0.72.
Can the proposals be reviewed asap?
I suggest this being worked by expert contributor group.
cc: @mountiny

@mananjadhav
Copy link
Collaborator

I think @shubham1206agra's proposal is good too as it is recommended way by react-native-animated.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

📣 @mananjadhav Please request via NewDot manual requests for the Reviewer role ($1000)

@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

📣 @shubham1206agra 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 📖

@mountiny
Copy link
Contributor

Jumping in to speed this up @wojtus7 Could you please overlook the PR as well? We need to update the reanimated again and prevent the crash. Thanks!

@shubham1206agra Can you create the PR quickly? Thanks 🙇

@shubham1206agra
Copy link
Contributor

PR will be ready today.

I just need to get Android test.

@shubham1206agra
Copy link
Contributor

@mountiny Should I rebump react-native-reanimated to v3.4?

@shubham1206agra
Copy link
Contributor

📣 @shubham1206agra 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 📖

BTW I am still not hired.

@mountiny
Copy link
Contributor

@jliexpensify will hopefully get around to it in next 24 hours.

@jliexpensify
Copy link
Contributor

Hired @shubham1206agra

@melvin-bot
Copy link

melvin-bot bot commented Aug 1, 2023

🎯 ⚡️ Woah @mananjadhav / @shubham1206agra, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

  • when @shubham1206agra got assigned: 2023-07-28 13:13:51 Z
  • when the PR got merged: 2023-08-01 09:27:59 UTC

On to the next one 🚀

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Aug 3, 2023
@melvin-bot melvin-bot bot changed the title [$1000] Android - Emoji - App crashes after selecting Emoji icon that is above the "Frequently used label" [HOLD for payment 2023-08-10] [$1000] Android - Emoji - App crashes after selecting Emoji icon that is above the "Frequently used label" Aug 3, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Aug 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2023

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

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

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

For reference, here are some details about the assignees on this issue:

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

@mananjadhav
Copy link
Collaborator

This was more of a fix related to upgrading the reanimated version and not a dev bug per say. Hence there is no offending PR.

I also don't think we should be adding a regression test for this one. The EmojiPicker should already consist of the tests for this.

@jliexpensify
Copy link
Contributor

jliexpensify commented Aug 9, 2023

Great, here's a Payment Summary:

Upworks job

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 9, 2023
@mananjadhav
Copy link
Collaborator

@jliexpensify I've raised my request on NewDot.

@JmillsExpensify
Copy link

Reviewed the details for @mananjadhav. This is approved for payment in NewDot.

@jliexpensify
Copy link
Contributor

Job closed in Upworks!

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 Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests