Page MenuHomePhabricator

DRAFT, recurring events need optional end dates
ClosedPublic

Authored by lpriestley on May 31 2015, 11:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 17, 12:42 PM
Unknown Object (File)
Fri, Mar 15, 7:02 AM
Unknown Object (File)
Sun, Mar 10, 9:31 PM
Unknown Object (File)
Feb 15 2024, 9:06 PM
Unknown Object (File)
Feb 10 2024, 9:55 AM
Unknown Object (File)
Feb 9 2024, 5:22 AM
Unknown Object (File)
Feb 3 2024, 1:19 PM
Unknown Object (File)
Jan 30 2024, 8:00 PM
Subscribers

Details

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

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

Repository
rP Phabricator
Branch
calendarrecurrenceenddate
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6425
Build 6447: [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.

src/applications/calendar/query/PhabricatorCalendarEventQuery.php
163

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.

src/applications/calendar/query/PhabricatorCalendarEventQuery.php
163

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

src/applications/calendar/controller/PhabricatorCalendarEventEditController.php
191–193

debugging code

src/applications/calendar/query/PhabricatorCalendarEventQuery.php
153–154

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

158

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.
src/applications/calendar/controller/PhabricatorCalendarEventEditController.php
144

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.