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

Vault Timeout Service Tests & Bug Fix #7021

Merged
merged 2 commits into from
Nov 29, 2023
Merged

Conversation

justindbaur
Copy link
Member

@justindbaur justindbaur commented Nov 29, 2023

Type of change

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

Objective

While working on a different ticket I started working on tests for VaultSettingsService and noticed a bug while writing the tests. The other ticket may not be completed as quickly so I decided to make a PR for these items instead of waiting for the other work.

Helps with PM-4996

Code changes

  • libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts: Add a lot of tests to cover the service.
  • libs/common/src/services/vault-timeout/vault-timeout.service.ts: Fix a bug where we would be getting the vault timeout actions for the current active user vs getting them for the user actively locking.

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

Get availableTimeoutActions for the user actually locking
instead of getting the timeout actions for the active user.
@justindbaur justindbaur requested a review from a team as a code owner November 29, 2023 13:55
@github-actions github-actions bot added the needs-qa Marks a PR as requiring QA approval label Nov 29, 2023
expectNoAction("2");
});

it("should run action on an account that hasn't been active for greater than 1 minute and has a vault timeout for 1 minutes", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: I prefer single assert tests, which would make this test a describe block with four tests in it

@MGibson1 MGibson1 removed the needs-qa Marks a PR as requiring QA approval label Nov 29, 2023
@justindbaur justindbaur merged commit 3451ee8 into master Nov 29, 2023
61 of 63 checks passed
@justindbaur justindbaur deleted the vault-timeout-service-tests branch November 29, 2023 14:31
jlf0dev pushed a commit that referenced this pull request Nov 30, 2023
* Add Tests to VaultTimeoutService

* Fix Bug

Get availableTimeoutActions for the user actually locking
instead of getting the timeout actions for the active user.
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.

None yet

2 participants