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

[$500] Android - Workspace - App crashes when add second member after killing the app #32869

Closed
1 of 6 tasks
lanitochka17 opened this issue Dec 11, 2023 · 33 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 11, 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!


Version Number: 1.4.11-1
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Navigate to New dot App
  2. Go to Settings> Workspace> Members> Invite
  3. Invite any user and tap Next
  4. Add custom invite message and tap Invite
  5. Kill the app
  6. Open the app
    7, Go to Settings> Workspace> Members> Invite
    8, Invite any user and tap Next

Expected Result:

"Add message" screen should remain the previous message and user should be able to finish the flow

Actual Result:

App crashes when add second user to workspace

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6309625_1702327809260.az_recorder_20231211_175030.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0195d5d4886bc42022
  • Upwork Job ID: 1734325799100907520
  • Last Price Increase: 2024-01-08
@lanitochka17 lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 11, 2023
@melvin-bot melvin-bot bot changed the title Android - Workspace - App crashes when add second member after killing the app [$500] Android - Workspace - App crashes when add second member after killing the app Dec 11, 2023
Copy link

melvin-bot bot commented Dec 11, 2023

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

Copy link

melvin-bot bot commented Dec 11, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0195d5d4886bc42022

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 11, 2023
Copy link

melvin-bot bot commented Dec 11, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Copy link

melvin-bot bot commented Dec 11, 2023

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

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Dec 12, 2023

Screenshot 2023-12-12 at 22 44 42

I'm not sure if this is the bug we're trying to fix.
But everything seems to match
The only thing is that this crash can happen without killing the app

Proposal

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

Android - Workspace - App crashes when add second member after killing the app

What is the root cause of that problem?

The problem is due to the fact that when mounting the Screen, the ref is not yet ready

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

We can update this function like

focusWelcomeMessageInput() {
    this.focusTimeout = setTimeout(() => {
        if (!this.welcomeMessageInputRef) {
            return;
        }
        if (this.welcomeMessageInputRef.focus) {
            this.welcomeMessageInputRef.focus();
        }
        // Below condition is needed for web, desktop and mweb only, for native cursor is set at end by default.
        if (this.welcomeMessageInputRef.value && this.welcomeMessageInputRef.setSelectionRange) {
            const length = this.welcomeMessageInputRef.value.length;
            this.welcomeMessageInputRef.setSelectionRange(length, length);
        }
    }, CONST.ANIMATED_TRANSITION);
}

this.welcomeMessageInputRef.focus();

What alternative solutions did you explore? (Optional)

Alternatively, we can add a recursion that will check for focus availability every 300 seconds until the ref is available (We can create special function for class component which we will use for autofocus)

focusWelcomeMessageInput() {
    const setFocus = () => {
        if (this.welcomeMessageInputRef) {
            this.welcomeMessageInputRef.focus();
            // Below condition is needed for web, desktop, and mweb only; for native, the cursor is set at the end by default.
            if (this.welcomeMessageInputRef.value && this.welcomeMessageInputRef.setSelectionRange) {
                const length = this.welcomeMessageInputRef.value.length;
                this.welcomeMessageInputRef.setSelectionRange(length, length);
            }
        } else {
            setTimeout(setFocus, CONST.ANIMATED_TRANSITION); // Check again after a delay if focus is still undefined
        }
    };

    this.focusTimeout = setTimeout(setFocus, CONST.ANIMATED_TRANSITION);
}

or we can just update this line like and increase the waiting time (In theory this should help)

}, CONST.ANIMATED_TRANSITION * 2);

}, CONST.ANIMATED_TRANSITION);


Or we can wrap the code inside setTimeout in InteractionManager.runAfterInteractionsor replace setTimeout with InteractionManager.runAfterInteractions

    focusWelcomeMessageInput() {
        this.focusTimeout = setTimeout(() => {
            InteractionManager.runAfterInteractions(() => {
                if (!this.welcomeMessageInputRef) {
                   return;
                }
                this.welcomeMessageInputRef.focus();
                // Below condition is needed for web, desktop and mweb only, for native cursor is set at end by default.
                if (this.welcomeMessageInputRef.value && this.welcomeMessageInputRef.setSelectionRange) {
                    const length = this.welcomeMessageInputRef.value.length;
                    this.welcomeMessageInputRef.setSelectionRange(length, length);
                }
            });
        }, CONST.ANIMATED_TRANSITION);
    }

Or we can refactor this screen from class to function and use useAutoFocusInput

@trjExpensify
Copy link
Contributor

@situchan can you review this proposal, please? Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Dec 15, 2023
Copy link

melvin-bot bot commented Dec 18, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Dec 18, 2023
@situchan
Copy link
Contributor

@ZhenjaHorbach are you able to reproduce constantly?

@melvin-bot melvin-bot bot removed the Overdue label Dec 18, 2023
@ZhenjaHorbach
Copy link
Contributor

@ZhenjaHorbach are you able to reproduce constantly?

this doesn't happen all the time but I was able to reproduce several times

@melvin-bot melvin-bot bot added the Overdue label Dec 20, 2023
Copy link

melvin-bot bot commented Dec 21, 2023

@trjExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@trjExpensify
Copy link
Contributor

@situchan what do you think about this proposal?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 21, 2023
Copy link

melvin-bot bot commented Dec 25, 2023

@trjExpensify @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Dec 25, 2023

@trjExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented Dec 25, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Dec 27, 2023

@trjExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented Dec 29, 2023

@trjExpensify, @situchan Still overdue 6 days?! Let's take care of this!

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

Copy link

melvin-bot bot commented Jan 1, 2024

@trjExpensify @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

Copy link

melvin-bot bot commented Jan 1, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jan 1, 2024

@trjExpensify @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

Copy link

melvin-bot bot commented Jan 1, 2024

@trjExpensify, @situchan 10 days overdue. I'm getting more depressed than Marvin.

@trjExpensify
Copy link
Contributor

@situchan can we get an update here, please?

@melvin-bot melvin-bot bot removed the Overdue label Jan 3, 2024
@situchan
Copy link
Contributor

situchan commented Jan 3, 2024

I am back from holidays. Catching up today

@melvin-bot melvin-bot bot added the Overdue label Jan 5, 2024
@trjExpensify
Copy link
Contributor

@situchan bump please, thanks!

@melvin-bot melvin-bot bot removed the Overdue label Jan 8, 2024
@situchan
Copy link
Contributor

situchan commented Jan 8, 2024

@ZhenjaHorbach please update proposal based on latest codebase. That component was fully refactored in #32139

Copy link

melvin-bot bot commented Jan 8, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jan 8, 2024

@trjExpensify @situchan this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

  • Decide whether any proposals currently meet our guidelines and can be approved as-is today
  • If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@melvin-bot melvin-bot bot added Internal Requires API changes or must be handled by Expensify staff and removed 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 Jan 8, 2024
Copy link

melvin-bot bot commented Jan 8, 2024

Current assignee @situchan is eligible for the Internal assigner, not assigning anyone new.

@ZhenjaHorbach
Copy link
Contributor

@ZhenjaHorbach please update proposal based on latest codebase. That component was fully refactored in #32139

I checked the new code )
And I think this bug is no longer relevant
I tried to reproduce
But it didn't work out for me
Since now instead of custom autofocus we now use useAutoFocusInput which we had no problems with

@melvin-bot melvin-bot bot added the Overdue label Jan 10, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

Copy link

melvin-bot bot commented Jan 11, 2024

@trjExpensify, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@situchan
Copy link
Contributor

We can close this for now.
I will bump to reopen when reproducible again

@melvin-bot melvin-bot bot removed the Overdue label Jan 11, 2024
@trjExpensify
Copy link
Contributor

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

5 participants