feat(datasource-pylon): close and create-with-notification action plugins (EXT-12)

[christophebrun-forest]

Part of EXT-12 — Story 8 of EXT-4.

Two of the three plugins the story scopes. SnoozeIssue — POST /issues/{id}/snooze, the Pylon bonus with no Zendesk equivalent — is not in here, and the client exposes no snooze endpoint yet, so EXT-12 stays open.

What

File	Role
issue_enums.rb	the closed sets POST /issues and PATCH /issues/{id} document, shared by the two forms
plugins/issue_targets.rb	which issues an action was fired on: a column of the host collection, or the primary keys
plugins/close_issue.rb + close_issue/messages.rb	single and bulk state update, per-id rescue, and what the operator reads back
plugins/create_issue_with_notification.rb	the registration, its options, the success message
create_issue_with_notification/form_builder.rb	the form, the templates, the {{ record.field }} interpolation
create_issue_with_notification/payload.rb	what the filled form becomes on the wire

Both are subclasses of ForestAdminDatasourceCustomizer::Plugins::Plugin, registered by the host project through use, on any collection — no relation to Pylon is needed, Pylon creating the contact on the fly from the form's email.

The write goes through the client, not through the collection

An action is registered on the host collection, which is rarely PylonIssue, and going through a Pylon collection would mean resolving it from a datasource the plugin was not given. So both plugins call Client::Writes directly, and none of the guards EXT-11 built on the collection path applies here: no writable-column filter, no create/update direction, no MAX_WRITE_REQUESTS, and no WriteRejectedError.

That last one is worth a decision rather than an omission, because it is what the operator sees when Pylon refuses:

• CloseIssue handles it. One rescue per id, and the failed ids are named in the action's own result message with e.message — which is APIError's, carrying HTTP 4xx <Pylon's reason> (request_id: …). A single issue Pylon refuses (deleted, outside the token's scope, a status slug it does not know) costs neither the batch nor the reason. This is the caller APIError was documented for.
• CreateIssueWithNotification does not. client.create_issue is unrescued, so an APIError leaves the executor and reaches ErrorTranslator, which recognises neither HttpException, BusinessError nor ForestException and answers 'Unexpected error' with the status. The likeliest way this action fails — a required field Pylon wants, a value the endpoint refuses — therefore arrives with nothing in it. This is the one thing in the PR I would fix before merge, and the shape is already there: either the result_builder.error that CloseIssue uses per id, or surface_write_rejection's rule of translating the 4xx and leaving a 5xx alone.

Decisions worth reviewing

A state is not checked against the list Pylon ships. STANDARD_STATES is what a form offers, never what a write is checked against: an organization defines its own statuses on top of the five built-in ones, and refusing a slug would refuse the very workflow they built. :state therefore takes any non-empty string, and closed is the default.

One variant per scope, where the Zendesk plugin builds four. Zendesk has two terminal statuses to tell apart; Pylon has closed and, past it, the custom slugs — which the :state option takes, rather than a second dimension of action names nobody would read. Messages words a custom slug as "move to X" and the universal state as "close", so the action reads as the verb the operator fired.

An internal issue travels as no metadata at all. Where Zendesk notifies as a side effect of a public comment, Pylon says it outright: destination_metadata.destination names the channel body_html is delivered through, and its absence is what leaves the issue internal — the form the reference names for "do not contact the requester", and the one that stays right if Pylon adds a required companion field to a real destination. The "Send as internal note" checkbox is that choice, and it wins over the configured destination: it is the operator's call, made on the record they are looking at.

The Message field is value:, not default_value:, as soon as templates are configured. drop_default runs once — the data key sticks after the first render — where drop_deferred re-evaluates on every fetch, which is what re-fires the message proc when Template changes. The proc returns nil unless field_changed?('Template'), so set_watch_changes carries the operator's own edits across renders instead of overwriting them. The two-page wizard exists for the same reason, and stays homogeneous: ActionCollectionDecorator rejects a form mixing Page elements with non-Page ones.

{{ record.field }} is HTML-escaped into the message and not into the subject. The message ships as body_html and is delivered as such, so an unescaped < or & from a record value would break the outbound message or smuggle markup into it. The subject and the requester email are not HTML and are interpolated raw. A token whose field is nil interpolates to nothing rather than to "nil".

No Type field on the form. POST /issues does not take one — Pylon accepts type on an update only, which is what Issue::UPDATE_ONLY already says. No Priority default either, and none read back: Pylon never returns the priority of an issue, which is why no column carries it and why the field says so.

Nothing in the target resolution raises. IssueTargets answers "no issue selected" through the action's message when a column was renamed or a record the scope hides is selected, rather than through a stack trace in the panel. Same for the form: a requester_email_default resolver that raises, or a record that cannot be fetched for interpolation, degrades to no prefill and a warning.

The id writeback is best-effort. Pylon has no transaction to roll back and the issue exists whether or not the host record could be stamped with its id, so a failed writeback is reported as a warning appended to the success message rather than as a failure of the action.

Verification

701 examples, 0 failures — coverage 1551/1551 lines (100%, threshold 90), branch 94.3%. RuboCop clean over the 64 files of the package.

Plugin specs cover: the option validation of both plugins; the scopes built and their names overridden; a batch where one id fails and the rest land, with the failed ones named; no target resolved from a renamed column; the payload of each destination, the internal note winning over the configured one, and the email metadata only on an email delivery; the two form shapes, the template re-firing the message on a Template change and not otherwise, the token interpolation escaped into the message and raw into the subject, and a nil field interpolating to nothing; the writeback skipped, done, and failed.

Rebased onto feat/datasource-pylon after EXT-11 landed: the plugins touch no file of the write path, so the rebase carried no conflict.

Note

Add CloseIssue and CreateIssueWithNotification plugins to Pylon datasource

• Adds the CloseIssue plugin, which registers single and bulk actions to update Pylon issue states. It resolves target IDs via IssueTargets and handles errors per-ID so partial failures do not abort bulk operations.
• Adds the CreateIssueWithNotification plugin, which registers a single action to create issues and notify requesters. It uses FormBuilder for dynamic forms with record-based token interpolation and Payload to format API requests.
• Introduces shared helpers: IssueEnums for validating options, Messages for operator feedback, and IssueTargets for extracting issue IDs from records.
• Risk: CloseIssue.apply_state catches errors per-ID, so partial failures happen without aborting; CreateIssueWithNotification.write_back_issue_id logs warnings on failure but proceeds with the action.

📊 Macroscope summarized 6598012. 7 files reviewed, 9 issues evaluated, 7 issues filtered, 2 comments posted

🗂️ Filtered Issues

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/plugins/create_issue_with_notification/form_builder.rb — 2 comments posted, 9 evaluated, 7 filtered

• line 49: template_field reserves the visible value No template but also appends template titles without rejecting that same title. If an email_templates entry is titled No template, selecting it is indistinguishable from the sentinel and message_value returns '' instead of its content, making that configured template unusable. [ Out of scope (post-validation triage) ]
• line 111: by_title = templates.to_h silently collapses templates that share a title, while template_field still exposes both duplicate labels. With two configured templates named alike, either selection resolves to the last template's content, so the earlier template can never be selected. Validate title uniqueness or use unique option values. [ Out of scope ]
• line 111: templates.to_h collapses templates that share a title, while the enum still exposes both duplicate labels. Selecting either duplicate therefore always inserts the last template's content, so earlier templates with the same title are silently unusable. The plugin performs no uniqueness validation on email_templates. [ Out of scope (post-validation triage) ]
• line 111: message_value indexes templates only by their unvalidated :title, while No template is also used as a reserved sentinel. If two configured templates share a title, to_h silently keeps only the last one's content; if a template is titled No template, selecting it takes the sentinel branch and clears the Message instead of loading its content. Template titles should be validated as unique and distinct from NO_TEMPLATE (or selection should use non-colliding IDs). [ Out of scope (post-validation triage) ]
• line 116: message_value treats every selection titled No template as the sentinel, but template_field also permits an email template with that exact title and no validation reserves it. Configuring such a template makes it impossible to select: choosing it always returns '' instead of its content (and also creates duplicate enum values). [ Out of scope (post-validation triage) ]
• line 116: A configured template whose title is exactly "No template" is added to the enum, but message_value always treats that selected title as the sentinel and returns an empty message at line 116. Since template titles are not validated or reserved by the plugin, that legitimate template can never be selected and its content is silently discarded. [ Out of scope (post-validation triage) ]
• line 116: message_value treats the display label "No template" as a reserved sentinel, but template titles are not validated against it. If a configured template has that title, the enum contains two indistinguishable entries and selecting either always returns '', making that template impossible to use. Reject/reserve this title or use a distinct internal enum value. [ Out of scope (post-validation triage) ]

[linear-code]

EXT-12

[qltysh]

7 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with high complexity (count = 6): executor	5
qlty	Structure	Function with many parameters (count = 4): build_action	2

[qltysh]

Function with many parameters (count = 4): build_action [qlty:function-parameters]

[qltysh]

Function with high complexity (count = 6): executor [qlty:function-complexity]

[qltysh]

Function with many parameters (count = 4): success_message [qlty:function-parameters]

[qltysh]

Function with high complexity (count = 6): requester_default [qlty:function-complexity]

[qltysh]

Function with high complexity (count = 6): message_value [qlty:function-complexity]

[qltysh]

Function with high complexity (count = 5): interpolate [qlty:function-complexity]

[qltysh]

Function with high complexity (count = 7): resolve_issue_ids [qlty:function-complexity]

[macroscopeapp]

🟡 Medium create_issue_with_notification/form_builder.rb:126

fetch_record requests an empty projection, so production ActionContext#get_record returns no record fields and every {{ record.field }} interpolation resolves to ''; a proc-based requester_email_default likewise receives an empty record and resolves incorrectly. The fake context hides this by ignoring its fields argument. Extract the referenced fields and pass a populated projection to get_record (or otherwise request the full record).

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/plugins/create_issue_with_notification/form_builder.rb around line 126:

`fetch_record` requests an empty projection, so production `ActionContext#get_record` returns no record fields and every `{{ record.field }}` interpolation resolves to `''`; a proc-based `requester_email_default` likewise receives an empty record and resolves incorrectly. The fake context hides this by ignoring its `fields` argument. Extract the referenced fields and pass a populated projection to `get_record` (or otherwise request the full record).

[macroscopeapp]

🟡 Medium create_issue_with_notification/form_builder.rb:68

When email_templates is non-empty, message_field omits the configured default_message, so the form opens with a blank required Message because Template initially equals NO_TEMPLATE; selecting NO_TEMPLATE later also replaces the message with ''. Preserve the escaped default_message as default_value and have message_value return that value for NO_TEMPLATE, while keeping deferred template interpolation for other selections.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/plugins/create_issue_with_notification/form_builder.rb around line 68:

When `email_templates` is non-empty, `message_field` omits the configured `default_message`, so the form opens with a blank required `Message` because `Template` initially equals `NO_TEMPLATE`; selecting `NO_TEMPLATE` later also replaces the message with `''`. Preserve the escaped `default_message` as `default_value` and have `message_value` return that value for `NO_TEMPLATE`, while keeping deferred template interpolation for other selections.