[PM-9113] Trim Whitespace from email in sponsorship form

cturnbull-bitwarden commented 3 months ago

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-9113

📔 Objective

This PR aims to trim whitespace from the email field when setting up a families sponsorship. It accomplishes this by adding a validator which prevents whitespace from being added as a value.

Addtionally, this PR moves the sponsorship files to the billing folder.

📸 Screenshots

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 30.11%. Comparing base (3f0f5af) to head (716520c).

Files	Patch %	Lines
...lar/src/directives/input-strip-spaces.directive.ts	33.33%	3 Missing and 1 partial :warning:
apps/web/src/app/oss-routing.module.ts	0.00%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #9781 +/- ## ========================================== - Coverage 30.12% 30.11% -0.01% ========================================== Files 2576 2576 Lines 75446 75449 +3 Branches 14113 14114 +1 ========================================== - Hits 22729 22723 -6 - Misses 50952 50961 +9 Partials 1765 1765 ```

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

github-actions[bot] commented 3 months ago

Checkmarx One – Scan Summary & Details – 870d1ada-b92c-49e4-b042-c4465694a434

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	Client_Privacy_Violation	/apps/desktop/src/auth/lock.component.html: 32	Attack Vector

Fixed Issues

Severity	Issue	Source File / Package
	Client_Privacy_Violation	/apps/web/src/app/auth/lock.component.ts: 18
	Client_Privacy_Violation	/apps/web/src/app/auth/lock.component.ts: 46

cturnbull-bitwarden commented 3 months ago

I don't think a validator is the right tool for this - it's not validating the form input (it never returns a ValidationError), it's mutating it. I think this is better suited to a directive, like in this example. However it may also be worth talking to the CL team about in case it's functionality that can be incorporated directly into our CL forms.

Thanks for this, @eliykat. I definitely should have checked that we didn't already have a directive that did this. Turns out we did, so I pushed an update to just use that instead!

cturnbull-bitwarden commented 2 months ago

@eliykat @dani-garcia @amorask-bitwarden Hey folks, I just pushed a change addressing an issue where, if the user pressed Enter after entering an email address with spaces anywhere, the directive wouldn't update the underlying control value. I resolved this by explicitly updating both the element and the control inside the directive whenever the input event fires. Let me know your thoughts, or if you think there's a better way to do this!

Commit here: https://github.com/bitwarden/clients/pull/9781/commits/0d4bf012c31744d54b3e86db34f5f12d9d4bee45

bitwarden / clients