MLPAB-1855: Enable non-admins to edit details (#1071)

ministryofjustice · Feb 23, 2024 · 90d8377 · 90d8377
1 parent 79b359a
commit 90d8377
Show file tree

Hide file tree

Showing 22 changed files with 348 additions and 241 deletions.
diff --git a/cypress/e2e/supporter/edit-member.cy.js b/cypress/e2e/supporter/edit-member.cy.js
@@ -1,60 +1,111 @@
 describe('Edit member', () => {
-    beforeEach(() => {
-        cy.visit("/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&members=1");
+    describe('admin', () => {
+        it('can edit a team members name', () => {
+            cy.visit("/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&members=1&permission=admin");
 
-        cy.url().should('contain', "/manage-organisation/manage-team-members");
-        cy.contains('a', "Alice Moxom").click()
+            cy.url().should('contain', "/manage-organisation/manage-team-members");
+            cy.contains('a', "Alice Moxom").click()
 
-        cy.url().should('contain', "/manage-organisation/manage-team-members/edit-team-member");
-    });
+            cy.url().should('contain', "/manage-organisation/manage-team-members/edit-team-member");
 
-    it('can edit a team members name', () => {
-        cy.checkA11yApp();
+            cy.checkA11yApp();
 
-        cy.get('#f-first-names').clear().type('John');
-        cy.get('#f-last-name').clear().type('Doe');
+            cy.get('#f-first-names').clear().type('John');
+            cy.get('#f-last-name').clear().type('Doe');
 
-        cy.contains('button', "Save").click()
+            cy.contains('button', "Save").click()
 
-        cy.url().should('contain', "/manage-organisation/manage-team-members");
+            cy.url().should('contain', "/manage-organisation/manage-team-members");
 
-        cy.contains('Team member’s name updated to John Doe');
-        cy.contains('a', "John Doe")
-    })
+            cy.checkA11yApp();
+
+            cy.contains('Team member’s name updated to John Doe');
+            cy.contains('a', "John Doe")
+        })
+
+        it('can edit own name', () => {
+            cy.visit("/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&members=1&[email protected]&permission=admin");
+
+            cy.url().should('contain', "/manage-organisation/manage-team-members");
+            cy.contains('a', "Alice Moxom").click()
+
+            cy.url().should('contain', "/manage-organisation/manage-team-members/edit-team-member");
 
-    it('can edit own name', () => {
-        // TODO update to a full test when admins can set their own names during org creation
-        cy.visit("/supporter/manage-organisation/manage-team-members?nameUpdated=John+Doe&selfUpdated=1");
+            cy.checkA11yApp();
 
-        cy.contains('Your name has been updated to John Doe');
+            cy.get('#f-first-names').clear().type('John');
+            cy.get('#f-last-name').clear().type('Doe');
+
+            cy.contains('button', "Save").click()
+
+            cy.url().should('contain', "/manage-organisation/manage-team-members");
+
+            cy.checkA11yApp();
+
+            cy.contains('Your name has been updated to John Doe');
+            cy.contains('a', "John Doe")
+        })
     })
 
-    it('errors when empty', () => {
-        cy.get('#f-first-names').clear();
-        cy.get('#f-last-name').clear();
+    describe('non-admin', () => {
+        it('can edit own name', () => {
+            cy.visit("/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&members=1&[email protected]");
+
+            cy.contains('a', 'Manage your details').click();
+            cy.url().should('contain', "/manage-organisation/manage-team-members/edit-team-member");
+
+            cy.checkA11yApp();
+            cy.contains('Your name');
 
-        cy.contains('button', "Save").click()
+            cy.get('#f-first-names').clear ().type('John');
+            cy.get('#f-last-name').clear().type('Doe');
 
-        cy.checkA11yApp();
+            cy.contains('button', "Save").click()
 
-        cy.get('.govuk-error-summary').within(() => {
-            cy.contains('Enter first names');
-            cy.contains('Enter last name');
+            cy.url().should('contain', "/dashboard");
+
+            cy.checkA11yApp();
+            cy.contains('Your name has been updated to John Doe');
+        })
+    })
+
+    describe('errors', () => {
+        beforeEach(() => {
+            cy.visit("/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&members=1");
+
+            cy.url().should('contain', "/manage-organisation/manage-team-members");
+            cy.contains('a', "Alice Moxom").click()
+
+            cy.url().should('contain', "/manage-organisation/manage-team-members/edit-team-member");
         });
 
-        cy.contains('[for=f-first-names] + .govuk-error-message', 'Enter first names');
-        cy.contains('[for=f-last-name] + .govuk-error-message', 'Enter last name');
-    });
+        it('errors when empty', () => {
+            cy.get('#f-first-names').clear();
+            cy.get('#f-last-name').clear();
 
-    it('errors when names too long', () => {
-        cy.get('#f-first-names').invoke('val', 'a '.repeat(54));
-        cy.get('#f-last-name').invoke('val', 'b '.repeat(62));
+            cy.contains('button', "Save").click()
 
-        cy.contains('button', "Save").click()
+            cy.checkA11yApp();
 
-        cy.checkA11yApp();
+            cy.get('.govuk-error-summary').within(() => {
+                cy.contains('Enter first names');
+                cy.contains('Enter last name');
+            });
 
-        cy.contains('[for=f-first-names] + .govuk-error-message', 'First names must be 53 characters or less');
-        cy.contains('[for=f-last-name] + .govuk-error-message', 'Last name must be 61 characters or less');
+            cy.contains('[for=f-first-names] + .govuk-error-message', 'Enter first names');
+            cy.contains('[for=f-last-name] + .govuk-error-message', 'Enter last name');
+        });
+
+        it('errors when names too long', () => {
+            cy.get('#f-first-names').invoke('val', 'a '.repeat(54));
+            cy.get('#f-last-name').invoke('val', 'b '.repeat(62));
+
+            cy.contains('button', "Save").click()
+
+            cy.checkA11yApp();
+
+            cy.contains('[for=f-first-names] + .govuk-error-message', 'First names must be 53 characters or less');
+            cy.contains('[for=f-last-name] + .govuk-error-message', 'Last name must be 61 characters or less');
+        });
     });
 })
diff --git a/cypress/e2e/supporter/manage-team-members.cy.js b/cypress/e2e/supporter/manage-team-members.cy.js
@@ -1,6 +1,6 @@
 describe('Organisation details', () => {
     it('shows invited and joined members', () => {
-        cy.visit('/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&invitedMembers=2&members=2');
+        cy.visit('/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&invitedMembers=2&members=2&permission=admin');
 
         cy.checkA11yApp();
         cy.contains("a", "Manage team members").click()

diff --git a/internal/app/organisation_store.go b/internal/app/organisation_store.go
@@ -57,11 +57,12 @@ func (s *organisationStore) Create(ctx context.Context, name string) (*actor.Org
 	}
 
 	member := &actor.Member{
-		PK:        organisationKey(organisationID),
-		SK:        memberKey(data.SessionID),
-		ID:        s.uuidString(),
-		Email:     data.Email,
-		CreatedAt: s.now(),
+		PK:         organisationKey(organisationID),
+		SK:         memberKey(data.SessionID),
+		ID:         s.uuidString(),
+		Email:      data.Email,
+		CreatedAt:  s.now(),
+		Permission: actor.Admin,
 	}
 
 	if err := s.dynamoClient.Create(ctx, member); err != nil {

diff --git a/internal/app/organisation_store_test.go b/internal/app/organisation_store_test.go
@@ -27,11 +27,12 @@ func TestOrganisationStoreCreate(t *testing.T) {
 
 	dynamoClient.EXPECT().
 		Create(ctx, &actor.Member{
-			PK:        "ORGANISATION#a-uuid",
-			SK:        "MEMBER#an-id",
-			ID:        "a-uuid",
-			CreatedAt: testNow,
-			Email:     "[email protected]",
+			PK:         "ORGANISATION#a-uuid",
+			SK:         "MEMBER#an-id",
+			ID:         "a-uuid",
+			CreatedAt:  testNow,
+			Email:      "[email protected]",
+			Permission: actor.Admin,
 		}).
 		Return(nil).
 		Once()

diff --git a/internal/page/app_data.go b/internal/page/app_data.go
@@ -26,6 +26,8 @@ type AppData struct {
 	OrganisationName     string
 	IsManageOrganisation bool
 	LoginSessionEmail    string
+	Permission           actor.Permission
+	LoggedInSupporterID  string
 }
 
 func (d AppData) Redirect(w http.ResponseWriter, r *http.Request, url string) error {
@@ -58,3 +60,7 @@ func (d AppData) IsReplacementAttorney() bool {
 func (d AppData) IsTrustCorporation() bool {
 	return d.ActorType == actor.TypeTrustCorporation || d.ActorType == actor.TypeReplacementTrustCorporation
 }
+
+func (d AppData) IsAdmin() bool {
+	return d.Permission.IsAdmin()
+}
diff --git a/internal/page/app_data_test.go b/internal/page/app_data_test.go
@@ -71,3 +71,8 @@ func TestIsTrustCorporation(t *testing.T) {
 	assert.False(t, AppData{ActorType: actor.TypeAttorney, AttorneyUID: actoruid.New()}.IsTrustCorporation())
 	assert.False(t, AppData{ActorType: actor.TypeReplacementAttorney, AttorneyUID: actoruid.New()}.IsTrustCorporation())
 }
+
+func TestAppDataIsAdmin(t *testing.T) {
+	assert.True(t, AppData{Permission: actor.Admin}.IsAdmin())
+	assert.False(t, AppData{}.IsAdmin())
+}
diff --git a/internal/page/fixtures/supporter.go b/internal/page/fixtures/supporter.go
@@ -33,16 +33,18 @@ func Supporter(sessionStore sesh.Store, organisationStore OrganisationStore, don
 			members        = r.FormValue("members")
 			organisation   = r.FormValue("organisation")
 			redirect       = r.FormValue("redirect")
+			asMember       = r.FormValue("asMember")
+			permission     = r.FormValue("permission")
 
 			supporterSub       = random.String(16)
 			supporterSessionID = base64.StdEncoding.EncodeToString([]byte(supporterSub))
-			ctx                = page.ContextWithSessionData(r.Context(), &page.SessionData{SessionID: supporterSessionID, Email: testEmail})
+			supporterCtx       = page.ContextWithSessionData(r.Context(), &page.SessionData{SessionID: supporterSessionID, Email: testEmail})
 		)
 
 		loginSession := &sesh.LoginSession{Sub: supporterSub, Email: testEmail}
 
 		if organisation == "1" {
-			org, err := organisationStore.Create(ctx, random.String(12))
+			org, err := organisationStore.Create(supporterCtx, random.String(12))
 			if err != nil {
 				return err
 			}
@@ -87,29 +89,50 @@ func Supporter(sessionStore sesh.Store, organisationStore OrganisationStore, don
 
 			if members != "" {
 				n, err := strconv.Atoi(members)
+				if err != nil {
+					return fmt.Errorf("members should be a number")
+				}
+
+				memberEmailSub := make(map[string]string)
+
+				permission, err := actor.ParsePermission(permission)
+				if err != nil {
+					permission = actor.None
+				}
 
 				for i, member := range orgMemberNames {
 					if i == n {
 						break
 					}
 
+					email := strings.ToLower(fmt.Sprintf("%s-%[email protected]", member.Firstnames, member.Lastname))
+					sub := []byte(random.String(16))
+					memberCtx := page.ContextWithSessionData(r.Context(), &page.SessionData{SessionID: base64.StdEncoding.EncodeToString(sub), Email: email})
+
 					if err = memberStore.Create(
-						page.ContextWithSessionData(r.Context(), &page.SessionData{SessionID: random.String(12)}),
+						memberCtx,
 						&actor.MemberInvite{
 							PK:              random.String(12),
 							SK:              random.String(12),
 							CreatedAt:       time.Now(),
 							UpdatedAt:       time.Now(),
 							OrganisationID:  org.ID,
-							Email:           strings.ToLower(fmt.Sprintf("%s-%[email protected]", member.Firstnames, member.Lastname)),
+							Email:           email,
 							FirstNames:      member.Firstnames,
 							LastName:        member.Lastname,
-							Permission:      actor.Admin,
+							Permission:      permission,
 							ReferenceNumber: random.String(12),
 						},
 					); err != nil {
 						return err
 					}
+
+					memberEmailSub[email] = string(sub)
+				}
+
+				if sub, found := memberEmailSub[asMember]; found {
+					loginSession.Email = asMember
+					loginSession.Sub = sub
 				}
 			}
 		}

diff --git a/internal/page/paths.go b/internal/page/paths.go
@@ -184,7 +184,10 @@ func (p SupporterPath) RedirectQuery(w http.ResponseWriter, r *http.Request, app
 }
 
 func (p SupporterPath) IsManageOrganisation() bool {
-	return p == Paths.Supporter.OrganisationDetails || p == Paths.Supporter.EditOrganisationName || p == Paths.Supporter.ManageTeamMembers
+	return p == Paths.Supporter.OrganisationDetails ||
+		p == Paths.Supporter.EditOrganisationName ||
+		p == Paths.Supporter.ManageTeamMembers ||
+		p == Paths.Supporter.EditMember
 }
 
 type AttorneyPaths struct {

diff --git a/internal/page/paths_test.go b/internal/page/paths_test.go
@@ -204,7 +204,11 @@ func TestSupporterPathRedirect(t *testing.T) {
 
 func TestSupporterPathIsManageOrganisation(t *testing.T) {
 	assert.False(t, Paths.Supporter.Dashboard.IsManageOrganisation())
+
 	assert.True(t, Paths.Supporter.OrganisationDetails.IsManageOrganisation())
+	assert.True(t, Paths.Supporter.EditOrganisationName.IsManageOrganisation())
+	assert.True(t, Paths.Supporter.ManageTeamMembers.IsManageOrganisation())
+	assert.True(t, Paths.Supporter.EditMember.IsManageOrganisation())
 }
 
 func TestCanGoTo(t *testing.T) {

diff --git a/internal/page/supporter/dashboard.go b/internal/page/supporter/dashboard.go
@@ -2,6 +2,7 @@ package supporter
 
 import (
 	"net/http"
+	"net/url"
 
 	"github.com/ministryofjustice/opg-go-common/template"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
@@ -13,6 +14,7 @@ type dashboardData struct {
 	App    page.AppData
 	Errors validation.List
 	Donors []actor.DonorProvidedDetails
+	Query  url.Values
 }
 
 func Dashboard(tmpl template.Template, organisationStore OrganisationStore) Handler {
@@ -22,6 +24,6 @@ func Dashboard(tmpl template.Template, organisationStore OrganisationStore) Hand
 			return err
 		}
 
-		return tmpl(w, &dashboardData{App: appData, Donors: donors})
+		return tmpl(w, &dashboardData{App: appData, Donors: donors, Query: r.URL.Query()})
 	}
 }
diff --git a/internal/page/supporter/dashboard_test.go b/internal/page/supporter/dashboard_test.go
@@ -3,6 +3,7 @@ package supporter
 import (
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
@@ -11,7 +12,7 @@ import (
 
 func TestGetDashboard(t *testing.T) {
 	w := httptest.NewRecorder()
-	r, _ := http.NewRequest(http.MethodGet, "/", nil)
+	r, _ := http.NewRequest(http.MethodGet, "/?a=b", nil)
 
 	donors := []actor.DonorProvidedDetails{{LpaID: "abc"}}
 
@@ -25,6 +26,7 @@ func TestGetDashboard(t *testing.T) {
 		Execute(w, &dashboardData{
 			App:    testAppData,
 			Donors: donors,
+			Query:  url.Values{"a": {"b"}},
 		}).
 		Return(expectedError)
 

diff --git a/internal/page/supporter/edit_member.go b/internal/page/supporter/edit_member.go
@@ -54,7 +54,12 @@ func EditMember(tmpl template.Template, memberStore MemberStore) Handler {
 					return err
 				}
 
-				return page.Paths.Supporter.ManageTeamMembers.RedirectQuery(w, r, appData, query)
+				redirect := page.Paths.Supporter.ManageTeamMembers
+				if !appData.IsAdmin() {
+					redirect = page.Paths.Supporter.Dashboard
+				}
+
+				return redirect.RedirectQuery(w, r, appData, query)
 			}
 		}