Page MenuHomePhabricator

DRAFT, recurring events need optional end dates

Authored by lpriestley on May 31 2015, 11:02 PM.
Attached Files
F10756663: D13088.id31647.diff
Wed, May 25, 6:19 AM
F10756662: D13088.id31646.diff
Wed, May 25, 6:19 AM
F10756661: D13088.id31598.diff
Wed, May 25, 6:19 AM
F10756660: D13088.diff
Wed, May 25, 6:19 AM
F10753364: D13088.id31645.diff
Tue, May 24, 12:23 PM
Unknown Object (File)
Apr 13 2017, 9:22 AM
Unknown Object (File)
Nov 18 2016, 2:18 PM
Unknown Object (File)
Nov 8 2016, 10:48 PM


Group Reviewers
Blessed Reviewers
Maniphest Tasks
T8357: Add recurrence end date
Restricted Diffusion Commit
rP95551a1a5a6e: DRAFT, recurring events need optional end dates

Ref T8357, DRAFT, recurring events need optional end dates

Test Plan

Edit recurring event, set end date, save, recurring ghosts should not generate after end date

Diff Detail

rP Phabricator
Lint Passed
Tests Passed
Build Status
Buildable 6405
Build 6427: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

lpriestley retitled this revision from to DRAFT, recurring events need optional end dates.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
lpriestley edited edge metadata.

brokenz update

Fixing editor. Seems to work, although not respecting recurrence end date yet.

epriestley edited edge metadata.
This revision is now accepted and ready to land.Jun 1 2015, 1:46 PM

(Not sure if you just wanted to kick this in as-is or wait to actually respect the setting, but this looks fine so far.)

Oh thanks. I will add to this a bit more before it's ready, but thanks for looking at it :3

lpriestley edited edge metadata.

Actually respect recurrence end date

One possible issue inline, the rest looks good to me.

163 ↗(On Diff #31620)

I think this change isn't correct:

  • Create a daily event a year ago.
  • View this month.

I'd expect $sequence_start to be about 365, and $instance_count to be about 30. So this will evaluate to:

for ($index = 365; $index < 30; $index++) {

...and not enter the loop body, because $index < 30 is never true.

Create a daily event a year ago.

Er, create a "daily" recurring event with the first instance starting a year ago.

163 ↗(On Diff #31620)

Just tested, and this actually works as expected, but it surfaced a small bug - recurrence end date always gets set. Will fix.

Disabled recurrence end date should not save


debugging code

153 ↗(On Diff #31642)

It looks like $instance_count is the index number of the last index here

158 ↗(On Diff #31642)

It looks like $instance_count is a count here. I think one of these is wrong? As written, probably this one should be $sequence_start + $this->getRawResultLimit() and the variable should be $sequence_end?

lpriestley marked 3 inline comments as done.

Correct edit options for recurring events v ghost events

lpriestley edited edge metadata.
epriestley edited edge metadata.
epriestley added inline comments.

Seems slightly weird that we clone $start_value here and $end_value above?

This revision is now accepted and ready to land.Jun 2 2015, 1:50 AM
lpriestley edited edge metadata.

Using end_value as default recurrence end date instead of start value

This revision was automatically updated to reflect the committed changes.