Test and fix time accounting API endpoint bug (replaces #2243)

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

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

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