Skip to content

Commit

Permalink
Ensure get_fields always returns strings, not symbols, for consistency.
Browse files Browse the repository at this point in the history
  • Loading branch information
gregschmit committed Mar 1, 2023
1 parent 64491c7 commit 458464c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 11 deletions.
5 changes: 3 additions & 2 deletions lib/rest_framework/controller_mixins/models.rb
Expand Up @@ -91,9 +91,10 @@ def get_label(s)

# Get the available fields. Returning `nil` indicates that anything should be accepted. If
# `fallback` is true, then we should fallback to this controller's model columns, or an empty
# array.
# array. This should always return an array of strings, no symbols, and possibly `nil` (only if
# `fallback` is false).
def get_fields(input_fields: nil, fallback: true)
input_fields ||= self.fields if fallback
input_fields ||= self.fields&.map(&:to_s) if fallback

# If fields is a hash, then parse it.
if input_fields.is_a?(Hash)
Expand Down
12 changes: 5 additions & 7 deletions lib/rest_framework/filters.rb
Expand Up @@ -14,14 +14,12 @@ class RESTFramework::ModelFilter < RESTFramework::BaseFilter
# Get a list of filterset fields for the current action. Fallback to columns because we don't want
# to try filtering by any query parameter because that could clash with other query parameters.
def _get_fields
return (
@controller.class.filterset_fields || @controller.get_fields(fallback: true)
).map(&:to_s)
# Always return a list of strings; `@controller.get_fields` already does this.
return @controller.class.filterset_fields&.map(&:to_s) || @controller.get_fields(fallback: true)
end

# Filter params for keys allowed by the current action's filterset_fields/fields config.
def _get_filter_params
# Map filterset fields to strings because query parameter keys are strings.
fields = self._get_fields

return @controller.request.query_parameters.select { |p, _|
Expand Down Expand Up @@ -69,7 +67,7 @@ class RESTFramework::ModelOrderingFilter < RESTFramework::BaseFilter
# Get a list of ordering fields for the current action. Do not fallback to columns in case the
# user wants to order by a virtual column.
def _get_fields
return (@controller.class.ordering_fields || @controller.get_fields)&.map(&:to_s)
return @controller.class.ordering_fields&.map(&:to_s) || @controller.get_fields
end

# Convert ordering string to an ordering configuration.
Expand Down Expand Up @@ -119,10 +117,10 @@ class RESTFramework::ModelSearchFilter < RESTFramework::BaseFilter
# common string-like columns by default.
def _get_fields
if search_fields = @controller.class.search_fields
return search_fields
return search_fields&.map(&:to_s)
end

columns = @controller.class.get_model.columns_hash.keys
columns = @controller.class.get_model.column_names
return @controller.get_fields(fallback: true).select { |f|
f.in?(RESTFramework.config.search_columns) && f.in?(columns)
}
Expand Down
6 changes: 4 additions & 2 deletions lib/rest_framework/utils.rb
Expand Up @@ -150,11 +150,13 @@ def self.parse_fields_hash(fields_hash, model, exclude_associations: nil)
Rails.logger.warn("RRF: Unknown key in fields hash: #{k}")
end

return parsed_fields
# We should always return strings, not symbols.
return parsed_fields.map(&:to_s)
end

# Get the fields for a given model, including not just columns (which includes
# foreign keys), but also associations.
# foreign keys), but also associations. Note that we always return an array of
# strings, not symbols.
def self.fields_for(model, exclude_associations: nil)
foreign_keys = model.reflect_on_all_associations(:belongs_to).map(&:foreign_key)

Expand Down

0 comments on commit 458464c

Please sign in to comment.