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

Add Dynamic Template Support #447

Closed
thinkingserious opened this issue Jul 26, 2018 · 17 comments · Fixed by #449
Closed

Add Dynamic Template Support #447

thinkingserious opened this issue Jul 26, 2018 · 17 comments · Fixed by #449
Labels
difficulty: medium fix is medium in difficulty status: work in progress Twilio or the community is in the process of implementing type: twilio enhancement feature request on Twilio's roadmap

Comments

@thinkingserious
Copy link
Contributor

Issue Summary

On 7/24/2018, our team publicly launched dynamic content for transactional templates. It is now available for all customers sending over v3 of our Mail Send API. Iterate over lists, handle conditionals and more, thanks to native support for a subset of Handlebars syntax!

More information can be found in our blog post announcement.

You can currently use this feature by manually creating the request body as shown here.

Now, we need to create helper code and examples for this SDK.

Acceptance Criteria

Documentation

@thinkingserious thinkingserious added status: help wanted requesting help from the community difficulty: medium fix is medium in difficulty up-for-grabs type: twilio enhancement feature request on Twilio's roadmap labels Jul 26, 2018
@marcusborges1
Copy link
Contributor

Hello! I'm working on this issue.

@Octember
Copy link

Bump on this. I just got started using Sendgrid and there is no mention that certain libraries are completely incompatible with the recommended new path for dynamic email templates.

Had to spend hours tracking down this issue, to realize it doesn't work at all 😞

I'll have to go with legacy templates for now.

@thinkingserious
Copy link
Contributor Author

Hello @Octember,

You can use dynamic email templates with this library right now; however, you will need to build the request body yourself. Please see this example.

One way to do this is to use our helper to build the request body per usual, then print out the request body and replace substitutions key with dynamic_template_data.

That said, I think PR #449 is close.

With Best Regards,

Elmer

@Octember
Copy link

Thanks for the advice @thinkingserious but I think both approaches, 1) building the JSON manually and 2) Hacking out the request body are both quite hacky, should not be encouraged for production-level APIs.

I'll wait a few days on #449 and check back here.

I appreciate the quick response! Cheers.

@nickneiman
Copy link

nickneiman commented Jul 31, 2018

@Octember, I just had the same experience. One alternative I went with is creating a subclass of Personalization that contains the dynamic template data property. It was pretty straight forward, not too hacky and seems to be working.

public class DynamicPersonalization extends Personalization {

    @JsonProperty("dynamic_template_data")
    public Map<String, String> dynamicTemplateData;

}

Something similar here: #446 (comment)

edit: my mistake, dynamicTemplateData is a property of Personalization and not Mail.

@hmeerlo
Copy link

hmeerlo commented Aug 1, 2018

I think it is strange that a company that has a SaaS proposition does not maintain its language integrations itself. You bring out a new v3 version of your API and you rely on the open source community to implement this. People pay for SendGrid, they may expect a maintained version of the library then as well. And it is just a small layer so I so no reason why SendGrid shouldn't maintain this themselves.

@thinkingserious
Copy link
Contributor Author

Hello @hmeerlo,

Thanks for the feedback, we love to hear from you and appreciate it!

All issues posted on our open source repos are indeed on our backlog to implement internally. The challenge is one of bandwidth at the moment, and fortunately we have an amazing community that contributes consistently, allowing us to serve you much faster than we could on our own.

To that end, work has begun on this issue (#449) and I suspect that PR will get merged shortly. If you have a moment, your feedback on that implementation would be amazing and further help speed this process :) If not, no worries. I appreciate that you already took the time to communicate your thoughts with us.

Thanks!

With Best Regards,

Elmer

@hmeerlo
Copy link

hmeerlo commented Aug 1, 2018

@thinkingserious that is good to hear, but in that case I don't understand why you release a new API version without having the supporting libraries ready. But hey, that's a business decision. I use the workaround suggested by @nickneiman for now (which works great).

@thinkingserious
Copy link
Contributor Author

Hello @hmeerlo,

It's a matter of determining priorities against a large backlog with many various customer needs. In this case, since there are working workarounds, the priority is reduced a bit vs. other items. Determining priorities correctly is a challenge, but I think our team's RICE method is the best prioritization system I've seen for our type of work at this scale. That said, we are always open to improvements and feedback. Additionally, we are bit more backlogged than usual, but we are doing our best to catch back up.

Thanks for following up and letting us know that the workaround worked for you. Comments like yours help the community and will likely save someone else time.

I don't think it will be long before the helpers are added to all of our SDKs. In the meantime, I thank you for your patience.

With Best Regards,

Elmer

@leebenson
Copy link

leebenson commented Aug 9, 2018

Come on guys, this ought to be done already. Issue reported 2 weeks ago, and it's still not possible to do a simple mail merge via the official SDKs?

(FWIW, I'm using the Python SDK, which also doesn't have it!)

@hpoul
Copy link

hpoul commented Aug 9, 2018

@leebenson try out #446 (comment) .. support is just 1 copy&paste away, works for me (TM)

@leebenson
Copy link

thanks @hpoul, but that's Java. I'm talking about adding official support across all SDKs, including Python and Ruby.

Besides - it's a band-aid. We shouldn't be monkey-patching an incomplete SDK that will soon change from under us. This is a really simple issue their own devs could complete in an hour.

@hpoul
Copy link

hpoul commented Aug 9, 2018

@leebenson well probably check with the python/ruby issue tracker then.. and to be fair, this is perfectly fine sub classing, monkey patching in java requires quite some effort... and fwiw, i'm just using sendgrid for a week, but I don't have the impression that their API libraries are really that sophisticated or have a high priority for them.. so I wouldn't hold my breath..

@leebenson
Copy link

leebenson commented Aug 9, 2018

@hpoul, I've reached the same conclusion. Probably best just to build the JSON request manually than rely on SDK abstractions. This is proof of SDK drift that I wouldn't want to risk relying on in the future.

@thinkingserious
Copy link
Contributor Author

Hi @leebenson,

Thanks for taking the time to voice your concerns as votes and comments help push the priority up our backlog. If you wish to contribute, additional eyes on this PR is appreciated.

Hi @hpoul,

Thanks for trying to help out, we appreciate your support!

With Best Regards,

Elmer

@michizhou
Copy link

@thinkingserious I would be willing to take on this issue. Just let me know what to do!

@thinkingserious thinkingserious added status: work in progress Twilio or the community is in the process of implementing and removed good first issue labels Oct 3, 2018
@thinkingserious thinkingserious removed hacktoberfest status: help wanted requesting help from the community labels Oct 3, 2018
@thinkingserious
Copy link
Contributor Author

Hi @michizhou,

My apologies, I had this issue mis-labeled, it is already under development here.

I do hope you will consider other contributions as part of our Hacktoberfest celebration.

Thanks!

With Best Regards,

Elmer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium fix is medium in difficulty status: work in progress Twilio or the community is in the process of implementing type: twilio enhancement feature request on Twilio's roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants