[DC-792] injects matchingKey in the perform block #2119
Conversation
| description: 'The user ID', | ||
| type: 'string', | ||
| required: true, | ||
| category: 'identifier' |
There was a problem hiding this comment.
Still trying to understand the multiple identifiers case, suppose we had an additional identifier field 'email' here. Then we would automatically push a 'Record Matching' dropdown field that users would pick userID or email from, and depending on that the matchingKey would pull the correct value from the payload for the perform block? Is that a good understanding?
There was a problem hiding this comment.
Still trying to understand the multiple identifiers case, suppose we had an additional
identifierfield 'email' here. Then we would automatically push a 'Record Matching' dropdown field that users would pick userID or email from, and depending on that thematchingKeywould pull the correct value from the payload for the perform block? Is that a good understanding?
@nick-Ag Exactly 💯 . So in the UI, the selected field out of all the identifier fields defined would have been stored in mapping['__segment_internal_matching_key'], which is exactly what matchingKey is set to.
| @@ -197,6 +197,13 @@ const isSyncMode = (value: unknown): value is SyncMode => { | |||
| return syncModeTypes.find((validValue) => value === validValue) !== undefined | |||
| } | |||
|
|
|||
| const INTERNAL_HIDDEN_FIELDS = ['__segment_internal_sync_mode', '__segment_internal_matching_key'] | |||
There was a problem hiding this comment.
Nice! more tidy than before 👏
| @@ -297,6 +302,10 @@ export class Action<Settings, Payload extends JSONLikeObject, AudienceSettings = | |||
|
|
|||
| const syncMode = this.definition.syncMode ? bundle.mapping?.['__segment_internal_sync_mode'] : undefined | |||
|
|
|||
| const matchingKey = Object.values(this.definition.fields).find((field) => field.category === 'identifier') | |||
There was a problem hiding this comment.
Safe to remove this check to simplify? Can we just make an attempt to pass bundle.mapping?.['__segment_internal_matching_key'] ... I think will be undefined if the destination does not support record matching anyways.
There was a problem hiding this comment.
Safe to remove this check to simplify? Can we just make an attempt to pass
bundle.mapping?.['__segment_internal_matching_key']... I think will be undefined if the destination does not support record matching anyways.
Yeah I guess I was trying to be defensive lol. By the way, do you think there should be validation to make sure that the value in bundle.mapping?.['__segment_internal_matching_key'] is one of the identifier fields?
I'm on the fence. On one hand the UI is unlikely to cause invalid values but on another hand, things happen e.g someone can save a bad mapping via PAPI?
There was a problem hiding this comment.
I considered removing the condition to simplify future changes. If we decide to change the field category used as the indicator for matching keys, fewer parts of the codebase will need to be updated.
There was a problem hiding this comment.
Also no need for validation since even if someone creates a __segment_internal_matching_key with bad value it won't cause any significant issues in the perform block
|
hi @konoufo PR deployed |
This PR adds the ability to access user-selected record matching field in the perform block. The
matchingKeyis the field key for one of the action fields of categoryidentifier. The record matching field is meant to be selected via a dropdown in the mapping UI for RETL (upcoming work). See the relevant section of the blueprint for more context.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.