Skip to content
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

Prevent creating duplicate contacts in the contacts/batch/new #5396

Merged
merged 2 commits into from
Dec 21, 2017

Conversation

alanhartless
Copy link
Contributor

Q A
Bug fix? y
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

The api/contact/new endpoint will update a contact if a unique identifier field matched instead of creating a new. But the batch endpoint created all new contacts regardless. This PR makes the batch endpoint act the same as the single contact endpoint.

Steps to reproduce the bug:

  1. Post the following payload to api/contacts/batch/new
[
  {
    "email": "email1@email.com",
    "firstname": "BatchUpdate"
  },
  {
    "email": "email2@email.com",
    "firstname": "BatchUpdate2"
  },
  {
    "email": "email3@email.com",
    "firstname": "BatchUpdate3"
  }
]
  1. Post it again and notice that the three contacts will be created again.

Steps to test this PR:

  1. Delete the contacts then repeat. This time the second post will update the contacts instead of creating new.

@alanhartless alanhartless added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Nov 29, 2017
@alanhartless alanhartless added this to the 2.12.1 milestone Nov 29, 2017
@javjim javjim self-assigned this Dec 19, 2017
@javjim
Copy link

javjim commented Dec 20, 2017

Tested and workin properly, was able to reproduce bug, and verify fix

@javjim javjim added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Dec 20, 2017
@javjim javjim removed their assignment Dec 20, 2017
@escopecz escopecz self-assigned this Dec 21, 2017
Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as described 👍

@escopecz escopecz added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Dec 21, 2017
@escopecz escopecz merged commit 1a9d40e into mautic:staging Dec 21, 2017
@dbhurley dbhurley removed the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Dec 21, 2017
@alanhartless alanhartless deleted the batch-contact-unique-identifier branch January 5, 2018 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants