-
Notifications
You must be signed in to change notification settings - Fork 1
feat(datasource-pylon): write operations (CRUD) (EXT-11) #362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
christophebrun-forest
merged 8 commits into
feat/datasource-pylon
from
ext-11-crud-writes
Aug 21, 2026
+1,654
−103
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
03c47f3
feat(pylon): write operations (CRUD)
christophebrun-forest 89b75cd
fix(pylon): review findings on the write path
christophebrun-forest 02a16d4
fix(pylon): second review pass on the write path
christophebrun-forest bed97b7
fix(pylon): third review pass on the write path
christophebrun-forest 01f486b
fix(pylon): fourth review pass on the write path
christophebrun-forest c8934b1
refactor(pylon): trim the comments on the write path
christophebrun-forest dda783a
fix(pylon): charge a write the reads it spends resolving its selection
christophebrun-forest 44cd3c2
fix(pylon): leave a custom field read-only until Pylon flags it writable
christophebrun-forest File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
88 changes: 88 additions & 0 deletions
88
packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client/writes.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 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) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.