[HOLD for payment 2021-11-04] [n6-hold bonus with payment reminder] Report a bug, doesn't work as expected.

[Santhosh-Sellavel]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to Settings -> About
2. Click on Report a bug
   3.Concierge chat will be opened now.
3. Now repeat steps 1 & 2.
4. Concierge is not open instead navigate back to the Settings page

For new users it's bit more iritating. They would never know conciegre help. Because the new user will have only concierge active by default.

Expected Result:

I believe Report a bug should open the Concierge chat every time if that's the desired action.

Actual Result:

It's not opened instead navigated back to the Settings page.

Simulator.Screen.Recording.-.iPhone.12.-.2021-08-11.at.18.08.56.mp4

Workaround:

Yes, the user could manually close the settings page.

Suggestions

IMO Just opening the concierge does not look like the user can report bugs in concierge chat. We can show some additional messages like you can start reporting by just adding certain placeholder text in chat input. Ex What can I help you with?, Tell us your problem, etc.

Platform:

All of the following
Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @mateocole (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[Santhosh-Sellavel]

Proposal

The below lines invoke the action to open the Concierge chat.

App/src/pages/settings/AboutPage.js

Lines 67 to 69 in 83dd58e

	action: () => {
	fetchOrCreateChatReport([session.email, CONST.EMAIL.CONCIERGE], true);
	},

Simple solution:

We can straightaway call Navigation.dismissModal() in action block to dismiss the view.
Or we could add an optional callback in fetchOrCreateChatReport.
And once the chat is fetched/created we can use the callback to dismiss the settings view.

[mateocole]

Alright, to clean this up a little. It sounds like there might be two things being asked here.

1. Reporting a bug is taking you back to the settings and not to concierge
2. Concierge might not be the best route for reporting a bug

For this specific issue, can this be reorganized to fit scenario 1 as that would be the specific bug?

[Santhosh-Sellavel]

Reporting a bug is taking you back to the settings and not to concierge

Yes, I understood this.

Concierge might not be the best route for reporting a bug
For this specific issue, can this be reorganized to fit scenario 1 as that would be the specific bug?

What do you mean here, I'm not following @mateocole

[parasharrajat]

First-time navigation is working but the second time there is an issue. Not going back does not seem the root cause of it. Did you find why it is not working for the second time?

[Santhosh-Sellavel]

First-time navigation is working but the second time there is an issue. Not going back does not seem the root cause of it. Did you find why it is not working for the second time?

Yes, of course, We don't allow to navigate to reportId which is already active, that is Concierge.

@parasharrajat
Also If you are a new user you won't even get navigated for the first time.

[mateocole]

What do you mean here, I'm not following @mateocole

The intended action would be to direct you to concierge to report the bug. This would not be relative to this issue as it would be a separate proposal as that would be changing the intended action, if I am understanding the suggestion correctly.

Said differently, I think the focus of the this issue should be to focus on fixing the bug that does not redirect back to concierge.

[Santhosh-Sellavel]

If we focus on navigating to Concierge for reporting bugs, then yes, @mateocole my suggestion could be a separate proposal.

[parasharrajat]

Yes, But you open the about page again. You are on /settings/about page. and thus navigating to the Report route should work.

for example, you can navigate to settings/payments page and then settings/about-page , then back to settings/payments.

Yes, of course, We don't allow to navigate to reportId which is already active, that is Concierge.

Could you please link me to where are we doing that?

This seems like more of an issue with Navigation.navigate doing Navigation.dismissModal() sounds like a hack to fix this as we are already navigating to the Report route.

calling both Navigation.dismissModal() and Navigation.navigate shows that there is some issue with navigation implementation.

[Santhosh-Sellavel]

Here,

Calling Navigation.dismissModal() was a simple solution suggestion as I said in the proposal, yeah I agree with you completely it sounds like a hack, but it works without any issues!

Calling both Navigation.dismissModal() and Navigation.navigate shows that there is some issue with navigation implementation.

There is no issue with implementation, cases are handled well.

[Santhosh-Sellavel]

@parasharrajat do you get it now?

Here we can make some kind of exception for concierge, we have activeReportID could use that to find whether it's Concierge using states or we have to add something in the state to track currently active login something like that.

[Santhosh-Sellavel]

dismissingModals will happen anyway abruptly, we could just do it proactively, there's nothing wrong with that!
But that's not the only solution here!

[parasharrajat]

dismissingModals will happen anyway abruptly, we could just do it proactively, there's nothing wrong with that!

Definitely navigating two times is an issue.

Navigation.dismissModal() was a simple solution suggestion as I said in the proposal, yeah I agree with you completely it sounds like a hack, but it works without any issues!

Mostly navigate should handle this. Navigate method is adjusted many times to make it work as it does now. You can look into the history of it. So if we found an issue with it my suggestion is we improve it.

There is no issue with implementation, cases are handled well.

First time it work as it should and we can navigate to concierge report but second time it does not. How can you say There is no issue with implementation, cases are handled well.

[Santhosh-Sellavel]

Mostly navigate should handle this. Navigate method is adjusted many times to make it work as it does now. You can look into the history of it. So if we found an issue with it my suggestion is we improve it.

Sure @parasharrajat , as i said earlierdismissModal It was not the only solution.

Thanks for inputs, I'll look through the history.

What do you think about this
#4545 (comment)

Please post your thoughts this it will be help ful. Thanks

[parasharrajat]

Here we can make some kind of exception for concierge, we have activeReportID could use that to find whether it's Concierge using states or we have to add something in the state to track currently active login something like that.

Sounds very complicated. Not sure how feasible it would be.

[mateocole]

@Santhosh-Sellavel any update on solutoins?

[Santhosh-Sellavel]

@mateocole What's your thought on this solution
#4545 (comment)

[parasharrajat]

@mateocole Please review.

Proposal

1. As I explained above that there is an issue with our core Navigate method.
2. We do block refreshing the page when we navigate to the existing report. It helps us remove the rerender of the many heavy components.
3. But we are not handling the navigation correctly while doing it.

To fix it we have to think about the modals and other app architecture. We should pop the top Stack router and then close the modal.
TO do this. we can update the code

App/src/libs/Navigation/CustomActions.js

Line 27 in 083405a

navigationRef.current.dispatch(StackActions.pop());

to

   if (activeReportID === params.reportID) {
            if (state.type !== 'drawer') {
                navigationRef.current.dispatch(() => {
                    // If there are multiple routes then we can pop back to the first route
                    if (state.routes.length > 1) {
                        return StackActions.popToTop();
                    }

                    // Otherwise, we are already on the last page of a modal so just do nothing here 
                    // as goBack() will navigate us
                    // back to the screen we were on before we opened the modal.
                    return StackActions.pop(0);
                });
                Navigation.goBack();
            }
            return DrawerActions.closeDrawer();
        }

It fixes the core issue throughout the app

Also, this action is doing two things that it should not and we should break this into two actions. I will refactor this CustomAction to two separate actions to correctly reflects what's happening here.

[Santhosh-Sellavel]

@parasharrajat Does this solution work mate?

[Santhosh-Sellavel]

@MitchExpensify or @AndrewGable

As posted in #expensify-open-source here

This change is in effect as of yesterday and the bonus for independently reporting an issue is only valid from yesterday forward. If there are existing, open, GH issues that you believe you should be compensated for reporting a bug, please comment on the issue.

I believe I should be compensated for reporting this issue. Thanks!

[MitchExpensify]

Thanks for the catch @Santhosh-Sellavel ! Offer sent so I can pay you for the reporting of the issue

[kadiealexander]

Hi @parasharrajat, I've placed this issue on hold as per this update, as we are prioritising issues related to a feature release scheduled for 9/24. As an apology for the delay, we will add a $100 bonus as a thank you for waiting.

[MitchExpensify]

Not overdue

[MitchExpensify]

On hold

[kadiealexander]

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

[MitchExpensify]

on hold

[MitchExpensify]

On Hold

[parasharrajat]

@MitchExpensify Should not be on hold any more. N6 is over.

[MitchExpensify]

thats correct @parasharrajat, taking off hold!

[MitchExpensify]

Not overdue, awaiting payment

[mallenexpensify]

Paid @Santhosh-Sellavel $250 for reporting
Paid @parasharrajat, inc. $250 for n6-hold
Thanks gents, closing