diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon.rb
index 24aeadbba..2bc610d5b 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon.rb
@@ -20,6 +20,23 @@ class ConfigurationError < Error; end
# they learn which one.
class UnsupportedOperatorError < ForestAdminDatasourceToolkit::Exceptions::ValidationError; end
+ # The three write errors below descend from ValidationError for that same
+ # reason: each names something the operator did and can undo.
+
+ # A verb Pylon's API has no endpoint for, a field it only accepts in the other
+ # direction, or a write reaching more records than one pass may cover.
+ class UnsupportedWriteError < ForestAdminDatasourceToolkit::Exceptions::ValidationError; end
+
+ # A write Pylon performed on some of its records and then failed on: one
+ # record is one request, so the ones before the failure stay written, and a
+ # retry of the whole selection would write them a second time.
+ class PartialWriteError < ForestAdminDatasourceToolkit::Exceptions::ValidationError; end
+
+ # A write Pylon itself refused, carrying the reason it gave — the likeliest
+ # way a write fails. Only its 4xx travels this way: a 5xx or a dropped
+ # connection is not the operator's to act on and stays the APIError it was.
+ class WriteRejectedError < ForestAdminDatasourceToolkit::Exceptions::ValidationError; end
+
# Raised when a Pylon API call fails. Carries the HTTP status and the
# (parsed) response body so callers — smart actions in particular — can
# surface Pylon's own validation message instead of a generic string.
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 101217930..c477e7646 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
@@ -2,6 +2,8 @@ module ForestAdminDatasourcePylon
# Long by line count only: the public surface is one explicit method per Pylon
# endpoint, each delegating to the shared helpers below.
class Client # rubocop:disable Metrics/ClassLength
+ include Writes
+
MAX_SEARCH_LIMIT = 1000
# Bounds `collect_pages`, which asks for a whole dataset rather than a
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client/writes.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client/writes.rb
new file mode 100644
index 000000000..9a50e9cb9
--- /dev/null
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client/writes.rb
@@ -0,0 +1,88 @@
+module ForestAdminDatasourcePylon
+ class Client
+ # The write half of the client: one explicit method per Pylon write
+ # endpoint, each delegating to the shared helpers below.
+ #
+ # Nothing here degrades. `best_effort` exists for the calls whose result
+ # enriches a page — a thread that could not be read costs a column — where a
+ # write that silently did nothing would tell the operator their edit landed.
+ #
+ # Pylon exposes no POST or DELETE on users, and no DELETE on teams. The
+ # collections answer those, not the client, which only spells the endpoints
+ # that exist.
+ module Writes
+ # `title` and `body_html` are the two fields POST /issues requires.
+ def create_issue(attributes) = post_resource('issues', attributes)
+ def update_issue(id, attributes) = patch_resource('issues', id, attributes)
+ def delete_issue(id) = delete_resource('issues', id)
+
+ def create_account(attributes) = post_resource('accounts', attributes)
+ def update_account(id, attributes) = patch_resource('accounts', id, attributes)
+ def delete_account(id) = delete_resource('accounts', id)
+
+ def create_contact(attributes) = post_resource('contacts', attributes)
+ def update_contact(id, attributes) = patch_resource('contacts', id, attributes)
+ def delete_contact(id) = delete_resource('contacts', id)
+
+ def create_team(attributes) = post_resource('teams', attributes)
+ def update_team(id, attributes) = patch_resource('teams', id, attributes)
+
+ def update_user(id, attributes) = patch_resource('users', id, attributes)
+
+ private
+
+ def post_resource(resource, attributes)
+ operation = "create(#{resource})"
+
+ must_succeed(operation) { extract_written(connection.post(resource, attributes).body, operation) }
+ end
+
+ # The id comes from the record the operator acted on, so it is escaped
+ # before being joined to the path, like every read does.
+ def patch_resource(resource, id, attributes)
+ path = "#{resource}/#{Faraday::Utils.escape(id)}"
+ operation = "update(#{path})"
+
+ must_succeed(operation) { extract_updated(connection.patch(path, attributes).body, operation) }
+ end
+
+ # Answers true rather than the body: Pylon returns 200 or 204 with nothing
+ # worth reading, and a caller has no record left to serialize.
+ def delete_resource(resource, id)
+ path = "#{resource}/#{Faraday::Utils.escape(id)}"
+
+ must_succeed("delete(#{path})") do
+ connection.delete(path)
+ true
+ end
+ end
+
+ # Pylon answers a write with the written record under `data`. Anything else
+ # broke the contract: `extract_data` hands the body back untouched when
+ # `data` is absent, which is what a read wants and a write must not accept
+ # — the collection would serialize the envelope into a record with no id.
+ def extract_written(body, operation)
+ record = body['data'] if body.is_a?(Hash)
+ return record if record.is_a?(Hash)
+
+ refuse_body_shape(body, operation, "missing 'data'")
+ end
+
+ # An update discards its record, so a 204, an empty body or a null `data`
+ # is the write having landed with nothing to hand back: raising there would
+ # report a failure on a record Pylon already patched, and abort the records
+ # a bulk edit had left to write.
+ def extract_updated(body, operation)
+ record = body['data'] if body.is_a?(Hash)
+ return record if record.nil? || record.is_a?(Hash)
+
+ refuse_body_shape(body, operation, "'data' is not a record")
+ end
+
+ def refuse_body_shape(body, operation, detail)
+ raise APIError,
+ "Pylon API #{operation} returned an unexpected body shape (#{detail}): #{body.inspect}"
+ end
+ end
+ end
+end
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account.rb
index 0f5a09f0f..42bb57aca 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account.rb
@@ -4,6 +4,13 @@ class Account < CursorCollection
include SchemaDefinition
include Serializer
+ # Pylon reads an account's type back as `type` and takes it as
+ # `account_type`.
+ RENAMES = { 'type' => 'account_type' }.freeze
+
+ # An account is created enabled; only `PATCH /accounts/{id}` disables one.
+ UPDATE_ONLY = %w[is_disabled].freeze
+
def initialize(datasource, custom_fields: [])
super(datasource, 'PylonAccount', custom_fields: custom_fields, searchable: true)
end
@@ -12,6 +19,13 @@ def initialize(datasource, custom_fields: [])
def filter_table = ApiFilters
+ def create_record(payload) = datasource.client.create_account(payload)
+ def update_record(id, payload) = datasource.client.update_account(id, payload)
+ def delete_record(id) = datasource.client.delete_account(id)
+
+ def update_only_fields = UPDATE_ONLY
+ def payload_renames = RENAMES
+
def unsortable_warning
'[forest_admin_datasource_pylon] PylonAccount cannot honour the requested order; neither GET /accounts ' \
'nor POST /accounts/search takes a sort parameter, so accounts come back in the order the API imposes.'
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account/schema_definition.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account/schema_definition.rb
index 00bcc089f..bea8623a8 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account/schema_definition.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/account/schema_definition.rb
@@ -1,10 +1,12 @@
module ForestAdminDatasourcePylon
module Collections
class Account < CursorCollection
- # Every column is read-only in this story: writes land in a later one. No
- # column is sortable either — neither `GET /accounts` nor
- # `POST /accounts/search` exposes a sort parameter, so advertising a
- # sortable column would let the UI ask for an order the API cannot honour.
+ # A column is writable when `POST /accounts` or `PATCH /accounts/{id}`
+ # accepts it, in the shape it is read under — the Json columns holding
+ # objects rather than plain strings are left read-only, see below. No
+ # column is sortable — neither `GET /accounts` nor `POST /accounts/search`
+ # exposes a sort parameter, so advertising a sortable column would let the
+ # UI ask for an order the API cannot honour.
#
# Filter operators are not chosen here: they come from
# `ApiFilters::API_FILTERS`, which mirrors the allow-list of the API. A
@@ -41,29 +43,40 @@ def define_relations
def define_identity_fields
add_column('id', 'String', is_primary_key: true)
- add_column('name', 'String')
+ add_column('name', 'String', writable: true)
# Left as String rather than Enum: Pylon ships customer / partner /
- # prospect but lets an organization define its own account types.
- add_column('type', 'String')
- add_column('is_disabled', 'Boolean')
+ # prospect but lets an organization define its own account types. It
+ # is written under the name `account_type`, see `Account::RENAMES`.
+ add_column('type', 'String', writable: true)
+ # Writable on an update only: an account is created enabled.
+ add_column('is_disabled', 'Boolean', writable: true)
end
# `domain` and `primary_domain` carry the same value; both are kept
# because Pylon returns both, and only the `domains` list is filterable.
+ # Neither is writable: `domains` is the list the API takes, and writing
+ # one of its two projections would leave the other stale.
def define_domain_fields
add_column('domain', 'String')
add_column('primary_domain', 'String')
- add_column('domains', 'Json')
- add_column('tags', 'Json')
+ add_column('domains', 'Json', writable: true)
+ add_column('tags', 'Json', writable: true)
end
def define_ownership_fields
# Flattened from the nested `{ id: ..., email: ... }` object Pylon
# returns; a plain column, see `define_relations` above.
- add_column('owner_id', 'String')
+ add_column('owner_id', 'String', writable: true)
+ # Read-only although the endpoint takes it: the column shows
+ # `{external_id, label}` objects, and the write shape the reference
+ # documents is not that one — writing one for the other would replace
+ # the ids of the account with something it cannot read.
add_column('external_ids', 'Json')
end
+ # Both belong to the integrations Pylon syncs them from: `crm_settings`
+ # is absent from every write endpoint, and `channels` — which they do
+ # take — holds objects, like `external_ids` above.
def define_integration_fields
add_column('channels', 'Json')
add_column('crm_settings', 'Json')
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/base_collection.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/base_collection.rb
index f6b209458..ee82a9f5f 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/base_collection.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/base_collection.rb
@@ -1,6 +1,8 @@
module ForestAdminDatasourcePylon
module Collections
class BaseCollection < ForestAdminDatasourceToolkit::Collection
+ include Writes
+
ColumnSchema = ForestAdminDatasourceToolkit::Schema::ColumnSchema
ManyToOneSchema = ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema
OneToManySchema = ForestAdminDatasourceToolkit::Schema::Relations::OneToManySchema
@@ -198,18 +200,18 @@ def api_filters
end
end
- # A native column: read-only in this story — writes land in a later one —
+ # A native column: read-only unless the collection declares it `writable`,
# and never groupable, as no Pylon endpoint aggregates. It is not sortable
# either, the ColumnSchema default, because no search endpoint takes a sort
# parameter. Filter operators are not chosen here: they come from
# `filter_table`, which mirrors the allow-list of the API, so a column
# missing from it gets none and the UI offers no filter Pylon would refuse.
- def add_column(name, type, is_primary_key: false)
+ def add_column(name, type, is_primary_key: false, writable: false)
add_field(name, ColumnSchema.new(column_type: type,
filter_operators: filter_table.forest_operators(name),
is_primary_key: is_primary_key,
is_groupable: false,
- is_read_only: true))
+ is_read_only: !writable))
end
# A record read through the endpoint of an id that is not the primary key
@@ -262,6 +264,11 @@ def default_pk_sort?(sort)
normalized_sort_clauses(sort) == normalized_sort_clauses(SortFactory.by_primary_keys(self))
end
+ # The search box sends an empty string once the operator clears it.
+ def no_search?(filter)
+ filter&.search.to_s.strip.empty?
+ end
+
def timezone_for(caller)
return 'UTC' unless caller.respond_to?(:timezone)
@@ -332,11 +339,15 @@ def walker
@walker ||= Pagination::CursorWalker.new
end
+ # A set of ids, not a list: the same one named twice is one record, so a
+ # lookup spends one request on it and a delete does not answer 404 the
+ # second time. The caps count records rather than mentions for the same
+ # reason.
def id_values(node)
return nil unless node.is_a?(Leaf) && node.field == 'id'
return nil unless [Operators::EQUAL, Operators::IN].include?(node.operator)
- Array(node.value).map(&:to_s).reject(&:empty?)
+ Array(node.value).map(&:to_s).reject(&:empty?).uniq
end
def and_branch?(node)
@@ -346,9 +357,11 @@ def and_branch?(node)
# An `id` the short-circuit could not take out of the tree has no
# translation left: the endpoint filters no id server-side, and an id under
# an OR cannot be narrowed to a lookup because the other side of the union
- # would bring in records the lookup never fetched. The UI does offer both
- # an `id equals` filter and the or/and toggle, so this is worth an error an
- # operator can act on rather than the translator's "add it to api_filters".
+ # would bring in records the lookup never fetched. Worth an error an
+ # operator can act on rather than the translator's "add it to api_filters",
+ # because two things they do reach it: the `id equals` filter next to the
+ # or/and toggle, and an excluding selection — "every record except these" —
+ # which arrives as `id not_in` and is no filter they wrote.
#
# A collection whose endpoint does filter id declares it in `api_filters`
# and never short-circuits, so the translator handles its ids like any
@@ -358,9 +371,11 @@ def ensure_no_stray_id!(node)
return unless node.some_leaf { |leaf| leaf.field == 'id' }
raise UnsupportedOperatorError,
- "A filter on 'id' has to be combined with 'and' conditions only: Pylon cannot filter on id, so the " \
- 'agent reads the records by id and applies the rest in memory, which an id inside an `or` would ' \
- 'silently widen. Rewrite the filter with `and`, or filter on another field.'
+ "#{name} cannot answer this selection: Pylon cannot filter on id, so the agent reads the records " \
+ 'by id and applies the rest in memory, which only an `and` of `id equals` / `id in` conditions ' \
+ 'names a set of records to read. An id inside an `or` names none, and neither does an exclusion, ' \
+ 'which is what selecting every record except a few sends. Select the records to act on rather ' \
+ 'than the ones to leave out, rewrite the filter with `and`, or filter on another field.'
end
def resolve_relation_conditions(caller, node)
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact.rb
index 822df0797..3799e615e 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact.rb
@@ -4,6 +4,11 @@ class Contact < CursorCollection
include SchemaDefinition
include Serializer
+ # `POST /contacts` takes the primary address alone; the other ones are set
+ # on an existing contact, through the list.
+ CREATE_ONLY = %w[email].freeze
+ UPDATE_ONLY = %w[emails].freeze
+
def initialize(datasource, custom_fields: [])
super(datasource, 'PylonContact', custom_fields: custom_fields, searchable: true)
end
@@ -12,6 +17,13 @@ def initialize(datasource, custom_fields: [])
def filter_table = ApiFilters
+ def create_record(payload) = datasource.client.create_contact(payload)
+ def update_record(id, payload) = datasource.client.update_contact(id, payload)
+ def delete_record(id) = datasource.client.delete_contact(id)
+
+ def create_only_fields = CREATE_ONLY
+ def update_only_fields = UPDATE_ONLY
+
def unsortable_warning
'[forest_admin_datasource_pylon] PylonContact cannot honour the requested order; neither GET /contacts ' \
'nor POST /contacts/search takes a sort parameter, so contacts come back in the order the API imposes.'
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact/schema_definition.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact/schema_definition.rb
index 176a83a08..aaa804abd 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact/schema_definition.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/contact/schema_definition.rb
@@ -1,10 +1,12 @@
module ForestAdminDatasourcePylon
module Collections
class Contact < CursorCollection
- # Every column is read-only in this story: writes land in a later one. No
- # column is sortable either — neither `GET /contacts` nor
- # `POST /contacts/search` exposes a sort parameter, so advertising a
- # sortable column would let the UI ask for an order the API cannot honour.
+ # A column is writable when `POST /contacts` or `PATCH /contacts/{id}`
+ # accepts it, in the shape it is read under — the Json columns holding
+ # objects rather than plain strings are left read-only, see below. No
+ # column is sortable — neither `GET /contacts` nor `POST /contacts/search`
+ # exposes a sort parameter, so advertising a sortable column would let the
+ # UI ask for an order the API cannot honour.
#
# Filter operators are not chosen here: they come from
# `ApiFilters::API_FILTERS`, which mirrors the allow-list of the API. A
@@ -41,11 +43,12 @@ def define_relations
def define_identity_fields
add_column('id', 'String', is_primary_key: true)
- add_column('name', 'String')
+ add_column('name', 'String', writable: true)
# Flattened from the nested `{ id: ..., external_ids: ... }` object
# Pylon returns, and kept as a column next to the `account` relation
- # it is the key of: the search endpoint filters it.
- add_column('account_id', 'String')
+ # it is the key of: the search endpoint filters it. Writable, which is
+ # what opens the relation editor — see the party fields of PylonIssue.
+ add_column('account_id', 'String', writable: true)
# Read-only Json, and deliberately unfilterable although the search
# endpoint does not offer it either: the API matches bare external-id
# strings while the column shows `{external_id, label}` objects, so a
@@ -55,20 +58,33 @@ def define_identity_fields
# `email` and `primary_phone_number` carry the primary value; the lists
# hold every address and number, and neither list is filterable.
+ #
+ # `email` is written on a create and `emails` on an update, one direction
+ # each: `POST /contacts` takes the primary address alone, and the other
+ # ones are set on an existing contact. Two writable projections of the
+ # same addresses would otherwise travel in one patch, the list leaving
+ # out whatever the primary carries.
+ #
+ # `phone_numbers` is not writable at all: it holds objects, and the
+ # shape the endpoint takes them in is not the one the column shows.
def define_contact_fields
- add_column('email', 'String')
- add_column('emails', 'Json')
- add_column('primary_phone_number', 'String')
+ add_column('email', 'String', writable: true)
+ add_column('emails', 'Json', writable: true)
+ add_column('primary_phone_number', 'String', writable: true)
add_column('phone_numbers', 'Json')
- add_column('avatar_url', 'String')
+ add_column('avatar_url', 'String', writable: true)
end
def define_portal_fields
# Left as String rather than Enum: Pylon documents no_access / member
# / admin, but an organization can define its own portal roles, which
- # is what `portal_role_id` points at.
+ # is what `portal_role_id` points at. The id is the one written and the
+ # name is read-only, like `role_id` and `role_name` on PylonUser:
+ # whichever of two projections Pylon ignored would come back stale.
add_column('portal_role', 'String')
- add_column('portal_role_id', 'String')
+ add_column('portal_role_id', 'String', writable: true)
+ # Owned by the integrations the contact was seen through; no endpoint
+ # takes it.
add_column('integration_user_ids', 'Json')
end
end
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/cursor_collection.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/cursor_collection.rb
index 797f8e6ed..43d68a666 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/cursor_collection.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/cursor_collection.rb
@@ -125,11 +125,6 @@ def records_by_id(id)
[]
end
-
- # The search box sends an empty string once the operator clears it.
- def no_search?(filter)
- filter.search.to_s.strip.empty?
- end
end
end
end
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/fetch_all_collection.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/fetch_all_collection.rb
index 9cac206d3..f29e8c851 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/fetch_all_collection.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/fetch_all_collection.rb
@@ -76,18 +76,18 @@ def records_indexed_by_id(ids)
protected
- # Every column is read-only in this story: writes land in a later one.
+ # A column is read-only unless the collection declares it `writable`.
# Scalar columns are sortable and groupable because the in-memory sort and
# aggregation honour anything asked of them; a Json column is none of the
# three, as it holds a list whose Pylon semantics have no in-memory
# counterpart — the same reason the primary-key residual guard refuses one.
- def add_column(name, type, is_primary_key: false)
+ def add_column(name, type, is_primary_key: false, writable: false)
add_field(name, ColumnSchema.new(column_type: type,
filter_operators: self.class.operators_for(type),
is_primary_key: is_primary_key,
is_sortable: type != 'Json',
is_groupable: type != 'Json',
- is_read_only: true))
+ is_read_only: !writable))
end
# Pylon defines custom fields on issues, accounts and contacts only, so
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue.rb
index fdf48b40f..1dac463f2 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue.rb
@@ -46,6 +46,15 @@ class Issue < BaseCollection
# that would let this cap grow.
MAX_MESSAGE_EMBEDS = 10
+ # `body_html` is the first message of the thread, which `POST /issues`
+ # requires and `PATCH /issues/{id}` does not carry; `author_unverified`
+ # qualifies that message and travels with it.
+ CREATE_ONLY = %w[body_html author_unverified].freeze
+
+ # Pylon creates every issue as `new`, of the type it decides, and takes
+ # both on an update only.
+ UPDATE_ONLY = %w[state type].freeze
+
def initialize(datasource, custom_fields: [])
super(datasource, 'PylonIssue', custom_fields: custom_fields, searchable: true)
end
@@ -62,6 +71,27 @@ def list(caller, filter, projection)
def filter_table = ApiFilters
+ def create_record(payload) = datasource.client.create_issue(payload)
+ def update_record(id, payload) = datasource.client.update_issue(id, payload)
+ def delete_record(id) = datasource.client.delete_issue(id)
+
+ def create_only_fields = CREATE_ONLY
+ def update_only_fields = UPDATE_ONLY
+
+ # An issue is read through `GET /issues/{id}`, one request per record: a
+ # selection resolved or compared that way spends the write budget twice
+ # over, so it divides the records one pass reaches rather than fitting
+ # beside them.
+ def requests_per_record_read = 1
+
+ # Never past the primary-key fan-out either: a write resolving named ids
+ # through `list` goes through `fetch_by_ids`, which truncates with a
+ # warning past this many, and a truncated resolution would write to a
+ # subset of the selection while reporting the whole of it. The budget is
+ # the tighter of the two at today's numbers; the clamp keeps that true if
+ # either moves.
+ def max_resolvable_ids(reads: 0) = [super, MAX_ID_LOOKUPS].min
+
def sortable_fields
PYLON_SORTABLE
end
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue/schema_definition.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue/schema_definition.rb
index 838d04045..dae0fda73 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue/schema_definition.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/issue/schema_definition.rb
@@ -1,9 +1,11 @@
module ForestAdminDatasourcePylon
module Collections
class Issue < BaseCollection
- # Every column is read-only in this story: writes land in a later one. No
- # column is sortable either — `/issues/search` exposes no sort parameter at
- # all, results always come back ordered by `created_at` descending, so
+ # A column is writable when `POST /issues` or `PATCH /issues/{id}` accepts
+ # it, the two directions being told apart by `Issue::CREATE_ONLY` and
+ # `Issue::UPDATE_ONLY`; everything Pylon computes stays read-only. No
+ # column is sortable, `/issues/search` exposing no sort parameter at all:
+ # results always come back ordered by `created_at` descending, so
# advertising a sortable column would let the UI ask for an order the API
# cannot honour.
#
@@ -57,16 +59,20 @@ def define_identity_fields
end
def define_content_fields
- add_column('title', 'String')
- add_column('body_html', 'String')
+ add_column('title', 'String', writable: true)
+ # Writable on creation only: it is the first message of the thread,
+ # which `PATCH /issues/{id}` does not carry.
+ add_column('body_html', 'String', writable: true)
# Left as String rather than Enum: Pylon ships five built-in states
- # but organisations define their own on top of them.
- add_column('state', 'String')
- add_column('type', 'String')
+ # but organisations define their own on top of them. Writable on an
+ # update only — every issue is created `new`.
+ add_column('state', 'String', writable: true)
+ add_column('type', 'String', writable: true)
+ # Where the issue came from: Pylon sets it, no endpoint takes it.
add_column('source', 'String')
- add_column('tags', 'Json')
- add_column('customer_portal_visible', 'Boolean')
- add_column('author_unverified', 'Boolean')
+ add_column('tags', 'Json', writable: true)
+ add_column('customer_portal_visible', 'Boolean', writable: true)
+ add_column('author_unverified', 'Boolean', writable: true)
add_column('number_of_touches', 'Number')
define_thread_field
end
@@ -90,8 +96,16 @@ def define_thread_field
# Flattened from the nested `{id: …}` objects Pylon returns, and kept as
# columns next to the relations they are the keys of: they are what the
# search endpoint filters, on this side and on the reverse one.
+ #
+ # Writable, although `GeneratorField` forces a foreign key read-only in
+ # the emitted schema whatever the datasource says, so the detail view has
+ # one editor per key rather than two. What the flag opens is that editor:
+ # the `BelongsTo` reads its own read-only state off the key column, and
+ # the front sends the choice back as the very column named here.
def define_party_fields
- %w[account_id requester_id assignee_id team_id].each { |field| add_column(field, 'String') }
+ %w[account_id requester_id assignee_id team_id].each do |field|
+ add_column(field, 'String', writable: true)
+ end
end
def define_time_fields
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/team.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/team.rb
index d4e2aca49..1f621df12 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/team.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/team.rb
@@ -7,6 +7,10 @@ def initialize(datasource)
protected
+ # Pylon exposes no DELETE on a team, so that hook is left to refuse.
+ def create_record(payload) = datasource.client.create_team(payload)
+ def update_record(id, payload) = datasource.client.update_team(id, payload)
+
def fetch_all
datasource.client.fetch_teams
end
@@ -33,10 +37,12 @@ def define_relations
def define_schema
add_column('id', 'String', is_primary_key: true)
- add_column('name', 'String')
+ add_column('name', 'String', writable: true)
# A list, so neither filterable nor sortable, and no relation either:
- # see `define_relations` above.
- add_column('user_ids', 'Json')
+ # see `define_relations` above. `POST /teams` and `PATCH /teams/{id}`
+ # take the members as this very list, and the one sent replaces the
+ # membership whole.
+ add_column('user_ids', 'Json', writable: true)
end
end
end
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/user.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/user.rb
index 7b90eedfe..149cb0918 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/user.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/user.rb
@@ -9,6 +9,10 @@ def initialize(datasource)
protected
+ # Pylon exposes no POST and no DELETE on a user: an agent is invited and
+ # deactivated from Pylon itself, so the other two hooks are left to refuse.
+ def update_record(id, payload) = datasource.client.update_user(id, payload)
+
# `include_deactivated` is left at the client default of true on purpose:
# a deactivated agent stays the assignee and the author of the issues they
# handled, and a record the rest of the panel points at has to stay
@@ -42,19 +46,21 @@ def define_relations
origin_key: 'assignee_id', origin_key_target: 'id'))
end
+ # `PATCH /users/{id}` takes the name, the avatar, the role and the status,
+ # and nothing else.
def define_schema
add_column('id', 'String', is_primary_key: true)
- add_column('name', 'String')
+ add_column('name', 'String', writable: true)
add_column('email', 'String')
# The other addresses of the same agent: a list, so it is neither
# filterable nor sortable. `email` carries the primary one.
add_column('emails', 'Json')
- add_column('avatar_url', 'String')
+ add_column('avatar_url', 'String', writable: true)
# Left as String rather than Enum: Pylon documents active / away /
# out_of_office on the update endpoint, but does not promise the read
# side is limited to them.
- add_column('status', 'String')
- add_column('role_id', 'String')
+ add_column('status', 'String', writable: true)
+ add_column('role_id', 'String', writable: true)
add_column('role_name', 'String')
add_column('is_deactivated', 'Boolean')
end
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/writes.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/writes.rb
new file mode 100644
index 000000000..313e46b39
--- /dev/null
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/writes.rb
@@ -0,0 +1,377 @@
+module ForestAdminDatasourcePylon
+ module Collections
+ # The write half of every Pylon collection. Included by `BaseCollection`, so
+ # a collection only declares the client calls, through the `*_record` hooks,
+ # and the fields Pylon accepts in one direction only. A hook left alone
+ # refuses the verb — no POST or DELETE on users, no DELETE on teams —
+ # instead of the contract's NotImplementedError, read by the agent as a 500.
+ #
+ # What may be written is `is_read_only` on the column, the way `api_filters`
+ # is what may be filtered: no second list to keep in step with the schema.
+ module Writes # rubocop:disable Metrics/ModuleLength
+ # Re-declared rather than borrowed from BaseCollection: a method defined
+ # here resolves a constant against this module and its ancestors, never
+ # against the class including it.
+ Filter = ForestAdminDatasourceToolkit::Components::Query::Filter
+ Page = ForestAdminDatasourceToolkit::Components::Query::Page
+ Projection = ForestAdminDatasourceToolkit::Components::Query::Projection
+ Leaf = ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf
+ Operators = ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators
+
+ # What one filter-driven update or delete may spend, in requests. Pylon
+ # allows 10 to 20 per minute, so a costlier selection is refused rather
+ # than written halfway.
+ #
+ # The budget covers the whole pass, not its writes: where a record is read
+ # through its own endpoint, resolving the selection costs a request per
+ # record and reading a stored value costs another, so a cap counting the
+ # writes alone would let one pass spend three times this.
+ MAX_WRITE_REQUESTS = 20
+
+ # The refusal comes before the payload and the ids: everything on the way
+ # there answers with something else — a count, a field of the wrong
+ # direction — for a selection that was never the problem.
+ def create(_caller, data)
+ refuse_write('created') unless write_endpoint?(:create_record)
+
+ serialize(create_record(build_payload(writable_attributes(data), :create)))
+ rescue APIError => e
+ surface_write_rejection(e)
+ end
+
+ # What the patch may write is settled before the ids are, so a patch naming
+ # nothing writable is not refused for reaching too many records.
+ def update(caller, filter, patch)
+ refuse_write('updated') unless write_endpoint?(:update_record)
+
+ attributes = writable_attributes(patch)
+ return if attributes.empty?
+
+ ids = ids_for(caller, filter, extra_reads: stored_read?(attributes) ? 1 : 0)
+ return if ids.empty?
+
+ payload = build_payload(attributes, :update, caller: caller, ids: ids)
+ return if payload.empty?
+
+ write_each(ids, 'updated') { |id| update_record(id, payload) }
+ end
+
+ def delete(caller, filter)
+ refuse_write('deleted') unless write_endpoint?(:delete_record)
+
+ write_each(ids_for(caller, filter), 'deleted') { |id| delete_record(id) }
+ end
+
+ protected
+
+ # One Pylon write endpoint each, overridden by the collections having one.
+ def create_record(_payload) = refuse_write('created')
+ def update_record(_id, _payload) = refuse_write('updated')
+ def delete_record(_id) = refuse_write('deleted')
+
+ # The Forest schema carries a single read-only flag per column, so both
+ # directions offer these; the two lists tell them apart at write time.
+ def create_only_fields = [].freeze
+ def update_only_fields = [].freeze
+
+ # Columns whose Pylon write name differs from the one they are read under.
+ def payload_renames = {}.freeze
+
+ # What reading one record costs here. Nothing where the search endpoint
+ # filters `id`, a whole selection travelling in one request whatever its
+ # size; one request where an id is read through its own endpoint.
+ def requests_per_record_read = 0
+
+ # How many records one write may reach: the budget divided by what each of
+ # them costs — the write itself, plus the `reads` the path still owes it.
+ def max_write_targets(reads: 0)
+ MAX_WRITE_REQUESTS / (1 + (reads * requests_per_record_read))
+ end
+
+ # How many ids a filter may name before the resolution is refused rather
+ # than spent: the same reach, a named id being read before it is written
+ # to. `nil` is no bound, a read costing nothing per record.
+ def max_resolvable_ids(reads: 0)
+ return nil if requests_per_record_read.zero?
+
+ max_write_targets(reads: reads)
+ end
+
+ # The records a filter-driven write applies to: exact, or refused — the
+ # caller writes one request per id and reports success for the whole
+ # selection, so a subset may never be answered quietly.
+ #
+ # An `id equals`/`id in` filter alone — what the record detail and the bulk
+ # selection send — costs no request. Anything else goes through `list`.
+ def ids_for(caller, filter, extra_reads: 0)
+ tree = filter&.condition_tree
+ if (named = id_values(tree)) && no_search?(filter)
+ cap = max_write_targets(reads: extra_reads)
+ refuse_too_many_targets(named.size, cap) if named.size > cap
+ return named
+ end
+
+ # A selection naming ids is resolved by reading each of them; any other
+ # one by a single page of the collection's own read, whose cost does not
+ # grow with the count.
+ named_ids = filtered_ids(tree)
+ reads = extra_reads + (named_ids ? 1 : 0)
+ bound = named_ids && max_resolvable_ids(reads: reads)
+ refuse_unresolvable_selection(named_ids.size, bound) if bound && named_ids.size > bound
+
+ resolve_ids_by_list(caller, filter, reads: reads)
+ end
+
+ private
+
+ # The `*_record` hook is the declaration that the collection wired the
+ # endpoint, read here rather than repeated in a list of supported verbs.
+ def write_endpoint?(hook)
+ method(hook).owner != Writes
+ end
+
+ # One request per record, so a failure on the k-th leaves the k-1 before it
+ # written. The error names them: raising the API error alone reads as
+ # "nothing happened", and retrying on that reading would write them twice.
+ def write_each(ids, verb)
+ written = []
+
+ ids.each do |id|
+ yield id
+ written << id
+ rescue APIError => e
+ # Always raises, so nothing reaches the partial report below: with no
+ # record written the failure is the whole of what happened.
+ surface_write_rejection(e) if written.empty?
+
+ refuse_partial_write(verb, written, id, ids.size, e)
+ end
+ end
+
+ # A 4xx names something the operator did, and travels as the
+ # ValidationError whose message the agent surfaces where the APIError it
+ # arrived as would be answered with 'Unexpected error'. Anything else is
+ # Pylon or the network failing, which no edit of theirs would change.
+ #
+ # Only the write goes through here: a 4xx raised while resolving the
+ # selection still reaches them as a 500, reporting a read failure as a
+ # refused write being the worse of the two.
+ def surface_write_rejection(error)
+ raise error unless (400..499).cover?(error.status.to_i)
+
+ raise WriteRejectedError, error.message
+ end
+
+ # One record past the cap is asked for, so an overflow is seen rather than
+ # guessed from a full page.
+ def resolve_ids_by_list(caller, filter, reads:)
+ cap = max_write_targets(reads: reads)
+ window = Page.new(offset: 0, limit: cap + 1)
+ query = (filter || Filter.new).override(page: window)
+ records = list(caller, query, Projection.new(['id']))
+ refuse_unbounded_targets(cap) if records.size > cap
+
+ records.filter_map { |record| record['id'] }.uniq
+ end
+
+ # The ids a filter names, as a leaf of its own or inside a top-level `and`.
+ # Unlike `extract_id_lookup`, nothing is asserted about the rest of the
+ # tree — the leftovers travel to `list` — nor about the sibling conditions,
+ # so this counts records *named*, never records the write applies to. The
+ # first id leaf of an `and` of two wins, over-refusing a narrower one.
+ def filtered_ids(node)
+ named = id_values(node)
+ return named if named
+ return nil unless and_branch?(node)
+
+ Array(node.conditions).filter_map { |child| id_values(child) }.first
+ end
+
+ # The writable attributes, in the shape the endpoint takes them.
+ def build_payload(attributes, direction, caller: nil, ids: [])
+ attrs = honour_write_direction(attributes, direction, caller, ids)
+ # Pylon fills in what a create leaves out; on an update a nil is the
+ # operator clearing a value, so it travels.
+ attrs = attrs.compact if direction == :create
+
+ custom, native = split_custom_fields(attrs)
+ payload = native.transform_keys { |field| payload_renames.fetch(field, field) }
+ payload['custom_fields'] = custom unless custom.empty?
+ payload
+ end
+
+ # Everything else is dropped rather than refused: the front sends the
+ # fields of its form, and a read-only one reaching the payload is the
+ # agent's doing, not a request the operator made.
+ def writable_attributes(data)
+ attrs = data.is_a?(Hash) ? data.transform_keys(&:to_s) : {}
+
+ attrs.select { |field, _value| writable_column?(field) }
+ end
+
+ def writable_column?(field)
+ column = schema[:fields][field]
+
+ column&.type == 'Column' && !column.is_read_only
+ end
+
+ # Whether the patch will have `stored_values` read every record it reaches
+ # before a field of the wrong direction is dropped or refused, which the
+ # cap has to charge it for: see `ids_for`.
+ def stored_read?(attributes) = (attributes.keys & create_only_fields).any?
+
+ # A field of the other direction is dropped when it asks for nothing, and
+ # refused when the operator really changed it: answering an edit with a
+ # success Pylon did not perform is worse than an error naming the field.
+ #
+ # Only a create can tell without reading, Pylon filling it in with exactly
+ # what a blank value asks for. On an update the stored value is what
+ # settles it — an unchecked box is nothing over a stored `false`, and a
+ # real edit over a stored `true`.
+ def honour_write_direction(attrs, direction, caller, ids)
+ wrong = attrs.keys & (direction == :create ? update_only_fields : create_only_fields)
+ return attrs if wrong.empty?
+
+ asked = if direction == :create
+ wrong.reject { |field| blank_write_value?(attrs[field]) }
+ else
+ wrong - unchanged_fields(caller, ids, wrong, attrs)
+ end
+ refuse_wrong_direction(asked, direction) unless asked.empty?
+
+ attrs.except(*wrong)
+ end
+
+ # What a form sends for a field the operator never touched: no value at
+ # all, an unchecked box, an empty list. A `0` or a string is a value only
+ # the other endpoint could write.
+ def blank_write_value?(value)
+ return true if value.nil? || value == false
+ return value.empty? if value.respond_to?(:empty?)
+
+ false
+ end
+
+ # The wrong-direction fields already holding the value the patch asks for.
+ # One record the read did not hand back is enough to refuse them all:
+ # nothing here may claim a value is unchanged on a record it never read.
+ def unchanged_fields(caller, ids, fields, attrs)
+ return [] if fields.empty?
+
+ stored = stored_values(caller, ids, fields)
+ return [] if stored.size < ids.size
+
+ fields.select { |field| stored.all? { |record| same_write_value?(record[field], attrs[field]) } }
+ end
+
+ # Two blanks are the same state: Pylon returns a null where the form sends
+ # `false` or an empty string for the same untouched field. Strings are
+ # compared stripped, `body_html` travelling through an editor that may hand
+ # back the markup it was given re-indented — and refusing an edit nobody
+ # made is the one error the operator cannot act on.
+ def same_write_value?(stored, asked)
+ return true if blank_write_value?(stored) && blank_write_value?(asked)
+ return stored.to_s.strip == asked.to_s.strip if stored.is_a?(String) || asked.is_a?(String)
+
+ stored == asked
+ end
+
+ # Read only when the patch names a field of the wrong direction, and only
+ # for those fields. One request where the endpoint filters `id`, one per
+ # record where an id is read through its own endpoint — which the cap does
+ # charge the patch for, `stored_read?` declaring it before the ids are
+ # resolved.
+ #
+ # By id rather than through the caller's filter: that filter was already
+ # resolved into these ids, so re-running it would spend those requests
+ # twice and, carrying no page, walk every record it matches.
+ def stored_values(caller, ids, fields)
+ query = Filter.new(condition_tree: Leaf.new('id', Operators::IN, ids),
+ page: Page.new(offset: 0, limit: ids.size))
+
+ list(caller, query, Projection.new(['id'] + fields))
+ end
+
+ # Pylon reads its custom fields back as a map indexed by slug and writes
+ # them as a list, `values` for a multi-value field and `value` for every
+ # other — a select by the slug of its option, what the Enum advertises.
+ def split_custom_fields(attrs)
+ by_column = custom_fields_by_column
+ entries = []
+
+ native = attrs.each_with_object({}) do |(field, value), rest|
+ custom_field = by_column[field]
+ custom_field ? entries << custom_field_entry(custom_field, value) : rest[field] = value
+ end
+
+ [entries, native]
+ end
+
+ def custom_fields_by_column
+ @custom_fields_by_column ||= custom_fields.to_h { |field| [field[:column_name], field] }
+ end
+
+ def custom_field_entry(custom_field, value)
+ slug = custom_field[:column_name]
+ return { 'slug' => slug, 'values' => Array(value) } if custom_field[:multi_value]
+
+ { 'slug' => slug, 'value' => value }
+ end
+
+ def refuse_write(verb)
+ raise UnsupportedWriteError,
+ "A #{name} record cannot be #{verb}: the Pylon API exposes no endpoint for it."
+ end
+
+ # Every offending field at once: refusing them one at a time would have the
+ # operator undo one, retry, and learn about the next.
+ def refuse_wrong_direction(fields, direction)
+ them = fields.one? ? 'it' : 'them'
+ detail = if direction == :create
+ "Pylon only accepts #{them} on an existing record: create the record, then edit it."
+ else
+ "Pylon only accepts #{them} when the record is created, and exposes no endpoint to change " \
+ "#{them} afterwards."
+ end
+
+ named = fields.map { |field| "'#{field}'" }.join(', ')
+ raise UnsupportedWriteError, "#{named} cannot be set here on a #{name}: #{detail}"
+ end
+
+ # The count is exact here, the filter having named the ids.
+ def refuse_too_many_targets(count, cap)
+ refuse_write_reach("applies to #{count} #{name} records, more than the #{cap} one pass covers")
+ end
+
+ # The resolution only knows the selection overflows: reporting the size of
+ # its window would name 21 records to a selection holding thousands.
+ def refuse_unbounded_targets(cap)
+ refuse_write_reach("applies to more than the #{cap} #{name} records one pass covers")
+ end
+
+ def refuse_write_reach(reach)
+ raise UnsupportedWriteError,
+ "This write #{reach}: Pylon writes one record per request, against a budget of ten to twenty " \
+ 'requests per minute, and a write stopping halfway would report a success it did not perform. ' \
+ 'Narrow the selection to reach the records past this point.'
+ end
+
+ # How many of the named ids the rest of the filter matches is unknown here,
+ # so the count is reported as what it is: records named.
+ def refuse_unresolvable_selection(count, bound)
+ raise UnsupportedWriteError,
+ "This write names #{count} #{name} records and filters them further, which #{name} answers with " \
+ "one request per named record, on top of the one each write costs: more than the #{bound} one " \
+ 'pass covers. Select fewer records, or drop the other conditions to write the ones named.'
+ end
+
+ def refuse_partial_write(verb, written, failed_id, total, error)
+ raise PartialWriteError,
+ "#{written.size} of #{total} #{name} records were #{verb} and then '#{failed_id}' failed: " \
+ "#{error.message}. The records already #{verb} are #{written.join(", ")}, and they stay " \
+ "#{verb} — the ones after them were left untouched. Retry the write on the untouched records " \
+ "alone: retrying the whole selection would perform it twice on the ones already #{verb}."
+ end
+ end
+ end
+end
diff --git a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/retry_policy.rb b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/retry_policy.rb
index 5aaa3a1ae..5b1224a86 100644
--- a/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/retry_policy.rb
+++ b/packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/retry_policy.rb
@@ -19,12 +19,17 @@ class RetryPolicy
Faraday::RetriableResponse, Faraday::ConnectionFailed
].freeze
- # Faraday only retries these by default; a 429 is safe to retry on any verb
- # because Pylon rejected the request before processing it, whereas a 502 on a
- # POST /issues may well have created the issue. This has to go through
- # retry_if rather than methods: faraday-retry ORs the two, so methods can
- # only widen the set, never restrict it.
- IDEMPOTENT_METHODS = %i[delete get head options put].freeze
+ # The verbs that change nothing, so any transient failure is worth another
+ # attempt. Narrower than faraday-retry's idempotent default: a 502 or a
+ # dropped connection on the way back from a DELETE Pylon did perform is
+ # replayed into a 404, which the write path then surfaces as a deletion that
+ # failed when it landed.
+ #
+ # A 429 stays safe to retry on any verb, Pylon having rejected the request
+ # before processing it, and travels through retry_if rather than through this
+ # list: faraday-retry ORs the two, so methods can only widen the set, never
+ # restrict it.
+ RETRYABLE_METHODS = %i[get head options].freeze
RETRY_IF = ->(env, _exception) { env[:status] == 429 }
BACKOFF_FACTOR = 2
@@ -45,7 +50,7 @@ def to_faraday_options
backoff_factor: BACKOFF_FACTOR,
retry_statuses: STATUSES,
exceptions: EXCEPTIONS,
- methods: IDEMPOTENT_METHODS,
+ methods: RETRYABLE_METHODS,
retry_if: RETRY_IF
}
end
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
index 2286d64e4..c98e8827f 100644
--- 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
@@ -1,8 +1,9 @@
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.
+ # entries shaped `{ column_name:, schema:, multi_value: }` — what
+ # `add_custom_fields` registers on a collection, and what the payload
+ # builder writes a value back through.
#
# `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
@@ -38,6 +39,9 @@ class CustomFieldsIntrospector
'multiselect' => 'Json'
}.freeze
+ # The types Pylon writes back through `values` rather than `value`.
+ MULTI_VALUE_TYPES = %w[multiselect].freeze
+
BASE_OPS = (Maps::EQUALITY.keys + Maps::PRESENCE.keys).freeze
# A date drops the membership operators on the way, `Rules` granting a DATE
@@ -97,19 +101,19 @@ def build_entry(raw, object_type)
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) }
+ { column_name: slug, schema: build_schema(raw, column_type),
+ multi_value: MULTI_VALUE_TYPES.include?(raw['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.
+ # A custom field is writable when Pylon says it is: it flags the ones
+ # synced from an app or an integration, which its own endpoints refuse.
+ # Nothing is sortable -- no Pylon endpoint takes a sort parameter -- and
+ # nothing is groupable: 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_read_only: !writable_definition?(raw),
is_sortable: false,
is_groupable: false }
@@ -137,6 +141,28 @@ def option_slugs(raw)
end
end
+ # Only an explicit `false` opens a custom field to writes. A definition
+ # carrying no flag at all is left read-only and reported: this datasource
+ # advertises nothing an endpoint would refuse, and reading the absence as
+ # "writable" would turn every field synced from an app into an editor whose
+ # every save Pylon rejects -- where reading it as "read-only" costs the
+ # capability and says so once per boot.
+ def writable_definition?(raw)
+ return true if raw['is_read_only'] == false
+ return false if raw['is_read_only'] == true
+
+ warn_unflagged_writability(raw)
+ false
+ end
+
+ def warn_unflagged_writability(raw)
+ ForestAdminDatasourcePylon.logger.warn(
+ "[forest_admin_datasource_pylon] Custom field '#{raw["slug"]}' carries no 'is_read_only' flag; " \
+ 'leaving it read-only. Pylon refuses a write on the fields it syncs from an app or an integration, ' \
+ 'and nothing here can tell this one apart from those without the flag.'
+ )
+ end
+
def warn_unknown_type(raw, slug, object_type)
ForestAdminDatasourcePylon.logger.warn(
"[forest_admin_datasource_pylon] Custom field '#{slug}' on #{object_type} has type " \
diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/client/writes_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/client/writes_spec.rb
new file mode 100644
index 000000000..4b0e8fe72
--- /dev/null
+++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/client/writes_spec.rb
@@ -0,0 +1,159 @@
+RSpec.describe ForestAdminDatasourcePylon::Client::Writes do
+ let(:retry_policy) { ForestAdminDatasourcePylon::RetryPolicy.new(max_retries: 2, interval: 0) }
+ let(:configuration) { ForestAdminDatasourcePylon::Configuration.new(api_key: 'k', retry_policy: retry_policy) }
+ let(:client) { ForestAdminDatasourcePylon::Client.new(configuration) }
+ let(:base) { configuration.url }
+
+ def json(payload, status = 200)
+ { status: status, body: payload.is_a?(String) ? payload : payload.to_json,
+ headers: { 'Content-Type' => 'application/json' } }
+ end
+
+ # One method per endpoint, and the endpoint is the whole of what each one
+ # knows: the payload is the collection's to build.
+ describe 'the endpoint each write reaches' do
+ {
+ create_issue: [:post, 'issues'], create_account: [:post, 'accounts'],
+ create_contact: [:post, 'contacts'], create_team: [:post, 'teams']
+ }.each do |method, (verb, path)|
+ it "#{method} posts to /#{path}" do
+ stub_request(verb, "#{base}/#{path}").to_return(json('data' => { 'id' => 'x' }))
+
+ expect(client.public_send(method, 'name' => 'Acme')).to eq('id' => 'x')
+ expect(WebMock).to have_requested(verb, "#{base}/#{path}").with(body: { 'name' => 'Acme' })
+ end
+ end
+
+ {
+ update_issue: 'issues', update_account: 'accounts', update_contact: 'contacts',
+ update_team: 'teams', update_user: 'users'
+ }.each do |method, path|
+ it "#{method} patches /#{path}/{id}" do
+ stub_request(:patch, "#{base}/#{path}/x1").to_return(json('data' => { 'id' => 'x1' }))
+
+ expect(client.public_send(method, 'x1', 'name' => 'Acme')).to eq('id' => 'x1')
+ expect(WebMock).to have_requested(:patch, "#{base}/#{path}/x1").with(body: { 'name' => 'Acme' })
+ end
+ end
+
+ { delete_issue: 'issues', delete_account: 'accounts', delete_contact: 'contacts' }.each do |method, path|
+ it "#{method} deletes /#{path}/{id}" do
+ stub_request(:delete, "#{base}/#{path}/x1").to_return(status: 204)
+
+ expect(client.public_send(method, 'x1')).to be(true)
+ expect(WebMock).to have_requested(:delete, "#{base}/#{path}/x1")
+ end
+ end
+ end
+
+ describe 'the record a write answers with' do
+ it 'unwraps the "data" envelope' do
+ stub_request(:post, "#{base}/issues")
+ .to_return(json('data' => { 'id' => 'i1', 'title' => 'Boom' }, 'request_id' => 'req_1'))
+
+ expect(client.create_issue('title' => 'Boom')).to eq('id' => 'i1', 'title' => 'Boom')
+ end
+
+ # A read hands an unwrapped body back untouched; a write must not, or the
+ # collection would serialize an envelope into a record carrying no id.
+ it 'raises when the envelope carries no record' do
+ stub_request(:post, "#{base}/issues").to_return(json('request_id' => 'req_1'))
+
+ expect { client.create_issue('title' => 'Boom') }
+ .to raise_error(ForestAdminDatasourcePylon::APIError, /create\(issues\).*unexpected body shape/m)
+ end
+
+ it 'raises when the record is not an object' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => 'ok'))
+
+ expect { client.update_issue('i1', 'title' => 'Boom') }
+ .to raise_error(ForestAdminDatasourcePylon::APIError, %r{update\(issues/i1\)})
+ end
+
+ # An update's record is discarded by the collection, so an answer carrying
+ # none is the write having landed with nothing to hand back — raising there
+ # would report a failure on a record Pylon already patched.
+ it 'accepts an update answered with no body at all' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(status: 204)
+
+ expect(client.update_issue('i1', 'title' => 'Boom')).to be_nil
+ end
+
+ it 'accepts an update answered without a record' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => nil, 'request_id' => 'req_1'))
+ stub_request(:patch, "#{base}/issues/i2").to_return(json('request_id' => 'req_2'))
+
+ expect(client.update_issue('i1', 'title' => 'Boom')).to be_nil
+ expect(client.update_issue('i2', 'title' => 'Boom')).to be_nil
+ end
+ end
+
+ describe 'a failed write' do
+ it 'raises an APIError carrying the status and the request id' do
+ stub_request(:post, "#{base}/issues")
+ .to_return(json({ 'message' => 'title is required', 'request_id' => 'req_9' }, 422))
+
+ expect { client.create_issue({}) }.to raise_error(ForestAdminDatasourcePylon::APIError) { |error|
+ expect(error.status).to eq(422)
+ expect(error.message).to include('create(issues)', 'HTTP 422', 'title is required', 'req_9')
+ }
+ end
+
+ it 'names the deleted record in the operation' do
+ stub_request(:delete, "#{base}/issues/i1").to_return(json({ 'message' => 'gone' }, 404))
+
+ expect { client.delete_issue('i1') }
+ .to raise_error(ForestAdminDatasourcePylon::APIError, %r{delete\(issues/i1\)})
+ end
+
+ # Ids reach the client from a filter the operator set, so they are escaped
+ # rather than joined to the path as they come.
+ it 'escapes an id that would otherwise alter the request path' do
+ stub_request(:patch, "#{base}/issues/a%2Fb").to_return(json('data' => { 'id' => 'a/b' }))
+
+ client.update_issue('a/b', 'title' => 'Boom')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/a%2Fb")
+ end
+ end
+
+ # A 429 is refused before Pylon processes the request, so replaying it creates
+ # nothing twice; a 502 may well have created the issue, and is not replayed.
+ describe 'retrying a write' do
+ it 'retries a rate-limited create' do
+ stub_request(:post, "#{base}/issues")
+ .to_return(json({ 'message' => 'slow down' }, 429))
+ .then.to_return(json('data' => { 'id' => 'i1' }))
+
+ expect(client.create_issue('title' => 'Boom')).to eq('id' => 'i1')
+ expect(WebMock).to have_requested(:post, "#{base}/issues").twice
+ end
+
+ it 'does not retry a create that failed on a gateway error' do
+ stub_request(:post, "#{base}/issues").to_return(json({ 'message' => 'bad gateway' }, 502))
+
+ expect { client.create_issue('title' => 'Boom') }.to raise_error(ForestAdminDatasourcePylon::APIError)
+ expect(WebMock).to have_requested(:post, "#{base}/issues").once
+ end
+
+ it 'retries a rate-limited delete' do
+ stub_request(:delete, "#{base}/issues/i1")
+ .to_return(json({ 'message' => 'slow down' }, 429))
+ .then.to_return(json({}, 204))
+
+ expect(client.delete_issue('i1')).to be(true)
+ expect(WebMock).to have_requested(:delete, "#{base}/issues/i1").twice
+ end
+
+ # A 502 on the way back from a DELETE Pylon did perform would be replayed
+ # into a 404, which the write path surfaces as a deletion that failed when
+ # it landed -- a report of something that did not happen. So the gateway
+ # error stays what it is, on a delete as on a create.
+ it 'does not retry a delete that failed on a gateway error' do
+ stub_request(:delete, "#{base}/issues/i1").to_return(json({ 'message' => 'bad gateway' }, 502))
+
+ expect { client.delete_issue('i1') }.to raise_error(ForestAdminDatasourcePylon::APIError, /502/)
+ expect(WebMock).to have_requested(:delete, "#{base}/issues/i1").once
+ end
+ end
+end
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 0e9bf0b16..cc6d468bc 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
@@ -102,12 +102,20 @@ def stub_search(payload = { 'data' => [account_payload('acc-1')] })
expect(collection.fields['latest_customer_activity_time'].column_type).to eq('Date')
end
- # Neither endpoint exposes a sort parameter, and writes land in a later story.
- it 'declares every column read-only and non-sortable' do
- expect(columns.values.map(&:is_read_only).uniq).to eq([true])
+ # Neither endpoint exposes a sort parameter.
+ it 'declares every column non-sortable' do
expect(columns.values.map(&:is_sortable).uniq).to eq([false])
end
+ # The Json columns holding objects — `external_ids`, `channels` — stay
+ # read-only although the endpoint takes them: their write shape is not the
+ # one the column shows.
+ it 'declares writable exactly the columns an endpoint takes in the shape they are read' do
+ writable = columns.reject { |_name, column| column.is_read_only }.keys
+
+ expect(writable).to contain_exactly('name', 'type', 'is_disabled', 'domains', 'tags', 'owner_id')
+ end
+
# No Pylon endpoint aggregates, and the pages of a cursor walk are not the
# dataset: a chart grouped by one of these columns would answer a fraction
# as if it were the whole collection.
diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/base_collection_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/base_collection_spec.rb
index 607b2d93f..edeeed5a9 100644
--- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/base_collection_spec.rb
+++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/base_collection_spec.rb
@@ -497,7 +497,7 @@ def returning(*ids)
node = branch('Or', [leaf('id', operators::EQUAL, 'uuid-1'), leaf('state', operators::EQUAL, 'new')])
expect { collection.build_pylon_filter(nil, filter(condition_tree: node)) }
- .to raise_error(UnsupportedOperatorError, /has to be combined with 'and' conditions only/)
+ .to raise_error(UnsupportedOperatorError, /An id inside an `or` names none/)
end
# A collection whose endpoint filters id server-side never short-circuits,
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 93ca7c47b..cde96b091 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
@@ -106,12 +106,23 @@ def stub_search(payload = { 'data' => [contact_payload('con-1')] })
expect(collection.fields['integration_user_ids'].column_type).to eq('Json')
end
- # Neither endpoint exposes a sort parameter, and writes land in a later story.
- it 'declares every column read-only and non-sortable' do
- expect(columns.values.map(&:is_read_only).uniq).to eq([true])
+ # Neither endpoint exposes a sort parameter.
+ it 'declares every column non-sortable' do
expect(columns.values.map(&:is_sortable).uniq).to eq([false])
end
+ # `phone_numbers` and `external_ids` stay read-only although the endpoint
+ # takes them: they hold objects, in a shape the write side does not
+ # document as the one the column shows. `portal_role` stays read-only next
+ # to the `portal_role_id` it is the name of, so one patch never carries two
+ # projections of the same role.
+ it 'declares writable exactly the columns an endpoint takes in the shape they are read' do
+ writable = columns.reject { |_name, column| column.is_read_only }.keys
+
+ expect(writable).to contain_exactly('name', 'account_id', 'email', 'emails', 'avatar_url',
+ 'primary_phone_number', 'portal_role_id')
+ end
+
# No Pylon endpoint aggregates, and the pages of a cursor walk are not the
# dataset: a chart grouped by one of these columns would answer a fraction
# as if it were the whole collection.
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 bc57d1d88..d82490d9e 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
@@ -84,12 +84,21 @@ def columns
expect(collection.fields['resolution_time'].column_type).to eq('Date')
end
- # /issues/search exposes no sort parameter, and writes land in a later story.
- it 'declares every column read-only and non-sortable' do
- expect(columns.values.map(&:is_read_only).uniq).to eq([true])
+ # /issues/search exposes no sort parameter.
+ it 'declares every column non-sortable' do
expect(columns.values.map(&:is_sortable).uniq).to eq([false])
end
+ # Writable is what POST /issues or PATCH /issues/{id} accepts; everything
+ # Pylon computes itself stays read-only.
+ it 'declares writable exactly the columns an endpoint takes' do
+ writable = columns.reject { |_name, column| column.is_read_only }.keys
+
+ expect(writable).to contain_exactly('title', 'body_html', 'state', 'type', 'tags',
+ 'customer_portal_visible', 'author_unverified',
+ 'account_id', 'requester_id', 'assignee_id', 'team_id')
+ end
+
# No Pylon endpoint aggregates, and the pages of a cursor walk are not the
# dataset: a chart grouped by one of these columns would answer a fraction
# as if it were the whole collection.
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 f9c0bf080..b8df431e8 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
@@ -74,14 +74,21 @@ def columns
expect(collection.fields['id'].is_primary_key).to be(true)
end
- # Writes land in a later story; the order is honoured in memory over the
- # complete dataset, so both scalar columns can be sorted on.
- it 'declares every column read-only and both scalar columns sortable' do
- expect(columns.values.map(&:is_read_only).uniq).to eq([true])
+ # The order is honoured in memory over the complete dataset, so both
+ # scalar columns can be sorted on.
+ it 'declares both scalar columns sortable' do
expect(columns.except('user_ids').values.map(&:is_sortable).uniq).to eq([true])
expect(collection.fields['user_ids'].is_sortable).to be(false)
end
+ # POST /teams and PATCH /teams/{id} take the name and the members, and
+ # Pylon names the id itself.
+ it 'declares writable exactly the columns an endpoint takes' do
+ writable = columns.reject { |_name, column| column.is_read_only }.keys
+
+ expect(writable).to contain_exactly('name', 'user_ids')
+ end
+
# GET /teams carries neither a search nor a filter parameter, and Pylon
# exposes no count.
it 'leaves search and count disabled' do
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 c17d0dcfe..500a9cd16 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
@@ -83,14 +83,21 @@ def columns
.to eq(['String'])
end
- # Writes land in a later story; the order is honoured in memory over the
- # complete dataset, so every scalar column can be sorted on.
- it 'declares every column read-only and every scalar column sortable' do
- expect(columns.values.map(&:is_read_only).uniq).to eq([true])
+ # The order is honoured in memory over the complete dataset, so every
+ # scalar column can be sorted on.
+ it 'declares every scalar column sortable' do
expect(columns.except('emails').values.map(&:is_sortable).uniq).to eq([true])
expect(collection.fields['emails'].is_sortable).to be(false)
end
+ # PATCH /users/{id} takes these four and nothing else: an address is
+ # proven by the agent signing in, and the deactivation happens in Pylon.
+ it 'declares writable exactly the columns the update endpoint takes' do
+ writable = columns.reject { |_name, column| column.is_read_only }.keys
+
+ expect(writable).to contain_exactly('name', 'avatar_url', 'status', 'role_id')
+ end
+
# GET /users carries no search parameter, and Pylon exposes no count.
it 'leaves search and count disabled' do
expect(collection.is_searchable?).to be(false)
diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/writes_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/writes_spec.rb
new file mode 100644
index 000000000..3d0252522
--- /dev/null
+++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/collections/writes_spec.rb
@@ -0,0 +1,674 @@
+module ForestAdminDatasourcePylon
+ RSpec.describe Collections::Writes do
+ def filter(condition_tree: nil, search: nil)
+ ForestAdminDatasourceToolkit::Components::Query::Filter.new(condition_tree: condition_tree, search: search)
+ end
+
+ def leaf(field, operator, value)
+ ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf
+ .new(field, operator, value)
+ end
+
+ def branch(aggregator, conditions)
+ ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeBranch
+ .new(aggregator, conditions)
+ end
+
+ def id_filter(operator, value)
+ filter(condition_tree: leaf('id', operator, value))
+ end
+
+ def json(payload, status = 200)
+ { status: status, body: payload.to_json, headers: { 'Content-Type' => 'application/json' } }
+ end
+
+ def custom_field(type, slug, extra = {})
+ { 'id' => "cf_#{slug}", 'slug' => slug, 'label' => slug, 'type' => type,
+ 'object_type' => 'issue', 'is_read_only' => false }.merge(extra)
+ end
+
+ def options(*slugs)
+ { 'select_metadata' => { 'options' => slugs.map { |slug| { 'label' => slug.upcase, 'slug' => slug } } } }
+ end
+
+ def issue_payload(id, overrides = {})
+ { 'id' => id, 'number' => 12, 'title' => 'Boom', 'body_html' => 'boom
', 'state' => 'new',
+ 'type' => 'ticket', 'source' => 'manual', 'account' => { 'id' => 'acc-1' }, 'tags' => %w[urgent],
+ 'custom_fields' => {}, 'created_at' => '2026-08-07T13:06:22Z' }.merge(overrides)
+ end
+
+ # A text field, a select read and written by the slug of its option, a
+ # multiselect Pylon takes through `values`, and one it syncs from an app and
+ # refuses to be written.
+ let(:issue_custom_fields) do
+ [custom_field('text', 'severity'),
+ custom_field('select', 'priority', options('p1', 'p2')),
+ custom_field('multiselect', 'regions', options('us', 'emea')),
+ custom_field('text', 'synced_id', 'is_read_only' => true)]
+ end
+
+ let(:datasource) { Datasource.new(api_key: 'k') }
+ let(:base) { datasource.configuration.url }
+ let(:issues) { datasource.get_collection('PylonIssue') }
+ let(:accounts) { datasource.get_collection('PylonAccount') }
+ let(:contacts) { datasource.get_collection('PylonContact') }
+ let(:teams) { datasource.get_collection('PylonTeam') }
+ let(:users) { datasource.get_collection('PylonUser') }
+ let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators }
+
+ before { stub_custom_fields(issue: issue_custom_fields) }
+
+ describe '#create' do
+ it 'posts the writable columns and answers with the serialized record' do
+ stub_request(:post, "#{base}/issues").to_return(json('data' => issue_payload('i1')))
+
+ record = issues.create(nil, 'title' => 'Boom', 'body_html' => 'boom
', 'tags' => %w[urgent],
+ 'account_id' => 'acc-1')
+
+ expect(record).to include('id' => 'i1', 'title' => 'Boom', 'account_id' => 'acc-1')
+ expect(WebMock).to have_requested(:post, "#{base}/issues")
+ .with(body: { 'title' => 'Boom', 'body_html' => 'boom
',
+ 'tags' => %w[urgent], 'account_id' => 'acc-1' })
+ end
+
+ # The schema is the single source of truth for what may be written: a
+ # read-only column reaching the payload is the agent's doing, not a request
+ # the operator made, so it is dropped rather than refused.
+ it 'drops the read-only columns and the keys the schema does not know' do
+ stub_request(:post, "#{base}/issues").to_return(json('data' => issue_payload('i1')))
+
+ issues.create(nil, 'title' => 'Boom', 'id' => 'i9', 'number' => 3, 'link' => 'http://x',
+ 'created_at' => '2026-01-01', 'source' => 'manual', 'messages' => [],
+ 'number_of_touches' => 4, 'not_a_column' => 'x')
+
+ expect(WebMock).to have_requested(:post, "#{base}/issues").with(body: { 'title' => 'Boom' })
+ end
+
+ # Pylon fills in what a create leaves out, so a form field the operator
+ # never touched travels as nothing at all rather than as an explicit null.
+ it 'drops the columns left empty' do
+ stub_request(:post, "#{base}/issues").to_return(json('data' => issue_payload('i1')))
+
+ issues.create(nil, 'title' => 'Boom', 'team_id' => nil, 'tags' => nil)
+
+ expect(WebMock).to have_requested(:post, "#{base}/issues").with(body: { 'title' => 'Boom' })
+ end
+ end
+
+ describe '#create with custom fields' do
+ it 'writes them as a list, through `value` or `values`, and leaves the synced one out' do
+ stub_request(:post, "#{base}/issues").to_return(json('data' => issue_payload('i1')))
+
+ issues.create(nil, 'title' => 'Boom', 'severity' => 'high', 'priority' => 'p2',
+ 'regions' => %w[us emea], 'synced_id' => 'zzz')
+
+ expect(WebMock).to have_requested(:post, "#{base}/issues").with(
+ body: { 'title' => 'Boom',
+ 'custom_fields' => [{ 'slug' => 'severity', 'value' => 'high' },
+ { 'slug' => 'priority', 'value' => 'p2' },
+ { 'slug' => 'regions', 'values' => %w[us emea] }] }
+ )
+ end
+
+ it 'sends no custom_fields key when none was set' do
+ stub_request(:post, "#{base}/issues").to_return(json('data' => issue_payload('i1')))
+
+ issues.create(nil, 'title' => 'Boom')
+
+ expect(WebMock).to(have_requested(:post, "#{base}/issues").with { |req| !req.body.include?('custom_fields') })
+ end
+ end
+
+ # Pylon takes `state` and `type` on an update only, and Forest has one
+ # read-only flag per column to say so with.
+ describe '#create naming a field Pylon only takes on an update' do
+ it 'refuses the create, naming the field' do
+ expect { issues.create(nil, 'title' => 'Boom', 'state' => 'closed') }
+ .to raise_error(UnsupportedWriteError, /'state' cannot be set here on a PylonIssue/)
+ end
+
+ it 'asks for nothing when the field carries no value' do
+ stub_request(:post, "#{base}/issues").to_return(json('data' => issue_payload('i1')))
+
+ issues.create(nil, 'title' => 'Boom', 'state' => nil, 'type' => '')
+
+ expect(WebMock).to have_requested(:post, "#{base}/issues").with(body: { 'title' => 'Boom' })
+ end
+ end
+
+ describe '#update' do
+ it 'patches the record the filter names, without reading it back first' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ issues.update(nil, id_filter(operators::EQUAL, 'i1'), 'title' => 'Louder')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1").with(body: { 'title' => 'Louder' })
+ expect(WebMock).not_to have_requested(:get, "#{base}/issues/i1")
+ end
+
+ it 'patches every record an `in` filter names' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+ stub_request(:patch, "#{base}/issues/i2").to_return(json('data' => issue_payload('i2')))
+
+ issues.update(nil, id_filter(operators::IN, %w[i1 i2]), 'state' => 'closed')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1").with(body: { 'state' => 'closed' })
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i2").with(body: { 'state' => 'closed' })
+ end
+
+ it 'sends nothing when every key of the patch is read-only' do
+ issues.update(nil, id_filter(operators::EQUAL, 'i1'), 'number' => 9, 'link' => 'http://x')
+
+ expect(WebMock).not_to have_requested(:patch, "#{base}/issues/i1")
+ end
+
+ # The cap bounds a write, and a patch naming nothing writable is not one:
+ # it is settled before the ids are, so the selection is never resolved and
+ # never refused for its width.
+ it 'sends nothing, and refuses nothing, when the patch is read-only over a wide selection' do
+ ids = Array.new(21) { |index| "i#{index}" }
+
+ expect { issues.update(nil, id_filter(operators::IN, ids), 'number' => 9) }.not_to raise_error
+ expect(WebMock).not_to have_requested(:patch, %r{/issues/})
+ end
+
+ # The scope the operator's role carries rides along as an `and`, so the ids
+ # are resolved through the collection's own read and a record the scope
+ # excludes is never written to.
+ it 'resolves the ids through a read when the filter carries more than an id' do
+ stub_request(:get, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1', 'state' => 'closed')))
+
+ issues.update(nil, filter(condition_tree: branch('And', [leaf('id', operators::EQUAL, 'i1'),
+ leaf('state', operators::EQUAL, 'new')])),
+ 'title' => 'Louder')
+
+ expect(WebMock).to have_requested(:get, "#{base}/issues/i1")
+ expect(WebMock).not_to have_requested(:patch, "#{base}/issues/i1")
+ end
+ end
+
+ # The record a write answers with is discarded here, so a patch Pylon
+ # answers with no body at all wrote the record just the same — and the rest
+ # of the selection is written rather than aborted on it.
+ describe '#update answered with no record' do
+ it 'writes every record of the selection' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(status: 204)
+ stub_request(:patch, "#{base}/issues/i2").to_return(json('data' => nil))
+
+ issues.update(nil, id_filter(operators::IN, %w[i1 i2]), 'state' => 'closed')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1")
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i2")
+ end
+ end
+
+ # One record is one request, so a failure on the k-th leaves the k-1 before
+ # it written and written for good: the error names them, where the API error
+ # alone would read as "the write failed, nothing happened" and a retry of the
+ # whole selection would write them twice.
+ describe 'a write failing partway through the selection' do
+ it 'names the records already written' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+ stub_request(:patch, "#{base}/issues/i2").to_return(json({ 'message' => 'state is invalid' }, 422))
+
+ expect { issues.update(nil, id_filter(operators::IN, %w[i1 i2]), 'state' => 'closed') }
+ .to raise_error(PartialWriteError, /1 of 2 PylonIssue records were updated and then 'i2' failed/)
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1")
+ end
+
+ it 'names the records already deleted' do
+ stub_request(:delete, "#{base}/issues/i1").to_return(status: 204)
+ stub_request(:delete, "#{base}/issues/i2").to_return(json({ 'message' => 'gone' }, 404))
+
+ expect { issues.delete(nil, id_filter(operators::IN, %w[i1 i2])) }
+ .to raise_error(PartialWriteError, /records already deleted are i1/)
+ end
+
+ # Nothing was written, so the failure is the whole of what happened and
+ # travels as the reason Pylon gave — as a ValidationError, the agent
+ # answering an APIError with 'Unexpected error' whatever it carries.
+ it 'surfaces the refusal itself when the first record failed' do
+ stub_request(:delete, "#{base}/issues/i1").to_return(json({ 'message' => 'gone' }, 404))
+
+ expect { issues.delete(nil, id_filter(operators::IN, %w[i1 i2])) }
+ .to raise_error(WriteRejectedError, %r{delete\(issues/i1\).*gone}m)
+ end
+ end
+
+ # Pylon's own refusal is the likeliest way a write fails, and `APIError`
+ # descends from the package's Error, which the agent's translator answers
+ # with 'Unexpected error': a 4xx is re-raised as a ValidationError so the
+ # reason reaches the operator, and nothing else is.
+ describe 'a write Pylon refused' do
+ it 'surfaces the reason a rejected create was given' do
+ stub_request(:post, "#{base}/issues")
+ .to_return(json({ 'message' => 'title is required' }, 422))
+
+ expect { issues.create(nil, 'title' => '', 'body_html' => 'b
') }
+ .to raise_error(WriteRejectedError, /title is required/)
+ end
+
+ it 'surfaces the reason a rejected update was given' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json({ 'message' => 'unknown state' }, 422))
+
+ expect { issues.update(nil, id_filter(operators::EQUAL, 'i1'), 'state' => 'nope') }
+ .to raise_error(WriteRejectedError, /unknown state/)
+ end
+
+ # Not the operator's to fix, and not theirs to be told to fix: a gateway
+ # error stays the APIError it was, carrying its status for the agent to
+ # answer with.
+ it 'leaves a Pylon-side failure as it was' do
+ stub_request(:post, "#{base}/issues").to_return(json({ 'message' => 'boom' }, 500))
+
+ expect { issues.create(nil, 'title' => 'Boom', 'body_html' => 'b
') }
+ .to raise_error(APIError) { |error| expect(error.status).to eq(500) }
+ end
+ end
+
+ # `id in` names the records to act on, and the same one named twice is one
+ # record: writing it twice would answer 404 on the second delete and report
+ # a partial failure of a delete that fully succeeded.
+ describe 'an id named twice in the same selection' do
+ it 'writes the record once' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ issues.update(nil, id_filter(operators::IN, %w[i1 i1]), 'state' => 'closed')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1").once
+ end
+
+ # The caps bound records, not mentions: a selection naming the same id
+ # over and over reaches one record and is not refused for reaching many.
+ it 'counts it once against the cap' do
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ issues.update(nil, id_filter(operators::IN, ['i1'] * 25), 'state' => 'closed')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1").once
+ end
+ end
+
+ describe '#update naming a field Pylon only takes on a create' do
+ it 'refuses the update when the operator changed it' do
+ stub_request(:get, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ expect { issues.update(nil, id_filter(operators::EQUAL, 'i1'), 'body_html' => 'louder
') }
+ .to raise_error(UnsupportedWriteError, /'body_html' cannot be set here on a PylonIssue/)
+ end
+
+ # A form resending an untouched field asks for nothing, so the rest of the
+ # edit goes through rather than erroring on a value nobody changed.
+ it 'drops it, and writes the rest, when it holds the value already stored' do
+ stub_request(:get, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ issues.update(nil, id_filter(operators::EQUAL, 'i1'),
+ 'body_html' => 'boom
', 'title' => 'Louder')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1").with(body: { 'title' => 'Louder' })
+ end
+
+ # An unchecked box over a record holding nothing is not an edit: Pylon
+ # returns a null where the form sends `false`, and refusing that pair
+ # would fail every edit whose form carries one.
+ it 'drops a boolean left false over a record holding nothing' do
+ stub_request(:get, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ issues.update(nil, id_filter(operators::EQUAL, 'i1'), 'author_unverified' => false, 'title' => 'Louder')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1").with(body: { 'title' => 'Louder' })
+ end
+
+ # Over a record holding `true` the same `false` is the operator unchecking
+ # the box: Pylon cannot write it, and dropping it would report an edit it
+ # never performed.
+ it 'refuses a boolean the operator unchecked' do
+ stub_request(:get, "#{base}/issues/i1")
+ .to_return(json('data' => issue_payload('i1', 'author_unverified' => true)))
+
+ expect { issues.update(nil, id_filter(operators::EQUAL, 'i1'), 'author_unverified' => false) }
+ .to raise_error(UnsupportedWriteError, /'author_unverified' cannot be set here on a PylonIssue/)
+ end
+
+ # Same story for a value cleared rather than unchecked: an empty body over
+ # a stored one is an edit, where an empty body over an empty one is not.
+ it 'refuses a string the operator cleared' do
+ stub_request(:get, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ expect { issues.update(nil, id_filter(operators::EQUAL, 'i1'), 'body_html' => '') }
+ .to raise_error(UnsupportedWriteError, /'body_html' cannot be set here on a PylonIssue/)
+ end
+
+ # The markup an editor hands back may be the markup it was given,
+ # re-indented. Refusing that would name a field the operator never touched.
+ it 'drops a string the editor only re-indented' do
+ stub_request(:get, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+ stub_request(:patch, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+
+ issues.update(nil, id_filter(operators::EQUAL, 'i1'),
+ 'body_html' => " boom
\n", 'title' => 'Louder')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/issues/i1").with(body: { 'title' => 'Louder' })
+ end
+
+ # Nothing here may claim a value is unchanged on a record it never read,
+ # and one record short of the selection is enough: dropping the field would
+ # write the rest of the patch to a record whose stored value is unknown and
+ # report the whole edit as performed.
+ it 'refuses it when one of the named records could not be read' do
+ stub_request(:get, "#{base}/issues/i1").to_return(json('data' => issue_payload('i1')))
+ stub_request(:get, "#{base}/issues/i2").to_return(json({ 'message' => 'gone' }, 404))
+
+ expect do
+ issues.update(nil, id_filter(operators::IN, %w[i1 i2]),
+ 'body_html' => 'boom
', 'title' => 'Louder')
+ end.to raise_error(UnsupportedWriteError, /'body_html' cannot be set here on a PylonIssue/)
+ expect(WebMock).not_to have_requested(:patch, %r{/issues/})
+ end
+
+ # Both offending fields at once: refusing them one at a time would have the
+ # operator undo one, retry, and learn about the next.
+ it 'names every field of the wrong direction in one message' do
+ stub_request(:get, "#{base}/issues/i1")
+ .to_return(json('data' => issue_payload('i1', 'author_unverified' => true)))
+
+ expect do
+ issues.update(nil, id_filter(operators::EQUAL, 'i1'),
+ 'body_html' => 'louder
', 'author_unverified' => false)
+ end.to raise_error(UnsupportedWriteError, /'body_html', 'author_unverified' cannot be set here/)
+ end
+
+ # The filter was already resolved into ids, so reading it again would spend
+ # the same requests twice and, carrying no page of its own, walk every
+ # record it matches instead of the one about to be written.
+ it 'reads the stored value by id rather than running the filter a second time' do
+ contact = { 'id' => 'c1', 'name' => 'Ada', 'email' => 'ada@acme.test' }
+ stub_request(:post, "#{base}/contacts/search").to_return(json('data' => [contact]))
+ stub_request(:get, "#{base}/contacts/c1").to_return(json('data' => contact))
+ stub_request(:patch, "#{base}/contacts/c1").to_return(json('data' => contact))
+
+ contacts.update(nil, filter(condition_tree: leaf('name', operators::EQUAL, 'Ada')),
+ 'email' => 'ada@acme.test', 'avatar_url' => 'http://x')
+
+ expect(WebMock).to have_requested(:post, "#{base}/contacts/search").once
+ expect(WebMock).to have_requested(:get, "#{base}/contacts/c1")
+ expect(WebMock).to have_requested(:patch, "#{base}/contacts/c1").with(body: { 'avatar_url' => 'http://x' })
+ end
+ end
+
+ describe '#delete' do
+ it 'deletes every record the filter names' do
+ stub_request(:delete, "#{base}/issues/i1").to_return(status: 204)
+ stub_request(:delete, "#{base}/issues/i2").to_return(status: 204)
+
+ issues.delete(nil, id_filter(operators::IN, %w[i1 i2]))
+
+ expect(WebMock).to have_requested(:delete, "#{base}/issues/i1")
+ expect(WebMock).to have_requested(:delete, "#{base}/issues/i2")
+ end
+
+ it 'deletes nothing when the filter matches no record' do
+ stub_request(:post, "#{base}/accounts/search").to_return(json('data' => []))
+
+ accounts.delete(nil, filter(condition_tree: leaf('name', operators::EQUAL, 'Nope')))
+
+ expect(WebMock).not_to have_requested(:delete, %r{/accounts/})
+ end
+ end
+
+ # One request per record against a budget of ten to twenty per minute: past
+ # the cap the write is refused rather than applied to the first records and
+ # reported as done for the whole selection.
+ describe 'a write reaching more records than one pass covers' do
+ it 'refuses it before spending a single request' do
+ ids = Array.new(21) { |index| "i#{index}" }
+
+ expect { issues.delete(nil, id_filter(operators::IN, ids)) }
+ .to raise_error(UnsupportedWriteError, /applies to 21 PylonIssue records/)
+ expect(WebMock).not_to have_requested(:delete, %r{/issues/})
+ end
+
+ # The resolution asks for one record past the cap, so the overflow is seen
+ # rather than counted: reporting the window as a count would name 21 to an
+ # operator whose selection holds thousands.
+ it 'refuses it when the overflow only shows once the filter is resolved, without naming a count' do
+ stub_request(:post, "#{base}/accounts/search")
+ .to_return(json('data' => Array.new(21) { |index| { 'id' => "a#{index}", 'name' => 'Acme' } }))
+
+ expect { accounts.delete(nil, filter(condition_tree: leaf('name', operators::EQUAL, 'Acme'))) }
+ .to raise_error(UnsupportedWriteError, /applies to more than the 20 PylonAccount records one pass covers/)
+ expect(WebMock).not_to have_requested(:delete, %r{/accounts/})
+ end
+
+ # The ids a filter names are not the records the write applies to when the
+ # filter narrows them further: the collections filtering `id` server-side
+ # learn the real count in one request, and write it.
+ it 'writes the records a narrowed selection really matches' do
+ stub_request(:post, "#{base}/accounts/search")
+ .to_return(json('data' => [{ 'id' => 'a1', 'name' => 'Acme' }]))
+ stub_request(:patch, "#{base}/accounts/a1").to_return(json('data' => { 'id' => 'a1' }))
+
+ ids = Array.new(25) { |index| "a#{index}" }
+ accounts.update(nil, filter(condition_tree: branch('And', [leaf('id', operators::IN, ids),
+ leaf('name', operators::EQUAL, 'Acme')])),
+ 'name' => 'Acme Inc')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/accounts/a1").with(body: { 'name' => 'Acme Inc' })
+ end
+
+ # An issue is read one request per named id, so past the fan-out the
+ # resolution would stop short and the write would cover part of the
+ # selection. Refused — as the ids it names, never as records it was found
+ # to apply to.
+ it 'refuses more named ids than the collection can resolve, without claiming they all match' do
+ ids = Array.new(25) { |index| "i#{index}" }
+
+ expect do
+ issues.update(nil, filter(condition_tree: branch('And', [leaf('id', operators::IN, ids),
+ leaf('state', operators::EQUAL, 'new')])),
+ 'title' => 'Louder')
+ end.to raise_error(UnsupportedWriteError, /names 25 PylonIssue records and filters them further/)
+ expect(WebMock).not_to have_requested(:get, %r{/issues/})
+ end
+
+ # The cap is a budget of requests, not of writes: where a record is read
+ # through its own endpoint, every read the path owes it comes out of the
+ # same twenty, so the records one pass reaches halve for each of them.
+ it 'halves the reach when the patch has every record read before it is written' do
+ ids = Array.new(11) { |index| "i#{index}" }
+
+ expect { issues.update(nil, id_filter(operators::IN, ids), 'body_html' => 'x
', 'title' => 'Louder') }
+ .to raise_error(UnsupportedWriteError, /applies to 11 PylonIssue records, more than the 10 one pass/)
+ expect(WebMock).not_to have_requested(:get, %r{/issues/})
+ expect(WebMock).not_to have_requested(:patch, %r{/issues/})
+ end
+
+ # The same eleven records, with nothing to read before writing to them.
+ it 'keeps the full reach when the patch owes the records no read' do
+ stub_request(:patch, %r{/issues/i\d+}).to_return(json('data' => issue_payload('i1')))
+
+ issues.update(nil, id_filter(operators::IN, Array.new(11) { |index| "i#{index}" }), 'title' => 'Louder')
+
+ expect(WebMock).to have_requested(:patch, %r{/issues/i\d+}).times(11)
+ end
+
+ # A selection naming ids is resolved by reading each of them, so it is
+ # bounded by the same halved reach — and refused before the first of those
+ # reads rather than after twenty of them.
+ it 'refuses a resolution costing a request per record past the halved reach' do
+ ids = Array.new(12) { |index| "i#{index}" }
+
+ expect do
+ issues.update(nil, filter(condition_tree: branch('And', [leaf('id', operators::IN, ids),
+ leaf('state', operators::EQUAL, 'new')])),
+ 'title' => 'Louder')
+ end.to raise_error(UnsupportedWriteError, /names 12 PylonIssue records .* more than the 10 one pass covers/m)
+ expect(WebMock).not_to have_requested(:get, %r{/issues/})
+ end
+
+ # A selection naming no id is resolved by one page of the search endpoint,
+ # whose cost does not grow with the count: nothing to charge per record, so
+ # the full reach stands.
+ it 'keeps the full reach when the resolution costs one request whatever the count' do
+ stub_request(:post, "#{base}/issues/search")
+ .to_return(json('data' => Array.new(21) { |index| issue_payload("i#{index}") }))
+
+ expect { issues.delete(nil, filter(condition_tree: leaf('state', operators::EQUAL, 'new'))) }
+ .to raise_error(UnsupportedWriteError, /more than the 20 PylonIssue records one pass covers/)
+ expect(WebMock).to have_requested(:post, "#{base}/issues/search").once
+ end
+
+ # And nothing is charged per record where a record costs no request of its
+ # own: the search endpoint filters `id`, so reading the stored value of a
+ # whole selection is one request, whatever the reach.
+ it 'keeps the full reach on a collection whose read does not fan out' do
+ ids = Array.new(11) { |index| "c#{index}" }
+ stored = ids.map { |id| { 'id' => id, 'name' => 'Ada', 'email' => 'ada@acme.test' } }
+ stub_request(:post, "#{base}/contacts/search").to_return(json('data' => stored))
+ stub_request(:patch, %r{/contacts/c\d+}).to_return(json('data' => stored.first))
+
+ contacts.update(nil, id_filter(operators::IN, ids), 'email' => 'ada@acme.test', 'name' => 'Ada Lovelace')
+
+ expect(WebMock).to have_requested(:post, "#{base}/contacts/search").once
+ expect(WebMock).to have_requested(:patch, %r{/contacts/c\d+}).times(11)
+ end
+
+ # "Select all except these" reaches PylonIssue as `id not_in`, which its
+ # endpoint cannot filter: the read refuses it, and so does the delete. The
+ # message names that selection rather than the `and`/`or` of a filter the
+ # operator never wrote.
+ it 'refuses an excluding selection on the collection that cannot filter an id' do
+ expect { issues.delete(nil, id_filter(operators::NOT_IN, %w[i1])) }
+ .to raise_error(UnsupportedOperatorError,
+ /Select the records to act on rather than the ones to leave out/)
+ end
+ end
+
+ describe 'a verb Pylon exposes no endpoint for' do
+ it 'refuses to create a user' do
+ expect { users.create(nil, 'name' => 'Ada') }
+ .to raise_error(UnsupportedWriteError, /A PylonUser record cannot be created/)
+ end
+
+ it 'refuses to delete a user' do
+ expect { users.delete(nil, id_filter(operators::EQUAL, 'u1')) }
+ .to raise_error(UnsupportedWriteError, /A PylonUser record cannot be deleted/)
+ end
+
+ it 'refuses to delete a team' do
+ expect { teams.delete(nil, id_filter(operators::EQUAL, 't1')) }
+ .to raise_error(UnsupportedWriteError, /A PylonTeam record cannot be deleted/)
+ end
+
+ # The refusal holds whatever the selection reaches, so it comes before the
+ # ids are resolved: answering with the cap would send the operator to
+ # narrow a selection that was never the problem, and answering a selection
+ # matching nothing with a silent success would report a delete on a
+ # collection that cannot perform one.
+ it 'refuses a selection wider than the cap without naming the cap' do
+ expect { teams.delete(nil, id_filter(operators::IN, (1..25).map { |i| "t#{i}" })) }
+ .to raise_error(UnsupportedWriteError, /A PylonTeam record cannot be deleted/)
+ end
+
+ it 'refuses a selection matching nothing rather than answering it' do
+ expect { teams.delete(nil, filter(condition_tree: leaf('name', operators::EQUAL, 'nope'))) }
+ .to raise_error(UnsupportedWriteError, /A PylonTeam record cannot be deleted/)
+ end
+
+ it 'refuses before spending a request to resolve the selection' do
+ expect { teams.delete(nil, filter(condition_tree: leaf('name', operators::EQUAL, 'x'))) }
+ .to raise_error(UnsupportedWriteError)
+ expect(WebMock).not_to have_requested(:get, "#{base}/teams")
+ end
+ end
+
+ describe 'the collections read through their own endpoints' do
+ it 'writes an account type under the name Pylon takes it as' do
+ stub_request(:post, "#{base}/accounts").to_return(json('data' => { 'id' => 'a1', 'name' => 'Acme' }))
+
+ accounts.create(nil, 'name' => 'Acme', 'type' => 'customer', 'domains' => %w[acme.test])
+
+ expect(WebMock).to have_requested(:post, "#{base}/accounts")
+ .with(body: { 'name' => 'Acme', 'account_type' => 'customer', 'domains' => %w[acme.test] })
+ end
+
+ it 'refuses to disable an account that does not exist yet' do
+ expect { accounts.create(nil, 'name' => 'Acme', 'is_disabled' => true) }
+ .to raise_error(UnsupportedWriteError, /'is_disabled' cannot be set here on a PylonAccount/)
+ end
+
+ # An account is created enabled, which is what the form asks for when the
+ # box is left unchecked: the create it produces is the one requested.
+ it 'creates an account whose update-only boolean is left false' do
+ stub_request(:post, "#{base}/accounts").to_return(json('data' => { 'id' => 'a1', 'name' => 'Acme' }))
+
+ accounts.create(nil, 'name' => 'Acme', 'is_disabled' => false)
+
+ expect(WebMock).to have_requested(:post, "#{base}/accounts").with(body: { 'name' => 'Acme' })
+ end
+
+ # `POST /contacts` takes the primary address and `PATCH /contacts/{id}` the
+ # list, so one payload never carries both projections of the addresses.
+ it 'creates a contact with its primary address' do
+ stub_request(:post, "#{base}/contacts").to_return(json('data' => { 'id' => 'c1', 'name' => 'Ada' }))
+
+ contacts.create(nil, 'name' => 'Ada', 'email' => 'ada@acme.test', 'emails' => [])
+
+ expect(WebMock).to have_requested(:post, "#{base}/contacts")
+ .with(body: { 'name' => 'Ada', 'email' => 'ada@acme.test' })
+ end
+
+ it 'refuses to change the primary address of an existing contact' do
+ stub_request(:get, "#{base}/contacts/c1")
+ .to_return(json('data' => { 'id' => 'c1', 'name' => 'Ada', 'email' => 'ada@acme.test' }))
+
+ expect { contacts.update(nil, id_filter(operators::EQUAL, 'c1'), 'email' => 'new@acme.test') }
+ .to raise_error(UnsupportedWriteError, /'email' cannot be set here on a PylonContact/)
+ end
+
+ it 'patches a contact' do
+ stub_request(:patch, "#{base}/contacts/c1").to_return(json('data' => { 'id' => 'c1', 'name' => 'Ada' }))
+
+ contacts.update(nil, id_filter(operators::EQUAL, 'c1'), 'name' => 'Ada', 'account_id' => 'a1')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/contacts/c1")
+ .with(body: { 'name' => 'Ada', 'account_id' => 'a1' })
+ end
+
+ # The only create whose record is serialized by a collection read in whole:
+ # `POST /teams` answers with the members nested where the column carries
+ # their ids, so the flattening the read side does has to run here too.
+ it 'creates a team and flattens the members of the record it answers with' do
+ stub_request(:post, "#{base}/teams")
+ .to_return(json('data' => { 'id' => 't1', 'name' => 'Support',
+ 'users' => [{ 'id' => 'u1', 'email' => 'ada@acme.test' },
+ { 'id' => 'u2' }] }))
+
+ record = teams.create(nil, 'name' => 'Support', 'user_ids' => %w[u1 u2])
+
+ expect(record).to eq('id' => 't1', 'name' => 'Support', 'user_ids' => %w[u1 u2])
+ expect(WebMock).to have_requested(:post, "#{base}/teams")
+ .with(body: { 'name' => 'Support', 'user_ids' => %w[u1 u2] })
+ end
+
+ it 'replaces the members of a team' do
+ stub_request(:patch, "#{base}/teams/t1").to_return(json('data' => { 'id' => 't1', 'name' => 'Support' }))
+
+ teams.update(nil, id_filter(operators::EQUAL, 't1'), 'name' => 'Support', 'user_ids' => %w[u1 u2])
+
+ expect(WebMock).to have_requested(:patch, "#{base}/teams/t1")
+ .with(body: { 'name' => 'Support', 'user_ids' => %w[u1 u2] })
+ end
+
+ it 'patches the status of a user' do
+ stub_request(:patch, "#{base}/users/u1").to_return(json('data' => { 'id' => 'u1', 'name' => 'Ada' }))
+
+ users.update(nil, id_filter(operators::EQUAL, 'u1'), 'status' => 'away', 'email' => 'ada@acme.test')
+
+ expect(WebMock).to have_requested(:patch, "#{base}/users/u1").with(body: { 'status' => 'away' })
+ end
+ end
+ end
+end
diff --git a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/retry_policy_spec.rb b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/retry_policy_spec.rb
index f88450901..d8d2cfe21 100644
--- a/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/retry_policy_spec.rb
+++ b/packages/forest_admin_datasource_pylon/spec/forest_admin_datasource_pylon/retry_policy_spec.rb
@@ -36,10 +36,17 @@
expect(options[:exceptions]).to include(Faraday::ConnectionFailed, Faraday::RetriableResponse)
end
- it 'limits blanket retries to idempotent verbs' do
- expect(options[:methods]).to eq(%i[delete get head options put])
+ it 'limits blanket retries to the verbs that read' do
+ expect(options[:methods]).to eq(%i[get head options])
expect(options[:methods]).not_to include(:post, :patch)
end
+
+ # A 502 on the way back from a DELETE Pylon did perform would be replayed
+ # into a 404, which the write path surfaces as a deletion that failed when
+ # it landed. Only its 429 is retried, through RETRY_IF.
+ it 'never replays a delete on anything but a 429' do
+ expect(options[:methods]).not_to include(:delete)
+ end
end
describe 'RETRY_IF' do
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
index 975864321..ea2c78af2 100644
--- 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
@@ -162,12 +162,12 @@ def operators_of(type, **extra)
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.
+ # No endpoint sorts, ever; read-only is Pylon's own call, which it makes for
+ # the fields an app or an integration syncs.
describe 'the schema every custom field gets' do
- it 'is read-only and unsortable, whatever Pylon declares' do
+ it 'is unsortable, and read-only when Pylon says so' do
allow(client).to receive(:fetch_custom_fields).with('issue')
- .and_return([definition('text', 'is_read_only' => false)])
+ .and_return([definition('text', 'is_read_only' => true)])
schema = introspector.issue_custom_fields.first[:schema]
@@ -175,6 +175,27 @@ def operators_of(type, **extra)
expect(schema.is_sortable).to be(false)
end
+ it 'is writable when Pylon declares the field editable' do
+ allow(client).to receive(:fetch_custom_fields).with('issue')
+ .and_return([definition('text', 'is_read_only' => false)])
+
+ expect(introspector.issue_custom_fields.first[:schema].is_read_only).to be(false)
+ end
+
+ # A definition carrying no flag is left read-only: this datasource
+ # advertises nothing an endpoint would refuse, and the fields Pylon syncs
+ # from an app are exactly the ones the flag tells apart, so reading its
+ # absence as "editable" would offer an editor whose every save is rejected.
+ it 'is read-only, and says so, when Pylon declares nothing' do
+ allow(ForestAdminDatasourcePylon.logger).to receive(:warn)
+ allow(client).to receive(:fetch_custom_fields).with('issue')
+ .and_return([definition('text', 'is_read_only' => nil)])
+
+ expect(introspector.issue_custom_fields.first[:schema].is_read_only).to be(true)
+ expect(ForestAdminDatasourcePylon.logger)
+ .to have_received(:warn).with(/carries no 'is_read_only' flag; leaving it read-only/)
+ 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
@@ -192,8 +213,20 @@ def operators_of(type, **extra)
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.keys).to eq(%i[column_name schema multi_value])
expect(introspector.issue_custom_fields.first[:column_name]).to eq('sev_level')
end
+
+ # Pylon writes a multiselect back through `values` and every other type
+ # through `value`, so the payload builder is told which one this is rather
+ # than guessing it from the Json column type.
+ it 'flags a multiselect as multi-valued, and nothing else' do
+ allow(client).to receive(:fetch_custom_fields).with('issue')
+ .and_return([definition('multiselect', **select_metadata('p1')),
+ definition('select', **select_metadata('p1')),
+ definition('text')])
+
+ expect(introspector.issue_custom_fields.map { |cf| cf[:multi_value] }).to eq([true, false, false])
+ end
end
end