Migrate existing organizations state from StateService → StateProvider

[addisonbeck]

Type of change

- [ ] Bug fix
- [x] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

This PR migrates OrganizationService over to StateProvider.

‼️ Attention non Admin Console teams:
The changes to your files here are a result of making two methods in OrganizationService async. In most cases, this
just involves adding an await. In a few select cases this involves swapping to an observable that is exposed from
OrganizationService that accomplishes the same need. In most cases all of these changes are small.
All of these changes are on the last commit: 5a114f6

Before you submit

• Please add unit tests where it makes sense to do so (encouraged but not required)
• If this change requires a documentation update - notify the documentation team
• If this change has particular deployment requirements - notify the DevOps team
• Ensure that all UI additions follow WCAG AA requirements

References

• AC-2009: Transition OrganizationService to use StateProvider
• Add state definitions for Admin Console services #7776
• Wire up key definitions for OrganizationService #7781
• [PM-6136] Properly set up StateProvider dependency for OrganizationService #7841
• Migrate provider service to state provider #8173

[codecov]

Codecov Report

Attention: Patch coverage is 50.40650% with 61 lines in your changes are missing coverage. Please review.

Project coverage is 26.20%. Comparing base (ca86288) to head (d2f9644).

Files	Patch %	Lines
...rets-manager/projects/project/project.component.ts	0.00%	7 Missing ⚠️
...ganizations/settings/two-factor-setup.component.ts	0.00%	6 Missing ⚠️
...c/app/secrets-manager/shared/new-menu.component.ts	0.00%	6 Missing ⚠️
...sole/services/organization/organization.service.ts	80.00%	3 Missing and 2 partials ⚠️
.../secrets-manager/shared/org-suspended.component.ts	0.00%	3 Missing ⚠️
libs/importer/src/components/import.component.ts	0.00%	3 Missing ⚠️
apps/browser/src/background/main.background.ts	0.00%	2 Missing ⚠️
apps/desktop/src/app/app.component.ts	0.00%	2 Missing ⚠️
.../organizations/reporting/reports-home.component.ts	0.00%	2 Missing ⚠️
apps/web/src/app/app.component.ts	0.00%	2 Missing ⚠️
... and 22 more

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7895   +/-   ##
=======================================
  Coverage   26.20%   26.20%           
=======================================
  Files        2280     2280           
  Lines       66727    66735    +8     
  Branches    12549    12550    +1     
=======================================
+ Hits        17483    17489    +6     
- Misses      47881    47883    +2     
  Partials     1363     1363           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bitwarden-bot]

Checkmarx One – Scan Summary & Details – 25ceb7aa-26da-48db-8216-4e6673958871

No New Or Fixed Issues Found

[justindbaur]

I understand the desire for smaller PR's but I don't like that this will cause there to be 2 places the data is saved. The migration will delete it from the state service location but then if someone uses the application it will get saved there again. To finish this off you'd need another migration just deleting the data.

[addisonbeck]

@justindbaur that makes sense. It is important to me to make incremental steps here for several reasons. Is there any good way for me to "queue" up a migration so that the migration can be written and tested without being implemented yet? I had thought of just removing it from the migration list & renaming the file without a number. Then applying the migration would just involve numbering the migration and adding it to the list. The still seems a little complicated though, so I'm wondering if you've got any better ideas?

[justindbaur]

@addisonbeck There is no way to queue up a migration, moving the backing location of your data is kind of an all or nothing move. If you're trying to keep things small you can delete some of the type assertions in here: https://github.com/bitwarden/clients/pull/7895/files#diff-397e257f75aaf8565e7025b97bbf6d6c0830fb174aceaa38522097a74422f2dcR54-R103

If the properties aren't referenced in the migration code they don't really contribute, what's is most important are the types for the account object shape leading up to your data. Then your data can be represented with an unknown.

Then, if you use the same observable names that the state service ones use you can switch over their backing data right now. Then you can have another 1 or 2 PR's that rename those observables to the new name if you want and one for deleting the state service methods and account property.

With those changes I think this PR would actually be slightly smaller.

[addisonbeck]

Renaming the exposed methods & observables separately is a good idea. I'll go that route, thanks!

[addisonbeck]

@justindbaur @eliykat This ended up getting bigger since many of OrganizationService's methods were made async. Would y'all mind giving it a review before I open it up to teams to review the changes to OrganizationService callers?

[eliykat]

Looking good overall!

[coltonhurst]

SM files look good