Skip to content

Commit

Permalink
Fix weird but necessary edge case in serializable_opts monkey-patch (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
gregorbg authored Jan 22, 2025
1 parent d0939aa commit d0f17dc
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
9 changes: 7 additions & 2 deletions app/models/person.rb
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,16 @@ def serializable_hash(options = nil)
# Passing down options from Person to User (which are completely different models in the DB!)
# is a horrible idea. Unfortunately, our external APIs have come to rely on it,
# so we need to hack around it.
# First, we check whether the developer specifically gave instructions about which `user` properties they want.
hash_include_specified = options&.dig(:include).is_a? Hash
user_pass_down_options = hash_include_specified ? options&.dig(:include, :user) : options
# Second, we need to apply some crazy merging logic:
# - `merge_union` makes sure that only values specified in USER_COMMON_SERIALIZE_OPTIONS kick in
# - `filter` makes sure that when the result of `merge_union` are empty, the defaults from
# User::DEFAULT_SERIALIZE_OPTIONS can override.
user_override_options = USER_COMMON_SERIALIZE_OPTIONS.merge_union(options&.stringify_keys)
.filter { |_, v| v.present? }
# However, if the developer explicitly specified that the `user` include should be empty, then respect that.
user_override_options = USER_COMMON_SERIALIZE_OPTIONS.merge_serialization_opts(user_pass_down_options&.with_indifferent_access)
.filter { |_, v| hash_include_specified || v.present? }

# If there's a user for this Person, merge in all their data,
# the Person's data takes priority, though.
Expand Down
11 changes: 9 additions & 2 deletions config/initializers/monkey_patches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,16 @@ def xss_aware_join(delimiter = '')
end

Hash.class_eval do
def merge_union(other = nil)
def merge_serialization_opts(other = nil)
self.to_h do |key, value|
[key, value & (other&.fetch(key.to_s, []) || [])]
# Try to read `key` from the other hash, fall back to empty array.
other_value = (other&.fetch(key.to_s, []) || [])

# Merge arrays together, making sure to respect the difference between symbols and strings.
merged_value = value.map(&:to_sym) & other_value.map(&:to_sym)

# Return the merged result associated with the original (common) key
[key, merged_value]
end
end
end
Expand Down

0 comments on commit d0f17dc

Please sign in to comment.