Issues/50 from ical, cleaned up

[wvengen]

Cleanup of #226.

Squashed all commits into one, removed non-ical changes, unbroke tests, and committed. Sorry to have lost all the contributors, but it was too unwieldy to rebase. Credits to @skyporter @jgauby @btucker @spra85.

I hope this helps in getting iCal parsing finally merged.

[spra85]

👍 Looks great @wvengen . Thanks for taking the time to rework the PR.

[btucker]

👍 Thanks @wvengen!

[avit]

Thanks so much for doing this, it's really helpful. I'll review and get this merged hopefully today.

[wvengen]

Great! If there's anything I can do still, let me know.

[nielsgl]

Anything that I can help out with to get this functionality merged into the master?

[Ventajou]

Hi, I'm curious about when this will be merged as well!

[Bezalja]

Yes, I'm also waiting for this ;)

[willemave]

Is this almost ready to merge?

[fbonetti]

Any word on this?

[wvengen]

I found it ready at submission, and considering the positive comments, I assume it has passed some measure code review. I'd say: ready to be merged.

[Thann]

ActiveSupport::DateTime.to_time returns a DateTime unless the offset == 0 (UTC)
Instead, saying DateTime.parse(value).utc.to_time (".utc" has moved) will convert to UTC then successfully convert to Time.

[wvengen]

Thanks for your feedback! I'm trying to see where it happens, but all I can find is that to_time returns a DateTime; ActiveSupport doesn't seem to change that method, right? Also:

$ rails c
> dt=DateTime.parse('2015-10-10T00:00:00+00:00')
Sat, 10 Oct 2015 00:00:00 +0000
> dt.to_time.utc
2015-10-10 00:00:00 UTC
> dt.utc.to_time
2015-10-10 02:00:00 +0200
> dt=DateTime.parse('2015-10-10T00:00:00+02:00')
Sat, 10 Oct 2015 00:00:00 +0200
> dt.to_time.utc
2015-10-09 22:00:00 UTC
> dt.utc.to_time
2015-10-10 00:00:00 +0200

So if we want utc, to_time.utc is right. I'd need to look at what happens here and what the ical standard expects, but I think to_time.utc is right. What do you think?

Thanks for taking a look at this.

[Thann]

Hey! I should preface this by saying I'm just a third party whose interested in this project, and I'm not entirely knowledgeable of everything.
Anyway, Here's the behaviour I have observed:

[1] pry(main)> DateTime.now.to_s
=> "2014-11-19T10:03:57-08:00"
[2] pry(main)> DateTime.now.class
=> DateTime
[3] pry(main)> DateTime.now.to_time.class
=> DateTime
[4] pry(main)> DateTime.now.utc.to_time.class
=> Time
[5] pry(main)> show-method DateTime.now.to_time

From: /home/thann/.rvm/gems/ruby-2.1.2/gems/activesupport-3.2.19/lib/active_support/core_ext/date_time/conversions.rb @ line 68:
Owner: DateTime
Visibility: public
Number of lines: 3

def to_time
  self.offset == 0 ? ::Time.utc_time(year, month, day, hour, min, sec, sec_fraction * (RUBY_VERSION < '1.9' ? 86400000000 : 1000000)) : self
end

Presumably different versions of ActiveSupport behave differently, so IDK if everyone will have this issue, I just noticed that IceCube throws "DateTime support is deprecated" messages whenever I call the from_ical function because of this.

[wvengen]

Ah thanks. I'm using Rails 4.1.6, which behaves differently:

[1] pry(main)> DateTime.now
=> Wed, 19 Nov 2014 22:05:32 +0100
[2] pry(main)> DateTime.now.class
=> DateTime
[3] pry(main)> DateTime.now.to_time.class
=> Time
[4] pry(main)> DateTime.now.utc.to_time.class
=> Time
[5] pry(main)> show-method DateTime.now.to_time
Error: Cannot locate this method: to_time.

I'm a bit at loss what to do now. It would be nice to know if the code functions incorrectly because of this - in that case I'd like to add a rails version conditional. If it's just a warning, I'd be inclined to leave it as it is.

And, thanks for your input - I'm not knowledgeable of everything, but perhaps together we cover the base :)

[Thann]

This didn't seem obvious to me before, but I'm pretty sure a simple Time.parse(value).utc will get the job done.

[wvengen]

Good idea. I can't find it in Ruby doc nor Rails, but apidock shows it being present in 1.9 as well. But they're there, and with this change the ical specs still work for both Ruby 1.9.3 and 2.1.3 as well as ActiveSupport 4.1.8, 4.0.12, 3.0.20 and 3.1.12.

[Thann]

Awesome! It's unfortunate the documentation is so spotty.
Thanks dude =)

[arangamani]

@avit @seejohnrun Any word on when this will get merged?

[nielsgl]

I'm using @wvengen 's fork for now and it works great!

—
Sent from Mailbox

On Wed, Dec 10, 2014 at 3:37 PM, Kannan Manickam [email protected]
wrote:

@avit @seejohnrun Any word on when this will get merged?

Reply to this email directly or view it on GitHub:
#258 (comment)

[bconway]

Looking for this as well in order to persist schedules outside of a Ruby context. Thanks.

[airblade]

Just wondering whether there's an ETA on merging this (or something similar)?

[bconway]

Would it be possible to support RDATE as a SingleOccurrenceRule? I just realized this patchset does not, which is not ideal.

[jboler]

We're using @wvengen 's fork in production and it works great.

[mgold]

I also would find this quite useful.

[jbat]

+1

[jbat]

User @wvengen fork in production now as well

[mib32]

💋 💋 💋
that's terrifically easied my job

[wvengen]

Thank you!