Split/merge WIP

2025-12-17 14:53:13 +01:00 · 2025-12-17 14:53:13 +01:00 · d4ce94ddf8
commit d4ce94ddf8
parent 78d2ff66b2
2 changed files with 1412 additions and 0 deletions
--- a/docs/bridge-ticket-split-merge-investigation.md
+++ b/docs/bridge-ticket-split-merge-investigation.md
@ -0,0 +1,506 @@
+# Zammad Ticket Splits & Merges with Bridge Channels
+
+## Investigation Summary
+
+This document analyzes how Zammad handles ticket splits and merges, and the implications for our custom bridge channels (WhatsApp, Signal, Voice).
+
+## Current State
+
+### How Zammad Handles Split/Merge (Built-in)
+
+#### Merge (`ticket.merge_to` in `app/models/ticket.rb:330`)
+
+When ticket A is merged into ticket B:
+
+1. All articles from A are moved to B
+2. A "parent" link is created between A and B
+3. Mentions and external links are migrated
+4. Source ticket A's state is set to "merged"
+5. Source ticket A's owner is reset to System (id: 1)
+
+**Critical issue:** Ticket preferences are NOT copied or migrated. The target ticket B keeps its original preferences, and source ticket A's preferences become orphaned.
+
+```ruby
+# From app/models/ticket.rb - merge_to method
+# Articles are moved:
+Ticket::Article.where(ticket_id: id).update_all(['ticket_id = ?', data[:ticket_id]])
+
+# But preferences are never touched - they stay on the source ticket
+```
+
+#### Split (`app/models/form_updater/concerns/applies_split_ticket_article.rb`)
+
+When an article is split from ticket A to create new ticket C:
+
+1. Basic ticket attributes are copied (group, customer, state, priority, title)
+2. Attachments are cloned
+3. A link is created to the original ticket
+4. `owner_id` is explicitly deleted (not copied)
+
+**Critical issue:** Preferences are NOT copied. The new ticket C has no channel metadata.
+
+```ruby
+# From applies_split_ticket_article.rb
+def attributes_to_apply
+  attrs = selected_ticket_article.ticket.attributes
+  attrs['title'] = selected_ticket_article.subject if selected_ticket_article.subject.present?
+  attrs['body']  = body_with_form_id_urls
+  attrs.delete 'owner_id'  # Explicitly deleted
+  attrs
+  # Note: preferences are NOT included in .attributes
+end
+```
+
+#### Email Follow-up Handling (`app/models/channel/filter/follow_up_merged.rb`)
+
+Zammad has a postmaster filter that handles incoming emails to merged tickets:
+
+```ruby
+def self.run(_channel, mail, _transaction_params)
+  return if mail[:'x-zammad-ticket-id'].blank?
+
+  referenced_ticket = Ticket.find_by(id: mail[:'x-zammad-ticket-id'])
+  return if referenced_ticket.blank?
+
+  new_target_ticket = find_merge_follow_up_ticket(referenced_ticket)
+  return if new_target_ticket.blank?
+
+  mail[:'x-zammad-ticket-id'] = new_target_ticket.id
+end
+```
+
+This follows the parent link to find the active target ticket. **This only works for email** - no equivalent exists for other channels like Telegram, WhatsApp, or Signal.
+
+---
+
+## Bridge Channel Metadata Structure
+
+Our bridge channels store critical routing metadata in `ticket.preferences`:
+
+### WhatsApp
+
+```ruby
+ticket.preferences = {
+  channel_id: 123,
+  cdr_whatsapp: {
+    bot_token: "abc123",      # Identifies which bot/channel
+    chat_id: "+1234567890"    # Customer's phone number - WHERE TO SEND
+  }
+}
+```
+
+### Signal (Direct Message)
+
+```ruby
+ticket.preferences = {
+  channel_id: 456,
+  cdr_signal: {
+    bot_token: "xyz789",
+    chat_id: "+1234567890"    # Customer's phone number
+  }
+}
+```
+
+### Signal (Group)
+
+```ruby
+ticket.preferences = {
+  channel_id: 456,
+  cdr_signal: {
+    bot_token: "xyz789",
+    chat_id: "group.abc123...",     # Signal group ID
+    group_joined: true,              # Whether customer accepted invite
+    group_joined_at: "2024-01-01",
+    original_recipient: "+1234567890"
+  }
+}
+```
+
+---
+
+## How Bridge Channels Use This Metadata
+
+### Outgoing Messages
+
+The communication jobs (`CommunicateCdrWhatsappJob`, `CommunicateCdrSignalJob`) rely entirely on ticket preferences:
+
+```ruby
+# From communicate_cdr_whatsapp_job.rb
+def perform(article_id)
+  article = Ticket::Article.find(article_id)
+  ticket = Ticket.lookup(id: article.ticket_id)
+  
+  # These MUST exist or the job fails:
+  unless ticket.preferences['cdr_whatsapp']['bot_token']
+    log_error(article, "Can't find ticket.preferences['cdr_whatsapp']['bot_token']")
+  end
+  unless ticket.preferences['cdr_whatsapp']['chat_id']
+    log_error(article, "Can't find ticket.preferences['cdr_whatsapp']['chat_id']")
+  end
+  
+  channel = Channel.lookup(id: ticket.preferences['channel_id'])
+  result = channel.deliver(article)  # Uses chat_id to know where to send
+end
+```
+
+### Incoming Messages
+
+The webhook controllers look up existing tickets:
+
+**WhatsApp** (`channels_cdr_whatsapp_controller.rb`):
+```ruby
+# Find open ticket for this customer
+state_ids = Ticket::State.where(name: %w[closed merged removed]).pluck(:id)
+ticket = Ticket.where(customer_id: customer.id)
+               .where.not(state_id: state_ids)
+               .order(:updated_at).first
+```
+
+**Signal Groups** (`channels_cdr_signal_controller.rb`):
+```ruby
+# Find ticket by group ID in preferences
+ticket = Ticket.where.not(state_id: state_ids)
+               .where("preferences LIKE ?", "%channel_id: #{channel.id}%")
+               .where("preferences LIKE ?", "%chat_id: #{receiver_phone_number}%")
+               .order(updated_at: :desc)
+               .first
+```
+
+---
+
+## Problem Scenarios
+
+### Scenario 1: Merge Bridge Ticket → Non-Bridge Ticket
+
+**Setup:** Ticket A (has WhatsApp metadata) merged into Ticket B (no bridge metadata)
+
+**What happens:**
+- A's articles move to B
+- A's preferences stay on A (now in merged state)
+- B still has no bridge preferences
+
+**Result:** Agent replies on ticket B fail - no `chat_id` to send to.
+
+### Scenario 2: Merge Bridge Ticket → Different Bridge Ticket
+
+**Setup:** Ticket A (WhatsApp to +111) merged into Ticket B (WhatsApp to +222)
+
+**What happens:**
+- A's articles move to B
+- B keeps its preferences (`chat_id: +222`)
+
+**Result:** Agent replies go to +222, not to +111. Customer +111 never receives responses.
+
+### Scenario 3: Split Article from Bridge Ticket
+
+**Setup:** Split an article from Ticket A (has WhatsApp metadata) to create Ticket C
+
+**What happens:**
+- New ticket C is created with no preferences
+- C is linked to A
+
+**Result:** Agent cannot reply via WhatsApp on ticket C at all - job fails immediately.
+
+### Scenario 4: Incoming Message to Merged Ticket's Customer
+
+**Setup:** Ticket A (customer +111) was merged into B. Customer +111 sends new message.
+
+**What happens:**
+- Webhook finds customer by phone number
+- Looks for open ticket for customer
+- A is excluded (merged state)
+- Either finds B (if same customer) or creates new ticket
+
+**Result:** May work if B has same customer, but conversation context is fragmented.
+
+### Scenario 5: Signal Group Ticket Merged
+
+**Setup:** Ticket A (Signal group X) merged into Ticket B (no Signal metadata)
+
+**What happens:**
+- All group messages went to A
+- A is now merged, B has no group reference
+- New messages from group X create a new ticket (can't find existing by group ID)
+
+**Result:** Conversation splits into multiple tickets unexpectedly.
+
+---
+
+## Recommended Solutions
+
+### Option 1: Preferences Migration on Merge (Recommended)
+
+Create a concern that copies bridge channel metadata when tickets are merged:
+
+```ruby
+# app/models/ticket/merge_bridge_channel_preferences.rb
+module Ticket::MergeBridgeChannelPreferences
+  extend ActiveSupport::Concern
+
+  included do
+    after_update :migrate_bridge_preferences_on_merge
+  end
+
+  private
+
+  def migrate_bridge_preferences_on_merge
+    return unless saved_change_to_state_id?
+    return unless state.state_type.name == 'merged'
+    
+    target_ticket = find_merge_target
+    return unless target_ticket
+
+    # Copy bridge preferences if target doesn't have them
+    %w[cdr_whatsapp cdr_signal cdr_voice].each do |channel_key|
+      next unless preferences[channel_key].present?
+      next if target_ticket.preferences[channel_key].present?
+      
+      target_ticket.preferences[channel_key] = preferences[channel_key].deep_dup
+      target_ticket.preferences['channel_id'] ||= preferences['channel_id']
+    end
+    
+    target_ticket.save! if target_ticket.changed?
+  end
+
+  def find_merge_target
+    Link.list(link_object: 'Ticket', link_object_value: id)
+        .find { |l| l['link_type'] == 'parent' && l['link_object'] == 'Ticket' }
+        &.then { |l| Ticket.find_by(id: l['link_object_value']) }
+  end
+end
+```
+
+**Pros:**
+- Handles the common case (merging bridge ticket into non-bridge ticket)
+- Automatic, no agent action required
+- Non-destructive (doesn't overwrite existing preferences)
+
+**Cons:**
+- Doesn't handle case where both tickets have different bridge metadata
+- May need additional logic for conflicting preferences
+
+### Option 2: Follow-up Filter for Bridge Channels
+
+Create filters similar to `FollowUpMerged` that redirect incoming bridge messages:
+
+```ruby
+# Modify webhook controllers to check for merged tickets
+def find_active_ticket_for_customer(customer, state_ids)
+  ticket = Ticket.where(customer_id: customer.id)
+                 .where.not(state_id: state_ids)
+                 .order(:updated_at).first
+  
+  # If ticket is merged, follow parent link
+  if ticket&.state&.state_type&.name == 'merged'
+    ticket = find_merge_target(ticket) || ticket
+  end
+  
+  ticket
+end
+
+def find_merge_target(ticket)
+  Link.list(link_object: 'Ticket', link_object_value: ticket.id)
+      .filter_map do |link|
+        next if link['link_type'] != 'parent'
+        next if link['link_object'] != 'Ticket'
+        
+        Ticket.joins(state: :state_type)
+              .where.not(ticket_state_types: { name: 'merged' })
+              .find_by(id: link['link_object_value'])
+      end.first
+end
+```
+
+**Pros:**
+- Handles incoming messages to merged tickets correctly
+- Follows same pattern as Zammad's email handling
+
+**Cons:**
+- Requires modifying webhook controllers
+- Only handles incoming direction, not outgoing
+
+### Option 3: Copy Preferences on Split
+
+Modify the split form updater or add a callback to copy bridge preferences:
+
+```ruby
+# Add to ticket creation from split
+module Ticket::SplitBridgeChannelPreferences
+  extend ActiveSupport::Concern
+
+  included do
+    after_create :copy_bridge_preferences_from_source
+  end
+
+  private
+
+  def copy_bridge_preferences_from_source
+    # Find source ticket via link
+    source_link = Link.list(link_object: 'Ticket', link_object_value: id)
+                      .find { |l| l['link_type'] == 'child' }
+    return unless source_link
+    
+    source_ticket = Ticket.find_by(id: source_link['link_object_value'])
+    return unless source_ticket
+    
+    # Copy bridge preferences
+    %w[cdr_whatsapp cdr_signal cdr_voice channel_id].each do |key|
+      next unless source_ticket.preferences[key].present?
+      self.preferences[key] = source_ticket.preferences[key].deep_dup
+    end
+    
+    save! if changed?
+  end
+end
+```
+
+### Option 4: UI Warning + Manual Handling
+
+Add frontend validation to warn agents:
+
+1. Check for bridge preferences before merge/split
+2. Show warning dialog explaining implications
+3. Optionally provide UI to manually transfer channel association
+
+```typescript
+// In merge confirmation dialog
+const hasBridgeChannel = ticket.preferences?.cdr_whatsapp || 
+                          ticket.preferences?.cdr_signal;
+if (hasBridgeChannel) {
+  showWarning(
+    "This ticket uses WhatsApp/Signal messaging. " +
+    "Merging may affect message routing. " +
+    "Replies will be sent to the target ticket's contact."
+  );
+}
+```
+
+### Option 5: Multi-Channel Preferences (Long-term)
+
+Allow tickets to have multiple channel associations:
+
+```ruby
+ticket.preferences = {
+  bridge_channels: [
+    { type: 'cdr_whatsapp', chat_id: '+111...', channel_id: 1, customer_id: 100 },
+    { type: 'cdr_whatsapp', chat_id: '+222...', channel_id: 1, customer_id: 101 },
+    { type: 'cdr_signal', chat_id: 'group.xxx', channel_id: 2 }
+  ]
+}
+```
+
+This would require significant refactoring of communication jobs to handle multiple recipients.
+
+---
+
+## Signal Groups - Special Considerations
+
+Signal groups add complexity:
+
+1. **Group ID is the routing key**, not phone number
+2. **Multiple customers** might be in the same group
+3. **`group_joined` flag** tracks invite acceptance - messages can't be sent until true
+4. **Group membership changes** could affect ticket routing
+
+### Merge Rules for Signal Groups
+
+| Source Ticket | Target Ticket | Recommendation |
+|---------------|---------------|----------------|
+| Signal group A | No Signal | Copy preferences (Option 1) |
+| Signal group A | Signal group A (same) | Safe to merge |
+| Signal group A | Signal group B (different) | **Block or warn** - can't merge different group conversations |
+| Signal group A | Signal DM | **Block or warn** - different communication modes |
+
+Consider adding validation:
+
+```ruby
+def validate_signal_group_merge(source, target)
+  source_group = source.preferences.dig('cdr_signal', 'chat_id')
+  target_group = target.preferences.dig('cdr_signal', 'chat_id')
+  
+  return true if source_group.blank? || target_group.blank?
+  return true if source_group == target_group
+  
+  # Different groups - this is problematic
+  raise Exceptions::UnprocessableEntity, 
+    "Cannot merge tickets from different Signal groups"
+end
+```
+
+---
+
+## Recommended Implementation Path
+
+### Phase 1: Immediate (Low Risk)
+
+1. **Add preferences migration on merge** (Option 1)
+   - Only copies if target doesn't have existing preferences
+   - Handles most common case safely
+
+2. **Add preferences copy on split** (Option 3)
+   - New tickets get parent's channel metadata
+   - Enables replies on split tickets
+
+### Phase 2: Short-term
+
+3. **Add follow-up handling in webhooks** (Option 2)
+   - Modify webhook controllers to follow merge parent links
+   - Handles incoming messages to merged ticket's customer
+
+4. **Add UI warnings** (Option 4)
+   - Warn agents about implications
+   - Especially for conflicting metadata scenarios
+
+### Phase 3: Medium-term
+
+5. **Add merge validation for Signal groups**
+   - Block merging tickets from different groups
+   - Or add clear warning about implications
+
+6. **Add audit logging**
+   - Track when preferences are migrated
+   - Help agents understand what happened
+
+---
+
+## Files to Modify
+
+### Zammad Addon (zammad-addon-bridge)
+
+| File | Change |
+|------|--------|
+| `src/app/models/ticket/merge_bridge_channel_preferences.rb` | New - preferences migration |
+| `src/app/models/ticket/split_bridge_channel_preferences.rb` | New - preferences copy on split |
+| `src/app/controllers/channels_cdr_whatsapp_controller.rb` | Add merge follow-up handling |
+| `src/app/controllers/channels_cdr_signal_controller.rb` | Add merge follow-up handling |
+| `src/config/initializers/bridge.rb` | Include new concerns in Ticket model |
+
+### Link Frontend (optional)
+
+| File | Change |
+|------|--------|
+| Merge dialog component | Add warning for bridge tickets |
+
+---
+
+## Testing Scenarios
+
+1. Merge WhatsApp ticket → empty ticket → verify agent can reply
+2. Merge WhatsApp ticket → WhatsApp ticket (same number) → verify routing
+3. Merge WhatsApp ticket → WhatsApp ticket (different number) → verify warning/behavior
+4. Split article from WhatsApp ticket → verify new ticket has preferences
+5. Customer sends message after their ticket was merged → verify routing
+6. Merge Signal group ticket → verify group_joined flag is preserved
+7. Merge two different Signal group tickets → verify validation/warning
+
+---
+
+## References
+
+- Zammad merge implementation: `app/models/ticket.rb:330-450`
+- Zammad split implementation: `app/models/form_updater/concerns/applies_split_ticket_article.rb`
+- Zammad email follow-up filter: `app/models/channel/filter/follow_up_merged.rb`
+- Bridge WhatsApp controller: `packages/zammad-addon-bridge/src/app/controllers/channels_cdr_whatsapp_controller.rb`
+- Bridge Signal controller: `packages/zammad-addon-bridge/src/app/controllers/channels_cdr_signal_controller.rb`
+- Bridge WhatsApp job: `packages/zammad-addon-bridge/src/app/jobs/communicate_cdr_whatsapp_job.rb`
+- Bridge Signal job: `packages/zammad-addon-bridge/src/app/jobs/communicate_cdr_signal_job.rb`