Tricks and regrets re:update_change

Chat / Discussions
,
1

Just want to share the bug I caused and spent too long debugging.

I got this order item that has amount (required amount) and amount_shipped on it. You know, sometimes shipping isn’t done in a single batch.

Instead of writing a plain and simple context function, I thought it was a good idea to use a changeset with an update_change to streamline the process and the client only has to supply a new value to amount_shipped, thus avoiding a controller action, a context function and potentially a virtual field.

So I ended up with something like this:

  def changeset(order_item, %{amount_shipped: _} = attrs) do
    order_item
    |> cast(attrs, [:amount_shipped])
    |> update_change(:amount_shipped, &(&1 + order_item.amount_shipped))
  end

The atom key amount_shipped in pattern matching is just because this particular changeset clause is only used with processed data. Irrelevant here.

This approach worked relatively well, until some point where a weird bug was reported. Sometimes, the amount_shipped is not updated.

This took me way too long than it should have taken to figure out… The bug turned out to be simple, if 500 was already shipped and you ship another 500, this is not a “change” and does not trigger update_change.

I could have taken the chance to rewrite it back to plain and simple albeit more verbose solutions… :wink: but I ended up with this fix:

  def changeset(order_item, %{amount_shipped: amount_shipped} = attrs) do
    order_item
    |> cast(attrs, [:amount_shipped])
    # in case same amount
    |> force_change(:amount_shipped, amount_shipped)
    |> update_change(:amount_shipped, &(&1 + order_item.amount_shipped))
  end

Not promoting the trick or anything. Just sharing some head-scratching fun I had.

6 Likes
2

Tbh that sounds like the more appropriate solution to me. The customer is no longer submitting the schemas fields. They’re submitting deltas. The way to handle that would imo be validating their input based on a schema for the input. Then do the business logic of applying the deltas and use the result as the changes for the persisted data.

E.g. validating :amount_shipped to be >= 0 has completely different meaning applied to the delta send by the customer than it has to the resulting total on the schema. Trying to conflate the two will imo only be a source of confusion in the future.

A more viable shortcut could imo be an virtual fields for :amount_shipped_delta vs the :amount_shipped total.

6 Likes
3

Yup, totally agree. A virtual field would be more appropriate.

It’s a low usage in-house project so I prioritized fun and experimenting with different solutions, instead of a simple and easy to understand one.

Over the years I had some good fun with update_change. Given that a field is never directly writable by a user, I have done things

  1. like this one, using it to avoid an extra delta and kinda auto sum.
  2. on password field I do update_change(:password, &Bcrypt.hash_pwd_salt/1), so I don’t have a virtual password and a materialized hashed_password

A lot of fun.

4

That’s a fun bug, thanks for sharing!

Out of curisoity, could the force_change trick open the door to the opposite problem? For example, 500 was already shipped and user updates an unrelated field or “messes” with :amount_shipped field without actually meaning to change it, then sees an unexpected doubling of :amount_shipped.

To add to the point about deltas made by @LostKobrakai, tracking each delta would also be useful for auditability down the line. Overwriting :amount_shipped means you won’t have the information necessary to retroactively support any feature requests to display amount shipped over time.

5

Hi, glad you found it interesting.

To answer this, I have to come back to quote my own text in the post first:

  1. I have multi clause changeset functions, this clause is only dealing with :amount_shipped and has no chance confusing with other fields
  2. This clause has atom key pattern matching, which means it is only explicitly used in the program, and it cannot be called with user input directly

This is a very good point. I didn’t show everything here. I only showed order and order-items here, there are actually associated shipping and shipping-items. When a shipping is approved, it updates the items in its parent order with its shipping-items. History can be constructed by querying all the shippings relevant to an order, and details are in their items :slight_smile:

And you probably guessed by now, each order-item is updated with amount_shipped from the according shipping-item’s amount.

1 Like