feat(datasource-pylon): custom fields introspection (EXT-10)

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client.rb

@@ -98,6 +98,21 @@ def fetch_team(id)
  fetch_resource('teams', id)
  end
 
+ # The custom-field definitions of one object type. `object_type` is
+ # mandatory on this endpoint, so a schema spanning several collections costs
+ # one call per collection rather than one call in total.
+ #
+ # Degrades to an empty list: this is read while the agent boots, and a token
+ # missing the permission — or a Pylon that happens to be down right then —
+ # has to cost the operator the custom columns, not the whole datasource.
+ def fetch_custom_fields(object_type)
+ params = { 'object_type' => object_type }
+
+ best_effort("fetch_custom_fields(#{object_type})", default: []) do
+ must_succeed('custom-fields') { collect_pages('custom-fields', params) }
+ end
+ end
+
  private
 
  def search_resource(path, limit:, cursor: nil, filter: nil, search_text: nil)
@@ -126,15 +141,19 @@ def fetch_all(path, params = {})
  # sent. `CursorWalker` answers the other question — the offset/limit window a
  # list view asks for — and is not what this needs.
  #
+ # `params` ride along on every page, cursor included: a mandatory parameter
+ # dropped on the second request answers a different question than the first.
+ #
  # An empty page and a cursor that does not move both stop the loop: neither
  # happens today, but a walk driven by a remote value stops on its own terms.
- def collect_pages(path)
+ def collect_pages(path, params = {})
  records = []
  cursor = nil
  pages = 0
 
  loop do
- page = to_search_page(connection.get(path, cursor.nil? ? {} : { 'cursor' => cursor }).body)
+ query = cursor.nil? ? params : params.merge('cursor' => cursor)
+ page = to_search_page(connection.get(path, query).body)
  records.concat(page.records)
  pages += 1
  break if page.next_cursor.nil? || page.next_cursor == cursor || page.records.empty?

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/record_serialization.rb

@@ -13,7 +13,7 @@ def nested_id(value)
  def add_custom_field_values(record, values)
  custom_fields.each do |cf|
  entry = values.is_a?(Hash) ? values[cf[:column_name]] : nil
- record[cf[:column_name]] = custom_field_value(entry)
+ record[cf[:column_name]] = coerce_custom_field(custom_field_value(entry), cf[:schema].column_type)
  end
  end
 
@@ -24,6 +24,52 @@ def custom_field_value(entry)
 
  entry.key?('value') ? entry['value'] : entry['values']
  end
+
+ # The API reference documents what a custom field *is*, never the form its
+ # value is read back in, so a Number answering `"42"` is not ruled out. A
+ # value that is not already of its column's type is therefore converted --
+ # the in-memory pass of the primary-key short-circuit would otherwise drop
+ # a row on `"42" == 42.0`, comparing a read string with the float
+ # `ConditionTreeParser.cast_to_type` casts the filter to.
+ #
+ # A date stays the string it is: the filter carries an ISO8601 string too,
+ # and comparing two of those is the ordering itself.
+ def coerce_custom_field(value, column_type)
+ return nil if value.nil?
+
+ case column_type
+ when 'Number' then coerce_number(value)
+ when 'Boolean' then coerce_boolean(value)
+ else value
+ end
+ end
+
+ # A value already numeric is handed back untouched: `ConditionTreeLeaf#match`
+ # compares with `==` and `Array#include?`, both of which hold across Integer
+ # and Float, so nothing needs widening -- and widening would make an integer
+ # field display the `12.0` it does not hold, where `FilterValue#format_float`
+ # narrows the very same value on the way out. A string is read to the
+ # tightest form for that reason, the two halves agreeing on what an integer
+ # looks like.
+ #
+ # A number that cannot be read reads as absent rather than as zero.
+ def coerce_number(value)
+ return value if value.is_a?(Numeric)
+
+ float = Float(value, exception: false)
+ return nil if float.nil?
+
+ float == float.to_i ? float.to_i : float
+ end
+
+ # An empty boolean reads as absent as well: `false` is an answer of its own,
+ # and Pylon gave none.
+ def coerce_boolean(value)
+ return value if [true, false].include?(value)
+ return nil if value.to_s.strip.empty?
+
+ !%w[false 0 no].include?(value.to_s.strip.downcase)
+ end
  end
  end
 end

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/datasource.rb

@@ -15,10 +15,25 @@ def initialize(api_key:, **options)
  # The five collections are registered together: each one declares relations
  # pointing at the others, and a relation whose foreign collection is missing
  # is a schema the agent refuses to boot on.
+ #
+ # Their custom fields are introspected here, one call per object type, as
+ # Pylon indexes its definitions by object type and asks for one on every
+ # call. Each entry stays on the collection it belongs to: a custom field is
+ # filtered through the very slug it is read by, which the collection's
+ # `api_filters` already carries, so there is no datasource-wide mapping to
+ # hold — and two Pylon datasources in the same agent share nothing.
+ #
+ # An introspection that fails costs the custom columns, not the datasource:
+ # `fetch_custom_fields` degrades to an empty list and the agent boots on the
+ # native schema.
  def register_collections
- add_collection(Collections::Issue.new(self))
- add_collection(Collections::Account.new(self))
- add_collection(Collections::Contact.new(self))
+ custom_fields = Schema::CustomFieldsIntrospector.new(@client)
+
+ add_collection(Collections::Issue.new(self, custom_fields: custom_fields.issue_custom_fields))
+ add_collection(Collections::Account.new(self, custom_fields: custom_fields.account_custom_fields))
+ add_collection(Collections::Contact.new(self, custom_fields: custom_fields.contact_custom_fields))
+ # Pylon carries custom fields on issues, accounts and contacts only:
+ # neither an agent nor a team has any.
  add_collection(Collections::User.new(self))
  add_collection(Collections::Team.new(self))
  end

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/condition_tree_translator.rb

@@ -24,6 +24,13 @@ class ConditionTreeTranslator
  LIST_OPERATORS = %w[in not_in].freeze
  VALUELESS_OPERATORS = %w[is_set is_unset].freeze
 
+ # The comparisons Pylon reads as a moment in time: what tells FilterValue
+ # that a bare date is a date, and not a piece of text a field happens to
+ # hold. Read off the emitted operator rather than off the column type,
+ # which the translator does not see -- and it is the operator that decides
+ # the format anyway.
+ TIME_OPERATORS = %w[time_is_after time_is_before].freeze
+
  def self.call(condition_tree, api_filters: {}, timezone: nil)
  return nil if condition_tree.nil?
 
@@ -89,7 +96,7 @@ def with_value(filter, operator, leaf)
  return filter if VALUELESS_OPERATORS.include?(operator)
  return filter.merge('values' => @value.list(leaf)) if LIST_OPERATORS.include?(operator)
 
- filter.merge('value' => @value.single(leaf))
+ filter.merge('value' => @value.single(leaf, time: TIME_OPERATORS.include?(operator)))
  end
 
  # `present`, `blank` and `missing` are advertised on every field carrying

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/filter_value.rb

@@ -1,3 +1,4 @@
+require 'date'
 require 'active_support/core_ext/time/zones'
 
 module ForestAdminDatasourcePylon
@@ -6,20 +7,30 @@ module Query
  # Split from the translator, which knows the shape of the condition tree but
  # not the format of what the leaves carry.
  class FilterValue
+ # A date carrying no time of day, which is what the frontend sends for a
+ # Dateonly column -- a shape only a custom field has, no native column
+ # being typed that way.
+ DATE_ONLY = /\A\d{4}-\d{2}-\d{2}\z/
+
  def initialize(timezone: nil)
  @timezone = timezone.to_s.strip.empty? ? 'UTC' : timezone
  end
 
- def single(leaf)
+ # `time` says the operator this value travels with is one of Pylon's time
+ # comparisons, which is what decides whether a bare date is a date or a
+ # piece of text: the same string on a text field is a value of its own.
+ def single(leaf, time: false)
  raise_nil_value(leaf.field) if leaf.value.nil?
 
- format(leaf.value)
+ format(leaf.value, time: time)
  end
 
  # Dropping the blanks would silently answer a different question: `not_in
  # [nil, 'open']` was asked to exclude the blank records and would come
  # back including them. An empty list is just as bad the other way round,
  # translating to a filter matching everything.
+ #
+ # No time comparison takes a list, so nothing here is read as a date.
  def list(leaf)
  values = Array(leaf.value)
  raise_empty_list(leaf) if values.empty?
@@ -30,21 +41,49 @@ def list(leaf)
 
  private
 
- # Numbers and booleans travel as they are: the filter is JSON, not a query
- # string, so only the date types need a wire format.
- def format(value)
+ # Booleans travel as they are: the filter is JSON, not a query string, so
+ # only the dates and the numbers the agent widened need a wire format.
+ def format(value, time: false)
  case value
  when Time, DateTime then value.to_time.utc.iso8601
  when Date then format_date(value)
+ when Float then format_float(value)
+ when String then time ? format_time_string(value) : value
  else value
  end
  end
 
+ # The agent casts every Number column with `to_f`
+ # (`ConditionTreeParser.cast_to_type`), so an integer custom field would be
+ # filtered with `42.0` -- a form none of its values carry. A float with
+ # nothing after the point travels as the integer it is; a decimal keeps its
+ # own.
+ def format_float(value)
+ value == value.to_i ? value.to_i : value
+ end
+
+ # `time_is_after` receives a timestamp everywhere else in this datasource,
+ # a native date column being read and filtered as one: a Dateonly custom
+ # field cannot be the one field sending the same operator another shape.
+ # The bound is the one a Ruby `Date` already gets -- midnight in the
+ # timezone of the caller.
+ #
+ # A string this operator cannot read as a date is left to Pylon, which
+ # names what it refuses better than a guess here would.
+ def format_time_string(value)
+ return value unless DATE_ONLY.match?(value)
+
+ format_date(Date.parse(value))
+ rescue Date::Error
+ value
+ end
+
  # Only reached by a condition tree built in Ruby -- a segment or a scope
- # written as code. Everything coming through HTTP arrives as an ISO8601
- # string, already expressed in the timezone of the caller: the agent casts
- # a date filter with `value.to_s`, and the toolkit formats the bounds it
- # derives from Today / Previous* itself.
+ # written as code -- and by the bare date above. Everything else coming
+ # through HTTP arrives as an ISO8601 timestamp, already expressed in the
+ # timezone of the caller: the agent casts a date filter with `value.to_s`,
+ # and the toolkit formats the bounds it derives from Today / Previous*
+ # itself.
  def format_date(value)
  Time.use_zone(@timezone) { Time.zone.local(value.year, value.month, value.day).utc.iso8601 }
  rescue ArgumentError