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

SAK-50469 Calendar getEvents default to a one year range before and after when passed a null #12852

Merged
merged 6 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ public void removeCalendar(CalendarEdit edit)
* Takes several calendar References and merges their events from within a given time range.
*
* @param references The List of calendar References.
* @param range The time period to use to select events. If this is null, all times will be retrieved
* @param range The time period to use to select events. If this is null, one year before and after will be used.
* @return CalendarEventVector object with the union of all events from the list of calendars in the given time range.
*/
public CalendarEventVector getEvents(List references, TimeRange range);
Expand All @@ -367,7 +367,7 @@ public void removeCalendar(CalendarEdit edit)
* Takes several calendar References and merges their events from within a given time range.
*
* @param references The List of calendar References.
* @param range The time period to use to select events. If this is null, all times will be retrieved
* @param range The time period to use to select events. If this is null, one year before and after will be used.
* @param reverseOrder CalendarEventVector object will be ordered reverse.
* @return CalendarEventVector object with the union of all events from the list of calendars in the given time range.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ public CalendarEventVector getEvents(List references, TimeRange range, boolean r

if (references != null) {
if (range == null) {
range = m_timeService.newTimeRange(Instant.EPOCH, Instant.MAX);
range = getOneYearTimeRange();
}
List allEvents = new ArrayList();

Expand Down Expand Up @@ -385,7 +385,7 @@ private List<CalendarEvent> getEvents(List<String> references, TimeRange range,

if (references != null) {
if (range == null) {
range = m_timeService.newTimeRange(Instant.EPOCH, Instant.MAX);
range = getOneYearTimeRange();
}
for (String ref : references) {
try {
Expand Down Expand Up @@ -5939,6 +5939,17 @@ public boolean isCalendarToolInitialized(String siteId){
return true;
}

// Private helper method to generate a time range one year before and one year after the current time
private TimeRange getOneYearTimeRange() {
Instant now = Instant.now();

// Create a time range from one year ago to one year from now
Instant oneYearAgo = now.minus(365, ChronoUnit.DAYS);
Instant oneYearLater = now.plus(365, ChronoUnit.DAYS);

return m_timeService.newTimeRange(oneYearAgo, oneYearLater);
}

private String getDirectToolUrl(String siteId) throws IdUnusedException {
ToolConfiguration toolConfig = m_siteService.getSite(siteId).getToolForCommonId("sakai.schedule");
return m_serverConfigurationService.getPortalUrl() + "/directtool/" + toolConfig.getId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.TimeZone;
import java.util.Vector;

import lombok.extern.slf4j.Slf4j;
import org.w3c.dom.Element;

import org.xml.sax.Attributes;
Expand All @@ -44,6 +45,7 @@
/**
* This is a common base for the daily, weekly, monthly, and yearly recurrence rules.
*/
@Slf4j
public abstract class RecurrenceRuleBase implements RecurrenceRule
{
/** Every this many number of units: 1 would be daily/monthly/annually. */
Expand Down Expand Up @@ -103,7 +105,7 @@ public RecurrenceRuleBase(int interval, Time until)
* @param range A time range to limit the generated ranges.
* @param timeZone The time zone to use for displaying times.
* %%% Note: this is currently not implemented, and always uses the "local" zone.
* @return a List of RecurrenceInstance generated by this rule in this range.
* @return a List of max 1000 RecurrenceInstance generated by this rule in this range.
*/
public List generateInstances(TimeRange prototype, TimeRange range, TimeZone timeZone)
{
Expand Down Expand Up @@ -149,6 +151,7 @@ public List generateInstances(TimeRange prototype, TimeRange range, TimeZone tim
(GregorianCalendar) startCalendarDate.clone();

int currentCount = 1;
int maxItems = 1000;

do
{
Expand Down Expand Up @@ -189,11 +192,20 @@ public List generateInstances(TimeRange prototype, TimeRange range, TimeZone tim

// use this one
rv.add(new RecurrenceInstance(eventTimeRange, currentCount));

// Break if we reach the max limit
if (rv.size() >= maxItems)
{
log.warn("Reached the maximum limit of 1000 items for recurring events.");
break;
ottenhoff marked this conversation as resolved.
Show resolved Hide resolved
}
}

// if next starts after the range, stop generating
else if (isAfter(nextTime, range.lastTime()))
break;
{
break;
}

// advance interval years.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ public TimeRange newTimeRange(Time start, Time end)

public TimeRange newTimeRange(Instant start, Instant end)
{
return new MyTimeRange(newTime(start.getEpochSecond()), newTime(end.getEpochSecond()));
return new MyTimeRange(newTime(start.toEpochMilli()), newTime(end.toEpochMilli()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
import org.sakaiproject.time.impl.MyTime;
import org.sakaiproject.time.impl.BasicTimeService.MyTimeRange;

import java.time.Instant;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -70,4 +74,48 @@ public void testDurtation() {
assertEquals(tr1.duration(),42l);
}

@Test
public void newTimeRange_withValidInstants_shouldReturnCorrectTimeRange() {
BasicTimeService timeService = new BasicTimeService();
Instant start = Instant.parse("2023-01-01T00:00:00Z");
Instant end = Instant.parse("2023-01-02T00:00:00Z");

TimeRange timeRange = timeService.newTimeRange(start, end);

assertNotNull(timeRange);
assertEquals(start.toEpochMilli(), timeRange.firstTime().getTime());
assertEquals(end.toEpochMilli(), timeRange.lastTime().getTime());

// The duration should be 1 day
assertEquals(timeRange.duration(), 86400000);
}

@Test
public void newTimeRange_withStartAfterEnd_shouldSwapTimes() {
BasicTimeService timeService = new BasicTimeService();
Instant start = Instant.parse("2023-01-02T00:00:00Z");
Instant end = Instant.parse("2023-01-01T00:00:00Z");

TimeRange timeRange = timeService.newTimeRange(start, end);

assertNotNull(timeRange);
assertEquals(end.toEpochMilli(), timeRange.firstTime().getTime());
assertEquals(start.toEpochMilli(), timeRange.lastTime().getTime());
}

@Test
public void newTimeRange_withSameStartAndEnd_shouldReturnSingleTimeRange() {
BasicTimeService timeService = new BasicTimeService();
Instant start = Instant.parse("2023-01-01T00:00:00Z");

TimeRange timeRange = timeService.newTimeRange(start, start);

assertNotNull(timeRange);
assertEquals(start.toEpochMilli(), timeRange.firstTime().getTime());
assertEquals(start.toEpochMilli(), timeRange.lastTime().getTime());
assertTrue(timeRange.isSingleTime());

// Duration should be 0
assertEquals(timeRange.duration(), 0);
}
}
Loading