From 4ec3f1324d733d916ae2f1a92cb07f216b9ce1e5 Mon Sep 17 00:00:00 2001
From: Ryan Lue <hello@ryanlue.com>
Date: Tue, 7 Aug 2018 17:51:12 +0800
Subject: [PATCH] Change call log to display inactive users with strikethrough
 (fixes #2096)

---
 .../app/views/cti/caller_log.jst.eco          |  8 ++-
 app/assets/stylesheets/zammad.scss            |  2 +-
 app/models/cti/caller_id.rb                   | 28 +++------
 test/browser/integration_cti_test.rb          | 63 +++++++++++++++++++
 4 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/app/assets/javascripts/app/views/cti/caller_log.jst.eco b/app/assets/javascripts/app/views/cti/caller_log.jst.eco
index d00c71e4a..6265567d9 100644
--- a/app/assets/javascripts/app/views/cti/caller_log.jst.eco
+++ b/app/assets/javascripts/app/views/cti/caller_log.jst.eco
@@ -26,8 +26,10 @@
             <% if caller_id.user_id && App.User.exists(caller_id.user_id): %>
               <% shown = true %>
               <% user = App.User.find(caller_id.user_id) %>
+              <% classes = ['user-popover'] %>
+              <% classes.push('is-inactive') if !user.active %>
               <% if caller_id.level isnt 'known': %><%- @T('maybe') %> <% end %>
-              <span class="user-popover" data-id="<%- user.id %>"><%= user.displayNameLong() %></span><br>
+              <span class="<%= classes.join(' ') %>" data-id="<%- user.id %>"><%= user.displayNameLong() %></span><br>
             <% else if caller_id.comment: %>
               <% shown = true %>
               <%- @T('maybe') %> <%= caller_id.comment %><br>
@@ -52,8 +54,10 @@
             <% if caller_id.user_id && App.User.exists(caller_id.user_id): %>
               <% shown = true %>
               <% user = App.User.find(caller_id.user_id) %>
+              <% classes = ['user-popover'] %>
+              <% classes.push('is-inactive') if !user.active %>
               <% if caller_id.level isnt 'known': %><%- @T('maybe') %> <% end %>
-              <span class="user-popover" data-id="<%- user.id %>"><%= user.displayNameLong() %></span><br>
+              <span class="<%= classes.join(' ') %>" data-id="<%- user.id %>"><%= user.displayNameLong() %></span><br>
             <% else if caller_id.comment: %>
               <% shown = true %>
               <%- @T('maybe') %> <%= caller_id.comment %><br>
diff --git a/app/assets/stylesheets/zammad.scss b/app/assets/stylesheets/zammad.scss
index 3b0bf141c..2bb00cbe2 100644
--- a/app/assets/stylesheets/zammad.scss
+++ b/app/assets/stylesheets/zammad.scss
@@ -1053,7 +1053,7 @@ th.align-right {
   padding: 0;
 }
 
-.table tr.is-inactive {
+.table tr.is-inactive, .table tr td span.is-inactive {
   color: #bbb;
   text-decoration: line-through;
 
diff --git a/app/models/cti/caller_id.rb b/app/models/cti/caller_id.rb
index 0540c6654..77e6a347c 100644
--- a/app/models/cti/caller_id.rb
+++ b/app/models/cti/caller_id.rb
@@ -47,25 +47,17 @@ returns
 =end
 
     def self.lookup(caller_id)
+      lookup_ids =
+        ['known', 'maybe', nil].lazy.map do |level|
+          Cti::CallerId.select('MAX(id) as caller_id')
+                       .where({ caller_id: caller_id, level: level }.compact)
+                       .group(:user_id)
+                       .order('caller_id DESC')
+                       .limit(20)
+                       .map(&:caller_id)
+        end.find(&:present?)
 
-      result = []
-      ['known', 'maybe', nil].each do |level|
-
-        search_params = {
-          caller_id: caller_id,
-        }
-
-        if level
-          search_params[:level] = level
-        end
-
-        caller_ids = Cti::CallerId.select('MAX(id) as caller_id').where(search_params).group(:user_id).order('caller_id DESC').limit(20).map(&:caller_id)
-        Cti::CallerId.where(id: caller_ids).order(id: :desc).each do |record|
-          result.push record
-        end
-        break if result.present?
-      end
-      result
+      Cti::CallerId.where(id: lookup_ids).order(id: :desc).to_a
     end
 
 =begin
diff --git a/test/browser/integration_cti_test.rb b/test/browser/integration_cti_test.rb
index 9fd346393..e305ff463 100644
--- a/test/browser/integration_cti_test.rb
+++ b/test/browser/integration_cti_test.rb
@@ -150,4 +150,67 @@ class IntegrationCtiTest < TestCase
       value: '+49 30 609811111',
     )
   end
+
+  # Regression test for #2096
+  def test_inactive_users_displayed_with_strikethrough_in_caller_log
+    id = rand(99_999_999)
+
+    @browser = browser_instance
+    login(
+      username: 'master@example.com',
+      password: 'test',
+      url: browser_url,
+    )
+
+    # create inactive user with phone number (via API)
+    user_create(
+      data: {
+        login:     'test_user',
+        firstname: 'John',
+        lastname:  'Doe'
+      },
+    )
+
+    click(css: 'table.user-list > tbody > tr:first-of-type > td:first-of-type')
+    set(css: 'input[name="phone"]', value: '1234567890')
+    select(css: 'select[name="active"]', value: 'inactive')
+    click(css: 'button[type="submit"]')
+
+    # enable CTI
+    click(css: 'a[href="#manage"]')
+    click(css: 'a[href="#system/integration"]')
+    click(css: 'a[href="#system/integration/cti"]')
+
+    switch(
+      css: '.content.active .js-switch',
+      type: 'on'
+    )
+
+    watch_for(
+      css: 'a[href="#cti"]'
+    )
+
+    click(css: 'a[href="#cti"]')
+
+    # simulate CTI callback to/from inactive user
+    url = URI.join(browser_url, "api/v1/cti/#{ENV['CTI_TOKEN']}")
+    params = {
+      direction: 'in',
+      from: '1234567890',
+      to: '1234567890',
+      callId: "4991155921769858278-#{id}",
+      cause: 'busy'
+    }
+    Net::HTTP.post_form(url, params.merge(event: 'newCall'))
+    Net::HTTP.post_form(url, params.merge(event: 'hangup'))
+
+    # view caller log
+    click(css: 'a[href="#cti"]')
+
+    # assertion: names appear in strikethrough
+    match(
+      css: 'span.is-inactive',
+      value: 'John Doe',
+    )
+  end
 end