Skip to content

Fix #294 contact merging

ayduns requested to merge ayduns/stripe:fix_merge into 6.8

If a contact with a Stripe subscription is merged into another contact and the original deleted, the Stripe-related data is not updated resulting in later failures and confusion.

As identified in #294 (closed), there are 2 issues:

  1. the StripeCustomer table is not updated with the new contact_id
  2. the metadata at Stripe is not updated with the new contact_id and URL

This PR addresses both of these:

  1. The standard core merging process can handle the change of contact_id automatically providing the appropriate Foreign Keys are defined in the schema XML. The first commit retrofits a schema XML along with DAO/BAO. It also has the benefit of providing a standard CRUD APIv4 implementation. (More could be done to make use of this but is beyond the scope of fixing merging.)
  2. The third commit adds a post-merge hook to update the metadata of all Stripe Customers linked to this new contact_id.

The second commit adds a test that the StripeCustomer table is updated. It does not test that the metadata is correctly updated at Stripe.

It would be useful to get a few more eyes on this, particularly around edge cases.

  1. Is there any other data that needs to be updated as a result of a merge?
  2. If both contacts involved in a merge have entries in the StripeCustomer table, the result will be multiple customer ids linked to one contact id. This reflects the reality of the situation but are there any situations assuming one contact id links to one customer id?

Reviewer note: for the first commit, the main focus is the schema XML in StripeCustomer.xml and the resulting table creation statements in customers_install.sql The rest is generated automatically by civix from the xml.

  1. the original table create uses int(10) instead of int. Current practice mostly seems to just use int. Do we want to keep int(10)?
  2. there is a foreign key section for processor_id commented on in the XML since currently there is no FK. Is there any reason NOT to add a FK here?
Edited by ayduns

Merge request reports

Loading