parcelvoy / platform

Parcelvoy: Open source multi-channel marketing automation platform. Send data-driven emails, sms, push notifications and more!
https://parcelvoy.com
MIT License
261 stars 47 forks source link

Push notifications deeplink not wrapped #499

Closed reaper closed 1 month ago

reaper commented 2 months ago

Hi,

I'm currently trying to track push notification opens by following the documentation and integrating the Parcelvoy library on iOS. However, I noticed that the URL passed to the native side is not wrapped, which prevents the native library from sending the HTTP request to the Parcelvoy endpoint to track the push notification opening.

After checking the platform code, I found that the paramsToEncodedLink function (https://github.com/parcelvoy/platform/blob/a180ec4ced2ef20409088d272402fad380cacd6e/apps/platform/src/render/LinkService.ts#L22C14-L22C33) is only used in the subscription service to handle the unsubscribe email link (https://github.com/parcelvoy/platform/blob/a180ec4ced2ef20409088d272402fad380cacd6e/apps/platform/src/subscriptions/SubscriptionService.ts#L146).

I plan to create a pull request to wrap push notification deeplinks before sending them to the services. However, I wanted to check first: is there a specific reason this hasn’t been done yet, or is there another recommended approach?

Thank you!

pushchris commented 2 months ago

Overall definitely an addition that could be added! Had mostly been intending that field to be used for deeplinks but you're right, you can open it up for anything if you wanted and wrapping would allow for tracking.

This would be the way I currently think about link wrapping: Email: Zero problems wrapping, though some people opt out because they are wrapping themselves or are having issues with mobile links SMS: Never should be wrapped, takes up too many characters and looks super spammy Push Notification: 50/50 on if it should be wrapped. If you do wrap, it becomes a universal link which then requires the person implementing the app to have set that up correctly (including an assetlinks.json file for Android and a apple-app-site-association for iOS). This can be more work than some people want to do, especially if they are just using deeplinks for navigation and are tracking events at the destination of the navigation. Webhook: Never, mostly a raw data source

Currently the boolean toggle for link wrapping is universal, it might be that we should switch that to allow other integer values and make it be a segmented control to allow turning on for one or both of those sources (0 = off completely, 2 = both, 3 = email, 4 = notification or something)

The thing I would want to triple check in any implementation is that a link wrapped deep link still works properly. It should, but just something to check!