Skip to content

Conversation

@thewheat
Copy link
Contributor

@thewheat thewheat commented Jun 1, 2018

Fix for #197

  • Currently library fails tagging users via Tag.tag(tag, user);
    • if the user object has both user_id and email
    • and the email is used on multiple records
  • Due to code below where we prioritise email over user_id
    if (!Strings.isNullOrEmpty(id)) {
    userMap.put("id", id);
    usersLite.add(userMap);
    } else if (!Strings.isNullOrEmpty(email)) {
    userMap.put("email", email);
    usersLite.add(userMap);
    } else if (!Strings.isNullOrEmpty(userId)) {
    userMap.put("user_id", userId);
    usersLite.add(userMap);
    } else {
    logger.warn("no identifiers found for user tag target, skipping [" + tag + "] [" + user.toString() + "]");
    }

Current library would fail on this

        User user = new User().setUserId(USER_ID).setEmail(EMAIL_THAT_IS_ON_MULTIPLE_RECORDS);
        Tag tag = new Tag().setName("test-tag");
        tag = Tag.create(tag);
        Tag.tag(tag, user);

image

Essentially the code above would create this API request

{
  "name": "test-tag",
  "users":[
    {
      "email" : EMAIL_THAT_IS_ON_MULTIPLE_RECORDS
    }
   ]
}

Instead of

{
  "name": "test-tag",
  "users":[
    {
      "user_id" : USER_ID
    }
   ]
}

Reshuffling so user_id has precendence over this it will fix it

@keloe keloe self-requested a review July 20, 2018 17:09
@thewheat thewheat merged commit aec0278 into master Jul 21, 2018
@thewheat thewheat deleted the timlim/fix-tagging-with-multiple-identifiers branch July 21, 2018 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants