Codecov Report

Merging #593 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
+ Coverage   86.61%   86.65%   +0.04%     
==========================================
  Files          32       32              
  Lines         979      982       +3     
  Branches      131      132       +1     
==========================================
+ Hits          848      851       +3     
  Misses         67       67              
  Partials       64       64

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ac58c6...e1cc256. Read the comment docs.

Thank you so much for your PR @3lnc!
This is a great PR and it covers all the aspects required by the specifications.

Hello @3lnc,

Thanks again for the PR!

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

Team SendGrid DX

Should this not also remove the use of Substitution from mail_example.py since this has been deprecated?

I also think this is a different type of interface to existing personalizations etc so could be confusing to newcomers as to why dynamic_template_data is added differently, i.e. setting the attribute directly on the personalization rather than using a setter like all other attributes?

Just my thoughts...

@djw27 by the time this PR was opened I had no info on deprecation of substitution. Could you please provide reference for this info?

Regarding descriptors interface ­— personally, I really dislike the current state of codebase being overcrowded with getters. This is really not pythonic way, as there's no setters with custom logic. Hiding attributes with _name + @property is actually gives nothing but pain — in case you need to modify helper objects dynamically. But that's IMO only.

Regarding "raw" implementation of dynamic_template_data — this is conscious decision. Dynamic templates requires json. Which is pretty "technical". I mean, if json is needed for handlebars, you probably know what json is and how to map json into python datastructures. All the validation we can do on python side is done while serializing.

Making this interface via setters will require class(es) with recurrent references — all this just to make nested structure. Taking into account that data classes is almost wrappers around dictionaries — I'm pretty sure that raw implementation is better way.

Thanks for the feedback @djw27 and @3lnc!

I'm currently working on an update to this SDK where your feedback would be greatly helpful. If you have time, please take a look.

With Best Regards,

Elmer

@3lnc I 100% agree with your points! I don't blame you not knowing that substitutions are being deprecated, I stumbled across the documentation by chance for dynamic templates: https://dynamic-templates.api-docs.io/3.0/mail-send-with-dynamic-transactional-templates/v3-mail-send

I guess it's a wider question for @thinkingserious, but at stage it would be pretty difficult to completely overhaul the interface used for this library. I'll take a look at your link in the mean time!

Hello @djw27,

I'm looking forward to your feedback on the SDK update. Thanks!

With Best Regards,

Elmer

@thinkingserious Are substitutions still being deprecated? I remember reading this a few months back. However, now I can't find any reference to this (including on the link referenced above).