search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.58k stars 2.92k forks source link

Workspace - WS can't be deleted after Expensify cards was disabled #50684

Closed IuliiaHerets closed 1 day ago

IuliiaHerets commented 1 month ago

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: v9.0.48-0 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

Precondition: new private domain account. Workspace with added expensify card (physical or virtual).

  1. Go to Workspace > Expensify cards
  2. Click on added card > Deactivate card
  3. Go to Workspace > More features
  4. Disable Expensify cards
  5. Delete workspace

Expected Result:

Workspace deleted.

Actual Result:

An error occur during deletion. It says that my ws has expensify card when actually it hasn't. Error Message: Your company has enabled Expensify Cards. Please reach out to [concierge@expensify.com](mailto:concierge@expensify.com) to have the cards removed. Until then, the Workspace cannot be deleted.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/81077181-f12a-4500-be68-f768e6ae6953

View all open jobs on GitHub

melvin-bot[bot] commented 1 month ago

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

IuliiaHerets commented 1 month ago

We think that this bug might be related to #wave-collect - Release 2

IuliiaHerets commented 1 month ago

@joekaufmanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

joekaufmanexpensify commented 1 month ago

Not overdue. Will triage tomorrow

joekaufmanexpensify commented 1 month ago

Discussing here.

mountiny commented 1 month ago

We would like the modal to looks like this image

joekaufmanexpensify commented 1 month ago

@mountiny the more I think about it, I kinda wonder if this one is worth prioritizing right now. Deleting a workspace is in general such a rare action to take. Given this works now the same way it does in OldDot (which is totally fine), I wonder if it's worth just leaving as is for now until we experience a problem from it?

As this isn't really a bug now, and I wonder if it's worth investing resources in changing this design before really anyone is using this feature. What do you think?

mountiny commented 1 month ago

@joekaufmanexpensify I think its still just good to knock it out now, I think its a rare case, but when it happens its annoying, especially given its much more easier now to run into this case than then we required verified domain

mountiny commented 1 month ago

But I think we can make it weekly and maybe @nkuoch would be up for helping with this one

joekaufmanexpensify commented 1 month ago

Fair enough, sounds good. In addition to the deleting workspace case though, there's one other case we should align on. According to the design doc, the user should not be able to toggle off Expensify Card in More features when the workspace is provisioned at all. But in the above video, toggling this off is possible after terminating the workspace's only assigned card.

I'm thinking we intentionally allowed this when the workspace is provisioned (but there are no active cards). Is that right, @mountiny?

melvin-bot[bot] commented 1 month ago

@nkuoch @joekaufmanexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

joekaufmanexpensify commented 4 weeks ago

PRs in review!

nkuoch commented 4 weeks ago

Seems like toggling off doesn't actually do anything backend side? https://github.com/Expensify/Web-Expensify/pull/28072/files#diff-fd75745654fb72974da44d75c3835ee837a6e644a05a6ffaae48556924242946R877

cc @mountiny do you know if an issue currently exists for that?

mountiny commented 4 weeks ago

I am not sure how that PR is related. This is the method used to toggle the expensify card feature https://github.com/Expensify/Web-Expensify/blob/fe926b0bc18fb7496fbc31280f092c5d5240ba7c/lib/PolicyAPI.php#L3395-L3411 and it changes the flag, but I dont think it does much more right now

nkuoch commented 3 weeks ago

I'm thinking we intentionally allowed this when the workspace is provisioned (but there are no active cards). Is that right, @mountiny?

And should we deprovision it when toggling it off when there is no active card? cc @mountiny ?

mountiny commented 3 weeks ago

And should we deprovision it when toggling it off when there is no active card? cc @mountiny ?

Asked here

joekaufmanexpensify commented 3 weeks ago

Still ongoing

joekaufmanexpensify commented 2 weeks ago

Same

joekaufmanexpensify commented 1 week ago

PR still in review

joekaufmanexpensify commented 1 day ago

@nkuoch I see PR above was deployed. Are we good here now?

mountiny commented 1 day ago

Asked for a retest here

kavimuru commented 1 day ago

Not reproducible

https://github.com/user-attachments/assets/30614fb5-f534-49f4-b95d-8c8b94d27d02