-
Notifications
You must be signed in to change notification settings - Fork 283
[Stackadapt Audiences] - making marketing_status for non Audience payloads #3331
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
base: main
Are you sure you want to change the base?
Conversation
New required fields detectedWarning Your PR adds new required fields to an existing destination. Adding new required settings/mappings for a destination already in production requires updating existing customer destination configuration. Ignore this warning if this PR is for a new destination with no active customers in production. The following required fields were added in this PR:
Add these new fields as optional instead and assume default values in |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3331 +/- ##
==========================================
- Coverage 80.32% 79.97% -0.35%
==========================================
Files 1257 1204 -53
Lines 25159 22306 -2853
Branches 5217 4410 -807
==========================================
- Hits 20208 17839 -2369
+ Misses 4163 3685 -478
+ Partials 788 782 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hi @joe-ayoub-segment , I just checked with our internal team and realize that they are using a different API field for the marketing status so the following change is needed: I have tested the above change with your PR and it's working. |
Hi @Vanessa-SSY I've updated the PR as per your instructions. |
Looks good to me. Thanks! |
Hi @joe-ayoub-segment, we're doing end-to-end testing and not seeing the default mapping auto-populated for the standard properties(see screenshot1). The top 3 were defined manually by us. I see the fields mapped correctly in action tester (see screenshot 2) so I'm not able to re-produce the issue in dev environment. Could you help look into this issue? Is it possible to meet today's deployment if any change is needed? |
@harsh-joshi99 May I know why auto-merge is disabled? Can we still get this PR in deployment today? |
@Vanessa-SSY indicated that
marketing_status should be in common-fields.ts
Testing
To be tested in Production by @Vanessa-SSY
Unit tests updated.