From d33c80bbaa4aa01163efdb6c6bc6fa7f58dd46ed Mon Sep 17 00:00:00 2001 From: Muhammad Nuzaihan Date: Wed, 7 Feb 2018 17:15:00 +0800 Subject: [PATCH] Fixes and improves reporting in #1809 by not showing merge but still use user's input --- app/models/report.rb | 56 ++++------------------------- lib/report/ticket_first_solution.rb | 18 +++++++++- lib/report/ticket_generic_time.rb | 16 +++++++++ lib/report/ticket_moved.rb | 14 ++++++++ lib/report/ticket_reopened.rb | 8 +++++ test/integration/report_test.rb | 29 +++++++-------- 6 files changed, 75 insertions(+), 66 deletions(-) diff --git a/app/models/report.rb b/app/models/report.rb index 68879cb24..8546a649d 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -21,13 +21,7 @@ class Report selected: true, dataDownload: true, adapter: Report::TicketGenericTime, - params: { field: 'created_at' }, - condition: { - 'state' => { - 'operator' => 'is not', - 'value' => 'merged' - } - } + params: { field: 'created_at' } }, { name: 'closed', @@ -35,52 +29,28 @@ class Report selected: true, dataDownload: true, adapter: Report::TicketGenericTime, - params: { field: 'close_at' }, - condition: { - 'state' => { - 'operator' => 'is not', - 'value' => 'merged' - } - } + params: { field: 'close_at' } }, { name: 'backlog', display: 'Backlog', selected: true, dataDownload: false, - adapter: Report::TicketBacklog, - condition: { - 'state' => { - 'operator' => 'is not', - 'value' => 'merged' - } - } + adapter: Report::TicketBacklog }, { name: 'first_solution', display: 'First Solution', selected: false, dataDownload: true, - adapter: Report::TicketFirstSolution, - condition: { - 'ticket_state.name' => { - 'operator' => 'is not', - 'value' => 'merged' - } - } + adapter: Report::TicketFirstSolution }, { name: 'reopened', display: 'Reopened', selected: false, dataDownload: true, - adapter: Report::TicketReopened, - condition: { - 'ticket_state.name' => { - 'operator' => 'is not', - 'value' => 'merged' - } - } + adapter: Report::TicketReopened }, { name: 'movedin', @@ -88,13 +58,7 @@ class Report selected: false, dataDownload: true, adapter: Report::TicketMoved, - params: { type: 'in' }, - condition: { - 'ticket_state.name' => { - 'operator' => 'is not', - 'value' => 'merged' - } - } + params: { type: 'in' } }, { name: 'movedout', @@ -102,13 +66,7 @@ class Report selected: false, dataDownload: true, adapter: Report::TicketMoved, - params: { type: 'out' }, - condition: { - 'ticket_state.name' => { - 'operator' => 'is not', - 'value' => 'merged' - } - } + params: { type: 'out' } }, ] config[:metric][:count][:backend] = backend diff --git a/lib/report/ticket_first_solution.rb b/lib/report/ticket_first_solution.rb index 91302a94d..b0b928140 100644 --- a/lib/report/ticket_first_solution.rb +++ b/lib/report/ticket_first_solution.rb @@ -50,6 +50,14 @@ returns elsif params[:interval] == 'minute' stop = start + 1.minute end + + without_merged_tickets = { + 'ticket_state.name' => { + 'operator' => 'is not', + 'value' => 'merged' + } + } + params[:selector].merge!(without_merged_tickets) query, bind_params, tables = Ticket.selector2sql(params[:selector]) ticket_list = Ticket.select('tickets.id, tickets.close_at, tickets.created_at').where( 'tickets.close_at IS NOT NULL AND tickets.close_at >= ? AND tickets.close_at < ?', @@ -89,7 +97,15 @@ returns =end def self.items(params) - query, bind_params, tables = Ticket.selector2sql(params[:selector]) + without_merged_tickets = { + 'ticket_state.name' => { + 'operator' => 'is not', + 'value' => 'merged' + } + } + + selector = params[:selector].merge!(without_merged_tickets) + query, bind_params, tables = Ticket.selector2sql(selector) ticket_list = Ticket.select('tickets.id, tickets.close_at, tickets.created_at').where( 'tickets.close_at IS NOT NULL AND tickets.close_at >= ? AND tickets.close_at < ?', params[:range_start], diff --git a/lib/report/ticket_generic_time.rb b/lib/report/ticket_generic_time.rb index 3869f4e79..c1c0657db 100644 --- a/lib/report/ticket_generic_time.rb +++ b/lib/report/ticket_generic_time.rb @@ -29,10 +29,18 @@ returns field: params[:params][:field], } + without_merged_tickets = { + 'state' => { + 'operator' => 'is not', + 'value' => 'merged' + } + } + selector = params[:selector].clone if params[:params].present? && params[:params][:selector].present? selector = selector.merge(params[:params][:selector]) end + selector.merge!(without_merged_tickets) # do not show merged tickets in reports result_es = SearchIndexBackend.selectors(['Ticket'], selector, nil, nil, aggs_interval) @@ -140,10 +148,18 @@ returns limit = 100 end + without_merged_tickets = { + 'state' => { + 'operator' => 'is not', + 'value' => 'merged' + } + } + selector = params[:selector].clone if params[:params] && params[:params][:selector] selector = selector.merge(params[:params][:selector]) end + selector.merge!(without_merged_tickets) # do not show merged tickets in reports result = SearchIndexBackend.selectors(['Ticket'], selector, limit, nil, aggs_interval) return result if params[:sheet].present? diff --git a/lib/report/ticket_moved.rb b/lib/report/ticket_moved.rb index 88e4de29e..8ed7dccc5 100644 --- a/lib/report/ticket_moved.rb +++ b/lib/report/ticket_moved.rb @@ -60,6 +60,13 @@ returns end local_params = group_attributes(selector, params) local_selector = params[:selector].clone + without_merged_tickets = { + 'ticket_state.name' => { + 'operator' => 'is not', + 'value' => 'merged' + } + } + local_selector.merge!(without_merged_tickets) # do not show merged tickets in reports if params[:params][:type] == 'out' local_selector.delete('ticket.group_id') end @@ -110,6 +117,13 @@ returns end local_params = group_attributes(selector, params) local_selector = params[:selector].clone + without_merged_tickets = { + 'ticket_state.name' => { + 'operator' => 'is not', + 'value' => 'merged' + } + } + local_selector = params[:selector].merge!(without_merged_tickets) # do not show merged tickets in reports if params[:params][:type] == 'out' local_selector.delete('ticket.group_id') end diff --git a/lib/report/ticket_reopened.rb b/lib/report/ticket_reopened.rb index 9b7f7d5a6..405848482 100644 --- a/lib/report/ticket_reopened.rb +++ b/lib/report/ticket_reopened.rb @@ -52,6 +52,14 @@ returns elsif params[:interval] == 'minute' stop = start + 1.minute end + + without_merged_tickets = { + 'ticket_state.name' => { + 'operator' => 'is not', + 'value' => 'merged' + } + } + params[:selector].merge!(without_merged_tickets) # do not show merged tickets in reports count = history_count( object: 'Ticket', type: 'updated', diff --git a/test/integration/report_test.rb b/test/integration/report_test.rb index 1b4a1a57c..0ec471fd4 100644 --- a/test/integration/report_test.rb +++ b/test/integration/report_test.rb @@ -304,7 +304,7 @@ class ReportTest < ActiveSupport::TestCase assert_equal(0, result[7]) assert_equal(0, result[8]) assert_equal(2, result[9]) - assert_equal(2, result[10]) + assert_equal(1, result[10]) assert_equal(0, result[11]) assert_nil(result[12]) @@ -317,8 +317,7 @@ class ReportTest < ActiveSupport::TestCase assert_equal(@ticket5.id, result[:ticket_ids][0]) assert_equal(@ticket6.id, result[:ticket_ids][1]) assert_equal(@ticket7.id, result[:ticket_ids][2]) - assert_equal(@ticket8.id, result[:ticket_ids][3]) - assert_nil(result[:ticket_ids][4]) + assert_nil(result[:ticket_ids][3]) # month - with selector #1 result = Report::TicketFirstSolution.aggs( @@ -425,7 +424,7 @@ class ReportTest < ActiveSupport::TestCase assert_equal(0, result[7]) assert_equal(0, result[8]) assert_equal(1, result[9]) - assert_equal(2, result[10]) + assert_equal(1, result[10]) assert_equal(0, result[11]) assert_nil(result[12]) @@ -442,8 +441,7 @@ class ReportTest < ActiveSupport::TestCase assert(result) assert_equal(@ticket6.id, result[:ticket_ids][0]) assert_equal(@ticket7.id, result[:ticket_ids][1]) - assert_equal(@ticket8.id, result[:ticket_ids][2]) - assert_nil(result[:ticket_ids][3]) + assert_nil(result[:ticket_ids][2]) # week result = Report::TicketFirstSolution.aggs( @@ -929,7 +927,7 @@ class ReportTest < ActiveSupport::TestCase assert_equal(0, result[7]) assert_equal(0, result[8]) assert_equal(6, result[9]) - assert_equal(2, result[10]) + assert_equal(1, result[10]) assert_equal(0, result[11]) assert_nil(result[12]) @@ -940,15 +938,14 @@ class ReportTest < ActiveSupport::TestCase params: { field: 'created_at' }, ) assert(result) - assert_equal(@ticket8.id, result[:ticket_ids][0].to_i) - assert_equal(@ticket7.id, result[:ticket_ids][1].to_i) - assert_equal(@ticket6.id, result[:ticket_ids][2].to_i) - assert_equal(@ticket5.id, result[:ticket_ids][3].to_i) - assert_equal(@ticket4.id, result[:ticket_ids][4].to_i) - assert_equal(@ticket3.id, result[:ticket_ids][5].to_i) - assert_equal(@ticket2.id, result[:ticket_ids][6].to_i) - assert_equal(@ticket1.id, result[:ticket_ids][7].to_i) - assert_nil(result[:ticket_ids][8]) + assert_equal(@ticket7.id, result[:ticket_ids][0].to_i) + assert_equal(@ticket6.id, result[:ticket_ids][1].to_i) + assert_equal(@ticket5.id, result[:ticket_ids][2].to_i) + assert_equal(@ticket4.id, result[:ticket_ids][3].to_i) + assert_equal(@ticket3.id, result[:ticket_ids][4].to_i) + assert_equal(@ticket2.id, result[:ticket_ids][5].to_i) + assert_equal(@ticket1.id, result[:ticket_ids][6].to_i) + assert_nil(result[:ticket_ids][7]) # create at - selector with merge result = Report::TicketGenericTime.aggs(