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.

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

[#12494] Allow instructors to use start/end dates up to 12 months in future #12498

Merged
merged 24 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5a1e7d6
feat: add safety net for invalid dates
Zxun2 Jun 26, 2023
41b3717
feat: update start/end dates from 3 to 12 months
Zxun2 Jun 26, 2023
71c36f1
Merge branch 'master' into zongxun/use-start-dates-in-future
Zxun2 Jun 30, 2023
b914726
perf: reduce side effects
Zxun2 Jun 30, 2023
db8790f
Merge branch 'zongxun/use-start-dates-in-future' of https://github.co…
Zxun2 Jun 30, 2023
ddb339a
Merge branch 'master' into zongxun/use-start-dates-in-future
domlimm Jul 2, 2023
48f0f47
fix: update error message
Zxun2 Jul 3, 2023
9268bd3
feat: add safety net for time
Zxun2 Jul 3, 2023
857d7c7
Merge branch 'zongxun/use-start-dates-in-future' of https://github.co…
Zxun2 Jul 3, 2023
2e6e741
feat: testcase for `getInstantMonthsOffsetFromNow`
Zxun2 Jul 4, 2023
10f6459
fix: failing component tests
Zxun2 Jul 4, 2023
188f4bf
Merge branch 'master' into zongxun/use-start-dates-in-future
Zxun2 Jul 4, 2023
5deb933
Merge branch 'master' into zongxun/use-start-dates-in-future
Zxun2 Jul 6, 2023
9912e1e
refactor(sessions-edit-form): abstract model to base class
Zxun2 Jul 6, 2023
f8fda23
fix: linting
Zxun2 Jul 6, 2023
2a70f59
fix: inaccurate end time when start date > end date
Zxun2 Jul 7, 2023
6a246ac
misc: update javadocs
Zxun2 Jul 11, 2023
3bd19ee
misc: change twelve to 12
Zxun2 Jul 11, 2023
f55bcf7
fix: update missing attribute
Zxun2 Jul 11, 2023
105d262
fix: add missing edge case
Zxun2 Jul 11, 2023
2069940
Merge branch 'zongxun/use-start-dates-in-future' of https://github.co…
Zxun2 Jul 11, 2023
ab7e5af
fix: add missing edge case when startDate > endDate and hours are equal
Zxun2 Jul 12, 2023
2ab6738
Merge branch 'master' into zongxun/use-start-dates-in-future
Zxun2 Jul 12, 2023
15e44ea
Merge branch 'master' into zongxun/use-start-dates-in-future
weiquu Jul 12, 2023
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
30 changes: 16 additions & 14 deletions src/main/java/teammates/common/util/FieldValidator.java
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
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();
}

Zxun2 marked this conversation as resolved.
Show resolved Hide resolved
/**
* 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
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
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
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);
}

}
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
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
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) {
Zxun2 marked this conversation as resolved.
Show resolved Hide resolved
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
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