search

mikermcneil / machinepack-mandrill

Node.js machines for use with the Mandrill API
http://node-machine.org/machinepack-mandrill
5 stars 7 forks source link

Added send-template-email #2

Closed alexlenail closed 9 years ago

alexlenail commented 9 years ago

Do not merge: 0 tests have been done. I'm just opening this PR so we can kickoff the conversation about it. I was hoping you might point me in the right direction.

The first commit just makes a copy of send-plaintext-email. The second one makes the necessary modifications. If you diff the two of those you'll essentially see what I changed. I may not have changed enough, or too much, or not changed properly. Hoping you can take a look at this and let me know.

sgress454 commented 9 years ago

At a glance, it looks good! Two suggestions:

  1. Make the template name a required input.
  2. Make the template content a typeclass: "dictionary" instead of providing a string example for it. See https://github.com/treelinehq/machinepack-util/blob/master/machines/build-dictionary.js#L9 for reference. Then stringify the input yourself before providing it to the API--this allows end-users of the machine to pass in Javascript objects without caring about the underlying implementation.

Automated tests for API services are a bit tricky because you don't want to check your API credentials in with the tests. The best way to do it is using environment vars. I'm working on fixing up the other tests for this pack, so don't worry about that for now. In the meantime, you can test it pretty easily yourself by using the machinepack CLI:

npm install -g machinepack

then in your machinepack-mandrill directory, do:

mp exec send-template-email --apiKey=yourapikey --toEmail=johndoe@gmail.com --templateName=someTemplate --templateContent={"someKey":"someVal"}
alexlenail commented 9 years ago

Made some of the edits you recommended, but wasn't able to run that command on account of 

Running machine...

Unexpected error occurred:
 Error: Cannot find module 'machine'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/Users/Alex/Documents/machinepack-mandrill/index.js:2:18)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)

which also happened when I tried to send a plaintext email.

sgress454 commented 9 years ago

Good rule of thumb...when life gives you Cannot find module, make npm install!

npm install machine
alexlenail commented 9 years ago

So I've run into a new issue with MachinePacks...

bash-3.2$ mp exec send-template-email --apiKey=(theKey) --toEmail=zfrenchee@gmail.com --templateName=Welcome --templateContent=[{'name':'LINK','content':'someVal'}]

 Running machine...

Unexpected error occurred:
 Error: `run-machine-interactive` machine encountered 1 error(s) while validating runtime input values.
    at /usr/local/lib/node_modules/machinepack/node_modules/machine/lib/Machine.prototype.exec.js:53:19
    at Machine_prototype_exec [as exec] (/usr/local/lib/node_modules/machinepack/node_modules/machine/lib/Machine.prototype.exec.js:61:9)
    at /usr/local/lib/node_modules/machinepack/bin/machinepack-exec.js:149:14
    at identity (/usr/local/lib/node_modules/machinepack/bin/machinepack-exec.js:88:20)
    at Object.Machinepacks.readPackageJson.exec.success (/usr/local/lib/node_modules/machinepack/bin/machinepack-exec.js:117:11)
    at afterwards (/usr/local/lib/node_modules/machinepack/node_modules/machine/lib/intercept-exit-callbacks.js:128:21)
    at voided [as _onTimeout] (/usr/local/lib/node_modules/machinepack/node_modules/machine/lib/intercept-exit-callbacks.js:97:20)
    at Timer.listOnTimeout [as ontimeout] (timers.js:112:15)

On the other hand, 

mp exec send-plaintext-email --apiKey=(theKey) --toEmail=zfrenchee@gmail.com

works, but 

mp exec send-template-email --apiKey=(theKey) --toEmail=zfrenchee@gmail.com --templateName=Welcome --templateContent=[]

does not (with the same error). I definitely have a 'Welcome' Template in my account, so the question, I guess, is how do you pass an array to the machine/command line?

sgress454 commented 9 years ago

If you still have templateContent as a string, you might wrapping it in double-quotes on the command line. If that doesn't work, it may just be a case of something the command-line runner doesn't handle well (stringified JSON), in which case you can write a simple .js script to test out the machine:

require('./').sendTemplateEmail({
   apiKey: "theapikey",
   toEmail: "john@doe.com",
   templateName: "Welcome",
   templateContent: "[{'name': 'of attribute', 'content': 'of attribute'}]"
}).exec(console.log);
alexlenail commented 9 years ago

I think it... works? Or at least it's triggering the 'success' exit. I'd love for this to be merged as soon as possible. As github has pointed out, I was intending to use this for a project I'm working on with some friends.

alexlenail commented 9 years ago

Hi Scott, What do you think? Is it ready to merge? Is there anything else in particular I should handle first?

sgress454 commented 9 years ago

Yup--excellent job! Thanks Alexander!

alexlenail commented 9 years ago

Hi Scott, When does it become something I can npm install?

sgress454 commented 9 years ago

I just published v0.4.0, so you should be able to install it. It will be updated on node-machine.org in the next 10 minutes or so, and on Treeline in the next hour or so.