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.

Sign up for GitHub

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

Jump to bottom

[EC-788] Providers cannot utilize Master Password Reset in Managed Orgs after November release #4487

Merged
merged 3 commits into from
Feb 6, 2023
Merged

[EC-788] Providers cannot utilize Master Password Reset in Managed Orgs after November release #4487

merged 3 commits into from
Feb 6, 2023

Conversation

coroiu
Copy link
Contributor

@coroiu coroiu commented Jan 16, 2023

Type of change

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

Objective

The people component was updated to use the new PolicyService observable. However that broke compatibility with providers which need to manually fetch org info for their clients.

Bug was introduced in #3259

Screenshots

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

@coroiu
[EC-788] fix: people component using wrong policy list
673ae53 
The people component was updated to use the new PolicyService observable. However that broke compatibility with providers which need to manually fetch org info for their clients.
@coroiu coroiu requested a review from r-tome January 16, 2023 16:04
r-tome
r-tome previously approved these changes Jan 17, 2023
View reviewed changes
Copy link
Contributor

@r-tome r-tome left a comment

Choose a reason for hiding this comment

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

Nice work!

apps/web/src/app/organizations/manage/people.component.ts Outdated Show resolved Hide resolved
apps/web/src/app/organizations/manage/people.component.ts Outdated Show resolved Hide resolved
@coroiu coroiu added the needs-qa Marks a PR as requiring QA approval label Jan 17, 2023
@coroiu
Merge branch 'master' into EC-788-providers-cannot-utilize-master-pas…
beb79e6 
…sword-reset-in-managed-orgs-after-november-release
@coroiu coroiu requested a review from r-tome January 23, 2023 13:43
@coroiu
Copy link
Contributor Author

coroiu commented Jan 23, 2023

@r-tome I just merged master so no new changes but you approval was dismissed :)

r-tome
r-tome previously approved these changes Jan 23, 2023
View reviewed changes
@github-actions github-actions bot temporarily deployed to Web Vault - QA January 25, 2023 03:54 Inactive
@github-actions github-actions bot temporarily deployed to Web Vault - QA February 5, 2023 23:24 Inactive
@coroiu
Merge branch 'master' into EC-788-providers-cannot-utilize-master-pas…
d45524b 
…sword-reset-in-managed-orgs-after-november-release
@coroiu coroiu removed the needs-qa Marks a PR as requiring QA approval label Feb 6, 2023
@coroiu coroiu merged commit a4aa042 into master Feb 6, 2023
@coroiu coroiu deleted the EC-788-providers-cannot-utilize-master-password-reset-in-managed-orgs-after-november-release branch February 6, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@r-tome r-tome r-tome approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@coroiu @r-tome