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 2022-01-25] Android - App crashed after removing all permissions #7008

Closed
kbecciv opened this issue Jan 4, 2022 · 18 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Jan 4, 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!


Action Performed:

  1. Lunch app in an Android device.
  2. Open a conversation with an attachment in it.
  3. Tap on the attachment to view it in Attachment view
  4. Tap on download icon to download the attachment
  5. Kill the app and go to your phone Setting
  6. Remove all permission for New Expensify
  7. Disable "Remove permissions and free up space" tag.
  8. Go back to New Expensify app icon and open it

Expected Result:

New Expensify app is working as expected without crash

Actual Result:

App crashed after removing all permissions.

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Android

Version Number: 1.1.24-8

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

148008838-9dbf42fb-e5b1-4a1c-852d-bb3c8a26243c.mp4

Upwork job URL: https://www.upwork.com/jobs/~01175e803854a62ae4

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

@MelvinBot
Copy link

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

@aswin-s
Copy link
Contributor

aswin-s commented Jan 4, 2022

This issue is unrelated to #6613.

The crash is caused due to view state is not being persisted across activity restarts when permission is changed.

@aswin-s
Copy link
Contributor

aswin-s commented Jan 4, 2022

Original crash log.
image

It seems like react-native-screens was not properly configured for android.
https://github.com/software-mansion/react-native-screens#android

On Android the View state is not persisted consistently across Activity restarts, which can lead to crashes in those cases. It is recommended to override the native Android method called on Activity restarts in your main Activity, to avoid these crashes.

For most people using an app built from the react-native template, that means editing MainActivity.java, likely located in android/app/src/main/java//MainActivity.java

You should add this code, which specifically discards any Activity state persisted during the Activity restart process, to avoid inconsistencies that lead to crashes.

import android.os.Bundle;

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(null);
}

@danieldoglas danieldoglas added Improvement Item broken or needs improvement. External Added to denote the issue can be worked on by a contributor labels Jan 6, 2022
@MelvinBot
Copy link

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

@danieldoglas danieldoglas removed their assignment Jan 6, 2022
@MelvinBot MelvinBot removed the Overdue label Jan 6, 2022
@trjExpensify
Copy link
Contributor

Job posted to Upwork: https://www.upwork.com/jobs/~01175e803854a62ae4

@MelvinBot
Copy link

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

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 7, 2022
@MelvinBot
Copy link

Triggered auto assignment to @pecanoro (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@aswin-s
Copy link
Contributor

aswin-s commented Jan 7, 2022

@rushatgabhane @pecanoro I've posted the reason for the crash & how to fix above. This was incorrectly tagged as a possible regression from one of my earlier PRs. That's why I had pitched in quite early.

@pecanoro
Copy link
Contributor

pecanoro commented Jan 7, 2022

@aswin-s Thanks for the insight! Go ahead with the proposed solution, @trjExpensify will send the job offer in Upwork!

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 7, 2022
@MelvinBot
Copy link

📣 @aswin-s You have been assigned to this job by @pecanoro!
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 📖

@trjExpensify
Copy link
Contributor

Awesome! @aswin-s can you apply to the job on Upwork and then I'll hire you and start the contract. 👍

@rushatgabhane
Copy link
Member

Makes sense. Thanks for the deets @aswin-s!

@trjExpensify
Copy link
Contributor

Sent you the offer on Upwork, @aswin-s. 👍

@pecanoro pecanoro added the Reviewing Has a PR in review label Jan 17, 2022
@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@trjExpensify
Copy link
Contributor

@mvtglobally were you testing on staging? Looks like this was deployed to staging a few days ago: #7100 (comment)

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 18, 2022
@botify
Copy link

botify commented Jan 18, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.30-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 2022-01-25. 🎊

@botify botify changed the title Android - App crashed after removing all permissions [HOLD for payment 2022-01-25] Android - App crashed after removing all permissions Jan 18, 2022
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@trjExpensify
Copy link
Contributor

Cool, we've hit 7 days since this was deployed to prod. I've settled up with you on Upwork, @aswin-s. Closing this out, thanks everyone! 🎉

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 Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants