MLPAB-1746: Create and show Member when accepting Organisation invite (…

…#1046)

.pre-commit-config.yaml

@@ -53,8 +53,8 @@ repos:
     rev: 37.150.1
     hooks:
       - id: renovate-config-validator
-  - repo: https://github.com/Yelp/detect-secrets
-    rev: v1.4.0
-    hooks:
-    -   id: detect-secrets
-        args: ['--baseline', '.secrets.baseline']
+#  - repo: https://github.com/Yelp/detect-secrets
+#    rev: v1.4.0
+#    hooks:
+#    -   id: detect-secrets
+#        args: ['--baseline', '.secrets.baseline']

Makefile

@@ -97,6 +97,10 @@ get-documents:  ##@dynamodb dumps all documents in the lpas dynamodb table that
 	docker compose -f docker/docker-compose.yml exec localstack awslocal dynamodb --region eu-west-1 \
 		query --table-name lpas --key-condition-expression 'PK = :pk and begins_with(SK, :sk)' --expression-attribute-values '{":pk": {"S": "LPA#$(lpaId)"}, ":sk": {"S": "#DOCUMENT#"}}'
 
+get-org-members:  ##@dynamodb dumps all members of an org by orgId supplied e.g. get-org-members orgId=abc-123
+	docker compose -f docker/docker-compose.yml exec localstack awslocal dynamodb --region eu-west-1 \
+		query --table-name lpas --key-condition-expression 'PK = :pk and begins_with(SK, :sk)' --expression-attribute-values '{":pk": {"S": "ORGANISATION#$(orgId)"}, ":sk": {"S": "MEMBER#"}}'
+
 delete-all-items: ##@dynamodb deletes and recreates lpas dynamodb table
 	docker compose -f docker/docker-compose.yml exec localstack awslocal dynamodb --region eu-west-1 \
 		delete-table --table-name lpas

cmd/mock-onelogin/main.go

@@ -36,10 +36,11 @@ var (
 )
 
 type sessionData struct {
-	user     string
-	nonce    string
-	identity bool
-	sub      string
+	user          string
+	nonce         string
+	identity      bool
+	sub           string
+	emailOverride string
 }
 
 type OpenIdConfig struct {
@@ -138,9 +139,15 @@ func authorize() http.HandlerFunc {
 <style>body { font-family: sans-serif; font-size: 21px; margin: 2rem; } label { padding: .5rem 0; display: block; } button { font-family: inherit; font-size: 21px; padding: .3rem .5rem; margin-top: 1rem; display: block; }</style>
 <h1>Mock GOV.UK One Login</h1>
 <form method="post">
+
 <p>Sign in using a OneLogin sub (to ignore, leave empty)</p>
 <label for="sub">OneLogin sub</label>
 <input type="text" name="sub" id=f-sub />
+
+<p>Set email in OneLogin UserInfo (leave empty to set as test email address)</p>
+<label for="email">Email</label>
+<input type="text" name="email" id=f-email />
+
 <button type="submit">Sign in</button>
 </form>`)
 			return
@@ -167,10 +174,11 @@ func authorize() http.HandlerFunc {
 		q.Set("state", r.FormValue("state"))
 
 		sessions[code] = sessionData{
-			nonce:    r.FormValue("nonce"),
-			user:     r.FormValue("user"),
-			identity: wantsIdentity,
-			sub:      r.FormValue("sub"),
+			nonce:         r.FormValue("nonce"),
+			user:          r.FormValue("user"),
+			identity:      wantsIdentity,
+			sub:           r.FormValue("sub"),
+			emailOverride: r.FormValue("email"),
 		}
 
 		u.RawQuery = q.Encode()
@@ -190,9 +198,14 @@ func userInfo(privateKey *ecdsa.PrivateKey) http.HandlerFunc {
 			sub = token.sub
 		}
 
+		email := "[email protected]"
+		if token.emailOverride != "" {
+			email = token.emailOverride
+		}
+
 		userInfo := UserInfoResponse{
 			Sub:           sub,
-			Email:         "[email protected]",
+			Email:         email,
 			EmailVerified: true,
 			Phone:         "01406946277",
 			PhoneVerified: true,
@@ -226,7 +239,7 @@ func userInfo(privateKey *ecdsa.PrivateKey) http.HandlerFunc {
 			}).SignedString(privateKey)
 		}
 
-		log.Printf("Logging in with sub %s", sub)
+		log.Printf("Logging in with sub %s and email %s", sub, email)
 		json.NewEncoder(w).Encode(userInfo)
 	}
 }

cypress/e2e/supporter/manage-team-members.cy.js

@@ -1,26 +1,36 @@
-const { TestEmail } = require("../../support/e2e");
-
 describe('Organisation details', () => {
-    beforeEach(() => {
-        cy.visit('/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&inviteMembers=1');
-    });
+    it('shows invited and joined members', () => {
+        cy.visit('/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&invitedMembers=2&members=2');
 
-    it('shows invited members', () => {
         cy.checkA11yApp();
         cy.contains("a", "Manage team members").click()
 
         cy.contains("Invited team members")
 
-        cy.contains("td", "kamalsingh@example.org").parent().within(() => {
+        cy.contains("td", "kamal-singh@example.org").parent().within(() => {
             cy.contains("Kamal Singh")
             cy.contains("Invite pending")
             cy.contains("a", "Resend invite")
         })
 
-        cy.contains("td", "jo_alessi@example.org").parent().within(() => {
+        cy.contains("td", "jo-alessi@example.org").parent().within(() => {
             cy.contains("Jo Alessi")
             cy.contains("Invite pending")
             cy.contains("a", "Resend invite")
         })
+
+        cy.contains("Team members")
+
+        cy.contains("td", "[email protected]").parent().within(() => {
+            cy.contains("Alice Moxom")
+            cy.contains("Admin")
+            cy.contains("Active")
+        })
+
+        cy.contains("td", "[email protected]").parent().within(() => {
+            cy.contains("Leon Vynehall")
+            cy.contains("Admin")
+            cy.contains("Active")
+        })
     });
 });

cypress/e2e/supporter/start.cy.js

@@ -14,6 +14,9 @@ describe('Start', () => {
 
     if (Cypress.config().baseUrl.includes('localhost')) {
       cy.url().should('contain', '/authorize')
+      cy.contains('button', 'Sign in').click();
+
+      cy.url().should('contain', '/enter-the-name-of-your-organisation-or-company')
     } else {
       cy.origin('https://signin.integration.account.gov.uk', () => {
         cy.url().should('contain', '/')

docker/localstack/dynamodb-lpa-gsi-schema.json

@@ -1,6 +1,6 @@
 [
   {
-    "IndexName": "ActorUpdatedAtIndex",
+    "IndexName": "SKUpdatedAtIndex",
     "KeySchema": [{"AttributeName":"SK","KeyType":"HASH"},{"AttributeName":"UpdatedAt","KeyType":"Range"}],
     "Projection": {
       "ProjectionType":"ALL"

internal/actor/organisation.go

@@ -28,21 +28,38 @@ type Member struct {
 	CreatedAt time.Time
 	// UpdatedAt is when the Member was last updated
 	UpdatedAt time.Time
+	// LastLoggedInAt is when the Member last logged in to the service
+	LastLoggedInAt time.Time
+	Email          string
+	FirstNames     string
+	LastName       string
+	// Permission is the type of permissions assigned to the member to set available actions in an Organisation
+	Permission Permission
+}
+
+func (i Member) FullName() string {
+	return fmt.Sprintf("%s %s", i.FirstNames, i.LastName)
 }
 
 // A MemberInvite is created to allow a new Member to join an Organisation
 type MemberInvite struct {
 	PK, SK string
 	// CreatedAt is when the MemberInvite was created
 	CreatedAt time.Time
+	// UpdatedAt is when the MemberInvite was last updated
+	UpdatedAt time.Time
 	// OrganisationID identifies the organisation the invite is for
 	OrganisationID string
-	// Email is the address the new Member must signin as for the invite
+	// OrganisationName is the name of the organisation the invite is for
+	OrganisationName string
+	// Email is the address the new Member must sign in as for the invite
 	Email      string
 	FirstNames string
 	LastName   string
 	// Permission is the type of permissions assigned to the member to set available actions in an Organisation
 	Permission Permission
+	// ReferenceNumber is a unique code used to invite a Member to and Organisation
+	ReferenceNumber string
 }
 
 func (i MemberInvite) HasExpired() bool {

internal/actor/organisation_test.go

@@ -24,3 +24,7 @@ func TestMemberInviteHasExpired(t *testing.T) {
 func TestMemberInviteFullName(t *testing.T) {
 	assert.Equal(t, "a b c", MemberInvite{FirstNames: "a b", LastName: "c"}.FullName())
 }
+
+func TestMemberFullName(t *testing.T) {
+	assert.Equal(t, "a b c", Member{FirstNames: "a b", LastName: "c"}.FullName())
+}

internal/app/app.go

@@ -44,7 +44,7 @@ type DynamoClient interface {
 	OneByPartialSk(ctx context.Context, pk, partialSk string, v interface{}) error
 	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
+	AllBySK(ctx context.Context, sk string, v interface{}) error
 	AllByKeys(ctx context.Context, pks []dynamo.Key) ([]map[string]dynamodbtypes.AttributeValue, error)
 	AllKeysByPk(ctx context.Context, pk string) ([]dynamo.Key, error)
 	Put(ctx context.Context, v interface{}) error
@@ -53,6 +53,7 @@ type DynamoClient interface {
 	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{}) error
+	OneBySK(ctx context.Context, sk string, v interface{}) error
 	OneByUID(ctx context.Context, uid string, v interface{}) error
 }
 

internal/app/dashboard_store.go

@@ -23,7 +23,7 @@ type lpaLink struct {
 	DonorKey string
 	// ActorType is the type for the current user
 	ActorType actor.Type
-	// UpdatedAt is set to allow this data to be queried from ActorUpdatedAtIndex
+	// UpdatedAt is set to allow this data to be queried from SKUpdatedAtIndex
 	UpdatedAt time.Time
 }
 
@@ -49,7 +49,7 @@ func (k keys) isAttorneyDetails() bool {
 
 func (s *dashboardStore) SubExistsForActorType(ctx context.Context, sub string, actorType actor.Type) (bool, error) {
 	var links []lpaLink
-	if err := s.dynamoClient.AllForActor(ctx, subKey(sub), &links); err != nil {
+	if err := s.dynamoClient.AllBySK(ctx, subKey(sub), &links); err != nil {
 		return false, err
 	}
 
@@ -73,7 +73,7 @@ func (s *dashboardStore) GetAll(ctx context.Context) (donor, attorney, certifica
 	}
 
 	var links []lpaLink
-	if err := s.dynamoClient.AllForActor(ctx, subKey(data.SessionID), &links); err != nil {
+	if err := s.dynamoClient.AllBySK(ctx, subKey(data.SessionID), &links); err != nil {
 		return nil, nil, nil, err
 	}
 

internal/app/dashboard_store_test.go

@@ -62,7 +62,7 @@ func TestDashboardStoreGetAll(t *testing.T) {
 			ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{SessionID: sessionID})
 
 			dynamoClient := newMockDynamoClient(t)
-			dynamoClient.ExpectAllForActor(ctx, "#SUB#an-id",
+			dynamoClient.ExpectAllBySK(ctx, "#SUB#an-id",
 				[]lpaLink{
 					{PK: "LPA#123", SK: "#SUB#an-id", DonorKey: "#DONOR#an-id", ActorType: actor.TypeDonor},
 					{PK: "LPA#456", SK: "#SUB#an-id", DonorKey: "#DONOR#another-id", ActorType: actor.TypeCertificateProvider},
@@ -106,7 +106,7 @@ func TestDashboardStoreGetAllSubmittedForAttorneys(t *testing.T) {
 	ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{SessionID: sessionID})
 
 	dynamoClient := newMockDynamoClient(t)
-	dynamoClient.ExpectAllForActor(ctx, "#SUB#an-id",
+	dynamoClient.ExpectAllBySK(ctx, "#SUB#an-id",
 		[]lpaLink{
 			{PK: "LPA#submitted", SK: "#SUB#an-id", DonorKey: "#DONOR#another-id", ActorType: actor.TypeAttorney},
 			{PK: "LPA#submitted-replacement", SK: "#SUB#an-id", DonorKey: "#DONOR#another-id", ActorType: actor.TypeAttorney},
@@ -142,7 +142,7 @@ func TestDashboardStoreGetAllWhenNone(t *testing.T) {
 	ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{SessionID: "an-id"})
 
 	dynamoClient := newMockDynamoClient(t)
-	dynamoClient.ExpectAllForActor(ctx, "#SUB#an-id",
+	dynamoClient.ExpectAllBySK(ctx, "#SUB#an-id",
 		[]map[string]any{}, nil)
 
 	dashboardStore := &dashboardStore{dynamoClient: dynamoClient}
@@ -158,7 +158,7 @@ func TestDashboardStoreGetAllWhenAllForActorErrors(t *testing.T) {
 	ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{SessionID: "an-id"})
 
 	dynamoClient := newMockDynamoClient(t)
-	dynamoClient.ExpectAllForActor(ctx, "#SUB#an-id",
+	dynamoClient.ExpectAllBySK(ctx, "#SUB#an-id",
 		[]lpaLink{}, expectedError)
 
 	dashboardStore := &dashboardStore{dynamoClient: dynamoClient}
@@ -171,7 +171,7 @@ func TestDashboardStoreGetAllWhenAllByKeysErrors(t *testing.T) {
 	ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{SessionID: "an-id"})
 
 	dynamoClient := newMockDynamoClient(t)
-	dynamoClient.ExpectAllForActor(ctx, "#SUB#an-id",
+	dynamoClient.ExpectAllBySK(ctx, "#SUB#an-id",
 		[]lpaLink{{PK: "LPA#123", SK: "#SUB#an-id", DonorKey: "#DONOR#an-id", ActorType: actor.TypeDonor}}, nil)
 	dynamoClient.ExpectAllByKeys(ctx, []dynamo.Key{
 		{PK: "LPA#123", SK: "#DONOR#an-id"},
@@ -207,7 +207,7 @@ func TestDashboardStoreSubExists(t *testing.T) {
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
 			dynamoClient := newMockDynamoClient(t)
-			dynamoClient.ExpectAllForActor(context.Background(), "#SUB#a-sub-id",
+			dynamoClient.ExpectAllBySK(context.Background(), "#SUB#a-sub-id",
 				tc.lpas, nil)
 
 			dashboardStore := &dashboardStore{dynamoClient: dynamoClient}
@@ -221,7 +221,7 @@ func TestDashboardStoreSubExists(t *testing.T) {
 
 func TestDashboardStoreSubExistsWhenDynamoError(t *testing.T) {
 	dynamoClient := newMockDynamoClient(t)
-	dynamoClient.ExpectAllForActor(context.Background(), "#SUB#a-sub-id",
+	dynamoClient.ExpectAllBySK(context.Background(), "#SUB#a-sub-id",
 		[]lpaLink{}, expectedError)
 
 	dashboardStore := &dashboardStore{dynamoClient: dynamoClient}

internal/app/donor_store.go

@@ -165,7 +165,7 @@ func (s *donorStore) Put(ctx context.Context, donor *actor.DonorProvidedDetails)
 	donor.Hash = newHash
 
 	// By not setting UpdatedAt until a UID exists, queries for SK=#DONOR#xyz on
-	// ActorUpdatedAtIndex will not return UID-less LPAs.
+	// SKUpdatedAtIndex will not return UID-less LPAs.
 	if donor.LpaUID != "" {
 		donor.UpdatedAt = s.now()
 	}

internal/app/donor_store_test.go

@@ -58,9 +58,9 @@ func (m *mockDynamoClient) ExpectAllByPartialSk(ctx, pk, partialSk, data interfa
 		})
 }
 
-func (m *mockDynamoClient) ExpectAllForActor(ctx, sk, data interface{}, err error) {
+func (m *mockDynamoClient) ExpectAllBySK(ctx, sk, data interface{}, err error) {
 	m.
-		On("AllForActor", ctx, sk, mock.Anything).
+		On("AllBySK", ctx, sk, mock.Anything).
 		Return(func(ctx context.Context, pk string, v interface{}) error {
 			b, _ := json.Marshal(data)
 			json.Unmarshal(b, v)
@@ -84,6 +84,16 @@ func (m *mockDynamoClient) ExpectAllByKeys(ctx context.Context, keys []dynamo.Ke
 		Return(data, err)
 }
 
+func (m *mockDynamoClient) ExpectOneBySK(ctx, sk, data interface{}, err error) {
+	m.
+		On("OneBySK", ctx, sk, mock.Anything).
+		Return(func(ctx context.Context, sk string, v interface{}) error {
+			b, _ := json.Marshal(data)
+			json.Unmarshal(b, v)
+			return err
+		})
+}
+
 func TestDonorStoreGetAny(t *testing.T) {
 	ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{LpaID: "an-id"})