[AC-1713] [Flexible collections] Add feature flags to clients #6486

eliykat · 2023-10-03T03:44:54Z

Type of change

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

Objective

Add feature flags for the client-side changes in Flexible Collections.

The goal is to allow an early merge of the feature branch into master. This means that additional changes can branch from master and have access to our WIP, however they'll have to be flagged where appropriate.

In case it helps: here's the diff comparing this branch to master, which is useful to review all changes on the feature branch with the flags from this PR, to make sure nothing's been missed. master...flexible-collections/add-feature-flags

Code changes

This adds the following feature flags:

BulkCollectionAccess- shows/hides the button which opens the bulk access modal. I expect we can enable this before Flexible Collections is released, so this is flagged separately to keep our options open.
FlexibleCollections - shows/hides
- collection management settings in the org account component
- Can Manage permission in the access selector.

Note that the access selector is largely a presentational component, however I still thought it better that the feature flag behaviour is encapsulated within this component rather than passing an identical value from the 6 different locations that it's used. Also note feature flags are short-lived, so I think this is a temporary change that doesn't really affect its presentational status.

Screenshots

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

bitwarden-bot · 2023-10-03T04:46:22Z

Checkmarx One – Scan Summary & Details – cc68e700-628b-4ec9-8a5b-18390fc19471

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	Use_Of_Hardcoded_Password	/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts: 94	Attack Vector
	Use_of_Broken_or_Risky_Cryptographic_Algorithm	/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts: 126	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	Client_Password_In_Comment	/apps/browser/src/autofill/services/autofill.service.ts: 331
	Use_Of_Hardcoded_Password	/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts: 94
	Use_of_Broken_or_Risky_Cryptographic_Algorithm	/apps/browser/src/autofill/browser/main-context-menu-handler.spec.ts: 125

…/add-feature-flags

shane-melton

Looks pretty solid to me, nice one! The biggest thing I saw was a few missing $ for the new observable properties.

apps/web/src/app/vault/individual-vault/vault.component.ts

apps/web/src/app/vault/org-vault/vault.component.ts

...p/admin-console/organizations/shared/components/access-selector/access-selector.component.ts

…to flexible-collections/add-feature-flags

eliykat · 2023-10-11T04:02:17Z

I realised that Storybook couldn't resolve the ConfigService dependency, so it seemed easier to go the other route and return it to be purely presentational, passing in a boolean to enable flexible collections or not.

* [AC-1117] Add manage permission (#5910) * Add 'manage' option to collection access permissions * Add 'manage' to collection permissions * remove service accidentally committed from another branch * Update CLI commands * update message casing to be consistent * access selector model updates * [AC-1374] Limit collection create/delete (#5963) * feat: udate request/response/data/domain models for new column, refs AC-1374 * feat: create collection management ui, refs AC-1374 * fix: remove limitCollectionCdOwnerAdmin boolean from org update request, refs AC-1374 * fix: moved collection management UI, removed comments, refs AC-1374 * fix: observable chaining now properly calls API when local org updated, refs AC-1374 * fix: remove unused form template variables, refs AC-1374 * fix: clean up observable chain, refs AC-1374 * fix: remove parent.parent route, refs AC-1374 * fix: add cd explaination, refs AC-1374 * [AC-1649] Remove organizationId from collection-bulk-delete.request (#6343) * refactor: remove organizationId from collection-bulk-delete-request, refs AC-1649 * refactor: remove request model from dialog component, refs AC-1649 * [AC-1174] Bulk collection management (#6133) * [AC-1174] Add bulk edit collection access event type * [AC-1174] Add bulk edit collection access menu option * [AC-1174] Add initial bulk collections access dialog * [AC-1174] Add logic to open bulk edit collections dialog * [AC-1174] Move AccessItemView helper methods to access selector model to be shared * [AC-1174] Add access selector to bulk collections dialog * [AC-1174] Add bulk assign access method to collection-admin service * [AC-1174] Introduce strongly typed BulkCollectionAccessRequest model * [AC-1174] Update vault item event type name * Update DialogService dependency --------- Co-authored-by: Thomas Rittson <trittson@bitwarden.com> * Rename LimitCollectionCdOwnerAdmin -> LimitCollectionCreationDeletion (#6409) * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" Pushed to feature branch instead of a new one This reverts commit 65cd395. * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" This reverts commit f7fa30b. * [AC-1680] Add manage property to collection view and response models (#6417) * Add manage property to synced Collection data * Update tests * feat: add LimitCollectionCreationDeletion conditional to canCreateNewCollections logic, refs AC-1659 (#6429) * [AC-1669] Enforce Can Manage permission on Collection dialog (#6493) * [AC-1669] Cleanup unhandled promise warnings * [AC-1669] Force change detection to ensure AccessSelector has the most recent items * [AC-1669] Initially select acting member when creating a new collection * [AC-1669] Add validator to ensure manage permission is selected * [AC-1669] Update error toast logic to support access tab errors * [AC-1669] Add error icon * [AC-1713] [Flexible collections] Add feature flags to clients (#6486) * Add FlexibleCollections and BulkCollectionAccess flags * Flag Collection Management settings * Flag bulk collection access dialog * Flag collection access modal changes * [AC-1662] Add LimitCollecitonCreationDeletion conditional to CanDelete logic (#6526) * feat: implement limitCollectionCreationDeletion into canDelete logic, refs AC-1662 * feat: make canDelete functions backwards compatible with feature flag, refs AC-1662 * feat: update vault-items.component for async getter, refs AC-1662 * feat: update configService injection, refs AC-1662 * feat: add config service to canDelete reference, refs AC-1662 * fix: remove configservice dependency from views, refs AC-1757 (#6686) * Add missing provider to vault-items.stories (#6690) * Fix imports after update from master --------- Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com> Co-authored-by: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Co-authored-by: Vincent Salucci <vincesalucci21@gmail.com> Co-authored-by: Shane Melton <smelton@bitwarden.com>

* [AC-1117] Add manage permission (bitwarden#5910) * Add 'manage' option to collection access permissions * Add 'manage' to collection permissions * remove service accidentally committed from another branch * Update CLI commands * update message casing to be consistent * access selector model updates * [AC-1374] Limit collection create/delete (bitwarden#5963) * feat: udate request/response/data/domain models for new column, refs AC-1374 * feat: create collection management ui, refs AC-1374 * fix: remove limitCollectionCdOwnerAdmin boolean from org update request, refs AC-1374 * fix: moved collection management UI, removed comments, refs AC-1374 * fix: observable chaining now properly calls API when local org updated, refs AC-1374 * fix: remove unused form template variables, refs AC-1374 * fix: clean up observable chain, refs AC-1374 * fix: remove parent.parent route, refs AC-1374 * fix: add cd explaination, refs AC-1374 * [AC-1649] Remove organizationId from collection-bulk-delete.request (bitwarden#6343) * refactor: remove organizationId from collection-bulk-delete-request, refs AC-1649 * refactor: remove request model from dialog component, refs AC-1649 * [AC-1174] Bulk collection management (bitwarden#6133) * [AC-1174] Add bulk edit collection access event type * [AC-1174] Add bulk edit collection access menu option * [AC-1174] Add initial bulk collections access dialog * [AC-1174] Add logic to open bulk edit collections dialog * [AC-1174] Move AccessItemView helper methods to access selector model to be shared * [AC-1174] Add access selector to bulk collections dialog * [AC-1174] Add bulk assign access method to collection-admin service * [AC-1174] Introduce strongly typed BulkCollectionAccessRequest model * [AC-1174] Update vault item event type name * Update DialogService dependency --------- Co-authored-by: Thomas Rittson <trittson@bitwarden.com> * Rename LimitCollectionCdOwnerAdmin -> LimitCollectionCreationDeletion (bitwarden#6409) * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" Pushed to feature branch instead of a new one This reverts commit 65cd395. * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" This reverts commit f7fa30b. * [AC-1680] Add manage property to collection view and response models (bitwarden#6417) * Add manage property to synced Collection data * Update tests * feat: add LimitCollectionCreationDeletion conditional to canCreateNewCollections logic, refs AC-1659 (bitwarden#6429) * [AC-1669] Enforce Can Manage permission on Collection dialog (bitwarden#6493) * [AC-1669] Cleanup unhandled promise warnings * [AC-1669] Force change detection to ensure AccessSelector has the most recent items * [AC-1669] Initially select acting member when creating a new collection * [AC-1669] Add validator to ensure manage permission is selected * [AC-1669] Update error toast logic to support access tab errors * [AC-1669] Add error icon * [AC-1713] [Flexible collections] Add feature flags to clients (bitwarden#6486) * Add FlexibleCollections and BulkCollectionAccess flags * Flag Collection Management settings * Flag bulk collection access dialog * Flag collection access modal changes * [AC-1662] Add LimitCollecitonCreationDeletion conditional to CanDelete logic (bitwarden#6526) * feat: implement limitCollectionCreationDeletion into canDelete logic, refs AC-1662 * feat: make canDelete functions backwards compatible with feature flag, refs AC-1662 * feat: update vault-items.component for async getter, refs AC-1662 * feat: update configService injection, refs AC-1662 * feat: add config service to canDelete reference, refs AC-1662 * fix: remove configservice dependency from views, refs AC-1757 (bitwarden#6686) * Add missing provider to vault-items.stories (bitwarden#6690) * Fix imports after update from master --------- Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com> Co-authored-by: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Co-authored-by: Vincent Salucci <vincesalucci21@gmail.com> Co-authored-by: Shane Melton <smelton@bitwarden.com>

* [AC-1117] Add manage permission (#5910) * Add 'manage' option to collection access permissions * Add 'manage' to collection permissions * remove service accidentally committed from another branch * Update CLI commands * update message casing to be consistent * access selector model updates * [AC-1374] Limit collection create/delete (#5963) * feat: udate request/response/data/domain models for new column, refs AC-1374 * feat: create collection management ui, refs AC-1374 * fix: remove limitCollectionCdOwnerAdmin boolean from org update request, refs AC-1374 * fix: moved collection management UI, removed comments, refs AC-1374 * fix: observable chaining now properly calls API when local org updated, refs AC-1374 * fix: remove unused form template variables, refs AC-1374 * fix: clean up observable chain, refs AC-1374 * fix: remove parent.parent route, refs AC-1374 * fix: add cd explaination, refs AC-1374 * [AC-1649] Remove organizationId from collection-bulk-delete.request (#6343) * refactor: remove organizationId from collection-bulk-delete-request, refs AC-1649 * refactor: remove request model from dialog component, refs AC-1649 * [AC-1174] Bulk collection management (#6133) * [AC-1174] Add bulk edit collection access event type * [AC-1174] Add bulk edit collection access menu option * [AC-1174] Add initial bulk collections access dialog * [AC-1174] Add logic to open bulk edit collections dialog * [AC-1174] Move AccessItemView helper methods to access selector model to be shared * [AC-1174] Add access selector to bulk collections dialog * [AC-1174] Add bulk assign access method to collection-admin service * [AC-1174] Introduce strongly typed BulkCollectionAccessRequest model * [AC-1174] Update vault item event type name * Update DialogService dependency --------- Co-authored-by: Thomas Rittson <trittson@bitwarden.com> * Rename LimitCollectionCdOwnerAdmin -> LimitCollectionCreationDeletion (#6409) * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" Pushed to feature branch instead of a new one This reverts commit 65cd395. * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" This reverts commit f7fa30b. * [AC-1680] Add manage property to collection view and response models (#6417) * Add manage property to synced Collection data * Update tests * feat: add LimitCollectionCreationDeletion conditional to canCreateNewCollections logic, refs AC-1659 (#6429) * [AC-1669] Enforce Can Manage permission on Collection dialog (#6493) * [AC-1669] Cleanup unhandled promise warnings * [AC-1669] Force change detection to ensure AccessSelector has the most recent items * [AC-1669] Initially select acting member when creating a new collection * [AC-1669] Add validator to ensure manage permission is selected * [AC-1669] Update error toast logic to support access tab errors * [AC-1669] Add error icon * [AC-1713] [Flexible collections] Add feature flags to clients (#6486) * Add FlexibleCollections and BulkCollectionAccess flags * Flag Collection Management settings * Flag bulk collection access dialog * Flag collection access modal changes * feat: update event type with new collection management event, refs AC-1179 * [AC-1662] Add LimitCollecitonCreationDeletion conditional to CanDelete logic (#6526) * feat: implement limitCollectionCreationDeletion into canDelete logic, refs AC-1662 * feat: make canDelete functions backwards compatible with feature flag, refs AC-1662 * feat: update vault-items.component for async getter, refs AC-1662 * feat: update configService injection, refs AC-1662 * feat: add config service to canDelete reference, refs AC-1662 * fix: remove configservice dependency from views, refs AC-1757 (#6686) * feat: add new event type and update event service, refs AC-1179 * Add missing provider to vault-items.stories (#6690) * fix: enum typo and name scheme, refs AC-1179 * fix: update message example, refs AC-1179 * fix: modifiedCollectionManagement message example, refs AC-1179 --------- Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com> Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Thomas Rittson <trittson@bitwarden.com> Co-authored-by: Shane Melton <smelton@bitwarden.com>

…6703) * [AC-1117] Add manage permission (#5910) * Add 'manage' option to collection access permissions * Add 'manage' to collection permissions * remove service accidentally committed from another branch * Update CLI commands * update message casing to be consistent * access selector model updates * [AC-1374] Limit collection create/delete (#5963) * feat: udate request/response/data/domain models for new column, refs AC-1374 * feat: create collection management ui, refs AC-1374 * fix: remove limitCollectionCdOwnerAdmin boolean from org update request, refs AC-1374 * fix: moved collection management UI, removed comments, refs AC-1374 * fix: observable chaining now properly calls API when local org updated, refs AC-1374 * fix: remove unused form template variables, refs AC-1374 * fix: clean up observable chain, refs AC-1374 * fix: remove parent.parent route, refs AC-1374 * fix: add cd explaination, refs AC-1374 * [AC-1649] Remove organizationId from collection-bulk-delete.request (#6343) * refactor: remove organizationId from collection-bulk-delete-request, refs AC-1649 * refactor: remove request model from dialog component, refs AC-1649 * [AC-1174] Bulk collection management (#6133) * [AC-1174] Add bulk edit collection access event type * [AC-1174] Add bulk edit collection access menu option * [AC-1174] Add initial bulk collections access dialog * [AC-1174] Add logic to open bulk edit collections dialog * [AC-1174] Move AccessItemView helper methods to access selector model to be shared * [AC-1174] Add access selector to bulk collections dialog * [AC-1174] Add bulk assign access method to collection-admin service * [AC-1174] Introduce strongly typed BulkCollectionAccessRequest model * [AC-1174] Update vault item event type name * Update DialogService dependency --------- Co-authored-by: Thomas Rittson <trittson@bitwarden.com> * Rename LimitCollectionCdOwnerAdmin -> LimitCollectionCreationDeletion (#6409) * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" Pushed to feature branch instead of a new one This reverts commit 65cd395. * Add manage property to synced Collection data * Revert "Add manage property to synced Collection data" This reverts commit f7fa30b. * [AC-1680] Add manage property to collection view and response models (#6417) * Add manage property to synced Collection data * Update tests * feat: add LimitCollectionCreationDeletion conditional to canCreateNewCollections logic, refs AC-1659 (#6429) * [AC-1669] Enforce Can Manage permission on Collection dialog (#6493) * [AC-1669] Cleanup unhandled promise warnings * [AC-1669] Force change detection to ensure AccessSelector has the most recent items * [AC-1669] Initially select acting member when creating a new collection * [AC-1669] Add validator to ensure manage permission is selected * [AC-1669] Update error toast logic to support access tab errors * [AC-1669] Add error icon * [AC-1713] [Flexible collections] Add feature flags to clients (#6486) * Add FlexibleCollections and BulkCollectionAccess flags * Flag Collection Management settings * Flag bulk collection access dialog * Flag collection access modal changes * [AC-1662] Add LimitCollecitonCreationDeletion conditional to CanDelete logic (#6526) * feat: implement limitCollectionCreationDeletion into canDelete logic, refs AC-1662 * feat: make canDelete functions backwards compatible with feature flag, refs AC-1662 * feat: update vault-items.component for async getter, refs AC-1662 * feat: update configService injection, refs AC-1662 * feat: add config service to canDelete reference, refs AC-1662 * fix: remove configservice dependency from views, refs AC-1757 (#6686) * Add missing provider to vault-items.stories (#6690) * [AC-1730] Add new AllowAdminAccessToAllCollectionItems setting to org response models * [AC-1730] Add new AllowAdminAccessToAllCollectionItems setting to org request model * [AC-1730] Add new collection setting to UI and include it when submitting * [AC-1122] Add V1 feature flag * [AC-1122] Hide new setting behind V1 feature flag * [AC-1122] Add support for disabled form control --------- Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com> Co-authored-by: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Co-authored-by: Vincent Salucci <vincesalucci21@gmail.com> Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Thomas Rittson <trittson@bitwarden.com>

eliykat added 3 commits October 2, 2023 08:25

First pass at feature flagging

6cb1eca

Update flag name

1163a20

Add BulkEditCollectionAccess flag

8ab1dcb

eliykat changed the base branch from master to feature/flexible-collections October 3, 2023 03:45

eliykat changed the title ~~Flexible collections - add feature flags~~ [Flexible collections] Add feature flags to clients Oct 3, 2023

eliykat added 5 commits October 9, 2023 09:50

Merge branch 'feature/flexible-collections' into flexible-collections…

27d70bd

…/add-feature-flags

Change property name

88ec465

Remove unnecessary early return

5e0d03c

Fix tests

55ed6cb

Rename flags

fdc01c2

eliykat marked this pull request as ready for review October 9, 2023 01:12

eliykat requested review from a team as code owners October 9, 2023 01:12

eliykat requested review from r-tome and Jingo88 and removed request for a team October 9, 2023 01:12

Jingo88 approved these changes Oct 9, 2023

View reviewed changes

r-tome approved these changes Oct 9, 2023

View reviewed changes

shane-melton requested changes Oct 9, 2023

View reviewed changes

Address review feedback

4cbac73

eliykat requested a review from shane-melton October 10, 2023 01:14

eliykat added 3 commits October 10, 2023 13:07

Merge remote-tracking branch 'origin/feature/flexible-collections' in…

ad7b39a

…to flexible-collections/add-feature-flags

Fix: inject abstraction

faccce8

Add feature flags to collection dialog changes

1afc94d

vincentsalucci mentioned this pull request Oct 10, 2023

[AC-1662] Add LimitCollecitonCreationDeletion conditional to CanDelete logic #6526

Merged

5 tasks

shane-melton approved these changes Oct 10, 2023

View reviewed changes

eliykat added 2 commits October 11, 2023 12:00

Update test dependencies

0792321

Remove configService from accessSelector

50c6479

Fix linting

4bb93b3

eliykat requested a review from shane-melton October 11, 2023 04:02

eliykat changed the title ~~[Flexible collections] Add feature flags to clients~~ [AC-1713] [Flexible collections] Add feature flags to clients Oct 11, 2023

shane-melton approved these changes Oct 11, 2023

View reviewed changes

eliykat merged commit 4bb7edd into feature/flexible-collections Oct 11, 2023

eliykat deleted the flexible-collections/add-feature-flags branch October 11, 2023 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AC-1713] [Flexible collections] Add feature flags to clients #6486

[AC-1713] [Flexible collections] Add feature flags to clients #6486

eliykat commented Oct 3, 2023 •

edited

Loading

bitwarden-bot commented Oct 3, 2023

shane-melton left a comment

eliykat commented Oct 11, 2023

[AC-1713] [Flexible collections] Add feature flags to clients #6486

[AC-1713] [Flexible collections] Add feature flags to clients #6486

Conversation

eliykat commented Oct 3, 2023 • edited Loading

Type of change

Objective

Code changes

Screenshots

Before you submit

bitwarden-bot commented Oct 3, 2023

New Issues

Fixed Issues

shane-melton left a comment

Choose a reason for hiding this comment

eliykat commented Oct 11, 2023

eliykat commented Oct 3, 2023 •

edited

Loading