From b95c090c103e5cbbe5dc7d6e2b15f8661e309274 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 09:49:46 +0200 Subject: [PATCH 1/9] feat(zendesk): add CreateTicketWithNotification and CloseTicket smart actions Two smart actions on the Zendesk datasource, both configurable from Datasource.new and reusable on any host collection via register_on: - CreateTicketWithNotification (auto-registered on ZendeskUser) Opens a ticket from a host record. The requester is identified by an email entered in the form, optionally pre-filled by requester_email_default (literal String at datasource level, String or Proc on register_on). Subject and Message defaults support {{record.}} interpolation. Message uses the RichText widget and ships to Zendesk as html_body. Optional ticket_id_field on register_on writes the new ticket id back to a configured field on the host record (best-effort: failure logs a warn and surfaces in the success message without rolling back the ticket). - CloseTicket (opt-in on ZendeskTicket) Two no-form actions per status (Single + Bulk) that flip the ticket status to solved or closed. Opt-in via close_ticket_statuses: %w[solved closed] on Datasource.new (empty by default). Internal: BaseCollection delegates execute/get_form to an internal ActionCollectionDecorator so actions registered at the datasource level get the full form lifecycle (defaults, conditions, watch_changes) that the agent applies to customizer-defined actions. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../forest_admin_datasource_zendesk/Gemfile | 1 + .../Gemfile-test | 1 + .../actions/close_ticket.rb | 73 ++++ .../create_ticket_with_notification.rb | 182 +++++++++ .../collections/base_collection.rb | 26 ++ .../collections/ticket.rb | 1 + .../collections/user.rb | 21 +- .../datasource.rb | 34 +- .../actions/close_ticket_spec.rb | 118 ++++++ .../create_ticket_with_notification_spec.rb | 361 ++++++++++++++++++ .../collections/ticket_spec.rb | 3 +- .../collections/user_spec.rb | 4 +- .../datasource_spec.rb | 32 ++ .../spec/spec_helper.rb | 1 + 14 files changed, 842 insertions(+), 16 deletions(-) create mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb create mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb create mode 100644 packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb create mode 100644 packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb diff --git a/packages/forest_admin_datasource_zendesk/Gemfile b/packages/forest_admin_datasource_zendesk/Gemfile index 56cf384f3..c229ff1d5 100644 --- a/packages/forest_admin_datasource_zendesk/Gemfile +++ b/packages/forest_admin_datasource_zendesk/Gemfile @@ -2,6 +2,7 @@ source 'https://rubygems.org' gemspec +gem 'forest_admin_datasource_customizer' gem 'forest_admin_datasource_toolkit' gem 'rake', '~> 13.0' gem 'rubocop', '1.86.1' diff --git a/packages/forest_admin_datasource_zendesk/Gemfile-test b/packages/forest_admin_datasource_zendesk/Gemfile-test index 126a451a7..64fab449a 100644 --- a/packages/forest_admin_datasource_zendesk/Gemfile-test +++ b/packages/forest_admin_datasource_zendesk/Gemfile-test @@ -9,6 +9,7 @@ gem 'rubocop-performance', '1.26.1' gem 'rubocop-rspec', '3.9.0' group :development, :test do + gem 'forest_admin_datasource_customizer', path: '../forest_admin_datasource_customizer' gem 'forest_admin_datasource_toolkit', path: '../forest_admin_datasource_toolkit' gem 'rspec', '~> 3.0' gem 'simplecov', '~> 0.22', require: false diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb new file mode 100644 index 000000000..1b73db5f5 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb @@ -0,0 +1,73 @@ +module ForestAdminDatasourceZendesk + module Actions + # No-form actions exposed on the ZendeskTicket collection that flip the + # ticket status to `solved` or `closed`. Each target status is exposed in + # both Single and Bulk scopes, so the actions surface from a ticket's + # detail page *and* from a multi-selection on the index. + # + # Status semantics: `solved` is the standard "resolved" workflow — the + # requester can still reopen during Zendesk's reopen window. `closed` is + # terminal; Zendesk itself sometimes rejects the direct `open → closed` + # transition, in which case the underlying API error bubbles up (we don't + # silently retry via `solved`). + # + # Opt-in registration: neither status is registered by default. Pass + # `close_ticket_statuses: %w[solved closed]` (or any subset) to + # `Datasource.new` to expose the variants you want. + module CloseTicket + BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction + ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope + + STATUSES = %w[solved closed].freeze + + NAMES = { + 'solved' => { single: 'Mark as solved', bulk: 'Mark selected as solved' }.freeze, + 'closed' => { single: 'Mark as closed', bulk: 'Mark selected as closed' }.freeze + }.freeze + + SCOPES = { single: ActionScope::SINGLE, bulk: ActionScope::BULK }.freeze + + module_function + + def variants(statuses = STATUSES) + unknown = statuses - STATUSES + if unknown.any? + raise ForestAdminDatasourceToolkit::Exceptions::ForestException, + "Unknown close_ticket_statuses: #{unknown.join(", ")}. Allowed: #{STATUSES.join(", ")}." + end + + statuses.flat_map do |status| + NAMES[status].map { |scope_key, name| [name, status, SCOPES[scope_key]] } + end + end + + # Reads `datasource.close_ticket_statuses` by default — falls back to an + # explicit `statuses:` kwarg if a caller wants to override (e.g. when + # attaching CloseTicket to a different collection). + def register_on(collection, datasource, statuses: nil) + variants(statuses || datasource.close_ticket_statuses).each do |name, status, scope| + collection.add_action(name, build(datasource, status: status, scope: scope)) + end + end + + def build(datasource, status:, scope:) + BaseAction.new(scope: scope, &executor(datasource, status)) + end + + def executor(datasource, status) + lambda do |context, result_builder| + ids = Array(context.record_ids).compact + next result_builder.error(message: 'No ticket selected.') if ids.empty? + + ids.each { |id| datasource.client.update_ticket(id, 'status' => status) } + result_builder.success(message: success_message(ids, status)) + end + end + + def success_message(ids, status) + verb = status == 'closed' ? 'closed' : 'marked as solved' + ids.size == 1 ? "Ticket ##{ids.first} #{verb}." : "#{ids.size} tickets #{verb}." + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb new file mode 100644 index 000000000..b8921a9bd --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb @@ -0,0 +1,182 @@ +module ForestAdminDatasourceZendesk + module Actions + # Smart action that opens a Zendesk ticket from any host collection. The + # selected record needs no relation to Zendesk: the requester is identified + # by an email entered (or pre-filled) in the form, and Zendesk creates the + # user record on the fly if it doesn't already exist. + # + # ZendeskUser auto-registers this action with a resolver that pre-fills the + # requester email from the user's `email` field. To expose the action on + # any other collection (e.g. a Postgres `customers` table customized via + # the agent), call `register_on` from the agent setup: + # + # ForestAdminDatasourceZendesk::Actions::CreateTicketWithNotification + # .register_on(customer, datasource, + # default_subject: 'Refund for {{record.email}}', + # default_message: '

Hi {{record.name}},

', + # requester_email_default: ->(record) { record['email'] }) + # + # `requester_email_default` accepts either: + # * a String — used as a literal email default (static), or + # * a Proc `record -> email_string` — evaluated against the selected + # record when the form opens (dynamic). + # + # `ticket_id_field` (register_on only) names a writable field on the host + # collection that receives the freshly-created ticket id. The update is + # best-effort: a failure (missing field, validation error, etc.) is logged + # and surfaced in the success message but doesn't roll back the ticket. + # + # The Message field uses Forest's RichText widget and ships as `html_body`. + # Subject and Message defaults support `{{record.}}` tokens resolved + # against the selected record when the form opens. + module CreateTicketWithNotification + BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction + ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope + FieldType = ForestAdminDatasourceCustomizer::Decorators::Action::Types::FieldType + + NAME = 'Create ticket and notify'.freeze + TOKEN_RE = /\{\{\s*record\.([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}/ + + module_function + + # All-kwargs registration — the ParameterLists cop counts each kwarg + # toward its 5-param limit, which doesn't really apply to named args. + def register_on(collection, datasource, default_subject: nil, default_message: nil, # rubocop:disable Metrics/ParameterLists + requester_email_default: nil, ticket_id_field: nil) + collection.add_action(NAME, build(datasource, + default_subject: default_subject, + default_message: default_message, + requester_email_default: requester_email_default, + ticket_id_field: ticket_id_field)) + end + + def build(datasource, default_subject: nil, default_message: nil, + requester_email_default: nil, ticket_id_field: nil) + BaseAction.new( + scope: ActionScope::SINGLE, + form: form(default_subject, default_message, requester_email_default), + &executor(datasource, ticket_id_field) + ) + end + + def form(default_subject, default_message, requester_email_default) + [ + { type: FieldType::STRING, label: 'Requester email', is_required: true, + description: 'Email of the Zendesk requester. Pre-filled from the selected record when available.', + default_value: requester_default(requester_email_default) }, + { type: FieldType::STRING, label: 'Subject', is_required: true, + default_value: template_default(default_subject) }, + { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, + description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ + 'default Zendesk notification email to the requester.', + default_value: template_default(default_message) }, + { type: FieldType::ENUM, label: 'Priority', enum_values: Collections::Ticket::ENUM_PRIORITY, + default_value: 'normal' }, + { type: FieldType::ENUM, label: 'Type', enum_values: Collections::Ticket::ENUM_TYPE }, + { type: FieldType::BOOLEAN, label: 'Send as internal note', + description: 'When checked, the first comment is private and no email is sent to the requester.', + default_value: false } + ] + end + + def executor(datasource, ticket_id_field = nil) + lambda do |context, result_builder| + values = context.form_values + email = values['Requester email'] + next result_builder.error(message: 'Requester email is required.') unless present?(email) + + ticket = datasource.client.create_ticket(build_payload(values, email)) + ticket_id = ticket.respond_to?(:[]) ? ticket['id'] : nil + + writeback = write_back_ticket_id(context, ticket_id_field, ticket_id) + result_builder.success(message: success_message(ticket_id, values, writeback)) + end + end + + # Returns one of :skipped, :ok, or [:failed, "reason"]. The host record + # update is best-effort: a writeback failure mustn't roll back the ticket + # we already created Zendesk-side, so we surface the issue in the success + # message and the agent log instead of erroring out. + def write_back_ticket_id(context, field, ticket_id) + return :skipped if field.nil? || ticket_id.nil? + + context.collection.update(context.filter, { field => ticket_id }) + :ok + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to store ticket id in '#{field}': #{e.class}: #{e.message}" + ) + [:failed, "#{e.class}: #{e.message}"] + end + + def build_payload(values, email) + internal_note = truthy?(values['Send as internal note']) + payload = { + 'requester' => { 'email' => email }, + 'subject' => values['Subject'], + 'comment' => { 'html_body' => values['Message'], 'public' => !internal_note } + } + payload['priority'] = values['Priority'] if present?(values['Priority']) + payload['type'] = values['Type'] if present?(values['Type']) + payload + end + + def requester_default(value) + return nil if value.nil? + return value if value.is_a?(String) + + lambda do |context| + record = fetch_record(context) + record.empty? ? nil : value.call(record) + rescue StandardError + nil + end + end + + def template_default(template) + return nil unless present?(template) + return template unless template.match?(TOKEN_RE) + + ->(context) { interpolate(template, fetch_record(context)) } + end + + def fetch_record(context) + context.get_record([]) || {} + rescue StandardError + {} + end + + def interpolate(template, record) + template.gsub(TOKEN_RE) do + key = ::Regexp.last_match(1) + value = record[key] || record[key.to_sym] + value.nil? ? '' : value.to_s + end + end + + def success_message(ticket_id, values, writeback = :skipped) + base = base_success_message(ticket_id, values) + return base unless writeback.is_a?(Array) && writeback.first == :failed + + # Ticket created Zendesk-side, but the writeback to the host collection failed. + "#{base} (warning: could not store the ticket id on the record — #{writeback.last})" + end + + def base_success_message(ticket_id, values) + if truthy?(values['Send as internal note']) + ticket_id ? "Ticket ##{ticket_id} created (internal note, no email)." : 'Ticket created (internal note).' + else + ticket_id ? "Ticket ##{ticket_id} created and requester notified." : 'Ticket created and requester notified.' + end + end + + def truthy?(value) + value == true || value.to_s.casecmp('true').zero? + end + + def present?(value) + !value.nil? && value.to_s != '' + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb index 08f4b54d5..0872e406f 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb @@ -11,6 +11,26 @@ class BaseCollection < ForestAdminDatasourceToolkit::Collection DATE_OPS = [Operators::EQUAL, Operators::BEFORE, Operators::AFTER, Operators::PRESENT, Operators::BLANK].freeze + # Actions registered via `add_action` live in `schema[:actions]` and surface + # in the frontend through the agent's ActionCollectionDecorator (which merges + # them via `refine_schema`). Execution and form rendering, however, fall + # through to this collection — Forest's Collection contract leaves them + # NotImplementedError. We pipe both through an internal decorator instance so + # we reuse the same form lifecycle (default evaluation, conditional fields, + # change watching) the agent uses for customizer-defined actions. + def add_action(name, action) + super + action_runtime.add_action(name, action) + end + + def execute(caller, name, data, filter = nil) + action_runtime.execute(caller, name, data, filter) + end + + def get_form(caller, name, data = nil, filter = nil, metas = {}) + action_runtime.get_form(caller, name, data, filter, metas) + end + def aggregate(caller, filter, aggregation, _limit = nil) unless aggregation.operation == 'Count' && aggregation.field.nil? && aggregation.groups.empty? raise ForestAdminDatasourceToolkit::Exceptions::ForestException, @@ -91,6 +111,12 @@ def build_zendesk_query(caller, filter) private + def action_runtime + @action_runtime ||= ForestAdminDatasourceCustomizer::Decorators::Action::ActionCollectionDecorator.new( + self, datasource + ) + end + def sort_field_and_direction(entry) return [entry.field, entry.ascending] if entry.respond_to?(:field) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb index 4475d1662..da7214041 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb @@ -35,6 +35,7 @@ def initialize(datasource, custom_fields: []) @custom_fields = custom_fields define_schema define_relations + Actions::CloseTicket.register_on(self, datasource) enable_search enable_count end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb index f85407f55..4659e6545 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb @@ -6,6 +6,8 @@ class User < BaseCollection ManyToOneSchema = ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema OneToManySchema = ForestAdminDatasourceToolkit::Schema::Relations::OneToManySchema ENUM_ROLE = %w[end-user agent admin].freeze + BASE_ATTR_KEYS = %w[id email name role phone organization_id time_zone locale verified suspended + created_at updated_at].freeze ZENDESK_SORTABLE = { 'created_at' => 'created_at', @@ -18,6 +20,12 @@ def initialize(datasource, custom_fields: []) @custom_fields = custom_fields define_schema define_relations + Actions::CreateTicketWithNotification.register_on( + self, datasource, + default_subject: datasource.default_ticket_subject, + default_message: datasource.default_ticket_message, + requester_email_default: datasource.requester_email_default || ->(record) { record['email'] } + ) enable_search enable_count end @@ -100,22 +108,11 @@ def define_relations def serialize(user) attrs = attrs_of(user) - result = base_attributes(attrs) + result = BASE_ATTR_KEYS.to_h { |k| [k, attrs[k]] } user_fields = attrs['user_fields'] || {} @custom_fields.each { |cf| result[cf[:column_name]] = user_fields[cf[:zendesk_key]] } result end - - def base_attributes(attrs) - { - 'id' => attrs['id'], 'email' => attrs['email'], 'name' => attrs['name'], - 'role' => attrs['role'], 'phone' => attrs['phone'], - 'organization_id' => attrs['organization_id'], - 'time_zone' => attrs['time_zone'], 'locale' => attrs['locale'], - 'verified' => attrs['verified'], 'suspended' => attrs['suspended'], - 'created_at' => attrs['created_at'], 'updated_at' => attrs['updated_at'] - } - end end end end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb index 396d15490..932895d90 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb @@ -1,12 +1,42 @@ module ForestAdminDatasourceZendesk class Datasource < ForestAdminDatasourceToolkit::Datasource - attr_reader :client, :configuration, :custom_field_mapping + attr_reader :client, :configuration, :custom_field_mapping, + :default_ticket_subject, :default_ticket_message, :requester_email_default, + :close_ticket_statuses - def initialize(subdomain:, username:, token:) + # Optional kwargs: + # + # CreateTicketWithNotification smart action (auto-registered on ZendeskUser): + # * `default_ticket_subject` / `default_ticket_message` are strings that + # support `{{record.}}` tokens, interpolated against the + # selected record when the form opens. `default_ticket_message` is + # rendered in a RichText widget and shipped to Zendesk as `html_body`. + # * `requester_email_default` is a literal email String used as the + # static default for the "Requester email" form field. When unset, + # ZendeskUser falls back to reading `record['email']` off the + # selected user. (When attaching the action to a non-ZendeskUser + # collection via `register_on(...)`, this kwarg also accepts a Proc + # `record -> email_string` for callers that need to compute it.) + # + # CloseTicket smart actions (registered on ZendeskTicket): + # * `close_ticket_statuses` is the opt-in list of close actions to + # expose, picked from `%w[solved closed]`. Defaults to `[]` — no + # close actions are registered unless explicitly listed. Each listed + # status registers both a Single and a Bulk variant. + # + # All-kwargs constructor — the ParameterLists cop counts each kwarg toward + # its 5-param limit, which doesn't really apply to named arguments. + def initialize(subdomain:, username:, token:, # rubocop:disable Metrics/ParameterLists + default_ticket_subject: nil, default_ticket_message: nil, + requester_email_default: nil, close_ticket_statuses: []) super() @configuration = Configuration.new(subdomain: subdomain, username: username, token: token) @client = Client.new(@configuration) @custom_field_mapping = {} + @default_ticket_subject = default_ticket_subject + @default_ticket_message = default_ticket_message + @requester_email_default = requester_email_default + @close_ticket_statuses = Array(close_ticket_statuses).map(&:to_s) register_collections end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb new file mode 100644 index 000000000..026208695 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb @@ -0,0 +1,118 @@ +module ForestAdminDatasourceZendesk + RSpec.describe Actions::CloseTicket do + let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } + let(:close_ticket_statuses) { [] } + let(:datasource) do + instance_double(ForestAdminDatasourceZendesk::Datasource, + client: client, custom_field_mapping: {}, + close_ticket_statuses: close_ticket_statuses) + end + let(:result_builder) { ForestAdminDatasourceCustomizer::Decorators::Action::ResultBuilder.new } + let(:action_scope) { ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope } + + describe '.variants' do + it 'yields all four variants when both statuses are requested' do + variants = described_class.variants(%w[solved closed]) + expect(variants.map { |name, _status, _scope| name }).to contain_exactly( + 'Mark as solved', 'Mark selected as solved', + 'Mark as closed', 'Mark selected as closed' + ) + + scopes = variants.each_with_object({}) { |(name, _status, scope), h| h[name] = scope } + expect(scopes['Mark as solved']).to eq(action_scope::SINGLE) + expect(scopes['Mark selected as solved']).to eq(action_scope::BULK) + expect(scopes['Mark as closed']).to eq(action_scope::SINGLE) + expect(scopes['Mark selected as closed']).to eq(action_scope::BULK) + end + + it 'yields only the requested status variants (subset filtering)' do + names = described_class.variants(%w[solved]).map(&:first) + expect(names).to contain_exactly('Mark as solved', 'Mark selected as solved') + end + + it 'yields nothing for an empty list' do + expect(described_class.variants([])).to be_empty + end + + it 'raises a ForestException on an unknown status' do + expect { described_class.variants(%w[solved unknown]) } + .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ForestException, /Unknown.*unknown/) + end + end + + describe 'executor' do + let(:context) { Struct.new(:record_ids).new([42]) } + + it 'PUTs status=solved for the selected ticket id' do + allow(client).to receive(:update_ticket) + + result = described_class.executor(datasource, 'solved').call(context, result_builder) + + expect(client).to have_received(:update_ticket).with(42, 'status' => 'solved') + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('Ticket #42', 'marked as solved') + end + + it 'PUTs status=closed for every id when run in bulk' do + allow(client).to receive(:update_ticket) + bulk_context = Struct.new(:record_ids).new([7, 8, 9]) + + result = described_class.executor(datasource, 'closed').call(bulk_context, result_builder) + + [7, 8, 9].each { |id| expect(client).to have_received(:update_ticket).with(id, 'status' => 'closed') } + expect(result[:message]).to include('3 tickets closed') + end + + it 'returns an error and skips the API when no ids are present' do + allow(client).to receive(:update_ticket) + empty_context = Struct.new(:record_ids).new([]) + + result = described_class.executor(datasource, 'solved').call(empty_context, result_builder) + + expect(client).not_to have_received(:update_ticket) + expect(result[:type]).to eq('Error') + end + end + + describe 'integration with Ticket collection' do + let(:ticket_collection) { Collections::Ticket.new(datasource) } + let(:filter) do + ForestAdminDatasourceToolkit::Components::Query::Filter.new( + condition_tree: ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf.new( + 'id', 'equal', 42 + ) + ) + end + + it 'registers nothing by default (opt-in)' do + action_keys = ticket_collection.schema[:actions].keys + expect(action_keys).not_to include('Mark as solved', 'Mark as closed', + 'Mark selected as solved', 'Mark selected as closed') + end + + context 'when close_ticket_statuses opts in to solved only' do + let(:close_ticket_statuses) { %w[solved] } + + it 'registers only the solved variants' do + keys = ticket_collection.schema[:actions].keys + expect(keys).to include('Mark as solved', 'Mark selected as solved') + expect(keys).not_to include('Mark as closed', 'Mark selected as closed') + end + end + + context 'when close_ticket_statuses opts in to both statuses' do + let(:close_ticket_statuses) { %w[solved closed] } + + it 'wires the action through Collection#execute end-to-end' do + allow(client).to receive(:fetch_tickets_by_ids).with([42]).and_return(42 => { 'id' => 42 }) + allow(client).to receive(:update_ticket) + + result = ticket_collection.execute(nil, 'Mark as solved', {}, filter) + + expect(client).to have_received(:update_ticket).with(42, 'status' => 'solved') + expect(result[:type]).to eq('Success') + end + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb new file mode 100644 index 000000000..51bf8fb69 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb @@ -0,0 +1,361 @@ +module ForestAdminDatasourceZendesk + # Tiny PORO standing in for an ActionContextSingle in unit tests. We can't + # use Struct here because `Struct` mixes in Enumerable, which already defines + # `#filter` — having a `:filter` member triggers Lint/StructNewOverride and + # is genuinely ambiguous. + class FakeActionContext + attr_reader :form_values, :collection, :filter, :record_id + + def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) + @form_values = form_values + @collection = collection + @filter = filter + @record_id = record_id + end + end + + RSpec.describe Actions::CreateTicketWithNotification do + let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } + let(:default_subject) { nil } + let(:default_message) { nil } + let(:requester_email_default) { nil } + let(:datasource_requester_default) { nil } + let(:datasource) do + instance_double(ForestAdminDatasourceZendesk::Datasource, + client: client, custom_field_mapping: {}, + default_ticket_subject: default_subject, + default_ticket_message: default_message, + requester_email_default: datasource_requester_default) + end + + let(:context) { Struct.new(:form_values).new(form_values) } + let(:result_builder) { ForestAdminDatasourceCustomizer::Decorators::Action::ResultBuilder.new } + let(:executor) { described_class.executor(datasource) } + + describe '.build' do + it 'returns a SINGLE-scoped action with the documented form fields' do + action = described_class.build(datasource) + + expect(action.scope).to eq(ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::SINGLE) + labels = action.form.map { |f| f[:label] } + expect(labels).to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) + end + + it 'uses RichText for the Message widget' do + message_field = described_class.build(datasource).form.find { |f| f[:label] == 'Message' } + expect(message_field[:widget]).to eq('RichText') + end + + it 'marks Requester email as required' do + field = described_class.build(datasource).form.first + expect(field[:label]).to eq('Requester email') + expect(field[:is_required]).to be(true) + end + end + + describe 'requester_email_default' do + let(:context_double) do + instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_record: { 'id' => 42, 'email' => 'alice@x.com', 'name' => 'Alice' }) + end + + it 'leaves the field empty (returns nil) when no default is configured' do + expect(described_class.build(datasource).form.first[:default_value]).to be_nil + end + + context 'when given a literal email String' do + it 'uses it verbatim as a static default (no record lookup)' do + action = described_class.build(datasource, requester_email_default: 'support@example.com') + expect(action.form.first[:default_value]).to eq('support@example.com') + end + end + + context 'when given a Proc resolver' do + let(:resolver) { ->(record) { record['email'] } } + + it 'pre-fills the email field from the selected record via the resolver' do + action = described_class.build(datasource, requester_email_default: resolver) + requester_proc = action.form.first[:default_value] + + expect(requester_proc.call(context_double)).to eq('alice@x.com') + end + + it 'survives a record lookup that raises (returns nil)' do + allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') + action = described_class.build(datasource, requester_email_default: resolver) + + expect(action.form.first[:default_value].call(context_double)).to be_nil + end + end + end + + describe 'subject / message defaults' do + let(:default_subject) { 'Welcome' } + let(:default_message) { '

Hi

' } + + it 'uses configured static defaults verbatim' do + action = described_class.build(datasource, + default_subject: default_subject, + default_message: default_message) + expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') + expect(action.form.find { |f| f[:label] == 'Message' }[:default_value]).to eq('

Hi

') + end + + context 'with {{record.}} tokens' do + let(:context_double) do + instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_record: { 'id' => 42, 'name' => 'Alice', 'email' => 'alice@x.com' }) + end + + it 'substitutes record fields at form-open time' do + action = described_class.build(datasource, + default_subject: 'Follow-up for {{record.name}}', + default_message: '

Hello {{record.name}} ({{record.email}})

') + + subject_proc = action.form.find { |f| f[:label] == 'Subject' }[:default_value] + message_proc = action.form.find { |f| f[:label] == 'Message' }[:default_value] + + expect(subject_proc.call(context_double)).to eq('Follow-up for Alice') + expect(message_proc.call(context_double)).to eq('

Hello Alice (alice@x.com)

') + end + + it 'falls back to empty strings when the record lookup fails' do + allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') + action = described_class.build(datasource, default_subject: 'Hi {{record.name}}') + + expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value].call(context_double)).to eq('Hi ') + end + end + end + + describe 'executor' do + context 'with a public html comment (default)' do + let(:form_values) do + { 'Requester email' => 'alice@x.com', 'Subject' => 'Refund', 'Message' => 'Hi there', + 'Priority' => 'high', 'Type' => 'question', 'Send as internal note' => false } + end + + it 'creates a ticket targeting the requester by email and embeds the html comment' do + allow(client).to receive(:create_ticket) do |payload| + expect(payload).to eq( + 'requester' => { 'email' => 'alice@x.com' }, + 'subject' => 'Refund', + 'comment' => { 'html_body' => 'Hi there', 'public' => true }, + 'priority' => 'high', + 'type' => 'question' + ) + { 'id' => 7 } + end + + result = executor.call(context, result_builder) + expect(client).to have_received(:create_ticket) + expect(result[:message]).to include('Ticket #7', 'notified') + end + end + + context 'with internal note' do + let(:form_values) do + { 'Requester email' => 'b@x.com', 'Subject' => 'Internal', 'Message' => 'For agents only', + 'Send as internal note' => true } + end + + it 'flips the comment to private and omits the notify wording' do + allow(client).to receive(:create_ticket).and_return('id' => 9) + + result = executor.call(context, result_builder) + expect(client).to have_received(:create_ticket).with(hash_including( + 'comment' => { 'html_body' => 'For agents only', + 'public' => false } + )) + expect(result[:message]).to include('internal note') + end + end + + context 'without a requester email' do + let(:form_values) { { 'Requester email' => nil, 'Subject' => 'S', 'Message' => 'M' } } + + it 'returns an error and does not call the client' do + allow(client).to receive(:create_ticket) + + result = executor.call(context, result_builder) + expect(result[:type]).to eq('Error') + expect(client).not_to have_received(:create_ticket) + end + end + + context 'with empty optional fields' do + let(:form_values) do + { 'Requester email' => 'c@x.com', 'Subject' => 'S', 'Message' => 'M', + 'Priority' => nil, 'Type' => '', 'Send as internal note' => nil } + end + + it 'omits empty optional keys from the payload' do + allow(client).to receive(:create_ticket) do |payload| + expect(payload.keys).not_to include('priority', 'type') + expect(payload['comment']['public']).to be(true) + { 'id' => 1 } + end + + executor.call(context, result_builder) + expect(client).to have_received(:create_ticket) + end + end + + context 'with ticket_id_field configured (writeback to host record)' do + let(:form_values) do + { 'Requester email' => 'd@x.com', 'Subject' => 'S', 'Message' => 'M', + 'Send as internal note' => false } + end + let(:host_collection) { instance_double('RelaxedCollection') } # rubocop:disable RSpec/VerifiedDoubleReference + let(:filter) { instance_double('Filter') } # rubocop:disable RSpec/VerifiedDoubleReference + let(:context) do + FakeActionContext.new(form_values: form_values, collection: host_collection, filter: filter) + end + let(:executor_with_writeback) { described_class.executor(datasource, 'zendesk_ticket_id') } + + before do + allow(client).to receive(:create_ticket).and_return('id' => 77) + end + + it 'updates the host record with the new ticket id under the configured field' do + allow(host_collection).to receive(:update) + + result = executor_with_writeback.call(context, result_builder) + + expect(host_collection).to have_received(:update).with(filter, { 'zendesk_ticket_id' => 77 }) + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('Ticket #77') + expect(result[:message]).not_to include('warning') + end + + it 'logs and surfaces a warning when the host update fails but still succeeds' do + allow(host_collection).to receive(:update).and_raise(StandardError, 'field is read-only') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + + result = executor_with_writeback.call(context, result_builder) + + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('Ticket #77', 'warning', 'field is read-only') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('zendesk_ticket_id', 'field is read-only')) + end + + it 'does not attempt any update when ticket_id_field is nil' do + allow(host_collection).to receive(:update) + + executor.call(context, result_builder) # executor without ticket_id_field + + expect(host_collection).not_to have_received(:update) + end + end + end + + describe '.register_on' do + let(:host_collection) do + Class.new do + attr_reader :registered + + def initialize = @registered = {} + def add_action(name, action) = @registered[name] = action + end.new + end + + it 'attaches the action to an arbitrary collection-like target' do + described_class.register_on(host_collection, datasource, + default_subject: 'Welcome', + requester_email_default: ->(r) { r['mail'] }) + + expect(host_collection.registered).to have_key(described_class::NAME) + action = host_collection.registered[described_class::NAME] + expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') + end + + it 'threads ticket_id_field to the executor (writeback enabled at registration)' do + described_class.register_on(host_collection, datasource, ticket_id_field: 'zd_id') + + relax = instance_double('RelaxedCollection') # rubocop:disable RSpec/VerifiedDoubleReference + filter = instance_double('Filter') # rubocop:disable RSpec/VerifiedDoubleReference + allow(relax).to receive(:update) + allow(client).to receive(:create_ticket).and_return('id' => 5) + + ctx = FakeActionContext.new( + form_values: { 'Requester email' => 'a@b.com', 'Subject' => 'S', 'Message' => 'M' }, + collection: relax, filter: filter + ) + host_collection.registered[described_class::NAME].execute.call(ctx, result_builder) + + expect(relax).to have_received(:update).with(filter, { 'zd_id' => 5 }) + end + end + + describe 'datasource-level requester_email_default' do + context 'when set to a literal email String' do + let(:datasource_requester_default) { 'support@example.com' } + + it 'becomes the static default of the ZendeskUser action (no record lookup)' do + user_collection = Collections::User.new(datasource) + requester_field = user_collection.schema[:actions][described_class::NAME].form.first + + expect(requester_field.default_value).to eq('support@example.com') + end + end + + context 'when set to a Proc (advanced)' do + let(:datasource_requester_default) { ->(record) { record['primary_email'] } } + let(:context_double) do + instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_record: { 'primary_email' => 'custom@x.com', 'email' => 'fallback@x.com' }) + end + + it 'overrides the ZendeskUser hardcoded record-reading fallback' do + user_collection = Collections::User.new(datasource) + requester_field = user_collection.schema[:actions][described_class::NAME].form.first + + expect(requester_field.default_value.call(context_double)).to eq('custom@x.com') + end + end + end + + describe 'integration with User collection' do + let(:user_collection) { Collections::User.new(datasource) } + let(:filter) do + ForestAdminDatasourceToolkit::Components::Query::Filter.new( + condition_tree: ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf.new( + 'id', 'equal', 42 + ) + ) + end + + it 'is registered on the ZendeskUser schema under the documented name' do + expect(user_collection.schema[:actions]).to have_key(described_class::NAME) + end + + it 'marks the action form as dynamic so the agent re-fetches it when opened' do + # The requester_email_default resolver injects a lambda, so the form + # must be re-evaluated per selected record (not cached statically). + expect(user_collection.schema[:actions][described_class::NAME].static_form).to be(false) + end + + it 'returns a typed form through Collection#get_form (regression for NotImplementedError)' do + allow(client).to receive(:find_user).with(42).and_return(Struct.new(:attributes).new({ 'id' => 42 })) + + form = user_collection.get_form(nil, described_class::NAME, nil, filter) + labels = form.map(&:label) + expect(labels).to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) + end + + it 'runs the action through Collection#execute (regression for NotImplementedError)' do + allow(client).to receive(:find_user).with(42).and_return(Struct.new(:attributes).new({ 'id' => 42 })) + allow(client).to receive(:create_ticket).and_return('id' => 99) + + result = user_collection.execute(nil, described_class::NAME, + { 'Requester email' => 'alice@x.com', 'Subject' => 'S', 'Message' => 'M' }, + filter) + expect(client).to have_received(:create_ticket).with(hash_including( + 'requester' => { 'email' => 'alice@x.com' } + )) + expect(result[:type]).to eq('Success') + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb index 4f822319a..4359eb959 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb @@ -12,7 +12,8 @@ module ForestAdminDatasourceZendesk let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } let(:datasource) do instance_double(ForestAdminDatasourceZendesk::Datasource, - client: client, custom_field_mapping: {}) + client: client, custom_field_mapping: {}, + close_ticket_statuses: []) end let(:collection) { described_class.new(datasource) } diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb index d34d96445..b5fa873af 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb @@ -10,7 +10,9 @@ module ForestAdminDatasourceZendesk let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } let(:datasource) do instance_double(ForestAdminDatasourceZendesk::Datasource, - client: client, custom_field_mapping: {}) + client: client, custom_field_mapping: {}, + default_ticket_subject: nil, default_ticket_message: nil, + requester_email_default: nil) end let(:collection) { described_class.new(datasource) } diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb index c72afda36..7e7b5bb9e 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb @@ -65,6 +65,38 @@ expect(ds.custom_field_mapping['tier']).to eq('tier') end + it 'defaults action templates to nil and close actions to none when no kwargs are supplied' do + ds = described_class.new(**valid_args) + expect(ds.default_ticket_subject).to be_nil + expect(ds.default_ticket_message).to be_nil + expect(ds.requester_email_default).to be_nil + expect(ds.close_ticket_statuses).to eq([]) + end + + it 'stores configured action defaults verbatim for the smart action to consume' do + ds = described_class.new(**valid_args, + default_ticket_subject: 'Welcome {{record.name}}', + default_ticket_message: '

Hi {{record.name}}

', + requester_email_default: 'support@example.com', + close_ticket_statuses: %w[solved closed]) + expect(ds.default_ticket_subject).to eq('Welcome {{record.name}}') + expect(ds.default_ticket_message).to eq('

Hi {{record.name}}

') + expect(ds.requester_email_default).to eq('support@example.com') + expect(ds.close_ticket_statuses).to eq(%w[solved closed]) + end + + it 'normalizes close_ticket_statuses entries to strings' do + ds = described_class.new(**valid_args, close_ticket_statuses: %i[solved closed]) + expect(ds.close_ticket_statuses).to eq(%w[solved closed]) + end + + it 'opts in CloseTicket variants on the Ticket schema when close_ticket_statuses is set' do + ds = described_class.new(**valid_args, close_ticket_statuses: %w[closed]) + actions = ds.get_collection('ZendeskTicket').schema[:actions].keys + expect(actions).to include('Mark as closed', 'Mark selected as closed') + expect(actions).not_to include('Mark as solved', 'Mark selected as solved') + end + it 'isolates custom field mappings between two datasource instances' do # Multi-tenancy: each datasource owns its own mapping. stub_request(:get, "#{base}/ticket_fields") diff --git a/packages/forest_admin_datasource_zendesk/spec/spec_helper.rb b/packages/forest_admin_datasource_zendesk/spec/spec_helper.rb index 561c180a8..4630eecc7 100644 --- a/packages/forest_admin_datasource_zendesk/spec/spec_helper.rb +++ b/packages/forest_admin_datasource_zendesk/spec/spec_helper.rb @@ -19,6 +19,7 @@ SimpleCov.coverage_dir 'coverage' require 'webmock/rspec' +require 'forest_admin_datasource_customizer' require 'forest_admin_datasource_zendesk' WebMock.disable_net_connect!(allow_localhost: true) From 57f68ff91f5b55c17eaf050c51735edde22f7e2b Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 10:15:34 +0200 Subject: [PATCH 2/9] fix(zendesk): harden smart actions (HTML escape + per-id rescue) Two follow-up fixes from code review: - CreateTicketWithNotification: HTML-escape {{record.}} tokens when interpolating into the Message template. Without this, a record value containing `<`, `&`, or markup would break the outbound HTML or smuggle markup into the email Zendesk triggers send to the requester. Subject interpolation remains unescaped since it's a plain-text field. - CloseTicket: walk ticket ids one by one inside the executor instead of letting the first Zendesk API error abort the rest of a bulk run. The most common case is Zendesk rejecting the direct open -> closed transition; previously the entire action returned a 500. Now: * All ids succeed: Success with the count. * Some fail (bulk): Success with the success count + list of failed ids. * All fail: Error with the underlying API reason. Each failure is also logged via the package logger. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../actions/close_ticket.rb | 40 ++++++++++++++-- .../create_ticket_with_notification.rb | 20 +++++--- .../actions/close_ticket_spec.rb | 46 +++++++++++++++++++ .../create_ticket_with_notification_spec.rb | 26 +++++++++++ 4 files changed, 122 insertions(+), 10 deletions(-) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb index 1b73db5f5..905147f34 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb @@ -59,14 +59,46 @@ def executor(datasource, status) ids = Array(context.record_ids).compact next result_builder.error(message: 'No ticket selected.') if ids.empty? - ids.each { |id| datasource.client.update_ticket(id, 'status' => status) } - result_builder.success(message: success_message(ids, status)) + succeeded, failed = apply_status(datasource, ids, status) + if succeeded.empty? + result_builder.error(message: error_message(failed, status)) + else + result_builder.success(message: success_message(succeeded, failed, status)) + end end end - def success_message(ids, status) + # Walks ids one by one so a single rejected transition (e.g. Zendesk + # refusing `open -> closed`) doesn't abort the rest of a bulk run. + # Returns `[succeeded_ids, failures]` where `failures` is `[[id, reason], ...]`. + def apply_status(datasource, ids, status) + succeeded = [] + failed = [] + ids.each do |id| + datasource.client.update_ticket(id, 'status' => status) + succeeded << id + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to set ticket ##{id} to '#{status}': #{e.class}: #{e.message}" + ) + failed << [id, "#{e.class}: #{e.message}"] + end + [succeeded, failed] + end + + def success_message(succeeded, failed, status) verb = status == 'closed' ? 'closed' : 'marked as solved' - ids.size == 1 ? "Ticket ##{ids.first} #{verb}." : "#{ids.size} tickets #{verb}." + base = succeeded.size == 1 ? "Ticket ##{succeeded.first} #{verb}." : "#{succeeded.size} tickets #{verb}." + return base if failed.empty? + + "#{base} #{failed.size} failed: #{failed.map(&:first).join(", ")}." + end + + def error_message(failed, status) + verb = status == 'closed' ? 'close' : 'mark as solved' + return "Failed to #{verb} ticket ##{failed.first.first}: #{failed.first.last}" if failed.size == 1 + + "Failed to #{verb} all #{failed.size} tickets. First error: #{failed.first.last}" end end end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb index b8921a9bd..4ff60cf51 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb @@ -1,3 +1,5 @@ +require 'cgi' + module ForestAdminDatasourceZendesk module Actions # Smart action that opens a Zendesk ticket from any host collection. The @@ -65,11 +67,11 @@ def form(default_subject, default_message, requester_email_default) description: 'Email of the Zendesk requester. Pre-filled from the selected record when available.', default_value: requester_default(requester_email_default) }, { type: FieldType::STRING, label: 'Subject', is_required: true, - default_value: template_default(default_subject) }, + default_value: template_default(default_subject, escape_html: false) }, { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ 'default Zendesk notification email to the requester.', - default_value: template_default(default_message) }, + default_value: template_default(default_message, escape_html: true) }, { type: FieldType::ENUM, label: 'Priority', enum_values: Collections::Ticket::ENUM_PRIORITY, default_value: 'normal' }, { type: FieldType::ENUM, label: 'Type', enum_values: Collections::Ticket::ENUM_TYPE }, @@ -133,11 +135,11 @@ def requester_default(value) end end - def template_default(template) + def template_default(template, escape_html:) return nil unless present?(template) return template unless template.match?(TOKEN_RE) - ->(context) { interpolate(template, fetch_record(context)) } + ->(context) { interpolate(template, fetch_record(context), escape_html: escape_html) } end def fetch_record(context) @@ -146,11 +148,17 @@ def fetch_record(context) {} end - def interpolate(template, record) + # `escape_html` matters for the Message template, which is rendered as HTML + # by the Zendesk comment endpoint (and possibly emailed to the requester). + # Without escaping, a record value containing `<`, `&`, or markup would + # break the layout or, worse, smuggle markup into the outbound email. + def interpolate(template, record, escape_html:) template.gsub(TOKEN_RE) do key = ::Regexp.last_match(1) value = record[key] || record[key.to_sym] - value.nil? ? '' : value.to_s + next '' if value.nil? + + escape_html ? CGI.escapeHTML(value.to_s) : value.to_s end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb index 026208695..38630666b 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb @@ -72,6 +72,52 @@ module ForestAdminDatasourceZendesk expect(client).not_to have_received(:update_ticket) expect(result[:type]).to eq('Error') end + + context 'when Zendesk rejects some ids (partial success on bulk)' do + let(:bulk_context) { Struct.new(:record_ids).new([7, 8, 9]) } + + it 'continues with the remaining ids and surfaces the failures in the message' do + # Mimics Zendesk refusing the `open -> closed` transition for id 8. + allow(client).to receive(:update_ticket) + .with(8, anything).and_raise(StandardError, 'cannot transition open to closed') + allow(client).to receive(:update_ticket).with(7, anything) + allow(client).to receive(:update_ticket).with(9, anything) + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + + result = described_class.executor(datasource, 'closed').call(bulk_context, result_builder) + + expect(client).to have_received(:update_ticket).with(7, 'status' => 'closed') + expect(client).to have_received(:update_ticket).with(8, 'status' => 'closed') + expect(client).to have_received(:update_ticket).with(9, 'status' => 'closed') + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('2 tickets closed', '1 failed', '8') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('#8', 'cannot transition')) + end + + it 'returns an Error when every id fails' do + allow(client).to receive(:update_ticket).and_raise(StandardError, 'permission denied') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn).exactly(3).times + + result = described_class.executor(datasource, 'closed').call(bulk_context, result_builder) + + expect(result[:type]).to eq('Error') + expect(result[:message]).to include('Failed to close', '3 tickets', 'permission denied') + end + end + + context 'when the only ticket fails (single scope)' do + it 'returns an Error with the underlying reason' do + allow(client).to receive(:update_ticket) + .and_raise(StandardError, 'invalid transition') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + + result = described_class.executor(datasource, 'closed').call(context, result_builder) + + expect(result[:type]).to eq('Error') + expect(result[:message]).to include('Failed to close', '#42', 'invalid transition') + end + end end describe 'integration with Ticket collection' do diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb index 51bf8fb69..527eabe6d 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb @@ -128,6 +128,32 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) end end + describe 'HTML escaping on the Message template' do + let(:context_double) do + instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_record: { 'name' => "O'Brien ", 'note' => '' }) + end + + it 'escapes interpolated values in the Message (RichText -> html_body)' do + # Otherwise a record value containing `<` or `&` would break the + # outbound HTML or smuggle markup into the requester email. + action = described_class.build(datasource, + default_message: '

Hi {{record.name}} - {{record.note}}

') + message_proc = action.form.find { |f| f[:label] == 'Message' }[:default_value] + + expect(message_proc.call(context_double)).to eq( + '

Hi O'Brien <admin> - <script>alert(1)</script>

' + ) + end + + it 'leaves Subject interpolation unescaped (plain-text field)' do + action = described_class.build(datasource, default_subject: 'Re: {{record.name}}') + subject_proc = action.form.find { |f| f[:label] == 'Subject' }[:default_value] + + expect(subject_proc.call(context_double)).to eq("Re: O'Brien ") + end + end + describe 'executor' do context 'with a public html comment (default)' do let(:form_values) do From fc9076e1025ae6e67d7966cf0fe263ebe89cde4d Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 10:37:03 +0200 Subject: [PATCH 3/9] chore(zendesk): code review polish (silent rescues, uniq, ASCII) Follow-up review nits, none functional: - CreateTicketWithNotification: log a warn before swallowing StandardError in `requester_default` and `fetch_record`. Previously, a typo in a user-supplied resolver lambda or a transient list failure would leave the email field silently empty with no signal anywhere. - Datasource: `.uniq` on `close_ticket_statuses` so an accidental `%w[solved solved closed]` no longer crashes registration with "Action ... already defined". - ASCII em-dash in the writeback-failure success message replaced by a colon, avoiding encoding hazards on downstream consumers. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../create_ticket_with_notification.rb | 12 ++++++++--- .../datasource.rb | 4 +++- .../create_ticket_with_notification_spec.rb | 21 +++++++++++++++++-- .../datasource_spec.rb | 7 +++++++ 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb index 4ff60cf51..d9aa61204 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb @@ -130,7 +130,10 @@ def requester_default(value) lambda do |context| record = fetch_record(context) record.empty? ? nil : value.call(record) - rescue StandardError + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] requester_email_default resolver raised: #{e.class}: #{e.message}" + ) nil end end @@ -144,7 +147,10 @@ def template_default(template, escape_html:) def fetch_record(context) context.get_record([]) || {} - rescue StandardError + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to fetch record for token interpolation: #{e.class}: #{e.message}" + ) {} end @@ -167,7 +173,7 @@ def success_message(ticket_id, values, writeback = :skipped) return base unless writeback.is_a?(Array) && writeback.first == :failed # Ticket created Zendesk-side, but the writeback to the host collection failed. - "#{base} (warning: could not store the ticket id on the record — #{writeback.last})" + "#{base} (warning: could not store the ticket id on the record: #{writeback.last})" end def base_success_message(ticket_id, values) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb index 932895d90..372df2ce0 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb @@ -36,7 +36,9 @@ def initialize(subdomain:, username:, token:, # rubocop:disable Metrics/Paramete @default_ticket_subject = default_ticket_subject @default_ticket_message = default_ticket_message @requester_email_default = requester_email_default - @close_ticket_statuses = Array(close_ticket_statuses).map(&:to_s) + # Dedup defensively: duplicates would make register_on raise + # "Action ... already defined" on the second registration. + @close_ticket_statuses = Array(close_ticket_statuses).map(&:to_s).uniq register_collections end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb index 527eabe6d..88711e318 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb @@ -80,11 +80,25 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) expect(requester_proc.call(context_double)).to eq('alice@x.com') end - it 'survives a record lookup that raises (returns nil)' do + it 'survives a record lookup that raises (returns nil) and logs the cause' do allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) action = described_class.build(datasource, requester_email_default: resolver) expect(action.form.first[:default_value].call(context_double)).to be_nil + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('fetch record', 'StandardError', 'boom')) + end + + it 'logs and returns nil when the resolver proc itself raises' do + allow(context_double).to receive(:get_record).and_return({ 'email' => 'a@b.com' }) + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + broken_resolver = ->(_record) { raise StandardError, 'typo in lambda' } + action = described_class.build(datasource, requester_email_default: broken_resolver) + + expect(action.form.first[:default_value].call(context_double)).to be_nil + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('requester_email_default', 'typo in lambda')) end end end @@ -119,11 +133,14 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) expect(message_proc.call(context_double)).to eq('

Hello Alice (alice@x.com)

') end - it 'falls back to empty strings when the record lookup fails' do + it 'falls back to empty strings when the record lookup fails (logged)' do allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) action = described_class.build(datasource, default_subject: 'Hi {{record.name}}') expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value].call(context_double)).to eq('Hi ') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('fetch record', 'boom')) end end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb index 7e7b5bb9e..d83cf10e7 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb @@ -90,6 +90,13 @@ expect(ds.close_ticket_statuses).to eq(%w[solved closed]) end + it 'dedupes close_ticket_statuses so duplicates do not crash registration' do + # Without uniq this would crash with "Action ... already defined" on the + # second registration of the same status. + ds = described_class.new(**valid_args, close_ticket_statuses: %w[solved solved closed]) + expect(ds.close_ticket_statuses).to eq(%w[solved closed]) + end + it 'opts in CloseTicket variants on the Ticket schema when close_ticket_statuses is set' do ds = described_class.new(**valid_args, close_ticket_statuses: %w[closed]) actions = ds.get_collection('ZendeskTicket').schema[:actions].keys From a731e0a30b0e10c6fda8e3b6b5a24b71d334dad5 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 11:24:26 +0200 Subject: [PATCH 4/9] chore(zendesk): trim over-documented comments to match repo style The new files were sitting at 19-35% comment density while the rest of the Zendesk package is at 0-2%. Dropped tutorial-style docstrings and restating comments; kept only the genuinely non-obvious notes (Zendesk open->closed transition, html_body escaping, writeback best-effort, internal ActionCollectionDecorator reuse). Final density: 3-6%, in line with the surrounding code. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../actions/close_ticket.rb | 23 ++------- .../create_ticket_with_notification.rb | 47 +++---------------- .../collections/base_collection.rb | 9 +--- .../datasource.rb | 24 ---------- 4 files changed, 12 insertions(+), 91 deletions(-) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb index 905147f34..eb84768ae 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb @@ -1,19 +1,7 @@ module ForestAdminDatasourceZendesk module Actions - # No-form actions exposed on the ZendeskTicket collection that flip the - # ticket status to `solved` or `closed`. Each target status is exposed in - # both Single and Bulk scopes, so the actions surface from a ticket's - # detail page *and* from a multi-selection on the index. - # - # Status semantics: `solved` is the standard "resolved" workflow — the - # requester can still reopen during Zendesk's reopen window. `closed` is - # terminal; Zendesk itself sometimes rejects the direct `open → closed` - # transition, in which case the underlying API error bubbles up (we don't - # silently retry via `solved`). - # - # Opt-in registration: neither status is registered by default. Pass - # `close_ticket_statuses: %w[solved closed]` (or any subset) to - # `Datasource.new` to expose the variants you want. + # Zendesk sometimes rejects the direct `open -> closed` transition; we + # surface the API error per-id rather than retrying via `solved`. module CloseTicket BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope @@ -41,9 +29,6 @@ def variants(statuses = STATUSES) end end - # Reads `datasource.close_ticket_statuses` by default — falls back to an - # explicit `statuses:` kwarg if a caller wants to override (e.g. when - # attaching CloseTicket to a different collection). def register_on(collection, datasource, statuses: nil) variants(statuses || datasource.close_ticket_statuses).each do |name, status, scope| collection.add_action(name, build(datasource, status: status, scope: scope)) @@ -68,9 +53,7 @@ def executor(datasource, status) end end - # Walks ids one by one so a single rejected transition (e.g. Zendesk - # refusing `open -> closed`) doesn't abort the rest of a bulk run. - # Returns `[succeeded_ids, failures]` where `failures` is `[[id, reason], ...]`. + # Per-id rescue so a single transition rejection doesn't abort bulk. def apply_status(datasource, ids, status) succeeded = [] failed = [] diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb index d9aa61204..de4221235 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb @@ -2,35 +2,9 @@ module ForestAdminDatasourceZendesk module Actions - # Smart action that opens a Zendesk ticket from any host collection. The - # selected record needs no relation to Zendesk: the requester is identified - # by an email entered (or pre-filled) in the form, and Zendesk creates the - # user record on the fly if it doesn't already exist. - # - # ZendeskUser auto-registers this action with a resolver that pre-fills the - # requester email from the user's `email` field. To expose the action on - # any other collection (e.g. a Postgres `customers` table customized via - # the agent), call `register_on` from the agent setup: - # - # ForestAdminDatasourceZendesk::Actions::CreateTicketWithNotification - # .register_on(customer, datasource, - # default_subject: 'Refund for {{record.email}}', - # default_message: '

Hi {{record.name}},

', - # requester_email_default: ->(record) { record['email'] }) - # - # `requester_email_default` accepts either: - # * a String — used as a literal email default (static), or - # * a Proc `record -> email_string` — evaluated against the selected - # record when the form opens (dynamic). - # - # `ticket_id_field` (register_on only) names a writable field on the host - # collection that receives the freshly-created ticket id. The update is - # best-effort: a failure (missing field, validation error, etc.) is logged - # and surfaced in the success message but doesn't roll back the ticket. - # - # The Message field uses Forest's RichText widget and ships as `html_body`. - # Subject and Message defaults support `{{record.}}` tokens resolved - # against the selected record when the form opens. + # Zendesk creates the requester user automatically from the form's email, + # so the host record needs no relation to Zendesk and the action can be + # `register_on`'d on any collection. module CreateTicketWithNotification BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope @@ -41,8 +15,6 @@ module CreateTicketWithNotification module_function - # All-kwargs registration — the ParameterLists cop counts each kwarg - # toward its 5-param limit, which doesn't really apply to named args. def register_on(collection, datasource, default_subject: nil, default_message: nil, # rubocop:disable Metrics/ParameterLists requester_email_default: nil, ticket_id_field: nil) collection.add_action(NAME, build(datasource, @@ -95,10 +67,8 @@ def executor(datasource, ticket_id_field = nil) end end - # Returns one of :skipped, :ok, or [:failed, "reason"]. The host record - # update is best-effort: a writeback failure mustn't roll back the ticket - # we already created Zendesk-side, so we surface the issue in the success - # message and the agent log instead of erroring out. + # Best-effort: a writeback failure mustn't roll back the ticket we + # already created Zendesk-side. def write_back_ticket_id(context, field, ticket_id) return :skipped if field.nil? || ticket_id.nil? @@ -154,10 +124,8 @@ def fetch_record(context) {} end - # `escape_html` matters for the Message template, which is rendered as HTML - # by the Zendesk comment endpoint (and possibly emailed to the requester). - # Without escaping, a record value containing `<`, `&`, or markup would - # break the layout or, worse, smuggle markup into the outbound email. + # Message ships as html_body; an unescaped `<` or `&` from a record value + # would break the outbound email or smuggle markup into it. def interpolate(template, record, escape_html:) template.gsub(TOKEN_RE) do key = ::Regexp.last_match(1) @@ -172,7 +140,6 @@ def success_message(ticket_id, values, writeback = :skipped) base = base_success_message(ticket_id, values) return base unless writeback.is_a?(Array) && writeback.first == :failed - # Ticket created Zendesk-side, but the writeback to the host collection failed. "#{base} (warning: could not store the ticket id on the record: #{writeback.last})" end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb index 0872e406f..a4d49b87d 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb @@ -11,13 +11,8 @@ class BaseCollection < ForestAdminDatasourceToolkit::Collection DATE_OPS = [Operators::EQUAL, Operators::BEFORE, Operators::AFTER, Operators::PRESENT, Operators::BLANK].freeze - # Actions registered via `add_action` live in `schema[:actions]` and surface - # in the frontend through the agent's ActionCollectionDecorator (which merges - # them via `refine_schema`). Execution and form rendering, however, fall - # through to this collection — Forest's Collection contract leaves them - # NotImplementedError. We pipe both through an internal decorator instance so - # we reuse the same form lifecycle (default evaluation, conditional fields, - # change watching) the agent uses for customizer-defined actions. + # Pipe through an internal ActionCollectionDecorator so actions registered + # at the datasource level reuse the agent's form lifecycle for free. def add_action(name, action) super action_runtime.add_action(name, action) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb index 372df2ce0..0ab1da1ee 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb @@ -4,28 +4,6 @@ class Datasource < ForestAdminDatasourceToolkit::Datasource :default_ticket_subject, :default_ticket_message, :requester_email_default, :close_ticket_statuses - # Optional kwargs: - # - # CreateTicketWithNotification smart action (auto-registered on ZendeskUser): - # * `default_ticket_subject` / `default_ticket_message` are strings that - # support `{{record.}}` tokens, interpolated against the - # selected record when the form opens. `default_ticket_message` is - # rendered in a RichText widget and shipped to Zendesk as `html_body`. - # * `requester_email_default` is a literal email String used as the - # static default for the "Requester email" form field. When unset, - # ZendeskUser falls back to reading `record['email']` off the - # selected user. (When attaching the action to a non-ZendeskUser - # collection via `register_on(...)`, this kwarg also accepts a Proc - # `record -> email_string` for callers that need to compute it.) - # - # CloseTicket smart actions (registered on ZendeskTicket): - # * `close_ticket_statuses` is the opt-in list of close actions to - # expose, picked from `%w[solved closed]`. Defaults to `[]` — no - # close actions are registered unless explicitly listed. Each listed - # status registers both a Single and a Bulk variant. - # - # All-kwargs constructor — the ParameterLists cop counts each kwarg toward - # its 5-param limit, which doesn't really apply to named arguments. def initialize(subdomain:, username:, token:, # rubocop:disable Metrics/ParameterLists default_ticket_subject: nil, default_ticket_message: nil, requester_email_default: nil, close_ticket_statuses: []) @@ -36,8 +14,6 @@ def initialize(subdomain:, username:, token:, # rubocop:disable Metrics/Paramete @default_ticket_subject = default_ticket_subject @default_ticket_message = default_ticket_message @requester_email_default = requester_email_default - # Dedup defensively: duplicates would make register_on raise - # "Action ... already defined" on the second registration. @close_ticket_statuses = Array(close_ticket_statuses).map(&:to_s).uniq register_collections From 49af998b02523e828051e9d72d732b7e53c22992 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 12:09:47 +0200 Subject: [PATCH 5/9] feat(zendesk): action_name, email templates wizard, priority/type overrides Four new optional kwargs on Actions::CreateTicketWithNotification, all also propagated through Datasource.new for the ZendeskUser auto-registration: - action_name: overrides the label the action is registered under (defaults to 'Create ticket and notify' as before). - email_templates: array of { title:, content: } hashes. When present, the form becomes a two-page wizard: page 1 picks a template (or 'No template'), page 2 is the body form with Message pre-filled from the selection. 'No template' yields a strictly empty Message; default_ticket_message is ignored when templates are configured (strict opt-in to the wizard). - priority_override: when set, the Priority dropdown is removed from the form and this value is forced in the payload sent to Zendesk. - type_override: same for Type. Useful when Zendesk's setup requires those fields but you don't want the agent to choose. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../create_ticket_with_notification.rb | 144 ++++++++++++------ .../collections/user.rb | 8 +- .../datasource.rb | 18 ++- .../create_ticket_with_notification_spec.rb | 120 ++++++++++++++- .../collections/user_spec.rb | 3 +- .../datasource_spec.rb | 32 ++++ 6 files changed, 266 insertions(+), 59 deletions(-) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb index de4221235..b17bcdef1 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb @@ -5,68 +5,81 @@ module Actions # Zendesk creates the requester user automatically from the form's email, # so the host record needs no relation to Zendesk and the action can be # `register_on`'d on any collection. - module CreateTicketWithNotification + module CreateTicketWithNotification # rubocop:disable Metrics/ModuleLength BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope FieldType = ForestAdminDatasourceCustomizer::Decorators::Action::Types::FieldType NAME = 'Create ticket and notify'.freeze + NO_TEMPLATE = 'No template'.freeze TOKEN_RE = /\{\{\s*record\.([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}/ module_function - def register_on(collection, datasource, default_subject: nil, default_message: nil, # rubocop:disable Metrics/ParameterLists - requester_email_default: nil, ticket_id_field: nil) - collection.add_action(NAME, build(datasource, - default_subject: default_subject, - default_message: default_message, - requester_email_default: requester_email_default, - ticket_id_field: ticket_id_field)) - end - - def build(datasource, default_subject: nil, default_message: nil, - requester_email_default: nil, ticket_id_field: nil) - BaseAction.new( - scope: ActionScope::SINGLE, - form: form(default_subject, default_message, requester_email_default), - &executor(datasource, ticket_id_field) - ) + def register_on(collection, datasource, **opts) + collection.add_action(opts[:action_name] || NAME, build(datasource, **opts.except(:action_name))) + end + + def build(datasource, **opts) + opts[:email_templates] = Array(opts[:email_templates]).compact + BaseAction.new(scope: ActionScope::SINGLE, form: form(opts), &executor(datasource, opts)) end - def form(default_subject, default_message, requester_email_default) + # When email_templates are configured the form becomes a two-page wizard + # (template selection, then the prefilled body). Otherwise the original + # flat form is used. The ActionCollectionDecorator rejects forms that mix + # pages with non-page elements, so we keep each mode strictly homogeneous. + def form(opts) + body = body_fields(opts) + return body if opts[:email_templates].empty? + [ - { type: FieldType::STRING, label: 'Requester email', is_required: true, - description: 'Email of the Zendesk requester. Pre-filled from the selected record when available.', - default_value: requester_default(requester_email_default) }, - { type: FieldType::STRING, label: 'Subject', is_required: true, - default_value: template_default(default_subject, escape_html: false) }, - { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, - description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ - 'default Zendesk notification email to the requester.', - default_value: template_default(default_message, escape_html: true) }, - { type: FieldType::ENUM, label: 'Priority', enum_values: Collections::Ticket::ENUM_PRIORITY, - default_value: 'normal' }, - { type: FieldType::ENUM, label: 'Type', enum_values: Collections::Ticket::ENUM_TYPE }, - { type: FieldType::BOOLEAN, label: 'Send as internal note', - description: 'When checked, the first comment is private and no email is sent to the requester.', - default_value: false } + { type: 'Layout', component: 'Page', next_button_label: 'Continue', + elements: [template_field(opts[:email_templates])] }, + { type: 'Layout', component: 'Page', previous_button_label: 'Back', + elements: body } ] end - def executor(datasource, ticket_id_field = nil) + def body_fields(opts) + fields = [requester_field(opts[:requester_email_default]), + subject_field(opts[:default_subject]), + message_field(opts[:default_message], opts[:email_templates])] + fields << priority_field unless present?(opts[:priority_override]) + fields << type_field unless present?(opts[:type_override]) + fields << internal_note_field + fields + end + + def executor(datasource, opts = {}) lambda do |context, result_builder| values = context.form_values email = values['Requester email'] next result_builder.error(message: 'Requester email is required.') unless present?(email) - ticket = datasource.client.create_ticket(build_payload(values, email)) + payload = build_payload(values, email, opts) + ticket = datasource.client.create_ticket(payload) ticket_id = ticket.respond_to?(:[]) ? ticket['id'] : nil - writeback = write_back_ticket_id(context, ticket_id_field, ticket_id) + writeback = write_back_ticket_id(context, opts[:ticket_id_field], ticket_id) result_builder.success(message: success_message(ticket_id, values, writeback)) end end + def build_payload(values, email, opts = {}) + internal_note = truthy?(values['Send as internal note']) + payload = { + 'requester' => { 'email' => email }, + 'subject' => values['Subject'], + 'comment' => { 'html_body' => values['Message'], 'public' => !internal_note } + } + priority = present?(opts[:priority_override]) ? opts[:priority_override] : values['Priority'] + type = present?(opts[:type_override]) ? opts[:type_override] : values['Type'] + payload['priority'] = priority if present?(priority) + payload['type'] = type if present?(type) + payload + end + # Best-effort: a writeback failure mustn't roll back the ticket we # already created Zendesk-side. def write_back_ticket_id(context, field, ticket_id) @@ -81,16 +94,44 @@ def write_back_ticket_id(context, field, ticket_id) [:failed, "#{e.class}: #{e.message}"] end - def build_payload(values, email) - internal_note = truthy?(values['Send as internal note']) - payload = { - 'requester' => { 'email' => email }, - 'subject' => values['Subject'], - 'comment' => { 'html_body' => values['Message'], 'public' => !internal_note } - } - payload['priority'] = values['Priority'] if present?(values['Priority']) - payload['type'] = values['Type'] if present?(values['Type']) - payload + def requester_field(default) + { type: FieldType::STRING, label: 'Requester email', is_required: true, + description: 'Email of the Zendesk requester. Pre-filled from the selected record when available.', + default_value: requester_default(default) } + end + + def template_field(templates) + { type: FieldType::ENUM, label: 'Template', is_required: true, + enum_values: [NO_TEMPLATE] + templates.map { |t| t[:title] }, + default_value: NO_TEMPLATE, + description: 'Pick a template to pre-fill the Message on the next page.' } + end + + def subject_field(default_subject) + { type: FieldType::STRING, label: 'Subject', is_required: true, + default_value: template_default(default_subject, escape_html: false) } + end + + def message_field(default_message, templates) + { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, + description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ + 'default Zendesk notification email to the requester.', + default_value: message_default(default_message, templates) } + end + + def priority_field + { type: FieldType::ENUM, label: 'Priority', + enum_values: Collections::Ticket::ENUM_PRIORITY, default_value: 'normal' } + end + + def type_field + { type: FieldType::ENUM, label: 'Type', enum_values: Collections::Ticket::ENUM_TYPE } + end + + def internal_note_field + { type: FieldType::BOOLEAN, label: 'Send as internal note', + description: 'When checked, the first comment is private and no email is sent to the requester.', + default_value: false } end def requester_default(value) @@ -115,6 +156,17 @@ def template_default(template, escape_html:) ->(context) { interpolate(template, fetch_record(context), escape_html: escape_html) } end + # When email_templates are configured the dropdown drives the Message + # entirely: picking 'No template' yields an empty body, picking a title + # injects the configured content. `default_message` is ignored in that + # mode (strict opt-in to the template wizard). + def message_default(default_message, templates) + return template_default(default_message, escape_html: true) if templates.empty? + + by_title = templates.to_h { |t| [t[:title], t[:content].to_s] } + ->(context) { by_title[context.get_form_value('Template')].to_s } + end + def fetch_record(context) context.get_record([]) || {} rescue StandardError => e diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb index 4659e6545..ed2f00038 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb @@ -1,6 +1,6 @@ module ForestAdminDatasourceZendesk module Collections - class User < BaseCollection + class User < BaseCollection # rubocop:disable Metrics/ClassLength include Searchable ManyToOneSchema = ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema @@ -22,9 +22,13 @@ def initialize(datasource, custom_fields: []) define_relations Actions::CreateTicketWithNotification.register_on( self, datasource, + action_name: datasource.default_ticket_action_name, default_subject: datasource.default_ticket_subject, default_message: datasource.default_ticket_message, - requester_email_default: datasource.requester_email_default || ->(record) { record['email'] } + requester_email_default: datasource.requester_email_default || ->(record) { record['email'] }, + email_templates: datasource.email_templates, + priority_override: datasource.priority_override, + type_override: datasource.type_override ) enable_search enable_count diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb index 0ab1da1ee..fccc252e1 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb @@ -2,19 +2,23 @@ module ForestAdminDatasourceZendesk class Datasource < ForestAdminDatasourceToolkit::Datasource attr_reader :client, :configuration, :custom_field_mapping, :default_ticket_subject, :default_ticket_message, :requester_email_default, + :default_ticket_action_name, :email_templates, + :priority_override, :type_override, :close_ticket_statuses - def initialize(subdomain:, username:, token:, # rubocop:disable Metrics/ParameterLists - default_ticket_subject: nil, default_ticket_message: nil, - requester_email_default: nil, close_ticket_statuses: []) + def initialize(subdomain:, username:, token:, **opts) super() @configuration = Configuration.new(subdomain: subdomain, username: username, token: token) @client = Client.new(@configuration) @custom_field_mapping = {} - @default_ticket_subject = default_ticket_subject - @default_ticket_message = default_ticket_message - @requester_email_default = requester_email_default - @close_ticket_statuses = Array(close_ticket_statuses).map(&:to_s).uniq + @default_ticket_subject = opts[:default_ticket_subject] + @default_ticket_message = opts[:default_ticket_message] + @requester_email_default = opts[:requester_email_default] + @default_ticket_action_name = opts[:default_ticket_action_name] + @email_templates = Array(opts[:email_templates]).compact + @priority_override = opts[:priority_override] + @type_override = opts[:type_override] + @close_ticket_statuses = Array(opts[:close_ticket_statuses]).map(&:to_s).uniq register_collections end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb index 88711e318..2c83a8387 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb @@ -20,12 +20,20 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) let(:default_message) { nil } let(:requester_email_default) { nil } let(:datasource_requester_default) { nil } + let(:datasource_action_name) { nil } + let(:datasource_email_templates) { [] } + let(:datasource_priority_override) { nil } + let(:datasource_type_override) { nil } let(:datasource) do instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}, default_ticket_subject: default_subject, default_ticket_message: default_message, - requester_email_default: datasource_requester_default) + requester_email_default: datasource_requester_default, + default_ticket_action_name: datasource_action_name, + email_templates: datasource_email_templates, + priority_override: datasource_priority_override, + type_override: datasource_type_override) end let(:context) { Struct.new(:form_values).new(form_values) } @@ -244,7 +252,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) end end - context 'with ticket_id_field configured (writeback to host record)' do + context 'with ticket_id_field configured (writeback to host record)' do # rubocop:disable RSpec/MultipleMemoizedHelpers let(:form_values) do { 'Requester email' => 'd@x.com', 'Subject' => 'S', 'Message' => 'M', 'Send as internal note' => false } @@ -254,7 +262,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) let(:context) do FakeActionContext.new(form_values: form_values, collection: host_collection, filter: filter) end - let(:executor_with_writeback) { described_class.executor(datasource, 'zendesk_ticket_id') } + let(:executor_with_writeback) { described_class.executor(datasource, ticket_id_field: 'zendesk_ticket_id') } before do allow(client).to receive(:create_ticket).and_return('id' => 77) @@ -400,5 +408,111 @@ def add_action(name, action) = @registered[name] = action expect(result[:type]).to eq('Success') end end + + describe 'action_name override' do + let(:collection) do + Class.new do + attr_reader :registered + + def initialize = @registered = {} + def add_action(name, action) = @registered[name] = action + end.new + end + + it 'registers the action under the configured name (default kept when omitted)' do + described_class.register_on(collection, datasource) + described_class.register_on(collection, datasource, action_name: 'Custom label') + + expect(collection.registered.keys).to contain_exactly(described_class::NAME, 'Custom label') + end + end + + describe 'email_templates wizard' do + let(:templates) do + [{ title: 'Welcome', content: '

Welcome aboard!

' }, + { title: 'Refund', content: '

Refund processed.

' }] + end + + it 'flips the form into a two-page wizard (Template first, body second)' do + action = described_class.build(datasource, email_templates: templates) + + expect(action.form.size).to eq(2) + page_one, page_two = action.form + expect(page_one[:component]).to eq('Page') + expect(page_one[:elements].map { |f| f[:label] }).to eq(['Template']) + expect(page_two[:elements].map { |f| f[:label] }) + .to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) + end + + it 'lists No template + each template title in the dropdown' do + action = described_class.build(datasource, email_templates: templates) + template_field = action.form.first[:elements].first + expect(template_field[:enum_values]).to eq(['No template', 'Welcome', 'Refund']) + expect(template_field[:default_value]).to eq('No template') + end + + it 'pre-fills Message with the selected template content' do + action = described_class.build(datasource, email_templates: templates) + message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_form_value: 'Refund') + + expect(message_field[:default_value].call(ctx)).to eq('

Refund processed.

') + end + + it "yields an empty Message when 'No template' is selected" do + action = described_class.build(datasource, email_templates: templates) + message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_form_value: 'No template') + + expect(message_field[:default_value].call(ctx)).to eq('') + end + + it 'keeps the original flat form when no templates are configured' do + action = described_class.build(datasource, email_templates: []) + expect(action.form.first[:component]).to be_nil # not a Page + end + end + + describe 'priority_override / type_override' do + it 'omits the Priority field and forces the value in the payload' do + action = described_class.build(datasource, priority_override: 'urgent') + labels = action.form.map { |f| f[:label] } + expect(labels).not_to include('Priority') + + allow(client).to receive(:create_ticket).and_return('id' => 1) + described_class.executor(datasource, priority_override: 'urgent').call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('priority' => 'urgent')) + end + + it 'omits the Type field and forces the value in the payload' do + action = described_class.build(datasource, type_override: 'incident') + labels = action.form.map { |f| f[:label] } + expect(labels).not_to include('Type') + + allow(client).to receive(:create_ticket).and_return('id' => 1) + described_class.executor(datasource, type_override: 'incident').call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('type' => 'incident')) + end + + it 'forces the override even when the form value is also present' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + described_class.executor(datasource, priority_override: 'urgent').call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', 'Subject' => 'S', + 'Message' => 'M', 'Priority' => 'low' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('priority' => 'urgent')) + end + end end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb index b5fa873af..e2a84bfbb 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb @@ -12,7 +12,8 @@ module ForestAdminDatasourceZendesk instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}, default_ticket_subject: nil, default_ticket_message: nil, - requester_email_default: nil) + requester_email_default: nil, default_ticket_action_name: nil, + email_templates: [], priority_override: nil, type_override: nil) end let(:collection) { described_class.new(datasource) } diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb index d83cf10e7..ededd6b95 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb @@ -125,4 +125,36 @@ expect(a.custom_field_mapping).to have_key('custom_1111') expect(b.custom_field_mapping).not_to have_key('custom_1111') end + + it 'exposes the new ticket action kwargs (action_name, templates, overrides) as attr_readers' do + templates = [{ title: 'Welcome', content: '

Hi

' }] + ds = described_class.new(**valid_args, + default_ticket_action_name: 'Open ticket', + email_templates: templates, + priority_override: 'urgent', + type_override: 'incident') + expect(ds.default_ticket_action_name).to eq('Open ticket') + expect(ds.email_templates).to eq(templates) + expect(ds.priority_override).to eq('urgent') + expect(ds.type_override).to eq('incident') + end + + it 'propagates the new kwargs into the ZendeskUser auto-registered action' do + templates = [{ title: 'Welcome', content: '

Hi

' }] + ds = described_class.new(**valid_args, + default_ticket_action_name: 'Open ticket', + email_templates: templates, + priority_override: 'urgent', + type_override: 'incident') + + actions = ds.get_collection('ZendeskUser').schema[:actions] + expect(actions.keys).to include('Open ticket') + + # Two-page wizard because templates are configured; Priority/Type are + # omitted from the body page since overrides are forced. + form = actions['Open ticket'].form + expect(form.size).to eq(2) + body_labels = form.last.elements.map(&:label) + expect(body_labels).not_to include('Priority', 'Type') + end end From 27a8df104d3c193b318a27ca6e140b9244b05c49 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 12:32:57 +0200 Subject: [PATCH 6/9] fix(zendesk): Message stays empty + interpolate tokens in templates Two related bugs in the template wizard: 1. The Message field used `default_value:` so drop_default skipped the proc once data['Message'] was cached, meaning selecting a template never refreshed the field. Switch to `value:` (re-evaluated every form fetch by drop_deferred) and gate the proc on `field_changed?('Template')`: emit the content when Template just changed, return nil otherwise so the agent's set_watch_changes carries over whatever the user typed. 2. `{{record.}}` tokens inside a template's content were not interpolated. Run the selected template content through the same HTML-escaping interpolate helper used for default_ticket_message (short-circuit when no token is present to skip the record fetch). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../create_ticket_with_notification.rb | 39 +++++++++++----- .../create_ticket_with_notification_spec.rb | 46 ++++++++++++++++--- 2 files changed, 67 insertions(+), 18 deletions(-) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb index b17bcdef1..a2972d2c2 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb @@ -113,10 +113,17 @@ def subject_field(default_subject) end def message_field(default_message, templates) - { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, - description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ - 'default Zendesk notification email to the requester.', - default_value: message_default(default_message, templates) } + field = { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, + description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ + 'default Zendesk notification email to the requester.' } + return field.merge(default_value: template_default(default_message, escape_html: true)) if templates.empty? + + # `value:` (not `default_value:`) — drop_default only runs when data + # doesn't already have the key, but after the first render the agent + # caches Message='' in data, so a default_value proc would never re-fire + # on Template change. `value:` is re-evaluated by drop_deferred on every + # form fetch. + field.merge(value: message_value(templates)) end def priority_field @@ -156,15 +163,23 @@ def template_default(template, escape_html:) ->(context) { interpolate(template, fetch_record(context), escape_html: escape_html) } end - # When email_templates are configured the dropdown drives the Message - # entirely: picking 'No template' yields an empty body, picking a title - # injects the configured content. `default_message` is ignored in that - # mode (strict opt-in to the template wizard). - def message_default(default_message, templates) - return template_default(default_message, escape_html: true) if templates.empty? - + # Returns the template content (with `{{record.X}}` tokens interpolated + # against the host record) when Template was just changed by the user; + # returns nil otherwise so the agent's set_watch_changes carries over the + # current Message input. 'No template' (or any unknown title) yields ''. + def message_value(templates) by_title = templates.to_h { |t| [t[:title], t[:content].to_s] } - ->(context) { by_title[context.get_form_value('Template')].to_s } + lambda do |context| + return nil unless context.field_changed?('Template') + + title = context.get_form_value('Template') + return '' if title == NO_TEMPLATE + + content = by_title[title].to_s + return content unless content.match?(TOKEN_RE) + + interpolate(content, fetch_record(context), escape_html: true) + end end def fetch_record(context) diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb index 2c83a8387..cf804f783 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb @@ -451,22 +451,56 @@ def add_action(name, action) = @registered[name] = action expect(template_field[:default_value]).to eq('No template') end - it 'pre-fills Message with the selected template content' do + it 'pre-fills Message via `value:` when Template was the just-changed field' do + # `value:` (not `default_value:`) so drop_deferred re-evaluates the proc + # on every form fetch — drop_default's `data.key?` check would otherwise + # block re-evaluation after the first render. action = described_class.build(datasource, email_templates: templates) message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - get_form_value: 'Refund') + field_changed?: true, get_form_value: 'Refund') - expect(message_field[:default_value].call(ctx)).to eq('

Refund processed.

') + expect(message_field[:value].call(ctx)).to eq('

Refund processed.

') end - it "yields an empty Message when 'No template' is selected" do + it "yields an empty Message when 'No template' was just selected" do action = described_class.build(datasource, email_templates: templates) message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - get_form_value: 'No template') + field_changed?: true, get_form_value: 'No template') - expect(message_field[:default_value].call(ctx)).to eq('') + expect(message_field[:value].call(ctx)).to eq('') + end + + it 'returns nil (carry over current input) when another field triggered the re-fetch' do + action = described_class.build(datasource, email_templates: templates) + message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: false) + + expect(message_field[:value].call(ctx)).to be_nil + end + + it 'interpolates {{record.}} tokens inside the selected template content' do + templated = [{ title: 'Welcome', content: '

Hi {{record.name}} ({{record.email}})

' }] + action = described_class.build(datasource, email_templates: templated) + message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: true, get_form_value: 'Welcome', + get_record: { 'name' => 'Alice', 'email' => 'a@b.com' }) + + expect(message_field[:value].call(ctx)).to eq('

Hi Alice (a@b.com)

') + end + + it 'HTML-escapes interpolated record values inside template content' do + templated = [{ title: 'Bug', content: '

{{record.note}}

' }] + action = described_class.build(datasource, email_templates: templated) + message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: true, get_form_value: 'Bug', + get_record: { 'note' => '' }) + + expect(message_field[:value].call(ctx)).to eq('

<script>x</script>

') end it 'keeps the original flat form when no templates are configured' do From 21ceb6276bd0f9dcdbc09a7b39264ed0c458e4aa Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 14:00:15 +0200 Subject: [PATCH 7/9] feat(zendesk): sender_email kwarg + auto-derive requester name - sender_email kwarg on Datasource.new and register_on. Mapped to Zendesk's `recipient` field in the create-ticket payload (the support address the ticket is tied to, which is also the From address of the notification email sent to the requester). Propagated from the datasource to the ZendeskUser auto-registered action. - Fix ZendeskAPI::Error::RecordInvalid 'Requester: Name: is too short': Zendesk auto-creates the requester user from the email when no match, but its validation requires a non-empty name. Derive name from the email's local-part ('john.doe@acme.com' -> 'john.doe') so the create step succeeds. Ignored when the email maps to an existing user. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../create_ticket_with_notification.rb | 19 ++++++- .../collections/user.rb | 3 +- .../datasource.rb | 3 +- .../create_ticket_with_notification_spec.rb | 57 +++++++++++++++++-- .../collections/user_spec.rb | 3 +- .../datasource_spec.rb | 4 +- 6 files changed, 77 insertions(+), 12 deletions(-) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb index a2972d2c2..ed9a44ff7 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb @@ -69,17 +69,30 @@ def executor(datasource, opts = {}) def build_payload(values, email, opts = {}) internal_note = truthy?(values['Send as internal note']) payload = { - 'requester' => { 'email' => email }, + # Zendesk creates the requester on the fly when no user matches the + # email, but its validation requires a non-empty name (>= 1 char). + # Derive it from the email's local-part so the create-user step + # doesn't fail; ignored when the email maps to an existing user. + 'requester' => { 'email' => email, 'name' => derive_requester_name(email) }, 'subject' => values['Subject'], 'comment' => { 'html_body' => values['Message'], 'public' => !internal_note } } priority = present?(opts[:priority_override]) ? opts[:priority_override] : values['Priority'] type = present?(opts[:type_override]) ? opts[:type_override] : values['Type'] - payload['priority'] = priority if present?(priority) - payload['type'] = type if present?(type) + payload['priority'] = priority if present?(priority) + payload['type'] = type if present?(type) + # Zendesk uses `recipient` to store the support address the ticket is + # associated with — that's the address replies/notifications appear + # to come FROM, so we expose it as `sender_email` to callers. + payload['recipient'] = opts[:sender_email] if present?(opts[:sender_email]) payload end + def derive_requester_name(email) + local = email.to_s.split('@').first.to_s + local.empty? ? email.to_s : local + end + # Best-effort: a writeback failure mustn't roll back the ticket we # already created Zendesk-side. def write_back_ticket_id(context, field, ticket_id) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb index ed2f00038..d0fefc881 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb @@ -28,7 +28,8 @@ def initialize(datasource, custom_fields: []) requester_email_default: datasource.requester_email_default || ->(record) { record['email'] }, email_templates: datasource.email_templates, priority_override: datasource.priority_override, - type_override: datasource.type_override + type_override: datasource.type_override, + sender_email: datasource.sender_email ) enable_search enable_count diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb index fccc252e1..a3975bc37 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb @@ -3,7 +3,7 @@ class Datasource < ForestAdminDatasourceToolkit::Datasource attr_reader :client, :configuration, :custom_field_mapping, :default_ticket_subject, :default_ticket_message, :requester_email_default, :default_ticket_action_name, :email_templates, - :priority_override, :type_override, + :priority_override, :type_override, :sender_email, :close_ticket_statuses def initialize(subdomain:, username:, token:, **opts) @@ -18,6 +18,7 @@ def initialize(subdomain:, username:, token:, **opts) @email_templates = Array(opts[:email_templates]).compact @priority_override = opts[:priority_override] @type_override = opts[:type_override] + @sender_email = opts[:sender_email] @close_ticket_statuses = Array(opts[:close_ticket_statuses]).map(&:to_s).uniq register_collections diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb index cf804f783..ecf417944 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb @@ -24,6 +24,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) let(:datasource_email_templates) { [] } let(:datasource_priority_override) { nil } let(:datasource_type_override) { nil } + let(:datasource_sender_email) { nil } let(:datasource) do instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}, @@ -33,7 +34,8 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) default_ticket_action_name: datasource_action_name, email_templates: datasource_email_templates, priority_override: datasource_priority_override, - type_override: datasource_type_override) + type_override: datasource_type_override, + sender_email: datasource_sender_email) end let(:context) { Struct.new(:form_values).new(form_values) } @@ -78,7 +80,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) end end - context 'when given a Proc resolver' do + context 'when given a Proc resolver' do # rubocop:disable RSpec/MultipleMemoizedHelpers let(:resolver) { ->(record) { record['email'] } } it 'pre-fills the email field from the selected record via the resolver' do @@ -189,7 +191,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) it 'creates a ticket targeting the requester by email and embeds the html comment' do allow(client).to receive(:create_ticket) do |payload| expect(payload).to eq( - 'requester' => { 'email' => 'alice@x.com' }, + 'requester' => { 'email' => 'alice@x.com', 'name' => 'alice' }, 'subject' => 'Refund', 'comment' => { 'html_body' => 'Hi there', 'public' => true }, 'priority' => 'high', @@ -367,7 +369,7 @@ def add_action(name, action) = @registered[name] = action end end - describe 'integration with User collection' do + describe 'integration with User collection' do # rubocop:disable RSpec/MultipleMemoizedHelpers let(:user_collection) { Collections::User.new(datasource) } let(:filter) do ForestAdminDatasourceToolkit::Components::Query::Filter.new( @@ -403,7 +405,8 @@ def add_action(name, action) = @registered[name] = action { 'Requester email' => 'alice@x.com', 'Subject' => 'S', 'Message' => 'M' }, filter) expect(client).to have_received(:create_ticket).with(hash_including( - 'requester' => { 'email' => 'alice@x.com' } + 'requester' => { 'email' => 'alice@x.com', + 'name' => 'alice' } )) expect(result[:type]).to eq('Success') end @@ -548,5 +551,49 @@ def add_action(name, action) = @registered[name] = action expect(client).to have_received(:create_ticket).with(hash_including('priority' => 'urgent')) end end + + describe 'requester name auto-derivation' do + # Zendesk's "create user on the fly from an email" path requires a + # non-empty `name` on the requester. We derive it from the email's + # local-part so the create succeeds; Zendesk silently ignores the name + # when the email already maps to an existing user. + it 'sends the email local-part as requester.name in the payload' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + described_class.executor(datasource).call( + FakeActionContext.new(form_values: { 'Requester email' => 'john.doe@acme.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including( + 'requester' => { 'email' => 'john.doe@acme.com', + 'name' => 'john.doe' } + )) + end + end + + describe 'sender_email' do + it 'maps to Zendesk `recipient` in the payload when configured' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + described_class.executor(datasource, sender_email: 'support@acme.com').call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('recipient' => 'support@acme.com')) + end + + it 'omits recipient from the payload when sender_email is blank' do + allow(client).to receive(:create_ticket) do |payload| + expect(payload).not_to have_key('recipient') + { 'id' => 1 } + end + described_class.executor(datasource).call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket) + end + end end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb index e2a84bfbb..6659b8d12 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb @@ -13,7 +13,8 @@ module ForestAdminDatasourceZendesk client: client, custom_field_mapping: {}, default_ticket_subject: nil, default_ticket_message: nil, requester_email_default: nil, default_ticket_action_name: nil, - email_templates: [], priority_override: nil, type_override: nil) + email_templates: [], priority_override: nil, type_override: nil, + sender_email: nil) end let(:collection) { described_class.new(datasource) } diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb index ededd6b95..c7b6d2614 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb @@ -132,11 +132,13 @@ default_ticket_action_name: 'Open ticket', email_templates: templates, priority_override: 'urgent', - type_override: 'incident') + type_override: 'incident', + sender_email: 'support@acme.com') expect(ds.default_ticket_action_name).to eq('Open ticket') expect(ds.email_templates).to eq(templates) expect(ds.priority_override).to eq('urgent') expect(ds.type_override).to eq('incident') + expect(ds.sender_email).to eq('support@acme.com') end it 'propagates the new kwargs into the ZendeskUser auto-registered action' do From a8ec9b6e66f9b40256b65791a2b3c004df77ffbe Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 17:13:52 +0200 Subject: [PATCH 8/9] refactor(zendesk): move smart actions to plugins, host-agnostic Drops auto-registration on ZendeskTicket / ZendeskUser and slims the Datasource constructor to just credentials. CreateTicketWithNotification and CloseTicket are now opted in per host collection via plugins, with the Zendesk ticket id read from a configurable column on the host record (e.g. last_zendesk_ticket_id). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../actions/close_ticket.rb | 45 ++++-- .../collections/ticket.rb | 1 - .../collections/user.rb | 13 +- .../datasource.rb | 17 +-- .../plugins/close_ticket.rb | 22 +++ .../create_ticket_with_notification.rb | 19 +++ .../actions/close_ticket_spec.rb | 144 ++++++++++-------- .../create_ticket_with_notification_spec.rb | 129 ++-------------- .../collections/ticket_spec.rb | 3 +- .../collections/user_spec.rb | 6 +- .../datasource_spec.rb | 79 +--------- .../plugins/close_ticket_spec.rb | 52 +++++++ .../create_ticket_with_notification_spec.rb | 44 ++++++ 13 files changed, 275 insertions(+), 299 deletions(-) create mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb create mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb create mode 100644 packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb create mode 100644 packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb index eb84768ae..e06d38c0a 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb @@ -1,7 +1,10 @@ module ForestAdminDatasourceZendesk module Actions # Zendesk sometimes rejects the direct `open -> closed` transition; we - # surface the API error per-id rather than retrying via `solved`. + # surface the API error per-id rather than retrying via `solved`. The + # action is registered on an arbitrary host collection — the Zendesk + # ticket id is read from a configurable column (`ticket_id_field`) on + # the host record(s). module CloseTicket BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope @@ -9,8 +12,10 @@ module CloseTicket STATUSES = %w[solved closed].freeze NAMES = { - 'solved' => { single: 'Mark as solved', bulk: 'Mark selected as solved' }.freeze, - 'closed' => { single: 'Mark as closed', bulk: 'Mark selected as closed' }.freeze + 'solved' => { single: 'Mark Zendesk ticket as solved', + bulk: 'Mark selected Zendesk tickets as solved' }.freeze, + 'closed' => { single: 'Mark Zendesk ticket as closed', + bulk: 'Mark selected Zendesk tickets as closed' }.freeze }.freeze SCOPES = { single: ActionScope::SINGLE, bulk: ActionScope::BULK }.freeze @@ -21,7 +26,7 @@ def variants(statuses = STATUSES) unknown = statuses - STATUSES if unknown.any? raise ForestAdminDatasourceToolkit::Exceptions::ForestException, - "Unknown close_ticket_statuses: #{unknown.join(", ")}. Allowed: #{STATUSES.join(", ")}." + "Unknown CloseTicket statuses: #{unknown.join(", ")}. Allowed: #{STATUSES.join(", ")}." end statuses.flat_map do |status| @@ -29,20 +34,20 @@ def variants(statuses = STATUSES) end end - def register_on(collection, datasource, statuses: nil) - variants(statuses || datasource.close_ticket_statuses).each do |name, status, scope| - collection.add_action(name, build(datasource, status: status, scope: scope)) + def register_on(collection, datasource, ticket_id_field:, statuses: nil) + variants(statuses || STATUSES).each do |name, status, scope| + collection.add_action(name, build(datasource, status: status, scope: scope, ticket_id_field: ticket_id_field)) end end - def build(datasource, status:, scope:) - BaseAction.new(scope: scope, &executor(datasource, status)) + def build(datasource, status:, scope:, ticket_id_field:) + BaseAction.new(scope: scope, &executor(datasource, status, ticket_id_field)) end - def executor(datasource, status) + def executor(datasource, status, ticket_id_field) lambda do |context, result_builder| - ids = Array(context.record_ids).compact - next result_builder.error(message: 'No ticket selected.') if ids.empty? + ids = resolve_ticket_ids(context, ticket_id_field) + next result_builder.error(message: "No Zendesk ticket id found in '#{ticket_id_field}'.") if ids.empty? succeeded, failed = apply_status(datasource, ids, status) if succeeded.empty? @@ -53,6 +58,22 @@ def executor(datasource, status) end end + # Reads the host record(s) and extracts the Zendesk ticket id from the + # configured field. Falls back to context.record_ids only when the + # host collection is itself the Zendesk Ticket collection (in which + # case ticket_id_field == 'id' is the canonical setup). + def resolve_ticket_ids(context, ticket_id_field) + records = context.get_records([ticket_id_field]) + records = [records].compact unless records.is_a?(Array) + records.filter_map { |r| r[ticket_id_field] || r[ticket_id_field.to_sym] } + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to resolve ticket ids from '#{ticket_id_field}': " \ + "#{e.class}: #{e.message}" + ) + [] + end + # Per-id rescue so a single transition rejection doesn't abort bulk. def apply_status(datasource, ids, status) succeeded = [] diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb index da7214041..4475d1662 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb @@ -35,7 +35,6 @@ def initialize(datasource, custom_fields: []) @custom_fields = custom_fields define_schema define_relations - Actions::CloseTicket.register_on(self, datasource) enable_search enable_count end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb index d0fefc881..bb20d9f92 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/user.rb @@ -1,6 +1,6 @@ module ForestAdminDatasourceZendesk module Collections - class User < BaseCollection # rubocop:disable Metrics/ClassLength + class User < BaseCollection include Searchable ManyToOneSchema = ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema @@ -20,17 +20,6 @@ def initialize(datasource, custom_fields: []) @custom_fields = custom_fields define_schema define_relations - Actions::CreateTicketWithNotification.register_on( - self, datasource, - action_name: datasource.default_ticket_action_name, - default_subject: datasource.default_ticket_subject, - default_message: datasource.default_ticket_message, - requester_email_default: datasource.requester_email_default || ->(record) { record['email'] }, - email_templates: datasource.email_templates, - priority_override: datasource.priority_override, - type_override: datasource.type_override, - sender_email: datasource.sender_email - ) enable_search enable_count end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb index a3975bc37..396d15490 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb @@ -1,25 +1,12 @@ module ForestAdminDatasourceZendesk class Datasource < ForestAdminDatasourceToolkit::Datasource - attr_reader :client, :configuration, :custom_field_mapping, - :default_ticket_subject, :default_ticket_message, :requester_email_default, - :default_ticket_action_name, :email_templates, - :priority_override, :type_override, :sender_email, - :close_ticket_statuses + attr_reader :client, :configuration, :custom_field_mapping - def initialize(subdomain:, username:, token:, **opts) + def initialize(subdomain:, username:, token:) super() @configuration = Configuration.new(subdomain: subdomain, username: username, token: token) @client = Client.new(@configuration) @custom_field_mapping = {} - @default_ticket_subject = opts[:default_ticket_subject] - @default_ticket_message = opts[:default_ticket_message] - @requester_email_default = opts[:requester_email_default] - @default_ticket_action_name = opts[:default_ticket_action_name] - @email_templates = Array(opts[:email_templates]).compact - @priority_override = opts[:priority_override] - @type_override = opts[:type_override] - @sender_email = opts[:sender_email] - @close_ticket_statuses = Array(opts[:close_ticket_statuses]).map(&:to_s).uniq register_collections end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb new file mode 100644 index 000000000..a6c8a6d95 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb @@ -0,0 +1,22 @@ +module ForestAdminDatasourceZendesk + module Plugins + # Registers "Mark as solved" / "Mark as closed" smart actions on an + # arbitrary host collection. The host record(s) must carry a column + # (`ticket_id_field`) holding the Zendesk ticket id to close. + class CloseTicket < ForestAdminDatasourceCustomizer::Plugins::Plugin + def run(_datasource_customizer, collection_customizer = nil, options = {}) + datasource = options[:datasource] + ticket_id_field = options[:ticket_id_field] + raise ArgumentError, 'CloseTicket plugin requires :datasource' unless datasource + raise ArgumentError, 'CloseTicket plugin requires :ticket_id_field' unless ticket_id_field + raise ArgumentError, 'CloseTicket plugin requires a collection' unless collection_customizer + + Actions::CloseTicket.register_on( + collection_customizer, datasource, + ticket_id_field: ticket_id_field, + statuses: options[:statuses] + ) + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb new file mode 100644 index 000000000..ae928e691 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb @@ -0,0 +1,19 @@ +module ForestAdminDatasourceZendesk + module Plugins + # Registers the "Create ticket and notify" smart action on an arbitrary + # host collection. The Zendesk datasource instance must be passed in via + # options so the action can reach the client; everything else mirrors + # Actions::CreateTicketWithNotification.register_on. + class CreateTicketWithNotification < ForestAdminDatasourceCustomizer::Plugins::Plugin + def run(_datasource_customizer, collection_customizer = nil, options = {}) + datasource = options[:datasource] + raise ArgumentError, 'CreateTicketWithNotification plugin requires :datasource' unless datasource + raise ArgumentError, 'CreateTicketWithNotification plugin requires a collection' unless collection_customizer + + Actions::CreateTicketWithNotification.register_on( + collection_customizer, datasource, **options.except(:datasource) + ) + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb index 38630666b..d6bca3faa 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb @@ -1,33 +1,43 @@ module ForestAdminDatasourceZendesk + # Stand-in for an action context. The executor only reads + # `get_records(fields)` so we don't need the full ActionContext class here. + class FakeCloseContext + def initialize(records: []) + @records = records + end + + def get_records(_fields = []) + @records + end + end + RSpec.describe Actions::CloseTicket do let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } - let(:close_ticket_statuses) { [] } let(:datasource) do - instance_double(ForestAdminDatasourceZendesk::Datasource, - client: client, custom_field_mapping: {}, - close_ticket_statuses: close_ticket_statuses) + instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}) end let(:result_builder) { ForestAdminDatasourceCustomizer::Decorators::Action::ResultBuilder.new } let(:action_scope) { ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope } + let(:ticket_id_field) { 'last_zendesk_ticket_id' } describe '.variants' do it 'yields all four variants when both statuses are requested' do variants = described_class.variants(%w[solved closed]) expect(variants.map { |name, _status, _scope| name }).to contain_exactly( - 'Mark as solved', 'Mark selected as solved', - 'Mark as closed', 'Mark selected as closed' + 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved', + 'Mark Zendesk ticket as closed', 'Mark selected Zendesk tickets as closed' ) scopes = variants.each_with_object({}) { |(name, _status, scope), h| h[name] = scope } - expect(scopes['Mark as solved']).to eq(action_scope::SINGLE) - expect(scopes['Mark selected as solved']).to eq(action_scope::BULK) - expect(scopes['Mark as closed']).to eq(action_scope::SINGLE) - expect(scopes['Mark selected as closed']).to eq(action_scope::BULK) + expect(scopes['Mark Zendesk ticket as solved']).to eq(action_scope::SINGLE) + expect(scopes['Mark selected Zendesk tickets as solved']).to eq(action_scope::BULK) + expect(scopes['Mark Zendesk ticket as closed']).to eq(action_scope::SINGLE) + expect(scopes['Mark selected Zendesk tickets as closed']).to eq(action_scope::BULK) end it 'yields only the requested status variants (subset filtering)' do names = described_class.variants(%w[solved]).map(&:first) - expect(names).to contain_exactly('Mark as solved', 'Mark selected as solved') + expect(names).to contain_exactly('Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved') end it 'yields nothing for an empty list' do @@ -41,54 +51,67 @@ module ForestAdminDatasourceZendesk end describe 'executor' do - let(:context) { Struct.new(:record_ids).new([42]) } + let(:executor) { described_class.executor(datasource, 'solved', ticket_id_field) } - it 'PUTs status=solved for the selected ticket id' do + it 'reads the ticket id from the host field and PUTs status=solved' do allow(client).to receive(:update_ticket) + context = FakeCloseContext.new(records: [{ ticket_id_field => 42 }]) - result = described_class.executor(datasource, 'solved').call(context, result_builder) + result = executor.call(context, result_builder) expect(client).to have_received(:update_ticket).with(42, 'status' => 'solved') expect(result[:type]).to eq('Success') expect(result[:message]).to include('Ticket #42', 'marked as solved') end - it 'PUTs status=closed for every id when run in bulk' do + it 'PUTs status=closed for every host record when run in bulk' do allow(client).to receive(:update_ticket) - bulk_context = Struct.new(:record_ids).new([7, 8, 9]) + bulk_executor = described_class.executor(datasource, 'closed', ticket_id_field) + bulk_context = FakeCloseContext.new( + records: [7, 8, 9].map { |id| { ticket_id_field => id } } + ) - result = described_class.executor(datasource, 'closed').call(bulk_context, result_builder) + result = bulk_executor.call(bulk_context, result_builder) [7, 8, 9].each { |id| expect(client).to have_received(:update_ticket).with(id, 'status' => 'closed') } expect(result[:message]).to include('3 tickets closed') end - it 'returns an error and skips the API when no ids are present' do + it 'returns an error when no host record carries a ticket id' do allow(client).to receive(:update_ticket) - empty_context = Struct.new(:record_ids).new([]) + context = FakeCloseContext.new(records: [{ ticket_id_field => nil }]) - result = described_class.executor(datasource, 'solved').call(empty_context, result_builder) + result = executor.call(context, result_builder) expect(client).not_to have_received(:update_ticket) expect(result[:type]).to eq('Error') + expect(result[:message]).to include(ticket_id_field) + end + + it 'works with symbol keys on the host record' do + allow(client).to receive(:update_ticket) + context = FakeCloseContext.new(records: [{ ticket_id_field.to_sym => 99 }]) + + executor.call(context, result_builder) + + expect(client).to have_received(:update_ticket).with(99, 'status' => 'solved') end context 'when Zendesk rejects some ids (partial success on bulk)' do - let(:bulk_context) { Struct.new(:record_ids).new([7, 8, 9]) } + let(:bulk_executor) { described_class.executor(datasource, 'closed', ticket_id_field) } + let(:bulk_context) do + FakeCloseContext.new(records: [7, 8, 9].map { |id| { ticket_id_field => id } }) + end it 'continues with the remaining ids and surfaces the failures in the message' do - # Mimics Zendesk refusing the `open -> closed` transition for id 8. allow(client).to receive(:update_ticket) .with(8, anything).and_raise(StandardError, 'cannot transition open to closed') allow(client).to receive(:update_ticket).with(7, anything) allow(client).to receive(:update_ticket).with(9, anything) allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) - result = described_class.executor(datasource, 'closed').call(bulk_context, result_builder) + result = bulk_executor.call(bulk_context, result_builder) - expect(client).to have_received(:update_ticket).with(7, 'status' => 'closed') - expect(client).to have_received(:update_ticket).with(8, 'status' => 'closed') - expect(client).to have_received(:update_ticket).with(9, 'status' => 'closed') expect(result[:type]).to eq('Success') expect(result[:message]).to include('2 tickets closed', '1 failed', '8') expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) @@ -99,65 +122,58 @@ module ForestAdminDatasourceZendesk allow(client).to receive(:update_ticket).and_raise(StandardError, 'permission denied') allow(ForestAdminDatasourceZendesk.logger).to receive(:warn).exactly(3).times - result = described_class.executor(datasource, 'closed').call(bulk_context, result_builder) + result = bulk_executor.call(bulk_context, result_builder) expect(result[:type]).to eq('Error') expect(result[:message]).to include('Failed to close', '3 tickets', 'permission denied') end end - context 'when the only ticket fails (single scope)' do - it 'returns an Error with the underlying reason' do - allow(client).to receive(:update_ticket) - .and_raise(StandardError, 'invalid transition') + context 'when get_records itself raises' do + it 'logs and returns an Error message without calling the client' do + context = instance_double( + ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle + ) + allow(context).to receive(:get_records).and_raise(StandardError, 'boom') allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + allow(client).to receive(:update_ticket) - result = described_class.executor(datasource, 'closed').call(context, result_builder) + result = executor.call(context, result_builder) + expect(client).not_to have_received(:update_ticket) expect(result[:type]).to eq('Error') - expect(result[:message]).to include('Failed to close', '#42', 'invalid transition') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including(ticket_id_field, 'boom')) end end end - describe 'integration with Ticket collection' do - let(:ticket_collection) { Collections::Ticket.new(datasource) } - let(:filter) do - ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf.new( - 'id', 'equal', 42 - ) - ) - end + describe '.register_on' do + let(:host_collection) do + Class.new do + attr_reader :registered - it 'registers nothing by default (opt-in)' do - action_keys = ticket_collection.schema[:actions].keys - expect(action_keys).not_to include('Mark as solved', 'Mark as closed', - 'Mark selected as solved', 'Mark selected as closed') + def initialize = @registered = {} + def add_action(name, action) = @registered[name] = action + end.new end - context 'when close_ticket_statuses opts in to solved only' do - let(:close_ticket_statuses) { %w[solved] } + it 'registers the four variants on an arbitrary host collection' do + described_class.register_on(host_collection, datasource, ticket_id_field: ticket_id_field) - it 'registers only the solved variants' do - keys = ticket_collection.schema[:actions].keys - expect(keys).to include('Mark as solved', 'Mark selected as solved') - expect(keys).not_to include('Mark as closed', 'Mark selected as closed') - end + expect(host_collection.registered.keys).to contain_exactly( + 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved', + 'Mark Zendesk ticket as closed', 'Mark selected Zendesk tickets as closed' + ) end - context 'when close_ticket_statuses opts in to both statuses' do - let(:close_ticket_statuses) { %w[solved closed] } + it 'registers only the subset specified via :statuses' do + described_class.register_on(host_collection, datasource, + ticket_id_field: ticket_id_field, statuses: %w[solved]) - it 'wires the action through Collection#execute end-to-end' do - allow(client).to receive(:fetch_tickets_by_ids).with([42]).and_return(42 => { 'id' => 42 }) - allow(client).to receive(:update_ticket) - - result = ticket_collection.execute(nil, 'Mark as solved', {}, filter) - - expect(client).to have_received(:update_ticket).with(42, 'status' => 'solved') - expect(result[:type]).to eq('Success') - end + expect(host_collection.registered.keys).to contain_exactly( + 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved' + ) end end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb index ecf417944..45ed96afc 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb @@ -16,26 +16,8 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) RSpec.describe Actions::CreateTicketWithNotification do let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } - let(:default_subject) { nil } - let(:default_message) { nil } - let(:requester_email_default) { nil } - let(:datasource_requester_default) { nil } - let(:datasource_action_name) { nil } - let(:datasource_email_templates) { [] } - let(:datasource_priority_override) { nil } - let(:datasource_type_override) { nil } - let(:datasource_sender_email) { nil } let(:datasource) do - instance_double(ForestAdminDatasourceZendesk::Datasource, - client: client, custom_field_mapping: {}, - default_ticket_subject: default_subject, - default_ticket_message: default_message, - requester_email_default: datasource_requester_default, - default_ticket_action_name: datasource_action_name, - email_templates: datasource_email_templates, - priority_override: datasource_priority_override, - type_override: datasource_type_override, - sender_email: datasource_sender_email) + instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}) end let(:context) { Struct.new(:form_values).new(form_values) } @@ -80,7 +62,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) end end - context 'when given a Proc resolver' do # rubocop:disable RSpec/MultipleMemoizedHelpers + context 'when given a Proc resolver' do let(:resolver) { ->(record) { record['email'] } } it 'pre-fills the email field from the selected record via the resolver' do @@ -114,13 +96,10 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) end describe 'subject / message defaults' do - let(:default_subject) { 'Welcome' } - let(:default_message) { '

Hi

' } - it 'uses configured static defaults verbatim' do action = described_class.build(datasource, - default_subject: default_subject, - default_message: default_message) + default_subject: 'Welcome', + default_message: '

Hi

') expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') expect(action.form.find { |f| f[:label] == 'Message' }[:default_value]).to eq('

Hi

') end @@ -254,7 +233,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) end end - context 'with ticket_id_field configured (writeback to host record)' do # rubocop:disable RSpec/MultipleMemoizedHelpers + context 'with ticket_id_field configured (writeback to host record)' do let(:form_values) do { 'Requester email' => 'd@x.com', 'Subject' => 'S', 'Message' => 'M', 'Send as internal note' => false } @@ -264,7 +243,9 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) let(:context) do FakeActionContext.new(form_values: form_values, collection: host_collection, filter: filter) end - let(:executor_with_writeback) { described_class.executor(datasource, ticket_id_field: 'zendesk_ticket_id') } + let(:executor_with_writeback) do + described_class.executor(datasource, ticket_id_field: 'last_zendesk_ticket_id') + end before do allow(client).to receive(:create_ticket).and_return('id' => 77) @@ -275,7 +256,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) result = executor_with_writeback.call(context, result_builder) - expect(host_collection).to have_received(:update).with(filter, { 'zendesk_ticket_id' => 77 }) + expect(host_collection).to have_received(:update).with(filter, { 'last_zendesk_ticket_id' => 77 }) expect(result[:type]).to eq('Success') expect(result[:message]).to include('Ticket #77') expect(result[:message]).not_to include('warning') @@ -290,7 +271,7 @@ def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) expect(result[:type]).to eq('Success') expect(result[:message]).to include('Ticket #77', 'warning', 'field is read-only') expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) - .with(a_string_including('zendesk_ticket_id', 'field is read-only')) + .with(a_string_including('last_zendesk_ticket_id', 'field is read-only')) end it 'does not attempt any update when ticket_id_field is nil' do @@ -339,94 +320,12 @@ def add_action(name, action) = @registered[name] = action expect(relax).to have_received(:update).with(filter, { 'zd_id' => 5 }) end - end - - describe 'datasource-level requester_email_default' do - context 'when set to a literal email String' do - let(:datasource_requester_default) { 'support@example.com' } - - it 'becomes the static default of the ZendeskUser action (no record lookup)' do - user_collection = Collections::User.new(datasource) - requester_field = user_collection.schema[:actions][described_class::NAME].form.first - - expect(requester_field.default_value).to eq('support@example.com') - end - end - - context 'when set to a Proc (advanced)' do - let(:datasource_requester_default) { ->(record) { record['primary_email'] } } - let(:context_double) do - instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - get_record: { 'primary_email' => 'custom@x.com', 'email' => 'fallback@x.com' }) - end - - it 'overrides the ZendeskUser hardcoded record-reading fallback' do - user_collection = Collections::User.new(datasource) - requester_field = user_collection.schema[:actions][described_class::NAME].form.first - - expect(requester_field.default_value.call(context_double)).to eq('custom@x.com') - end - end - end - - describe 'integration with User collection' do # rubocop:disable RSpec/MultipleMemoizedHelpers - let(:user_collection) { Collections::User.new(datasource) } - let(:filter) do - ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf.new( - 'id', 'equal', 42 - ) - ) - end - - it 'is registered on the ZendeskUser schema under the documented name' do - expect(user_collection.schema[:actions]).to have_key(described_class::NAME) - end - - it 'marks the action form as dynamic so the agent re-fetches it when opened' do - # The requester_email_default resolver injects a lambda, so the form - # must be re-evaluated per selected record (not cached statically). - expect(user_collection.schema[:actions][described_class::NAME].static_form).to be(false) - end - - it 'returns a typed form through Collection#get_form (regression for NotImplementedError)' do - allow(client).to receive(:find_user).with(42).and_return(Struct.new(:attributes).new({ 'id' => 42 })) - - form = user_collection.get_form(nil, described_class::NAME, nil, filter) - labels = form.map(&:label) - expect(labels).to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) - end - - it 'runs the action through Collection#execute (regression for NotImplementedError)' do - allow(client).to receive(:find_user).with(42).and_return(Struct.new(:attributes).new({ 'id' => 42 })) - allow(client).to receive(:create_ticket).and_return('id' => 99) - - result = user_collection.execute(nil, described_class::NAME, - { 'Requester email' => 'alice@x.com', 'Subject' => 'S', 'Message' => 'M' }, - filter) - expect(client).to have_received(:create_ticket).with(hash_including( - 'requester' => { 'email' => 'alice@x.com', - 'name' => 'alice' } - )) - expect(result[:type]).to eq('Success') - end - end - - describe 'action_name override' do - let(:collection) do - Class.new do - attr_reader :registered - - def initialize = @registered = {} - def add_action(name, action) = @registered[name] = action - end.new - end - it 'registers the action under the configured name (default kept when omitted)' do - described_class.register_on(collection, datasource) - described_class.register_on(collection, datasource, action_name: 'Custom label') + it 'registers under the configured name when action_name is provided' do + described_class.register_on(host_collection, datasource) + described_class.register_on(host_collection, datasource, action_name: 'Custom label') - expect(collection.registered.keys).to contain_exactly(described_class::NAME, 'Custom label') + expect(host_collection.registered.keys).to contain_exactly(described_class::NAME, 'Custom label') end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb index 4359eb959..4f822319a 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/ticket_spec.rb @@ -12,8 +12,7 @@ module ForestAdminDatasourceZendesk let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } let(:datasource) do instance_double(ForestAdminDatasourceZendesk::Datasource, - client: client, custom_field_mapping: {}, - close_ticket_statuses: []) + client: client, custom_field_mapping: {}) end let(:collection) { described_class.new(datasource) } diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb index 6659b8d12..d34d96445 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/collections/user_spec.rb @@ -10,11 +10,7 @@ module ForestAdminDatasourceZendesk let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } let(:datasource) do instance_double(ForestAdminDatasourceZendesk::Datasource, - client: client, custom_field_mapping: {}, - default_ticket_subject: nil, default_ticket_message: nil, - requester_email_default: nil, default_ticket_action_name: nil, - email_templates: [], priority_override: nil, type_override: nil, - sender_email: nil) + client: client, custom_field_mapping: {}) end let(:collection) { described_class.new(datasource) } diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb index c7b6d2614..6e0f47871 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/datasource_spec.rb @@ -29,6 +29,12 @@ ) end + it 'registers no smart actions by default (actions are opt-in via plugins)' do + ds = described_class.new(**valid_args) + expect(ds.get_collection('ZendeskTicket').schema[:actions]).to be_empty + expect(ds.get_collection('ZendeskUser').schema[:actions]).to be_empty + end + it 'forwards discovered ticket custom fields into the Ticket schema' do stub_request(:get, "#{base}/ticket_fields") .to_return(status: 200, @@ -65,45 +71,6 @@ expect(ds.custom_field_mapping['tier']).to eq('tier') end - it 'defaults action templates to nil and close actions to none when no kwargs are supplied' do - ds = described_class.new(**valid_args) - expect(ds.default_ticket_subject).to be_nil - expect(ds.default_ticket_message).to be_nil - expect(ds.requester_email_default).to be_nil - expect(ds.close_ticket_statuses).to eq([]) - end - - it 'stores configured action defaults verbatim for the smart action to consume' do - ds = described_class.new(**valid_args, - default_ticket_subject: 'Welcome {{record.name}}', - default_ticket_message: '

Hi {{record.name}}

', - requester_email_default: 'support@example.com', - close_ticket_statuses: %w[solved closed]) - expect(ds.default_ticket_subject).to eq('Welcome {{record.name}}') - expect(ds.default_ticket_message).to eq('

Hi {{record.name}}

') - expect(ds.requester_email_default).to eq('support@example.com') - expect(ds.close_ticket_statuses).to eq(%w[solved closed]) - end - - it 'normalizes close_ticket_statuses entries to strings' do - ds = described_class.new(**valid_args, close_ticket_statuses: %i[solved closed]) - expect(ds.close_ticket_statuses).to eq(%w[solved closed]) - end - - it 'dedupes close_ticket_statuses so duplicates do not crash registration' do - # Without uniq this would crash with "Action ... already defined" on the - # second registration of the same status. - ds = described_class.new(**valid_args, close_ticket_statuses: %w[solved solved closed]) - expect(ds.close_ticket_statuses).to eq(%w[solved closed]) - end - - it 'opts in CloseTicket variants on the Ticket schema when close_ticket_statuses is set' do - ds = described_class.new(**valid_args, close_ticket_statuses: %w[closed]) - actions = ds.get_collection('ZendeskTicket').schema[:actions].keys - expect(actions).to include('Mark as closed', 'Mark selected as closed') - expect(actions).not_to include('Mark as solved', 'Mark selected as solved') - end - it 'isolates custom field mappings between two datasource instances' do # Multi-tenancy: each datasource owns its own mapping. stub_request(:get, "#{base}/ticket_fields") @@ -125,38 +92,4 @@ expect(a.custom_field_mapping).to have_key('custom_1111') expect(b.custom_field_mapping).not_to have_key('custom_1111') end - - it 'exposes the new ticket action kwargs (action_name, templates, overrides) as attr_readers' do - templates = [{ title: 'Welcome', content: '

Hi

' }] - ds = described_class.new(**valid_args, - default_ticket_action_name: 'Open ticket', - email_templates: templates, - priority_override: 'urgent', - type_override: 'incident', - sender_email: 'support@acme.com') - expect(ds.default_ticket_action_name).to eq('Open ticket') - expect(ds.email_templates).to eq(templates) - expect(ds.priority_override).to eq('urgent') - expect(ds.type_override).to eq('incident') - expect(ds.sender_email).to eq('support@acme.com') - end - - it 'propagates the new kwargs into the ZendeskUser auto-registered action' do - templates = [{ title: 'Welcome', content: '

Hi

' }] - ds = described_class.new(**valid_args, - default_ticket_action_name: 'Open ticket', - email_templates: templates, - priority_override: 'urgent', - type_override: 'incident') - - actions = ds.get_collection('ZendeskUser').schema[:actions] - expect(actions.keys).to include('Open ticket') - - # Two-page wizard because templates are configured; Priority/Type are - # omitted from the body page since overrides are forced. - form = actions['Open ticket'].form - expect(form.size).to eq(2) - body_labels = form.last.elements.map(&:label) - expect(body_labels).not_to include('Priority', 'Type') - end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb new file mode 100644 index 000000000..73002ee42 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb @@ -0,0 +1,52 @@ +module ForestAdminDatasourceZendesk + RSpec.describe Plugins::CloseTicket do + let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } + let(:datasource) do + instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}) + end + let(:collection_customizer) do + Class.new do + attr_reader :registered + + def initialize = @registered = {} + def add_action(name, action) = @registered[name] = action + end.new + end + + it 'registers both solved and closed variants by default' do + described_class.new.run(nil, collection_customizer, + datasource: datasource, ticket_id_field: 'last_zendesk_ticket_id') + + expect(collection_customizer.registered.keys).to contain_exactly( + 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved', + 'Mark Zendesk ticket as closed', 'Mark selected Zendesk tickets as closed' + ) + end + + it 'honors :statuses to register a subset of variants' do + described_class.new.run(nil, collection_customizer, + datasource: datasource, ticket_id_field: 'last_zendesk_ticket_id', + statuses: %w[solved]) + + expect(collection_customizer.registered.keys).to contain_exactly( + 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved' + ) + end + + it 'raises ArgumentError without :datasource' do + expect { described_class.new.run(nil, collection_customizer, ticket_id_field: 'id') } + .to raise_error(ArgumentError, /datasource/) + end + + it 'raises ArgumentError without :ticket_id_field' do + expect { described_class.new.run(nil, collection_customizer, datasource: datasource) } + .to raise_error(ArgumentError, /ticket_id_field/) + end + + it 'raises ArgumentError without a collection_customizer' do + expect do + described_class.new.run(nil, nil, datasource: datasource, ticket_id_field: 'id') + end.to raise_error(ArgumentError, /collection/) + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb new file mode 100644 index 000000000..314bbec35 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb @@ -0,0 +1,44 @@ +module ForestAdminDatasourceZendesk + RSpec.describe Plugins::CreateTicketWithNotification do + let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } + let(:datasource) do + instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}) + end + let(:collection_customizer) do + Class.new do + attr_reader :registered + + def initialize = @registered = {} + def add_action(name, action) = @registered[name] = action + end.new + end + + it 'delegates registration to Actions::CreateTicketWithNotification.register_on' do + described_class.new.run(nil, collection_customizer, + datasource: datasource, + default_subject: 'Welcome', + ticket_id_field: 'last_zendesk_ticket_id') + + expect(collection_customizer.registered).to have_key(Actions::CreateTicketWithNotification::NAME) + action = collection_customizer.registered[Actions::CreateTicketWithNotification::NAME] + expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') + end + + it 'honors :action_name from options' do + described_class.new.run(nil, collection_customizer, + datasource: datasource, action_name: 'Open ticket') + + expect(collection_customizer.registered.keys).to contain_exactly('Open ticket') + end + + it 'raises ArgumentError without :datasource' do + expect { described_class.new.run(nil, collection_customizer, {}) } + .to raise_error(ArgumentError, /datasource/) + end + + it 'raises ArgumentError without a collection_customizer' do + expect { described_class.new.run(nil, nil, datasource: datasource) } + .to raise_error(ArgumentError, /collection/) + end + end +end From 8aca51b9a110b4ed6accd72ce8c5549fb2178bcc Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 13 May 2026 18:35:11 +0200 Subject: [PATCH 9/9] refactor(zendesk): polish plugin architecture - Inline Actions::* into their Plugins:: counterparts; drop the actions/ directory and the dead ActionCollectionDecorator plumbing in BaseCollection (the customizer wraps collections itself now). - Extract FormBuilder and Messages sub-modules to keep each plugin file focused on registration and orchestration. - Share Zendesk enum values via a new TicketEnums module so the schema and form builder reference a single source. - CloseTicket: replace the single statuses option with orthogonal statuses + scopes so callers can pick solved vs closed and single vs bulk independently. - CloseTicket: swap Zendesks raw "closed prevents ticket update" stack for a clean message: success ("was already closed") when targeting closed, Error ("cannot reopen to mark as solved") when targeting solved. - Trim over-documented comments and route specs through #run. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../actions/close_ticket.rb | 109 ---- .../create_ticket_with_notification.rb | 243 --------- .../collections/base_collection.rb | 21 - .../collections/ticket.rb | 4 - .../collections/ticket/schema_definition.rb | 9 +- .../plugins/close_ticket.rb | 125 ++++- .../plugins/close_ticket/messages.rb | 41 ++ .../create_ticket_with_notification.rb | 100 +++- .../form_builder.rb | 149 ++++++ .../ticket_enums.rb | 8 + .../actions/close_ticket_spec.rb | 180 ------- .../create_ticket_with_notification_spec.rb | 498 ------------------ .../plugins/close_ticket_spec.rb | 268 +++++++++- .../create_ticket_with_notification_spec.rb | 446 +++++++++++++++- 14 files changed, 1087 insertions(+), 1114 deletions(-) delete mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb delete mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb create mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket/messages.rb create mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification/form_builder.rb create mode 100644 packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/ticket_enums.rb delete mode 100644 packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb delete mode 100644 packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb deleted file mode 100644 index e06d38c0a..000000000 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/close_ticket.rb +++ /dev/null @@ -1,109 +0,0 @@ -module ForestAdminDatasourceZendesk - module Actions - # Zendesk sometimes rejects the direct `open -> closed` transition; we - # surface the API error per-id rather than retrying via `solved`. The - # action is registered on an arbitrary host collection — the Zendesk - # ticket id is read from a configurable column (`ticket_id_field`) on - # the host record(s). - module CloseTicket - BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction - ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope - - STATUSES = %w[solved closed].freeze - - NAMES = { - 'solved' => { single: 'Mark Zendesk ticket as solved', - bulk: 'Mark selected Zendesk tickets as solved' }.freeze, - 'closed' => { single: 'Mark Zendesk ticket as closed', - bulk: 'Mark selected Zendesk tickets as closed' }.freeze - }.freeze - - SCOPES = { single: ActionScope::SINGLE, bulk: ActionScope::BULK }.freeze - - module_function - - def variants(statuses = STATUSES) - unknown = statuses - STATUSES - if unknown.any? - raise ForestAdminDatasourceToolkit::Exceptions::ForestException, - "Unknown CloseTicket statuses: #{unknown.join(", ")}. Allowed: #{STATUSES.join(", ")}." - end - - statuses.flat_map do |status| - NAMES[status].map { |scope_key, name| [name, status, SCOPES[scope_key]] } - end - end - - def register_on(collection, datasource, ticket_id_field:, statuses: nil) - variants(statuses || STATUSES).each do |name, status, scope| - collection.add_action(name, build(datasource, status: status, scope: scope, ticket_id_field: ticket_id_field)) - end - end - - def build(datasource, status:, scope:, ticket_id_field:) - BaseAction.new(scope: scope, &executor(datasource, status, ticket_id_field)) - end - - def executor(datasource, status, ticket_id_field) - lambda do |context, result_builder| - ids = resolve_ticket_ids(context, ticket_id_field) - next result_builder.error(message: "No Zendesk ticket id found in '#{ticket_id_field}'.") if ids.empty? - - succeeded, failed = apply_status(datasource, ids, status) - if succeeded.empty? - result_builder.error(message: error_message(failed, status)) - else - result_builder.success(message: success_message(succeeded, failed, status)) - end - end - end - - # Reads the host record(s) and extracts the Zendesk ticket id from the - # configured field. Falls back to context.record_ids only when the - # host collection is itself the Zendesk Ticket collection (in which - # case ticket_id_field == 'id' is the canonical setup). - def resolve_ticket_ids(context, ticket_id_field) - records = context.get_records([ticket_id_field]) - records = [records].compact unless records.is_a?(Array) - records.filter_map { |r| r[ticket_id_field] || r[ticket_id_field.to_sym] } - rescue StandardError => e - ForestAdminDatasourceZendesk.logger.warn( - "[forest_admin_datasource_zendesk] failed to resolve ticket ids from '#{ticket_id_field}': " \ - "#{e.class}: #{e.message}" - ) - [] - end - - # Per-id rescue so a single transition rejection doesn't abort bulk. - def apply_status(datasource, ids, status) - succeeded = [] - failed = [] - ids.each do |id| - datasource.client.update_ticket(id, 'status' => status) - succeeded << id - rescue StandardError => e - ForestAdminDatasourceZendesk.logger.warn( - "[forest_admin_datasource_zendesk] failed to set ticket ##{id} to '#{status}': #{e.class}: #{e.message}" - ) - failed << [id, "#{e.class}: #{e.message}"] - end - [succeeded, failed] - end - - def success_message(succeeded, failed, status) - verb = status == 'closed' ? 'closed' : 'marked as solved' - base = succeeded.size == 1 ? "Ticket ##{succeeded.first} #{verb}." : "#{succeeded.size} tickets #{verb}." - return base if failed.empty? - - "#{base} #{failed.size} failed: #{failed.map(&:first).join(", ")}." - end - - def error_message(failed, status) - verb = status == 'closed' ? 'close' : 'mark as solved' - return "Failed to #{verb} ticket ##{failed.first.first}: #{failed.first.last}" if failed.size == 1 - - "Failed to #{verb} all #{failed.size} tickets. First error: #{failed.first.last}" - end - end - end -end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb deleted file mode 100644 index ed9a44ff7..000000000 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/actions/create_ticket_with_notification.rb +++ /dev/null @@ -1,243 +0,0 @@ -require 'cgi' - -module ForestAdminDatasourceZendesk - module Actions - # Zendesk creates the requester user automatically from the form's email, - # so the host record needs no relation to Zendesk and the action can be - # `register_on`'d on any collection. - module CreateTicketWithNotification # rubocop:disable Metrics/ModuleLength - BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction - ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope - FieldType = ForestAdminDatasourceCustomizer::Decorators::Action::Types::FieldType - - NAME = 'Create ticket and notify'.freeze - NO_TEMPLATE = 'No template'.freeze - TOKEN_RE = /\{\{\s*record\.([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}/ - - module_function - - def register_on(collection, datasource, **opts) - collection.add_action(opts[:action_name] || NAME, build(datasource, **opts.except(:action_name))) - end - - def build(datasource, **opts) - opts[:email_templates] = Array(opts[:email_templates]).compact - BaseAction.new(scope: ActionScope::SINGLE, form: form(opts), &executor(datasource, opts)) - end - - # When email_templates are configured the form becomes a two-page wizard - # (template selection, then the prefilled body). Otherwise the original - # flat form is used. The ActionCollectionDecorator rejects forms that mix - # pages with non-page elements, so we keep each mode strictly homogeneous. - def form(opts) - body = body_fields(opts) - return body if opts[:email_templates].empty? - - [ - { type: 'Layout', component: 'Page', next_button_label: 'Continue', - elements: [template_field(opts[:email_templates])] }, - { type: 'Layout', component: 'Page', previous_button_label: 'Back', - elements: body } - ] - end - - def body_fields(opts) - fields = [requester_field(opts[:requester_email_default]), - subject_field(opts[:default_subject]), - message_field(opts[:default_message], opts[:email_templates])] - fields << priority_field unless present?(opts[:priority_override]) - fields << type_field unless present?(opts[:type_override]) - fields << internal_note_field - fields - end - - def executor(datasource, opts = {}) - lambda do |context, result_builder| - values = context.form_values - email = values['Requester email'] - next result_builder.error(message: 'Requester email is required.') unless present?(email) - - payload = build_payload(values, email, opts) - ticket = datasource.client.create_ticket(payload) - ticket_id = ticket.respond_to?(:[]) ? ticket['id'] : nil - - writeback = write_back_ticket_id(context, opts[:ticket_id_field], ticket_id) - result_builder.success(message: success_message(ticket_id, values, writeback)) - end - end - - def build_payload(values, email, opts = {}) - internal_note = truthy?(values['Send as internal note']) - payload = { - # Zendesk creates the requester on the fly when no user matches the - # email, but its validation requires a non-empty name (>= 1 char). - # Derive it from the email's local-part so the create-user step - # doesn't fail; ignored when the email maps to an existing user. - 'requester' => { 'email' => email, 'name' => derive_requester_name(email) }, - 'subject' => values['Subject'], - 'comment' => { 'html_body' => values['Message'], 'public' => !internal_note } - } - priority = present?(opts[:priority_override]) ? opts[:priority_override] : values['Priority'] - type = present?(opts[:type_override]) ? opts[:type_override] : values['Type'] - payload['priority'] = priority if present?(priority) - payload['type'] = type if present?(type) - # Zendesk uses `recipient` to store the support address the ticket is - # associated with — that's the address replies/notifications appear - # to come FROM, so we expose it as `sender_email` to callers. - payload['recipient'] = opts[:sender_email] if present?(opts[:sender_email]) - payload - end - - def derive_requester_name(email) - local = email.to_s.split('@').first.to_s - local.empty? ? email.to_s : local - end - - # Best-effort: a writeback failure mustn't roll back the ticket we - # already created Zendesk-side. - def write_back_ticket_id(context, field, ticket_id) - return :skipped if field.nil? || ticket_id.nil? - - context.collection.update(context.filter, { field => ticket_id }) - :ok - rescue StandardError => e - ForestAdminDatasourceZendesk.logger.warn( - "[forest_admin_datasource_zendesk] failed to store ticket id in '#{field}': #{e.class}: #{e.message}" - ) - [:failed, "#{e.class}: #{e.message}"] - end - - def requester_field(default) - { type: FieldType::STRING, label: 'Requester email', is_required: true, - description: 'Email of the Zendesk requester. Pre-filled from the selected record when available.', - default_value: requester_default(default) } - end - - def template_field(templates) - { type: FieldType::ENUM, label: 'Template', is_required: true, - enum_values: [NO_TEMPLATE] + templates.map { |t| t[:title] }, - default_value: NO_TEMPLATE, - description: 'Pick a template to pre-fill the Message on the next page.' } - end - - def subject_field(default_subject) - { type: FieldType::STRING, label: 'Subject', is_required: true, - default_value: template_default(default_subject, escape_html: false) } - end - - def message_field(default_message, templates) - field = { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, - description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ - 'default Zendesk notification email to the requester.' } - return field.merge(default_value: template_default(default_message, escape_html: true)) if templates.empty? - - # `value:` (not `default_value:`) — drop_default only runs when data - # doesn't already have the key, but after the first render the agent - # caches Message='' in data, so a default_value proc would never re-fire - # on Template change. `value:` is re-evaluated by drop_deferred on every - # form fetch. - field.merge(value: message_value(templates)) - end - - def priority_field - { type: FieldType::ENUM, label: 'Priority', - enum_values: Collections::Ticket::ENUM_PRIORITY, default_value: 'normal' } - end - - def type_field - { type: FieldType::ENUM, label: 'Type', enum_values: Collections::Ticket::ENUM_TYPE } - end - - def internal_note_field - { type: FieldType::BOOLEAN, label: 'Send as internal note', - description: 'When checked, the first comment is private and no email is sent to the requester.', - default_value: false } - end - - def requester_default(value) - return nil if value.nil? - return value if value.is_a?(String) - - lambda do |context| - record = fetch_record(context) - record.empty? ? nil : value.call(record) - rescue StandardError => e - ForestAdminDatasourceZendesk.logger.warn( - "[forest_admin_datasource_zendesk] requester_email_default resolver raised: #{e.class}: #{e.message}" - ) - nil - end - end - - def template_default(template, escape_html:) - return nil unless present?(template) - return template unless template.match?(TOKEN_RE) - - ->(context) { interpolate(template, fetch_record(context), escape_html: escape_html) } - end - - # Returns the template content (with `{{record.X}}` tokens interpolated - # against the host record) when Template was just changed by the user; - # returns nil otherwise so the agent's set_watch_changes carries over the - # current Message input. 'No template' (or any unknown title) yields ''. - def message_value(templates) - by_title = templates.to_h { |t| [t[:title], t[:content].to_s] } - lambda do |context| - return nil unless context.field_changed?('Template') - - title = context.get_form_value('Template') - return '' if title == NO_TEMPLATE - - content = by_title[title].to_s - return content unless content.match?(TOKEN_RE) - - interpolate(content, fetch_record(context), escape_html: true) - end - end - - def fetch_record(context) - context.get_record([]) || {} - rescue StandardError => e - ForestAdminDatasourceZendesk.logger.warn( - "[forest_admin_datasource_zendesk] failed to fetch record for token interpolation: #{e.class}: #{e.message}" - ) - {} - end - - # Message ships as html_body; an unescaped `<` or `&` from a record value - # would break the outbound email or smuggle markup into it. - def interpolate(template, record, escape_html:) - template.gsub(TOKEN_RE) do - key = ::Regexp.last_match(1) - value = record[key] || record[key.to_sym] - next '' if value.nil? - - escape_html ? CGI.escapeHTML(value.to_s) : value.to_s - end - end - - def success_message(ticket_id, values, writeback = :skipped) - base = base_success_message(ticket_id, values) - return base unless writeback.is_a?(Array) && writeback.first == :failed - - "#{base} (warning: could not store the ticket id on the record: #{writeback.last})" - end - - def base_success_message(ticket_id, values) - if truthy?(values['Send as internal note']) - ticket_id ? "Ticket ##{ticket_id} created (internal note, no email)." : 'Ticket created (internal note).' - else - ticket_id ? "Ticket ##{ticket_id} created and requester notified." : 'Ticket created and requester notified.' - end - end - - def truthy?(value) - value == true || value.to_s.casecmp('true').zero? - end - - def present?(value) - !value.nil? && value.to_s != '' - end - end - end -end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb index a4d49b87d..08f4b54d5 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb @@ -11,21 +11,6 @@ class BaseCollection < ForestAdminDatasourceToolkit::Collection DATE_OPS = [Operators::EQUAL, Operators::BEFORE, Operators::AFTER, Operators::PRESENT, Operators::BLANK].freeze - # Pipe through an internal ActionCollectionDecorator so actions registered - # at the datasource level reuse the agent's form lifecycle for free. - def add_action(name, action) - super - action_runtime.add_action(name, action) - end - - def execute(caller, name, data, filter = nil) - action_runtime.execute(caller, name, data, filter) - end - - def get_form(caller, name, data = nil, filter = nil, metas = {}) - action_runtime.get_form(caller, name, data, filter, metas) - end - def aggregate(caller, filter, aggregation, _limit = nil) unless aggregation.operation == 'Count' && aggregation.field.nil? && aggregation.groups.empty? raise ForestAdminDatasourceToolkit::Exceptions::ForestException, @@ -106,12 +91,6 @@ def build_zendesk_query(caller, filter) private - def action_runtime - @action_runtime ||= ForestAdminDatasourceCustomizer::Decorators::Action::ActionCollectionDecorator.new( - self, datasource - ) - end - def sort_field_and_direction(entry) return [entry.field, entry.ascending] if entry.respond_to?(:field) diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb index 4475d1662..ac9d7e6a2 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket.rb @@ -8,10 +8,6 @@ class Ticket < BaseCollection ManyToOneSchema = ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema - ENUM_STATUS = %w[new open pending hold solved closed].freeze - ENUM_PRIORITY = %w[low normal high urgent].freeze - ENUM_TYPE = %w[problem incident question task].freeze - ZENDESK_SORTABLE = { 'updated_at' => 'updated_at', 'created_at' => 'created_at', diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket/schema_definition.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket/schema_definition.rb index b1a76d438..cb21ebb51 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket/schema_definition.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/ticket/schema_definition.rb @@ -18,11 +18,14 @@ def define_schema add_field('description', ColumnSchema.new(column_type: 'String', filter_operators: [], is_read_only: false, is_sortable: false)) add_field('status', ColumnSchema.new(column_type: 'Enum', filter_operators: STRING_OPS, - enum_values: ENUM_STATUS, is_read_only: false, is_sortable: true)) + enum_values: TicketEnums::STATUS, is_read_only: false, + is_sortable: true)) add_field('priority', ColumnSchema.new(column_type: 'Enum', filter_operators: STRING_OPS, - enum_values: ENUM_PRIORITY, is_read_only: false, is_sortable: true)) + enum_values: TicketEnums::PRIORITY, is_read_only: false, + is_sortable: true)) add_field('ticket_type', ColumnSchema.new(column_type: 'Enum', filter_operators: STRING_OPS, - enum_values: ENUM_TYPE, is_read_only: false, is_sortable: true)) + enum_values: TicketEnums::TYPE, is_read_only: false, + is_sortable: true)) add_field('requester_id', ColumnSchema.new(column_type: 'Number', filter_operators: NUMBER_OPS, is_read_only: false, is_sortable: true)) add_field('assignee_id', ColumnSchema.new(column_type: 'Number', filter_operators: NUMBER_OPS, diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb index a6c8a6d95..5b6da151b 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket.rb @@ -1,9 +1,28 @@ module ForestAdminDatasourceZendesk module Plugins - # Registers "Mark as solved" / "Mark as closed" smart actions on an - # arbitrary host collection. The host record(s) must carry a column - # (`ticket_id_field`) holding the Zendesk ticket id to close. + # The Zendesk ticket id is read from a configurable column on the host + # record(s); Zendesk sometimes rejects the direct `open -> closed` + # transition so failures are surfaced per-id rather than retried. class CloseTicket < ForestAdminDatasourceCustomizer::Plugins::Plugin + BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction + ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope + + STATUSES = %w[solved closed].freeze + SCOPE_KEYS = %i[single bulk].freeze + + # Zendesk refuses any update on a closed ticket with this exact + # wording — detected so we can swap the raw stack for a clean message. + ALREADY_CLOSED_PATTERN = 'closed prevents ticket update'.freeze + + NAMES = { + 'solved' => { single: 'Mark Zendesk ticket as solved', + bulk: 'Mark selected Zendesk tickets as solved' }.freeze, + 'closed' => { single: 'Mark Zendesk ticket as closed', + bulk: 'Mark selected Zendesk tickets as closed' }.freeze + }.freeze + + SCOPES = { single: ActionScope::SINGLE, bulk: ActionScope::BULK }.freeze + def run(_datasource_customizer, collection_customizer = nil, options = {}) datasource = options[:datasource] ticket_id_field = options[:ticket_id_field] @@ -11,11 +30,103 @@ def run(_datasource_customizer, collection_customizer = nil, options = {}) raise ArgumentError, 'CloseTicket plugin requires :ticket_id_field' unless ticket_id_field raise ArgumentError, 'CloseTicket plugin requires a collection' unless collection_customizer - Actions::CloseTicket.register_on( - collection_customizer, datasource, - ticket_id_field: ticket_id_field, - statuses: options[:statuses] + statuses = normalize_statuses(options[:statuses]) + scopes = normalize_scopes(options[:scopes]) + + variants(statuses, scopes).each do |name, status, scope| + collection_customizer.add_action(name, build_action(datasource, status, scope, ticket_id_field)) + end + end + + private + + def normalize_statuses(value) + list = Array(value).map(&:to_s).uniq + list = STATUSES if list.empty? + unknown = list - STATUSES + return list if unknown.empty? + + raise ForestAdminDatasourceToolkit::Exceptions::ForestException, + "Unknown CloseTicket statuses: #{unknown.join(", ")}. Allowed: #{STATUSES.join(", ")}." + end + + def normalize_scopes(value) + list = Array(value).map(&:to_sym).uniq + list = SCOPE_KEYS if list.empty? + unknown = list - SCOPE_KEYS + return list if unknown.empty? + + raise ForestAdminDatasourceToolkit::Exceptions::ForestException, + "Unknown CloseTicket scopes: #{unknown.join(", ")}. Allowed: #{SCOPE_KEYS.join(", ")}." + end + + def variants(statuses, scopes) + statuses.flat_map do |status| + scopes.map { |scope_key| [NAMES[status][scope_key], status, SCOPES[scope_key]] } + end + end + + def build_action(datasource, status, scope, ticket_id_field) + BaseAction.new(scope: scope, &executor(datasource, status, ticket_id_field)) + end + + def executor(datasource, status, ticket_id_field) + lambda do |context, result_builder| + ids = resolve_ticket_ids(context, ticket_id_field) + next result_builder.error(message: "No Zendesk ticket id found in '#{ticket_id_field}'.") if ids.empty? + + succeeded, already_closed, failed = apply_status(datasource, ids, status) + + # Closed tickets can't be reopened to 'solved'; fold into failures. + if status == 'solved' + failed += already_closed.map { |id| [id, 'ticket is already closed (cannot reopen to mark as solved)'] } + already_closed = [] + end + + if succeeded.empty? && already_closed.empty? + result_builder.error(message: Messages.error(failed, status)) + else + result_builder.success(message: Messages.success(succeeded, already_closed, failed, status)) + end + end + end + + def resolve_ticket_ids(context, ticket_id_field) + records = context.get_records([ticket_id_field]) + records = [records].compact unless records.is_a?(Array) + records.filter_map { |r| r[ticket_id_field] || r[ticket_id_field.to_sym] } + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to resolve ticket ids from '#{ticket_id_field}': " \ + "#{e.class}: #{e.message}" ) + [] + end + + # Per-id rescue so a single transition rejection doesn't abort bulk. + def apply_status(datasource, ids, status) + succeeded = [] + already_closed = [] + failed = [] + ids.each do |id| + datasource.client.update_ticket(id, 'status' => status) + succeeded << id + rescue StandardError => e + if already_closed?(e) + already_closed << id + else + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to set ticket ##{id} to '#{status}': " \ + "#{e.class}: #{e.message}" + ) + failed << [id, "#{e.class}: #{e.message}"] + end + end + [succeeded, already_closed, failed] + end + + def already_closed?(error) + error.message.to_s.include?(ALREADY_CLOSED_PATTERN) end end end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket/messages.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket/messages.rb new file mode 100644 index 000000000..9b777225a --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/close_ticket/messages.rb @@ -0,0 +1,41 @@ +module ForestAdminDatasourceZendesk + module Plugins + class CloseTicket + module Messages + module_function + + def success(succeeded, already_closed, failed, status) + [succeeded_phrase(succeeded, status), already_closed_phrase(already_closed), + failed_phrase(failed)].compact.join(' ') + end + + def error(failed, status) + verb = status == 'closed' ? 'close' : 'mark as solved' + return "Failed to #{verb} ticket ##{failed.first.first}: #{failed.first.last}" if failed.size == 1 + + "Failed to #{verb} all #{failed.size} tickets. First error: #{failed.first.last}" + end + + def succeeded_phrase(succeeded, status) + return nil if succeeded.empty? + + verb = status == 'closed' ? 'closed' : 'marked as solved' + succeeded.size == 1 ? "Ticket ##{succeeded.first} #{verb}." : "#{succeeded.size} tickets #{verb}." + end + + def already_closed_phrase(already_closed) + return nil if already_closed.empty? + return "Ticket ##{already_closed.first} was already closed." if already_closed.size == 1 + + "#{already_closed.size} tickets were already closed: #{already_closed.join(", ")}." + end + + def failed_phrase(failed) + return nil if failed.empty? + + "#{failed.size} failed: #{failed.map(&:first).join(", ")}." + end + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb index ae928e691..b65b0b8f8 100644 --- a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification.rb @@ -1,18 +1,106 @@ module ForestAdminDatasourceZendesk module Plugins - # Registers the "Create ticket and notify" smart action on an arbitrary - # host collection. The Zendesk datasource instance must be passed in via - # options so the action can reach the client; everything else mirrors - # Actions::CreateTicketWithNotification.register_on. + # Zendesk creates the requester user on the fly from the form's email, + # so the action can be registered on any host collection — no relation + # to Zendesk needed. class CreateTicketWithNotification < ForestAdminDatasourceCustomizer::Plugins::Plugin + BaseAction = ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction + ActionScope = ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope + + NAME = 'Create ticket and notify'.freeze + def run(_datasource_customizer, collection_customizer = nil, options = {}) datasource = options[:datasource] raise ArgumentError, 'CreateTicketWithNotification plugin requires :datasource' unless datasource raise ArgumentError, 'CreateTicketWithNotification plugin requires a collection' unless collection_customizer - Actions::CreateTicketWithNotification.register_on( - collection_customizer, datasource, **options.except(:datasource) + opts = options.except(:datasource) + opts[:email_templates] = Array(opts[:email_templates]).compact + collection_customizer.add_action(opts[:action_name] || NAME, build_action(datasource, opts)) + end + + private + + def build_action(datasource, opts) + BaseAction.new(scope: ActionScope::SINGLE, form: FormBuilder.build(opts), &executor(datasource, opts)) + end + + def executor(datasource, opts) + lambda do |context, result_builder| + values = context.form_values + email = values['Requester email'] + next result_builder.error(message: 'Requester email is required.') unless present?(email) + + payload = build_payload(values, email, opts) + ticket = datasource.client.create_ticket(payload) + ticket_id = ticket.respond_to?(:[]) ? ticket['id'] : nil + + writeback = write_back_ticket_id(context, opts[:ticket_id_field], ticket_id) + result_builder.success(message: success_message(ticket_id, values, writeback)) + end + end + + def build_payload(values, email, opts) + internal_note = truthy?(values['Send as internal note']) + payload = { + # Zendesk's create-user-on-the-fly requires a non-empty `name`; + # derive from the email's local-part. Ignored if the user exists. + 'requester' => { 'email' => email, 'name' => derive_requester_name(email) }, + 'subject' => values['Subject'], + 'comment' => { 'html_body' => values['Message'], 'public' => !internal_note } + } + priority = present?(opts[:priority_override]) ? opts[:priority_override] : values['Priority'] + type = present?(opts[:type_override]) ? opts[:type_override] : values['Type'] + payload['priority'] = priority if present?(priority) + payload['type'] = type if present?(type) + # Zendesk's `recipient` = the support address replies come FROM. + payload['recipient'] = opts[:sender_email] if present?(opts[:sender_email]) + payload + end + + def derive_requester_name(email) + local = email.to_s.split('@').first.to_s + local.empty? ? email.to_s : local + end + + # Best-effort: a writeback failure mustn't roll back the Zendesk ticket. + def write_back_ticket_id(context, field, ticket_id) + return :skipped if field.nil? || ticket_id.nil? + + context.collection.update(context.filter, { field => ticket_id }) + :ok + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to store ticket id in '#{field}': #{e.class}: #{e.message}" ) + [:failed, "#{e.class}: #{e.message}"] + end + + def success_message(ticket_id, values, writeback = :skipped) + base = base_success_message(ticket_id, values) + return base unless writeback.is_a?(Array) && writeback.first == :failed + + "#{base} (warning: could not store the ticket id on the record: #{writeback.last})" + end + + def base_success_message(ticket_id, values) + if truthy?(values['Send as internal note']) + return 'Ticket created (internal note).' unless ticket_id + + "Ticket ##{ticket_id} created (internal note, no email)." + else + return 'Ticket created and requester notified.' unless ticket_id + + "Ticket ##{ticket_id} created and requester notified." + end + end + + def truthy?(value) + value == true || value.to_s.casecmp('true').zero? + end + + def present?(value) + !value.nil? && value.to_s != '' end end end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification/form_builder.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification/form_builder.rb new file mode 100644 index 000000000..527ef985a --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification/form_builder.rb @@ -0,0 +1,149 @@ +require 'cgi' + +module ForestAdminDatasourceZendesk + module Plugins + class CreateTicketWithNotification + module FormBuilder + FieldType = ForestAdminDatasourceCustomizer::Decorators::Action::Types::FieldType + + NO_TEMPLATE = 'No template'.freeze + TOKEN_RE = /\{\{\s*record\.([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}/ + + module_function + + # ActionCollectionDecorator rejects forms that mix Page elements with + # non-Page elements, so each mode (flat / wizard) stays homogeneous. + def build(opts) + body = body_fields(opts) + return body if opts[:email_templates].empty? + + [ + { type: 'Layout', component: 'Page', next_button_label: 'Continue', + elements: [template_field(opts[:email_templates])] }, + { type: 'Layout', component: 'Page', previous_button_label: 'Back', + elements: body } + ] + end + + def body_fields(opts) + fields = [requester_field(opts[:requester_email_default]), + subject_field(opts[:default_subject]), + message_field(opts[:default_message], opts[:email_templates])] + fields << priority_field unless present?(opts[:priority_override]) + fields << type_field unless present?(opts[:type_override]) + fields << internal_note_field + fields + end + + def requester_field(default) + { type: FieldType::STRING, label: 'Requester email', is_required: true, + description: 'Email of the Zendesk requester. Pre-filled from the selected record when available.', + default_value: requester_default(default) } + end + + def template_field(templates) + { type: FieldType::ENUM, label: 'Template', is_required: true, + enum_values: [NO_TEMPLATE] + templates.map { |t| t[:title] }, + default_value: NO_TEMPLATE, + description: 'Pick a template to pre-fill the Message on the next page.' } + end + + def subject_field(default_subject) + { type: FieldType::STRING, label: 'Subject', is_required: true, + default_value: template_default(default_subject, escape_html: false) } + end + + def message_field(default_message, templates) + field = { type: FieldType::STRING, label: 'Message', widget: 'RichText', is_required: true, + description: 'Sent as the ticket\'s first comment (HTML). Public comments trigger the ' \ + 'default Zendesk notification email to the requester.' } + return field.merge(default_value: template_default(default_message, escape_html: true)) if templates.empty? + + # `value:` (not `default_value:`) — drop_default runs once (data + # key sticks after the first render); drop_deferred re-evaluates + # on every fetch, so Template changes re-fire the message proc. + field.merge(value: message_value(templates)) + end + + def priority_field + { type: FieldType::ENUM, label: 'Priority', + enum_values: TicketEnums::PRIORITY, default_value: 'normal' } + end + + def type_field + { type: FieldType::ENUM, label: 'Type', enum_values: TicketEnums::TYPE } + end + + def internal_note_field + { type: FieldType::BOOLEAN, label: 'Send as internal note', + description: 'When checked, the first comment is private and no email is sent to the requester.', + default_value: false } + end + + def requester_default(value) + return nil if value.nil? + return value if value.is_a?(String) + + lambda do |context| + record = fetch_record(context) + record.empty? ? nil : value.call(record) + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] requester_email_default resolver raised: #{e.class}: #{e.message}" + ) + nil + end + end + + def template_default(template, escape_html:) + return nil unless present?(template) + return template unless template.match?(TOKEN_RE) + + ->(context) { interpolate(template, fetch_record(context), escape_html: escape_html) } + end + + # Returns nil unless Template was just changed, so set_watch_changes + # carries over the user's current Message edits between renders. + def message_value(templates) + by_title = templates.to_h { |t| [t[:title], t[:content].to_s] } + lambda do |context| + return nil unless context.field_changed?('Template') + + title = context.get_form_value('Template') + return '' if title == NO_TEMPLATE + + content = by_title[title].to_s + return content unless content.match?(TOKEN_RE) + + interpolate(content, fetch_record(context), escape_html: true) + end + end + + def fetch_record(context) + context.get_record([]) || {} + rescue StandardError => e + ForestAdminDatasourceZendesk.logger.warn( + "[forest_admin_datasource_zendesk] failed to fetch record for token interpolation: #{e.class}: #{e.message}" + ) + {} + end + + # Message ships as html_body — unescaped `<` or `&` from a record + # value would break the outbound email or smuggle markup into it. + def interpolate(template, record, escape_html:) + template.gsub(TOKEN_RE) do + key = ::Regexp.last_match(1) + value = record[key] || record[key.to_sym] + next '' if value.nil? + + escape_html ? CGI.escapeHTML(value.to_s) : value.to_s + end + end + + def present?(value) + !value.nil? && value.to_s != '' + end + end + end + end +end diff --git a/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/ticket_enums.rb b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/ticket_enums.rb new file mode 100644 index 000000000..f34776664 --- /dev/null +++ b/packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/ticket_enums.rb @@ -0,0 +1,8 @@ +module ForestAdminDatasourceZendesk + # Shared between the Ticket schema and plugins that build ticket forms. + module TicketEnums + STATUS = %w[new open pending hold solved closed].freeze + PRIORITY = %w[low normal high urgent].freeze + TYPE = %w[problem incident question task].freeze + end +end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb deleted file mode 100644 index d6bca3faa..000000000 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/close_ticket_spec.rb +++ /dev/null @@ -1,180 +0,0 @@ -module ForestAdminDatasourceZendesk - # Stand-in for an action context. The executor only reads - # `get_records(fields)` so we don't need the full ActionContext class here. - class FakeCloseContext - def initialize(records: []) - @records = records - end - - def get_records(_fields = []) - @records - end - end - - RSpec.describe Actions::CloseTicket do - let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } - let(:datasource) do - instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}) - end - let(:result_builder) { ForestAdminDatasourceCustomizer::Decorators::Action::ResultBuilder.new } - let(:action_scope) { ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope } - let(:ticket_id_field) { 'last_zendesk_ticket_id' } - - describe '.variants' do - it 'yields all four variants when both statuses are requested' do - variants = described_class.variants(%w[solved closed]) - expect(variants.map { |name, _status, _scope| name }).to contain_exactly( - 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved', - 'Mark Zendesk ticket as closed', 'Mark selected Zendesk tickets as closed' - ) - - scopes = variants.each_with_object({}) { |(name, _status, scope), h| h[name] = scope } - expect(scopes['Mark Zendesk ticket as solved']).to eq(action_scope::SINGLE) - expect(scopes['Mark selected Zendesk tickets as solved']).to eq(action_scope::BULK) - expect(scopes['Mark Zendesk ticket as closed']).to eq(action_scope::SINGLE) - expect(scopes['Mark selected Zendesk tickets as closed']).to eq(action_scope::BULK) - end - - it 'yields only the requested status variants (subset filtering)' do - names = described_class.variants(%w[solved]).map(&:first) - expect(names).to contain_exactly('Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved') - end - - it 'yields nothing for an empty list' do - expect(described_class.variants([])).to be_empty - end - - it 'raises a ForestException on an unknown status' do - expect { described_class.variants(%w[solved unknown]) } - .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ForestException, /Unknown.*unknown/) - end - end - - describe 'executor' do - let(:executor) { described_class.executor(datasource, 'solved', ticket_id_field) } - - it 'reads the ticket id from the host field and PUTs status=solved' do - allow(client).to receive(:update_ticket) - context = FakeCloseContext.new(records: [{ ticket_id_field => 42 }]) - - result = executor.call(context, result_builder) - - expect(client).to have_received(:update_ticket).with(42, 'status' => 'solved') - expect(result[:type]).to eq('Success') - expect(result[:message]).to include('Ticket #42', 'marked as solved') - end - - it 'PUTs status=closed for every host record when run in bulk' do - allow(client).to receive(:update_ticket) - bulk_executor = described_class.executor(datasource, 'closed', ticket_id_field) - bulk_context = FakeCloseContext.new( - records: [7, 8, 9].map { |id| { ticket_id_field => id } } - ) - - result = bulk_executor.call(bulk_context, result_builder) - - [7, 8, 9].each { |id| expect(client).to have_received(:update_ticket).with(id, 'status' => 'closed') } - expect(result[:message]).to include('3 tickets closed') - end - - it 'returns an error when no host record carries a ticket id' do - allow(client).to receive(:update_ticket) - context = FakeCloseContext.new(records: [{ ticket_id_field => nil }]) - - result = executor.call(context, result_builder) - - expect(client).not_to have_received(:update_ticket) - expect(result[:type]).to eq('Error') - expect(result[:message]).to include(ticket_id_field) - end - - it 'works with symbol keys on the host record' do - allow(client).to receive(:update_ticket) - context = FakeCloseContext.new(records: [{ ticket_id_field.to_sym => 99 }]) - - executor.call(context, result_builder) - - expect(client).to have_received(:update_ticket).with(99, 'status' => 'solved') - end - - context 'when Zendesk rejects some ids (partial success on bulk)' do - let(:bulk_executor) { described_class.executor(datasource, 'closed', ticket_id_field) } - let(:bulk_context) do - FakeCloseContext.new(records: [7, 8, 9].map { |id| { ticket_id_field => id } }) - end - - it 'continues with the remaining ids and surfaces the failures in the message' do - allow(client).to receive(:update_ticket) - .with(8, anything).and_raise(StandardError, 'cannot transition open to closed') - allow(client).to receive(:update_ticket).with(7, anything) - allow(client).to receive(:update_ticket).with(9, anything) - allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) - - result = bulk_executor.call(bulk_context, result_builder) - - expect(result[:type]).to eq('Success') - expect(result[:message]).to include('2 tickets closed', '1 failed', '8') - expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) - .with(a_string_including('#8', 'cannot transition')) - end - - it 'returns an Error when every id fails' do - allow(client).to receive(:update_ticket).and_raise(StandardError, 'permission denied') - allow(ForestAdminDatasourceZendesk.logger).to receive(:warn).exactly(3).times - - result = bulk_executor.call(bulk_context, result_builder) - - expect(result[:type]).to eq('Error') - expect(result[:message]).to include('Failed to close', '3 tickets', 'permission denied') - end - end - - context 'when get_records itself raises' do - it 'logs and returns an Error message without calling the client' do - context = instance_double( - ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle - ) - allow(context).to receive(:get_records).and_raise(StandardError, 'boom') - allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) - allow(client).to receive(:update_ticket) - - result = executor.call(context, result_builder) - - expect(client).not_to have_received(:update_ticket) - expect(result[:type]).to eq('Error') - expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) - .with(a_string_including(ticket_id_field, 'boom')) - end - end - end - - describe '.register_on' do - let(:host_collection) do - Class.new do - attr_reader :registered - - def initialize = @registered = {} - def add_action(name, action) = @registered[name] = action - end.new - end - - it 'registers the four variants on an arbitrary host collection' do - described_class.register_on(host_collection, datasource, ticket_id_field: ticket_id_field) - - expect(host_collection.registered.keys).to contain_exactly( - 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved', - 'Mark Zendesk ticket as closed', 'Mark selected Zendesk tickets as closed' - ) - end - - it 'registers only the subset specified via :statuses' do - described_class.register_on(host_collection, datasource, - ticket_id_field: ticket_id_field, statuses: %w[solved]) - - expect(host_collection.registered.keys).to contain_exactly( - 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved' - ) - end - end - end -end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb deleted file mode 100644 index 45ed96afc..000000000 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/actions/create_ticket_with_notification_spec.rb +++ /dev/null @@ -1,498 +0,0 @@ -module ForestAdminDatasourceZendesk - # Tiny PORO standing in for an ActionContextSingle in unit tests. We can't - # use Struct here because `Struct` mixes in Enumerable, which already defines - # `#filter` — having a `:filter` member triggers Lint/StructNewOverride and - # is genuinely ambiguous. - class FakeActionContext - attr_reader :form_values, :collection, :filter, :record_id - - def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) - @form_values = form_values - @collection = collection - @filter = filter - @record_id = record_id - end - end - - RSpec.describe Actions::CreateTicketWithNotification do - let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } - let(:datasource) do - instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}) - end - - let(:context) { Struct.new(:form_values).new(form_values) } - let(:result_builder) { ForestAdminDatasourceCustomizer::Decorators::Action::ResultBuilder.new } - let(:executor) { described_class.executor(datasource) } - - describe '.build' do - it 'returns a SINGLE-scoped action with the documented form fields' do - action = described_class.build(datasource) - - expect(action.scope).to eq(ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::SINGLE) - labels = action.form.map { |f| f[:label] } - expect(labels).to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) - end - - it 'uses RichText for the Message widget' do - message_field = described_class.build(datasource).form.find { |f| f[:label] == 'Message' } - expect(message_field[:widget]).to eq('RichText') - end - - it 'marks Requester email as required' do - field = described_class.build(datasource).form.first - expect(field[:label]).to eq('Requester email') - expect(field[:is_required]).to be(true) - end - end - - describe 'requester_email_default' do - let(:context_double) do - instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - get_record: { 'id' => 42, 'email' => 'alice@x.com', 'name' => 'Alice' }) - end - - it 'leaves the field empty (returns nil) when no default is configured' do - expect(described_class.build(datasource).form.first[:default_value]).to be_nil - end - - context 'when given a literal email String' do - it 'uses it verbatim as a static default (no record lookup)' do - action = described_class.build(datasource, requester_email_default: 'support@example.com') - expect(action.form.first[:default_value]).to eq('support@example.com') - end - end - - context 'when given a Proc resolver' do - let(:resolver) { ->(record) { record['email'] } } - - it 'pre-fills the email field from the selected record via the resolver' do - action = described_class.build(datasource, requester_email_default: resolver) - requester_proc = action.form.first[:default_value] - - expect(requester_proc.call(context_double)).to eq('alice@x.com') - end - - it 'survives a record lookup that raises (returns nil) and logs the cause' do - allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') - allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) - action = described_class.build(datasource, requester_email_default: resolver) - - expect(action.form.first[:default_value].call(context_double)).to be_nil - expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) - .with(a_string_including('fetch record', 'StandardError', 'boom')) - end - - it 'logs and returns nil when the resolver proc itself raises' do - allow(context_double).to receive(:get_record).and_return({ 'email' => 'a@b.com' }) - allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) - broken_resolver = ->(_record) { raise StandardError, 'typo in lambda' } - action = described_class.build(datasource, requester_email_default: broken_resolver) - - expect(action.form.first[:default_value].call(context_double)).to be_nil - expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) - .with(a_string_including('requester_email_default', 'typo in lambda')) - end - end - end - - describe 'subject / message defaults' do - it 'uses configured static defaults verbatim' do - action = described_class.build(datasource, - default_subject: 'Welcome', - default_message: '

Hi

') - expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') - expect(action.form.find { |f| f[:label] == 'Message' }[:default_value]).to eq('

Hi

') - end - - context 'with {{record.}} tokens' do - let(:context_double) do - instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - get_record: { 'id' => 42, 'name' => 'Alice', 'email' => 'alice@x.com' }) - end - - it 'substitutes record fields at form-open time' do - action = described_class.build(datasource, - default_subject: 'Follow-up for {{record.name}}', - default_message: '

Hello {{record.name}} ({{record.email}})

') - - subject_proc = action.form.find { |f| f[:label] == 'Subject' }[:default_value] - message_proc = action.form.find { |f| f[:label] == 'Message' }[:default_value] - - expect(subject_proc.call(context_double)).to eq('Follow-up for Alice') - expect(message_proc.call(context_double)).to eq('

Hello Alice (alice@x.com)

') - end - - it 'falls back to empty strings when the record lookup fails (logged)' do - allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') - allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) - action = described_class.build(datasource, default_subject: 'Hi {{record.name}}') - - expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value].call(context_double)).to eq('Hi ') - expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) - .with(a_string_including('fetch record', 'boom')) - end - end - end - - describe 'HTML escaping on the Message template' do - let(:context_double) do - instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - get_record: { 'name' => "O'Brien ", 'note' => '' }) - end - - it 'escapes interpolated values in the Message (RichText -> html_body)' do - # Otherwise a record value containing `<` or `&` would break the - # outbound HTML or smuggle markup into the requester email. - action = described_class.build(datasource, - default_message: '

Hi {{record.name}} - {{record.note}}

') - message_proc = action.form.find { |f| f[:label] == 'Message' }[:default_value] - - expect(message_proc.call(context_double)).to eq( - '

Hi O'Brien <admin> - <script>alert(1)</script>

' - ) - end - - it 'leaves Subject interpolation unescaped (plain-text field)' do - action = described_class.build(datasource, default_subject: 'Re: {{record.name}}') - subject_proc = action.form.find { |f| f[:label] == 'Subject' }[:default_value] - - expect(subject_proc.call(context_double)).to eq("Re: O'Brien ") - end - end - - describe 'executor' do - context 'with a public html comment (default)' do - let(:form_values) do - { 'Requester email' => 'alice@x.com', 'Subject' => 'Refund', 'Message' => 'Hi there', - 'Priority' => 'high', 'Type' => 'question', 'Send as internal note' => false } - end - - it 'creates a ticket targeting the requester by email and embeds the html comment' do - allow(client).to receive(:create_ticket) do |payload| - expect(payload).to eq( - 'requester' => { 'email' => 'alice@x.com', 'name' => 'alice' }, - 'subject' => 'Refund', - 'comment' => { 'html_body' => 'Hi there', 'public' => true }, - 'priority' => 'high', - 'type' => 'question' - ) - { 'id' => 7 } - end - - result = executor.call(context, result_builder) - expect(client).to have_received(:create_ticket) - expect(result[:message]).to include('Ticket #7', 'notified') - end - end - - context 'with internal note' do - let(:form_values) do - { 'Requester email' => 'b@x.com', 'Subject' => 'Internal', 'Message' => 'For agents only', - 'Send as internal note' => true } - end - - it 'flips the comment to private and omits the notify wording' do - allow(client).to receive(:create_ticket).and_return('id' => 9) - - result = executor.call(context, result_builder) - expect(client).to have_received(:create_ticket).with(hash_including( - 'comment' => { 'html_body' => 'For agents only', - 'public' => false } - )) - expect(result[:message]).to include('internal note') - end - end - - context 'without a requester email' do - let(:form_values) { { 'Requester email' => nil, 'Subject' => 'S', 'Message' => 'M' } } - - it 'returns an error and does not call the client' do - allow(client).to receive(:create_ticket) - - result = executor.call(context, result_builder) - expect(result[:type]).to eq('Error') - expect(client).not_to have_received(:create_ticket) - end - end - - context 'with empty optional fields' do - let(:form_values) do - { 'Requester email' => 'c@x.com', 'Subject' => 'S', 'Message' => 'M', - 'Priority' => nil, 'Type' => '', 'Send as internal note' => nil } - end - - it 'omits empty optional keys from the payload' do - allow(client).to receive(:create_ticket) do |payload| - expect(payload.keys).not_to include('priority', 'type') - expect(payload['comment']['public']).to be(true) - { 'id' => 1 } - end - - executor.call(context, result_builder) - expect(client).to have_received(:create_ticket) - end - end - - context 'with ticket_id_field configured (writeback to host record)' do - let(:form_values) do - { 'Requester email' => 'd@x.com', 'Subject' => 'S', 'Message' => 'M', - 'Send as internal note' => false } - end - let(:host_collection) { instance_double('RelaxedCollection') } # rubocop:disable RSpec/VerifiedDoubleReference - let(:filter) { instance_double('Filter') } # rubocop:disable RSpec/VerifiedDoubleReference - let(:context) do - FakeActionContext.new(form_values: form_values, collection: host_collection, filter: filter) - end - let(:executor_with_writeback) do - described_class.executor(datasource, ticket_id_field: 'last_zendesk_ticket_id') - end - - before do - allow(client).to receive(:create_ticket).and_return('id' => 77) - end - - it 'updates the host record with the new ticket id under the configured field' do - allow(host_collection).to receive(:update) - - result = executor_with_writeback.call(context, result_builder) - - expect(host_collection).to have_received(:update).with(filter, { 'last_zendesk_ticket_id' => 77 }) - expect(result[:type]).to eq('Success') - expect(result[:message]).to include('Ticket #77') - expect(result[:message]).not_to include('warning') - end - - it 'logs and surfaces a warning when the host update fails but still succeeds' do - allow(host_collection).to receive(:update).and_raise(StandardError, 'field is read-only') - allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) - - result = executor_with_writeback.call(context, result_builder) - - expect(result[:type]).to eq('Success') - expect(result[:message]).to include('Ticket #77', 'warning', 'field is read-only') - expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) - .with(a_string_including('last_zendesk_ticket_id', 'field is read-only')) - end - - it 'does not attempt any update when ticket_id_field is nil' do - allow(host_collection).to receive(:update) - - executor.call(context, result_builder) # executor without ticket_id_field - - expect(host_collection).not_to have_received(:update) - end - end - end - - describe '.register_on' do - let(:host_collection) do - Class.new do - attr_reader :registered - - def initialize = @registered = {} - def add_action(name, action) = @registered[name] = action - end.new - end - - it 'attaches the action to an arbitrary collection-like target' do - described_class.register_on(host_collection, datasource, - default_subject: 'Welcome', - requester_email_default: ->(r) { r['mail'] }) - - expect(host_collection.registered).to have_key(described_class::NAME) - action = host_collection.registered[described_class::NAME] - expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') - end - - it 'threads ticket_id_field to the executor (writeback enabled at registration)' do - described_class.register_on(host_collection, datasource, ticket_id_field: 'zd_id') - - relax = instance_double('RelaxedCollection') # rubocop:disable RSpec/VerifiedDoubleReference - filter = instance_double('Filter') # rubocop:disable RSpec/VerifiedDoubleReference - allow(relax).to receive(:update) - allow(client).to receive(:create_ticket).and_return('id' => 5) - - ctx = FakeActionContext.new( - form_values: { 'Requester email' => 'a@b.com', 'Subject' => 'S', 'Message' => 'M' }, - collection: relax, filter: filter - ) - host_collection.registered[described_class::NAME].execute.call(ctx, result_builder) - - expect(relax).to have_received(:update).with(filter, { 'zd_id' => 5 }) - end - - it 'registers under the configured name when action_name is provided' do - described_class.register_on(host_collection, datasource) - described_class.register_on(host_collection, datasource, action_name: 'Custom label') - - expect(host_collection.registered.keys).to contain_exactly(described_class::NAME, 'Custom label') - end - end - - describe 'email_templates wizard' do - let(:templates) do - [{ title: 'Welcome', content: '

Welcome aboard!

' }, - { title: 'Refund', content: '

Refund processed.

' }] - end - - it 'flips the form into a two-page wizard (Template first, body second)' do - action = described_class.build(datasource, email_templates: templates) - - expect(action.form.size).to eq(2) - page_one, page_two = action.form - expect(page_one[:component]).to eq('Page') - expect(page_one[:elements].map { |f| f[:label] }).to eq(['Template']) - expect(page_two[:elements].map { |f| f[:label] }) - .to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) - end - - it 'lists No template + each template title in the dropdown' do - action = described_class.build(datasource, email_templates: templates) - template_field = action.form.first[:elements].first - expect(template_field[:enum_values]).to eq(['No template', 'Welcome', 'Refund']) - expect(template_field[:default_value]).to eq('No template') - end - - it 'pre-fills Message via `value:` when Template was the just-changed field' do - # `value:` (not `default_value:`) so drop_deferred re-evaluates the proc - # on every form fetch — drop_default's `data.key?` check would otherwise - # block re-evaluation after the first render. - action = described_class.build(datasource, email_templates: templates) - message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } - ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - field_changed?: true, get_form_value: 'Refund') - - expect(message_field[:value].call(ctx)).to eq('

Refund processed.

') - end - - it "yields an empty Message when 'No template' was just selected" do - action = described_class.build(datasource, email_templates: templates) - message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } - ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - field_changed?: true, get_form_value: 'No template') - - expect(message_field[:value].call(ctx)).to eq('') - end - - it 'returns nil (carry over current input) when another field triggered the re-fetch' do - action = described_class.build(datasource, email_templates: templates) - message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } - ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - field_changed?: false) - - expect(message_field[:value].call(ctx)).to be_nil - end - - it 'interpolates {{record.}} tokens inside the selected template content' do - templated = [{ title: 'Welcome', content: '

Hi {{record.name}} ({{record.email}})

' }] - action = described_class.build(datasource, email_templates: templated) - message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } - ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - field_changed?: true, get_form_value: 'Welcome', - get_record: { 'name' => 'Alice', 'email' => 'a@b.com' }) - - expect(message_field[:value].call(ctx)).to eq('

Hi Alice (a@b.com)

') - end - - it 'HTML-escapes interpolated record values inside template content' do - templated = [{ title: 'Bug', content: '

{{record.note}}

' }] - action = described_class.build(datasource, email_templates: templated) - message_field = action.form.last[:elements].find { |f| f[:label] == 'Message' } - ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, - field_changed?: true, get_form_value: 'Bug', - get_record: { 'note' => '' }) - - expect(message_field[:value].call(ctx)).to eq('

<script>x</script>

') - end - - it 'keeps the original flat form when no templates are configured' do - action = described_class.build(datasource, email_templates: []) - expect(action.form.first[:component]).to be_nil # not a Page - end - end - - describe 'priority_override / type_override' do - it 'omits the Priority field and forces the value in the payload' do - action = described_class.build(datasource, priority_override: 'urgent') - labels = action.form.map { |f| f[:label] } - expect(labels).not_to include('Priority') - - allow(client).to receive(:create_ticket).and_return('id' => 1) - described_class.executor(datasource, priority_override: 'urgent').call( - FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', - 'Subject' => 'S', 'Message' => 'M' }), - result_builder - ) - expect(client).to have_received(:create_ticket).with(hash_including('priority' => 'urgent')) - end - - it 'omits the Type field and forces the value in the payload' do - action = described_class.build(datasource, type_override: 'incident') - labels = action.form.map { |f| f[:label] } - expect(labels).not_to include('Type') - - allow(client).to receive(:create_ticket).and_return('id' => 1) - described_class.executor(datasource, type_override: 'incident').call( - FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', - 'Subject' => 'S', 'Message' => 'M' }), - result_builder - ) - expect(client).to have_received(:create_ticket).with(hash_including('type' => 'incident')) - end - - it 'forces the override even when the form value is also present' do - allow(client).to receive(:create_ticket).and_return('id' => 1) - described_class.executor(datasource, priority_override: 'urgent').call( - FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', 'Subject' => 'S', - 'Message' => 'M', 'Priority' => 'low' }), - result_builder - ) - expect(client).to have_received(:create_ticket).with(hash_including('priority' => 'urgent')) - end - end - - describe 'requester name auto-derivation' do - # Zendesk's "create user on the fly from an email" path requires a - # non-empty `name` on the requester. We derive it from the email's - # local-part so the create succeeds; Zendesk silently ignores the name - # when the email already maps to an existing user. - it 'sends the email local-part as requester.name in the payload' do - allow(client).to receive(:create_ticket).and_return('id' => 1) - described_class.executor(datasource).call( - FakeActionContext.new(form_values: { 'Requester email' => 'john.doe@acme.com', - 'Subject' => 'S', 'Message' => 'M' }), - result_builder - ) - expect(client).to have_received(:create_ticket).with(hash_including( - 'requester' => { 'email' => 'john.doe@acme.com', - 'name' => 'john.doe' } - )) - end - end - - describe 'sender_email' do - it 'maps to Zendesk `recipient` in the payload when configured' do - allow(client).to receive(:create_ticket).and_return('id' => 1) - described_class.executor(datasource, sender_email: 'support@acme.com').call( - FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', - 'Subject' => 'S', 'Message' => 'M' }), - result_builder - ) - expect(client).to have_received(:create_ticket).with(hash_including('recipient' => 'support@acme.com')) - end - - it 'omits recipient from the payload when sender_email is blank' do - allow(client).to receive(:create_ticket) do |payload| - expect(payload).not_to have_key('recipient') - { 'id' => 1 } - end - described_class.executor(datasource).call( - FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', - 'Subject' => 'S', 'Message' => 'M' }), - result_builder - ) - expect(client).to have_received(:create_ticket) - end - end - end -end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb index 73002ee42..8f9fc1c2c 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/close_ticket_spec.rb @@ -1,9 +1,24 @@ module ForestAdminDatasourceZendesk + # Stand-in for an action context. The executor only reads + # `get_records(fields)` so we don't need the full ActionContext class here. + class FakeCloseContext + def initialize(records: []) + @records = records + end + + def get_records(_fields = []) + @records + end + end + RSpec.describe Plugins::CloseTicket do let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } let(:datasource) do instance_double(ForestAdminDatasourceZendesk::Datasource, client: client, custom_field_mapping: {}) end + let(:result_builder) { ForestAdminDatasourceCustomizer::Decorators::Action::ResultBuilder.new } + let(:action_scope) { ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope } + let(:ticket_id_field) { 'last_zendesk_ticket_id' } let(:collection_customizer) do Class.new do attr_reader :registered @@ -13,40 +28,243 @@ def add_action(name, action) = @registered[name] = action end.new end - it 'registers both solved and closed variants by default' do + def register(opts = {}) described_class.new.run(nil, collection_customizer, - datasource: datasource, ticket_id_field: 'last_zendesk_ticket_id') - - expect(collection_customizer.registered.keys).to contain_exactly( - 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved', - 'Mark Zendesk ticket as closed', 'Mark selected Zendesk tickets as closed' - ) + { datasource: datasource, ticket_id_field: ticket_id_field }.merge(opts)) + collection_customizer.registered end - it 'honors :statuses to register a subset of variants' do - described_class.new.run(nil, collection_customizer, - datasource: datasource, ticket_id_field: 'last_zendesk_ticket_id', - statuses: %w[solved]) + describe '#run' do + it 'registers all four variants by default (solved/closed × single/bulk)' do + register - expect(collection_customizer.registered.keys).to contain_exactly( - 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved' - ) - end + expect(collection_customizer.registered.keys).to contain_exactly( + 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved', + 'Mark Zendesk ticket as closed', 'Mark selected Zendesk tickets as closed' + ) + end + + it 'honors :statuses to keep only the requested status family' do + register(statuses: %w[solved]) + expect(collection_customizer.registered.keys).to contain_exactly( + 'Mark Zendesk ticket as solved', 'Mark selected Zendesk tickets as solved' + ) + end + + it 'honors :scopes to keep only the requested scopes' do + register(scopes: %i[single]) + expect(collection_customizer.registered.keys).to contain_exactly( + 'Mark Zendesk ticket as solved', 'Mark Zendesk ticket as closed' + ) + end + + it 'composes both :statuses and :scopes to a Cartesian subset (one action)' do + register(statuses: %w[closed], scopes: %i[bulk]) + expect(collection_customizer.registered.keys).to contain_exactly('Mark selected Zendesk tickets as closed') + end + + it 'accepts symbol statuses and string scopes interchangeably' do + register(statuses: %i[solved], scopes: %w[bulk]) + expect(collection_customizer.registered.keys).to contain_exactly('Mark selected Zendesk tickets as solved') + end - it 'raises ArgumentError without :datasource' do - expect { described_class.new.run(nil, collection_customizer, ticket_id_field: 'id') } - .to raise_error(ArgumentError, /datasource/) + it 'binds the right ActionScope to each registered action' do + register + registered = collection_customizer.registered + expect(registered['Mark Zendesk ticket as solved'].scope).to eq(action_scope::SINGLE) + expect(registered['Mark selected Zendesk tickets as solved'].scope).to eq(action_scope::BULK) + expect(registered['Mark Zendesk ticket as closed'].scope).to eq(action_scope::SINGLE) + expect(registered['Mark selected Zendesk tickets as closed'].scope).to eq(action_scope::BULK) + end + + it 'raises a ForestException on an unknown status' do + expect { register(statuses: %w[solved unknown]) } + .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ForestException, /Unknown.*unknown/) + end + + it 'raises a ForestException on an unknown scope' do + expect { register(scopes: %i[single weird]) } + .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ForestException, /Unknown.*weird/) + end + + it 'raises ArgumentError without :datasource' do + expect { described_class.new.run(nil, collection_customizer, ticket_id_field: 'id') } + .to raise_error(ArgumentError, /datasource/) + end + + it 'raises ArgumentError without :ticket_id_field' do + expect { described_class.new.run(nil, collection_customizer, datasource: datasource) } + .to raise_error(ArgumentError, /ticket_id_field/) + end + + it 'raises ArgumentError without a collection_customizer' do + expect do + described_class.new.run(nil, nil, datasource: datasource, ticket_id_field: 'id') + end.to raise_error(ArgumentError, /collection/) + end end - it 'raises ArgumentError without :ticket_id_field' do - expect { described_class.new.run(nil, collection_customizer, datasource: datasource) } - .to raise_error(ArgumentError, /ticket_id_field/) + describe 'executor' do + let(:solved_single) do + register(statuses: %w[solved], scopes: %i[single])['Mark Zendesk ticket as solved'] + end + let(:closed_bulk) do + register(statuses: %w[closed], scopes: %i[bulk])['Mark selected Zendesk tickets as closed'] + end + + it 'reads the ticket id from the host field and PUTs status=solved' do + allow(client).to receive(:update_ticket) + context = FakeCloseContext.new(records: [{ ticket_id_field => 42 }]) + + result = solved_single.execute.call(context, result_builder) + + expect(client).to have_received(:update_ticket).with(42, 'status' => 'solved') + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('Ticket #42', 'marked as solved') + end + + it 'PUTs status=closed for every host record when run in bulk' do + allow(client).to receive(:update_ticket) + bulk_context = FakeCloseContext.new( + records: [7, 8, 9].map { |id| { ticket_id_field => id } } + ) + + result = closed_bulk.execute.call(bulk_context, result_builder) + + [7, 8, 9].each { |id| expect(client).to have_received(:update_ticket).with(id, 'status' => 'closed') } + expect(result[:message]).to include('3 tickets closed') + end + + it 'returns an error when no host record carries a ticket id' do + allow(client).to receive(:update_ticket) + context = FakeCloseContext.new(records: [{ ticket_id_field => nil }]) + + result = solved_single.execute.call(context, result_builder) + + expect(client).not_to have_received(:update_ticket) + expect(result[:type]).to eq('Error') + expect(result[:message]).to include(ticket_id_field) + end + + it 'works with symbol keys on the host record' do + allow(client).to receive(:update_ticket) + context = FakeCloseContext.new(records: [{ ticket_id_field.to_sym => 99 }]) + + solved_single.execute.call(context, result_builder) + + expect(client).to have_received(:update_ticket).with(99, 'status' => 'solved') + end + + context 'when Zendesk rejects some ids (partial success on bulk)' do + let(:bulk_context) do + FakeCloseContext.new(records: [7, 8, 9].map { |id| { ticket_id_field => id } }) + end + + it 'continues with the remaining ids and surfaces the failures in the message' do + allow(client).to receive(:update_ticket) + .with(8, anything).and_raise(StandardError, 'cannot transition open to closed') + allow(client).to receive(:update_ticket).with(7, anything) + allow(client).to receive(:update_ticket).with(9, anything) + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + + result = closed_bulk.execute.call(bulk_context, result_builder) + + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('2 tickets closed', '1 failed', '8') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('#8', 'cannot transition')) + end + + it 'returns an Error when every id fails' do + allow(client).to receive(:update_ticket).and_raise(StandardError, 'permission denied') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn).exactly(3).times + + result = closed_bulk.execute.call(bulk_context, result_builder) + + expect(result[:type]).to eq('Error') + expect(result[:message]).to include('Failed to close', '3 tickets', 'permission denied') + end + end + + context 'when get_records itself raises' do + it 'logs and returns an Error message without calling the client' do + context = instance_double( + ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle + ) + allow(context).to receive(:get_records).and_raise(StandardError, 'boom') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + allow(client).to receive(:update_ticket) + + result = solved_single.execute.call(context, result_builder) + + expect(client).not_to have_received(:update_ticket) + expect(result[:type]).to eq('Error') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including(ticket_id_field, 'boom')) + end + end end - it 'raises ArgumentError without a collection_customizer' do - expect do - described_class.new.run(nil, nil, datasource: datasource, ticket_id_field: 'id') - end.to raise_error(ArgumentError, /collection/) + describe "Zendesk's 'closed prevents ticket update' error" do + let(:already_closed_error) do + StandardError.new( + 'Zendesk API call failed: update(tickets/254): ZendeskAPI::Error::RecordInvalid: ' \ + '{"status" => [{"description" => "closed prevents ticket update"}]}' + ) + end + let(:solved_single) do + register(statuses: %w[solved], scopes: %i[single])['Mark Zendesk ticket as solved'] + end + let(:closed_single) do + register(statuses: %w[closed], scopes: %i[single])['Mark Zendesk ticket as closed'] + end + let(:closed_bulk) do + register(statuses: %w[closed], scopes: %i[bulk])['Mark selected Zendesk tickets as closed'] + end + + it "with status='closed' on an already-closed ticket: clean Success ('was already closed')" do + allow(client).to receive(:update_ticket).and_raise(already_closed_error) + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + context = FakeCloseContext.new(records: [{ ticket_id_field => 254 }]) + + result = closed_single.execute.call(context, result_builder) + + expect(result[:type]).to eq('Success') + expect(result[:message]).to eq('Ticket #254 was already closed.') + # No warn log: an idempotent "already closed" is an expected state. + expect(ForestAdminDatasourceZendesk.logger).not_to have_received(:warn) + end + + it "with status='closed' in bulk: mixes succeeded + already-closed + other failures cleanly" do + allow(client).to receive(:update_ticket).with(7, anything) + allow(client).to receive(:update_ticket).with(8, anything).and_raise(already_closed_error) + allow(client).to receive(:update_ticket).with(9, anything).and_raise(StandardError, 'permission denied') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + context = FakeCloseContext.new(records: [7, 8, 9].map { |id| { ticket_id_field => id } }) + + result = closed_bulk.execute.call(context, result_builder) + + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('Ticket #7 closed.', 'Ticket #8 was already closed.', '1 failed: 9') + # Only the genuine failure is logged; "already closed" stays quiet. + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('#9', 'permission denied')) + expect(ForestAdminDatasourceZendesk.logger).not_to have_received(:warn) + .with(a_string_including('#8')) + end + + it "with status='solved' on an already-closed ticket: Error explaining it can't be reopened" do + allow(client).to receive(:update_ticket).and_raise(already_closed_error) + context = FakeCloseContext.new(records: [{ ticket_id_field => 254 }]) + + result = solved_single.execute.call(context, result_builder) + + expect(result[:type]).to eq('Error') + expect(result[:message]).to include('#254', 'already closed', 'cannot reopen') + # Make sure the raw API stack is gone. + expect(result[:message]).not_to include('RecordInvalid') + expect(result[:message]).not_to include('"description"') + end end end end diff --git a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb index 314bbec35..3399e7624 100644 --- a/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb +++ b/packages/forest_admin_datasource_zendesk/spec/forest_admin_datasource_zendesk/plugins/create_ticket_with_notification_spec.rb @@ -1,4 +1,19 @@ module ForestAdminDatasourceZendesk + # Tiny PORO standing in for an ActionContextSingle in unit tests. We can't + # use Struct here because `Struct` mixes in Enumerable, which already defines + # `#filter` — having a `:filter` member triggers Lint/StructNewOverride and + # is genuinely ambiguous. + class FakeActionContext + attr_reader :form_values, :collection, :filter, :record_id + + def initialize(form_values: nil, collection: nil, filter: nil, record_id: nil) + @form_values = form_values + @collection = collection + @filter = filter + @record_id = record_id + end + end + RSpec.describe Plugins::CreateTicketWithNotification do let(:client) { instance_double(ForestAdminDatasourceZendesk::Client) } let(:datasource) do @@ -12,33 +27,428 @@ def initialize = @registered = {} def add_action(name, action) = @registered[name] = action end.new end + let(:result_builder) { ForestAdminDatasourceCustomizer::Decorators::Action::ResultBuilder.new } + + def register(opts = {}) + described_class.new.run(nil, collection_customizer, { datasource: datasource }.merge(opts)) + collection_customizer.registered[opts[:action_name] || described_class::NAME] + end - it 'delegates registration to Actions::CreateTicketWithNotification.register_on' do - described_class.new.run(nil, collection_customizer, - datasource: datasource, - default_subject: 'Welcome', - ticket_id_field: 'last_zendesk_ticket_id') + describe '#run' do + it 'registers a SINGLE-scoped action under the default name with the documented form fields' do + action = register - expect(collection_customizer.registered).to have_key(Actions::CreateTicketWithNotification::NAME) - action = collection_customizer.registered[Actions::CreateTicketWithNotification::NAME] - expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') + expect(collection_customizer.registered.keys).to contain_exactly(described_class::NAME) + expect(action.scope).to eq(ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::SINGLE) + labels = action.form.map { |f| f[:label] } + expect(labels).to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) + end + + it 'honors :action_name as a custom label' do + register + register(action_name: 'Custom label') + + expect(collection_customizer.registered.keys).to contain_exactly(described_class::NAME, 'Custom label') + end + + it 'raises ArgumentError without :datasource' do + expect { described_class.new.run(nil, collection_customizer, {}) } + .to raise_error(ArgumentError, /datasource/) + end + + it 'raises ArgumentError without a collection_customizer' do + expect { described_class.new.run(nil, nil, datasource: datasource) } + .to raise_error(ArgumentError, /collection/) + end end - it 'honors :action_name from options' do - described_class.new.run(nil, collection_customizer, - datasource: datasource, action_name: 'Open ticket') + describe 'requester email field' do + let(:context_double) do + instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_record: { 'id' => 42, 'email' => 'alice@x.com', 'name' => 'Alice' }) + end + + it 'uses RichText for the Message widget and marks Requester email required' do + action = register + message_field = action.form.find { |f| f[:label] == 'Message' } + expect(message_field[:widget]).to eq('RichText') + expect(action.form.first[:label]).to eq('Requester email') + expect(action.form.first[:is_required]).to be(true) + end - expect(collection_customizer.registered.keys).to contain_exactly('Open ticket') + it 'leaves the field empty when no requester_email_default is configured' do + expect(register.form.first[:default_value]).to be_nil + end + + context 'with a literal email String' do + it 'uses it verbatim as a static default (no record lookup)' do + action = register(requester_email_default: 'support@example.com') + expect(action.form.first[:default_value]).to eq('support@example.com') + end + end + + context 'with a Proc resolver' do + let(:resolver) { ->(record) { record['email'] } } + + it 'pre-fills the email field from the selected record via the resolver' do + action = register(requester_email_default: resolver) + expect(action.form.first[:default_value].call(context_double)).to eq('alice@x.com') + end + + it 'returns nil (and logs) when the record lookup raises' do + allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + + action = register(requester_email_default: resolver) + expect(action.form.first[:default_value].call(context_double)).to be_nil + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('fetch record', 'StandardError', 'boom')) + end + + it 'returns nil (and logs) when the resolver proc itself raises' do + allow(context_double).to receive(:get_record).and_return({ 'email' => 'a@b.com' }) + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + broken_resolver = ->(_record) { raise StandardError, 'typo in lambda' } + + action = register(requester_email_default: broken_resolver) + expect(action.form.first[:default_value].call(context_double)).to be_nil + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('requester_email_default', 'typo in lambda')) + end + end end - it 'raises ArgumentError without :datasource' do - expect { described_class.new.run(nil, collection_customizer, {}) } - .to raise_error(ArgumentError, /datasource/) + describe 'subject / message defaults' do + it 'uses configured static defaults verbatim' do + action = register(default_subject: 'Welcome', default_message: '

Hi

') + expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value]).to eq('Welcome') + expect(action.form.find { |f| f[:label] == 'Message' }[:default_value]).to eq('

Hi

') + end + + context 'with {{record.}} tokens' do + let(:context_double) do + instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_record: { 'id' => 42, 'name' => 'Alice', 'email' => 'alice@x.com' }) + end + + it 'substitutes record fields at form-open time' do + action = register(default_subject: 'Follow-up for {{record.name}}', + default_message: '

Hello {{record.name}} ({{record.email}})

') + subject_proc = action.form.find { |f| f[:label] == 'Subject' }[:default_value] + message_proc = action.form.find { |f| f[:label] == 'Message' }[:default_value] + + expect(subject_proc.call(context_double)).to eq('Follow-up for Alice') + expect(message_proc.call(context_double)).to eq('

Hello Alice (alice@x.com)

') + end + + it 'falls back to empty strings when the record lookup fails (logged)' do + allow(context_double).to receive(:get_record).and_raise(StandardError, 'boom') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + + action = register(default_subject: 'Hi {{record.name}}') + expect(action.form.find { |f| f[:label] == 'Subject' }[:default_value].call(context_double)).to eq('Hi ') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('fetch record', 'boom')) + end + end + end + + describe 'HTML escaping on the Message template' do + let(:context_double) do + instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + get_record: { 'name' => "O'Brien ", 'note' => '' }) + end + + it 'escapes interpolated values in the Message (RichText -> html_body)' do + action = register(default_message: '

Hi {{record.name}} - {{record.note}}

') + message_proc = action.form.find { |f| f[:label] == 'Message' }[:default_value] + + expect(message_proc.call(context_double)).to eq( + '

Hi O'Brien <admin> - <script>alert(1)</script>

' + ) + end + + it 'leaves Subject interpolation unescaped (plain-text field)' do + action = register(default_subject: 'Re: {{record.name}}') + subject_proc = action.form.find { |f| f[:label] == 'Subject' }[:default_value] + + expect(subject_proc.call(context_double)).to eq("Re: O'Brien ") + end end - it 'raises ArgumentError without a collection_customizer' do - expect { described_class.new.run(nil, nil, datasource: datasource) } - .to raise_error(ArgumentError, /collection/) + describe 'executor' do + let(:context) { Struct.new(:form_values).new(form_values) } + + context 'with a public html comment (default)' do + let(:form_values) do + { 'Requester email' => 'alice@x.com', 'Subject' => 'Refund', 'Message' => 'Hi there', + 'Priority' => 'high', 'Type' => 'question', 'Send as internal note' => false } + end + + it 'creates a ticket targeting the requester by email and embeds the html comment' do + allow(client).to receive(:create_ticket) do |payload| + expect(payload).to eq( + 'requester' => { 'email' => 'alice@x.com', 'name' => 'alice' }, + 'subject' => 'Refund', + 'comment' => { 'html_body' => 'Hi there', 'public' => true }, + 'priority' => 'high', + 'type' => 'question' + ) + { 'id' => 7 } + end + + result = register.execute.call(context, result_builder) + expect(client).to have_received(:create_ticket) + expect(result[:message]).to include('Ticket #7', 'notified') + end + end + + context 'with internal note' do + let(:form_values) do + { 'Requester email' => 'b@x.com', 'Subject' => 'Internal', 'Message' => 'For agents only', + 'Send as internal note' => true } + end + + it 'flips the comment to private and omits the notify wording' do + allow(client).to receive(:create_ticket).and_return('id' => 9) + + result = register.execute.call(context, result_builder) + expect(client).to have_received(:create_ticket).with(hash_including( + 'comment' => { 'html_body' => 'For agents only', + 'public' => false } + )) + expect(result[:message]).to include('internal note') + end + end + + context 'without a requester email' do + let(:form_values) { { 'Requester email' => nil, 'Subject' => 'S', 'Message' => 'M' } } + + it 'returns an error and does not call the client' do + allow(client).to receive(:create_ticket) + + result = register.execute.call(context, result_builder) + expect(result[:type]).to eq('Error') + expect(client).not_to have_received(:create_ticket) + end + end + + context 'with empty optional fields' do + let(:form_values) do + { 'Requester email' => 'c@x.com', 'Subject' => 'S', 'Message' => 'M', + 'Priority' => nil, 'Type' => '', 'Send as internal note' => nil } + end + + it 'omits empty optional keys from the payload' do + allow(client).to receive(:create_ticket) do |payload| + expect(payload.keys).not_to include('priority', 'type') + expect(payload['comment']['public']).to be(true) + { 'id' => 1 } + end + + register.execute.call(context, result_builder) + expect(client).to have_received(:create_ticket) + end + end + + context 'with ticket_id_field configured (writeback to host record)' do + let(:form_values) do + { 'Requester email' => 'd@x.com', 'Subject' => 'S', 'Message' => 'M', + 'Send as internal note' => false } + end + let(:host_collection) { instance_double('RelaxedCollection') } # rubocop:disable RSpec/VerifiedDoubleReference + let(:filter) { instance_double('Filter') } # rubocop:disable RSpec/VerifiedDoubleReference + let(:context) do + FakeActionContext.new(form_values: form_values, collection: host_collection, filter: filter) + end + + before { allow(client).to receive(:create_ticket).and_return('id' => 77) } + + it 'updates the host record with the new ticket id under the configured field' do + allow(host_collection).to receive(:update) + action = register(ticket_id_field: 'last_zendesk_ticket_id') + + result = action.execute.call(context, result_builder) + + expect(host_collection).to have_received(:update).with(filter, { 'last_zendesk_ticket_id' => 77 }) + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('Ticket #77') + expect(result[:message]).not_to include('warning') + end + + it 'logs and surfaces a warning when the host update fails but still succeeds' do + allow(host_collection).to receive(:update).and_raise(StandardError, 'field is read-only') + allow(ForestAdminDatasourceZendesk.logger).to receive(:warn) + action = register(ticket_id_field: 'last_zendesk_ticket_id') + + result = action.execute.call(context, result_builder) + + expect(result[:type]).to eq('Success') + expect(result[:message]).to include('Ticket #77', 'warning', 'field is read-only') + expect(ForestAdminDatasourceZendesk.logger).to have_received(:warn) + .with(a_string_including('last_zendesk_ticket_id', 'field is read-only')) + end + + it 'does not attempt any update when ticket_id_field is omitted' do + allow(host_collection).to receive(:update) + + register.execute.call(context, result_builder) + + expect(host_collection).not_to have_received(:update) + end + end + end + + describe 'email_templates wizard' do + let(:templates) do + [{ title: 'Welcome', content: '

Welcome aboard!

' }, + { title: 'Refund', content: '

Refund processed.

' }] + end + + it 'flips the form into a two-page wizard (Template first, body second)' do + action = register(email_templates: templates) + + expect(action.form.size).to eq(2) + page_one, page_two = action.form + expect(page_one[:component]).to eq('Page') + expect(page_one[:elements].map { |f| f[:label] }).to eq(['Template']) + expect(page_two[:elements].map { |f| f[:label] }) + .to eq(['Requester email', 'Subject', 'Message', 'Priority', 'Type', 'Send as internal note']) + end + + it 'lists No template + each template title in the dropdown' do + template_field = register(email_templates: templates).form.first[:elements].first + expect(template_field[:enum_values]).to eq(['No template', 'Welcome', 'Refund']) + expect(template_field[:default_value]).to eq('No template') + end + + it 'pre-fills Message via `value:` when Template was the just-changed field' do + message_field = register(email_templates: templates).form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: true, get_form_value: 'Refund') + + expect(message_field[:value].call(ctx)).to eq('

Refund processed.

') + end + + it "yields an empty Message when 'No template' was just selected" do + message_field = register(email_templates: templates).form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: true, get_form_value: 'No template') + + expect(message_field[:value].call(ctx)).to eq('') + end + + it 'returns nil (carry over current input) when another field triggered the re-fetch' do + message_field = register(email_templates: templates).form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: false) + + expect(message_field[:value].call(ctx)).to be_nil + end + + it 'interpolates {{record.}} tokens inside the selected template content' do + templated = [{ title: 'Welcome', content: '

Hi {{record.name}} ({{record.email}})

' }] + message_field = register(email_templates: templated).form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: true, get_form_value: 'Welcome', + get_record: { 'name' => 'Alice', 'email' => 'a@b.com' }) + + expect(message_field[:value].call(ctx)).to eq('

Hi Alice (a@b.com)

') + end + + it 'HTML-escapes interpolated record values inside template content' do + templated = [{ title: 'Bug', content: '

{{record.note}}

' }] + message_field = register(email_templates: templated).form.last[:elements].find { |f| f[:label] == 'Message' } + ctx = instance_double(ForestAdminDatasourceCustomizer::Decorators::Action::Context::ActionContextSingle, + field_changed?: true, get_form_value: 'Bug', + get_record: { 'note' => '' }) + + expect(message_field[:value].call(ctx)).to eq('

<script>x</script>

') + end + + it 'keeps the flat form when no templates are configured' do + expect(register(email_templates: []).form.first[:component]).to be_nil # not a Page + end + end + + describe 'priority_override / type_override' do + it 'omits the Priority field and forces the value in the payload' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + + action = register(priority_override: 'urgent') + expect(action.form.map { |f| f[:label] }).not_to include('Priority') + + action.execute.call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('priority' => 'urgent')) + end + + it 'omits the Type field and forces the value in the payload' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + + action = register(type_override: 'incident') + expect(action.form.map { |f| f[:label] }).not_to include('Type') + + action.execute.call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('type' => 'incident')) + end + + it 'forces the override even when the form value is also present' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + register(priority_override: 'urgent').execute.call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', 'Subject' => 'S', + 'Message' => 'M', 'Priority' => 'low' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('priority' => 'urgent')) + end + end + + describe 'requester name auto-derivation' do + it 'sends the email local-part as requester.name in the payload' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + register.execute.call( + FakeActionContext.new(form_values: { 'Requester email' => 'john.doe@acme.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including( + 'requester' => { 'email' => 'john.doe@acme.com', + 'name' => 'john.doe' } + )) + end + end + + describe 'sender_email' do + it 'maps to Zendesk `recipient` in the payload when configured' do + allow(client).to receive(:create_ticket).and_return('id' => 1) + register(sender_email: 'support@acme.com').execute.call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket).with(hash_including('recipient' => 'support@acme.com')) + end + + it 'omits recipient from the payload when sender_email is blank' do + allow(client).to receive(:create_ticket) do |payload| + expect(payload).not_to have_key('recipient') + { 'id' => 1 } + end + register.execute.call( + FakeActionContext.new(form_values: { 'Requester email' => 'a@b.com', + 'Subject' => 'S', 'Message' => 'M' }), + result_builder + ) + expect(client).to have_received(:create_ticket) + end end end end