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

[$250] QBO - "Reconciliation account" disappears after selecting it #52548

Open
2 of 8 tasks
IuliiaHerets opened this issue Nov 14, 2024 · 28 comments
Open
2 of 8 tasks

[$250] QBO - "Reconciliation account" disappears after selecting it #52548

IuliiaHerets opened this issue Nov 14, 2024 · 28 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. 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 Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Nov 14, 2024

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: 9.0.62-1
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #51359
Email or phone of affected tester (no customers):
Issue reported by: Applause Internal Team

Action Performed:

Preconditions:

  • Use a new Gmail account.
  • Expensify Card is enabled.
  • Bank account is added in Expensify Card page.
  • Workspace is connected to QBO.
  1. Open the app
  2. Navigate to Workspace settings - Accounting - Card reconciliation
  3. Enable "Continuous Reconciliation"
  4. Click on your added bank account

Expected Result:

"Reconciliation account" should still be visible.

Actual Result:

"Reconciliation account" disappears after selecting it.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6665023_1731579153679.QGOY0671.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021860984129964489529
  • Upwork Job ID: 1860984129964489529
  • Last Price Increase: 2024-11-25
  • Automatic offers:
    • DylanDylann | Reviewer | 105055923
    • daledah | Contributor | 105055925
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 14, 2024
Copy link

melvin-bot bot commented Nov 14, 2024

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@etCoderDysto
Copy link
Contributor

etCoderDysto commented Nov 14, 2024

Edited by proposal-police: This proposal was edited at 2024-11-14 12:38:30 UTC.

Proposal

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

QBO - "Reconciliation account" disappears after selecting it

What is the root cause of that problem?

When trying to add a bank account on step 4, the network request fails, and error message from onyx will be stored in cardSettings.errors, but we are not displaying the error message in CardReconciliationPage.

cardSettings.errors

const [cardSettings] = useOnyx(`${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`);

Onyx failure data

const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID}`,
value: {
paymentBankAccountID: currentSettlementBankAccountID,
isLoading: false,
errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'),

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

  1. We should use ErrorMessageRow to display the error message below
{cardSettings?.errors && <ErrorMessageRow errors={cardSettings?.errors} />}
  1. Create a utility function that clears error message errors field of ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS}${workspaceAccountID
  2. Pass the utility fucntion to onClose prop of ErrorMessageRow
  3. Change styles according to design

Optionally, in failure data we can pass settlementBankAccountID to paymentBankAccountID to prevent the failed bank account from disappearing form Card reconciliation page, and change the bank to any available previous bank account by reseting paymentBankAccountID value to currentSettlementBankAccountID in the utility function

What alternative solutions did you explore? (Optional)

Result
Screen.Recording.2024-11-14.at.3.36.48.in.the.afternoon.mp4
Note we will fix the style

@daledah
Copy link
Contributor

daledah commented Nov 15, 2024

Edited by proposal-police: This proposal was edited at 2024-11-15 09:37:35 UTC.

Proposal

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

Bug 1: "Reconciliation account" disappears after selecting it.
Bug 2: Error message is not displayed when API is not called successfully.

What is the root cause of that problem?

Bug 1

We still display the Continuous Reconciliation toggle even though users haven't set up settlement bank account

Bug 2: Optional:

  • We don't have logic to display the error:

{!!paymentBankAccountID && !!isContinuousReconciliationOn && (
<MenuItemWithTopDescription
style={styles.mt5}
title={bankAccountTitle}
description={translate('workspace.accounting.reconciliationAccount')}
shouldShowRightIcon
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_ACCOUNTING_RECONCILIATION_ACCOUNT_SETTINGS.getRoute(policyID, connection))}
/>
)}

when API UpdateCardSettlementAccount encountered error.

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

const configurationOptions = [

We only should push cardReconciliation to configurationOptions if workspaceAccountID && areExpensifyCardEnabled

If user go to Card reconciliation by deeplink I suggest displaying the not found page or display a locked toggle (also use this condition workspaceAccountID && areExpensifyCardEnabled)

Additional we still can update the error handle if needed

  • We can display the error message in:

                        <MenuItemWithTopDescription errorText={errorText} ... />

with errorText = Object.values(getLatestError(cardSettings?.errors)).at(0).

The error message should be displayed in the MenuItemWithTopDescription because the error is related to paymentBankAccountID field.

What alternative solutions did you explore? (Optional)

@trjExpensify
Copy link
Contributor

Another one for your eyes @mountiny @MariaHCD @madmax330 @nkuoch @allgandalf @DylanDylann

@trjExpensify trjExpensify moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 15, 2024
@allgandalf
Copy link
Contributor

allgandalf commented Nov 15, 2024

Think it has to do more with Accounting and less expensify cards (Implementation wise), but happy to take this one on!

@trjExpensify
Copy link
Contributor

Gotcha, continuousReconciliation is only a feature of the Expensify Card.

@DylanDylann
Copy link
Contributor

@trjExpensify I already worked on Card reconciliation account setting Page before (when implementing the Expensify Card). Let me take this issue

@melvin-bot melvin-bot bot added the Overdue label Nov 18, 2024
@DylanDylann
Copy link
Contributor

I agreed with @daledah about Bug 1, in case the user go to accounting page right after login successful we also need to display the bank account list

About the main bug on OP, @daledah @etCoderDysto could anyone help to explain why the API failed?

@DylanDylann
Copy link
Contributor

DylanDylann commented Nov 18, 2024

@mountiny @MariaHCD Tag you here because it relates to the Expensify Card. In this issue, we have 2 problems:

  1. In case the user goes to the Card reconciliation page right after login is successful we also need to display the bank account list. To do that, private_expensifyCardSettings_ needs to be available when the user goes to the Card reconciliation Page. Currently, It is only available after we call API OpenPolicyExpensifyCardsPage (going to Expensify Card Page)

  2. To set settlementBankAccountID we have two ways:

  • In ExpensifyCard, we use ConfigureExpensifyCardsForPolicy API and It works well
Screenshot 2024-11-18 at 12 01 58 Screenshot 2024-11-18 at 12 02 05
  • In the Card reconciliation page, we use UpdateCardSettlementAccount API and It is failed
Screenshot 2024-11-18 at 12 01 34 Screenshot 2024-11-18 at 12 01 41

TBH, I don't understand why UpdateCardSettlementAccount API is failed in this case and ConfigureExpensifyCardsForPolicy still be successful. Could you please help to take a look?

@trjExpensify
Copy link
Contributor

@trjExpensify I already worked on Card reconciliation account setting Page before (when implementing the Expensify Card). Let me take this issue

Assigned you to help progress!

@mountiny mountiny self-assigned this Nov 18, 2024
@mountiny
Copy link
Contributor

Maria is out assigning. I am also working partial day @nkuoch in case you could take a look earlier

@garrettmknight garrettmknight added the Internal Requires API changes or must be handled by Expensify staff label Nov 19, 2024
@mountiny
Copy link
Contributor

@DylanDylann What bank accounts did you use to try it?

Did you provision the policy first and then you tried switching the bank account? You will have to use a different bank account for that and there is only one testing bank account you have access to right? So did you try switching to different bank account?

@DylanDylann
Copy link
Contributor

DylanDylann commented Nov 20, 2024

Ahh thanks for pointing out that. I only can access to one mock bank account

@DylanDylann
Copy link
Contributor

@mountiny You mean that if we haven't set up a settlement bank account, we shouldn't have access to Select Reconciliation account page.

@mountiny
Copy link
Contributor

@DylanDylann Yeah, I think if you do not have bank account set up, there is nothing to choose there. But also you cannot have expensify cards without settlement bank account

@DylanDylann
Copy link
Contributor

@mountiny I think we should disable and lock the toggle if users haven't setup the settle bank account like this. Wdyt?

Screenshot 2024-11-22 at 16 25 02

@daledah
Copy link
Contributor

daledah commented Nov 22, 2024

Updated proposal

@mountiny
Copy link
Contributor

Discussing internally cc @trjExpensify @JmillsExpensify

Maybe we should lock it or even hide the Expensify card reconciliation option if you do not have exepnsify cards. Or show some graphics CTA for the user in RHP to inform them about ECard option

@trjExpensify
Copy link
Contributor

hide the Expensify card reconciliation option if you do not have exepnsify cards.

I thought that was the logic? 😕

@JmillsExpensify
Copy link

Same, I thought we hid this when it didn't apply.

@mountiny
Copy link
Contributor

@DylanDylann there is the answer, lets hide the button and add lock the toggle in case of a deeplink to that RHP page if cards are not provisioned that is no workspaceAccountID + areExpensifyCardEnabled

@daledah
Copy link
Contributor

daledah commented Nov 22, 2024

@DylanDylann
Copy link
Contributor

@mountiny Should we open this issue for external contributors? If yes, @daledah's proposal looks fine

@mountiny mountiny added External Added to denote the issue can be worked on by a contributor and removed Internal Requires API changes or must be handled by Expensify staff labels Nov 25, 2024
@melvin-bot melvin-bot bot changed the title QBO - "Reconciliation account" disappears after selecting it [$250] QBO - "Reconciliation account" disappears after selecting it Nov 25, 2024
Copy link

melvin-bot bot commented Nov 25, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 25, 2024
Copy link

melvin-bot bot commented Nov 25, 2024

Current assignee @DylanDylann is eligible for the External assigner, not assigning anyone new.

Copy link

melvin-bot bot commented Nov 25, 2024

📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Nov 25, 2024

📣 @daledah 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

@daledah
Copy link
Contributor

daledah commented Nov 25, 2024

@DylanDylann PR is ready.

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. 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 Reviewing Has a PR in review Weekly KSv2
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

9 participants