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] [HOLD for payment 2024-06-11] [Wave Collect] [Xero] [Imported Features and Taxes] Make Categories, Tags, and Taxes read-only when the user is connected to Xero #39911

Closed
lakchote opened this issue Apr 9, 2024 · 54 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2

Comments

@lakchote
Copy link
Contributor

lakchote commented Apr 9, 2024

As noted here for QBO, we need to prevent the user from taking actions when the user is connected to Xero.

When the user connects to Xero:

  • optimistically clear all existing categories, tags and taxes the user might have had on the policy, when they add the Xero connection
  • prevent the user from turning off categories, tags and taxes features
  • prevent the user from deleting categories, tags and taxes imported from Xero
  • prevent the user from adding new categories, tags and taxes

This will be worked on by SWM engineers, see Slack discussion here.

Until the Xero authorization flow (NewDot issue) is done (which entails NewDot, Web-E and IS changes), we won't be able to work on this yet.

Issue OwnerCurrent Issue Owner: @
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01bf513d396cfd4d35
  • Upwork Job ID: 1800169199603790711
  • Last Price Increase: 2024-06-10
@lakchote lakchote added Weekly KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 9, 2024
@lakchote lakchote self-assigned this Apr 9, 2024
@lakchote lakchote moved this to Release 1: Spring 2024 (May) in [#whatsnext] #wave-collect Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 9, 2024
@lakchote lakchote added NewFeature Something to build that is a new item. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Apr 9, 2024
Copy link

melvin-bot bot commented Apr 9, 2024

Current assignee @joekaufmanexpensify is eligible for the NewFeature assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Apr 9, 2024
@joekaufmanexpensify
Copy link
Contributor

@lakchote should I be finding someone from SWM to do this, or just waiting for someone who already committed to do it to pick it up?

@lakchote lakchote changed the title [Wave Collect] [Xero] [Imported Features and Taxes] Make Categories, Tags, and Taxes read-only when the user is connected to Xero [HOLD #39725] [Wave Collect] [Xero] [Imported Features and Taxes] Make Categories, Tags, and Taxes read-only when the user is connected to Xero Apr 15, 2024
@lakchote
Copy link
Contributor Author

@lakchote should I be finding someone from SWM to do this, or just waiting for someone who already committed to do it to pick it up?

I've asked here, if SWM engineers can pick this up!

@SzymczakJ
Copy link
Contributor

Hey! I’m Jakub Szymczak from Software Mansion, an expert agency, and I’d like to work on this issue!

@trjExpensify trjExpensify moved this from Release 1: Spring 2024 (May) to Release 1.5: XeroCon 2024 (June 12th) in [#whatsnext] #wave-collect Apr 16, 2024
@joekaufmanexpensify
Copy link
Contributor

Great. TY! @SzymczakJ is there an eta for a PR here?

@SzymczakJ
Copy link
Contributor

@joekaufmanexpensify there should be the same task for QBO integration that would solve this issue(they also connect to accounting software and should make categories etc. read-only), but I cannot find it, so I'm not sure if it's already worked on by them. Also if nobody is taking care of it I do it 😃 cc @mountiny

@joekaufmanexpensify
Copy link
Contributor

@SzymczakJ, are you referring to this issue? If so, do you mean that the QBO issue will solve this for Xero too, and we can consolidate both into that issue?

@SzymczakJ
Copy link
Contributor

Sorry for late response, I was OOO. The issue you mentioned isn't solving this issue completely, it only disables switches for taxes/categories but the user is still able to edit categories etc.(as shown on video).

swtichblocked.mov

Previously I thought that making categories/taxes read-only was also the case for QBO and that's why I said it will be solved with QBO but maybe that isn't the case. Anyways the issue you mention isn't solving our issue completely and I don't see any open QBO issue that would solve this, so I could take care of that. cc @lakchote

Also is it only a Xero thing that the categories should be read-only and in case of QBO the user should be able to edit it freely?

@lakchote
Copy link
Contributor Author

lakchote commented May 6, 2024

Previously I thought that making categories/taxes read-only was also the case for QBO and that's why I said it will be solved with QBO but maybe that isn't the case. Anyways the issue you mention isn't solving our issue completely and I don't see any open QBO issue that would solve this, so I could take care of that. cc @lakchote

Also is it only a Xero thing that the categories should be read-only and in case of QBO the user should be able to edit it freely?

It shouldn't be a Xero thing only; from my understanding when you have an accounting integration connected it is the source of truth and the categories should come from this source only. So, it should be read only for both QBO and Xero from my understanding.

Could we have your opinions @trjExpensify @zanyrenney please?

@SzymczakJ
Copy link
Contributor

SzymczakJ commented May 6, 2024

Another question for design/product: How read-only state should be displayed? I have some idea but I want to make sure.
This is how edit category flow looks like

readwriteflow.mov

We have few options of editing:
a) enable/disable category with a toggle click
b) change the name of category with clicking on category name
c) delete category by clicking three dots
d) select multiple categories and disable/delete them

My idea would look like this:
We are disabling enable/disable toggle( a) option), making category name MenuItem non-interactive, so only the name is displayed ( b) option), deleting three dots on CategorySettingsPage ( c) option) and finally disabling select multiple options on category list on WorkspaceCategoriesPage ( d) option).

Screen.Recording.2024-05-06.at.15.45.39.mov

WDYT @zanyrenney @trjExpensify

@SzymczakJ
Copy link
Contributor

another question: the user has ability to enable/disable import of taxes,categories, etc.. We should only make taxes(and similarly categories, etc.) read-only when the import taxes toggle is on, right?

@trjExpensify
Copy link
Contributor

Hey - catching up here after being OoO yesterday. To be sure I'm following the question, we're trying to decide what you shouldn't be able to do with imported coding values (i.e categories, tags, tax rates etc) when connected to an accounting solution. Is that right?

If so, then the answer is you shouldn't be able to Add new values manually, and you shouldn't be able to Delete imported values. This was missed with the initial QBO implementation PRs, but I created an issue for it and a PR linked to it has merged awaiting a deploy: #41274

@SzymczakJ
Copy link
Contributor

SzymczakJ commented May 7, 2024

I've looked into this and found couple things that didn't get solved.

  1. You can still add/delete tags when being connected to accounting integration
Screenshot 2024-05-07 at 14 56 40 2. You can still add/ delete taxes when being connected to accounting integration Screenshot 2024-05-07 at 14 59 06 I could fix that issues in this task.

Also I have one important question. Should taxes, etc. be read-only when we are connected to accounting integration(with or without clicking import) or when we are connected to accounting integration AND clicking on import taxes, etc. I ask because sometimes we base on syncTax property and sometimes we base on isThereAnyAccountingConnection property and I got confused 😃
@trjExpensify @lakchote

@trjExpensify
Copy link
Contributor

  1. You can still add/delete tags when being connected to accounting integration
  2. You can still add/ delete taxes when being connected to accounting integration

Did you test on dev/main, yeah? If so, @hayata-suenaga what did your PR for #41274 implement?

Should taxes, etc. be read-only when we are connected to accounting integration(with or without clicking import) or when we are connected to accounting integration AND clicking on import taxes, etc

Can you rephrase the question, I'm not sure I'm following you.

@trjExpensify
Copy link
Contributor

All good, thanks! Assigning @hungvu193 to review.

@joekaufmanexpensify
Copy link
Contributor

PRs are merged but not yet deployed

@joekaufmanexpensify
Copy link
Contributor

PR on staging

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jun 4, 2024
@melvin-bot melvin-bot bot changed the title [Wave Collect] [Xero] [Imported Features and Taxes] Make Categories, Tags, and Taxes read-only when the user is connected to Xero [HOLD for payment 2024-06-11] [Wave Collect] [Xero] [Imported Features and Taxes] Make Categories, Tags, and Taxes read-only when the user is connected to Xero Jun 4, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jun 4, 2024
Copy link

melvin-bot bot commented Jun 4, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jun 4, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.78-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-06-11. 🎊

For reference, here are some details about the assignees on this issue:

  • @hungvu193 requires payment (Needs manual offer from BZ)
  • @SzymczakJ does not require payment (Contractor)

Copy link

melvin-bot bot commented Jun 4, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@hungvu193] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@joekaufmanexpensify
Copy link
Contributor

We already have a centralized testing script for this project, so IDT we need an ad-hoc regression test here.

@joekaufmanexpensify
Copy link
Contributor

@SzymczakJ is with SWM, so only payment needed here is $250 to @hungvu193 for C+ review via Upwork.

@joekaufmanexpensify joekaufmanexpensify added the External Added to denote the issue can be worked on by a contributor label Jun 10, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-06-11] [Wave Collect] [Xero] [Imported Features and Taxes] Make Categories, Tags, and Taxes read-only when the user is connected to Xero [$250] [HOLD for payment 2024-06-11] [Wave Collect] [Xero] [Imported Features and Taxes] Make Categories, Tags, and Taxes read-only when the user is connected to Xero Jun 10, 2024
Copy link

melvin-bot bot commented Jun 10, 2024

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

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

melvin-bot bot commented Jun 10, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 10, 2024
@joekaufmanexpensify joekaufmanexpensify added Weekly KSv2 and removed Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Jun 10, 2024
@hungvu193
Copy link
Contributor

Hey @joekaufmanexpensify , I got paid already so no payment needed here 😄

@joekaufmanexpensify
Copy link
Contributor

Ah, okay, sounds good! Just withdrew the upwork offer and closed the upwork job.

@joekaufmanexpensify
Copy link
Contributor

In that case, we should be all set here then. Thanks everyone!

@github-project-automation github-project-automation bot moved this from Release 1.5: XeroCon 2024 (June 12th) to Done in [#whatsnext] #wave-collect Jun 10, 2024
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 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

6 participants