clients
clients copied to clipboard
[EC-14] Refactor vault filter
Type of change
- [ ] Bug fix
- [x] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
There are two main objectives with this vault filter refactor:
- Make the vault filter component more dynamic by passing a list of properties that the vault filter can iterate and turn into different filter sections.
- Change the vault filter model to use nodes instead of just IDs. This will allow us to introduce and navigate collection rows in the cipher list easier since the selected collection will contain the entire sub-tree inside the model.
Most of these changes were kept in the web vault project (not in shared code). This was intended to speed up the delivery of this feature without messing with the underlying functionality of the filter in the desktop and browser.
Edit:
There are also UI changes being made in this refactor as outlined in [EC-14](https://bitwarden.atlassian.net/browse/EC-14). I am changing the order of the vault filters, making the All Items and Collections headers selectable, and setting up the vault for the introduction of collection rows.
Code changes
- apps/web/src/app/vault/vault-filter/shared/models/vault-filter.model.ts: The new vault filter model that uses nodes instead of just IDs.
- apps/web/src/app/vault/vault-filter/vault-filter.component.ts: Vault filter now describes how the entire filter should look. Disconnected from shared code to maintain desktop and browser
- apps/web/src/app/organizations/vault/vault-filter/vault-filter.component.ts: Org vault filter describes how org filter should look
- libs/angular/src/vault/vault-filter/models/vault-filter-section.ts: Consolidated the different filter sections into a single component.
- apps/web/src/app/vault/vault-filter/vault-filter.service.ts: Rewrote vault filter service to hold collection and filter observables. Also now uses angular DI. Also Deprecated the old vault filter service for future refactor.
Screenshots
New vault filter architecture:

Before (End User Vault):

After (End User Vault):

Before (Org Admin Vault):

After (Org Admin Vault):

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
@jlf0dev given that this just refactors (no actual Org Admin Refresh changes), can we target master instead? That all pods see the changes sooner, and you don't have to resolve conflicts (which would probably be difficult for this changeset).
@jlf0dev given that this just refactors (no actual Org Admin Refresh changes), can we target
masterinstead? That all pods see the changes sooner, and you don't have to resolve conflicts (which would probably be difficult for this changeset).
I would love to put this into master sooner rather than later, but with the UI changes I don't think that's going to be possible.
The Chromatic tests are failing because Storybook isn't building. Run npm run build-storybook locally, you'll get a bunch of error messages due to absolute imports. Change these to relative imports and it should work again.
#3636 will fix this vscode setting which keeps suggesting absolute paths when they should be relative.
Looked at the injector code again and it made more sense this time around.
EDIT: however, why can't we just use the component selector in the template like we do now?
My idea with passing the component in this way is that I can keep it completely dynamic. The vault-filter.component is responsible for providing a component to show if the options button is clicked. This component could be anything as long as it has an @Inject where we can plug in the filter object.
My idea with passing the component in this way is that I can keep it completely dynamic. The vault-filter.component is responsible for providing a component to show if the options button is clicked. This component could be anything as long as it has an @Inject where we can plug in the filter object.
Oh yes, it's dynamic - sorry, you already said that ;) Sounds good to me.
Please also fix linting after all changes are made.