search

forwardemail / email-templates

Create, preview (browser/iOS Simulator), and send custom email templates for Node.js. Made for @forwardemail, @ladjs, @cabinjs, @spamscanner, and @breejs.
https://forwardemail.net/docs/send-emails-with-node-js-javascript
MIT License
3.67k stars 337 forks source link

templateExists can never find a folder, remove check? #330

Closed pke closed 5 years ago

pke commented 6 years ago

While upping the code coverage I am stuck with this codepath. https://github.com/niftylettuce/email-templates/blob/a296f10818295353e9b34a196340338ac3fe406c/src/index.js#L125

To my understanding, getPaths will always return a file, never a folder?

if (stats.isDirectory()) {
  // a directory
  return {
    rel: path.join(rel, `index.${ext}`),
    ext
  };
}

So for folders it return <folder>/index.ext without further checking if this index file exists. So the check in templateExists() is actually never triggered and could be removed?

niftylettuce commented 5 years ago

you're correct, I will be fixing this in next release

niftylettuce commented 5 years ago

actually @pke this is needed b/c getPaths will not check if the file index.${ext} exists. so it is necessary in order to return false.

~to write coverage for this, simply create a folder without an index file~

niftylettuce commented 5 years ago

related to #332 - I am writing a fix so that if subject || html || text don't exist on message then to not send email and throw an error

niftylettuce commented 5 years ago

actually I think you're right, I don't have much time to look right now more at this but if you want to patch please do

pke commented 5 years ago

I am also a bit short on time right now, need to patch a koa-joi-router-docs package to emit more OpenAPI doc infos, but will come back to this later. I already have 90% code coverage here.