1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-27 17:22:42 +01:00
Commit graph

403 commits

Author SHA1 Message Date
epriestley
363084d4fa Fix an issue where setting a recurrence end date on a Calendar event without one could fatal
Summary: Ref T11816. The underlying format of recurrence end dates swapped around a bit and we now try to compare `null` to a valid date if you're setting it for the first time.

Test Plan:
  - On a new event, set a recurrence end date.
  - Then, removed a recurrence end date.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D17150
2017-01-06 16:36:09 -08:00
epriestley
7c37377e0d Set the viewer timezone properly on Calendar event RecurrenceSet objects
Summary: Ref T11801. In some cases, this could lead to us failing to generate the first recurrence in a series.

Test Plan: Imported `weekly.ics` (from D16974) and saw an event correctly occur on Aug 18, with my local timezone set to "America/Los_Angeles".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11801

Differential Revision: https://secure.phabricator.com/D16975
2016-12-02 07:29:48 -08:00
epriestley
5cc4f6407c Make the modal choice between "Edit This Event" and "Edit Future Events" more clear
Summary:
Fixes T11909. Ref T11816. Instead of offering a dropdown with choices between "Edit/Cancel/Reinstate This Event" and "Edit/Cancel/Reinstate Future Events", make the choice more explicit.

This dialog ends up pretty wordy but this edit is rare, so I think that's alright.

Test Plan: {F2046863}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816, T11909

Differential Revision: https://secure.phabricator.com/D16956
2016-11-28 10:47:58 -08:00
epriestley
22a566f732 Ignore Calendar date edits which just change the internal date timezone without rescheduling it
Summary:
Ref T11816. Currently, if someone in California creates an event and then someone in New York edits it, we generate a no-op "<user> changed the start time from 3PM to 3PM." transaction.

This is because the internal timezone of the event is changing, but the actual absolute time is not.

Instead, when an edit wouldn't reschedule an event and would only change the internal timezone, ignore the edit.

Test Plan:
  - Edited non-all-day events in PST / EST with out making changes (ignored).
  - Edited non-all-day events in PST / EST with changes (changes worked).
  - Performed the same edits with all-day events, which also were ignored and worked, respectively.
  - Pulled events in and out of all-day mode in different timezones, behavior seemeed reasonable.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16955
2016-11-28 10:33:59 -08:00
epriestley
c5162074a1 Fix an issue where internal Calendar DateTimes would not be correctly set to all-day
Summary:
Ref T11816. I don't really know what happened here, maybe I rewrote and broke this at the last second?

In most cases, we directly respect the `isAllDay` flag on the event, so the internal date state doesn't matter too much.

However, in the case of mail notifications, the raw internal state is relevant. This should fix mail notifications for all-day events.

(I might still turn them off since I'm not sure they're too useful, but it's good to have them working.)

Test Plan:
  - Created a new all-day event, verified database values wrote correctly.
  - Ran `bin/calendar notify --trace`, verified it picked up an all-day event tomorrow with a large enough `--minutes` value.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16954
2016-11-28 08:54:28 -08:00
epriestley
39509648f2 In Calendar mobile month view, color circles green for attending events too
Summary: Ref T11816. We currently color circles green if you're invited, but should color them green if you're attending, too.

Test Plan: Viewed calendar mobile month view, saw attending events in green.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16899
2016-11-18 11:56:40 -08:00
epriestley
132b0803cb Fix a couple of calendar export daterange issues
Summary:
Ref T11816. In some cases, Calendar would only export a subset of events because the "export" flag was ignored or the "display" parameter applied an improper date range to the query.

  - Make sure the `export` flag gets processed, even though it isn't a "real" field on the search engine.
  - Clear the "display" parameter to avoid date range windowing coming from the day/month logic.

Test Plan: Exported a "display=month" view, verified future events came with it.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16898
2016-11-18 10:07:35 -08:00
epriestley
f33b5c30eb Use the same date rendering display logic for both tooltips and subheaders
Summary: Ref T11816. This could be a little cleaner, but we currently have two copies of the logic. Get them using the same code. Once that's actually working I can go make the code a little prettier.

Test Plan: Viewed Calendar month view tooltips, saw the same values as subheaders.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16880
2016-11-16 08:37:25 -08:00
epriestley
91ee6b78df Make some confusing/weird Calendar fields not configurable on custom EditEngine forms
Summary:
Ref T11816.

  - Host gets weird behavior around defaulting to the viewer.
  - Invitees get weird behavior around defaulting to the viewer.
  - "All Day" is just sort of weird since start / end date aren't customizable.
  - Recurring/Frequency are weird here and don't make much sense.

I can't immediately come up with reasons that any of these are particularly useful/valuable to default. More of them can be made editable after T10222 gets sorted out.

Test Plan: Edited edit engine custom forms for Calendar events, saw a more sensible list of customizable fields (e.g., policy stuff).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16878
2016-11-16 07:49:00 -08:00
epriestley
6e5565b3ff Clean up a little more Calendar display logic
Summary: Uh, non-all-day-events said 1:30 - 2:29 PM, which is real silly.

Test Plan: Looked at a non-all-day-event.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16874
2016-11-15 14:42:35 -08:00
epriestley
c7f2e4a924 Document calendar summary icons
Summary:
Fixes T11809. Ref

  - Explicitly document the summary icon hints -- I don't think these are too hard to figure out (and maybe this stuff should just go in the tooltips) but we can start here.
  - Use color + shape to distinguish between "cancelled" and "declined", not just color (for users with vision accessibility issues).
  - Translate a "minute(s)" string into sensible English.
  - Use RSVP status on the month view green circle thing.

Test Plan:
  - Read docs.
  - Looked at month view.
  - Read reminder mail.
  - Viewed month view mobile view.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16872
2016-11-15 13:44:20 -08:00
epriestley
e713cc08e6 Make Calendar query for indirect invites/RSVPs by default, like Differential
Summary:
Ref T11816. Since the dashboard got updated, Differential now interprets "Responsible Users: epriestley" to mean "epriestley, or any project or package epriestley is part of". You can query for just "epriestley" with "exact(epriestley)".

Give Calendar invites the same behavior: "epriestley" means "any event epriestley is invited to, or a project they are a member of is invited to". Individual invites can be queried with "exact(epriestley)".

This is a little bit copy-pastey but I want to wait for a third use case to clean it up since I think I'm going to have to do a bunch of generalization around "how does an individual PHID get turned into a bunch of PHIDs".

Test Plan: Queried for "Invited: dog", "invited: viewer", "invited; exact(dog)", etc.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16870
2016-11-15 12:47:01 -08:00
epriestley
edba4bb8d4 Improve Calendar event behavior for group invites
Summary:
Ref T11816. Projects can be invited to an event, but the UI is currently fairly agnostic about them.

Instead, introduce the idea of "RSVPs", which are basically invites for you as an individual or for any group you're a part of. When we go to check if you're invited, we check for you individually first, then check for any groups you belong to if you haven't already accepted/declined.

On the calendar detail page:

  - Show the quick "Join" / "Decline" buttons if any project you're a member of is invited.
  - If you're invited, highlight any projects which you're a member of to make that more clear.

On other calendar views:

  - If you're invited as part of a project, show the "multiple users" icon.
  - If it's just you, continue showing the "add one user" icon.

Test Plan: Viewed month view, day view, detail view. Invited groups and individuals. Invited "Dog Project", accepted invite as user "Dog".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16868
2016-11-15 11:16:55 -08:00
epriestley
6464934cd6 Fix a bug on Calendar event day views for all day events in certain timezones
Summary:
Ref T11816. This logic was correct, we just did all the work and then mostly threw away the results. This worked correctly anyway in some timezones.

Instead, actually use `$min_date` and `$max_date`.

Test Plan: In "America/Toronto" with server in a more-western timezone, viewed a "Nov 11 - Nov 12" all-day event, saw those dates.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16867
2016-11-15 11:10:20 -08:00
epriestley
c9e140e283 Restore green color on mobile calendar month view when a day contains events you are invited to
Summary: Ref T11816. This got dropped somewhere along the way, so the mobile month view no longer showed a green-colored hint if a day has events you're invited to.

Test Plan: Viewed Calendar month view on mobile, saw green circles for days with invited events.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16852
2016-11-13 12:41:55 -08:00
epriestley
706c21375e Remove empty implementations of describeAutomaticCapabilities()
Summary:
This has been replaced by `PolicyCodex` after D16830. Also:

  - Rebuild Celerity map to fix grumpy unit test.
  - Fix one issue on the policy exception workflow to accommodate the new code.

Test Plan:
  - `arc unit --everything`
  - Viewed policy explanations.
  - Viewed policy errors.

Reviewers: chad

Reviewed By: chad

Subscribers: hach-que, PHID-OPKG-gm6ozazyms6q6i22gyam

Differential Revision: https://secure.phabricator.com/D16831
2016-11-09 15:24:22 -08:00
epriestley
4811e6e7c1 Require several advanced postgraduate degrees to understand object policies
Summary:
Fixes T11836. See some prior discussion in T8376#120613.

The policy hint in headers in the UI is not exhaustive, and can not reasonably be exhaustive. For example, on a revision, it may say "All Users", but really mean "All users who can see the space this object is in and the repository it belongs to, plus the revision author and reviewers".

These rules are explained if you click (and, often, in the documentation), but "All Users" is still at least somewhat misleading.

I don't think there's any perfect solution here that balances the needs of both new and experienced users perfectly, but this change tries to do a bit better about avoiding cases where we say something very open (like "All Users") when the real policy is not very open.

Specifically, I've made these changes to the header:

  - Spaces are now listed in the tag, so it will say `(S3 > All Users)` instead of `(All Users)`. They're already listed in the header, this just makes it more explicit that Spaces are a policy container and part of the view policy.
  - Extended policy objects are now listed in the tag, so it will say `(S3 > rARC > All Users)` for a revision in the Arcanist repository which is also in Space 3.
  - Objects can now provide a "Policy Codex", which is an object that represents a rulebook of more sophisticated policy descriptions. This codex can replace the tag with something else.
    - Imported calendar events now say "Uses Import Policy" instead of, e.g., "All Users".

I've made these changes to the policy dialog:

  - Split it into more visually separate sections.
  - Added an explicit section for extended policies ("You must also have access to these other objects: ...").
  - Broken the object policy rules into a "Special Rules" section (for rules like "you can only see a revision if you can see the repository it is part of") and an "Object Policy" section (for the actual object policy).
  - Tried to make it a little more readable?
  - The new policy dialogs are great to curl up with in front of a fire with a nice cup of cocoa.

I've made these changes to infrastructure:

  - Implementing `PhabricatorPolicyInterface` no longer requires you to implement `describeAutomaticCapability()`.
  - Instead, implement `PhabricatorPolicyCodexInterface` and return a `PhabricatorPolicyCodex` object.
  - This "codex" is a policy rulebook which can set all the policy icons, labels, colors, rules, etc., to properly explain complex policies.
  - Broadly, the old method was usually either not useful (most objects have no special rules) or not powerful enough (objects with special rules often need to do more in order to explain them).

Test Plan:
{F1912860}

{F1912861}

{F1912862}

{F1912863}

Reviewers: chad

Reviewed By: chad

Subscribers: avivey

Maniphest Tasks: T11836

Differential Revision: https://secure.phabricator.com/D16830
2016-11-09 15:05:38 -08:00
epriestley
6a7dde03cc On @username mentions in remarkup, show the "busy" dot color
Summary:
Ref T11809. I missed this when adding a "Busy" status.

Also the other dot is orange? Just make them all orange for consistency.

Test Plan: Viewed `@username` of busy users (orange), away users (red).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16819
2016-11-07 14:57:32 -08:00
epriestley
729492a8ff Allow transactions to specialize their mail headers for diff sections
Summary: Ref T7643. When we send mail about a change to a package description, allow it to say "CHANGES TO PACKAGE DESCRIPTION" instead of "EDIT DETAILS". Smooth!

Test Plan: {F1909417}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7643

Differential Revision: https://secure.phabricator.com/D16818
2016-11-07 12:16:39 -08:00
epriestley
87c4efdb63 Probably fix some display issues with all-day events?
Summary:
Ref T11801. These are pretty fiddly because users expect to see the end time for timed events ("10 AM - 11 AM" is ONE hour long) but not for all-day events ("Nov 2 - Nov 3" is TWO days long!)

We also want to store the thing the user actually entered so we don't lose data if they un-all-day the event later.

This may take a little more fiddling since it feels a little shaky, but I couldn't break this version immediately.

Test Plan: Imported a French holiday, got proper display in the UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11801

Differential Revision: https://secure.phabricator.com/D16815
2016-11-07 10:55:30 -08:00
epriestley
7ddd570fa5 Provide a standalone bin/calendar reload ... workflow for testing/debugging
Summary:
Ref T11801. This makes testing/debugging a little easier.

Also fix some inconsistencies with `importAuthorPHID` handling -- it should be the import's author PHID in all cases, so we update imported events properly.

Test Plan: Imported a French holiday with `bin/calendar reload ...`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11801

Differential Revision: https://secure.phabricator.com/D16814
2016-11-07 10:55:18 -08:00
epriestley
17bd483207 Queue large ICS files for background import
Summary: Ref T11801. When a file is larger than 512KB, queue it for background import instead of trying to do it in the foreground, sinc we risk hitting `max_execution_time`.

Test Plan: {F1906943}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11801

Differential Revision: https://secure.phabricator.com/D16805
2016-11-06 07:46:21 -08:00
epriestley
e1566bef63 Fix a Calendar import issue where we looked up attendees by object instead of name
Summary:
Ref T11801. This issue led to the stack trace in T11801#199042.

It wasn't obvious that this was wrong because the recover-on-duplicate-key code made it work correctly.

Test Plan: Imported an event with external attendees with no warnings in the log.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11801

Differential Revision: https://secure.phabricator.com/D16804
2016-11-05 11:12:20 -07:00
epriestley
90fb8a1516 Fix an issue with editing application default policies in Calendar
Summary: Ref T11816. We're running this code on empty events which haven't been initialized and don't have a source attached -- just use a more explanatory check which doesn't need anything attached.

Test Plan: Edited default Calendar policies.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16803
2016-11-04 16:55:56 -07:00
epriestley
e337029769 Allow users to mark themselves as "Available", "Busy" or "Away" while attending an event
Summary:
Ref T11816.

  - Now that we can do something meaningful with them, bring back the yellow dots for "busy".
  - Default to "busy" when attending events (we could make this "busy" for short events and "away" for long events or something).
  - Let users pick how to display their attending status on the event page.
  - Also show which event the user is attending since I had to mess with the cache code anyway. We can get rid of this again if it doesn't feel good.

Test Plan:
{F1904179}

{F1904180}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16802
2016-11-04 16:55:44 -07:00
epriestley
ac8b156e4b Raise ICS warnings in Phabricator on ICS import
Summary: Ref T11816. Depends on D16800. Show warnings generated by ICS import in the UI.

Test Plan: {F1904122}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11816

Differential Revision: https://secure.phabricator.com/D16801
2016-11-04 16:36:55 -07:00
epriestley
e4c6ae5345 Smooth out various transaction/editing behaviors for Calendar
Summary:
Ref T11809.

  - Allow users to remove the "Until" date from recurring events.
  - When removing "Until", show a sensible string ("...set this event to repeat forever.")
  - When users go through the "Make Recurring" workflow, don't require them to explicitly select "Recurring: Recurring" from the dropdown. This intent is clear from clicking "Make Recurring".
  - When editing "All Future Events", don't literally apply date changes to them, since that doesn't make sense. We update the template, then reschedule any events which haven't been edited already. I think this is what users probably mean if they make this edit.
  - When creating an event with a non-default icon, don't show "alice changed the icon from Default to Party.".
  - Hide the "recurring mode" transaction, which had no string ("alice edited this Event.") and was redundant anyway.
  - Also, add a little piece of developer text to make hunting these things down easier.

Test Plan: Edited various events, parents, children, made events recur, set until, unset until, viewed transactions, rescheduled parents, rescheduled children.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16796
2016-11-03 11:03:20 -07:00
epriestley
e9b861ff15 Write a basic Calendar user guide
Summary:
Ref T11809. Roughly documents most of the tricky/unintuitive stuff.

Also fixes a bug with "Make Recurring" with no "Until" date.

Test Plan: Read document.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16792
2016-11-02 14:48:29 -07:00
epriestley
29313372e7 Improve some commenting/editing behaviors for recurring events
Summary:
Ref T11809. Currently, commenting on a recurring event hits the same "one or all?" dialog that other edits do.

For comments and edits submitted via the comment widget, we can safely assume that you mean "just this one", since it doesn't really make sense to try to bulk-edit an event from that UI.

Test Plan: Commented on a recurring event parent and an event in the series.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16795
2016-11-02 14:08:55 -07:00
epriestley
64cf9204c1 In Calendar, only include the event description in the original event mail
Summary: Ref T11809. This makes the mail more consistent with Differential and Maniphest, which only include additional details in the first mail in the thread.

Test Plan:
  - Created an event with a description.
  - First mail included it.
  - Followups did not.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16794
2016-11-02 13:46:05 -07:00
epriestley
c9510cc118 Make it more clear that red dots next to usernames mean Calendar availability
Summary:
Ref T11809. We show a red dot next to a username to indicate that the user is away (on vacation, in a meeting, etc).

It's not very obvious what this means unless you know that's what it is: when you click the username or view a hovercard, there's no visual hint about what the red dot means. It does say "Away", but there is a lot of information and it doesn't visually connect the two.

Connect the two visually by putting a red dot next to the "Away" bit, too.

Test Plan:
Here's my version of it, this feels OK to me but could maybe be more designed:

{F1893916}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16791
2016-11-02 13:45:43 -07:00
epriestley
713f8fb373 Fix a bug which could cause imported events to set themselves as their own parents
Summary: Ref T11808. This variable is wrong, and would sometimes cause events to set themsevles as their own parents. They would then fail to load, and disrupt cursor paging.

Test Plan:
  - Reproduced T11808 locally by reloading test data 2+ times, creating events with themselves as their own parents.
  - Appplied fix.
  - Nuked data, reloaded, no more self-parents.
  - Test datafile: {F1894017}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11808

Differential Revision: https://secure.phabricator.com/D16793
2016-11-02 11:44:09 -07:00
epriestley
3f2f81a1c8 Remove obsolete Calendar event date storage fields
Summary: Ref T11809. These have been replaced with more flexible storage that accommodates a wider range of behaviors, including those in the ICS format and RRULEs.

Test Plan:
  - Ran migration.
  - Viewed, created, edited events.
  - Grepped for all removed names/symbols.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16789
2016-11-02 09:49:08 -07:00
epriestley
6982bded71 Remove ancient "Holiday" storage
Summary:
Ref T11809. This came out of Facebook many years ago for computing the number of business days that revisions had been stale.

We removed the little staleness marker a few months ago and haven't seen complaints about it.

If we did holidays now it would make sense to integrate them more directly with Calendar as real events, but I have no plans to pursue this anytime soon. It's easy enough to add the federal holidays manually (~5 minutes of work per year?) if you want them, and they're commentable/editable and you can add local holidays if you're not in the US.

Test Plan:
  - Ran `bin/storage upgrade -f`.
  - Grepped for `CalendarHoliday`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11809

Differential Revision: https://secure.phabricator.com/D16788
2016-11-01 16:03:44 -07:00
epriestley
191b9398a5 Fix some minor Calendar issues, including a paging issue on imports
Summary:
Fixes T11808. I couldn't reproduce the issue there locally so I'm just cheating a little bit until a better reproduction case shows up.

We don't need to do a full load here anyway, and testing for any row is more efficient.

Test Plan: Poked around imports without issues, but couldn't reproduce this problem locally.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11808

Differential Revision: https://secure.phabricator.com/D16787
2016-11-01 14:40:30 -07:00
epriestley
3e15e0b980 Store more datetime information on Calendar transactions and improve rendering behaviors
Summary:
Fixes T11805. Depends on D16785. This generally tries to smooth out transactions:

  - All-day stuff now says "Nov 3" instead of "Nov 3 12:00:00 AM".
  - Fewer weird bugs / extra transactions.
  - No more silly extra "yeah, you definitely set that event time" transaction on create.

Test Plan: Edited events; changed from all-day to not-all-day and back again, viewed transaction log.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11805

Differential Revision: https://secure.phabricator.com/D16786
2016-11-01 13:36:46 -07:00
epriestley
6b16f930c4 Automatically send (not-so-great) email notifications for upcoming events
Summary: Ref T7931. This is still quite rough, but should technically send vaguely-useful email as part of the standard trigger infrastructure.

Test Plan: Ran `bin/phd start`, created an event shortly, saw reminder email send in `bin/mail list-outbound`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7931

Differential Revision: https://secure.phabricator.com/D16784
2016-11-01 13:24:40 -07:00
epriestley
6e6ae36dcf Add a skeleton for Calendar notifications
Summary:
Ref T7931. I'm going to do this separate from existing infrastructure because:

  - events start at different times for different users;
  - I like the idea of being able to batch stuff (send one email about several upcoming events);
  - triggering on ghost/recurring events is a real complicated mess.

This puts a skeleton in place that finds all the events we need to notify about and writes some silly example bodies to stdout, marking that we notified users so they don't get notified again.

Test Plan:
Ran `bin/calendar notify`, got a "great" notification in the command output.

{F1891625}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7931

Differential Revision: https://secure.phabricator.com/D16783
2016-11-01 10:41:15 -07:00
epriestley
a0ea31f47f When users edit recurring events, prompt to "Edit This Event" or "Edit All Future Events"
Summary:
Fixes T11804. This probably isn't perfect but seems to work fairly reasonably and not be as much of a weird nonsense mess like the old behavior was.

When a user edits a recurring event, we ask them what they're trying to do. Then we more or less do that.

Test Plan:
  - Edited an event in the middle of a series.
  - Edited the first event in a series.
  - Edited "just this" and "all future" events in various places in a series.
  - Edited normal events.
  - Cancelled various events.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11804

Differential Revision: https://secure.phabricator.com/D16782
2016-10-31 16:38:32 -07:00
epriestley
208f8ed526 Support "Edit just this event" on the parent event in a series
Summary:
Ref T11804. This one is messy because we have to fork the //next// event, possibly creating it first.

Then we can edit the parent normally.

Test Plan: Cancelled the first event in a series, only that one cancelled.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11804

Differential Revision: https://secure.phabricator.com/D16781
2016-10-31 15:30:53 -07:00
epriestley
b084efb362 Record a "series parent PHID" on Calendar events that retains relationships after forks
Summary:
When you edit "X and all future events", X becomes the new parent of an event series.

Currently, it loses its relationship to its original parent. Instead, retain that relationship -- it's separate from the normal "parent", but we can use it to make the UI more clear or tweak behaviors later.

This mostly just keeps us from losing/destroying data that we might need/want later.

Test Plan:
  - Ran migrations.
  - Cancelled "X and all future events", saw sensible-appearing beahvior in the database for "seriesParentPHID".

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16780
2016-10-31 15:30:34 -07:00
epriestley
f44a9a4e48 Remove "isCancelledEvent()" wrapper on Calendar Events
Summary: Ref T11804. The field now reads the correct value directly and we don't need this wrapper.

Test Plan: Poked around Calendar without explosions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11804

Differential Revision: https://secure.phabricator.com/D16779
2016-10-31 15:30:21 -07:00
epriestley
91089acbe5 Begin navigating the mess that is edits to recurring events
Summary:
Ref T11804. This puts us on a path toward some kind of reasonable behavior here.

Currently, cancelling recurring events makes approximately zero sense ever in any situation.

Instead, give users the choice to cancel just the instance, or all future events. This is similar to Calendar.app. (Google Calendar has a third option, "All Events", which I may implement).

When the user picks something, basically do that.

The particulars of "do that" are messy. We have to split the series into two different series, stop the first series early, then edit the second series. Then we need to update any concrete events that are now part of the second series.

This code will get less junk in the next couple of diffs (I hope?) since I need to make it apply to edits, too, but this was a little easier to get started with.

Test Plan:
Cancelled an instance of an event; cancelled "All future events".

Both of them more or less worked in a reasonble way.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11804

Differential Revision: https://secure.phabricator.com/D16778
2016-10-31 14:20:55 -07:00
epriestley
8e5437226f Make calendar intepret all-day dates in a more consistent way
Summary:
In ICS, an event on "Nov 1" starts on "2016-11-01" and ends on "2016-11-02".

This is convenient for computers, but this isn't what users expect to enter in date controls. They expect to enter "nov 1" to "Nov 1" for a one-day, all-day event. This is consistent with other applications.

Store the value the user entered, but treat it as the first second of the next day when actually using it if the event is an all day event.

Test Plan:
Mucked around with multi-day all-day events, recurring all-day events, imports, etc. Couldn't catch any weird/unintuitive stuff anymore offhand.

(Previously, entering "Nov 1" to "Nov 2" created a one-day event, which was unclear.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16777
2016-10-31 14:19:35 -07:00
epriestley
f7b0c09ac4 Make the "All Day Event" control use a checkbox instead of a dropdown
Summary:
This feels a little cleaner:

  - Clean up transaction log a bit.
  - Use a checkbox instead of a two-option dropdown.

This is a little messy because the browser doesn't send anything if the user submits a form with an un-clicked checkbox.

We now send a dummy value ("Hey, there's definitely a checkbox in this form!") so the server can figure out what to do.

Test Plan:
  - Edited all-dayness of an event.
  - Viewed transaction log.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16776
2016-10-31 14:18:59 -07:00
epriestley
182611ef7e Schedule monthly events on the 29th, 30th or 31st relative to the end of the month
Summary:
Ref T11326. If you scheudle a monthly event on the 31st, the default behavior of RRULE means that it only occurs in months with 31 days.

This is actually how Google Calendar and Calendar.app both work: if you schedule a monthly event on the 31st, you get about six events per year.

This seems real confusing and bad to me?

Instead, if the user schedules a monthly event on the 29th, 30th or 31st, pretend they scheduled it on the "last day of the month" or "second-to-last day of the month" or similar, so they always get 12 events per year.

This could be slightly confusing too, but seems way less weird than not getting an event every month.

Test Plan: Scheduled events on the 31st of October, saw them occur in November too after the patch.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16775
2016-10-31 14:13:50 -07:00
epriestley
850fcf0207 Move event recurrence controls to a separate page on the workflow
Summary:
Ref T11326. Currently, the "Create Event" form is pretty wordy. One particular culprit is the "recurring" controls, which are (presumably) rarely used and visually complex.

  - Reflow the default form to hopefully feel a little better.
  - Move recurrence stuff to a separate workflow.

Test Plan:
{F1891355}

{F1891356}

{F1891357}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16774
2016-10-31 14:13:36 -07:00
epriestley
e1a9b76945 Give organizers in ICS exports a dummy email address to placate Gmail
Summary:
Ref T10747. In the in-email ICS event card that Gmail shows, it has a "Who" field which reads "Unknown Organizer*" if the URI for the organizer isn't email-address-like.

Previously, we used a URI like `https://phabricator.install.com/p/username`, which I think is OK as far as RFC 5545 is concerned, but Gmail doesn't like it.

Instead, use `PHID-USER-asdfa@phabricator.install.com`, which doesn't go anywhere, but makes Gmail happy. Users don't normally see this URI anyway.

Test Plan:
Got a readable "Who" in Gmail when importing an event exported from Calendar:

{F1890571}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16772
2016-10-30 09:20:54 -07:00
epriestley
d9c91f857c Apply a TYPE_CREATE transaction when importing events to improve strings in timeline
Summary: Ref T10747. This turns on the newer EditEngine behavior so we get a nice "X created this event." transaction, instead of an "X renamed this from <nothing> to Event Name."

Test Plan: Imported an event, saw a nice timeline.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16771
2016-10-30 09:20:15 -07:00
epriestley
96b064b7e9 Properly import all-day events in Calendar
Summary: Ref T10747. The transaction version of this copies the "all day" flag over properly, but this non-transaction version needs to copy it explicitly.

Test Plan: Imported an all-day event, saw it come in as all-day.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16770
2016-10-30 09:19:58 -07:00