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

Expose Client and MailService classes #650

Merged
merged 7 commits into from
Jan 12, 2018
Merged

Expose Client and MailService classes #650

merged 7 commits into from
Jan 12, 2018

Conversation

adamreisnz
Copy link
Contributor

@adamreisnz adamreisnz commented Jan 7, 2018

Fixes #649

Checklist

  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the [Contribution Guide] and my PR follows them.
  • I updated my branch with the master branch.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation about the functionality in the appropriate .md file
  • I have added in line documentation to the code I modified

Short description of what this PR does:

This PR exposes the Client and MailService classes via their default module exports allowing end users to manually instantiate client or mailer instances, so that multiple API keys or different settings can be used without affecting the global singleton client and mailer instances.

Example usage

Mailer:

//Import mail service class
const {MailService} = require('@sendgrid/mail');

//Instantiate mailers
const sgMail1 = new MailService();
const sgMail2 = new MailService();

//Set different API keys
sgMail1.setApiKey('KEY1');
sgMail2.setApiKey('KEY2');

Client:

//Import client class
const {Client} = require('@sendgrid/client');

//Instantiate clients
const sgClient1 = new Client();
const sgClient2 = new Client();

//Set different API keys
sgClient1.setApiKey('KEY1');
sgClient2.setApiKey('KEY2');

Notes

  • Backwards compatible as the singleton instances are still the default exports
  • TypeScript definitions have not been updated or amended
  • Documentation and use cases have been updated
  • No new tests added as no new functionality was added, merely exposed the classes
  • Ran tests on client and mail sub packages to verify still working ok

cc @thinkingserious @tflanagan

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Jan 7, 2018
@adamreisnz
Copy link
Contributor Author

adamreisnz commented Jan 7, 2018

@thinkingserious I've updated the license year as well to fix the failing build. Maybe put that check in first, so that the Travis build fails faster without having to run the rest of the tests first before it finds out something trivial like that fails?

I've also tried to fix the issue with the test:files command but it's still failing so will leave that for you to look in to.

@thinkingserious
Copy link
Contributor

Hi @adamreisnz,

LGTM

With regards to the tests, I agree.

For the remaining failing test, I believe this should fix it.

@thinkingserious thinkingserious merged commit 128115f into sendgrid:master Jan 12, 2018
@thinkingserious
Copy link
Contributor

Hello @adamreisnz,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

@J-Gonzalez
Copy link

Is this documented anywhere other than here? We had to dig for this one as we wanted different emails to go out from different subusers on our account (transactional vs marketing etc). This merged PR def does the job, it's just not documented anywhere from what I could find (and I always worry that non-documented items are the first to get changed without notice).

@adamreisnz
Copy link
Contributor Author

@J-Gonzalez
Copy link

Ah yea - It seems it's in the client README file, but not in the Mail README, but used to also be in the Mail USECASES file, but that since has been edited to link here and no longer has that info.

@thinkingserious
Copy link
Contributor

Thanks for the heads up @J-Gonzalez, I've filed an issue for a fix.

Thanks for helping out @adamreisnz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No clear usage path for multiple keys
3 participants