fix(#52,#54): Fix attribute synchronization for registered store attributes#55
fix(#52,#54): Fix attribute synchronization for registered store attributes#55mdayaram wants to merge 1 commit intopalkan:masterfrom
Conversation
…d store attributes Previously, registered store attributes could show inconsistent values between the store and Rails' attribute system, causing validation failures and incorrect behavior with methods like `attributes`, `*_before_type_cast`, and dirty tracking. This fix ensures proper synchronization between both systems. This PR introduces an `AttributesSync` module that maintains proper synchronization between store values and registered attributes by: - Overriding attribute accessors to ensure both systems stay in sync - Adding lifecycle callbacks to sync values after initialization and database loads - Fixing the `attributes` method to return actual store values instead of defaults - Ensuring proper type casting in both the store and attribute systems Fixes palkan#52 Fixes palkan#54
palkan
left a comment
palkan
left a comment
There was a problem hiding this comment.
Thanks for the fixes!
The PR confirms my original concerns regarding this feature.
The amount of hacks grows, and I want to be sure we do not interfere in any way with the stable functionality (no registered attributes). So, I'd like to have all the related functionality isolated as much as possible and only included when users opt-in for it.
See the comments for the details.
| _local_typed_stored_attributes[store_name][:types][name] = [type, options] | ||
|
|
||
| if store_attribute_register_attributes | ||
| _ensure_attributes_sync_module_included |
There was a problem hiding this comment.
Let's do this only if the feature is enabled (we probably can move the inclusion into the accessor method).
| end | ||
|
|
||
| def _define_store_attribute_accessors(store_name, name) | ||
| # Override getter to read from store |
There was a problem hiding this comment.
Do we need to override the getter? It's already define by the built-in store-accessor.
As I understand, we need to hack the writer to keep things in sync. For that, I would suggest having a dedicated module prepended with the sync code defined and a super call instead.
And this module should only be included if the feature is enabled.
2 participants
Previously, registered store attributes could show inconsistent values between the store and Rails' attribute system, causing validation failures and incorrect behavior with methods like
attributes,*_before_type_cast, and dirty tracking. This fix ensures proper synchronization between both systems.This PR introduces an
AttributesSyncmodule that maintains proper synchronization between store values and registered attributes by:attributesmethod to return actual store values instead of defaultsFixes #52
Fixes #54
Checklist