Browse Source

Fixes and improves reporting in #1809 by not showing merge but still use user's input

Muhammad Nuzaihan 7 years ago
parent
commit
d33c80bbaa

+ 7 - 49
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

+ 17 - 1
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],

+ 16 - 0
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?

+ 14 - 0
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

+ 8 - 0
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',

+ 13 - 16
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(