Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#11878] Get account request by uuid #13007

Merged
merged 3 commits into from
Apr 12, 2024
Merged

[#11878] Get account request by uuid #13007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e1c4be
change GetAccountRequestAction to get by id
cedricongjh Apr 11, 2024
9e8c56d
fix tests
cedricongjh Apr 11, 2024
469c3d0
remove unncessary todo
cedricongjh Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/e2e/java/teammates/e2e/cases/AdminHomePageE2ETest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ public void testAll() {
String failureMessage = homePage.getMessageForInstructor(1);
assertTrue(failureMessage.contains(
"\"invalidemail\" is not acceptable to TEAMMATES as a/an email because it is not in the correct format."));

assertNotNull(BACKDOOR.getAccountRequest(email, institute));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately, there is no way for us to get the UUID of the account request created by the page, so we cannot do this assertion anymore

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's no way, then that's fine, I guess? I just wonder if the fact that it's no longer testable is a code smell.

Copy link
Contributor

@dishenggg dishenggg Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a endpoint for all pending requests, we can try to get all pending requests and then filter out this particular request by comparing all the attributes?

Should not be too bad for the purposes of the E2E test since there should not be many account requests in the db?

Edit: Cedric's point on the success message makes sense as well and is also probably more maintainable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a endpoint for all pending requests, we can try to get all pending requests and then filter out this particular request by comparing all the attributes?

That could work too. It might even be more rigorous than checking the success message. However, if you all believe the success message is sufficient, then maybe it's fine.

// TODO: delete account request after get
// BACKDOOR.deleteAccountRequest(email, institute);
}

}
4 changes: 2 additions & 2 deletions src/e2e/java/teammates/e2e/cases/AdminSearchPageE2ETest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void testAll() {
searchPage.inputSearchContent(searchContent);
searchPage.clickSearchButton();
searchPage.clickResetAccountRequestButton(accountRequest);
assertNull(BACKDOOR.getAccountRequest(accountRequest.getEmail(), accountRequest.getInstitute()).getRegisteredAt());
assertNull(BACKDOOR.getAccountRequest(accountRequest.getId()).getRegisteredAt());

______TS("Typical case: Delete account request successful");
accountRequest = sqlTestData.accountRequests.get("unregisteredInstructor1");
Expand All @@ -141,7 +141,7 @@ public void testAll() {
searchPage.inputSearchContent(searchContent);
searchPage.clickSearchButton();
searchPage.clickDeleteAccountRequestButton(accountRequest);
assertNull(BACKDOOR.getAccountRequest(accountRequest.getEmail(), accountRequest.getInstitute()));
assertNull(BACKDOOR.getAccountRequest(accountRequest.getId()));
}

private String getExpectedStudentDetails(StudentAttributes student) {
Expand Down
3 changes: 2 additions & 1 deletion src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.List;
import java.util.UUID;

import org.testng.ITestContext;
import org.testng.annotations.AfterClass;
Expand Down Expand Up @@ -329,7 +330,7 @@ protected String getKeyForStudent(StudentAttributes student) {

@Override
protected AccountRequestAttributes getAccountRequest(AccountRequestAttributes accountRequest) {
return BACKDOOR.getAccountRequest(accountRequest.getEmail(), accountRequest.getInstitute());
return BACKDOOR.getAccountRequest(UUID.fromString(accountRequest.getId()));
}

NotificationAttributes getNotification(String notificationId) {
Expand Down
2 changes: 1 addition & 1 deletion src/e2e/java/teammates/e2e/cases/InstructorCourseJoinConfirmationPageE2ETest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void testAll() {
______TS("Click join link: valid account request key");

String regKey = BACKDOOR
.getRegKeyForAccountRequest("ICJoinConf.newinstr@gmail.tmt", "TEAMMATES Test Institute 1");
.getRegKeyForAccountRequest(sqlTestData.accountRequests.get("ICJoinConf.newinstr").getId());

joinLink = createFrontendUrl(Const.WebPageURIs.JOIN_PAGE)
.withIsCreatingAccount("true")
Expand Down
2 changes: 1 addition & 1 deletion src/it/java/teammates/it/ui/webapi/RejectAccountRequestActionIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ protected void tearDown() {
HibernateUtil.beginTransaction();
List<AccountRequest> accountRequests = logic.getAllAccountRequests();
for (AccountRequest ar : accountRequests) {
logic.deleteAccountRequest(ar.getEmail(), ar.getInstitute());
logic.deleteAccountRequest(ar.getId());
}
HibernateUtil.commitTransaction();
}
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/teammates/ui/webapi/GetAccountRequestAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package teammates.ui.webapi;

import java.util.UUID;

import teammates.common.util.Const;
import teammates.storage.sqlentity.AccountRequest;
import teammates.ui.output.AccountRequestData;
Expand All @@ -11,14 +13,12 @@ class GetAccountRequestAction extends AdminOnlyAction {

@Override
public JsonResult execute() {
String email = getNonNullRequestParamValue(Const.ParamsNames.INSTRUCTOR_EMAIL);
String institute = getNonNullRequestParamValue(Const.ParamsNames.INSTRUCTOR_INSTITUTION);
UUID id = getUuidRequestParamValue(Const.ParamsNames.ACCOUNT_REQUEST_ID);

AccountRequest accountRequest = sqlLogic.getAccountRequest(email, institute);
AccountRequest accountRequest = sqlLogic.getAccountRequest(id);

if (accountRequest == null) {
throw new EntityNotFoundException("Account request for email: "
+ email + " and institute: " + institute + " not found.");
throw new EntityNotFoundException("Account request with id: " + id.toString() + " does not exist.");
}

AccountRequestData output = new AccountRequestData(accountRequest);
Expand Down
10 changes: 4 additions & 6 deletions src/test/java/teammates/test/AbstractBackDoor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,10 +832,9 @@ public void deleteCourse(String courseId) {
/**
* Gets an account request from the database.
*/
public AccountRequestAttributes getAccountRequest(String email, String institute) {
public AccountRequestAttributes getAccountRequest(UUID id) {
Map<String, String> params = new HashMap<>();
params.put(Const.ParamsNames.INSTRUCTOR_EMAIL, email);
params.put(Const.ParamsNames.INSTRUCTOR_INSTITUTION, institute);
params.put(Const.ParamsNames.ACCOUNT_REQUEST_ID, id.toString());

ResponseBodyAndCode response = executeGetRequest(Const.ResourceURIs.ACCOUNT_REQUEST, params);
if (response.responseCode == HttpStatus.SC_NOT_FOUND) {
Expand All @@ -852,10 +851,9 @@ public AccountRequestAttributes getAccountRequest(String email, String institute
/**
* Gets registration key of an account request from the database.
*/
public String getRegKeyForAccountRequest(String email, String institute) {
public String getRegKeyForAccountRequest(UUID id) {
Map<String, String> params = new HashMap<>();
params.put(Const.ParamsNames.INSTRUCTOR_EMAIL, email);
params.put(Const.ParamsNames.INSTRUCTOR_INSTITUTION, institute);
params.put(Const.ParamsNames.ACCOUNT_REQUEST_ID, id.toString());

ResponseBodyAndCode response = executeGetRequest(Const.ResourceURIs.ACCOUNT_REQUEST, params);
if (response.responseCode == HttpStatus.SC_NOT_FOUND) {
Expand Down
Loading