Updates group to accept new group settings

[ladanazita]

Amplitude allows customers to send multiple types of groups on the same event, for example
group_type: org name
group_value: engineering

group_type: org id
group_value: 1234

The new Segment settings groupTypeTrait (group type) and groupTypeValue (group value) allow the user to set which keys the Segment integration will look for to determine what to set for group name and group values in Amplitude.

We will default to legacy behavior, which was incorrect prior to this PR (incorrectly setting group type as [Segment] Group and group.id to group name)

[SegmentBot]

By analyzing the blame information on this pull request, we identified and undefined to be a potential reviewer.

[f2prateek]

extra line?

[f2prateek]

extra '?

[f2prateek]

What happens if groupTypeTrait is not set?

[ladanazita]

It will default to the old behavior. Or are you saying to check if the value for this setting exists?

[f2prateek]

Not quite. I was wondering what would happen at payload.traits objectForKey:groupTypeTrait if groupTypeTrait is null here.

[ladanazita]

It will just default to the original behavior

[f2prateek]

could be payload.traits[groupTypeTrait]?

[f2prateek]

I don't understand why this is an if check.

What's the difference between these two pieces of code:

NSString *groupTypeTrait = self.settings[@"groupTypeTrait"];

NSString *groupTypeTrait;
if (self.settings[@"groupTypeTrait"]) {
  groupTypeTrait = self.settings[@"groupTypeTrait"];
}

They seem functionally equivalent to me.

[ladanazita]

I misunderstood your original question then about "What happens if groupTypeTrait is not set?"

will revert

[brennan]

This test looks ok, but I think it's a bit confusing since groupType:groupTrait should follow the pattern general:specific. Here, looks like specific:general (i.e. Segment:Technology).

[ladanazita]

makes sense, changed 👇

[f2prateek]

This block:

    NSString *groupTypeTrait = self.settings[@"groupTypeTrait"];
    NSString *groupTypeValue = self.settings[@"groupTypeValue"];
    NSString *groupName;
    NSString *groupValue;

    if (payload.traits[groupTypeTrait] && payload.traits[groupTypeValue]) {
        groupName = payload.traits[groupTypeTrait];
        groupValue = payload.traits[groupTypeValue];
    } else {
        groupName = payload.traits[@"name"] ?: @"[Segment] Group";
        groupValue = payload.groupId;
    }

Is likely more readable and less error prone as:

    NSString *groupTypeTrait = self.settings[@"groupTypeTrait"];
    NSString *groupTypeValue = self.settings[@"groupTypeValue"];
    NSString *groupName = payload.traits[groupTypeTrait];
    NSString *groupValue = payload.traits[groupTypeValue];

    if (!groupName || !groupValue) {
        groupName = payload.traits[@"name"] ?: @"[Segment] Group";
        groupValue = payload.groupId;
    }

[ladanazita]

#47