From cb29108134cfe44f025d2de5793ad835c47c6b50 Mon Sep 17 00:00:00 2001 From: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com> Date: Mon, 25 Mar 2024 16:25:50 +0800 Subject: [PATCH] [#11878] Modify CreateAccountRequestAction (#12913) * Add AccountCreateRequest instructorComments attribute * Add new AccountRequestData attributes * Remove check for registered instructor * Remove sending of registration email * Use AccountCreateRequest comments * Change output of CreateAccountRequestAction to AccountRequestData * Add CreateAccountRequestActionIT * Test execute with null arguments * Test execute with valid requests * Test execute on invalid arguments * Allow anybody to create an account request * Fix architecture test * Fix test * Update tests to verify search indexing --- .../webapi/CreateAccountRequestActionIT.java | 179 ++++++++++++++++-- .../attributes/AccountRequestAttributes.java | 8 +- .../ui/output/AccountRequestData.java | 27 ++- .../ui/request/AccountCreateRequest.java | 12 ++ .../ui/webapi/CreateAccountRequestAction.java | 33 ++-- .../logic/core/AccountRequestsLogicTest.java | 6 +- .../CreateAccountRequestActionTest.java | 42 +--- .../admin-home-page.component.spec.ts | 14 +- .../admin-home-page.component.ts | 6 +- src/web/services/account.service.ts | 4 +- src/web/services/search.service.spec.ts | 9 +- 11 files changed, 256 insertions(+), 84 deletions(-) diff --git a/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java b/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java index c2af0829c69..f1acd18c78c 100644 --- a/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java +++ b/src/it/java/teammates/it/ui/webapi/CreateAccountRequestActionIT.java @@ -2,12 +2,15 @@ import org.testng.annotations.Test; +import teammates.common.datatransfer.AccountRequestStatus; +import teammates.common.exception.InvalidParametersException; import teammates.common.util.Const; import teammates.common.util.EmailType; import teammates.common.util.EmailWrapper; import teammates.storage.sqlentity.AccountRequest; -import teammates.ui.output.JoinLinkData; +import teammates.ui.output.AccountRequestData; import teammates.ui.request.AccountCreateRequest; +import teammates.ui.request.InvalidHttpRequestBodyException; import teammates.ui.webapi.CreateAccountRequestAction; import teammates.ui.webapi.JsonResult; @@ -27,37 +30,171 @@ String getRequestMethod() { } @Override - @Test protected void testExecute() throws Exception { - // This is a minimal test; other cases are not tested due to upcoming changes in behaviour. + // This is separated into different test methods. + } + + @Test + void testExecute_nullEmail_throwsInvalidHttpRequestBodyException() { + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorName("Paul Atreides"); + request.setInstructorInstitution("House Atreides"); + InvalidHttpRequestBodyException ihrbException = verifyHttpRequestBodyFailure(request); + assertEquals("email cannot be null", ihrbException.getMessage()); + } + + @Test + void testExecute_nullName_throwsInvalidHttpRequestBodyException() { + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorEmail("kwisatz.haderach@atreides.org"); + request.setInstructorInstitution("House Atreides"); + InvalidHttpRequestBodyException ihrbException = verifyHttpRequestBodyFailure(request); + assertEquals("name cannot be null", ihrbException.getMessage()); + } + + @Test + void testExecute_nullInstitute_throwsInvalidHttpRequestBodyException() { + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorEmail("kwisatz.haderach@atreides.org"); + request.setInstructorName("Paul Atreides"); + InvalidHttpRequestBodyException ihrbException = verifyHttpRequestBodyFailure(request); + assertEquals("institute cannot be null", ihrbException.getMessage()); + } + + @Test + void testExecute_invalidEmail_throwsInvalidHttpRequestBodyException() { + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorEmail("invalid email address"); + request.setInstructorName("Paul Atreides"); + request.setInstructorInstitution("House Atreides"); + InvalidHttpRequestBodyException ihrbException = verifyHttpRequestBodyFailure(request); + String expectedMessage = "\"invalid email address\" is not acceptable to TEAMMATES as a/an email because it is not " + + "in the correct format. An email address contains some text followed by one '@' sign followed by some " + + "more text, and should end with a top level domain address like .com. It cannot be longer than 254 " + + "characters, cannot be empty and cannot contain spaces."; + assertEquals(expectedMessage, ihrbException.getMessage()); + } + + @Test + void testExecute_invalidName_throwsInvalidHttpRequestBodyException() { + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorEmail("kwisatz.haderach@atreides.org"); + request.setInstructorName("Pau| Atreides"); + request.setInstructorInstitution("House Atreides"); + InvalidHttpRequestBodyException ihrbException = verifyHttpRequestBodyFailure(request); + String expectedMessage = "\"Pau| Atreides\" is not acceptable to TEAMMATES as a/an person name because it contains " + + "invalid characters. A/An person name must start with an alphanumeric character, and cannot contain any " + + "vertical bar (|) or percent sign (%)."; + assertEquals(expectedMessage, ihrbException.getMessage()); + } + + @Test + void testExecute_invalidInstitute_throwsInvalidHttpRequestBodyException() { AccountCreateRequest request = new AccountCreateRequest(); - request.setInstructorEmail("ring-bearer@fellowship.net"); - request.setInstructorName("Frodo Baggins"); - request.setInstructorInstitution("The Fellowship of the Ring"); + request.setInstructorEmail("kwisatz.haderach@atreides.org"); + request.setInstructorName("Paul Atreides"); + request.setInstructorInstitution("House Atreide%"); + InvalidHttpRequestBodyException ihrbException = verifyHttpRequestBodyFailure(request); + String expectedMessage = "\"House Atreide%\" is not acceptable to TEAMMATES as a/an institute name because it " + + "contains invalid characters. A/An institute name must start with an alphanumeric character, and cannot " + + "contain any vertical bar (|) or percent sign (%)."; + assertEquals(expectedMessage, ihrbException.getMessage()); + } + + @Test + void testExecute_typicalCase_createsSuccessfully() { + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorEmail("kwisatz.haderach@atreides.org"); + request.setInstructorName("Paul Atreides"); + request.setInstructorInstitution("House Atreides"); + request.setInstructorComments("My road leads into the desert. I can see it."); CreateAccountRequestAction action = getAction(request); JsonResult result = getJsonResult(action); - JoinLinkData output = (JoinLinkData) result.getOutput(); - AccountRequest accountRequest = logic.getAccountRequest("ring-bearer@fellowship.net", "The Fellowship of the Ring"); - assertEquals("ring-bearer@fellowship.net", accountRequest.getEmail()); - assertEquals("Frodo Baggins", accountRequest.getName()); - assertEquals("The Fellowship of the Ring", accountRequest.getInstitute()); + AccountRequestData output = (AccountRequestData) result.getOutput(); + assertEquals("kwisatz.haderach@atreides.org", output.getEmail()); + assertEquals("Paul Atreides", output.getName()); + assertEquals("House Atreides", output.getInstitute()); + assertEquals(AccountRequestStatus.PENDING, output.getStatus()); + assertEquals("My road leads into the desert. I can see it.", output.getComments()); + assertNull(output.getRegisteredAt()); + AccountRequest accountRequest = logic.getAccountRequestByRegistrationKey(output.getRegistrationKey()); + assertEquals("kwisatz.haderach@atreides.org", accountRequest.getEmail()); + assertEquals("Paul Atreides", accountRequest.getName()); + assertEquals("House Atreides", accountRequest.getInstitute()); + assertEquals(AccountRequestStatus.PENDING, accountRequest.getStatus()); + assertEquals("My road leads into the desert. I can see it.", accountRequest.getComments()); assertNull(accountRequest.getRegisteredAt()); - assertEquals(accountRequest.getRegistrationUrl(), output.getJoinLink()); verifySpecifiedTasksAdded(Const.TaskQueue.SEARCH_INDEXING_QUEUE_NAME, 1); - verifyNumberOfEmailsSent(2); - EmailWrapper emailSent = mockEmailSender.getEmailsSent().get(0); - assertEquals(String.format(EmailType.NEW_INSTRUCTOR_ACCOUNT.getSubject(), "Frodo Baggins"), - emailSent.getSubject()); - assertEquals("ring-bearer@fellowship.net", emailSent.getRecipient()); - assertTrue(emailSent.getContent().contains(output.getJoinLink())); - EmailWrapper sentAdminAlertEmail = mockEmailSender.getEmailsSent().get(1); - // Check only the email type. The content of the email is not tested here, but in the email generator test(s). + verifyNumberOfEmailsSent(1); + EmailWrapper sentAdminAlertEmail = mockEmailSender.getEmailsSent().get(0); + assertEquals(EmailType.NEW_ACCOUNT_REQUEST_ADMIN_ALERT, sentAdminAlertEmail.getType()); + } + + @Test + void testExecute_leadingAndTrailingSpacesAndNullComments_createsSuccessfully() { + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorEmail(" kwisatz.haderach@atreides.org "); + request.setInstructorName(" Paul Atreides "); + request.setInstructorInstitution(" House Atreides "); + CreateAccountRequestAction action = getAction(request); + JsonResult result = getJsonResult(action); + AccountRequestData output = (AccountRequestData) result.getOutput(); + assertEquals("kwisatz.haderach@atreides.org", output.getEmail()); + assertEquals("Paul Atreides", output.getName()); + assertEquals("House Atreides", output.getInstitute()); + assertEquals(AccountRequestStatus.PENDING, output.getStatus()); + assertNull(output.getComments()); + assertNull(output.getRegisteredAt()); + AccountRequest accountRequest = logic.getAccountRequestByRegistrationKey(output.getRegistrationKey()); + assertEquals("kwisatz.haderach@atreides.org", accountRequest.getEmail()); + assertEquals("Paul Atreides", accountRequest.getName()); + assertEquals("House Atreides", accountRequest.getInstitute()); + assertEquals(AccountRequestStatus.PENDING, accountRequest.getStatus()); + assertNull(accountRequest.getComments()); + assertNull(accountRequest.getRegisteredAt()); + verifySpecifiedTasksAdded(Const.TaskQueue.SEARCH_INDEXING_QUEUE_NAME, 1); + verifyNumberOfEmailsSent(1); + EmailWrapper sentAdminAlertEmail = mockEmailSender.getEmailsSent().get(0); + assertEquals(EmailType.NEW_ACCOUNT_REQUEST_ADMIN_ALERT, sentAdminAlertEmail.getType()); + } + + @Test + void testExecute_accountRequestWithSameEmailAddressAndInstituteAlreadyExists_createsSuccessfully() + throws InvalidParametersException { + AccountRequest existingAccountRequest = logic.createAccountRequest("Paul Atreides", "kwisatz.haderach@atreides.org", + "House Atreides", AccountRequestStatus.PENDING, "My road leads into the desert. I can see it."); + AccountCreateRequest request = new AccountCreateRequest(); + request.setInstructorEmail("kwisatz.haderach@atreides.org"); + request.setInstructorName("Paul Atreides"); + request.setInstructorInstitution("House Atreides"); + request.setInstructorComments("My road leads into the desert. I can see it."); + CreateAccountRequestAction action = getAction(request); + JsonResult result = getJsonResult(action); + AccountRequestData output = (AccountRequestData) result.getOutput(); + assertEquals("kwisatz.haderach@atreides.org", output.getEmail()); + assertEquals("Paul Atreides", output.getName()); + assertEquals("House Atreides", output.getInstitute()); + assertEquals(AccountRequestStatus.PENDING, output.getStatus()); + assertEquals("My road leads into the desert. I can see it.", output.getComments()); + assertNull(output.getRegisteredAt()); + assertNotEquals(output.getRegistrationKey(), existingAccountRequest.getRegistrationKey()); + AccountRequest accountRequest = logic.getAccountRequestByRegistrationKey(output.getRegistrationKey()); + assertEquals("kwisatz.haderach@atreides.org", accountRequest.getEmail()); + assertEquals("Paul Atreides", accountRequest.getName()); + assertEquals("House Atreides", accountRequest.getInstitute()); + assertEquals(AccountRequestStatus.PENDING, accountRequest.getStatus()); + assertEquals("My road leads into the desert. I can see it.", accountRequest.getComments()); + assertNull(accountRequest.getRegisteredAt()); + verifySpecifiedTasksAdded(Const.TaskQueue.SEARCH_INDEXING_QUEUE_NAME, 1); + verifyNumberOfEmailsSent(1); + EmailWrapper sentAdminAlertEmail = mockEmailSender.getEmailsSent().get(0); assertEquals(EmailType.NEW_ACCOUNT_REQUEST_ADMIN_ALERT, sentAdminAlertEmail.getType()); } @Override + @Test protected void testAccessControl() throws Exception { - // This is not tested due to upcoming changes in behaviour. + verifyAccessibleWithoutLogin(); } } diff --git a/src/main/java/teammates/common/datatransfer/attributes/AccountRequestAttributes.java b/src/main/java/teammates/common/datatransfer/attributes/AccountRequestAttributes.java index 70f97471d6f..95550962ed5 100644 --- a/src/main/java/teammates/common/datatransfer/attributes/AccountRequestAttributes.java +++ b/src/main/java/teammates/common/datatransfer/attributes/AccountRequestAttributes.java @@ -15,7 +15,7 @@ * The data transfer object for {@link AccountRequest} entities. */ public final class AccountRequestAttributes extends EntityAttributes { - + private String id; private String email; private String name; private String institute; @@ -38,7 +38,7 @@ private AccountRequestAttributes(String email, String institute, String name) { public static AccountRequestAttributes valueOf(AccountRequest accountRequest) { AccountRequestAttributes accountRequestAttributes = new AccountRequestAttributes(accountRequest.getEmail(), accountRequest.getInstitute(), accountRequest.getName()); - + accountRequestAttributes.id = accountRequest.getId(); accountRequestAttributes.registrationKey = accountRequest.getRegistrationKey(); accountRequestAttributes.registeredAt = accountRequest.getRegisteredAt(); accountRequestAttributes.createdAt = accountRequest.getCreatedAt(); @@ -53,6 +53,10 @@ public static Builder builder(String email, String institute, String name) { return new Builder(email, institute, name); } + public String getId() { + return id; + } + public String getRegistrationKey() { return registrationKey; } diff --git a/src/main/java/teammates/ui/output/AccountRequestData.java b/src/main/java/teammates/ui/output/AccountRequestData.java index 2d50bcb1360..92dc77ed50a 100644 --- a/src/main/java/teammates/ui/output/AccountRequestData.java +++ b/src/main/java/teammates/ui/output/AccountRequestData.java @@ -2,6 +2,7 @@ import javax.annotation.Nullable; +import teammates.common.datatransfer.AccountRequestStatus; import teammates.common.datatransfer.attributes.AccountRequestAttributes; import teammates.storage.sqlentity.AccountRequest; @@ -9,36 +10,44 @@ * Output format of account request data. */ public class AccountRequestData extends ApiOutput { - + private final String id; private final String email; private final String name; private final String institute; private final String registrationKey; + private final AccountRequestStatus status; + @Nullable + private final String comments; @Nullable private final Long registeredAt; private final long createdAt; public AccountRequestData(AccountRequestAttributes accountRequestInfo) { - + this.id = accountRequestInfo.getId(); this.name = accountRequestInfo.getName(); this.email = accountRequestInfo.getEmail(); this.institute = accountRequestInfo.getInstitute(); this.registrationKey = accountRequestInfo.getRegistrationKey(); + this.comments = null; this.createdAt = accountRequestInfo.getCreatedAt().toEpochMilli(); if (accountRequestInfo.getRegisteredAt() == null) { + this.status = AccountRequestStatus.APPROVED; this.registeredAt = null; } else { + this.status = AccountRequestStatus.REGISTERED; this.registeredAt = accountRequestInfo.getRegisteredAt().toEpochMilli(); } } public AccountRequestData(AccountRequest accountRequest) { - + this.id = accountRequest.getId().toString(); this.name = accountRequest.getName(); this.email = accountRequest.getEmail(); this.institute = accountRequest.getInstitute(); this.registrationKey = accountRequest.getRegistrationKey(); + this.status = accountRequest.getStatus(); + this.comments = accountRequest.getComments(); this.createdAt = accountRequest.getCreatedAt().toEpochMilli(); if (accountRequest.getRegisteredAt() == null) { @@ -48,6 +57,10 @@ public AccountRequestData(AccountRequest accountRequest) { } } + public String getId() { + return id; + } + public String getInstitute() { return institute; } @@ -64,6 +77,14 @@ public String getRegistrationKey() { return registrationKey; } + public AccountRequestStatus getStatus() { + return status; + } + + public String getComments() { + return comments; + } + public Long getRegisteredAt() { return registeredAt; } diff --git a/src/main/java/teammates/ui/request/AccountCreateRequest.java b/src/main/java/teammates/ui/request/AccountCreateRequest.java index f3097ce1152..9e39b524549 100644 --- a/src/main/java/teammates/ui/request/AccountCreateRequest.java +++ b/src/main/java/teammates/ui/request/AccountCreateRequest.java @@ -3,6 +3,8 @@ import java.util.ArrayList; import java.util.List; +import javax.annotation.Nullable; + import teammates.common.util.FieldValidator; import teammates.common.util.StringHelper; @@ -14,6 +16,8 @@ public class AccountCreateRequest extends BasicRequest { private String instructorEmail; private String instructorName; private String instructorInstitution; + @Nullable + private String instructorComments; public String getInstructorEmail() { return instructorEmail; @@ -27,6 +31,10 @@ public String getInstructorInstitution() { return this.instructorInstitution; } + public String getInstructorComments() { + return this.instructorComments; + } + public void setInstructorName(String name) { this.instructorName = name; } @@ -39,6 +47,10 @@ public void setInstructorEmail(String instructorEmail) { this.instructorEmail = instructorEmail; } + public void setInstructorComments(String instructorComments) { + this.instructorComments = instructorComments; + } + @Override public void validate() throws InvalidHttpRequestBodyException { assertTrue(this.instructorEmail != null, "email cannot be null"); diff --git a/src/main/java/teammates/ui/webapi/CreateAccountRequestAction.java b/src/main/java/teammates/ui/webapi/CreateAccountRequestAction.java index b6243a0016f..34a450f3b65 100644 --- a/src/main/java/teammates/ui/webapi/CreateAccountRequestAction.java +++ b/src/main/java/teammates/ui/webapi/CreateAccountRequestAction.java @@ -4,14 +4,24 @@ import teammates.common.exception.InvalidParametersException; import teammates.common.util.EmailWrapper; import teammates.storage.sqlentity.AccountRequest; -import teammates.ui.output.JoinLinkData; +import teammates.ui.output.AccountRequestData; import teammates.ui.request.AccountCreateRequest; import teammates.ui.request.InvalidHttpRequestBodyException; /** * Creates a new account request. */ -public class CreateAccountRequestAction extends AdminOnlyAction { +public class CreateAccountRequestAction extends Action { + + @Override + AuthType getMinAuthLevel() { + return AuthType.PUBLIC; + } + + @Override + void checkSpecificAccessControl() throws UnauthorizedAccessException { + // Nothing needs to be done here because anybody should be able to create an account request. + } @Override public JsonResult execute() @@ -21,9 +31,10 @@ public JsonResult execute() String instructorName = createRequest.getInstructorName().trim(); String instructorEmail = createRequest.getInstructorEmail().trim(); String instructorInstitution = createRequest.getInstructorInstitution().trim(); - // TODO: This is a placeholder. It should be obtained from AccountCreateRequest, in a separate PR. - String comments = "PLACEHOLDER"; - + String comments = createRequest.getInstructorComments(); + if (comments != null) { + comments = comments.trim(); + } AccountRequest accountRequest; try { @@ -35,19 +46,9 @@ public JsonResult execute() } assert accountRequest != null; - - if (accountRequest.getRegisteredAt() != null) { - throw new InvalidOperationException("Cannot create account request as instructor has already registered."); - } - - String joinLink = accountRequest.getRegistrationUrl(); - - EmailWrapper email = emailGenerator.generateNewInstructorAccountJoinEmail( - instructorEmail, instructorName, joinLink); - emailSender.sendEmail(email); EmailWrapper adminAlertEmail = sqlEmailGenerator.generateNewAccountRequestAdminAlertEmail(accountRequest); emailSender.sendEmail(adminAlertEmail); - JoinLinkData output = new JoinLinkData(joinLink); + AccountRequestData output = new AccountRequestData(accountRequest); return new JsonResult(output); } diff --git a/src/test/java/teammates/logic/core/AccountRequestsLogicTest.java b/src/test/java/teammates/logic/core/AccountRequestsLogicTest.java index 88cfd8430b3..24a66afbe6d 100644 --- a/src/test/java/teammates/logic/core/AccountRequestsLogicTest.java +++ b/src/test/java/teammates/logic/core/AccountRequestsLogicTest.java @@ -9,6 +9,7 @@ import teammates.common.exception.InvalidParametersException; import teammates.common.util.Const; import teammates.common.util.FieldValidator; +import teammates.storage.entity.AccountRequest; import teammates.test.AssertHelper; /** @@ -108,7 +109,10 @@ public void testUpdateAccountRequest() throws Exception { @Test public void testDeleteAccountRequest() throws Exception { - AccountRequestAttributes a = dataBundle.accountRequests.get("unregisteredInstructor1"); + // This ensures the AccountRequestAttributes has the correct ID. + AccountRequestAttributes accountRequestAttributes = dataBundle.accountRequests.get("unregisteredInstructor1"); + AccountRequest accountRequest = accountRequestAttributes.toEntity(); + AccountRequestAttributes a = AccountRequestAttributes.valueOf(accountRequest); ______TS("silent deletion of non-existent account request"); diff --git a/src/test/java/teammates/ui/webapi/CreateAccountRequestActionTest.java b/src/test/java/teammates/ui/webapi/CreateAccountRequestActionTest.java index 1fb58f95c6b..2e1cfa96b05 100644 --- a/src/test/java/teammates/ui/webapi/CreateAccountRequestActionTest.java +++ b/src/test/java/teammates/ui/webapi/CreateAccountRequestActionTest.java @@ -4,9 +4,7 @@ import teammates.common.datatransfer.attributes.AccountRequestAttributes; import teammates.common.util.Const; -import teammates.common.util.EmailType; -import teammates.common.util.EmailWrapper; -import teammates.ui.output.JoinLinkData; +import teammates.ui.output.AccountRequestData; import teammates.ui.request.AccountCreateRequest; import teammates.ui.request.InvalidHttpRequestBodyException; @@ -64,45 +62,23 @@ protected void testExecute() { assertEquals(institute, accountRequestAttributes.getInstitute()); assertNotNull(accountRequestAttributes.getRegistrationKey()); - String joinLink = accountRequestAttributes.getRegistrationUrl(); - JoinLinkData output = (JoinLinkData) r.getOutput(); - assertEquals(joinLink, output.getJoinLink()); + String registrationKey = accountRequestAttributes.getRegistrationKey(); + AccountRequestData output = (AccountRequestData) r.getOutput(); + assertEquals(registrationKey, output.getRegistrationKey()); - verifyNumberOfEmailsSent(1); + verifyNoEmailsSent(); verifySpecifiedTasksAdded(Const.TaskQueue.SEARCH_INDEXING_QUEUE_NAME, 1); - EmailWrapper emailSent = mockEmailSender.getEmailsSent().get(0); - assertEquals(String.format(EmailType.NEW_INSTRUCTOR_ACCOUNT.getSubject(), name), - emailSent.getSubject()); - assertEquals(email, emailSent.getRecipient()); - assertTrue(emailSent.getContent().contains(joinLink)); - - ______TS("Account request already exists: instructor unregistered, email sent again"); + ______TS("Account request already exists: instructor unregistered"); a = getAction(req); r = getJsonResult(a); - output = (JoinLinkData) r.getOutput(); - assertEquals(joinLink, output.getJoinLink()); + output = (AccountRequestData) r.getOutput(); + assertEquals(registrationKey, output.getRegistrationKey()); - verifyNumberOfEmailsSent(1); + verifyNoEmailsSent(); verifyNoTasksAdded(); // Account request not added to search indexing queue - emailSent = mockEmailSender.getEmailsSent().get(0); - assertEquals(String.format(EmailType.NEW_INSTRUCTOR_ACCOUNT.getSubject(), name), - emailSent.getSubject()); - assertEquals(email, emailSent.getRecipient()); - assertTrue(emailSent.getContent().contains(joinLink)); - - ______TS("Account request already exists: instructor registered, InvalidOperationException thrown"); - - accountRequestAttributes = typicalBundle.accountRequests.get("instructor1OfCourse1"); - - req = buildCreateRequest(accountRequestAttributes.getName(), - accountRequestAttributes.getInstitute(), accountRequestAttributes.getEmail()); - - InvalidOperationException ioe = verifyInvalidOperation(req); - assertEquals("Cannot create account request as instructor has already registered.", ioe.getMessage()); - ______TS("Error: invalid parameter"); String invalidName = "James%20Bond99"; diff --git a/src/web/app/pages-admin/admin-home-page/admin-home-page.component.spec.ts b/src/web/app/pages-admin/admin-home-page/admin-home-page.component.spec.ts index e512fa76819..3b25ac9d1a4 100644 --- a/src/web/app/pages-admin/admin-home-page/admin-home-page.component.spec.ts +++ b/src/web/app/pages-admin/admin-home-page/admin-home-page.component.spec.ts @@ -12,6 +12,7 @@ import { LinkService } from '../../../services/link.service'; import { SimpleModalService } from '../../../services/simple-modal.service'; import { StatusMessageService } from '../../../services/status-message.service'; import { createMockNgbModalRef } from '../../../test-helpers/mock-ngb-modal-ref'; +import { AccountRequestStatus } from '../../../types/api-output'; import { AjaxLoadingModule } from '../../components/ajax-loading/ajax-loading.module'; import { LoadingSpinnerModule } from '../../components/loading-spinner/loading-spinner.module'; @@ -186,15 +187,24 @@ describe('AdminHomePageComponent', () => { }, ]; jest.spyOn(accountService, 'createAccountRequest').mockReturnValue(of({ - joinLink: 'http://localhost:4200/web/join', + id: 'some.person@example.com%NUS', + email: 'some.person@example.com', + name: 'Some Person', + institute: 'NUS', + status: AccountRequestStatus.APPROVED, + registrationKey: 'registrationKey', + createdAt: 528, })); + jest.spyOn(linkService, 'generateAccountRegistrationLink') + .mockReturnValue('http://localhost:4200/web/join?iscreatingaccount=true&key=registrationKey'); fixture.detectChanges(); const index: number = 0; component.addInstructor(index); expect(component.instructorsConsolidated[index].status).toEqual('SUCCESS'); - expect(component.instructorsConsolidated[index].joinLink).toEqual('http://localhost:4200/web/join'); + expect(component.instructorsConsolidated[index].joinLink) + .toEqual('http://localhost:4200/web/join?iscreatingaccount=true&key=registrationKey'); expect(component.activeRequests).toEqual(0); }); diff --git a/src/web/app/pages-admin/admin-home-page/admin-home-page.component.ts b/src/web/app/pages-admin/admin-home-page/admin-home-page.component.ts index 1e29bd2bdb8..e6544a0e7b7 100644 --- a/src/web/app/pages-admin/admin-home-page/admin-home-page.component.ts +++ b/src/web/app/pages-admin/admin-home-page/admin-home-page.component.ts @@ -8,7 +8,7 @@ import { CourseService } from '../../../services/course.service'; import { LinkService } from '../../../services/link.service'; import { SimpleModalService } from '../../../services/simple-modal.service'; import { StatusMessageService } from '../../../services/status-message.service'; -import { Account, Accounts, Courses, JoinLink } from '../../../types/api-output'; +import { Account, AccountRequest, Accounts, Courses, JoinLink } from '../../../types/api-output'; import { SimpleModalType } from '../../components/simple-modal/simple-modal-type'; import { ErrorMessageOutput } from '../../error-message-output'; @@ -117,10 +117,10 @@ export class AdminHomePageComponent { this.isAddingInstructors = false; })) .subscribe({ - next: (resp: JoinLink) => { + next: (resp: AccountRequest) => { instructor.status = 'SUCCESS'; instructor.statusCode = 200; - instructor.joinLink = resp.joinLink; + instructor.joinLink = this.linkService.generateAccountRegistrationLink(resp.registrationKey); this.activeRequests -= 1; }, error: (resp: ErrorMessageOutput) => { diff --git a/src/web/services/account.service.ts b/src/web/services/account.service.ts index f278335db55..914a825a21a 100644 --- a/src/web/services/account.service.ts +++ b/src/web/services/account.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@angular/core'; import { Observable } from 'rxjs'; import { HttpRequestService } from './http-request.service'; import { ResourceEndpoints } from '../types/api-const'; -import { Account, Accounts, JoinLink, MessageOutput } from '../types/api-output'; +import { Account, AccountRequest, Accounts, JoinLink, MessageOutput } from '../types/api-output'; import { AccountCreateRequest } from '../types/api-request'; /** @@ -29,7 +29,7 @@ export class AccountService { /** * Creates an account request by calling API. */ - createAccountRequest(request: AccountCreateRequest): Observable { + createAccountRequest(request: AccountCreateRequest): Observable { return this.httpRequestService.post(ResourceEndpoints.ACCOUNT_REQUEST, {}, request); } diff --git a/src/web/services/search.service.spec.ts b/src/web/services/search.service.spec.ts index affbf259b4a..e548b693cff 100644 --- a/src/web/services/search.service.spec.ts +++ b/src/web/services/search.service.spec.ts @@ -13,6 +13,7 @@ import createSpyFromClass from '../test-helpers/create-spy-from-class'; import { ResourceEndpoints } from '../types/api-const'; import { AccountRequest, + AccountRequestStatus, Course, FeedbackSession, FeedbackSessionPublishStatus, @@ -184,11 +185,13 @@ describe('SearchService', () => { }; const mockAccountRequest: AccountRequest = { + id: 'test@example.com%Test Institute', registrationKey: 'regkey', createdAt: 1585487897502, name: 'Test Instructor', institute: 'Test Institute', email: 'test@example.com', + status: AccountRequestStatus.APPROVED, }; beforeEach(() => { @@ -294,7 +297,11 @@ describe('SearchService', () => { it('should join account requests accurately when timezone can be guessed and instructor is registered', () => { jest.spyOn(timezoneService, 'guessTimezone').mockReturnValue('Asia/Singapore'); - const accountRequest: AccountRequest = { ...mockAccountRequest, registeredAt: 1685487897502 }; + const accountRequest: AccountRequest = { + ...mockAccountRequest, + registeredAt: 1685487897502, + status: AccountRequestStatus.REGISTERED, + }; const result: AccountRequestSearchResult = service.joinAdminAccountRequest(accountRequest); expect(result.email).toBe('test@example.com');