[Bug]: Missing support for critical email features

[habovh]

What's on your mind?

[Bug]: Missing support for critical email features

Missing support for the following API parameters:

Generic:

• custom_data

Mail type specific:

• include_unsubscribed
• disable_email_click_tracking

Lack of these features makes this SDK unusable for any kind of transactional email setup containing sensitive data such as password reset tokens. Setting these to the notification object results in the SDK stripping them away and eventually not sending them to the API.

Only solution is to resort to using fetch or similar tools to fetch data and completely lose type safety.

In my opinion this is a high priority issue that should be addressed, but I won't wait for an update nor contribute to the code: if I had the time to do so I would not be looking to use a third-party for my email and notifications needs. I expect paid software SDKs to work, especially for mature companies such as OneSignal. Guess I'm wrong, wasted my time, and I'll be looking elsewhere.

Code of Conduct

• I agree to follow this project's Code of Conduct

[emawby]

@habovh Hello thank you for reaching out we will investigate adding these parameters

[mikeislearning]

I'm having this issue as well, I'm unable to pass dynamic data into the template, which I assume is what the custom_data parameter would be used for. I've also tried the data parameter, but that doesn't work either.

UPDATE: Turns out I had to use {{ message.custom_data.data_field_here }} in my template instead of just using {{ data_field_here }}

[CHC383]

Hi, include_unsubscribed is available in the Notification class in SDK 5.0.0-alpha-02, but disable_email_click_tracking is still not available, and I don't see it in the coming changes in the api branch, is this missed?

@sherwinski for visibility

[CHC383]

I have created a PR for this: #95

In case anyone needs it sooner before the PR is merged and released, this is the pnpm patch I used to patch the code locally

diff --git a/dist/models/Notification.d.ts b/dist/models/Notification.d.ts
index 7e906c43795695db5cb00b88306b6d10f01976df..fa77fe924969a189f971d270af4e40493ecff056 100644
--- a/dist/models/Notification.d.ts
+++ b/dist/models/Notification.d.ts
@@ -113,6 +113,7 @@ export declare class Notification {
     'filters'?: Array<FilterExpression>;
     'custom_data'?: object;
     'send_after'?: string;
+    'disable_email_click_tracking'?: boolean;
     static readonly discriminator: string | undefined;
     static readonly attributeTypeMap: Array<{
         name: string;
diff --git a/dist/models/Notification.js b/dist/models/Notification.js
index fe0ca0f4c37cc4b4eb78a1594bfd364bcc0a5455..1926237359b95d26d2999227c950a0ca09c21e55 100644
--- a/dist/models/Notification.js
+++ b/dist/models/Notification.js
@@ -650,6 +650,12 @@ var Notification = (function () {
             "baseName": "send_after",
             "type": "string",
             "format": "date-time"
+        },
+        {
+          "name": "disable_email_click_tracking",
+          "baseName": "disable_email_click_tracking",
+          "type": "boolean",
+          "format": ""
         }
     ];
     return Notification;

[sherwinski]

Thanks for reporting this issue @habovh (and for the subsequent PR @CHC383)!

It looks like these parameters are missing in an upstream project used to autogenerate the code in this SDK. I'm currently working on some improvements to that project (hence the recent pushes to the api branch) so please allow me some time to wrap that up before investigating this further.

Unfortunately for that reason I won't be able to merge #95 without overwriting those changes in future releases. Apologies @CHC383 and thanks again for putting that together.

[CHC383]

Thanks for reporting this issue @habovh (and for the subsequent PR @CHC383)!

It looks like these parameters are missing in an upstream project used to autogenerate the code in this SDK. I'm currently working on some improvements to that project (hence the recent pushes to the api branch) so please allow me some time to wrap that up before investigating this further.

Unfortunately for that reason I won't be able to merge #95 without overwriting those changes in future releases. Apologies @CHC383 and thanks again for putting that together.

@sherwinski Understood, I already have the feeling that the SDKs are autogenerated and they need to be fixed in the upstream, I will close the PR and waiting for your improvements, in the meantime there is already a dirty workaround to patch the SDK by the users above.