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] Add Explanation for Locked Category Toggle When Importing from QuickBooks #56181

Open
maddylewis opened this issue Jan 31, 2025 · 32 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Overdue

Comments

@maddylewis
Copy link
Contributor

maddylewis commented Jan 31, 2025

Description:

Currently, when categories are imported via QuickBooks Online (QBO), the toggle to disable categories is locked (:lock:). However, no explanation is provided for why this setting is locked.

For other locked settings, we display a message explaining the reason behind the lock. This issue aims to bring consistency by adding an explanation when the category toggle is locked due to QBO import settings.

context: https://expensify.slack.com/archives/C06ML6X0W9L/p1738342329040109

Problem:

  • The toggle to disable categories is locked when categories are imported from QBO.
  • Users may not understand why they cannot disable this setting.
  • Other locked settings provide explanations, but this one does not.

Expected Behavior:

When the toggle to disable categories is locked due to QBO import, an explanation should be displayed.

The message should inform users that the setting is locked because:

  • Categories are imported from QuickBooks Online.
  • A category is required for an expense to successfully export to QBO.
Image
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021885384455720840045
  • Upwork Job ID: 1885384455720840045
  • Last Price Increase: 2025-01-31
  • Automatic offers:
    • Krishna2323 | Contributor | 105976451
Issue OwnerCurrent Issue Owner: @Ollyws
@maddylewis maddylewis added Bug Something is broken. Auto assigns a BugZero manager. Improvement Item broken or needs improvement. labels Jan 31, 2025
Copy link

melvin-bot bot commented Jan 31, 2025

Triggered auto assignment to @CortneyOfstad (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.

@melvin-bot melvin-bot bot added the Daily KSv2 label Jan 31, 2025
@maddylewis maddylewis added the External Added to denote the issue can be worked on by a contributor label Jan 31, 2025
@melvin-bot melvin-bot bot changed the title Add Explanation for Locked Category Toggle When Importing from QuickBooks [$250] Add Explanation for Locked Category Toggle When Importing from QuickBooks Jan 31, 2025
Copy link

melvin-bot bot commented Jan 31, 2025

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 31, 2025
Copy link

melvin-bot bot commented Jan 31, 2025

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

@Krishna2323
Copy link
Contributor

Krishna2323 commented Jan 31, 2025

Proposal

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

Add Explanation for Locked Category Toggle When Importing from QuickBooks

What is the root cause of that problem?

Improvement

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

  • We need to update the translation:

needCategoryForExportToIntegration: 'Require a category on every expense in order to export to',

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore? (Optional)

Copy link
Contributor

⚠️ Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

@Krishna2323
Copy link
Contributor

@maddylewis, do we need to update the subtitle Require a category on every expense in order to export to or create another one?

@Faisalmajeed123
Copy link

@Krishna2323 do you know about the community where Expensify contributors can ask queries? I am struggling to setup Android app locally.

Copy link

melvin-bot bot commented Jan 31, 2025

📣 @Faisalmajeed123! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@Faisalmajeed123
Copy link

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01fe0fae6b041a092e

Copy link

melvin-bot bot commented Jan 31, 2025

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

Copy link

melvin-bot bot commented Jan 31, 2025

📣 @tahalmg20! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

Copy link
Contributor

⚠️ Proposal Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

@tahalmg20
Copy link


Proposal

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

When categories are imported from QuickBooks Online (QBO), the toggle to disable categories is locked. However, there is no explanation displayed to users about why this toggle is locked. This inconsistency can confuse users, as other locked settings in the app provide explanatory messages.


What is the root cause of that problem?

The root cause is the absence of a conditionally displayed explanatory message in the relevant component when the toggle is locked due to QBO import settings. While the locked state is enforced, no user-facing feedback is provided to clarify the restriction.


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

To address this issue, we should:

  1. Add a conditional explanation: Modify the ToggleSettingOptionRow component (used in QuickbooksCompanyCardExpenseAccountPage.tsx) to include an explanation property that displays a message when the toggle is locked.
  2. Update localization files: Add a new translation key for the explanation message. Example:
    {
        "workspace.qbo.toggleLockedExplanation": "This setting is locked because categories are imported from QuickBooks Online. A category is required for an expense to successfully export to QBO."
    }

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

To prevent regressions, the following test scenarios should be covered:

  1. Toggle Locked State:

    • Verify that the toggle is locked when categories are imported from QBO.
    • Confirm that the explanation is displayed when the toggle is locked.
  2. Toggle Unlocked State:

    • Verify that the toggle becomes interactive when QBO import settings are not applied.
  3. Localization Validation:

    • Test that the correct translation key is displayed in the appropriate language.
  4. Basic Error Handling:

    • Ensure the app does not crash when qboConfig data is missing or incomplete.

What alternative solutions did you explore? (Optional)

@tahalmg20
Copy link

Contributor details

Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01cc1c2cf7d777490a

Copy link

melvin-bot bot commented Jan 31, 2025

⚠️ Invalid email. Please make sure to create an Expensify account with this email first here.

@tahalmg20
Copy link

Contributor details

Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01cc1c2cf7d777490a

Copy link

melvin-bot bot commented Jan 31, 2025

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

@tahalmg20
Copy link

Contributor details

Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01cc1c2cf7d777490a

Copy link

melvin-bot bot commented Jan 31, 2025

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@truph01
Copy link
Contributor

truph01 commented Jan 31, 2025

Proposal

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

  • Add Explanation for Locked Category Toggle When Importing from QuickBooks

What is the root cause of that problem?

  • We already have an explanation, but it's unclear and confusing for users:

needCategoryForExportToIntegration: 'Require a category on every expense in order to export to',

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

  • We should update the text to:
A category is required to export an expense to

or

Category required when accounting is connected to

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore? (Optional)

  • It can be Categories are imported, and a category is required for every expense to successfully export to so that we can cover:

The message should inform users that the setting is locked because:

  • Categories are imported from QuickBooks Online.
  • A category is required for an expense to successfully export to QBO.

@ikevin127
Copy link
Contributor

Proposal

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

❌ The toggle to disable categories is locked when categories are imported from QBO and users might not understand why they cannot disable this setting, while other locked settings provide explanations, this one doesn't.

What is the root cause of that problem?

ℹ Given the issue's expected behaviour, the root cause of the problem comes from this line where the copy is not informative enough to cover a complete explanation such that the reason for locked toggle is clear to the user.

♻ Alternatively, if the issue is not about the already existing copy from under the switch toggle but rather about offering the user some visual feedback for when a toggle is locked - then there's no root cause, but a new feature request instead.

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

🤖 With the help of AI, taking in consideration the expected behaviour, specifically the fact that the message should inform users that the setting is locked because:

  • Categories are imported from QuickBooks Online.
  • A category is required for an expense to successfully export to QBO.

🗒 Landed on the following updated copy (to be validated with Copy team):

🇺‍🇸 Categories are imported and required for every expense exported to {integrationName}
🇪‍🇸 Las categorías se importan y son requeridas para cada gasto exportado a {integrationName}

🛠 Given the issue's expected behaviour and to propose a slightly different approach, I think there are 2 ways to handle the fix such that things will be clear for the user:

  1. (cc @Expensify/design for a take on this) Wrap the current Switch component with our Tooltip component which will allow us to pass down through props from here (in our case), whether to enable the tooltip displaying specific explanatory text, which we can target exactly for the locked switch case scenario, but not exclusive since the 2 new props (shouldShowTooltip and tooltipText) allowing us to alert the user on why the option is locked:

Note

Notes

  1. The Tooltip component has dynamic placement depending on screen height and Switch toggle position, but will only show up on hover, meaning only cursor based platforms (Web / Desktop).
  2. We're already using the Tooltip in places like WS Switcher to let users know what the button does:
Image

Back to the Switch / Tooltip implementation:

Top positioned Bottom positioned
Image Image
  1. If we don't want the Switch / Tooltip implementation, we can simply update the translations from this line to the one mentioned above which is more clear. This can be observed above under the Switch component in the first picture.

🟢 We can implement only (2.), or both (1.) and (2.), depending on what the issue's expected behaviour means when saying (to be determined):

When the toggle to disable categories is locked due to QBO import, an explanation should be displayed.

Here's a test branch including both (1.) and (2.) solution implementetion.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

No specific testing scenario required in for this solution since regardless of the solution choice, this is a simple copy update or tooltip display fix which don't require functional testing.

@Ollyws
Copy link
Contributor

Ollyws commented Feb 3, 2025

Seems like this is just a simple copy change, and as @Krishna2323 was first to suggest that let's go with their proposal. We will have to clarify exactly what copy is required though.
🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Feb 3, 2025

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

@Beamanator
Copy link
Contributor

I think it's possible that purely a copy change could fix the issue, but @ikevin127 did provide some interesting additional ideas that could help with this issue - @dannymcclain (since you 👍 'd the proposal) do you agree we only need a copy change for this issue, not the additional changes recommended in this proposal? #56181 (comment)

@dannymcclain
Copy link
Contributor

I believe the way we typically handle these is not with a tooltip but instead with the little description text underneath the toggle. So I think we should stick with that pattern if we can. @Expensify/design does that sound right to you?

@dubielzyk-expensify
Copy link
Contributor

Yeah, we only use the tooltip in rare scenarios. Description feels right to me

@shawnborton
Copy link
Contributor

Yup, that sounds correct to me as well.

@Beamanator
Copy link
Contributor

Giggidy, thanks so much for the quick responses y'all!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 3, 2025
Copy link

melvin-bot bot commented Feb 3, 2025

📣 @Krishna2323 🎉 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 📖

@Beamanator
Copy link
Contributor

Going with krishna's solution 👍

@Krishna2323
Copy link
Contributor

@maddylewis, could you please share the new description? In the PR I have added:

Categories are imported from ${connectionName}, and a category is required for expenses to successfully export to ${connectionName}.

Las categorías se importan desde ${connectionName} y se requiere una categoría para que los gastos se exporten correctamente a ${connectionName}.

Copy link

melvin-bot bot commented Feb 7, 2025

@Beamanator, @CortneyOfstad, @Ollyws, @Krishna2323 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Feb 7, 2025
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 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Overdue
Projects
Status: MEDIUM
Development

No branches or pull requests