Use query params to check which device needs verif

[crisnicandrei]

Steps to test:

Add phone number
Click verify phone
it should redirect you as expected to phone verification

Navigate back
Go to email verification
It should take you to email verification

[codecov]

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 44.56%. Comparing base (f5fcea6) to head (700f82b).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
src/app/auth/components/verify/verify.component.ts	52.00%	8 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #560      +/-   ##
==========================================
- Coverage   44.57%   44.56%   -0.01%     
==========================================
  Files         369      370       +1     
  Lines       11271    11293      +22     
  Branches     1846     1852       +6     
==========================================
+ Hits         5024     5033       +9     
- Misses       6083     6092       +9     
- Partials      164      168       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[slifty]

Slight suggested refactor to make sure the code is easy to understand later; also it looks like tests currently fail.

[slifty]

This variable name no longer actually describes what it claims to contain -- specifically, needsEmail might contain false even though the current user does, in fact, need email. We should be careful here, as this represents tech debt / makes the variable un-intuitive.

Instead of changing the purpose of those variables I think it would be cleaner and more direct to populate and use a new variable which captures our true intent. For instance:

this.currentVerifyFlow = 'none';

if (this.needsEmail && !this.queryParams?.sendSms) {
  this.currentVerifyFlow = 'email';
} else if (this.needsPhone && !this.queryParams?.sendEmail) {
  this.currentVerifyFlow = 'phone';
}

Later in the component we would then branch behavior based on the value of this.currentVerifyFlow.

NOTE: I wrote this.queryParams?... because later we have an if (queryParams) which indicates it's possible for queryParams to be a nullish value, and accessing it directly would risk an error.

[slifty]

Sorry for still more changes here.

Right now this component has just too many potentially conflicting state-representing-attributes.

We should just have one: currentVerifyFlow

The value of that single attribute should determine what language is rendered, what flows are followed, etc.

Also I looked at the rest of the component and it looks like there is a bit of redundant logic which hasn't been updated. (for instance: onSubmit has return verifyPromise which has a bunch of logic around form titles and other things.). I couldn't add a comment insline since it's not part of the diff, but we should got through the entire component and clean it up / remove redundant logic and shift to currentVerifyFlow

[slifty]

It feels strange to have this switch case in a way that does not cover all three flow cases.

We can do this is because the "default" state of the component is email verification flow, but that is actually a brittle approach -- we could easily modify the defaults not realizing that the default isn't a generic pre-initialization state but is actually an email-initialization state.

I think this is the appropriate time to fix this.

Suggestions:

1. Remove the this.verifyingEmail and this.verifyingPhone attributes completely -- there is no valid use case where both can be true. Instead we would have ONLY currentVerifyFlow and the renderer would be updated to look at that variable instead.

2. Remove formTitle -- instead, src/app/auth/components/verify/verify.component.html should render the appropriate title based on the current verify flow (this is already done for other aspects of the form).

3. I think this means you can remove this switch statement entirely. Instead you would have a "sanity" check of "if none then redirect because that shouldn't be the case"

[crisnicandrei]

@slifty thanks Dan! This is such a great suggestion! Sorry I didn't catch it earlier