Change in how `Ash.Changeset.force_change_attributes` handles `nil` values?
i'm currently in the process of upgrading to the latest version. there seems to have been a change in how
Ash.Changeset.force_change_attributes handles nil values?
at least, that what i seem to be observing.
here's a simple example:
%JdlEngine.Hubspot.Contact{} |> Ash.Changeset.for_update(:update, %{}) |> Ash.Changeset.force_change_attributes(%{ city: "vice city", }) - this then has the updated value in the attributes (see screenshot 1: attributes: %{city: "vice city"})
%JdlEngine.Hubspot.Contact{} |> Ash.Changeset.for_update(:update, %{}) |> Ash.Changeset.force_change_attributes(%{ city: nil, }) - this then has no updated values in attributes (see screenshot 2 - attributes: %{})
why is that important for me?
we implement our own data layer for hubspot resources and i have one use case where i want to forcefully overwrite data and even set attributes to nil if that should be the case.
in the data layer implementation i'm currently relying on attributes to detect what attributes i need to update on hubspot. see screenshot 3, the cast_changeset-implementation where i use attributes to then build a map of changes. maybe that is the wrong approach entirely and i should change that, but that behaviour seems to have changed now since the attributes will not contain the e.g. city: nil anymore.
9 Replies
Hm...wouldn't that mean you can never use force change attribute to change something to
nil?
Oh
It's because you're passing in an empty record
Well, maybe not. In the example you showed you just do %Thing{}
Is that just a placeholder for a real update?Yes, that's just for the example. in the "real world" i have a record at hand that i'm trying to update.
Does it currently have
city as nil?
How far back are you upgrading from?
Regardless, if you can reproduce force_change_attribute not recording a change then it's definitely a bug. However, force_change_attribute doesn't record changes for attributes where it's not changing.Upgrading from 3.4.23 if I‘m not mistaken (sorry, currently on the road. Will check back tomorrow with more details).
I have a test suite in our code base where it seems to behave differently since the upgrade.
I will try to reproduce an example in the ash repo itself
However, force_change_attribute doesn’t record changes for attributes where it’s not changingDid that change? What’s the difference to a simple change_attribute then? Only the fact that it skips validations etc?
I think it changed quite a long time ago, I couldn't say which version. Now it essentially is just bypassing the warning about setting attributes in hooks, and writability checks
It doesn't bypass validations, it's just most commonly used in before/after action hooks when validations have already occurred
I have created some tests that show the behaviour changes I mentioned here:
- branched out and based on
v3.4.23 https://github.com/ash-project/ash/pull/2141 - tests pass
- branched out and based on main https://github.com/ash-project/ash/pull/2142 - 2nd test fails
I want to emphasize this again: "Note: this PR does not claim that force_change_attribute should behave as the tests expect it to behave!"
It's just something I observed.
My goal simply is to reset values, and in the best case I don't need to fetch the actual data of the resource from the data layer because I know upfront how the data needs to be reset and overridden in my use case. Therefore, the previous behaviour was a nice to have as I could simply change it to nil and it would be set to nil in the attributes mapInteresting. So what does your code to actually do that look like? Make an empty struct with just the id and pass that to update?
We can potentially handle that by detecting if the record in question was loaded from the db or not, we have that info
If not, we can consider everything as having been changed
My use case is „merge contacts on HubSpot“ (we have a HubSpot data layer)
Little bit tricky to explain but I’ll try.
We have contacts in our database. Each contact is linked to a contact on HubSpot.
When we merge contacts, we first merge them in our database.
Then, we merge the contacts on HubSpot.
However, the merge logic is little bit different than ours. So, to be sure the mirrored contact looks like the contact in our database, we simply override the all properties on HubSpot again.
Now, to do so, we don’t need to fetch the latest data again from HubSpot. We have a resource loaded with the correct ID. We know what attributes should be written and just simply go ahead and, so far, used „force_change_attributes“ and updated the resource. This then also ensured that an attribute was cleared (i.e. nil-ed) even if the merge process on HubSpot would have left the attribute with another value
I could simply refetch the resource from HubSpot, though and then the changes should be registered since a change is then detected
But I wanted to bring up this topic as i don’t know if that was an intend behavior change or not
Right, so I think your use case would be fixed by, when checking if some attribute is changed, we just factor in
changeset.data.__meta__.state
it would be an easy change and would solve your problem
basically, if changeset.data.__meta__.state == :built then we consider it to have been changed