Add BirthdayCalendarGenerator #239

DominikTo · 2015-07-07T14:11:37Z

No description provided.

Hywan · 2015-07-07T14:12:59Z

tests/VObject/VCardBirthdayTest.php

+BEGIN:VCARD
+VERSION:3.0
+N:Gump;Forrest;;Mr.
+FN:Forrest Gump


Hywan · 2015-07-07T14:13:33Z

👍 for me. Good job!

DominikTo · 2015-07-07T14:29:32Z

I wonder:

Should $format support using first name and last name separately (e.g. if a SUMMARY like John's Birthday -instead of John Doe's Birthday- is desired)?
Should this method support falling back to N?
Should this method maybe return null for vCards that have the proprietary X-ABShowAs:COMPANY set?
Should we optionally generate multiple recurring instances, to support e.g. John Doe's 30. Birthday in the SUMMARY?

evert · 2015-07-07T15:18:57Z

The only concern I have, is that I feel it might be better for this functionality to live outside of the vcard component. It's nice to keep the actual method that's in there, but perhaps there should be a separate BirthdayCalendarGenerator class.

Two reasons:

The functionality is very similar to FreebusyGenerator. Ideally they also have very similar API's
I imagine people will want to create birthdays calendars for multiple vcards at the same time. This would only allow creating 1 birthday VCALENDAR, so this would force people to run this multiple times and then merge afterwards. If there's an object similar to FreebusyGenerator, you could feed it multiple vcards.

evert · 2015-07-07T15:20:21Z

tests/VObject/VCardBirthdayTest.php

+VERSION:3.0
+N:Gump;Forrest;;Mr.
+FN:Forrest Gump
+BDAY:19850407


Hey that's my birthday! Are you calling me forest gump? :P

Regardless... you need to add a test for BDAY values without a year and without a month.

Haha, no, I just added an easter egg and with your name it would have been too obvious. ;-)

evert · 2015-07-07T15:20:46Z

It would also be real nice if you support ANNIVERSARY =)

DominikTo · 2015-07-07T15:24:29Z

Good idea with the BirthdayCalendarGenerator, now I also remember that we talked about this. =)

Hywan · 2015-07-07T16:54:10Z

BirthdayCalendarGenerator is better.

DominikTo · 2015-07-08T12:38:51Z

lib/BirthdayCalendarGenerator.php

+     *
+     * @var int
+     */
+    protected $defaultYear = 1900;


Maybe, once #242 is merged, the year defined in $minDate in the new Settings class should be used instead.

I don't know if I came up with 1900 here... but I would actually suggest putting this a lot further in the future. Putting it on 1900 means that usually 115 recurrence instances will be created to calculate birthdays, and this will not be very efficient. I'd set it to 2010 I think.

DominikTo · 2015-07-20T07:31:43Z

Ready for review.

evert · 2015-07-20T16:16:42Z

All events need to have VALUE=DATE.
Timezone is not relevant for those, so you can strip all timezone-related stuff.

evert · 2015-07-20T16:17:09Z

lib/BirthdayCalendarGenerator.php

+     *
+     * @var int
+     */
+    protected $defaultYear = 2000;


Should probably be a constant.

DominikTo · 2015-07-20T18:56:30Z

Thanks!

All events now have VALUE=DATE.
Timezone related stuff is stripped.
$defaultYear is now a DEFAULT_YEAR constant.
Added tests for previously untested stuff.

DominikTo · 2015-07-20T18:57:21Z

Looks like this can't get merged automatically though. Should I merge master into this branch or rebase on master?

evert · 2015-07-20T19:32:08Z

Merge master into this branch =)

DominikTo · 2015-07-20T19:35:02Z

Done.

# Conflicts: # CHANGELOG.md

Add BirthdayCalendarGenerator

evert · 2015-07-20T20:25:18Z

Awesome!

Hywan · 2015-07-21T05:56:14Z

Awesome \o/.

Hywan · 2015-07-21T05:59:02Z

CHANGELOG.md

@@ -37,8 +37,12 @@ ChangeLog
 * #197: The `$children` property on components has been changed from `public`
  to `protected`. Use the `children()` method instead to get a flat list of
  objects.
+<<<<<<< HEAD


Haaa damned, it is not supposed to be here 😉.

Addressed in fruux@87c6a50. Good.

Hywan reviewed Jul 7, 2015
View reviewed changes

tests/VObject/VCardBirthdayTest.php

BEGIN:VCARD

VERSION:3.0

N:Gump;Forrest;;Mr.

FN:Forrest Gump

Copy link

Member

Hywan Jul 7, 2015

Choose a reason for hiding this comment

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

😋

DominikTo force-pushed the birthday-event branch from cf229cb to f83ba4f Compare July 7, 2015 14:41

evert reviewed Jul 7, 2015
View reviewed changes

Hywan added the enhancement label Jul 7, 2015

DominikTo reviewed Jul 8, 2015
View reviewed changes

DominikTo added 6 commits July 9, 2015 00:04

Added getBirthdayEvent method for VCards

8619f79

Added BirthdayCalendarGenerator

caadb36

Rely on converter instead of duplicating X-APPLE-OMIT-YEAR handling

66c2468

Properly using params

d67f9ab

Fixed docblock

b2a44c6

Support for formatting

b05b356

DominikTo force-pushed the birthday-event branch from 5d1930f to b05b356 Compare July 8, 2015 22:06

cs fixes

130f362

DominikTo changed the title ~~Added getBirthdayEvent method for VCards~~ Add BirthdayCalendarGenerator Jul 9, 2015

DominikTo added the in-progress label Jul 9, 2015

DominikTo removed the in-progress label Jul 20, 2015

Adding a property for dates with unknown year

cc13ef0

DominikTo assigned evert Jul 20, 2015

evert reviewed Jul 20, 2015
View reviewed changes

lib/BirthdayCalendarGenerator.php

*

* @var int

*/

protected $defaultYear = 2000;

Copy link

Member

evert Jul 20, 2015

Choose a reason for hiding this comment

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

Should probably be a constant.

DominikTo added 3 commits July 20, 2015 20:45

Using VALUE=DATE

0253228

Timezone is now no longer needed

b9c364f

Added DEFAULT_YEAR constant

a5bf96f

DominikTo force-pushed the birthday-event branch from c89c3a7 to 2a62379 Compare July 20, 2015 18:52

Added a few more tests

2a62379

Merge branch 'master' into birthday-event

c9129dc

# Conflicts: # CHANGELOG.md

DominikTo added the version-4.x label Jul 20, 2015

evert added a commit that referenced this pull request Jul 20, 2015

Merge pull request #239 from fruux/birthday-event

0d1e240

Add BirthdayCalendarGenerator

evert merged commit 0d1e240 into master Jul 20, 2015

Hywan reviewed Jul 21, 2015
View reviewed changes

phsc84 mentioned this pull request Nov 24, 2015

Contact birthdays owncloud/contacts#5

Closed

evert deleted the birthday-event branch January 11, 2016 06:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BirthdayCalendarGenerator #239

Add BirthdayCalendarGenerator #239

DominikTo commented Jul 7, 2015

Hywan Jul 7, 2015

Hywan commented Jul 7, 2015

DominikTo commented Jul 7, 2015

evert commented Jul 7, 2015

evert Jul 7, 2015

DominikTo Jul 7, 2015

evert commented Jul 7, 2015

DominikTo commented Jul 7, 2015

Hywan commented Jul 7, 2015

DominikTo Jul 8, 2015

evert Jul 8, 2015

DominikTo commented Jul 20, 2015

evert commented Jul 20, 2015

evert Jul 20, 2015

DominikTo commented Jul 20, 2015

DominikTo commented Jul 20, 2015

evert commented Jul 20, 2015

DominikTo commented Jul 20, 2015

evert commented Jul 20, 2015

Hywan commented Jul 21, 2015

Hywan Jul 21, 2015

Hywan Jul 21, 2015

Add BirthdayCalendarGenerator #239

Add BirthdayCalendarGenerator #239

Conversation

DominikTo commented Jul 7, 2015

Choose a reason for hiding this comment

Hywan commented Jul 7, 2015

DominikTo commented Jul 7, 2015

evert commented Jul 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evert commented Jul 7, 2015

DominikTo commented Jul 7, 2015

Hywan commented Jul 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DominikTo commented Jul 20, 2015

evert commented Jul 20, 2015

Choose a reason for hiding this comment

DominikTo commented Jul 20, 2015

DominikTo commented Jul 20, 2015

evert commented Jul 20, 2015

DominikTo commented Jul 20, 2015

evert commented Jul 20, 2015

Hywan commented Jul 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment