Skip to content

Commit

Permalink
[TEAMMATES#12494] Allow instructors to use start/end dates up to 12 m…
Browse files Browse the repository at this point in the history 
…onths in future (TEAMMATES#12498)

* feat: add safety net for invalid dates

* feat: update start/end dates from 3 to 12 months

* perf: reduce side effects

* fix: update error message

* feat: add safety net for time

* feat: testcase for `getInstantMonthsOffsetFromNow`

* fix: failing component tests

* refactor(sessions-edit-form): abstract model to base class

* fix: linting

* fix: inaccurate end time when start date > end date

* misc: update javadocs

* misc: change twelve to 12

* fix: update missing attribute

* fix: add missing edge case

* fix: add missing edge case when startDate > endDate and hours are equal

---------

Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com>
Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com>
  • Loading branch information
@Zxun2 @domlimm @weiquu
3 people committed Jul 12, 2023
1 parent 4dfa419 commit 930c2b1
Show file tree
Hide file tree
Showing 12 changed files with 174 additions and 114 deletions.
30 changes: 16 additions & 14 deletions src/main/java/teammates/common/util/FieldValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ static String getValidityInfoForSizeCappedPossiblyEmptyString(String fieldName,
* Checks if the {@code startTime} is valid to be used as a session start time.
* Returns an empty string if it is valid, or an error message otherwise.
*
* <p>The {@code startTime} is valid if it is after 2 hours before now, before 90 days from now
* <p>The {@code startTime} is valid if it is after 2 hours before now, before 12 months from now
* and at exact hour mark.
*/
public static String getInvalidityInfoForNewStartTime(Instant startTime, String timeZone) {
Expand All @@ -655,18 +655,20 @@ public static String getInvalidityInfoForNewStartTime(Instant startTime, String
"2 hours before now", SESSION_START_TIME_FIELD_NAME,
(firstTime, secondTime) -> firstTime.isBefore(secondTime) || firstTime.equals(secondTime),
"The %s for this %s cannot be earlier than %s.");

if (!earlierThanThreeHoursBeforeNowError.isEmpty()) {
return earlierThanThreeHoursBeforeNowError;
}

Instant ninetyDaysFromNow = TimeHelper.getInstantDaysOffsetFromNow(90);
String laterThanNinetyDaysFromNowError = getInvalidityInfoForFirstTimeComparedToSecondTime(
ninetyDaysFromNow, startTime, SESSION_NAME,
"90 days from now", SESSION_START_TIME_FIELD_NAME,
Instant twelveMonthsFromNow = TimeHelper.getInstantMonthsOffsetFromNow(12, timeZone);
String laterThanTwelveMonthsFromNowError = getInvalidityInfoForFirstTimeComparedToSecondTime(
twelveMonthsFromNow, startTime, SESSION_NAME,
"12 months from now", SESSION_START_TIME_FIELD_NAME,
(firstTime, secondTime) -> firstTime.isAfter(secondTime) || firstTime.equals(secondTime),
"The %s for this %s cannot be later than %s.");
if (!laterThanNinetyDaysFromNowError.isEmpty()) {
return laterThanNinetyDaysFromNowError;

if (!laterThanTwelveMonthsFromNowError.isEmpty()) {
return laterThanTwelveMonthsFromNowError;
}

String notExactHourError = getInvalidityInfoForExactHourTime(startTime, timeZone, "start time");
Expand All @@ -681,7 +683,7 @@ public static String getInvalidityInfoForNewStartTime(Instant startTime, String
* Checks if the {@code endTime} is valid to be used as a session end time.
* Returns an empty string if it is valid, or an error message otherwise.
*
* <p>The {@code endTime} is valid if it is after 1 hour before now, before 180 days from now
* <p>The {@code endTime} is valid if it is after 1 hour before now, before 12 months from now
* and at exact hour mark.
*/
public static String getInvalidityInfoForNewEndTime(Instant endTime, String timeZone) {
Expand All @@ -695,14 +697,14 @@ public static String getInvalidityInfoForNewEndTime(Instant endTime, String time
return earlierThanThreeHoursBeforeNowError;
}

Instant oneHundredEightyDaysFromNow = TimeHelper.getInstantDaysOffsetFromNow(180);
String laterThanOneHundredEightyDaysError = getInvalidityInfoForFirstTimeComparedToSecondTime(
oneHundredEightyDaysFromNow, endTime, SESSION_NAME,
"180 days from now", SESSION_END_TIME_FIELD_NAME,
Instant twelveMonthsFromNow = TimeHelper.getInstantMonthsOffsetFromNow(12, timeZone);
String laterThanTwelveMonthsError = getInvalidityInfoForFirstTimeComparedToSecondTime(
twelveMonthsFromNow, endTime, SESSION_NAME,
"12 months from now", SESSION_END_TIME_FIELD_NAME,
(firstTime, secondTime) -> firstTime.isAfter(secondTime) || firstTime.equals(secondTime),
"The %s for this %s cannot be later than %s.");
if (!laterThanOneHundredEightyDaysError.isEmpty()) {
return laterThanOneHundredEightyDaysError;
if (!laterThanTwelveMonthsError.isEmpty()) {
return laterThanTwelveMonthsError;
}

String notExactHourError = getInvalidityInfoForExactHourTime(endTime, timeZone, "end time");
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/teammates/common/util/TimeHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,20 @@ public static Instant getInstantDaysOffsetFromNow(long offsetInDays) {
return Instant.now().plus(Duration.ofDays(offsetInDays));
}

/**
* Returns an Instant that is offset by a number of months from now.
*
* @param offsetInMonths integer number of months to offset by
* @param timeZone string representing the time zone to compute local datetime
* @return an Instant offset by {@code offsetInMonths} days
*/
public static Instant getInstantMonthsOffsetFromNow(long offsetInMonths, String timeZone) {
Instant now = Instant.now();
ZonedDateTime zdt = now.atZone(ZoneId.of(timeZone));
ZonedDateTime offsetZdt = zdt.plusMonths(offsetInMonths);
return offsetZdt.toInstant();
}

/**
* Returns an Instant that is offset by a number of days before now.
*
Expand Down
16 changes: 8 additions & 8 deletions src/test/java/teammates/common/util/FieldValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,11 +536,11 @@ public void testGetInvalidityInfoForNewStartTime_invalid_returnErrorString() {
assertEquals("The start time for this feedback session cannot be earlier than 2 hours before now.",
FieldValidator.getInvalidityInfoForNewStartTime(threeHoursBeforeNowRounded, Const.DEFAULT_TIME_ZONE));

Instant ninetyOneDaysFromNowRounded = TimeHelperExtension
.getInstantDaysOffsetFromNow(91)
Instant thirteenMonthsFromNow = TimeHelperExtension
.getInstantMonthsOffsetFromNow(13, Const.DEFAULT_TIME_ZONE)
.truncatedTo(ChronoUnit.HOURS);
assertEquals("The start time for this feedback session cannot be later than 90 days from now.",
FieldValidator.getInvalidityInfoForNewStartTime(ninetyOneDaysFromNowRounded, Const.DEFAULT_TIME_ZONE));
assertEquals("The start time for this feedback session cannot be later than 12 months from now.",
FieldValidator.getInvalidityInfoForNewStartTime(thirteenMonthsFromNow, Const.DEFAULT_TIME_ZONE));

Instant notAtHourMark = TimeHelperExtension
.getInstantHoursOffsetFromNow(1)
Expand Down Expand Up @@ -571,11 +571,11 @@ public void testGetInvalidityInfoForNewEndTime_invalid_returnErrorString() {
assertEquals("The end time for this feedback session cannot be earlier than 1 hour before now.",
FieldValidator.getInvalidityInfoForNewEndTime(twoHoursBeforeNowRounded, Const.DEFAULT_TIME_ZONE));

Instant oneHundredAndEightyOneDaysFromNowRounded = TimeHelperExtension
.getInstantDaysOffsetFromNow(181)
Instant thirteenMonthsFromNow = TimeHelperExtension
.getInstantMonthsOffsetFromNow(13, Const.DEFAULT_TIME_ZONE)
.truncatedTo(ChronoUnit.HOURS);
assertEquals("The end time for this feedback session cannot be later than 180 days from now.",
FieldValidator.getInvalidityInfoForNewEndTime(oneHundredAndEightyOneDaysFromNowRounded,
assertEquals("The end time for this feedback session cannot be later than 12 months from now.",
FieldValidator.getInvalidityInfoForNewEndTime(thirteenMonthsFromNow,
Const.DEFAULT_TIME_ZONE));

Instant notAtHourMark = TimeHelperExtension
Expand Down
14 changes: 14 additions & 0 deletions src/test/java/teammates/common/util/TimeHelperExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ public static Instant getInstantDaysOffsetFromNow(long offsetInDays) {
return Instant.now().plus(Duration.ofDays(offsetInDays));
}

/**
* Returns an Instant that is offset by a number of months from now.
*
* @param offsetInMonths integer number of months to offset by
* @param timeZone string representing the time zone to compute local datetime
* @return an Instant offset by {@code offsetInMonths} days
*/
public static Instant getInstantMonthsOffsetFromNow(long offsetInMonths, String timeZone) {
Instant now = Instant.now();
ZonedDateTime zdt = now.atZone(ZoneId.of(timeZone));
ZonedDateTime offsetZdt = zdt.plusMonths(offsetInMonths);
return offsetZdt.toInstant();
}

/**
* Returns an java.time.Instant object that is offset by a number of days from now truncated to days.
* @param offsetInDays number of days offset by (integer).
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/teammates/common/util/TimeHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.time.Month;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;

import org.testng.annotations.Test;
Expand Down Expand Up @@ -130,4 +131,19 @@ public void testGetInstantHoursOffsetFromNow() {
assertEquals(expected, actual);
}

@Test
public void testGetInstantMonthsOffsetFromNow() {
Instant expected = Instant.now().truncatedTo(ChronoUnit.DAYS);
Instant actual = TimeHelper.getInstantMonthsOffsetFromNow(0, Const.DEFAULT_TIME_ZONE)
.truncatedTo(ChronoUnit.DAYS);
assertEquals(expected, actual);

Instant now = Instant.now();
ZonedDateTime zdt = now.atZone(ZoneId.of(Const.DEFAULT_TIME_ZONE));
ZonedDateTime offsetZdt = zdt.plusMonths(12);
expected = offsetZdt.toInstant().truncatedTo(ChronoUnit.SECONDS);
actual = TimeHelper.getInstantMonthsOffsetFromNow(12, Const.DEFAULT_TIME_ZONE).truncatedTo(ChronoUnit.SECONDS);
assertEquals(expected, actual);
}

}
16 changes: 8 additions & 8 deletions src/web/app/components/session-edit-form/session-edit-form.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,17 +175,17 @@ export class SessionEditFormComponent {
/**
* Gets the maximum date for a session to be opened.
*
* <p> The maximum session opening datetime is 90 days from now.
* <p> The maximum session opening datetime is 12 months from now.
*/
get maxDateForSubmissionStart(): DateFormat {
const ninetyDaysFromNow = moment().tz(this.model.timeZone).add(90, 'days');
return this.datetimeService.getDateInstance(ninetyDaysFromNow);
const twelveMonthsFromNow = moment().tz(this.model.timeZone).add(12, 'months');
return this.datetimeService.getDateInstance(twelveMonthsFromNow);
}

/**
* Gets the maximum time for a session to be opened.
*
* <p> The maximum session opening datetime is 90 days from now.
* <p> The maximum session opening time is 23:59h.
*/
get maxTimeForSubmissionStart(): TimeFormat {
return getLatestTimeFormat();
Expand Down Expand Up @@ -232,17 +232,17 @@ export class SessionEditFormComponent {
/**
* Gets the maximum date for a session to be closed.
*
* <p> The maximum session closing datetime is 180 days from now.
* <p> The maximum session closing datetime is 12 months from now.
*/
get maxDateForSubmissionEnd(): DateFormat {
const oneHundredAndEightyDaysFromNow = moment().tz(this.model.timeZone).add(180, 'days');
return this.datetimeService.getDateInstance(oneHundredAndEightyDaysFromNow);
const twelveMonthsFromNow = moment().tz(this.model.timeZone).add(12, 'months');
return this.datetimeService.getDateInstance(twelveMonthsFromNow);
}

/**
* Gets the maximum time for a session to be closed.
*
* <p> The maximum session closing datetime is 180 days from now.
* <p> The maximum session closing time is 23:59H.
*/
get maxTimeForSubmissionEnd(): TimeFormat {
return getLatestTimeFormat();
Expand Down
8 changes: 8 additions & 0 deletions ...instructor/instructor-home-page/__snapshots__/instructor-home-page.component.spec.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ exports[`InstructorHomePageComponent should snap when courses are still loading
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down Expand Up @@ -105,6 +106,7 @@ exports[`InstructorHomePageComponent should snap with default fields 1`] = `
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down Expand Up @@ -179,6 +181,7 @@ exports[`InstructorHomePageComponent should snap with one course with error load
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down Expand Up @@ -329,6 +332,7 @@ exports[`InstructorHomePageComponent should snap with one course with one feedba
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down Expand Up @@ -846,6 +850,7 @@ exports[`InstructorHomePageComponent should snap with one course with two feedba
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down Expand Up @@ -1509,6 +1514,7 @@ exports[`InstructorHomePageComponent should snap with one course with unexpanded
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down Expand Up @@ -1760,6 +1766,7 @@ exports[`InstructorHomePageComponent should snap with one course with unpopulate
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down Expand Up @@ -2035,6 +2042,7 @@ exports[`InstructorHomePageComponent should snap with one course without feedbac
numberOfSessionsCopied="0"
progressBarService={[Function ProgressBarService]}
publishUnpublishRetryAttempts={[Function Number]}
sessionEditFormModel={[Function Object]}
simpleModalService={[Function SimpleModalService]}
statusMessageService={[Function StatusMessageService]}
studentService={[Function StudentService]}
Expand Down
81 changes: 81 additions & 0 deletions src/web/app/pages-instructor/instructor-session-base-page.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,19 @@ import {
FeedbackQuestion,
FeedbackQuestions,
FeedbackSession,
FeedbackSessionPublishStatus,
FeedbackSessionStats,
FeedbackSessionSubmissionStatus,
ResponseVisibleSetting,
SessionVisibleSetting,
} from '../../types/api-output';
import { Intent } from '../../types/api-request';
import { getDefaultDateFormat, getLatestTimeFormat } from '../../types/datetime-const';
import { DEFAULT_NUMBER_OF_RETRY_ATTEMPTS } from '../../types/default-retry-attempts';
import { SortBy, SortOrder } from '../../types/sort-properties';
import { CopySessionModalResult } from '../components/copy-session-modal/copy-session-modal-model';
import { ErrorReportComponent } from '../components/error-report/error-report.component';
import { SessionEditFormModel } from '../components/session-edit-form/session-edit-form-model';
import { CopySessionResult, SessionsTableRowModel } from '../components/sessions-table/sessions-table-model';
import { SimpleModalType } from '../components/simple-modal/simple-modal-type';
import { ErrorMessageOutput } from '../error-message-output';
Expand All @@ -43,6 +47,43 @@ export abstract class InstructorSessionBasePageComponent {

private publishUnpublishRetryAttempts: number = DEFAULT_NUMBER_OF_RETRY_ATTEMPTS;

sessionEditFormModel: SessionEditFormModel = {
courseId: '',
timeZone: 'UTC',
courseName: '',
feedbackSessionName: '',
instructions: '',

submissionStartTime: getLatestTimeFormat(),
submissionStartDate: getDefaultDateFormat(),
submissionEndTime: getLatestTimeFormat(),
submissionEndDate: getDefaultDateFormat(),
gracePeriod: 0,

sessionVisibleSetting: SessionVisibleSetting.AT_OPEN,
customSessionVisibleTime: getLatestTimeFormat(),
customSessionVisibleDate: getDefaultDateFormat(),

responseVisibleSetting: ResponseVisibleSetting.CUSTOM,
customResponseVisibleTime: getLatestTimeFormat(),
customResponseVisibleDate: getDefaultDateFormat(),

submissionStatus: FeedbackSessionSubmissionStatus.OPEN,
publishStatus: FeedbackSessionPublishStatus.NOT_PUBLISHED,

isClosingEmailEnabled: true,
isPublishedEmailEnabled: true,

templateSessionName: '',

isSaving: false,
isEditable: false,
isDeleting: false,
isCopying: false,
hasVisibleSettingsPanelExpanded: false,
hasEmailSettingsPanelExpanded: false,
};

protected constructor(protected instructorService: InstructorService,
protected statusMessageService: StatusMessageService,
protected navigationService: NavigationService,
Expand Down Expand Up @@ -470,6 +511,46 @@ export abstract class InstructorSessionBasePageComponent {
modal.componentInstance.requestId = resp.error.requestId;
modal.componentInstance.errorMessage = resp.error.message;
}

triggerModelChange(data: SessionEditFormModel): void {
const { submissionStartDate, submissionEndDate, submissionStartTime, submissionEndTime } = data;

const startDate = new Date(submissionStartDate.year, submissionStartDate.month, submissionStartDate.day);
const endDate = new Date(submissionEndDate.year, submissionEndDate.month, submissionEndDate.day);

if (startDate > endDate) {
this.sessionEditFormModel = {
...data,
submissionEndDate: submissionStartDate,
submissionEndTime:
submissionStartTime.hour > submissionEndTime.hour || (
submissionStartTime.hour === submissionEndTime.hour
&& submissionStartTime.minute > submissionEndTime.minute
)
? submissionStartTime
: submissionEndTime,
};
} else if (startDate.toISOString() === endDate.toISOString() && submissionStartTime.hour > submissionEndTime.hour) {
this.sessionEditFormModel = {
...data,
submissionEndDate: submissionStartDate,
submissionEndTime: {
...submissionStartTime,
hour: submissionStartTime.hour,
},
};
} else if (startDate.toISOString() === endDate.toISOString() && submissionStartTime.hour === submissionEndTime.hour
&& submissionStartTime.minute > submissionEndTime.minute) {
this.sessionEditFormModel = {
...data,
submissionEndDate: submissionStartDate,
submissionEndTime: {
...submissionStartTime,
minute: submissionStartTime.minute,
},
};
}
}
}

interface SessionTimestampData {
Expand Down
2 changes: 1 addition & 1 deletion ...pages-instructor/instructor-session-edit-page/instructor-session-edit-page.component.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
(retryEvent)="loadFeedbackSession()">
<tm-session-edit-form *tmIsLoading="isLoadingFeedbackSession" [formMode]="SessionEditFormMode.EDIT"
[(model)]="sessionEditFormModel" (editExistingSessionEvent)="editExistingSessionHandler()"
(modelChange)="triggerModelChange($event)"
(cancelEditingSessionEvent)="cancelEditingSessionHandler()"
(deleteExistingSessionEvent)="deleteExistingSessionHandler()"
(copyCurrentSessionEvent)="copyCurrentSession()"></tm-session-edit-form>
</tm-loading-retry>

<tm-loading-retry [shouldShowRetry]="hasLoadingFeedbackQuestionsFailed" [message]="'Failed to load feedback questions'"
(retryEvent)="loadFeedbackQuestions()">
<div *ngIf="!isLoadingFeedbackQuestions && questionEditFormModels.length" class="offset-md-10 margin-vertical-20px">
Expand Down
Loading

0 comments on commit 930c2b1

Please sign in to comment.