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

[AC-1786] deprecate manager role #6931

Merged
merged 3 commits into from
Dec 12, 2023
Merged

Conversation

willmartian
Copy link
Contributor

@willmartian willmartian commented Nov 21, 2023

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 is stacked on top of #6906 because they overlap a good bit.

Deprecates the ability to designate a user as OrganizationUserType.Manager behind FeatureFlags.FlexibleCollections

Code changes

  • apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.html: Hides the Manager option in the member dialog UI
  • apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts: Maps Manager role to User role when form group loads Reverted, as this will be handled on the server instead.
  • libs/common/src/admin-console/enums/organization-user-type.enum.ts: Add deprecation comment to enum
  • libs/common/src/admin-console/models/domain/organization.ts: Add deprecation comment to model

Screenshots

FeatureFlag.FlexibleCollections is false:

image

FeatureFlag.FlexibleCollections is true:

image

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

@willmartian willmartian requested a review from a team as a code owner November 21, 2023 15:25
@willmartian willmartian changed the title Ps/ac 1786/deprecate manager role [AC-1786] deprecate manager role Nov 21, 2023
Copy link
Member

@eliykat eliykat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few other places where this is displayed in the front-end and we have to do this mapping. However, I noticed that it's also used in the CLI: https://github.com/bitwarden/clients/blob/master/apps/cli/src/commands/list.command.ts#L220-L233

Given that we have less control over when the CLI is updated, I think we have to update these mappings on the server side. (As @r-tome initially suggested.)

Can you please revert changes to member-dialog.component.ts as that will be done server-side. I'll make a ticket. Then the rest of the changes should be good to go, thank you!

@willmartian
Copy link
Contributor Author

Can you please revert changes to member-dialog.component.ts as that will be done server-side.

Done in 8245c67. Thanks!

@willmartian willmartian requested a review from eliykat December 7, 2023 16:17
Base automatically changed from ac/ac-1139/deprecate-custom-collection-perm to master December 8, 2023 18:07
@r-tome r-tome requested review from a team as code owners December 8, 2023 18:07
@r-tome r-tome requested a review from Jingo88 December 8, 2023 18:07
Copy link
Member

@eliykat eliykat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve merge conflicts, I think caused by the automatic re-basing.

@willmartian willmartian force-pushed the ps/AC-1786/deprecate-manager-role branch from 8245c67 to 9d746c7 Compare December 11, 2023 22:02
@willmartian willmartian requested a review from eliykat December 11, 2023 22:05
@bitwarden-bot
Copy link

Logo
Checkmarx One – Scan Summary & Detailsf284c245-0e24-4370-989d-dd4ae0f31ac1

No New Or Fixed Issues Found

@willmartian willmartian merged commit 02ba26e into master Dec 12, 2023
60 checks passed
@willmartian willmartian deleted the ps/AC-1786/deprecate-manager-role branch December 12, 2023 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants