MLPAB-2125: Use transactions when creating attorneys (#1244)

internal/app/app.go

@@ -55,6 +55,7 @@ type DynamoClient interface {
 	BatchPut(ctx context.Context, items []interface{}) error
 	OneBySK(ctx context.Context, sk dynamo.SK, v interface{}) error
 	OneByUID(ctx context.Context, uid string, v interface{}) error
+	WriteTransaction(ctx context.Context, transaction *dynamo.Transaction) error
 }
 
 type S3Client interface {
@@ -122,15 +123,15 @@ func App(
 	handleRoot(page.Paths.SignOut, None,
 		page.SignOut(logger, sessionStore, oneLoginClient, appPublicURL))
 	handleRoot(page.Paths.Fixtures, None,
-		fixtures.Donor(tmpls.Get("fixtures.gohtml"), sessionStore, donorStore, certificateProviderStore, attorneyStore, documentStore, eventClient, lpaStoreClient))
+		fixtures.Donor(tmpls.Get("fixtures.gohtml"), sessionStore, donorStore, certificateProviderStore, attorneyStore, documentStore, eventClient, lpaStoreClient, shareCodeStore))
 	handleRoot(page.Paths.CertificateProviderFixtures, None,
 		fixtures.CertificateProvider(tmpls.Get("certificate_provider_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, eventClient, lpaStoreClient, lpaDynamoClient, organisationStore, memberStore))
 	handleRoot(page.Paths.AttorneyFixtures, None,
-		fixtures.Attorney(tmpls.Get("attorney_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, attorneyStore, eventClient, lpaStoreClient, organisationStore, memberStore))
+		fixtures.Attorney(tmpls.Get("attorney_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, attorneyStore, eventClient, lpaStoreClient, organisationStore, memberStore, shareCodeStore))
 	handleRoot(page.Paths.SupporterFixtures, None,
 		fixtures.Supporter(sessionStore, organisationStore, donorStore, memberStore, lpaDynamoClient, searchClient, shareCodeStore, certificateProviderStore, attorneyStore, documentStore, eventClient, lpaStoreClient))
 	handleRoot(page.Paths.DashboardFixtures, None,
-		fixtures.Dashboard(tmpls.Get("dashboard_fixtures.gohtml"), sessionStore, donorStore, certificateProviderStore, attorneyStore))
+		fixtures.Dashboard(tmpls.Get("dashboard_fixtures.gohtml"), sessionStore, donorStore, certificateProviderStore, attorneyStore, shareCodeStore))
 	handleRoot(page.Paths.YourLegalRightsAndResponsibilities, None,
 		page.Guidance(tmpls.Get("your_legal_rights_and_responsibilities_general.gohtml")))
 	handleRoot(page.Paths.Start, None,
@@ -184,15 +185,13 @@ func App(
 
 	attorney.Register(
 		rootMux,
-		logger,
 		tmpls,
 		attorneyTmpls,
 		sessionStore,
 		attorneyStore,
 		oneLoginClient,
 		shareCodeStore,
 		errorHandler,
-		notFoundHandler,
 		dashboardStore,
 		lpaStoreClient,
 		lpaStoreResolvingService,

internal/app/attorney_store.go

@@ -6,7 +6,6 @@ import (
 	"time"
 
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
-	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor/actoruid"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/page"
 )
@@ -16,7 +15,7 @@ type attorneyStore struct {
 	now          func() time.Time
 }
 
-func (s *attorneyStore) Create(ctx context.Context, lpaOwnerKey dynamo.LpaOwnerKeyType, attorneyUID actoruid.UID, isReplacement, isTrustCorporation bool, email string) (*actor.AttorneyProvidedDetails, error) {
+func (s *attorneyStore) Create(ctx context.Context, shareCode actor.ShareCodeData, email string) (*actor.AttorneyProvidedDetails, error) {
 	data, err := page.SessionDataFromContext(ctx)
 	if err != nil {
 		return nil, err
@@ -29,24 +28,26 @@ func (s *attorneyStore) Create(ctx context.Context, lpaOwnerKey dynamo.LpaOwnerK
 	attorney := &actor.AttorneyProvidedDetails{
 		PK:                 dynamo.LpaKey(data.LpaID),
 		SK:                 dynamo.AttorneyKey(data.SessionID),
-		UID:                attorneyUID,
+		UID:                shareCode.ActorUID,
 		LpaID:              data.LpaID,
 		UpdatedAt:          s.now(),
-		IsReplacement:      isReplacement,
-		IsTrustCorporation: isTrustCorporation,
+		IsReplacement:      shareCode.IsReplacementAttorney,
+		IsTrustCorporation: shareCode.IsTrustCorporation,
 		Email:              email,
 	}
 
-	if err := s.dynamoClient.Create(ctx, attorney); err != nil {
-		return nil, err
-	}
-	if err := s.dynamoClient.Create(ctx, lpaLink{
-		PK:        dynamo.LpaKey(data.LpaID),
-		SK:        dynamo.SubKey(data.SessionID),
-		DonorKey:  lpaOwnerKey,
-		ActorType: actor.TypeAttorney,
-		UpdatedAt: s.now(),
-	}); err != nil {
+	transaction := dynamo.NewTransaction().
+		Put(attorney).
+		Put(lpaLink{
+			PK:        dynamo.LpaKey(data.LpaID),
+			SK:        dynamo.SubKey(data.SessionID),
+			DonorKey:  shareCode.LpaOwnerKey,
+			ActorType: actor.TypeAttorney,
+			UpdatedAt: s.now(),
+		}).
+		Delete(shareCode.PK, shareCode.SK)
+
+	if err = s.dynamoClient.WriteTransaction(ctx, transaction); err != nil {
 		return nil, err
 	}
 

internal/app/attorney_store_test.go

@@ -6,12 +6,14 @@ import (
 	"testing"
 	"time"
 
+	"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue"
+	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor/actoruid"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/dynamo"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/page"
 	"github.com/stretchr/testify/assert"
-	mock "github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/mock"
 )
 
 func TestAttorneyStoreCreate(t *testing.T) {
@@ -27,22 +29,61 @@ func TestAttorneyStoreCreate(t *testing.T) {
 
 	for name, tc := range testcases {
 		t.Run(name, func(t *testing.T) {
-			ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123", SessionID: "456"})
+			data := &page.SessionData{LpaID: "123", SessionID: "456"}
+			ctx := page.ContextWithSessionData(context.Background(), data)
 			now := time.Now()
+			nowFormatted := now.Format(time.RFC3339Nano)
 			uid := actoruid.New()
-			details := &actor.AttorneyProvidedDetails{PK: dynamo.LpaKey("123"), SK: dynamo.AttorneyKey("456"), UID: uid, LpaID: "123", UpdatedAt: now, IsReplacement: tc.replacement, IsTrustCorporation: tc.trustCorporation, Email: "[email protected]"}
+			details := &actor.AttorneyProvidedDetails{
+				PK:                 dynamo.LpaKey("123"),
+				SK:                 dynamo.AttorneyKey("456"),
+				UID:                uid,
+				LpaID:              "123",
+				UpdatedAt:          now,
+				IsReplacement:      tc.replacement,
+				IsTrustCorporation: tc.trustCorporation,
+				Email:              "[email protected]",
+			}
+
+			shareCode := actor.ShareCodeData{
+				PK:                    dynamo.ShareKey(dynamo.AttorneyShareKey("123")),
+				SK:                    dynamo.ShareSortKey(dynamo.MetadataKey("123")),
+				ActorUID:              uid,
+				IsReplacementAttorney: tc.replacement,
+				IsTrustCorporation:    tc.trustCorporation,
+				UpdatedAt:             now,
+				LpaOwnerKey:           dynamo.LpaOwnerKey(dynamo.DonorKey("donor")),
+			}
+
+			marshalledAttorney, _ := attributevalue.MarshalMap(details)
+
+			expectedTransaction := &dynamo.Transaction{
+				Puts: []*types.Put{
+					{Item: marshalledAttorney},
+					{Item: map[string]types.AttributeValue{
+						"PK":        &types.AttributeValueMemberS{Value: "LPA#123"},
+						"SK":        &types.AttributeValueMemberS{Value: "SUB#456"},
+						"DonorKey":  &types.AttributeValueMemberS{Value: "DONOR#donor"},
+						"ActorType": &types.AttributeValueMemberN{Value: "2"},
+						"UpdatedAt": &types.AttributeValueMemberS{Value: nowFormatted},
+					}},
+				},
+				Deletes: []*types.Delete{
+					{Key: map[string]types.AttributeValue{
+						"PK": &types.AttributeValueMemberS{Value: shareCode.PK.PK()},
+						"SK": &types.AttributeValueMemberS{Value: shareCode.SK.SK()},
+					}},
+				},
+			}
 
 			dynamoClient := newMockDynamoClient(t)
 			dynamoClient.EXPECT().
-				Create(ctx, details).
-				Return(nil)
-			dynamoClient.EXPECT().
-				Create(ctx, lpaLink{PK: dynamo.LpaKey("123"), SK: dynamo.SubKey("456"), DonorKey: dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), ActorType: actor.TypeAttorney, UpdatedAt: now}).
+				WriteTransaction(ctx, expectedTransaction).
 				Return(nil)
 
 			attorneyStore := &attorneyStore{dynamoClient: dynamoClient, now: func() time.Time { return now }}
 
-			attorney, err := attorneyStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), uid, tc.replacement, tc.trustCorporation, "[email protected]")
+			attorney, err := attorneyStore.Create(ctx, shareCode, "[email protected]")
 			assert.Nil(t, err)
 			assert.Equal(t, details, attorney)
 		})
@@ -54,7 +95,7 @@ func TestAttorneyStoreCreateWhenSessionMissing(t *testing.T) {
 
 	attorneyStore := &attorneyStore{dynamoClient: nil, now: nil}
 
-	_, err := attorneyStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), actoruid.New(), false, false, "")
+	_, err := attorneyStore.Create(ctx, actor.ShareCodeData{}, "")
 	assert.Equal(t, page.SessionMissingError{}, err)
 }
 
@@ -70,49 +111,29 @@ func TestAttorneyStoreCreateWhenSessionDataMissing(t *testing.T) {
 
 			attorneyStore := &attorneyStore{}
 
-			_, err := attorneyStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), actoruid.New(), false, false, "")
+			_, err := attorneyStore.Create(ctx, actor.ShareCodeData{}, "")
 			assert.NotNil(t, err)
 		})
 	}
 }
 
-func TestAttorneyStoreCreateWhenCreateError(t *testing.T) {
+func TestAttorneyStoreCreateWhenWriteTransactionError(t *testing.T) {
 	ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "123", SessionID: "456"})
 	now := time.Now()
 
-	testcases := map[string]func(*testing.T) *mockDynamoClient{
-		"certificate provider record": func(t *testing.T) *mockDynamoClient {
-			dynamoClient := newMockDynamoClient(t)
-			dynamoClient.EXPECT().
-				Create(ctx, mock.Anything).
-				Return(expectedError)
-
-			return dynamoClient
-		},
-		"link record": func(t *testing.T) *mockDynamoClient {
-			dynamoClient := newMockDynamoClient(t)
-			dynamoClient.EXPECT().
-				Create(ctx, mock.Anything).
-				Return(nil).
-				Once()
-			dynamoClient.EXPECT().
-				Create(ctx, mock.Anything).
-				Return(expectedError)
-
-			return dynamoClient
-		},
-	}
+	dynamoClient := newMockDynamoClient(t)
+	dynamoClient.EXPECT().
+		WriteTransaction(mock.Anything, mock.Anything).
+		Return(expectedError)
 
-	for name, makeMockDataStore := range testcases {
-		t.Run(name, func(t *testing.T) {
-			dynamoClient := makeMockDataStore(t)
+	attorneyStore := &attorneyStore{dynamoClient: dynamoClient, now: func() time.Time { return now }}
 
-			attorneyStore := &attorneyStore{dynamoClient: dynamoClient, now: func() time.Time { return now }}
+	_, err := attorneyStore.Create(ctx, actor.ShareCodeData{
+		PK: dynamo.ShareKey(dynamo.AttorneyShareKey("123")),
+		SK: dynamo.ShareSortKey(dynamo.MetadataKey("123")),
+	}, "")
+	assert.Equal(t, expectedError, err)
 
-			_, err := attorneyStore.Create(ctx, dynamo.LpaOwnerKey(dynamo.DonorKey("donor")), actoruid.New(), false, false, "")
-			assert.Equal(t, expectedError, err)
-		})
-	}
 }
 
 func TestAttorneyStoreGet(t *testing.T) {

internal/dynamo/client.go

@@ -426,3 +426,66 @@ func (c *Client) BatchPut(ctx context.Context, values []interface{}) error {
 
 	return err
 }
+
+func (c *Client) WriteTransaction(ctx context.Context, transaction *Transaction) error {
+	if len(transaction.Puts) == 0 && len(transaction.Deletes) == 0 {
+		return errors.New("WriteTransaction requires at least one transaction")
+	}
+
+	if transaction.Errors() != nil {
+		return transaction.Errors()
+	}
+
+	var items []types.TransactWriteItem
+
+	for _, value := range transaction.Puts {
+		value.TableName = aws.String(c.table)
+		items = append(items, types.TransactWriteItem{Put: value})
+	}
+
+	for _, value := range transaction.Deletes {
+		value.TableName = aws.String(c.table)
+		items = append(items, types.TransactWriteItem{Delete: value})
+	}
+
+	_, err := c.svc.TransactWriteItems(ctx, &dynamodb.TransactWriteItemsInput{
+		TransactItems: items,
+	})
+
+	return err
+}
+
+type Transaction struct {
+	Puts    []*types.Put
+	Deletes []*types.Delete
+	Errs    []error
+}
+
+func NewTransaction() *Transaction {
+	return &Transaction{}
+}
+
+func (t *Transaction) Put(v interface{}) *Transaction {
+	values, err := attributevalue.MarshalMap(v)
+
+	if err != nil {
+		t.Errs = append(t.Errs, err)
+	}
+
+	t.Puts = append(t.Puts, &types.Put{Item: values})
+	return t
+}
+
+func (t *Transaction) Delete(pk PK, sk SK) *Transaction {
+	t.Deletes = append(t.Deletes, &types.Delete{
+		Key: map[string]types.AttributeValue{
+			"PK": &types.AttributeValueMemberS{Value: pk.PK()},
+			"SK": &types.AttributeValueMemberS{Value: sk.SK()},
+		},
+	})
+	return t
+}
+
+func (t *Transaction) Errors() error {
+	return errors.Join(t.Errs...)
+}