From 7487639e62315d8900346296dddebc78c7d26eb7 Mon Sep 17 00:00:00 2001
From: larryruili <larryruili@google.com>
Date: Wed, 6 Jun 2018 09:07:21 -0700
Subject: [PATCH] Enhance transfer logic for transaction reporting

Explicit transfer acks/nacks reverse the roles for transaction reporting
tabulation- this adds a quick check to account for this going forward.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=199474444
---
 .../icann/TransactionsReportingQueryBuilder.java      |  3 +++
 .../reporting/icann/sql/transaction_counts.sql        | 11 ++++++++++-
 .../icann/testdata/transaction_counts_test.sql        | 11 ++++++++++-
 .../testdata/transaction_transfer_losing_test.sql     | 11 ++++++++++-
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java b/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java
index bb138dbf7..3034a5b0a 100644
--- a/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java
+++ b/java/google/registry/reporting/icann/TransactionsReportingQueryBuilder.java
@@ -113,6 +113,7 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder {
             .put("EARLIEST_REPORT_TIME", timestampFormatter.print(earliestReportTime))
             .put("LATEST_REPORT_TIME", timestampFormatter.print(latestReportTime))
             .put("CLIENT_ID", "clientId")
+            .put("OTHER_CLIENT_ID", "otherClientId")
             .put("TRANSFER_SUCCESS_FIELD", "TRANSFER_GAINING_SUCCESSFUL")
             .put("TRANSFER_NACKED_FIELD", "TRANSFER_GAINING_NACKED")
             .put("DEFAULT_FIELD", "field")
@@ -127,7 +128,9 @@ public final class TransactionsReportingQueryBuilder implements QueryBuilder {
             .put("HISTORYENTRY_TABLE", "HistoryEntry")
             .put("EARLIEST_REPORT_TIME", timestampFormatter.print(earliestReportTime))
             .put("LATEST_REPORT_TIME", timestampFormatter.print(latestReportTime))
+            // Roles are reversed for losing queries
             .put("CLIENT_ID", "otherClientId")
+            .put("OTHER_CLIENT_ID", "clientId")
             .put("TRANSFER_SUCCESS_FIELD", "TRANSFER_LOSING_SUCCESSFUL")
             .put("TRANSFER_NACKED_FIELD", "TRANSFER_LOSING_NACKED")
             .put("DEFAULT_FIELD", "NULL")
diff --git a/java/google/registry/reporting/icann/sql/transaction_counts.sql b/java/google/registry/reporting/icann/sql/transaction_counts.sql
index e9d730ab4..35c976a34 100644
--- a/java/google/registry/reporting/icann/sql/transaction_counts.sql
+++ b/java/google/registry/reporting/icann/sql/transaction_counts.sql
@@ -45,7 +45,16 @@ FROM (
     SUM(amount) AS metricValue
   FROM (
     SELECT
-      entries.%CLIENT_ID% AS clientId,
+      CASE
+        -- Explicit transfer acks (approve) and nacks (reject) are done
+        -- by the opposing registrar. Thus, for these specific actions,
+        -- we swap the 'otherClientId' with the 'clientId' to properly
+        -- account for this reversal.
+        WHEN (entries.type = 'DOMAIN_TRANSFER_APPROVE'
+          OR entries.type = 'DOMAIN_TRANSFER_REJECT')
+          THEN entries.%OTHER_CLIENT_ID%
+        ELSE entries.%CLIENT_ID%
+      END AS clientId,
       entries.domainTransactionRecords.tld[SAFE_OFFSET(index)] AS tld,
       entries.domainTransactionRecords.reportingTime[SAFE_OFFSET(index)]
           AS reportingTime,
diff --git a/javatests/google/registry/reporting/icann/testdata/transaction_counts_test.sql b/javatests/google/registry/reporting/icann/testdata/transaction_counts_test.sql
index 65f151a19..f7becd54d 100644
--- a/javatests/google/registry/reporting/icann/testdata/transaction_counts_test.sql
+++ b/javatests/google/registry/reporting/icann/testdata/transaction_counts_test.sql
@@ -45,7 +45,16 @@ FROM (
     SUM(amount) AS metricValue
   FROM (
     SELECT
-      entries.clientId AS clientId,
+      CASE
+        -- Explicit transfer acks (approve) and nacks (reject) are done
+        -- by the opposing registrar. Thus, for these specific actions,
+        -- we swap the 'otherClientId' with the 'clientId' to properly
+        -- account for this reversal.
+        WHEN (entries.type = 'DOMAIN_TRANSFER_APPROVE'
+          OR entries.type = 'DOMAIN_TRANSFER_REJECT')
+          THEN entries.otherClientId
+        ELSE entries.clientId
+      END AS clientId,
       entries.domainTransactionRecords.tld[SAFE_OFFSET(index)] AS tld,
       entries.domainTransactionRecords.reportingTime[SAFE_OFFSET(index)]
           AS reportingTime,
diff --git a/javatests/google/registry/reporting/icann/testdata/transaction_transfer_losing_test.sql b/javatests/google/registry/reporting/icann/testdata/transaction_transfer_losing_test.sql
index 3afae8c29..fc60eff27 100644
--- a/javatests/google/registry/reporting/icann/testdata/transaction_transfer_losing_test.sql
+++ b/javatests/google/registry/reporting/icann/testdata/transaction_transfer_losing_test.sql
@@ -45,7 +45,16 @@ FROM (
     SUM(amount) AS metricValue
   FROM (
     SELECT
-      entries.otherClientId AS clientId,
+      CASE
+        -- Explicit transfer acks (approve) and nacks (reject) are done
+        -- by the opposing registrar. Thus, for these specific actions,
+        -- we swap the 'otherClientId' with the 'clientId' to properly
+        -- account for this reversal.
+        WHEN (entries.type = 'DOMAIN_TRANSFER_APPROVE'
+          OR entries.type = 'DOMAIN_TRANSFER_REJECT')
+          THEN entries.clientId
+        ELSE entries.otherClientId
+      END AS clientId,
       entries.domainTransactionRecords.tld[SAFE_OFFSET(index)] AS tld,
       entries.domainTransactionRecords.reportingTime[SAFE_OFFSET(index)]
           AS reportingTime,