[#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
TEAMMATES · Mar 25, 2024 · cb29108 · cb29108
1 parent 126d2c4
commit cb29108
Show file tree

Hide file tree

Showing 11 changed files with 256 additions and 84 deletions.
diff --git 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
@@ -15,7 +15,7 @@
  * The data transfer object for {@link AccountRequest} entities.
  */
 public final class AccountRequestAttributes extends EntityAttributes<AccountRequest> {
-
+    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
@@ -2,43 +2,52 @@
 
 import javax.annotation.Nullable;
 
+import teammates.common.datatransfer.AccountRequestStatus;
 import teammates.common.datatransfer.attributes.AccountRequestAttributes;
 import teammates.storage.sqlentity.AccountRequest;
 
 /**
  * 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
@@ -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");