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-04-12] [$500] iOS - App freezes when try to delete PayPal payment #7768

Closed
kavimuru opened this issue Feb 16, 2022 · 53 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 Reviewing Has a PR in review

Comments

@kavimuru
Copy link

kavimuru commented Feb 16, 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. Launch App as login user
  2. Navigate to Settings >Payments.
  3. Try to remove existing PayPal

Expected Result:

PayPal must be deleted and app should not freeze

Actual Result:

PayPal cannot be deleted and app freeze.

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • iOS

Version Number: 1.1.39-1

Reproducible in staging?: Y

Reproducible in production?: Y

Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5455298_Paypal_1502.mp4

Expensify/Expensify Issue URL:
Issue reported by: Applause @aswin-s
Slack conversation:

View all open jobs on GitHub

@MelvinBot
Copy link

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

@tylerkaraszewski
Copy link
Contributor

Seems like an issue to fix. Probably doable by an external contributor.

@tylerkaraszewski tylerkaraszewski removed their assignment Feb 16, 2022
@tylerkaraszewski tylerkaraszewski added the External Added to denote the issue can be worked on by a contributor label Feb 16, 2022
@MelvinBot
Copy link

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

@Christinadobrzyn
Copy link
Contributor

Created job post in Upwork

• Internal post - https://www.upwork.com/ab/applicants/1494155927379894272/job-details
• External post - https://www.upwork.com/jobs/~01041a83a6ab91e727

@botify botify removed the Daily KSv2 label Feb 17, 2022
@MelvinBot MelvinBot added the Weekly KSv2 label Feb 17, 2022
@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 Feb 17, 2022
@MelvinBot
Copy link

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

@sobitneupane
Copy link
Contributor

It looks like it has something to do with ConfirmPopover opening before the DeleteMenu Popover is closed.

<TouchableOpacity
onPress={() => {
this.setState({
shouldShowDefaultDeleteMenu: false,
shouldShowConfirmPopover: true,
});
}}

This solves the issue but it might not be the best thing to do. It might help someone to find the solution.

  <TouchableOpacity
      onPress={() => {
          this.setState({
              shouldShowDefaultDeleteMenu: false,
          }, () => {
              setTimeout(()=>{
                  this.setState({
                      shouldShowConfirmPopover: true,
                  });
              },1);
          });
      }}

@parasharrajat
Copy link
Member

You are correct @sobitneupane This is the real issue behind this problem. We have faced the same issue before. I was trying to solve it yesterday for one of the Deploy blocker issues. And I did something like your solution which worked but
There was a stage where this solution didn't work. When we close the PasswordModal via clicking in the backdrop. I am trying to find a fix for that before proposing a solution here.

@parasharrajat
Copy link
Member

parasharrajat commented Feb 18, 2022

Proposal

This is a nasty bug with RN. It is somehow related to Modals. There are many related issues open to RN. Maybe upgrading the RN version will solve it. But I don't think that upgrading RN would be a safe or easier option. it could break many dependencies.

When two modal instances are opened at the same time then App freezes in React Native. Basically, all of the app is functioning but a small transparent layer is left behind when the modal closes which blocks all the interactions.

this also causes other types of issues like

/* setTimeout delays execution to the frame after the modal closes
.

Good read
facebook/react-native#32329 (comment)
software-mansion/react-native-reanimated#2244

So to solve this we have to make sure that we don't open two modals at a time.

Now there are two modals that open on this BasePaymentPage component. DeleteConfirm Modal and Password Modal.

These modals are opened after Main Payment Menu.

To safely open these we can rely on InteractionManager.runafterInteractions.

So we will update Both onPress handlers to use it.


E.g.

onPress={() => {
                                        this.setState({
                                            shouldShowDefaultDeleteMenu: false,
                                        });
                                        InteractionManager.runAfterInteractions(() => {
                                            this.setState({
                                                shouldShowPasswordPrompt: true,
                                                passwordButtonText: this.props.translate('paymentsPage.setDefaultConfirmation'),
                                            });
                                        });
                                    }}

This will make sure that both are visible.

But I saw one strange issue when we close the PasswordPopover via clicking the backdrop, the App froze. There is nothing in the code that tells me why this is happening only for PasswordPopover. So to fix this I used this snippet to close the modal.

 hidePasswordPrompt() {
      this.setState({shouldShowPasswordPrompt: false});
// Only for iOS , no visible effect on UI
      LayoutAnimation.configureNext(LayoutAnimation.create(50, LayoutAnimation.Types.easeInEaseOut, LayoutAnimation.Properties.opacity));
  }

Here Layoutshift causes hanged animation to flush. It fixes the issues.

@rushatgabhane
Copy link
Member

@mountiny Rajat doesn't need a C+ review, unassigning myself.

@rushatgabhane rushatgabhane removed their assignment Feb 21, 2022
@parasharrajat
Copy link
Member

I am very busy but I would like to test this via upgrading the RN version. Upgrading RN could be challenging. If this is an urgent issue, then I am happy to go with the above fix but I think we should wait till I test it on the newer RN version.

@mountiny
Copy link
Contributor

@parasharrajat I dont think this is very urgent in terms of production since this should still be in beta. What would be your ETA for trying to update RN?

@parasharrajat
Copy link
Member

Can't really tell. By weekend maybe.

@mountiny
Copy link
Contributor

I think we can wait for you to try to update the RN version, it could be beneficial in other ways too and as mentioned this bug is still behind beta, so hopefully no problem. If it would become a bigger problem, we have a solution to deploy if the need be.

@mountiny
Copy link
Contributor

mountiny commented Mar 1, 2022

@parasharrajat Any updates here?

@mountiny mountiny changed the title [HOLD for payment 2022-04-06] [$500] iOS - App freezes when try to delete PayPal payment [$500] iOS - App freezes when try to delete PayPal payment Mar 30, 2022
@mountiny mountiny removed the Awaiting Payment Auto-added when associated PR is deployed to production label Mar 30, 2022
@mountiny
Copy link
Contributor

Updated the title and Awaiting Payment label, we will wait for the other PR to address the missing case.

@parasharrajat
Copy link
Member

PR will be ready by EOD.

@parasharrajat
Copy link
Member

parasharrajat commented Apr 4, 2022

I was facing many issues with MAC. I finally manage to fix the MAC and build setup. I will create the PR by tomorrow ⏲️

@mountiny
Copy link
Contributor

mountiny commented Apr 4, 2022

Thanks for keeping us updated @parasharrajat

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@parasharrajat
Copy link
Member

@mountiny I can't reproduce this issue anymore. Are you able to provide the steps?

@mountiny
Copy link
Contributor

mountiny commented Apr 5, 2022

@parasharrajat that is good to hear it si not reproducible. The steps I followed were same as in the PR, would you be fine with holding the payment for the second week of retest by QA @mvtglobally so in 7 days and then if issue does not persist, the payment would be released? To follow the classic KI retest process?

@parasharrajat
Copy link
Member

Sure. No problem.

@mountiny mountiny changed the title [$500] iOS - App freezes when try to delete PayPal payment [HOLD for payment 2022-04-12] [$500] iOS - App freezes when try to delete PayPal payment Apr 7, 2022
@mountiny
Copy link
Contributor

mountiny commented Apr 7, 2022

Updated the title to be one week from the first re-test. @mvtglobally once you confirm next week the issue is not reproducible we are good to close this one out 🙌

The steps which led to the problem noticed before:

  1. Open the Payment Page from Settings => Payments.
  2. Click/Tap on any existing payment method Except Paypal.
  3. A new menu modal should open.
  4. Now select Make default payment Method.

Then the modal closed but the app froze.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)
@mountiny good to close it

@mountiny
Copy link
Contributor

@Christinadobrzyn This is good to be paid out to Rajat and closed then! Thanks!

Thank you @mvtglobally

@mountiny mountiny added Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 and removed Weekly KSv2 labels Apr 12, 2022
@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Apr 13, 2022

Original job expired so created new one - https://www.upwork.com/ab/applicants/1514041584503480320/job-details

Sent an offer to Rajat. I'll track and pay asap.

@Christinadobrzyn
Copy link
Contributor

Paid @parasharrajat and closed the job in Upwork. Closing this GH

@aswin-s
Copy link
Contributor

aswin-s commented Apr 13, 2022

@Christinadobrzyn Reporting bonus was not paid for this issue.

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Apr 13, 2022

Ah, sorry about that @aswin-s. I thought this was reported by our QA team. I can only find this instance of the issue being reported in our #open-source room by our QA team.

Can you please send me a link to the Bug report you posted in the open-source room? Thanks!

@aswin-s
Copy link
Contributor

aswin-s commented Apr 13, 2022

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Apr 13, 2022

Amazing! Thank you! Let me reopen the job and hire you so I can pay you.

@aswin-s I just sent an offer to this job - https://www.upwork.com/ab/applicants/1514123199221063680/job-details Thanks!

@aswin-s
Copy link
Contributor

aswin-s commented Apr 13, 2022

Amazing! Thank you! Let me reopen the job and hire you so I can pay you.

@aswin-s I just sent an offer to this job - https://www.upwork.com/ab/applicants/1514123199221063680/job-details Thanks!

@Christinadobrzyn Applied. Thanks.

@Christinadobrzyn
Copy link
Contributor

Thank you! just issued payment through the Upwork job. Thanks again for reaching out about that and sorry I missed that!

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 Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests