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

Resolve mismatch between FeedbackSession and FeedbackSessionAttributes isClosed, isOpened, isInGracePeriod #12758

Open
FergusMok opened this issue Feb 19, 2024 · 0 comments
Labels
c.Bug Bug/defect report committers only Difficult; better left for committers or more senior developers

Comments

@FergusMok
Copy link
Contributor

Currently, there's a mismatch in behavior of the methods during the migration.
FeedbackSession only takes into account endTime, but the original method takes into account graceTime, deadlineExtensions and endTime.

/**
* Checks if the feedback session is closed.
* This occurs only when the current time is after both the deadline and the grace period.
*/
public boolean isClosed() {
return !isOpened() && Instant.now().isAfter(endTime);
}
/**
* Checks if the feedback session is open.
* This occurs when the current time is either the start time or later but before the deadline.
*/
public boolean isOpened() {
Instant now = Instant.now();
return (now.isAfter(startTime) || now.equals(startTime)) && now.isBefore(endTime);
}
/**
* Checks if the feedback session is during the grace period.
* This occurs when the current time is after end time, but before the end of the grace period.
*/
public boolean isInGracePeriod() {
return Instant.now().isAfter(endTime) && !isClosed();
}

/**
* Checks if the feedback session is closed.
* This occurs when the current time is after both the deadline and the grace period.
*/
public boolean isClosed() {
return Instant.now().isAfter(getDeadline().plus(gracePeriod));
}
/**
* Checks if the feedback session is open.
* This occurs when the current time is either the start time or later but before the deadline.
*/
public boolean isOpened() {
Instant now = Instant.now();
return (now.isAfter(startTime) || now.equals(startTime)) && now.isBefore(getDeadline());
}
/**
* Checks if the feedback session is closed but still accepts responses.
* This occurs when the current time is either the deadline or later but still within the grace period.
*/
public boolean isInGracePeriod() {
Instant now = Instant.now();
Instant deadline = getDeadline();
Instant gracedEnd = deadline.plus(gracePeriod);
return (now.isAfter(deadline) || now.equals(deadline)) && (now.isBefore(gracedEnd) || now.equals(gracedEnd));
}

The actual behavior should instead use these methods:

/**
* Checks if the feedback session is opened given the extendedDeadline and grace period.
*/
public boolean isOpenedGivenExtendedDeadline(Instant extendedDeadline) {
Instant now = Instant.now();
return (now.isAfter(startTime) || now.equals(startTime))
&& now.isBefore(extendedDeadline.plus(gracePeriod)) || now.isBefore(endTime.plus(gracePeriod));
}
/**
* Checks if the feedback session is closed given the extendedDeadline and grace period.
* This occurs only when it is after the extended deadline or end time plus grace period.
*/
public boolean isClosedGivenExtendedDeadline(Instant extendedDeadline) {
Instant now = Instant.now();
return !isOpenedGivenExtendedDeadline(extendedDeadline)
&& now.isAfter(endTime.plus(gracePeriod)) && now.isAfter(extendedDeadline.plus(gracePeriod));
}
/**
* Checks if the feedback session is during the grace period given the extendedDeadline.
*/
public boolean isInGracePeriodGivenExtendedDeadline(Instant extendedDeadline) {
Instant now = Instant.now();
return now.isAfter(endTime) && now.isAfter(extendedDeadline) && !isClosedGivenExtendedDeadline(extendedDeadline);
}

Impact of bug
During the migration, due to similarity in names, some migrated actions are utilizing the wrong method.

Suggested change

  1. As the FeedbackSessionAttributes did not need a separate isXXXGivenExtendedDeadline set of methods, a suggestion is to rename them inFeedbackSession and make the method signatures match the set of methods in FeedbackSessionAttributes.
  2. Implement additional unit tests if need be.
@FergusMok FergusMok added c.Bug Bug/defect report committers only Difficult; better left for committers or more senior developers labels Feb 19, 2024
@FergusMok FergusMok changed the title Mismatch between FeedbackSession and FeedbackSessionAttributes isClosed, isOpened, isInGracePeriod Resolve mismatch between FeedbackSession and FeedbackSessionAttributes isClosed, isOpened, isInGracePeriod Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Bug Bug/defect report committers only Difficult; better left for committers or more senior developers
Projects
None yet
Development

No branches or pull requests

1 participant