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

EmailAPI Validation cleanup + test coverage #23

Merged
merged 6 commits into from
Oct 28, 2024
Merged

Conversation

UZ9
Copy link
Contributor

@UZ9 UZ9 commented Oct 24, 2024

  • Added full validation test coverage of EmailAPI-related methods
  • Moved all Error throws to JunoError to be more easily detected with unit tests
  • Fixed bug where an undefined email sender resulted in an undefined error
  • Refactored all validations to now use a centralized validation system to reduce chance of error
  • Added unit tests for string and email validation, with reproduction of aforementioned bug
  • Added jest to the project
  • Juno imports no longer require build/main in the name

- Moved all Error throws to JunoError to be more easily detected with
unit tests
- Refactored all validations to now use a centralized validation system
to reduce chance of error
- Added unit tests for string and email validation
- Added jest to the project
@UZ9 UZ9 requested a review from tmthecoder October 24, 2024 02:33
Copy link
Contributor Author

@UZ9 UZ9 left a comment

Choose a reason for hiding this comment

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

Added comments regarding changes considering this is a somewhat lengthy commit

src/lib/email.ts Outdated
throw new Error(
'Parameter recipients or content cannot be an empty array'

if (recipients.length === 0 && cc?.length === 0 && bcc?.length === 0) {
Copy link
Contributor Author

@UZ9 UZ9 Oct 24, 2024

Choose a reason for hiding this comment

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

Recipients can be length 0 (it was previously assumed it couldn't) as long as there is at least one of either recipient, cc, or bcc. This will likely be something we need to change with the schema as well considering recipients is not an optional field in it

@@ -63,7 +70,7 @@ export class EmailAPI {
);
return result.body;
} catch (e) {
throw new Error(e);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Throwing new Error here was previously obfuscating what the error actually was

}
}
}

const validateEmailRecipient = (recipient: EmailRecipient) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Validation methods moved to central validators.ts file

@@ -12,21 +13,21 @@ class JunoAPI {

get user(): UserAPI {
if (!this.userAPI) {
throw new Error('juno.init() must be called before using the Juno SDK');
throw new JunoValidationError('juno.init() must be called before using the Juno SDK');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Throwing a JunoValidationError instead of a normal Error helps ensure unit tests are catching the correction exception (it could accidentally catch something else and pass otherwise)

@UZ9 UZ9 changed the title Validation cleanup + tests EmailAPI Validation cleanup + test coverage Oct 24, 2024
@UZ9 UZ9 merged commit c163969 into main Oct 28, 2024
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.

1 participant