Page MenuHomePhabricator

Canceling calendar events should deactivate the event
ClosedPublic

Authored by lpriestley on Apr 29 2015, 3:39 AM.
Tags
None
Referenced Files
F14023260: D12604.diff
Thu, Nov 7, 12:16 AM
F14001572: D12604.diff
Fri, Oct 25, 9:41 AM
F13989176: D12604.id30267.diff
Mon, Oct 21, 6:37 PM
F13973873: D12604.id30275.diff
Fri, Oct 18, 3:06 AM
F13971628: D12604.id30275.diff
Thu, Oct 17, 2:26 PM
Unknown Object (File)
Oct 3 2024, 6:12 AM
Unknown Object (File)
Oct 2 2024, 12:45 AM
Unknown Object (File)
Oct 1 2024, 9:05 PM
Subscribers

Details

Summary

Closes T7943, Canceling calendar event should deactivate the event instead of destroying data.

Test Plan

Create an event, cancel it, see changed status icon, query for active events, event should not appear, query for deactivated events, event should appear in results.

Diff Detail

Repository
rP Phabricator
Branch
calendareventscancel
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 5588
Build 5607: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

lpriestley retitled this revision from to Canceling calendar events should deactivate the event.
lpriestley updated this object.
lpriestley edited the test plan for this revision. (Show Details)
lpriestley added a reviewer: epriestley.
epriestley edited edge metadata.

One inline about STRICT_ALL_TABLES that we should double check in person.

src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php
45–46

You should also setContinueOnMissingFields(true) here.

If we eventually add support for custom fields to events and an administrator adds a "required" custom field, adding this call will let the Editor know that it's still OK to cancel an event even if it doesn't have "Official Authorization Code (MANDATORY!!)" or whatever.

(When creating or editing an event, the call is omitted because we do want the user to have to provide all required fields.)

71–72

I think these are unused in this controller.

src/applications/calendar/editor/PhabricatorCalendarEventEditor.php
94

This probably needs an (int):

$object->setIsCancelled((int)$xaction->getNewValue());

We should configure STRICT_ALL_TABLES on your database so MySQL starts complaining about this. I can walk you through that.

src/applications/calendar/storage/PhabricatorCalendarEvent.php
260–266

You can omit this block, the DestructionEngine will take care of it for you as long as you implement PhabricatorApplicationTransactionInterface (which you already implemented).

Remainder of the method is good.

This revision now requires changes to proceed.Apr 29 2015, 1:00 PM
lpriestley edited edge metadata.
lpriestley marked 4 inline comments as done.

Addressing code review. Cleanup and int casting.

epriestley edited edge metadata.
This revision is now accepted and ready to land.Apr 29 2015, 3:38 PM
This revision was automatically updated to reflect the committed changes.