clients icon indicating copy to clipboard operation
clients copied to clipboard

[PM-6418] Fix environment selector on desktop

Open Hinton opened this issue 1 year ago • 2 comments
trafficstars

Type of change

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

Objective

The environment selector is broken on desktop. When selecting self-hosted and filling in a url, the selector fails to update when returning and instead produces the following console error.

ERROR Error: Uncaught (in promise): TypeError: Cannot read properties of undefined (reading 'updateEnvironmentInfo')

The picker also doesn't follow best practices and rather than relying on reactivity to show the correct information it relies on parent components calling into methods. This is a fragile pattern that should in almost all cases be avoided since it encourages tight coupling and suffers from the inappropriate Intimacy code smell.

Code changes

  • apps/desktop/src/auth/login/login.component.ts: Remove the ViewChild which resolves the tight coupling and inappropriate intimacy.
  • libs/angular/src/auth/components/environment-selector.component.ts: Convert environment selector to be reactive and react on config and url changes.

Question, why would the environment picker care about configurations?

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

Hinton avatar Feb 22 '24 13:02 Hinton

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 26.41%. Comparing base (165f9c4) to head (12baa98).

:exclamation: Current head 12baa98 differs from pull request most recent head 1fccc85. Consider uploading reports for the commit 1fccc85 to get more accurate results

Files Patch % Lines
.../auth/components/environment-selector.component.ts 0.00% 3 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8046      +/-   ##
==========================================
- Coverage   26.95%   26.41%   -0.55%     
==========================================
  Files        2322     2290      -32     
  Lines       67746    67089     -657     
  Branches    12671    12593      -78     
==========================================
- Hits        18262    17719     -543     
+ Misses      48090    47988     -102     
+ Partials     1394     1382      -12     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 22 '24 13:02 codecov[bot]

Logo Checkmarx One – Scan Summary & Detailscde36087-1b10-423a-9ac7-ff1716e2bffb

No New Or Fixed Issues Found

bitwarden-bot avatar Feb 22 '24 15:02 bitwarden-bot