rossta / montrose

Recurring events library for Ruby. Enumerable recurrence objects and convenient chainable interface.
https://rossta.net/montrose/
MIT License
843 stars 53 forks source link

Impossible to save and non ending Recurrence with ActiveRecord #113

Closed mmagn closed 5 years ago

mmagn commented 5 years ago

Hi, first of all thank you for your work on Montrose, your gem is really helpful and super elegant to use.

I am using the 0.9.0 version of the gem and I have an issue while trying to persist a Recurrence attribute of a model in database.

The problem occurs for a infinite Recurrence.

with this model defined :

class RecurringEvent < ApplicationRecord
  serialize :recurrence, Montrose::Recurrence

end

this code produces an infinite loop when the .save method is called :

event = RecurringEvent.new
event.recurrence = Montrose.weekly
event.save # infinite loop

I have investigated a little and I found the problem is coming from ActiveSupport which is calling .as_json while trying to jsonify the Recurrence attribute in order to store it (https://github.com/rails/rails/blob/v5.2.3/activesupport/lib/active_support/json/encoding.rb#L96)

We can isolate the problem by calling :

Montrose.weekly.as_json # infinite loop

While :

Montrose.weekly.to_json # returns "{\"every\":\"week\"}"

If we specify an end to the recurrence, there is no longer an infinite loop :

Montrose.weekly.until(2.weeks.since).as_json # returns ["2019-07-16T11:43:24.000+02:00", "2019-07-23T11:43:24.000+02:00", "2019-07-30T11:43:24.000+02:00"]

While :

Montrose.weekly.until(2.weeks.since).to_json # returns "{\"every\":\"week\",\"until\":\"2019-07-30 11:44:42 +0200\"}"

I'm not sure but I think the expected behaviour by ActiveSupport should be :

Montrose.weekly.until(2.weeks.since).as_json # returning {"every"=>"week", "until"=>"2019-07-30T12:02:41.000+02:00"}

I achieved this by defining as_json method for Recurrence class :

def as_json
  to_hash.as_json
end

After this change the following code is working perfectly :

event = RecurringEvent.new
event.recurrence = Montrose.weekly
event.save

What do you think about this?

Thanks for your help

rossta commented 5 years ago

Thanks for the detailed report. Yes, I agree it would be great to define Recurrence#as_json. Would you like to put together a PR?

mmagn commented 5 years ago

Yes I can prepare something tomorrow or the day after.