From 97b17caea4dbe0e2846f608e68fac846c7d065ad Mon Sep 17 00:00:00 2001
From: Ryan Lue <rl@zammad.com>
Date: Fri, 28 Sep 2018 08:49:06 +0200
Subject: [PATCH] Test and fix time accounting API endpoint bug (replaces
 #2243)

---
 .../time_accountings_controller.rb            |  2 +-
 spec/factories/ticket/time_accounting.rb      |  7 +--
 spec/requests/time_accounting_spec.rb         | 57 +++++++++++--------
 3 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/app/controllers/time_accountings_controller.rb b/app/controllers/time_accountings_controller.rb
index 0c581a854..1ff3b7d82 100644
--- a/app/controllers/time_accountings_controller.rb
+++ b/app/controllers/time_accountings_controller.rb
@@ -46,7 +46,7 @@ class TimeAccountingsController < ApplicationController
           end
         end
       end
-      if !customers[local_time_unit[:agent_id]]
+      if !agents[local_time_unit[:agent_id]]
         agent_user = User.lookup(id: local_time_unit[:agent_id])
         agent = '-'
         if agent_user
diff --git a/spec/factories/ticket/time_accounting.rb b/spec/factories/ticket/time_accounting.rb
index 11a56ecb3..18aab839b 100644
--- a/spec/factories/ticket/time_accounting.rb
+++ b/spec/factories/ticket/time_accounting.rb
@@ -1,10 +1,7 @@
 FactoryBot.define do
   factory :ticket_time_accounting, class: Ticket::TimeAccounting do
-    ticket_id { FactoryBot.create(:ticket).id }
-    ticket_article_id { FactoryBot.create(:ticket_article).id }
-    time_unit 200
+    ticket
+    time_unit { rand(100) }
     created_by_id 1
-    created_at Time.zone.now
-    updated_at Time.zone.now
   end
 end
diff --git a/spec/requests/time_accounting_spec.rb b/spec/requests/time_accounting_spec.rb
index 9bcc3b247..096f9e9d2 100644
--- a/spec/requests/time_accounting_spec.rb
+++ b/spec/requests/time_accounting_spec.rb
@@ -1,36 +1,43 @@
 require 'rails_helper'
 
-RSpec.describe 'Time Accounting', type: :request do
+RSpec.describe 'Time Accounting API endpoints', type: :request do
+  let(:admin)    { create(:admin_user) }
+  let(:customer) { create(:customer_user) }
+  let(:year)     { Time.current.year }
+  let(:month)    { Time.current.month }
 
-  let(:admin_user) do
-    create(:admin_user)
-  end
-  let(:customer_user) do
-    create(:customer_user)
-  end
-  let(:year) do
-    DateTime.now.utc.year
-  end
-  let(:month) do
-    DateTime.now.utc.month
-  end
+  describe '/api/v1/time_accounting/log/by_ticket' do
+    context 'when requesting a JSON response' do
+      # see https://github.com/zammad/zammad/pull/2243
+      context 'and logs exist for work performed by an agent who is also the customer of the ticket (#2243)' do
+        let(:ticket) { create(:ticket, customer: admin) }
+        let!(:time_log) { create(:ticket_time_accounting, ticket: ticket, created_by_id: admin.id) }
 
-  describe 'request handling' do
+        it 'responds with a non-nil value for each :agent key' do
+          authenticated_as(admin)
+          get "/api/v1/time_accounting/log/by_ticket/#{year}/#{month}", as: :json
 
-    it 'does time account report' do
-      group   = create(:group)
-      ticket  = create(:ticket, state: Ticket::State.lookup(name: 'open'), customer: customer_user )
-      article = create(:ticket_article, ticket_id: ticket.id, type: Ticket::Article::Type.lookup(name: 'note') )
+          expect(json_response.first).not_to include('agent' => nil)
+        end
+      end
+    end
 
-      create(:ticket_time_accounting, ticket_id: ticket.id, ticket_article_id: article.id)
+    context 'when requesting a log report download' do
+      it 'responds with an Excel spreadsheet' do
+        group   = create(:group)
+        ticket  = create(:ticket, state: Ticket::State.lookup(name: 'open'), customer: customer )
+        article = create(:ticket_article, ticket: ticket, type: Ticket::Article::Type.lookup(name: 'note') )
 
-      authenticated_as(admin_user)
-      get "/api/v1/time_accounting/log/by_ticket/#{year}/#{month}?download=true", params: {}
+        create(:ticket_time_accounting, ticket_id: ticket.id, ticket_article_id: article.id)
 
-      expect(response).to have_http_status(200)
-      expect(response['Content-Disposition']).to be_truthy
-      expect(response['Content-Disposition']).to eq("attachment; filename=\"by_ticket-#{year}-#{month}.xls\"")
-      expect(response['Content-Type']).to eq('application/vnd.ms-excel')
+        authenticated_as(admin)
+        get "/api/v1/time_accounting/log/by_ticket/#{year}/#{month}?download=true", params: {}
+
+        expect(response).to have_http_status(200)
+        expect(response['Content-Disposition']).to be_truthy
+        expect(response['Content-Disposition']).to eq("attachment; filename=\"by_ticket-#{year}-#{month}.xls\"")
+        expect(response['Content-Type']).to eq('application/vnd.ms-excel')
+      end
     end
   end
 end