Skip to content

Commit

Permalink
[#11371] Instructor sample data: defer until instructor joins (#11396)
Browse files Browse the repository at this point in the history
* Defer creation of sample data until after instructor joins

* Fix deleteAccountRequest

* Add loading spinner to create-account-page

* Add check for whether user already has account

* Update frontend tests

* Add component tests

* Add tests for AccountRequestAttributes

* Add component tests for create account actions

* Fix frontend lint errors

* Fix backend lint errors

* Fix e2e tests

* Add account requests to data bundle

* Update account  requests logic test

* Add e2e Tests

* Fix page not redirecting to login

* Fix lint errors

* Fix linting issues

* Change id of AccountRequest to use both email and institute

* Move create account page to user join page

* Update tests for user-join-page

* Add delete account request action

* Delete account request in AdminHomePageE2E test

* Add GetAccountRequest action

* Remove unnecessary calls to getAccountRequest

* Fix failing component tests

* Use instructorEmail instead of Email const

* Fix minor bugs and cleanup

* Remove trailing whitespace

* Add placeholder skeleton to frontend

* Update AccountRequest and DTO

* Move registration url creation to accountRequest dto

* Update createAccountAction to soft delete

* Update component tests

* Add endpoint to get account request status

* Update user join page

* Fix indentation

* Update tests for user-join-page

* Fix failing component tests

* Fix typos

* Fix typo which caused createdAt to be null

* Fix broken component tests

* Update exception to use specific exception type

* Change Instant to Long in AccountRequestData.java

* Refactor CreateAccountRequestActionTest to use getJoinLink method

* Fix lint issues

* Change getAccountRequest to throw exception

* Remove unnecessary null check

* Refactor user join page

* Remove instructor from registration url

* Remove instructor entity type from tests

* Update sample data to be more realistic

* Remove name and registration key update options

* Fix architecture test violations

* Seperate craete and update account request

* Streamline API for join status

* Minor bug fix

* Remove account request status constant

* Refactor get account request methods

* Simplify condition

* Fix typo

* Remove else block

* Create e2e test for account requests

* Fix lint issues

* Fix failing test

* Remove now unnecessary comment

* Revert unnecessary change

* Remove outdated comment

* Fix minor issues

* Remove additional word

* Replace boolean with string

* Catch error when accountRequestAttributes is null

* Remove entityType from join link

* Use invalidKey variable

* Remove name from update options

* Handle case where accountRequestAttributes not found with assertions instead

* Seperate try block in CreateAccountAction.java

* Handle cases in CreateAccountAction where exceptions should not be thrown

* Tweak comment language used to be consistent

* Collapse catch blocks in CreateAccountAction.java

Co-authored-by: Wilson Kurniawan <wkurniawan.92@gmail.com>
  • Loading branch information
samuelfangjw and wkurniawan07 committed Feb 5, 2022
1 parent 86e2fba commit 30ac04e
Show file tree
Hide file tree
Showing 46 changed files with 2,222 additions and 120 deletions.
10 changes: 2 additions & 8 deletions src/e2e/java/teammates/e2e/cases/AdminHomePageE2ETest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import org.testng.annotations.Test;

import teammates.common.datatransfer.attributes.CourseAttributes;
import teammates.common.util.AppUrl;
import teammates.common.util.Const;
import teammates.e2e.pageobjects.AdminHomePage;
Expand All @@ -26,9 +25,6 @@ public void testAll() {
String name = "AHPUiT Instrúctör WithPlusInEmail";
String email = "AHPUiT+++_.instr1!@gmail.tmt";
String institute = "TEAMMATES Test Institute 1";
String demoCourseId = "AHPUiT____.instr1_.gma-demo";

BACKDOOR.deleteCourse(demoCourseId);

homePage.queueInstructorForAdding(name, email, institute);

Expand All @@ -42,14 +38,12 @@ public void testAll() {
assertTrue(successMessage.contains(
"Instructor \"AHPUiT Instrúctör WithPlusInEmail\" has been successfully created"));

CourseAttributes demoCourse = getCourse(demoCourseId);
assertNotNull(demoCourse);

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."));

BACKDOOR.deleteCourse(demoCourseId);
assertNotNull(BACKDOOR.getAccountRequest(email, institute));
BACKDOOR.deleteAccountRequest(email, institute);
}

}
6 changes: 6 additions & 0 deletions src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import teammates.common.datatransfer.DataBundle;
import teammates.common.datatransfer.attributes.AccountAttributes;
import teammates.common.datatransfer.attributes.AccountRequestAttributes;
import teammates.common.datatransfer.attributes.CourseAttributes;
import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
import teammates.common.datatransfer.attributes.FeedbackResponseAttributes;
Expand Down Expand Up @@ -307,6 +308,11 @@ String getKeyForStudent(StudentAttributes student) {
return getStudent(student).getKey();
}

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

@Override
protected boolean doRemoveAndRestoreDataBundle(DataBundle testData) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,33 @@ public void testAll() {
______TS("Already joined, no confirmation page");

getNewPageInstance(joinLink, InstructorHomePage.class);

logout();

______TS("Click join link: invalid key");
joinLink = createFrontendUrl(Const.WebPageURIs.JOIN_PAGE)
.withIsCreatingAccount("true")
.withRegistrationKey(invalidKey);
confirmationPage = loginToPage(joinLink, CourseJoinConfirmationPage.class, "ICJoinConf.newinstr");

confirmationPage.verifyDisplayedMessage("The course join link is invalid. You may have "
+ "entered the URL incorrectly or the URL may correspond to a/an instructor that does not exist.");

______TS("Click join link: valid account request key");

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

joinLink = createFrontendUrl(Const.WebPageURIs.JOIN_PAGE)
.withIsCreatingAccount("true")
.withRegistrationKey(regKey);

confirmationPage = getNewPageInstance(joinLink, CourseJoinConfirmationPage.class);
confirmationPage.verifyJoiningUser("ICJoinConf.newinstr");
confirmationPage.confirmJoinCourse(InstructorHomePage.class);

______TS("Regkey for account request used, no confirmation page");

getNewPageInstance(joinLink, InstructorHomePage.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@
"institute": "TEAMMATES Test Institute 1"
}
},
"accountRequests": {
"ICJoinConf.instr.CS1101": {
"name": "Teammates Test 2",
"email": "ICJoinConf.instr2@gmail.tmt",
"institute": "TEAMMATES Test Institute 1",
"createdAt": "2011-01-01T00:00:00Z",
"registeredAt": "1970-02-14T00:00:00Z"
},
"ICJoinConf.newinstr": {
"name": "Teammates Test 3",
"email": "ICJoinConf.newinstr@gmail.tmt",
"institute": "TEAMMATES Test Institute 1",
"createdAt": "2011-01-01T00:00:00Z"
}
},
"courses": {
"ICJoinConf.CS1101": {
"id": "tm.e2e.ICJoinConf.CS1101",
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/teammates/common/datatransfer/DataBundle.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.Map;

import teammates.common.datatransfer.attributes.AccountAttributes;
import teammates.common.datatransfer.attributes.AccountRequestAttributes;
import teammates.common.datatransfer.attributes.CourseAttributes;
import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes;
import teammates.common.datatransfer.attributes.FeedbackResponseAttributes;
Expand All @@ -21,6 +22,7 @@
// CHECKSTYLE.OFF:JavadocVariable each field represents different entity types
public class DataBundle {
public Map<String, AccountAttributes> accounts = new LinkedHashMap<>();
public Map<String, AccountRequestAttributes> accountRequests = new LinkedHashMap<>();
public Map<String, CourseAttributes> courses = new LinkedHashMap<>();
public Map<String, InstructorAttributes> instructors = new LinkedHashMap<>();
public Map<String, StudentAttributes> students = new LinkedHashMap<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
package teammates.common.datatransfer.attributes;

import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

import teammates.common.util.Config;
import teammates.common.util.Const;
import teammates.common.util.FieldValidator;
import teammates.common.util.SanitizationHelper;
import teammates.storage.entity.AccountRequest;

/**
* The data transfer object for {@link AccountRequest} entities.
*/
public class AccountRequestAttributes extends EntityAttributes<AccountRequest> {

private String email;
private String name;
private String institute;
private Instant registeredAt;
private Instant createdAt;
private transient String registrationKey;

private AccountRequestAttributes(String email, String institute, String name) {
this.email = email;
this.institute = institute;
this.name = name;
this.registrationKey = null;
this.registeredAt = null;
this.createdAt = null;
}

/**
* Gets the {@link AccountRequestAttributes} instance of the given {@link AccountRequest}.
*/
public static AccountRequestAttributes valueOf(AccountRequest accountRequest) {
AccountRequestAttributes accountRequestAttributes = new AccountRequestAttributes(accountRequest.getEmail(),
accountRequest.getInstitute(), accountRequest.getName());

accountRequestAttributes.registrationKey = accountRequest.getRegistrationKey();
accountRequestAttributes.registeredAt = accountRequest.getRegisteredAt();
accountRequestAttributes.createdAt = accountRequest.getCreatedAt();

return accountRequestAttributes;
}

/**
* Returns a builder for {@link AccountRequestAttributes}.
*/
public static Builder builder(String email, String institute, String name) {
return new Builder(email, institute, name);
}

public String getRegistrationKey() {
return registrationKey;
}

public String getName() {
return name;
}

public String getEmail() {
return email;
}

public String getInstitute() {
return institute;
}

public Instant getRegisteredAt() {
return registeredAt;
}

public Instant getCreatedAt() {
return createdAt;
}

public String getRegistrationUrl() {
return Config.getFrontEndAppUrl(Const.WebPageURIs.JOIN_PAGE)
.withIsCreatingAccount("true")
.withRegistrationKey(this.getRegistrationKey())
.toAbsoluteString();
}

@Override
public List<String> getInvalidityInfo() {
List<String> errors = new ArrayList<>();

addNonEmptyError(FieldValidator.getInvalidityInfoForEmail(getEmail()), errors);
addNonEmptyError(FieldValidator.getInvalidityInfoForPersonName(getName()), errors);
addNonEmptyError(FieldValidator.getInvalidityInfoForInstituteName(getInstitute()), errors);

return errors;
}

@Override
public AccountRequest toEntity() {
AccountRequest accountRequest = new AccountRequest(getEmail(), getName(), getInstitute());

if (this.getRegistrationKey() != null) {
accountRequest.setRegistrationKey(this.getRegistrationKey());
}

if (this.getCreatedAt() != null) {
accountRequest.setCreatedAt(this.getCreatedAt());
}

accountRequest.setRegisteredAt(this.getRegisteredAt());

return accountRequest;
}

@Override
public String toString() {
return "[" + AccountRequestAttributes.class.getSimpleName() + "] email: "
+ getEmail() + " name: " + getName() + " institute: " + getInstitute();
}

@Override
public int hashCode() {
return (this.email + this.name + this.institute).hashCode();
}

@Override
public boolean equals(Object other) {
if (other == null) {
return false;
} else if (this == other) {
return true;
} else if (this.getClass() == other.getClass()) {
AccountRequestAttributes otherAccountRequest = (AccountRequestAttributes) other;
return Objects.equals(this.email, otherAccountRequest.email)
&& Objects.equals(this.institute, otherAccountRequest.institute)
&& Objects.equals(this.name, otherAccountRequest.name);
} else {
return false;
}
}

@Override
public void sanitizeForSaving() {
this.institute = SanitizationHelper.sanitizeTitle(institute);
this.name = SanitizationHelper.sanitizeName(name);
this.email = SanitizationHelper.sanitizeEmail(email);
}

/**
* Updates with {@link UpdateOptions}.
*/
public void update(UpdateOptions updateOptions) {
updateOptions.registeredAtOption.ifPresent(s -> registeredAt = s);
}

/**
* Returns a {@link UpdateOptions.Builder} to build {@link UpdateOptions} for an account request.
*/
public static UpdateOptions.Builder updateOptionsBuilder(String email, String institute) {
return new UpdateOptions.Builder(email, institute);
}

/**
* A builder for {@link AccountRequestAttributes}.
*/
public static class Builder extends BasicBuilder<AccountRequestAttributes, Builder> {
private final AccountRequestAttributes accountRequestAttributes;

private Builder(String email, String institute, String name) {
super(new UpdateOptions(email, institute));
thisBuilder = this;

accountRequestAttributes = new AccountRequestAttributes(email, institute, name);
}

@Override
public AccountRequestAttributes build() {
accountRequestAttributes.update(updateOptions);

return accountRequestAttributes;
}
}

/**
* Helper class to specify the fields to update in {@link AccountRequestAttributes}.
*/
public static class UpdateOptions {
private String email;
private String institute;

private UpdateOption<Instant> registeredAtOption = UpdateOption.empty();

private UpdateOptions(String email, String institute) {
assert email != null;
assert institute != null;

this.email = email;
this.institute = institute;
}

public String getEmail() {
return email;
}

public String getInstitute() {
return institute;
}

@Override
public String toString() {
return "AccountRequestAttributes.UpdateOptions ["
+ ", email = " + email
+ ", institute = " + institute
+ ", registeredAt = " + registeredAtOption
+ "]";
}

/**
* Builder class to build {@link UpdateOptions}.
*/
public static class Builder extends BasicBuilder<UpdateOptions, Builder> {
private Builder(String email, String institute) {
super(new UpdateOptions(email, institute));
thisBuilder = this;
}

@Override
public UpdateOptions build() {
return updateOptions;
}

}

}

/**
* Basic builder to build {@link AccountRequestAttributes} related classes.
*
* @param <T> type to be built
* @param <B> type of the builder
*/
private abstract static class BasicBuilder<T, B extends BasicBuilder<T, B>> {

UpdateOptions updateOptions;
B thisBuilder;

BasicBuilder(UpdateOptions updateOptions) {
this.updateOptions = updateOptions;
}

public B withRegisteredAt(Instant registeredAt) {
assert registeredAt != null;

updateOptions.registeredAtOption = UpdateOption.of(registeredAt);
return thisBuilder;
}

public abstract T build();

}
}
4 changes: 4 additions & 0 deletions src/main/java/teammates/common/util/AppUrl.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ public AppUrl withRegistrationKey(String key) {
return withParam(Const.ParamsNames.REGKEY, key);
}

public AppUrl withIsCreatingAccount(String isCreatingAccount) {
return withParam(Const.ParamsNames.IS_CREATING_ACCOUNT, isCreatingAccount);
}

public AppUrl withCourseId(String courseId) {
return withParam(Const.ParamsNames.COURSE_ID, courseId);
}
Expand Down
Loading

0 comments on commit 30ac04e

Please sign in to comment.