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

ref: convert mail and sms services into neverthrown variants #1344

Merged
merged 22 commits into from
Mar 17, 2021

Conversation

karrui
Copy link
Contributor

@karrui karrui commented Mar 10, 2021

Problem

This PR improves the type safety and error handling of MailService and SmsService by refactoring the exposed functions into their neverthrown variants.

Sorry for the huge PR, they were pretty interconnected; I tried to make the commits as concise as possible for ease of reviewing commit-by-commit.

Closes #1260

Solution

Improvements:

  • convert MailService functions to neverthrown variant
  • convert SmsService functions to neverthrown variant
  • use new neverthrown fns from mail and sms services in VerificationService
  • store original thrown Twilio send errors in SmsSendError for logging purposes
  • store original thrown mail send errors in MailSendError for logging purposes

Tests

  • All affected tests have been updated to conform to the new refactored functions.

For mail service

  • Create an email form, and add an email field with email verification and confirmation.
    1. Enter your email into the email form and verify your email. The email should be sent to the given email and the OTP should validate successfully.
    2. Submit the form. You (as form admin) should receive the usual form submission email with the correct answers. You (as the form filler) should also receive an acknowledgement email.

For sms service

  • Create verified sms field. Go to form and check if sms is received
    • Give invalid OTP. Correct invalid otp error should be shown.
    • Turn off data in Chrome console and submit OTP. Correct unknown error should be shown.
    • Enter correct OTP. Should be verified correctly.
  • Update emergency contact number
    • Give invalid OTP. Correct invalid otp error should be shown.
    • Turn off data in Chrome console and submit OTP. Correct unknown error should be shown.
    • Enter correct OTP. Should be verified correctly.

Directly use refactored function from MailService
test was failing due to old code using await syntax (and thus does not need a resolved value), and the new code uses neverthrow#andThen, which requires a promise to continue
@karrui karrui marked this pull request as ready for review March 11, 2021 02:18
@karrui karrui changed the title wip: convert mail and sms services into neverthrown variants ref: convert mail and sms services into neverthrown variants Mar 11, 2021
src/app/modules/verification/verification.service.ts Outdated Show resolved Hide resolved
src/app/modules/verification/verification.service.ts Outdated Show resolved Hide resolved
src/app/services/sms/sms.service.ts Outdated Show resolved Hide resolved
src/app/services/sms/sms.service.ts Outdated Show resolved Hide resolved
src/app/services/sms/sms.service.ts Outdated Show resolved Hide resolved
src/app/services/sms/sms.errors.ts Show resolved Hide resolved
@karrui karrui requested a review from mantariksh March 11, 2021 07:43
Copy link
Contributor

@mantariksh mantariksh left a comment

Choose a reason for hiding this comment

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

this is the way

@karrui karrui merged commit 10e8d7f into develop Mar 17, 2021
@karrui karrui deleted the ref/neverthrown-mail-service branch March 17, 2021 05:19
@karrui karrui mentioned this pull request Mar 23, 2021
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.

Investigate SMTPConnection errors and improve type safety around mail service
3 participants