diff --git a/src/main/java/teammates/ui/constants/ApiStringConst.java b/src/main/java/teammates/ui/constants/ApiStringConst.java new file mode 100644 index 00000000000..5158f2ac964 --- /dev/null +++ b/src/main/java/teammates/ui/constants/ApiStringConst.java @@ -0,0 +1,40 @@ +package teammates.ui.constants; + +import com.fasterxml.jackson.annotation.JsonValue; + +import teammates.common.util.FieldValidator; + +/** + * Special constants used by the back-end. + */ +public enum ApiStringConst { + // CHECKSTYLE.OFF:JavadocVariable + EMAIL_REGEX(escapeRegex(FieldValidator.REGEX_EMAIL)); + // CHECKSTYLE.ON:JavadocVariable + + private final Object value; + + ApiStringConst(Object value) { + this.value = value; + } + + @JsonValue + public Object getValue() { + return value; + } + + /** + * Escape regex pattern strings to ensure the pattern remains valid when converted to JS. + */ + private static String escapeRegex(String regexStr) { + String escapedRegexStr = regexStr; + // Double escape backslashes + escapedRegexStr = escapedRegexStr.replace("\\", "\\\\"); + // Replace possessive zero or more times quantifier *+ that the email pattern uses + // with greedy zero or more times quantifier * + // as possessive quantifiers are not supported in JavaScript + escapedRegexStr = escapedRegexStr.replace("*+", "*"); + return escapedRegexStr; + } + +} diff --git a/src/web/app/pages-static/request-page/__snapshots__/request-page.component.spec.ts.snap b/src/web/app/pages-static/request-page/__snapshots__/request-page.component.spec.ts.snap index aab4f634f16..3167fed5fd4 100644 --- a/src/web/app/pages-static/request-page/__snapshots__/request-page.component.spec.ts.snap +++ b/src/web/app/pages-static/request-page/__snapshots__/request-page.component.spec.ts.snap @@ -63,16 +63,6 @@ exports[`RequestPageComponent should render correctly after form is submitted 1` js@exampleu.edu - - - Home Page URL - - - u.exampleu.edu/jsmith - -
- - -
-
diff --git a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form-model.ts b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form-model.ts index 5b42bddc792..3c5179a161d 100644 --- a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form-model.ts +++ b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form-model.ts @@ -3,6 +3,5 @@ export type InstructorRequestFormModel = { institution: string, country: string, email: string, - homePage: string, comments: string, }; diff --git a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.html b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.html index a3aa5af576d..6753801b7db 100644 --- a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.html +++ b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.html @@ -11,10 +11,17 @@ This is the name that will be shown to your students. You may include salutation (Dr. Prof. etc.)

-
@@ -26,10 +33,18 @@

-
+ + +

@@ -42,11 +57,19 @@

-
+ + +

@@ -59,22 +82,18 @@ Note that this email address will be visible to the students you enroll in TEAMMATES.

-
+ -
-
-
- - - +

@@ -83,10 +102,16 @@ Any other comments/queries + [attr.aria-invalid]="comments.invalid">

- diff --git a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.scss b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.scss index addb0b11c7a..3e5249ca353 100644 --- a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.scss +++ b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.scss @@ -20,3 +20,7 @@ label.qn { .red-font { color: red; } + +.error-box { + margin: 1rem 0; +} diff --git a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.spec.ts b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.spec.ts index 67e0e3b94b7..3a5f4fba2b8 100644 --- a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.spec.ts +++ b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.spec.ts @@ -1,21 +1,34 @@ -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { ReactiveFormsModule } from '@angular/forms'; import { By } from '@angular/platform-browser'; -import { first } from 'rxjs'; +import { Observable, first } from 'rxjs'; import { InstructorRequestFormModel } from './instructor-request-form-model'; import { InstructorRequestFormComponent } from './instructor-request-form.component'; +import { AccountService } from '../../../../services/account.service'; +import { AccountCreateRequest } from '../../../../types/api-request'; describe('InstructorRequestFormComponent', () => { let component: InstructorRequestFormComponent; let fixture: ComponentFixture; + let accountService: AccountService; const typicalModel: InstructorRequestFormModel = { name: 'John Doe', institution: 'Example Institution', country: 'Example Country', email: 'jd@example.edu', - homePage: 'xyz.example.edu/john', comments: '', }; + const typicalCreateRequest: AccountCreateRequest = { + instructorEmail: typicalModel.email, + instructorName: typicalModel.name, + instructorInstitution: `${typicalModel.institution}, ${typicalModel.country}`, + }; + + const accountServiceStub: Partial = { + createAccountRequest: () => new Observable((subscriber) => { + subscriber.next(); + }), + }; /** * Fills in form fields with the given data. @@ -27,19 +40,24 @@ describe('InstructorRequestFormComponent', () => { component.institution.setValue(data.institution); component.country.setValue(data.country); component.email.setValue(data.email); - component.homePage.setValue(data.homePage); component.comments.setValue(data.comments); } - beforeEach(() => { + beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ declarations: [InstructorRequestFormComponent], imports: [ReactiveFormsModule], - }); + providers: [{ provide: AccountService, useValue: accountServiceStub }], + }) + .compileComponents(); + })); + + beforeEach(() => { fixture = TestBed.createComponent(InstructorRequestFormComponent); component = fixture.componentInstance; - + accountService = TestBed.inject(AccountService); fixture.detectChanges(); + jest.clearAllMocks(); }); it('should create', () => { @@ -50,17 +68,20 @@ describe('InstructorRequestFormComponent', () => { expect(fixture).toMatchSnapshot(); }); - it('should emit requestSubmissionEvent once when submit button is clicked', () => { - jest.spyOn(component.requestSubmissionEvent, 'emit'); + it('should run onSubmit() when submit button is clicked', () => { + jest.spyOn(component, 'onSubmit'); fillFormWith(typicalModel); const submitButton = fixture.debugElement.query(By.css('#submit-button')); submitButton.nativeElement.click(); - expect(component.requestSubmissionEvent.emit).toHaveBeenCalledTimes(1); + expect(component.onSubmit).toHaveBeenCalledTimes(1); }); it('should emit requestSubmissionEvent with the correct data when form is submitted', () => { + jest.spyOn(accountService, 'createAccountRequest').mockReturnValue( + new Observable((subscriber) => { subscriber.next(); })); + // Listen for emitted value let actualModel: InstructorRequestFormModel | null = null; component.requestSubmissionEvent.pipe(first()) @@ -74,7 +95,17 @@ describe('InstructorRequestFormComponent', () => { expect(actualModel!.institution).toBe(typicalModel.institution); expect(actualModel!.country).toBe(typicalModel.country); expect(actualModel!.email).toBe(typicalModel.email); - expect(actualModel!.homePage).toBe(typicalModel.homePage); expect(actualModel!.comments).toBe(typicalModel.comments); }); + + it('should send the correct request data when form is submitted', () => { + jest.spyOn(accountService, 'createAccountRequest').mockReturnValue( + new Observable((subscriber) => { subscriber.next(); })); + + fillFormWith(typicalModel); + component.onSubmit(); + + expect(accountService.createAccountRequest).toHaveBeenCalledTimes(1); + expect(accountService.createAccountRequest).toHaveBeenCalledWith(expect.objectContaining(typicalCreateRequest)); + }); }); diff --git a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.ts b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.ts index fff881cc607..02b2ae00fd2 100644 --- a/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.ts +++ b/src/web/app/pages-static/request-page/instructor-request-form/instructor-request-form.component.ts @@ -1,10 +1,11 @@ import { Component, EventEmitter, Output } from '@angular/core'; import { FormControl, FormGroup, Validators } from '@angular/forms'; +import { finalize } from 'rxjs'; import { InstructorRequestFormModel } from './instructor-request-form-model'; - -// Use regex to validate URL field as Angular does not have a built-in URL validator -// eslint-disable-next-line -const URL_REGEX = /(https?:\/\/)?(www\.)[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,4}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)|(https?:\/\/)?(www\.)?(?!ww)[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,4}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)/; +import { AccountService } from '../../../../services/account.service'; +import { AccountCreateRequest } from '../../../../types/api-request'; +import { FormValidator } from '../../../../types/form-validator'; +import { ErrorMessageOutput } from '../../../error-message-output'; @Component({ selector: 'tm-instructor-request-form', @@ -13,12 +14,35 @@ const URL_REGEX = /(https?:\/\/)?(www\.)[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,4 }) export class InstructorRequestFormComponent { + constructor(private accountService: AccountService) {} + + // Create members to be accessed in template + readonly STUDENT_NAME_MAX_LENGTH = FormValidator.STUDENT_NAME_MAX_LENGTH; + readonly INSTITUTION_NAME_MAX_LENGTH = FormValidator.INSTITUTION_NAME_MAX_LENGTH; + readonly COUNTRY_NAME_MAX_LENGTH = FormValidator.COUNTRY_NAME_MAX_LENGTH; + readonly EMAIL_MAX_LENGTH = FormValidator.EMAIL_MAX_LENGTH; + arf = new FormGroup({ - name: new FormControl('', [Validators.required]), - institution: new FormControl('', [Validators.required]), - country: new FormControl('', [Validators.required]), - email: new FormControl('', [Validators.required, Validators.email]), - homePage: new FormControl('', [Validators.pattern(URL_REGEX)]), + name: new FormControl('', [ + Validators.required, + Validators.maxLength(FormValidator.STUDENT_NAME_MAX_LENGTH), + Validators.pattern(FormValidator.NAME_REGEX), + ]), + institution: new FormControl('', [ + Validators.required, + Validators.maxLength(FormValidator.INSTITUTION_NAME_MAX_LENGTH), + Validators.pattern(FormValidator.NAME_REGEX), + ]), + country: new FormControl('', [ + Validators.required, + Validators.maxLength(FormValidator.COUNTRY_NAME_MAX_LENGTH), + Validators.pattern(FormValidator.NAME_REGEX), + ]), + email: new FormControl('', [ + Validators.required, + Validators.pattern(FormValidator.EMAIL_REGEX), + Validators.maxLength(FormValidator.EMAIL_MAX_LENGTH), + ]), comments: new FormControl(''), }, { updateOn: 'submit' }); @@ -27,23 +51,20 @@ export class InstructorRequestFormComponent { institution = this.arf.controls.institution; country = this.arf.controls.country; email = this.arf.controls.email; - homePage = this.arf.controls.homePage; comments = this.arf.controls.comments; hasSubmitAttempt = false; - + isLoading = false; @Output() requestSubmissionEvent = new EventEmitter(); + serverErrorMessage = ''; + checkIsFieldRequired(field: FormControl): boolean { return field.hasValidator(Validators.required); } - checkIsFieldInvalid(field: FormControl): boolean { - return field.invalid; - } - - checkCanSubmit(): boolean { - return true; // TODO: API integration + get canSubmit(): boolean { + return !this.isLoading; } getFieldValidationClasses(field: FormControl): string { @@ -60,39 +81,50 @@ export class InstructorRequestFormComponent { onSubmit(): void { this.hasSubmitAttempt = true; + this.isLoading = true; + this.serverErrorMessage = ''; if (this.arf.invalid) { + this.isLoading = false; // Do not submit form return; } const name = this.name.value!.trim(); const email = this.email.value!.trim(); + const comments = this.comments.value!.trim(); + + // Combine country and institution const country = this.country.value!.trim(); const institution = this.institution.value!.trim(); const combinedInstitution = `${institution}, ${country}`; - const homePage = this.homePage.value!; - const comments = this.comments.value!.trim(); - const submittedData = { - name, - email, - institution: combinedInstitution, - homePage, - comments, + const requestData: AccountCreateRequest = { + instructorEmail: email, + instructorName: name, + instructorInstitution: combinedInstitution, }; - // TODO: connect to API - // eslint-disable-next-line - submittedData; // PLACEHOLDER - - // Pass form input to parent to display confirmation - this.requestSubmissionEvent.emit({ - name, - institution, - country, - email, - homePage, - comments, - }); + + if (comments) { + requestData.instructorComments = comments; + } + + this.accountService.createAccountRequest(requestData) + .pipe(finalize(() => { this.isLoading = false; })) + .subscribe({ + next: () => { + // Pass form input to parent to display confirmation + this.requestSubmissionEvent.emit({ + name, + institution, + country, + email, + comments, + }); + }, + error: (resp: ErrorMessageOutput) => { + this.serverErrorMessage = resp.error.message; + }, + }); } } diff --git a/src/web/app/pages-static/request-page/request-page.component.html b/src/web/app/pages-static/request-page/request-page.component.html index 315ecb5b534..877da7af395 100644 --- a/src/web/app/pages-static/request-page/request-page.component.html +++ b/src/web/app/pages-static/request-page/request-page.component.html @@ -41,13 +41,6 @@

Email {{submittedFormData.email}} - - Home Page URL - - {{submittedFormData.homePage}} - - - Comments diff --git a/src/web/app/pages-static/request-page/request-page.component.spec.ts b/src/web/app/pages-static/request-page/request-page.component.spec.ts index 07b1c5d5dd2..9f4c3042247 100644 --- a/src/web/app/pages-static/request-page/request-page.component.spec.ts +++ b/src/web/app/pages-static/request-page/request-page.component.spec.ts @@ -42,7 +42,6 @@ describe('RequestPageComponent', () => { institution: 'University of Example', country: 'Example Republic', email: 'js@exampleu.edu', - homePage: 'u.exampleu.edu/jsmith', comments: '', }; fixture.detectChanges(); diff --git a/src/web/app/pages-static/request-page/request-page.module.ts b/src/web/app/pages-static/request-page/request-page.module.ts index 7333207fc0b..12a9d337875 100644 --- a/src/web/app/pages-static/request-page/request-page.module.ts +++ b/src/web/app/pages-static/request-page/request-page.module.ts @@ -2,6 +2,7 @@ import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; import { ReactiveFormsModule } from '@angular/forms'; import { RouterModule, Routes } from '@angular/router'; +import { NgbAlertModule } from '@ng-bootstrap/ng-bootstrap'; import { InstructorRequestFormComponent } from './instructor-request-form/instructor-request-form.component'; import { RequestPageComponent } from './request-page.component'; import { TeammatesRouterModule } from '../../components/teammates-router/teammates-router.module'; @@ -29,6 +30,7 @@ const routes: Routes = [ RouterModule.forChild(routes), TeammatesRouterModule, ReactiveFormsModule, + NgbAlertModule, ], }) export class RequestPageModule { } diff --git a/src/web/types/const.spec.ts b/src/web/types/const.spec.ts index f5f403835ec..6bc4e7112ec 100644 --- a/src/web/types/const.spec.ts +++ b/src/web/types/const.spec.ts @@ -1,4 +1,4 @@ -import { ApiConst } from './api-const'; +import { ApiConst, ApiStringConst } from './api-const'; import { FeedbackQuestionType } from './api-output'; import { DEFAULT_INSTRUCTOR_PRIVILEGE, @@ -68,6 +68,12 @@ describe('Constants', () => { expect(typeof ApiConst.NO_VALUE).toEqual('number'); }); + // Here we test that the constants are strings + it('should generate string constants correctly', () => { + expect(typeof ApiStringConst.EMAIL_REGEX).toEqual('string'); + expect(() => new RegExp(ApiStringConst.EMAIL_REGEX)).not.toThrow(); + }); + // Here we test that: // 1. The string is parseable to JSON // 2. The question type is correct diff --git a/src/web/types/form-validator.ts b/src/web/types/form-validator.ts index c2f44cf5beb..ea18815ca5d 100644 --- a/src/web/types/form-validator.ts +++ b/src/web/types/form-validator.ts @@ -1,4 +1,4 @@ -import { ApiConst } from './api-const'; +import { ApiConst, ApiStringConst } from './api-const'; /** * Represents the root FormValidator object of all form fields. @@ -33,4 +33,35 @@ export enum FormValidator { * Max length for the 'E-mail Address` field. */ EMAIL_MAX_LENGTH = ApiConst.EMAIL_MAX_LENGTH, + + /** + * Regex used to verify emails in the back-end. + */ + EMAIL_REGEX = ApiStringConst.EMAIL_REGEX, + + /** + * Regex used to verify names. + * + * Based on back-end's `FieldValidator.REGEX_NAME`. + * The back-end regex is not converted to use here as the pattern syntax is not accepted in JS. + */ + NAME_REGEX = '^[a-zA-Z0-9][^|%]*$', + + /** + * Regex used to verify country names. + * + * Based on back-end's `FieldValidator.REGEX_NAME`, but without needing to start with alphanumeric + * as the country is added to the end of the combined institute string. + */ + COUNTRY_REGEX = '^[^|%]*$', + + /** + * Max length for institution name in account request. (to be combined with country) + */ + INSTITUTION_NAME_MAX_LENGTH = 86, + + /** + * Max length for country in account request. (to be combined with institution name) + */ + COUNTRY_NAME_MAX_LENGTH = 40, }