Skip to content

Support for binary ids and rails custom fields#15

Open
pnegri wants to merge 1 commit intowbotelhos:mainfrom
iugu:fix_for_non_integer_ids
Open

Support for binary ids and rails custom fields#15
pnegri wants to merge 1 commit intowbotelhos:mainfrom
iugu:fix_for_non_integer_ids

Conversation

@pnegri
Copy link

@pnegri pnegri commented Apr 10, 2023

This fixes a problem when you try to use columns that aren't integer or string, also, when you use Rails 6+ Custom Field Types. When you call active model sanitizers, active model converts array of parameters by calling serializers for each model field/column, if you call "id" directly on these models, it will bypass the rails serializers and cast and will try to use the field.to_s as column value, resulting into a wrong value on the reference table.

Example 1:
class SampleModel
  # id as binary 4444, but displayed and A-B
end

Example 2:
class SampleModel
   attribute :id, :binary_uuid (displayed as: xxxx-yyyy-zzzz-wwwwwwww but saved as binary hex. Tries to save the xxxx-...)
end

Example 3:
class SampleModel
   attribute :id, :advanced_binary_uuid_with_short_uuid_support (displayed as: 6Y0xqMpcRdnpLwHVFjprkX but saved as binary hex. Tries to save 6Y0x instead of binary hex)
end

#{scope_where_query(resource)}
).squish

values = [sql, resource.class.base_class.name, resource.id]
Copy link
Author

Choose a reason for hiding this comment

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

Using .id will bypass rails field serialization

@wbotelhos
Copy link
Owner

Hey, @pnegri ,

I didn't receive notification about your PR, I'm sorry.

Do you think is it possible create a simple test to represent this case?

@wbotelhos
Copy link
Owner

Hi, @pnegri I didn't merge it yet because we need some kind of tests. As soon as I have time I'll study about this Rails feature that I never used so I can't understand the problem/solution for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants