Skip to content

Conversation

@vttranlina
Copy link
Member

No description provided.

Copy link
Member

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the test is useful, it do not give enough garanties.

Can we write test in other part of the code, eg in AlarmTriggerService ?

Image

We can clearly see that planning of the next occurrence happens only for recurring events. And not for events with several VALARMS.

Also this logic is flowed:

Optional<AlarmInstant> computeNextAlarmInstant(Calendar calendar, Username username);

If run falst enough we double trigger alarms which is a reported bug.

Evolution suggestion:

Optional<AlarmInstant> computeNextAlarmInstant(Calendar calendar, Username username, AlarmInstant latestTriggerAlarm);

with the constraint of the returned AlarmInstant to be strictly greater.

Can thos 2 changes be added here please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants