Skip to content

basic createTemplate task#1

Open
hybernaut wants to merge 30 commits intomasterfrom
template-api-support
Open

basic createTemplate task#1
hybernaut wants to merge 30 commits intomasterfrom
template-api-support

Conversation

@hybernaut
Copy link
Owner

No description provided.

Copy link

@derekrushforth derekrushforth left a comment

Choose a reason for hiding this comment

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

This looks awesome! Left some feedback for you. I know some of it is a little crazy but I'd really like to keep the grunt configuration as simple as possible for people using this.

Gruntfile.js Outdated
}
},

"postmark-servers": {

Choose a reason for hiding this comment

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

This is a neat idea. Can you move this to a separate PR? I think we should try to focus on sending and syncing templates with existing servers for now.

Gruntfile.js Outdated
build: {
name: "testing-template-node-js-" + new Date().valueOf(),
subject: "Testing grunt-postmark-templates",
// NOTE these are read from filesystem. globbing not supported

Choose a reason for hiding this comment

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

globbing not supported

👍

Copy link
Owner Author

Choose a reason for hiding this comment

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

I know that grunt has fancy src and dest file handling but in my first pass I did it in a less idiomatic way.

Gruntfile.js Outdated
name: "testing-template-node-js-" + new Date().valueOf(),
subject: "Testing grunt-postmark-templates",
// NOTE these are read from filesystem. globbing not supported
htmlFile: 'test/email.html',

Choose a reason for hiding this comment

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

For consistency with our API, can you rename these to htmlBody and textBody?

Copy link
Owner Author

Choose a reason for hiding this comment

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

In order to distinguish between literal htmlBody and htmlFile(filename) the task will accept either. I'll update the gruntfile to indicate this, and hopefully that will make it clearer.

Copy link
Owner Author

Choose a reason for hiding this comment

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

After working on this some more, I now agree with you @derekrushforth that these should be htmlBody and textBody. The entire workflow with mailmason is file based, so it doesn't really make sense to allow literal strings for template bodies.

Gruntfile.js Outdated
}
},

"postmark-templates": {
Copy link

@derekrushforth derekrushforth Dec 19, 2016

Choose a reason for hiding this comment

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

Having separate tasks for pushing new templates and editing existing templates might be hard to manage for users.

I imagine this would work as a single task(grunt postmark-sync), where it creates a new template if the referenced template isn't found on the server, otherwise it updates the existing template.

That way users don't have to worry about what lives on the server and what doesn't.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I built postmark-server to be idempotent, which wasn't too hard. The issue I encountered with doing the same for postmark-template is that you need the templateID to update, as I don't think template names are unique identifiers.

htmlBody: htmlBody,
subject: _data.subject || options.subject,
}, function(err, response) {
handleResponse(err, response, done);
Copy link

@derekrushforth derekrushforth Dec 19, 2016

Choose a reason for hiding this comment

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

I'd like to get editTempate (update) working as well, but haven't figured out where the templateIDs will come from. It might make sense to move this to a separate grunt plugin.

Can you see the TemplateId being returned from this response object? From here, could you maintain a reference to the email template in the file system, the server(s) it lives on, and template ID of server in an external JSON file?

I know it's a bit tough to manage on the plugin end, but it would be super nice for users to not even think about this.

Choose a reason for hiding this comment

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

A private config that the plugin maintains could look something like this:

[
  {
    "name": "welcome",
    "htmlBody": "test/welcome.html",
    "textBody": "test/welcome.txt",
    "servers": [
      {
        "token": "1234-5678",
        "templateId": "123456"
      },
      {...}
    ]
  }, {...}
] 

Copy link
Owner Author

Choose a reason for hiding this comment

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

For our system that will be stored in a database, which I imagine to be in a separate user-space gruntfile. Pulling it from another JSON config file (as you do with secrets.yml) seems like the idiomatic way to do it for Grunt. In my user-space gruntfile I imagine setting the config entries which are then used by the postmark gruntfile.

@derekrushforth
Copy link

derekrushforth commented Dec 19, 2016

Just for reference :)
ActiveCampaign/mailmason#25

@hybernaut
Copy link
Owner Author

simplified the configuration so that the user just specifies a set of templates in this form:

templates.json:

{
  "confirm_email": {
    "subject": "SiteWorx: Confirm your email",
    "htmlBody": "dist/confirm_email.html",
    "textBody": "dist/confirm_email.txt"
  }
}

or in the Mailmason Gruntfile:

"postmark-templates": {
  "confirm_email": {
    "subject": "SiteWorx: Confirm your email",
    "htmlBody": "dist/confirm_email.html",
    "textBody": "dist/confirm_email.txt"
  }
}

The next step will be to use a templateID for update if it appears in the configuration.

I'd also really like to write out a json file when the copies are finished with the new {name, templateID} pairs (possibly the templates.json file) because I need this information to configure my template-using app.

@jmas
Copy link

jmas commented Nov 23, 2017

@derekrushforth @hybernaut Hello guys, any progress on this PR? It's already done or have some critical issues?

@dprensha
Copy link

Any updates on this PR? Would love to have this to help with our automated builds.

@hybernaut
Copy link
Owner Author

I have not looked at this in a while, but I've been using it regularly since filing the PR.

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.

4 participants