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 committed Jan 21, 2025
1 parent 9ace921 commit 347d86c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
17 changes: 15 additions & 2 deletions app/models/person.rb
Original file line number Diff line number Diff line change
Expand Up @@ -253,10 +253,18 @@ def completed_solves_count
results.pluck("value1, value2, value3, value4, value5").flatten.count { |value| value > 0 }
end

def competition_count
competitions.count
end

def gender_visible?
%w(m f).include? gender
end

def visible_gender
gender_visible? ? gender : nil
end

def self.search(query, params: {})
persons = Person.current.includes(:user)
query.split.each do |part|
Expand Down Expand Up @@ -317,11 +325,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 key
[key, value & merged_value]
end
end
end
Expand Down

0 comments on commit 347d86c

Please sign in to comment.