MLPAB-1582 Send reduced-fee-requested correctly (#827)

This moves where document stuff is handled to be only happening in UploadEvidence or things it calls directly. It prevents the problem of Sent being set, then not sending the reduced-fee-requested event as no documents need sending.

cmd/event-received/main.go

@@ -116,7 +116,7 @@ func handler(ctx context.Context, event Event) error {
 
 	if event.isS3Event() {
 		s3Client := s3.NewClient(cfg, evidenceBucketName)
-		documentStore := app.NewDocumentStore(dynamoClient, s3Client, random.UuidString)
+		documentStore := app.NewDocumentStore(dynamoClient, nil, nil, nil, nil)
 
 		if err := handleObjectTagsAdded(ctx, dynamoClient, event.S3Event, s3Client, documentStore); err != nil {
 			return fmt.Errorf("ObjectTagging:Put: %w", err)

internal/app/app.go

@@ -9,7 +9,6 @@ import (
 	"time"
 
 	dynamodbtypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
-	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/google/uuid"
 	"github.com/gorilla/sessions"
 	"github.com/ministryofjustice/opg-go-common/logging"
@@ -50,15 +49,15 @@ type DynamoClient interface {
 	DeleteKeys(ctx context.Context, keys []dynamo.Key) error
 	DeleteOne(ctx context.Context, pk, sk string) error
 	Update(ctx context.Context, pk, sk string, values map[string]dynamodbtypes.AttributeValue, expression string) error
-	BatchPut(ctx context.Context, items []interface{}) (int, error)
+	BatchPut(ctx context.Context, items []interface{}) error
 }
 
 //go:generate mockery --testonly --inpackage --name S3Client --structname mockS3Client
 type S3Client interface {
 	PutObject(context.Context, string, []byte) error
-	PutObjectTagging(context.Context, string, []types.Tag) error
 	DeleteObject(context.Context, string) error
 	DeleteObjects(ctx context.Context, keys []string) error
+	PutObjectTagging(context.Context, string, map[string]string) error
 }
 
 //go:generate mockery --testonly --inpackage --name SessionStore --structname mockSessionStore
@@ -87,7 +86,7 @@ func App(
 	s3Client S3Client,
 	eventClient *event.Client,
 ) http.Handler {
-	documentStore := NewDocumentStore(lpaDynamoClient, s3Client, random.UuidString)
+	documentStore := NewDocumentStore(lpaDynamoClient, s3Client, eventClient, random.UuidString, time.Now)
 
 	donorStore := &donorStore{
 		dynamoClient:  lpaDynamoClient,
@@ -186,6 +185,7 @@ func App(
 		evidenceReceivedStore,
 		s3Client,
 		documentStore,
+		eventClient,
 	)
 
 	return withAppData(page.ValidateCsrf(rootMux, sessionStore, random.String, errorHandler), localizer, lang, rumConfig, staticHash, oneloginURL)

internal/app/app_test.go

@@ -8,20 +8,18 @@ import (
 	"github.com/gorilla/sessions"
 	"github.com/ministryofjustice/opg-go-common/logging"
 	"github.com/ministryofjustice/opg-go-common/template"
-	"github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/localize"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/notify"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/onelogin"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/page"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/pay"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/place"
-	"github.com/ministryofjustice/opg-modernising-lpa/internal/s3"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/sesh"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestApp(t *testing.T) {
-	app := App(&logging.Logger{}, &localize.Localizer{}, localize.En, template.Templates{}, nil, &dynamo.Client{}, "http://public.url", &pay.Client{}, &notify.Client{}, &place.Client{}, page.RumConfig{}, "?%3fNEI0t9MN", page.Paths, &onelogin.Client{}, "http://onelogin.url", &s3.Client{}, nil)
+	app := App(&logging.Logger{}, &localize.Localizer{}, localize.En, template.Templates{}, nil, nil, "http://public.url", &pay.Client{}, &notify.Client{}, &place.Client{}, page.RumConfig{}, "?%3fNEI0t9MN", page.Paths, &onelogin.Client{}, "http://onelogin.url", nil, nil)
 
 	assert.Implements(t, (*http.Handler)(nil), app)
 }

internal/app/document_store.go

@@ -3,30 +3,30 @@ package app
 import (
 	"context"
 	"errors"
-	"fmt"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo"
+	"github.com/ministryofjustice/opg-modernising-lpa/internal/event"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/page"
 )
 
-type PartialBatchWriteError struct {
-	Written  int
-	Expected int
-}
-
-func (e PartialBatchWriteError) Error() string {
-	return fmt.Sprintf("Expected to write %d but %d were written", e.Expected, e.Written)
-}
-
 type documentStore struct {
 	dynamoClient DynamoClient
 	s3Client     S3Client
+	eventClient  EventClient
 	randomUUID   func() string
+	now          func() time.Time
 }
 
-func NewDocumentStore(dynamoClient DynamoClient, s3Client S3Client, randomUUID func() string) *documentStore {
-	return &documentStore{dynamoClient: dynamoClient, s3Client: s3Client, randomUUID: randomUUID}
+func NewDocumentStore(dynamoClient DynamoClient, s3Client S3Client, eventClient EventClient, randomUUID func() string, now func() time.Time) *documentStore {
+	return &documentStore{
+		dynamoClient: dynamoClient,
+		s3Client:     s3Client,
+		eventClient:  eventClient,
+		randomUUID:   randomUUID,
+		now:          now,
+	}
 }
 
 func (s *documentStore) Create(ctx context.Context, lpa *page.Lpa, filename string, data []byte) (page.Document, error) {
@@ -79,50 +79,23 @@ func (s *documentStore) UpdateScanResults(ctx context.Context, lpaID, s3ObjectKe
 		"set VirusDetected = :virusDetected, Scanned = :scanned")
 }
 
-func (s *documentStore) BatchPut(ctx context.Context, documents []page.Document) error {
-	var converted []interface{}
-	for _, d := range documents {
-		converted = append(converted, d)
-	}
-
-	toWrite := len(converted)
-	written, err := s.dynamoClient.BatchPut(ctx, converted)
-
-	if err != nil {
-		return err
-	} else if written != toWrite {
-		return PartialBatchWriteError{Written: written, Expected: toWrite}
-	}
-
-	return nil
-}
-
 func (s *documentStore) Put(ctx context.Context, document page.Document) error {
 	return s.dynamoClient.Put(ctx, document)
 }
 
 func (s *documentStore) DeleteInfectedDocuments(ctx context.Context, documents page.Documents) error {
 	var dynamoKeys []dynamo.Key
-	var s3Keys []string
 
 	for _, d := range documents {
 		if d.VirusDetected {
-			dynamoKeys = append(dynamoKeys, dynamo.Key{
-				PK: d.PK,
-				SK: d.SK,
-			})
-			s3Keys = append(s3Keys, d.Key)
+			dynamoKeys = append(dynamoKeys, dynamo.Key{PK: d.PK, SK: d.SK})
 		}
 	}
 
 	if len(dynamoKeys) == 0 {
 		return nil
 	}
 
-	if err := s.s3Client.DeleteObjects(ctx, s3Keys); err != nil {
-		return err
-	}
-
 	return s.dynamoClient.DeleteKeys(ctx, dynamoKeys)
 }
 
@@ -134,6 +107,45 @@ func (s *documentStore) Delete(ctx context.Context, document page.Document) erro
 	return s.dynamoClient.DeleteOne(ctx, document.PK, document.SK)
 }
 
+func (s *documentStore) Submit(ctx context.Context, lpa *page.Lpa, documents page.Documents) error {
+	var unsentDocuments []any
+	var unsentDocumentKeys []string
+
+	tags := map[string]string{
+		"replicate":         "true",
+		"virus-scan-status": "ok",
+	}
+
+	for _, document := range documents {
+		if document.Sent.IsZero() && !document.VirusDetected {
+			document.Sent = s.now()
+			unsentDocuments = append(unsentDocuments, document)
+			unsentDocumentKeys = append(unsentDocumentKeys, document.Key)
+
+			if err := s.s3Client.PutObjectTagging(ctx, document.Key, tags); err != nil {
+				return err
+			}
+		}
+	}
+
+	if len(unsentDocuments) > 0 {
+		if err := s.eventClient.SendReducedFeeRequested(ctx, event.ReducedFeeRequested{
+			UID:              lpa.UID,
+			RequestType:      lpa.FeeType.String(),
+			Evidence:         unsentDocumentKeys,
+			EvidenceDelivery: lpa.EvidenceDelivery.String(),
+		}); err != nil {
+			return err
+		}
+
+		if err := s.dynamoClient.BatchPut(ctx, unsentDocuments); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func documentKey(s3Key string) string {
 	return "#DOCUMENT#" + s3Key
 }