TurtleAI / derive

An Event Sourcing and CQRS solution.
0 stars 1 forks source link

Ensure `merge` doesn't try to set missing fields to nil #24

Open venkatd opened 1 year ago

venkatd commented 1 year ago

The following code:

    merge({Turtle.Payments.Models.PaymentIntent, e.payment_intent_id},
      status: :processing,
      amount: e.amount,
      payment_method_id: e.payment_method_id,
      payment_method_type: String.to_atom(e.payment_method_type),
      payment_method_brand: e.payment_method_brand,
      payment_method_last4: e.payment_method_last4
    )

Fails in my codebase because it's not passing along the created_at field. It's trying to set the created_at to null but that probably means there's a bug in which missing fields in the struct are mistakenly set.

I need to write a test case to reproduce this behavior

rwillians commented 1 year ago

Hm it looks like merge/1 and replace/1 does the same thing.

If you are upserting a record, all required fields must be given. The intention in this use case reads better using the upsert/1 signature, as I mention here https://github.com/TurtleAI/derive/issues/19#issuecomment-1366821630

If you are simply updating fields in an existing record, then update/2 is the more intuitive option. Afaik update/2 doesn’t have the bug in question.