From 07554d7710dd326af78438109aa839829549d51f Mon Sep 17 00:00:00 2001
From: Jason Bornstein <131717043+jasonbornsteinMOOV@users.noreply.github.com>
Date: Tue, 1 Aug 2023 14:13:41 -0400
Subject: [PATCH] review fixes

---
 README.md                        | 12 +++++++++++-
 pkg/response/match/matcher.go    |  6 +++---
 pkg/service/config_test.go       |  2 ++
 pkg/service/environment.go       |  5 ++---
 pkg/service/model_config.go      | 21 +++++++++++----------
 pkg/service/model_config_test.go |  2 ++
 6 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/README.md b/README.md
index 1ba4bdb8..6141e3fd 100644
--- a/README.md
+++ b/README.md
@@ -103,7 +103,17 @@ match:
   routingNumber: <string>  # Exact match of ABA routing number (RDFIIdentification and CheckDigit)
   traceNumber: <string>    # Exact match of TraceNumber
   entryType: <string>      # Checks TransactionCode. Accepted values: credit, debit or prenote.
-# at most, one `copy` action and one other action will be returned
+# Matching will find at most 2 Actions: 1 Copy Action and 1 Return/Correction Action.
+# If the Return/Correction Action as no Delay, the Copy Action will be excluded.
+# Valid combinations include:
+#  1. Copy
+#  2. Return/Correction w/ Delay
+#  3. Return/Correction w/o Delay
+#  4. Copy and Return/Correction w/ Delay
+#  5. Nothing
+# Invalid combinations are:
+#  1. Copy + Return/Correction w/o Delay
+#  2. Copy w/ Delay (validated when reading configuration)
 action:
   # How long into the future should we wait before making the correction/return available?
   delay: <duration>
diff --git a/pkg/response/match/matcher.go b/pkg/response/match/matcher.go
index b046804c..94279ad6 100644
--- a/pkg/response/match/matcher.go
+++ b/pkg/response/match/matcher.go
@@ -40,9 +40,9 @@ func (m Matcher) FindAction(ed *ach.EntryDetail) (copyAction *service.Action, pr
 	 * Valid combinations include:
 	 * 1. Copy
 	 * 2. Return/Correction w/ Delay
-	 * 2. Return/Correction w/o Delay
-	 * 3. Copy and Return/Correction w/ Delay
-	 * 4. Nothing
+	 * 3. Return/Correction w/o Delay
+	 * 4. Copy and Return/Correction w/ Delay
+	 * 5. Nothing
 	 *
 	 * Invalid combinations are:
 	 * 1. Copy + Return/Correction w/o Delay
diff --git a/pkg/service/config_test.go b/pkg/service/config_test.go
index d80275e3..2a72d8bb 100644
--- a/pkg/service/config_test.go
+++ b/pkg/service/config_test.go
@@ -21,3 +21,5 @@ func Test_ConfigLoading(t *testing.T) {
 	err := ConfigService.Load(gc)
 	require.Nil(t, err)
 }
+
+// TODO JB: tests
diff --git a/pkg/service/environment.go b/pkg/service/environment.go
index 898c4f5d..3984b4e0 100644
--- a/pkg/service/environment.go
+++ b/pkg/service/environment.go
@@ -3,7 +3,6 @@
 package service
 
 import (
-	"errors"
 	ftp "goftp.io/server/core"
 
 	"github.com/gorilla/mux"
@@ -61,8 +60,8 @@ func LoadConfig(logger log.Logger) (*Config, error) {
 	if err := configService.Load(global); err != nil {
 		return nil, err
 	}
-	if !global.Validate() {
-		return nil, errors.New("invalid config")
+	if err := global.Validate(); err != nil {
+		return nil, err
 	}
 
 	cfg := &global.ACHTestHarness
diff --git a/pkg/service/model_config.go b/pkg/service/model_config.go
index f9fa5905..21219c1e 100644
--- a/pkg/service/model_config.go
+++ b/pkg/service/model_config.go
@@ -3,6 +3,7 @@
 package service
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
@@ -13,7 +14,7 @@ type GlobalConfig struct {
 	ACHTestHarness Config
 }
 
-func (gc *GlobalConfig) Validate() bool {
+func (gc *GlobalConfig) Validate() error {
 	return gc.ACHTestHarness.Validate()
 }
 
@@ -25,14 +26,14 @@ type Config struct {
 	Responses    []Response
 }
 
-func (cfg *Config) Validate() bool {
+func (cfg *Config) Validate() error {
 	for i := range cfg.Responses {
-		if !cfg.Responses[i].Validate() {
-			return false
+		if err := cfg.Responses[i].Validate(); err != nil {
+			return err
 		}
 	}
 
-	return true
+	return nil
 }
 
 func (cfg *Config) responsePaths() []string {
@@ -93,7 +94,7 @@ type Response struct {
 	Action Action
 }
 
-func (r *Response) Validate() bool {
+func (r *Response) Validate() error {
 	return r.Action.Validate()
 }
 
@@ -141,10 +142,10 @@ type Action struct {
 	Return     *Return
 }
 
-func (a *Action) Validate() bool {
+func (a *Action) Validate() error {
 	// Delay is only valid for Return and Correction
 	if a.Delay != nil && a.Copy != nil {
-		return false
+		return errors.New("Delay and Copy are not valid together in an Action")
 	}
 
 	// only allowed 1 of Copy, Return, Correction to be configured
@@ -159,10 +160,10 @@ func (a *Action) Validate() bool {
 		count++
 	}
 	if count > 1 {
-		return false
+		return errors.New("only 1 of Copy, Return, Correction can be configured in an Action")
 	}
 
-	return true
+	return nil
 }
 
 type Copy struct {
diff --git a/pkg/service/model_config_test.go b/pkg/service/model_config_test.go
index 9577119a..f308a32c 100644
--- a/pkg/service/model_config_test.go
+++ b/pkg/service/model_config_test.go
@@ -31,3 +31,5 @@ func TestConfig__Return(t *testing.T) {
 	r.Code = "R99"
 	require.Error(t, r.Validate())
 }
+
+// TODO JB: tests