Skip to content

Commit 41f368b

Browse files
committed
Fix bug with counting number of occurrences (some occurrences missed)
Switch Recurrence to use LocalDateTimes to avoid time zone and DST issues Fix breaking tests
1 parent 8fd061f commit 41f368b

File tree

2 files changed

+40
-15
lines changed

2 files changed

+40
-15
lines changed

app/src/main/java/org/gnucash/android/model/Recurrence.java

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
import org.joda.time.DateTime;
2626
import org.joda.time.Days;
2727
import org.joda.time.LocalDate;
28+
import org.joda.time.LocalDateTime;
2829
import org.joda.time.Months;
30+
import org.joda.time.ReadablePeriod;
2931
import org.joda.time.Weeks;
3032
import org.joda.time.Years;
3133

@@ -273,34 +275,60 @@ public int getCount(){
273275
if (mPeriodEnd == null)
274276
return -1;
275277

276-
int count = 0;
277-
DateTime startDate = new DateTime(mPeriodStart.getTime());
278-
DateTime endDate = new DateTime(mPeriodEnd.getTime());
278+
int multiple = mPeriodType.getMultiplier();
279+
ReadablePeriod jodaPeriod;
279280
switch (mPeriodType){
280281
case DAY:
281-
count = Days.daysBetween(startDate, endDate).getDays();
282+
jodaPeriod = Days.days(multiple);
282283
break;
283284
case WEEK:
284-
count = Weeks.weeksBetween(startDate, endDate).getWeeks();
285+
jodaPeriod = Weeks.weeks(multiple);
285286
break;
286287
case MONTH:
287-
count = Months.monthsBetween(startDate, endDate).getMonths();
288+
jodaPeriod = Months.months(multiple);
288289
break;
289290
case YEAR:
290-
count = Years.yearsBetween(startDate, endDate).getYears();
291+
jodaPeriod = Years.years(multiple);
291292
break;
293+
default:
294+
jodaPeriod = Months.months(multiple);
295+
}
296+
int count = 0;
297+
LocalDateTime startTime = new LocalDateTime(mPeriodStart.getTime());
298+
while (startTime.toDateTime().getMillis() < mPeriodEnd.getTime()){
299+
++count;
300+
startTime = startTime.plus(jodaPeriod);
292301
}
302+
return count;
293303

294-
return count/mPeriodType.getMultiplier();
304+
/*
305+
//this solution does not use looping, but is not very accurate
306+
307+
int multiplier = mPeriodType.getMultiplier();
308+
LocalDateTime startDate = new LocalDateTime(mPeriodStart.getTime());
309+
LocalDateTime endDate = new LocalDateTime(mPeriodEnd.getTime());
310+
switch (mPeriodType){
311+
case DAY:
312+
return Days.daysBetween(startDate, endDate).dividedBy(multiplier).getDays();
313+
case WEEK:
314+
return Weeks.weeksBetween(startDate, endDate).dividedBy(multiplier).getWeeks();
315+
case MONTH:
316+
return Months.monthsBetween(startDate, endDate).dividedBy(multiplier).getMonths();
317+
case YEAR:
318+
return Years.yearsBetween(startDate, endDate).dividedBy(multiplier).getYears();
319+
default:
320+
return -1;
321+
}
322+
*/
295323
}
296324

297325
/**
298326
* Sets the end time of this recurrence by specifying the number of occurences
299327
* @param numberOfOccurences Number of occurences from the start time
300328
*/
301329
public void setPeriodEnd(int numberOfOccurences){
302-
DateTime localDate = new DateTime(mPeriodStart.getTime());
303-
DateTime endDate;
330+
LocalDateTime localDate = new LocalDateTime(mPeriodStart.getTime());
331+
LocalDateTime endDate;
304332
int occurrenceDuration = numberOfOccurences * mPeriodType.getMultiplier();
305333
switch (mPeriodType){
306334
case DAY:
@@ -317,7 +345,7 @@ public void setPeriodEnd(int numberOfOccurences){
317345
endDate = localDate.plusYears(occurrenceDuration);
318346
break;
319347
}
320-
mPeriodEnd = new Timestamp(endDate.getMillis());
348+
mPeriodEnd = new Timestamp(endDate.toDateTime().getMillis());
321349
}
322350

323351
/**

app/src/test/java/org/gnucash/android/test/unit/service/ScheduledActionServiceTest.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,7 @@ public void missedScheduledTransactions_shouldBeGenerated(){
199199
actions.add(scheduledAction);
200200
ScheduledActionService.processScheduledActions(actions, mDb);
201201

202-
int weeks = Weeks.weeksBetween(startTime, new DateTime(2016, 8, 29, 10, 0)).getWeeks();
203-
int expectedTransactionCount = weeks/2;
204-
205-
assertThat(transactionsDbAdapter.getRecordsCount()).isEqualTo(expectedTransactionCount);
202+
assertThat(transactionsDbAdapter.getRecordsCount()).isEqualTo(7);
206203
}
207204

208205
public void endTimeInTheFuture_shouldExecuteOnlyUntilPresent(){

0 commit comments

Comments
 (0)