[WIP] Update mailingLists type for Loops
This PR modifies the mailingLists field from an array of objects to a single object. Since users can't manually provide objects for the array, the previous structure was not very useful. It's unlikely that event payloads would include an array of mailingLists objects by default.
With this update, users can now easily input multiple mailling list IDs when creating a mapping, which will look like the following:
This is more in line with the request parameters outlined in Loops API docs.
Considerations
I did run some queries to see if any users are actively making use of mailingLists with the action currently and there is one customer that is. That being the case we need to either:
- Create a new v2 action
- Add an additional field (one that uses array of objects, and one that is just an object)
- Notify the customer that is making use of this field of the change they will need to put in place
- while this option is not desirable, the use may be happy to accomodate the change as they are using a hacky work around to get the mailing lists ids that they want in the mapping
Testing
Include any additional information about the testing you have completed to ensure your changes behave as expected. For a speedy review, please check any of the tasks you completed below during your testing.
- [ ] Added unit tests for new functionality
- [ ] Tested end-to-end using the local server
- [ ] [Segmenters] Tested in the staging environment
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 82.94%. Comparing base (
d0f4128) to head (44474d3). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #2414 +/- ##
==========================================
+ Coverage 82.91% 82.94% +0.02%
==========================================
Files 943 944 +1
Lines 16437 16460 +23
Branches 3088 3098 +10
==========================================
+ Hits 13629 13652 +23
Misses 2614 2614
Partials 194 194
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks for this Thomas. We'll try it out.
How can we also test the UI you show in your screenshot? Action Tester has a different UI
How can we also test the UI you show in your screenshot? Action Tester has a different UI
@danrowden Afraid the actions tester won't mock the production UI accurately. That said the "Custom Attributes" field is the same type, so it will look the same in the UI:
Only difference is that the defaultUI option for mailingLists will default to showing the "Edit Object" pane of the mapping
This is what I see in Actions Tester. There is no way for me to add custom values to the right side of = here. Am I missing something in the set up to get it to work?
@danrowden here is a video of me inputting the key values: https://www.loom.com/share/928e1ca07771436989d466de9f5362ee?sid=b5e39c08-061e-47c6-a425-73df8a893093
if it helps happy to jump on a quick call to talk it through, lmk.
Thanks for this! That helps
@tcgilbert And just to confirm, users can send in mailing list data in an identify() call eg https://loops.so/docs/integrations/segment#sending-data-to-segment
For our existing customer on the current version, would this require adding a new field like mailingListData (named different), ask the customer to switch over and then remove the existing mailingLists in a future update?
@tcgilbert And just to confirm, users can send in mailing list data in an
identify()call eg https://loops.so/docs/integrations/segment#sending-data-to-segment
@danrowden yes, they could send with an identify call. If so it would likely be included in the traits object:
"traits": {
"mailingLists": {
"list_id_1": true
}
}
The user would have the flexibility to map this object to Mailing lists field as they see fit with the drafted option
For our existing customer on the current version, would this require adding a new field like
mailingListData(named different), ask the customer to switch over and then remove the existingmailingListsin a future update?
Yes, we'd need to leave the current field as is for now, and not remove it until we could confirm it is not in use. The other option would be to create a cloned action with the name "v2", that has the updated mailingList field as an object.
@tcgilbert We have checked our logs and haven't found any recent activity for calls from Segment containing mailingLists data (anything with that key have an empty array). We are happy to make this change ASAP so it can roll out. We'd really like to avoid creating a new V2 action
@tcgilbert We have checked our logs and haven't found any recent activity for calls from Segment containing
mailingListsdata (anything with that key have an empty array). We are happy to make this change ASAP so it can roll out. We'd really like to avoid creating a new V2 action
Hey @danrowden - @joe-ayoub-segment and I discussed, and we think it will fine to simply overwrite the existing field. If you have tested my recommendation and it works for you I can re-open this PR.
Otherwise, feel free to take this as a reference and submit your own PR that updates the maillingLists field
@tcgilbert I tested sending data in traits, both mapping them in the UI and without mapping and found some things to fix, which I've done in this PR: https://github.com/segmentio/action-destinations/pull/2464
thanks @danrowden. I see what you mean about the mailingLists in traits now. I've added my review here and will close this one out.