Merge 61b4765 into cb7dde3

.secrets.baseline

@@ -352,5 +352,5 @@
       }
     ]
   },
-  "generated_at": "2023-09-13T16:43:52Z"
+  "generated_at": "2023-10-17T10:56:56Z"
 }

cmd/mlpa/main.go

@@ -15,7 +15,6 @@ import (
 	"github.com/aws/aws-sdk-go-v2/aws"
 	v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
 	"github.com/aws/aws-sdk-go-v2/config"
-	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/gorilla/handlers"
 	"github.com/gorilla/sessions"
 	"github.com/ministryofjustice/opg-go-common/env"
@@ -31,6 +30,7 @@ import (
 	"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/secrets"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/telemetry"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/templatefn"
@@ -204,9 +204,7 @@ func main() {
 
 	uidClient := uid.New(uidBaseURL, httpClient, cfg, v4.NewSigner(), time.Now)
 
-	s3Client := s3.NewFromConfig(cfg, func(o *s3.Options) {
-		o.UsePathStyle = true
-	})
+	evidenceS3Client := s3.NewClient(cfg, evidenceBucketName)
 
 	mux := http.NewServeMux()
 	mux.HandleFunc(page.Paths.HealthCheck.Service.String(), func(w http.ResponseWriter, r *http.Request) {})
@@ -237,8 +235,7 @@ func main() {
 		signInClient,
 		uidClient,
 		oneloginURL,
-		s3Client,
-		evidenceBucketName,
+		evidenceS3Client,
 		eventClient,
 	)))
 	mux.Handle("/", app.App(
@@ -259,8 +256,7 @@ func main() {
 		signInClient,
 		uidClient,
 		oneloginURL,
-		s3Client,
-		evidenceBucketName,
+		evidenceS3Client,
 		eventClient,
 	))
 

cypress/e2e/donor/payment.cy.js

@@ -191,4 +191,27 @@ describe('Pay for LPA', () => {
         cy.url().should('contain', '/what-happens-after-no-fee');
         cy.checkA11yApp();
     })
+
+    it('can only delete evidence that has not been sent to OPG', () => {
+        cy.visit('/fixtures?redirect=/upload-evidence&progress=payForTheLpa&feeType=half-fee');
+        cy.checkA11yApp();
+
+        cy.get('input[type="file"]').attachFile(['dummy.pdf']);
+
+        cy.contains('button', 'Upload files').click()
+
+        cy.url().should('contain', '/upload-evidence');
+
+        cy.get('.govuk-summary-list').within(() => {
+            cy.contains('supporting-evidence.png').parent().should('not.contain', 'Delete');
+            cy.contains('dummy.pdf').parent().contains('button', 'Delete').click();
+        });
+
+        cy.url().should('contain', '/upload-evidence');
+        cy.checkA11yApp();
+
+        cy.get('.moj-banner').within(() => {
+            cy.contains('You have deleted file dummy.pdf');
+        });
+    })
 });

internal/app/app.go

@@ -8,8 +8,8 @@ import (
 	"strings"
 	"time"
 
-	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
-	"github.com/aws/aws-sdk-go-v2/service/s3"
+	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"
@@ -45,11 +45,18 @@ type DynamoClient interface {
 	AllByPartialSk(ctx context.Context, pk, partialSk string, v interface{}) error
 	LatestForActor(ctx context.Context, sk string, v interface{}) error
 	AllForActor(ctx context.Context, sk string, v interface{}) error
-	AllByKeys(ctx context.Context, pks []dynamo.Key) ([]map[string]types.AttributeValue, error)
+	AllByKeys(ctx context.Context, pks []dynamo.Key) ([]map[string]dynamodbtypes.AttributeValue, error)
 	Put(ctx context.Context, v interface{}) error
 	Create(ctx context.Context, v 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
+}
+
 //go:generate mockery --testonly --inpackage --name SessionStore --structname mockSessionStore
 type SessionStore interface {
 	Get(r *http.Request, name string) (*sessions.Session, error)
@@ -75,8 +82,7 @@ func App(
 	oneLoginClient *onelogin.Client,
 	uidClient *uid.Client,
 	oneloginURL string,
-	s3Client *s3.Client,
-	evidenceBucketName string,
+	s3Client S3Client,
 	eventClient *event.Client,
 ) http.Handler {
 	donorStore := &donorStore{
@@ -173,7 +179,6 @@ func App(
 		notifyClient,
 		evidenceReceivedStore,
 		s3Client,
-		evidenceBucketName,
 	)
 
 	return withAppData(page.ValidateCsrf(rootMux, sessionStore, random.String, errorHandler), localizer, lang, rumConfig, staticHash, oneloginURL)

internal/app/app_test.go

@@ -5,7 +5,6 @@ import (
 	"net/http/httptest"
 	"testing"
 
-	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/gorilla/sessions"
 	"github.com/ministryofjustice/opg-go-common/logging"
 	"github.com/ministryofjustice/opg-go-common/template"
@@ -17,13 +16,14 @@ import (
 	"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/ministryofjustice/opg-modernising-lpa/internal/uid"
 	"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{}, &identity.YotiClient{}, &notify.Client{}, &place.Client{}, page.RumConfig{}, "?%3fNEI0t9MN", page.Paths, &onelogin.Client{}, &uid.Client{}, "http://onelogin.url", &s3.Client{}, "bucket", nil)
+	app := App(&logging.Logger{}, &localize.Localizer{}, localize.En, template.Templates{}, nil, &dynamo.Client{}, "http://public.url", &pay.Client{}, &identity.YotiClient{}, &notify.Client{}, &place.Client{}, page.RumConfig{}, "?%3fNEI0t9MN", page.Paths, &onelogin.Client{}, &uid.Client{}, "http://onelogin.url", &s3.Client{}, nil)
 
 	assert.Implements(t, (*http.Handler)(nil), app)
 }

internal/app/donor_store.go

@@ -174,7 +174,7 @@ func (s *donorStore) Put(ctx context.Context, lpa *page.Lpa) error {
 	if lpa.UID != "" && lpa.Tasks.PayForLpa.IsPending() && lpa.HasUnsentReducedFeesEvidence() {
 		var unsentKeys []string
 
-		for _, evidence := range lpa.EvidenceKeys {
+		for _, evidence := range lpa.Evidence {
 			if evidence.Sent.IsZero() {
 				unsentKeys = append(unsentKeys, evidence.Key)
 			}
@@ -187,9 +187,9 @@ func (s *donorStore) Put(ctx context.Context, lpa *page.Lpa) error {
 		}); err != nil {
 			s.logger.Print(err)
 		} else {
-			for i, evidence := range lpa.EvidenceKeys {
+			for i, evidence := range lpa.Evidence {
 				if evidence.Sent.IsZero() {
-					lpa.EvidenceKeys[i].Sent = s.now()
+					lpa.Evidence[i].Sent = s.now()
 				}
 			}
 		}

internal/app/donor_store_test.go

@@ -476,7 +476,7 @@ func TestDonorStorePutWhenReducedFeeRequested(t *testing.T) {
 			UID:                            "M-1111",
 			UpdatedAt:                      now,
 			FeeType:                        page.HalfFee,
-			EvidenceKeys:                   []page.Evidence{{Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf", Sent: now}},
+			Evidence:                       []page.Evidence{{Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf", Sent: now}},
 			Tasks:                          page.Tasks{PayForLpa: actor.PaymentTaskPending},
 			HasSentApplicationUpdatedEvent: true,
 		}).
@@ -499,7 +499,7 @@ func TestDonorStorePutWhenReducedFeeRequested(t *testing.T) {
 		ID:                             "5",
 		UID:                            "M-1111",
 		FeeType:                        page.HalfFee,
-		EvidenceKeys:                   []page.Evidence{{Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf"}},
+		Evidence:                       []page.Evidence{{Key: "lpa-uid-evidence-a-uid", Filename: "whatever.pdf"}},
 		Tasks:                          page.Tasks{PayForLpa: actor.PaymentTaskPending},
 		HasSentApplicationUpdatedEvent: true,
 	})
@@ -519,7 +519,7 @@ func TestDonorStorePutWhenReducedFeeRequestedSentAndUnsentFees(t *testing.T) {
 			UID:       "M-1111",
 			UpdatedAt: now,
 			FeeType:   page.HalfFee,
-			EvidenceKeys: []page.Evidence{
+			Evidence: []page.Evidence{
 				{Key: "lpa-uid-evidence-a-uid-1", Filename: "whatever.pdf", Sent: now},
 				{Key: "lpa-uid-evidence-a-uid-2", Filename: "whenever.pdf", Sent: now},
 				{Key: "lpa-uid-evidence-a-uid-3", Filename: "whoever.pdf", Sent: now},
@@ -546,7 +546,7 @@ func TestDonorStorePutWhenReducedFeeRequestedSentAndUnsentFees(t *testing.T) {
 		ID:      "5",
 		UID:     "M-1111",
 		FeeType: page.HalfFee,
-		EvidenceKeys: []page.Evidence{
+		Evidence: []page.Evidence{
 			{Key: "lpa-uid-evidence-a-uid-1", Filename: "whatever.pdf"},
 			{Key: "lpa-uid-evidence-a-uid-2", Filename: "whenever.pdf", Sent: now},
 			{Key: "lpa-uid-evidence-a-uid-3", Filename: "whoever.pdf"},
@@ -574,7 +574,7 @@ func TestDonorStorePutWhenReducedFeeRequestedWontResend(t *testing.T) {
 		ID:                             "5",
 		UID:                            "M-1111",
 		Tasks:                          page.Tasks{PayForLpa: actor.PaymentTaskPending},
-		EvidenceKeys:                   []page.Evidence{{Key: "lpa-uid-evidence-a-uid-1", Filename: "whatever.pdf", Sent: now}},
+		Evidence:                       []page.Evidence{{Key: "lpa-uid-evidence-a-uid-1", Filename: "whatever.pdf", Sent: now}},
 		HasSentApplicationUpdatedEvent: true,
 	})
 	assert.Nil(t, err)
@@ -592,7 +592,7 @@ func TestDonorStorePutWhenReducedFeeRequestedWhenError(t *testing.T) {
 			ID:                             "5",
 			UID:                            "M-1111",
 			Tasks:                          page.Tasks{PayForLpa: actor.PaymentTaskPending},
-			EvidenceKeys:                   []page.Evidence{{Sent: now}, {}},
+			Evidence:                       []page.Evidence{{Sent: now}, {}},
 			UpdatedAt:                      now,
 			HasSentApplicationUpdatedEvent: true,
 		}).
@@ -615,7 +615,7 @@ func TestDonorStorePutWhenReducedFeeRequestedWhenError(t *testing.T) {
 		ID:                             "5",
 		UID:                            "M-1111",
 		Tasks:                          page.Tasks{PayForLpa: actor.PaymentTaskPending},
-		EvidenceKeys:                   []page.Evidence{{Sent: now}, {}},
+		Evidence:                       []page.Evidence{{Sent: now}, {}},
 		HasSentApplicationUpdatedEvent: true,
 	})
 	assert.Nil(t, err)

internal/page/data.go

@@ -181,13 +181,46 @@ type Lpa struct {
 
 	// FeeType is the type of fee the user is applying for
 	FeeType FeeType
-	// EvidenceKeys is the S3 keys for uploaded evidence with a record of when it's been sent to caseworkers
-	EvidenceKeys []Evidence
+	// Evidence is the documents uploaded by a donor to apply for non-full fees
+	Evidence Evidences
 
 	HasSentApplicationUpdatedEvent        bool
 	HasSentPreviousApplicationLinkedEvent bool
 }
 
+type Evidences []Evidence
+
+func (es *Evidences) Delete(evidenceKey string) bool {
+	idx := slices.IndexFunc(*es, func(e Evidence) bool { return e.Key == evidenceKey })
+	if idx == -1 {
+		return false
+	}
+
+	*es = slices.Delete(*es, idx, idx+1)
+
+	return true
+}
+
+func (es *Evidences) Keys() []string {
+	var keys []string
+
+	for _, e := range *es {
+		keys = append(keys, e.Key)
+	}
+
+	return keys
+}
+
+func (es *Evidences) GetByKey(key string) Evidence {
+	for _, e := range *es {
+		if e.Key == key {
+			return e
+		}
+	}
+
+	return Evidence{}
+}
+
 type Evidence struct {
 	Key      string
 	Filename string
@@ -504,7 +537,7 @@ func (l *Lpa) FeeAmount() int {
 }
 
 func (l *Lpa) HasUnsentReducedFeesEvidence() bool {
-	for _, evidence := range l.EvidenceKeys {
+	for _, evidence := range l.Evidence {
 		if evidence.Sent.IsZero() {
 			return true
 		}