[AC-2302] Extract device approve/deny logic into a service
Type of change
- [ ] Bug fix
- [ ] New feature development
- [X] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
Move all device approve / deny code from the component into a service.
Code changes
-
bitwarden_license/bit-common/*: Jest configs for
bit-common -
bitwarden_license/bit-common/src/admin-console/services/auth-requests/*: Moved files from
bit-webtobit-common -
bitwarden_license/bit-common/src/admin-console/services/auth-requests/organization-auth-request-api.service.ts: Renamed and removed
@Injectable()decoration -
bitwarden_license/bit-common/src/admin-console/services/auth-requests/organization-auth-request.service.spec.ts: Unit tests for
OrganizationAuthRequestService - bitwarden_license/bit-common/src/admin-console/services/auth-requests/organization-auth-request.service.ts: Created service with logic extracted from component
- bitwarden_license/bit-web/src/app/admin-console/organizations/core/*: Removed files as they are not necessary any longer
-
bitwarden_license/bit-web/src/app/admin-console/organizations/manage/device-approvals/device-approvals.component.ts: Extracted logic into
OrganizationAuthRequestService -
bitwarden_license/bit-web/src/app/admin-console/organizations/organizations.module.ts: Configured DI manually for
OrganizationAuthRequestApiServiceandOrganizationAuthRequestService
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
Codecov Report
Attention: Patch coverage is 55.55556% with 16 lines in your changes are missing coverage. Please review.
Project coverage is 27.91%. Comparing base (
4131aa9) to head (77675ad).
Additional details and impacted files
@@ Coverage Diff @@
## main #8818 +/- ##
==========================================
+ Coverage 27.89% 27.91% +0.01%
==========================================
Files 2357 2356 -1
Lines 69616 69619 +3
Branches 13119 13117 -2
==========================================
+ Hits 19421 19435 +14
+ Misses 48656 48644 -12
- Partials 1539 1540 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Checkmarx One – Scan Summary & Details – 822a5862-0c58-4d81-a32a-4baf6860795f
No New Or Fixed Issues Found
I had to spend a bit more time on this, here's the summary.
In https://github.com/bitwarden/clients/pull/8818/commits/5e66416c13dff18d61d09a2431cec4609140b1c1 I removed the @bitwarden/bit-common path mapping from apps/web, because the web vault is OSS code. It should never import BL code, otherwise it's no longer purely OSS. Instead, BL code is our entrypoint, and it imports OSS code. (You can see this because npm run build:bit points to a webpack file in the bit-web directory.) ((When you start updating the CLI, it will need a similar configuration.))
However, npm run build:bit was still using the tsconfig.json in the apps/web directory, because that's where the command was being run from. So removing that mapping broke the bit build.
The Angular Webpack Plugin does provide an option to specify the location of tsconfig, but it looks like at some stage it changed name: the docs call it tsconfig whereas our configuration uses tsConfigPath, so that wasn't actually working as intended.
Once I fixed that to use the bit-web tsconfig, the build broke because that tsconfig isn't fully set up. I had to copy across the configuration from apps/web/tsconfig.json and update relative paths, and then for good measure, remove the BL entrypoint from apps/web/tsconfig.json. (One annoying thing here is that the tsconfig extends option isn't very robust; as soon as you set a top-level configuration option in the child tsconfig, it'll blast over the parent tsconfig, so lots of copy/paste is required here to preserve nested config objects.)
On the plus side, this is now working how it should 😁 However you and @addisonbeck should review my commits.