Updating the start and end time of an event sets the values wrongly depending on the order the operations are done.

[NachoVazquez]

Current Behavior

When I have an existing event, containing for example the following start and end:

{
...
 start: "2024-02-29T17:00:00.000Z",
 end: "2024-02-29T17:20:00.000Z"
...
}

If I shift the time to the future, for example, 2 hours, by using the start() and end() setters in that order, I get that the start value is the old end value.

  event.start("2024-02-29T19:00:00.000Z");
  event.end("2024-02-29T19:20:00.000Z");

Then

console.log(event.start()) // "2024-02-29T17:20:00.000Z"

Something similar happens by shifting to the past.

Expected Behavior

I understand the validations that introduce this issue, but I think we are missing an API to modify the start and end at the same time to avoid this kind of issue.

Another option is having something like a builder pattern where the checks only happen after calling the .build() method. But that probably is a bigger refactor.

Having a shiftTime method can help, but sometimes you want to change both the start time and the duration of the event.

Additional Information

My workaround for now is to set the start one more time after setting the end time.

  event.start("2024-02-29T19:00:00.000Z");
  event.end("2024-02-29T19:20:00.000Z");
  event.start("2024-02-29T19:00:00.000Z");

[sebbo2002]

Thanks for your bug report, you're absolutely right of course. This is a problem I have never thought about. I would like to avoid an additional method as it requires additional complexity when using the library. I think it's a manageable breaking change if we do the validation (start < end) only when calling generate() or toString() and swap the values if needed. Taht should fix your issue, right? If so, I can take care of it within the next few days and provide a test version.

[NachoVazquez]

Hey @sebbo2002, thanks for the quick response, and thanks for the library; it is really useful.

Yeah, that would work; another alternative that comes to mind is to make whether to run or not validations configurable and make the default always to run. That way, you would not have any breaking changes, although it might still be weird.

Anyhow, whenever you decide which solution makes more sense, I'm also open to creating a PR.

[sebbo2002]

🎉 This issue has been resolved in version 7.0.0-develop.2 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release
• v7.0.0-develop.2

Your semantic-release bot 📦🚀

[sebbo2002]

I have now given it some more thought and have come to the conclusion that I now always execute the validation when a start() or end() is used as a getter or toJSON() or toString() is executed. This means that the actual functionality should not change as long as you only use the public APIs. Feel free to test the new pre-release version, you can install it with npm i ical-generator@next. Thank you very much.

[NachoVazquez]

Awesomeness!! Thanks for the quick release!

I'll give it a try later today!

[NachoVazquez]

Hey, Sorry for not getting back to you; I got sick and haven't been able to try the fix.

[sebbo2002]

No problem. We’ll write here again when you’re healthy again. Get well soon.

[nilsreichardt]

Thanks for the fix! I ran into the same problem and with the latest pre-release, it's working :)

[sebbo2002]

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release
• v7.0.0

Your semantic-release bot 📦🚀

[nacho-vazquez]

Sorry for not getting back here before @sebbo2002. Thanks so much for the fix!