Skip to content

Commit

Permalink
use transactions for reject account request action (#13001)
Browse files Browse the repository at this point in the history
  • Loading branch information
@domoberzin
domoberzin authored Apr 9, 2024
1 parent cfff21d commit 50c87bc
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 38 deletions.
63 changes: 42 additions & 21 deletions src/it/java/teammates/it/ui/webapi/RejectAccountRequestActionIT.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package teammates.it.ui.webapi;

import java.util.List;
import java.util.UUID;

import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

Expand All @@ -15,7 +17,6 @@
import teammates.common.util.HibernateUtil;
import teammates.common.util.SanitizationHelper;
import teammates.storage.sqlentity.AccountRequest;
import teammates.storage.sqlentity.Course;
import teammates.ui.output.AccountRequestData;
import teammates.ui.request.AccountRequestRejectionRequest;
import teammates.ui.request.InvalidHttpRequestBodyException;
Expand Down Expand Up @@ -49,9 +50,7 @@ public class RejectAccountRequestActionIT extends BaseActionIT<RejectAccountRequ
@Override
@BeforeMethod
protected void setUp() throws Exception {
super.setUp();
persistDataBundle(typicalBundle);
HibernateUtil.flushSession();
// no need to call super.setUp() because the action handles its own transactions
}

@Override
Expand All @@ -71,9 +70,11 @@ public void testExecute() throws Exception {

@Test
protected void testExecute_withReasonTitleAndBody_shouldRejectWithEmail()
throws InvalidOperationException, InvalidHttpRequestBodyException {
AccountRequest accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
accountRequest.setStatus(AccountRequestStatus.PENDING);
throws InvalidOperationException, InvalidHttpRequestBodyException, InvalidParametersException {
AccountRequest bundleAccountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
AccountRequest accountRequest = logic.createAccountRequestWithTransaction(bundleAccountRequest.getName(),
bundleAccountRequest.getEmail(), bundleAccountRequest.getInstitute(),
AccountRequestStatus.PENDING, bundleAccountRequest.getComments());
UUID id = accountRequest.getId();

AccountRequestRejectionRequest requestBody = new AccountRequestRejectionRequest(TYPICAL_TITLE, TYPICAL_BODY);
Expand Down Expand Up @@ -102,9 +103,11 @@ protected void testExecute_withReasonTitleAndBody_shouldRejectWithEmail()

@Test
protected void testExecute_withoutReasonTitleAndBody_shouldRejectWithoutEmail()
throws InvalidOperationException, InvalidHttpRequestBodyException {
AccountRequest accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
accountRequest.setStatus(AccountRequestStatus.PENDING);
throws InvalidOperationException, InvalidHttpRequestBodyException, InvalidParametersException {
AccountRequest bundleAccountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
AccountRequest accountRequest = logic.createAccountRequestWithTransaction(bundleAccountRequest.getName(),
bundleAccountRequest.getEmail(), bundleAccountRequest.getInstitute(),
AccountRequestStatus.PENDING, bundleAccountRequest.getComments());
UUID id = accountRequest.getId();

AccountRequestRejectionRequest requestBody = new AccountRequestRejectionRequest(null, null);
Expand All @@ -126,8 +129,11 @@ protected void testExecute_withoutReasonTitleAndBody_shouldRejectWithoutEmail()
}

@Test
protected void testExecute_withReasonBodyButNoTitle_shouldThrow() {
AccountRequest accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
protected void testExecute_withReasonBodyButNoTitle_shouldThrow() throws InvalidParametersException {
AccountRequest bundleAccountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
AccountRequest accountRequest = logic.createAccountRequestWithTransaction(bundleAccountRequest.getName(),
bundleAccountRequest.getEmail(), bundleAccountRequest.getInstitute(),
bundleAccountRequest.getStatus(), bundleAccountRequest.getComments());
UUID id = accountRequest.getId();

AccountRequestRejectionRequest requestBody = new AccountRequestRejectionRequest(null, TYPICAL_BODY);
Expand All @@ -140,8 +146,11 @@ protected void testExecute_withReasonBodyButNoTitle_shouldThrow() {
}

@Test
protected void testExecute_withReasonTitleButNoBody_shouldThrow() {
AccountRequest accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
protected void testExecute_withReasonTitleButNoBody_shouldThrow() throws InvalidParametersException {
AccountRequest bundleAccountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
AccountRequest accountRequest = logic.createAccountRequestWithTransaction(bundleAccountRequest.getName(),
bundleAccountRequest.getEmail(), bundleAccountRequest.getInstitute(),
bundleAccountRequest.getStatus(), bundleAccountRequest.getComments());
UUID id = accountRequest.getId();

AccountRequestRejectionRequest requestBody = new AccountRequestRejectionRequest(TYPICAL_TITLE, null);
Expand All @@ -155,9 +164,11 @@ protected void testExecute_withReasonTitleButNoBody_shouldThrow() {

@Test
protected void testExecute_alreadyRejected_shouldNotSendEmail()
throws InvalidOperationException, InvalidHttpRequestBodyException {
AccountRequest accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
accountRequest.setStatus(AccountRequestStatus.REJECTED);
throws InvalidOperationException, InvalidHttpRequestBodyException, InvalidParametersException {
AccountRequest bundleAccountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
AccountRequest accountRequest = logic.createAccountRequestWithTransaction(bundleAccountRequest.getName(),
bundleAccountRequest.getEmail(), bundleAccountRequest.getInstitute(),
AccountRequestStatus.REJECTED, bundleAccountRequest.getComments());
UUID id = accountRequest.getId();

AccountRequestRejectionRequest requestBody = new AccountRequestRejectionRequest(TYPICAL_TITLE, TYPICAL_BODY);
Expand All @@ -179,12 +190,12 @@ protected void testExecute_alreadyRejected_shouldNotSendEmail()
}

@Test
protected void testExecute_invalidUuid_shouldThrow() {
protected void testExecute_invalidUuid_shouldThrow() throws InvalidParametersException {
AccountRequestRejectionRequest requestBody = new AccountRequestRejectionRequest(null, null);
String[] params = new String[] {Const.ParamsNames.ACCOUNT_REQUEST_ID, "invalid"};

InvalidHttpParameterException ihpe = verifyHttpParameterFailure(requestBody, params);
assertEquals("Invalid UUID string: invalid", ihpe.getMessage());
assertEquals("Expected UUID value for id parameter, but found: [invalid]", ihpe.getMessage());
verifyNoEmailsSent();
}

Expand All @@ -202,7 +213,17 @@ protected void testExecute_accountRequestNotFound_shouldThrow() {
@Override
@Test
protected void testAccessControl() throws InvalidParametersException, EntityAlreadyExistsException {
Course course = typicalBundle.courses.get("course1");
verifyOnlyAdminCanAccess(course);
verifyOnlyAdminCanAccessWithTransaction();
}

@Override
@AfterMethod
protected void tearDown() {
HibernateUtil.beginTransaction();
List<AccountRequest> accountRequests = logic.getAllAccountRequests();
for (AccountRequest ar : accountRequests) {
logic.deleteAccountRequest(ar.getEmail(), ar.getInstitute());
}
HibernateUtil.commitTransaction();
}
}
2 changes: 1 addition & 1 deletion src/it/java/teammates/it/ui/webapi/UpdateAccountRequestActionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public void testExecute() throws Exception {

InvalidHttpParameterException ihpe = verifyHttpParameterFailure(requestBody, params);

assertEquals("Invalid UUID string: invalid", ihpe.getMessage());
assertEquals("Expected UUID value for id parameter, but found: [invalid]", ihpe.getMessage());

______TS("invalid email");
accountRequest = logic.createAccountRequestWithTransaction("name", "[email protected]",
Expand Down
18 changes: 9 additions & 9 deletions src/main/java/teammates/ui/webapi/RejectAccountRequestAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@
*/
public class RejectAccountRequestAction extends AdminOnlyAction {

@Override
public boolean isTransactionNeeded() {
return false;
}

@Override
public JsonResult execute() throws InvalidOperationException, InvalidHttpRequestBodyException {
String id = getNonNullRequestParamValue(Const.ParamsNames.ACCOUNT_REQUEST_ID);
UUID accountRequestId;

try {
accountRequestId = UUID.fromString(id);
} catch (IllegalArgumentException e) {
throw new InvalidHttpParameterException(e.getMessage(), e);
}
UUID accountRequestId = getUuidFromString(Const.ParamsNames.ACCOUNT_REQUEST_ID, id);

AccountRequest accountRequest = sqlLogic.getAccountRequest(accountRequestId);
AccountRequest accountRequest = sqlLogic.getAccountRequestWithTransaction(accountRequestId);

if (accountRequest == null) {
String errorMessage = String.format(Const.ACCOUNT_REQUEST_NOT_FOUND, accountRequestId.toString());
Expand All @@ -41,13 +40,14 @@ public JsonResult execute() throws InvalidOperationException, InvalidHttpRequest

try {
accountRequest.setStatus(AccountRequestStatus.REJECTED);
accountRequest = sqlLogic.updateAccountRequest(accountRequest);
accountRequest = sqlLogic.updateAccountRequestWithTransaction(accountRequest);
if (accountRequestRejectionRequest.checkHasReason()
&& initialStatus != AccountRequestStatus.REJECTED) {
EmailWrapper email = sqlEmailGenerator.generateAccountRequestRejectionEmail(accountRequest,
accountRequestRejectionRequest.getReasonTitle(), accountRequestRejectionRequest.getReasonBody());
emailSender.sendEmail(email);
}
taskQueuer.scheduleAccountRequestForSearchIndexing(accountRequest.getId().toString());
} catch (InvalidParametersException e) {
throw new InvalidHttpRequestBodyException(e);
} catch (EntityDoesNotExistException e) {
Expand Down
8 changes: 1 addition & 7 deletions src/main/java/teammates/ui/webapi/UpdateAccountRequestAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,7 @@ public boolean isTransactionNeeded() {
@Override
public JsonResult execute() throws InvalidOperationException, InvalidHttpRequestBodyException {
String id = getNonNullRequestParamValue(Const.ParamsNames.ACCOUNT_REQUEST_ID);
UUID accountRequestId;

try {
accountRequestId = UUID.fromString(id);
} catch (IllegalArgumentException e) {
throw new InvalidHttpParameterException(e.getMessage(), e);
}
UUID accountRequestId = getUuidFromString(Const.ParamsNames.ACCOUNT_REQUEST_ID, id);

AccountRequest accountRequest = sqlLogic.getAccountRequestWithTransaction(accountRequestId);

Expand Down

0 comments on commit 50c87bc

Please sign in to comment.