diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client.rb index 4e629245f..101217930 100644 --- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client.rb +++ b/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? diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/record_serialization.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/record_serialization.rb index 4024ab7fe..3f88fc699 100644 --- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/record_serialization.rb +++ b/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 diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/datasource.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/datasource.rb index 446eb45a5..3c5f510bb 100644 --- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/datasource.rb +++ b/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 diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/condition_tree_translator.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/condition_tree_translator.rb index 8b3e0cf86..0d448fb6f 100644 --- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/condition_tree_translator.rb +++ b/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 diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/filter_value.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/filter_value.rb index 9d53b3b7e..2b1676b7b 100644 --- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/filter_value.rb +++ b/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 diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/schema/custom_fields_introspector.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/schema/custom_fields_introspector.rb new file mode 100644 index 000000000..2286d64e4 --- /dev/null +++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/schema/custom_fields_introspector.rb @@ -0,0 +1,149 @@ +module ForestAdminDatasourcePylon + module Schema + # Turns the custom fields an organization defined in Pylon into columns, as + # entries shaped `{ column_name:, schema: }` — what `add_custom_fields` + # registers on a collection. + # + # `column_name` is the Pylon slug verbatim, and there is no second key + # carrying it: the slug is both what a read payload indexes the values by and + # what a search filter sends as `field`, so renaming the column would add a + # mapping to keep in step for nothing. + # + # Pylon defines custom fields per object type, and asks for that type on + # every call: `issue`, `account` and `contact` are the three this datasource + # has a collection for — it also exposes `task`, `project`, `meeting` and + # `opportunity`, while users and teams carry no custom field at all. + class CustomFieldsIntrospector + ColumnSchema = ForestAdminDatasourceToolkit::Schema::ColumnSchema + Maps = Query::OperatorMaps + + # A type absent from this table is skipped rather than guessed at: a column + # whose Forest type does not match what Pylon holds would filter and + # display wrong, which is worse than not being there. + # + # `user` holds a Pylon user id, kept a String rather than turned into a + # relation to PylonUser: a custom field is registered after the relations + # are declared, and a foreign key the operator can read is what this story + # promises. + PYLON_TO_COLUMN_TYPE = { + 'text' => 'String', + 'url' => 'String', + 'user' => 'String', + 'number' => 'Number', + 'decimal' => 'Number', + 'boolean' => 'Boolean', + 'date' => 'Dateonly', + 'datetime' => 'Date', + 'select' => 'Enum', + 'multiselect' => 'Json' + }.freeze + + BASE_OPS = (Maps::EQUALITY.keys + Maps::PRESENCE.keys).freeze + + # A date drops the membership operators on the way, `Rules` granting a DATE + # or a DATEONLY column no array operator -- the hazard already documented + # for MEMBERSHIP in `operator_maps.rb`. Native date columns declare the + # comparisons alone for the same reason. + # + # Dropping them is not what keeps `in` out of the UI, though, and nothing + # here can: `OperatorsEquivalenceCollectionDecorator` republishes it from + # the `equal` this set declares, the IN transform depending on EQUAL for + # every column type. A DATEONLY also gets `after_x_hours_ago` / + # `before_x_hours_ago` republished from the comparisons, `Times.compare` + # deriving them for that type where `Rules` refuses them. The validator + # then rejects all three, so the operator is offered a date filter the + # agent answers with a 400. The contradiction is the toolkit's to settle + # and is tracked as PRD-989; the set below is what Pylon accepts, which is + # the only question this table can answer. + TIME_OPS = (BASE_OPS - Maps::MEMBERSHIP.keys + Maps::TIME.keys).freeze + + # Drawn from `CUSTOM_FIELD_OPS`, the set every search endpoint accepts on a + # custom field, so a collection's clamp has nothing to drop. + # + # A Number gets no comparison: Pylon documents `time_is_after` / + # `time_is_before` for the bare comparisons and nothing else, so a numeric + # range would travel as a time filter. A multiselect gets nothing at all — + # its membership operators are not part of what a custom field accepts. + OPERATORS = { + 'String' => (BASE_OPS + Maps::FULL_TEXT.keys).freeze, + 'Enum' => BASE_OPS, + 'Number' => BASE_OPS, + 'Boolean' => BASE_OPS, + 'Date' => TIME_OPS, + 'Dateonly' => TIME_OPS, + 'Json' => [].freeze + }.freeze + + def initialize(client) + @client = client + end + + def issue_custom_fields = introspect('issue') + def account_custom_fields = introspect('account') + def contact_custom_fields = introspect('contact') + + private + + def introspect(object_type) + Array(@client.fetch_custom_fields(object_type)).filter_map { |raw| build_entry(raw, object_type) } + end + + def build_entry(raw, object_type) + return nil unless raw.is_a?(Hash) + + slug = raw['slug'].to_s + return nil if slug.empty? + + column_type = PYLON_TO_COLUMN_TYPE[raw['type']] + return warn_unknown_type(raw, slug, object_type) if column_type.nil? + + { column_name: slug, schema: build_schema(raw, column_type) } + end + + # Every custom field is read-only in this story, like every native column: + # writes land in story 7 (EXT-11), which is also where Pylon's own + # `is_read_only` flag starts being honoured. Nothing is sortable either -- + # no Pylon endpoint takes a sort parameter, and nothing is groupable, as + # Pylon aggregates nothing: one column left groupable turns `supportGroups` + # on for the whole collection, and the group-by the UI then offers errors. + def build_schema(raw, column_type) + opts = { column_type: column_type, + filter_operators: OPERATORS.fetch(column_type, []), + is_read_only: true, + is_sortable: false, + is_groupable: false } + + column_type == 'Enum' ? enum_schema(raw, opts) : ColumnSchema.new(**opts) + end + + # Pylon reads a select back — and filters it — as the slug of the option, + # never as its label, so those are the values the column advertises. + # + # Forest refuses an Enum carrying no value: a select whose options were all + # removed falls back to String, so the column still shows what it holds. + def enum_schema(raw, opts) + values = option_slugs(raw) + return ColumnSchema.new(**opts, enum_values: values) unless values.empty? + + ColumnSchema.new(**opts, column_type: 'String', filter_operators: OPERATORS.fetch('String')) + end + + def option_slugs(raw) + metadata = raw['select_metadata'] + options = metadata.is_a?(Hash) ? metadata['options'] : nil + + Array(options).filter_map do |option| + option['slug'] if option.is_a?(Hash) && !option['slug'].to_s.empty? + end + end + + def warn_unknown_type(raw, slug, object_type) + ForestAdminDatasourcePylon.logger.warn( + "[forest_admin_datasource_pylon] Custom field '#{slug}' on #{object_type} has type " \ + "#{raw["type"].inspect}, which this datasource cannot map to a Forest column; skipping." + ) + nil + end + end + end +end diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/client_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/client_spec.rb index d52037f84..2ab547e7e 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/client_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/client_spec.rb @@ -555,6 +555,61 @@ def page(records, cursor: nil) end end + describe '#fetch_custom_fields' do + let(:logger) { instance_double(Logger, warn: nil) } + + def definitions(records, cursor: nil) + body = { 'data' => records } + body['pagination'] = { 'cursor' => cursor, 'has_next_page' => true } if cursor + json(body) + end + + # `object_type` is mandatory: Pylon answers 400 without it, so the schema of + # each collection is read by its own call. + it 'asks for the definitions of one object type' do + stub_request(:get, "#{base}/custom-fields").with(query: { 'object_type' => 'issue' }) + .to_return(definitions([{ 'slug' => 'severity' }])) + + expect(client.fetch_custom_fields('issue')).to eq([{ 'slug' => 'severity' }]) + end + + # The mandatory parameter has to survive the walk: dropped on the second + # page, it would answer with the fields of another object type or with a 400. + it 'keeps the object type on every page of the walk' do + stub_request(:get, "#{base}/custom-fields").with(query: { 'object_type' => 'issue' }) + .to_return(definitions([{ 'slug' => 'severity' }], cursor: 'c1')) + stub_request(:get, "#{base}/custom-fields").with(query: { 'object_type' => 'issue', 'cursor' => 'c1' }) + .to_return(definitions([{ 'slug' => 'tier' }])) + + expect(client.fetch_custom_fields('issue')).to eq([{ 'slug' => 'severity' }, { 'slug' => 'tier' }]) + end + + it 'returns an empty list when the organization defined no custom field' do + stub_request(:get, "#{base}/custom-fields").with(query: { 'object_type' => 'account' }) + .to_return(definitions(nil)) + + expect(client.fetch_custom_fields('account')).to eq([]) + end + + # Read while the agent boots: the datasource has to come up on its native + # schema rather than fail to come up at all. + it 'degrades to an empty list and reports the failure' do + allow(ForestAdminDatasourcePylon).to receive(:logger).and_return(logger) + stub_request(:get, "#{base}/custom-fields").with(query: { 'object_type' => 'issue' }) + .to_return(json({ 'message' => 'boom' }, 500)) + + expect(client.fetch_custom_fields('issue')).to eq([]) + expect(logger).to have_received(:warn).with(/fetch_custom_fields\(issue\) failed; degrading.*HTTP 500 boom/) + end + + it 'degrades when the token is not allowed to read the definitions' do + stub_request(:get, "#{base}/custom-fields").with(query: { 'object_type' => 'issue' }) + .to_return(json({ 'message' => 'forbidden' }, 403)) + + expect(client.fetch_custom_fields('issue')).to eq([]) + end + end + describe 'rate limiting' do it 'retries a 429 and returns the eventual success' do stub_request(:get, "#{base}/me") diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/account_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/account_spec.rb index 1a5b7764c..0e9bf0b16 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/account_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/account_spec.rb @@ -57,6 +57,8 @@ def columns let(:base) { datasource.configuration.url } let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + before { stub_custom_fields } + def stub_list(query, payload) stub_request(:get, "#{base}/accounts").with(query: query).to_return(json(payload)) end diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/contact_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/contact_spec.rb index 5e66406dc..93ca7c47b 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/contact_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/contact_spec.rb @@ -55,6 +55,8 @@ def columns let(:base) { datasource.configuration.url } let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + before { stub_custom_fields } + def stub_list(query, payload) stub_request(:get, "#{base}/contacts").with(query: query).to_return(json(payload)) end diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue/messages_embedder_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue/messages_embedder_spec.rb index 51f03f241..46ad3dddd 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue/messages_embedder_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue/messages_embedder_spec.rb @@ -39,6 +39,8 @@ def stub_messages(issue_id, *payloads) let(:base) { datasource.configuration.url } let(:logger) { instance_double(Logger, warn: nil) } + before { stub_custom_fields } + describe 'the schema of the column' do it 'declares messages as an array of message shapes' do expect(issues.fields['messages'].column_type).to eq([Collections::Issue::MESSAGE_THREAD_SCHEMA]) diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue_spec.rb index 1792b932c..bc57d1d88 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/issue_spec.rb @@ -54,6 +54,8 @@ def columns let(:base) { datasource.configuration.url } let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + before { stub_custom_fields } + describe 'schema' do it 'is named PylonIssue' do expect(collection.name).to eq('PylonIssue') @@ -350,6 +352,92 @@ def columns expect(ForestAdminDatasourcePylon.logger) .to have_received(:warn).with(/cannot honour on a custom field \(starts_with\)/) end + + # The API reference documents what a custom field is, never the form its + # value is read back in. The column holds the form the agent gives a filter + # value of the same type, so the two stay comparable whichever one Pylon + # answered with. + describe 'the form a value is read in' do + def typed(type) + ForestAdminDatasourceToolkit::Schema::ColumnSchema + .new(column_type: type, filter_operators: [operators::EQUAL]) + end + + def entry(slug, value) + { slug => { 'slug' => slug, 'value' => value } } + end + + def read(fields) + stub_request(:post, "#{base}/issues/search") + .to_return(json('data' => [issue_payload('i1', 'custom_fields' => fields)])) + + collection.list(nil, filter, nil).first + end + + let(:collection) do + described_class.new(datasource, custom_fields: [{ column_name: 'nps', schema: typed('Number') }, + { column_name: 'vip', schema: typed('Boolean') }, + { column_name: 'renewal', schema: typed('Dateonly') }]) + end + + # A value already numeric keeps its own form: widening `42` into `42.0` + # would display a decimal the field does not hold, where the wire half + # narrows the same value back. A string is read to the tightest form, so + # both halves agree on what an integer looks like. + [[42, 42], ['42', 42], [' 42 ', 42], [42.0, 42.0], [42.5, 42.5], ['42.5', 42.5], + ['42.0', 42]].each do |raw, expected| + it "reads a number answered as #{raw.inspect} as #{expected.inspect}" do + expect(read(entry('nps', raw))['nps']).to eql(expected) + end + end + + it 'reads a number it cannot make sense of as absent rather than as zero' do + expect(read(entry('nps', 'n/a'))['nps']).to be_nil + end + + [[true, true], ['true', true], [false, false], ['false', false], ['0', false]].each do |raw, expected| + it "reads a boolean answered as #{raw.inspect} as #{expected}" do + expect(read(entry('vip', raw))['vip']).to be(expected) + end + end + + it 'reads an empty boolean as absent, false being an answer of its own' do + expect(read(entry('vip', ''))['vip']).to be_nil + end + + it 'leaves a date the ISO string the filter is compared with' do + expect(read(entry('renewal', '2026-08-01'))['renewal']).to eq('2026-08-01') + end + + it 'leaves a field the issue does not carry absent' do + expect(read({})).to include('nps' => nil, 'vip' => nil, 'renewal' => nil) + end + + # The one place the form decides the result: an id filter is answered by + # `GET /issues/{id}` and the rest is applied in memory, so a number left + # as `"42"` would be compared with the `42.0` the agent casts the filter + # to, and the row would be dropped without a word. Reading it as the + # Integer `42` is enough -- `match` compares with `==`. + it 'keeps a row matched on a number combined with an id lookup' do + stub_request(:get, "#{base}/issues/i1") + .to_return(json('data' => issue_payload('i1', 'custom_fields' => entry('nps', '42')))) + tree = branch('And', [id_leaf(operators::EQUAL, 'i1'), leaf('nps', operators::EQUAL, 42.0)]) + + expect(collection.list(nil, filter(condition_tree: tree), %w[id nps])) + .to eq([{ 'id' => 'i1', 'nps' => 42 }]) + end + + # The same row, matched through a membership filter: `Array#include?` + # compares with `==` too, so the Integer read answers the float list. + it 'keeps a row matched on a number list combined with an id lookup' do + stub_request(:get, "#{base}/issues/i1") + .to_return(json('data' => issue_payload('i1', 'custom_fields' => entry('nps', '42')))) + tree = branch('And', [id_leaf(operators::EQUAL, 'i1'), leaf('nps', operators::IN, [42.0, 7.0])]) + + expect(collection.list(nil, filter(condition_tree: tree), %w[id nps])) + .to eq([{ 'id' => 'i1', 'nps' => 42 }]) + end + end end describe '#list with a filter' do diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/relation_embedder_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/relation_embedder_spec.rb index 27d973559..c8bafeafa 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/relation_embedder_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/relation_embedder_spec.rb @@ -80,6 +80,8 @@ def columns_of(name) let(:contacts) { datasource.get_collection('PylonContact') } let(:base) { datasource.configuration.url } + before { stub_custom_fields } + describe 'what the projection asks for' do before { stub_issues(issue_payload('i1')) } diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/team_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/team_spec.rb index 02817a8ca..f9c0bf080 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/team_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/team_spec.rb @@ -57,6 +57,8 @@ def columns let(:base) { datasource.configuration.url } let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + before { stub_custom_fields } + describe 'schema' do it 'is named PylonTeam' do expect(collection.name).to eq('PylonTeam') diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/user_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/user_spec.rb index 2125ac7ad..c17d0dcfe 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/user_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/user_spec.rb @@ -60,6 +60,8 @@ def columns let(:base) { datasource.configuration.url } let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + before { stub_custom_fields } + describe 'schema' do it 'is named PylonUser' do expect(collection.name).to eq('PylonUser') diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/datasource_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/datasource_spec.rb index be80cd1af..e67058018 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/datasource_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/datasource_spec.rb @@ -1,5 +1,32 @@ RSpec.describe ForestAdminDatasourcePylon::Datasource do let(:datasource) { described_class.new(api_key: 'k') } + let(:base) { ForestAdminDatasourcePylon::Configuration::DEFAULT_BASE_URL } + + before { stub_custom_fields } + + def definition(slug, type: 'text', **extra) + { 'id' => "cf_#{slug}", 'slug' => slug, 'label' => slug.capitalize, 'type' => type }.merge(extra) + end + + def select_definition(slug, type: 'select', options: %w[p1 p2]) + definition(slug, type: type, + 'select_metadata' => { 'options' => options.map { |o| { 'label' => o.upcase, 'slug' => o } } }) + end + + def leaf(field, operator, value) + ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf + .new(field, operator, value) + end + + def filter(condition_tree) + ForestAdminDatasourceToolkit::Components::Query::Filter.new(condition_tree: condition_tree) + end + + def stub_issues(custom_fields) + stub_request(:post, "#{base}/issues/search") + .to_return(status: 200, headers: { 'Content-Type' => 'application/json' }, + body: { 'data' => [{ 'id' => 'i1', 'custom_fields' => custom_fields }] }.to_json) + end it 'builds a configuration from the api key' do expect(datasource.configuration.api_key).to eq('k') @@ -7,6 +34,7 @@ end it 'forwards the remaining options to the configuration' do + stub_custom_fields(base: 'https://pylon.test') custom = described_class.new(api_key: 'k', base_url: 'https://pylon.test/', timeout: 3) expect(custom.configuration.url).to eq('https://pylon.test') @@ -42,11 +70,115 @@ expect { described_class.new(api_key: nil) }.to raise_error(ForestAdminDatasourcePylon::ConfigurationError) end - # Nothing is introspected at boot yet: registering collections must not hit - # the API, so an agent boots even when Pylon is unreachable. - it 'does not call the API while registering collections' do - datasource + describe 'custom fields' do + # Pylon indexes its definitions by object type and asks for one on every + # call, so the three collections carrying custom fields cost one call each. + it 'reads the definitions of the three object types Pylon carries them on' do + datasource + + %w[issue account contact].each do |object_type| + expect(WebMock).to have_requested(:get, "#{base}/custom-fields") + .with(query: { 'object_type' => object_type }).once + end + end + + # A definition is a column of the collection it was declared on, and of no + # other: Pylon scopes a custom field to one object type. + it 'adds each definition to the collection its object type registers' do + stub_custom_fields(issue: [definition('severity')], account: [definition('tier')], + contact: [definition('nps', type: 'number')]) - expect(WebMock).not_to have_requested(:any, /usepylon/) + expect(datasource.get_collection('PylonIssue').fields).to have_key('severity') + expect(datasource.get_collection('PylonAccount').fields).to have_key('tier') + expect(datasource.get_collection('PylonContact').fields['nps'].column_type).to eq('Number') + expect(datasource.get_collection('PylonIssue').fields).not_to have_key('tier') + end + + # Users and teams have no custom field at all in Pylon, so nothing is asked + # for them -- an absent call rather than one answering an empty list. + it 'asks for no definition on users and teams' do + datasource + + %w[user team].each do |object_type| + expect(WebMock).not_to have_requested(:get, "#{base}/custom-fields") + .with(query: { 'object_type' => object_type }) + end + end + + # The definitions are read while the agent boots: a Pylon that is down, or a + # token without the permission, has to cost the operator the custom columns + # rather than the agent. + it 'boots on the native schema when the definitions cannot be read' do + stub_request(:get, "#{base}/custom-fields").to_return(status: 500, body: '{}', + headers: { 'Content-Type' => 'application/json' }) + + expect { datasource }.not_to raise_error + expect(datasource.collections.keys).to include('PylonIssue') + expect(datasource.get_collection('PylonIssue').custom_fields).to eq([]) + end + + # A custom field is filtered through the very slug it is read by, so nothing + # is held datasource-wide -- and two Pylon organizations in the same agent + # cannot end up advertising each other's columns. + it 'keeps the custom fields of one datasource out of another' do + stub_custom_fields(issue: [definition('severity')]) + first = described_class.new(api_key: 'k') + + stub_custom_fields(issue: [definition('tier')]) + second = described_class.new(api_key: 'k') + + expect(first.get_collection('PylonIssue').fields).to have_key('severity') + expect(first.get_collection('PylonIssue').fields).not_to have_key('tier') + expect(second.get_collection('PylonIssue').fields).to have_key('tier') + expect(second.get_collection('PylonIssue').fields).not_to have_key('severity') + end + + # The one place the introspected shape meets the read pipeline: a definition + # becomes a column, the payload's value is read through it, and a filter on + # it travels as the slug Pylon indexes it by -- the three are held together + # by the slug alone, so a spec on each in isolation would not catch a rename. + describe 'the round trip of an introspected field' do + it 'reads a select back and filters it by the option slug' do + stub_custom_fields(issue: [select_definition('priority_level')]) + stub_issues('priority_level' => { 'slug' => 'priority_level', 'value' => 'p1' }) + collection = datasource.get_collection('PylonIssue') + + rows = collection.list(nil, filter(leaf('priority_level', 'equal', 'p1')), nil) + + expect(collection.fields['priority_level'].enum_values).to eq(%w[p1 p2]) + expect(rows.first).to include('priority_level' => 'p1') + expect(WebMock).to have_requested(:post, "#{base}/issues/search") + .with(body: hash_including('filter' => { 'field' => 'priority_level', + 'operator' => 'equals', 'value' => 'p1' })) + end + + # A multiselect is read out of `values` rather than `value`, and Pylon + # accepts no filter on one -- the column carries the list and nothing else. + it 'reads a multiselect as the list of its option slugs, unfilterable' do + stub_custom_fields(issue: [select_definition('zones', type: 'multiselect', options: %w[eu us])]) + stub_issues('zones' => { 'slug' => 'zones', 'values' => %w[eu us] }) + collection = datasource.get_collection('PylonIssue') + + rows = collection.list(nil, nil, nil) + + expect(collection.fields['zones'].column_type).to eq('Json') + expect(collection.fields['zones'].filter_operators).to eq([]) + expect(rows.first).to include('zones' => %w[eu us]) + expect { collection.list(nil, filter(leaf('zones', 'contains', 'eu')), nil) } + .to raise_error(ForestAdminDatasourcePylon::UnsupportedOperatorError, /not supported on field 'zones'/) + end + end + + # Registration evaluates the collision against the final native schema, so a + # slug shadowing a column the collection already declares is left out. + it 'skips a definition colliding with a native column' do + allow(ForestAdminDatasourcePylon.logger).to receive(:warn) + stub_custom_fields(issue: [definition('title')]) + + expect(datasource.get_collection('PylonIssue').custom_fields).to eq([]) + expect(datasource.get_collection('PylonIssue').fields['title'].filter_operators).not_to be_empty + expect(ForestAdminDatasourcePylon.logger) + .to have_received(:warn).with(/'title' on collection 'PylonIssue' conflicts/) + end end end diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/query/condition_tree_translator_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/query/condition_tree_translator_spec.rb index 19dbfa1e1..4af782b73 100644 --- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/query/condition_tree_translator_spec.rb +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/query/condition_tree_translator_spec.rb @@ -96,6 +96,13 @@ def translate(tree, api_filters: default_filters, timezone: nil) .to eq('field' => 'body_html', 'operator' => 'string_does_not_contain', 'value' => 'boom') end + # A bare date is read off the operator, not off the shape of the string: a + # text field holding what looks like a date holds a value of its own. + it 'leaves a date-shaped value untouched on a field not compared as time' do + expect(translate(leaf('title', operators::I_CONTAINS, '2026-08-01'))) + .to include('value' => '2026-08-01') + end + it 'refuses equal on a text field, which Pylon cannot match exactly' do expect { translate(leaf('title', operators::EQUAL, 'Boom')) } .to raise_error(UnsupportedOperatorError, /not supported on field 'title'/) @@ -154,6 +161,20 @@ def translate(tree, api_filters: default_filters, timezone: nil) expect(filter['subfilters'].map { |sub| sub['operator'] }).to eq(%w[time_is_after time_is_before]) end + # A Dateonly column -- only a custom field is typed that way -- sends the + # date alone, where every native column sends `time_is_after` a timestamp. + # The bound is the one a Ruby Date already gets. + it 'reads a bare date string as midnight in the timezone of the caller' do + filter = translate(leaf('created_at', operators::GREATER_THAN, '2026-08-01'), timezone: 'Europe/Paris') + + expect(filter).to include('value' => '2026-07-31T22:00:00Z') + end + + it 'leaves a string no date can be read from to Pylon, which names what it refuses' do + expect(translate(leaf('created_at', operators::GREATER_THAN, '2026-13-45'))) + .to include('value' => '2026-13-45') + end + it 'falls back to UTC and warns on a timezone it does not know' do allow(ForestAdminDatasourcePylon.logger).to receive(:warn) @@ -297,6 +318,18 @@ def translate(tree, api_filters: default_filters, timezone: nil) expect(translate(leaf('customer_portal_visible', operators::EQUAL, false))) .to include('value' => false) end + + # The agent casts every Number column with `to_f`, so an integer custom + # field would be filtered with `42.0` -- a form none of its values carry. + # `be` rather than `eq`: `12.0 == 12` holds in Ruby, and the form that + # travels to Pylon is the one thing this is about. + it 'sends an integer-valued float as the integer it is' do + expect(translate(leaf('number_of_touches', operators::EQUAL, 12.0))['value']).to be(12) + end + + it 'keeps what a decimal carries after the point' do + expect(translate(leaf('number_of_touches', operators::EQUAL, 12.5))['value']).to be(12.5) + end end end end diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/schema/custom_fields_introspector_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/schema/custom_fields_introspector_spec.rb new file mode 100644 index 000000000..975864321 --- /dev/null +++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/schema/custom_fields_introspector_spec.rb @@ -0,0 +1,199 @@ +RSpec.describe ForestAdminDatasourcePylon::Schema::CustomFieldsIntrospector do + let(:client) { instance_double(ForestAdminDatasourcePylon::Client) } + let(:introspector) { described_class.new(client) } + let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + + def definition(type, slug: 'severity', **extra) + { 'id' => 'cf_1', 'slug' => slug, 'label' => slug.capitalize, 'type' => type, + 'object_type' => 'issue', 'is_read_only' => false }.merge(extra) + end + + def select_metadata(*slugs) + { 'select_metadata' => { 'options' => slugs.map { |slug| { 'label' => slug.upcase, 'slug' => slug } } } } + end + + # `object_type` is what Pylon indexes its definitions by, and each accessor + # reads the type of the collection it feeds. + describe 'the object type of each accessor' do + it 'reads the definitions of the matching Pylon object type' do + %w[issue account contact].each do |object_type| + allow(client).to receive(:fetch_custom_fields).with(object_type).and_return([]) + end + + introspector.issue_custom_fields + introspector.account_custom_fields + introspector.contact_custom_fields + + expect(client).to have_received(:fetch_custom_fields).with('issue') + expect(client).to have_received(:fetch_custom_fields).with('account') + expect(client).to have_received(:fetch_custom_fields).with('contact') + end + end + + describe 'column types' do + it 'maps every Pylon type onto the Forest column type holding it' do + definitions = %w[text url user number decimal boolean date datetime multiselect] + .map { |type| definition(type, slug: type) } + allow(client).to receive(:fetch_custom_fields).with('issue').and_return(definitions) + + types = introspector.issue_custom_fields.to_h { |cf| [cf[:column_name], cf[:schema].column_type] } + + expect(types).to eq('text' => 'String', 'url' => 'String', 'user' => 'String', + 'number' => 'Number', 'decimal' => 'Number', 'boolean' => 'Boolean', + 'date' => 'Dateonly', 'datetime' => 'Date', 'multiselect' => 'Json') + end + + # A type this datasource cannot map would filter and display wrong, which is + # worse for the operator than a column that is not there. + it 'skips a type it cannot map, and says which field it left out' do + allow(ForestAdminDatasourcePylon.logger).to receive(:warn) + allow(client).to receive(:fetch_custom_fields).with('account') + .and_return([definition('holographic', slug: 'tier')]) + + expect(introspector.account_custom_fields).to eq([]) + expect(ForestAdminDatasourcePylon.logger) + .to have_received(:warn).with(/'tier' on account has type "holographic".*skipping/) + end + + it 'skips a definition carrying no slug, which nothing could be read by' do + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('text', slug: ''), 'not a hash']) + + expect(introspector.issue_custom_fields).to eq([]) + end + end + + describe 'a select field' do + it 'advertises the option slugs, which are the values Pylon reads and filters' do + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('select', **select_metadata('p1', 'p2'))]) + + schema = introspector.issue_custom_fields.first[:schema] + + expect(schema.column_type).to eq('Enum') + expect(schema.enum_values).to eq(%w[p1 p2]) + end + + it 'reads a multiselect the same way' do + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('multiselect', **select_metadata('eu'))]) + + expect(introspector.issue_custom_fields.first[:schema].column_type).to eq('Json') + end + + # Forest refuses an Enum carrying no value, so a select whose options were + # all removed still shows what it holds instead of disappearing. + it 'falls back to String when every option is gone' do + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('select', **select_metadata)]) + + schema = introspector.issue_custom_fields.first[:schema] + + expect(schema.column_type).to eq('String') + expect(schema.enum_values).to eq([]) + expect(schema.filter_operators).to include(operators::I_CONTAINS) + end + + it 'ignores an option with no slug rather than advertising a blank value' do + metadata = { 'select_metadata' => { 'options' => [{ 'label' => 'P1' }, { 'slug' => 'p2' }, 'nope'] } } + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('select', **metadata)]) + + expect(introspector.issue_custom_fields.first[:schema].enum_values).to eq(%w[p2]) + end + end + + describe 'filter operators' do + def operators_of(type, **extra) + allow(client).to receive(:fetch_custom_fields).with('issue').and_return([definition(type, **extra)]) + + introspector.issue_custom_fields.first[:schema].filter_operators + end + + it 'lets a text field be matched, listed, checked for presence and searched' do + expect(operators_of('text')).to eq([operators::EQUAL, operators::IN, operators::NOT_IN, + operators::PRESENT, operators::BLANK, operators::MISSING, + operators::CONTAINS, operators::I_CONTAINS, + operators::NOT_CONTAINS, operators::NOT_I_CONTAINS]) + end + + it 'gives a date field the bounds Pylon compares dates with' do + expect(operators_of('datetime')).to include(operators::GREATER_THAN, operators::LESS_THAN) + end + + # `Rules` grants a DATE or a DATEONLY column no array operator, so an + # advertised `in` would be refused by `ConditionTreeValidator` -- a filter the + # UI offers and the agent then answers with a 400. + it 'withholds the membership operators from a date, which the agent refuses on one' do + %w[date datetime].each do |type| + expect(operators_of(type)).to eq([operators::EQUAL, + operators::PRESENT, operators::BLANK, operators::MISSING, + operators::GREATER_THAN, operators::LESS_THAN]) + end + end + + # Pylon spells the bare comparisons `time_is_after` / `time_is_before` and + # documents nothing else, so a numeric range would travel as a time filter. + # + # The presence family carries MISSING next to PRESENT and BLANK: Pylon spells + # absence through `is_unset` alone, and a field left without MISSING would + # refuse the very filter its endpoint can answer. + it 'gives a number no comparison, only equality and presence' do + expect(operators_of('number')) + .to eq([operators::EQUAL, operators::IN, operators::NOT_IN, + operators::PRESENT, operators::BLANK, operators::MISSING]) + end + + it 'gives a boolean equality and presence' do + expect(operators_of('boolean')).to eq([operators::EQUAL, operators::IN, operators::NOT_IN, + operators::PRESENT, operators::BLANK, operators::MISSING]) + end + + # A membership filter is not part of what a custom field accepts, and no + # in-memory pass can stand in for one on a list. + it 'leaves a multiselect unfilterable' do + expect(operators_of('multiselect')).to eq([]) + end + + it 'keeps an enum to equality and presence' do + expect(operators_of('select', **select_metadata('p1'))) + .to eq([operators::EQUAL, operators::IN, operators::NOT_IN, + operators::PRESENT, operators::BLANK, operators::MISSING]) + end + end + + # Writes land in story 7 (EXT-11), which is also where Pylon's own + # `is_read_only` starts being honoured; no endpoint sorts, ever. + describe 'the schema every custom field gets' do + it 'is read-only and unsortable, whatever Pylon declares' do + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('text', 'is_read_only' => false)]) + + schema = introspector.issue_custom_fields.first[:schema] + + expect(schema.is_read_only).to be(true) + expect(schema.is_sortable).to be(false) + end + + # `ColumnSchema` defaults this one to true, and the capabilities route turns + # `supportGroups` on as soon as a single field carries it: one custom field + # left groupable is the whole collection offering a chart `aggregate` raises + # on. Every native column declares it false for that reason. + it 'is not groupable, as Pylon exposes no aggregate endpoint' do + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('select', **select_metadata('p1'))]) + + expect(introspector.issue_custom_fields.first[:schema].is_groupable).to be(false) + end + + # The slug is both the key a read payload indexes the value by and the + # `field` a filter sends, so the column carries it unchanged. + it 'names the column after the Pylon slug, with nothing else to keep in step' do + allow(client).to receive(:fetch_custom_fields).with('issue') + .and_return([definition('text', slug: 'sev_level')]) + + expect(introspector.issue_custom_fields.first.keys).to eq(%i[column_name schema]) + expect(introspector.issue_custom_fields.first[:column_name]).to eq('sev_level') + end + end +end diff --git a/packages/forest_admin_datasource_pylon/spec/spec_helper.rb b/packages/forest_admin_datasource_pylon/spec/spec_helper.rb index 68c4001aa..4998deb37 100644 --- a/packages/forest_admin_datasource_pylon/spec/spec_helper.rb +++ b/packages/forest_admin_datasource_pylon/spec/spec_helper.rb @@ -24,7 +24,26 @@ WebMock.disable_net_connect!(allow_localhost: true) +# A datasource introspects the custom fields of its three object types while it +# registers its collections. A spec building one declares what those calls +# answer -- most of them, having nothing to do with custom fields, answer +# nothing. The base url is not taken from the datasource on purpose: reading it +# would build the datasource, and boot the introspection this stubs. +module PylonCustomFieldStubs + def stub_custom_fields(issue: [], account: [], contact: [], + base: ForestAdminDatasourcePylon::Configuration::DEFAULT_BASE_URL) + { 'issue' => issue, 'account' => account, 'contact' => contact }.each do |object_type, definitions| + stub_request(:get, "#{base}/custom-fields") + .with(query: { 'object_type' => object_type }) + .to_return(status: 200, body: { 'data' => definitions }.to_json, + headers: { 'Content-Type' => 'application/json' }) + end + end +end + RSpec.configure do |config| + config.include PylonCustomFieldStubs + config.expect_with :rspec do |c| c.syntax = :expect end