Summary:
Ref T11957. This renames the Configuration storage, transaction, query, and PHID type.
No rename on the actual menu item types yet, that's next (and should be the end of this, I think).
Test Plan:
- Viewed projects.
- Viewed profiles.
- Edited a project menu.
- Grepped for all renamed symbols, I think?
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11957
Differential Revision: https://secure.phabricator.com/D17027
Summary:
Ref T11922. After updating to HEAD of `master`, you need to manually rebuild the index. We don't do this during `bin/storage upgrade` because it can take a very long time (`secure.phabricator.com` took roughly an hour) and can happen while Phabricator is running.
However, if we don't warn users about this they'll just get a broken index unless they go read the changelog (or file an issue, then we tell them to go read the changelog).
This adds a very simple table for notes to administrators so we can write a "you need to go rebuild the index" note, then adds one.
Administrators clear the note by completing the activity and running `bin/config done reindex`. This isn't automatic because there are various strategies you can use to approach the issue, which I'll discuss in greater detail in the linked documentation.
Also, fix an issue where `bin/storage upgrade --apply <patch>` could try to re-mark an already-applied patch as applied.
Test Plan:
- Ran storage ugrades.
- Got instructions to rebuild search index.
- Cleared instructions with `bin/config done reindex`.
Reviewers: chad
Reviewed By: chad
Subscribers: avivey
Maniphest Tasks: T11922
Differential Revision: https://secure.phabricator.com/D16965
Summary:
Ref T6740. When we index a document, also save a copy of the stemmed version.
When querying, search the combined corpus for the terms.
(We may need to tune this a bit later since it's possible for literal, quoted terms to match in the stemmed section, but I think this wil rarely cause issues in practice.)
A downside here is that search sort of breaks if you upgrade into this and don't reindex. I wasn't able to find a way to issue the query that remained compatible with older indexes and didn't have awful performance, so my plan is:
- Put this on `secure`.
- Rebuild the index.
- If things look good after a couple of days, add a way that we can tell people they need to rebuild the search index with a setup warning.
We might get some reports between now and then, but if this is super awful we should know by the end of the weekend.
Test Plan:
WOW AMAZING
{F2021466}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6740
Differential Revision: https://secure.phabricator.com/D16947
Summary:
Ref T11741. InnoDB uses a stopwords table instead of a stopwords file.
During `storage upgrade`, synchronize the table from the stopwords file on disk.
Test Plan:
- Ran `storage upgrade`.
- Ran `select * from stopwords`, saw stopwords.
- Added some garbage to the table.
- Ran `storage upgrade`, saw it remove it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11741
Differential Revision: https://secure.phabricator.com/D16940
Summary:
Ref T11044. One popular tool in a modern operations environment is Puppet. The primary purpose of this tool is to randomly revert hosts to older or different configurations.
Introducing an element of chaotic unpredictability into operations trains staff to be on high alert at all times, rather than lulled into complacency by predictability or consistency.
When Puppet reverts a Phabricator host's configuration to an older version, we might start writing data to a lot of crazy places where it shouldn't go. This will create a big sticky mess that is virtually impossible to undo, mostly because we'll get two files with ID 123 or two tasks with ID 456 or whatever else and good luck with that.
Instead, after changing the partition layout, require `bin/storage partition` to be run. This writes a copy of the config everywhere.
Then, when we start serving web requests, make sure every database has the exact same config. This will foil Puppet by refusing to run requests on hosts it has reverted.
Test Plan:
- Changed partition configuration.
- Ran Phabricator.
- FOILED!
- Ran `bin/storage partition` to sync config.
- Things worked again.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11044
Differential Revision: https://secure.phabricator.com/D16910
Summary: Adds a headerimage and lets you set it on posts for added reverence. Is that a word?
Test Plan:
Add an image, see an image.
{F1923010}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16873
Summary: No view engine yet (adding header image next), but adds subtitle to display like PhameBlog
Test Plan: Add a subtitle, remove a subtitle.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16871
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
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
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
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
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
Summary: Makes a more complete PDF looking invoice form for printing in Phortune.
Test Plan: Make an invoice, click print view, print.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16762
Summary: Is a logo. For merchants.
Test Plan: Set a new logo, remove it. See on list.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T7607
Differential Revision: https://secure.phabricator.com/D16751
Summary:
Ref T10747. When we import a ".ics" file, represent any attendees as simple external references.
For consistency with other areas of the product, I've avoided disclosing email addresses. We'll try to get a real name if we can.
(We store addresses and could expose or use them later, or do some kind of masking junk like "epr...ley@g...l.com" which is utterly impossible to figure out.)
Test Plan: {F1888367}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16759
Summary:
Ref T10747. For URI-based (and, in the future, Google-based) imports, we can automatically refresh them periodically.
(In the general case there's no way to get a push notification for an ICS file, so we just have to do this every-so-often.)
Test Plan:
- Set an ICS file to update hourly.
- Used `bin/trigger fire --id ...` to fire it artificially.
- Saw Calendar update.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16752
Summary: Part of making this look/feel/be more professional is having decent receipts for billing, including contact information (whatever we want to put in there). I'm not using this anywhere at the moment, but will.
Test Plan: Add Contact Info, see Contact Info. Also, why is Remarkup not rendering with line breaks? Seems to be a OneOff thing... anywho... bears!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T7607
Differential Revision: https://secure.phabricator.com/D14125
Summary: Ref T11730. Removes the unused column, seen no issues during past week migrations.
Test Plan: Run migration, check database no longer contains column.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16711
Summary: Ref T10747. When stuff goes wrong (or right) let the user know what happened.
Test Plan: {F1870139}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16704
Summary:
Ref T10747. Adds a bunch of stuff so we can keep track of which events we've imported from external sources.
This doesn't do anything yet: you can't actually import anything.
Test Plan:
- Ran `bin/storage upgrade`.
- Clicked "Imports", saw an empty wasteland.
- Created/edited events.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16696
Summary: Ref T3165. Builds an ngram table for Conpherence Room titles, allowing a tokenizer for searching a subset of rooms.
Test Plan: Say `Gabbert` in two different rooms, search all, see two rooms returned. Search specific room, see specific result.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3165
Differential Revision: https://secure.phabricator.com/D16692
Summary:
Ref T10747. Rough flow is:
- Run a query.
- Select a new "Export Events..." action.
- This lets you define an "Export", which has a unique URL you can paste into Google Calendar or Calendar.app or whatever.
Most of this does nothing yet but here's the boilerplate.
Test Plan: Doesn't do anything yet.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16675
Summary:
Ref T10747.
- Store recurrence as RRULEs internally.
- Use RRULE constants.
- Migrate existing rules to RRULEs.
Test Plan: Ran migration, nothing seemed broken?
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16670
Summary:
Ref T10747. This deprecates "dateFrom", "dateTo", "allDayDateFrom", "allDayDateTo", and "recurrenceEndDate".
They are replaced with "utc*Epoch" fields (for querying) and CalendarDateTime objects (for start, end, until). These objects can represent the full range of dates and times expressible in ICS format, allowing us to import a wider range of ICS events.
Test Plan:
Ran migrations, viewed/edited Calendar, didn't catch anything catastrophcially broken.
This likely needs some followups, I'll keep it local for a bit until I'm confident I didn't break anything too catastrophically. I'm retaining the old data for now so we can likely fix things if it turns out there is some sort of issue.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16664
Summary:
Ref T10747. This does double-writes and starts generating/writing CalendarDateTimes.
This greater flexibility is necessary to support the full range of ICS-specifiable events, including "floating" events.
This doesn't do anything yet.
Test Plan: Created and edited events, verified sensible representations of corresponding datetimes appeared in the database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16661
Summary:
Ref T10747. Currently, Calendar events are mostly epoch-based and cheat a little bit for all-day events.
This already felt a little flimsy, and can't reasonably accommodate the full range of `.ics` events, which include "floating" events (e.g., occurs at 3PM regardless of timezone, like "Tea Time").
As a secondary issue, we identify instances of a recurring event by instance number (1, 2, 3, etc.). This can't accommodate the full range of `.ics` events, which include arbitrary additional "RDATE" events (e.g., recurrs every week, and also on these specific extra days).
However, we do need to store some epoch information so we can do query windowing: when the user looks at "October 2016", we want to select the smallest number of events that we can from the database initially, before refining them down to generate instances. We can't reasonably query the actual dates no matter how we store them because this depends on computing things like UNTIL, COUNT, initial dates, whether events are recurring or not, timezones, etc.
Instead, when we save an event compute the earliest second it occurs on in UTC and the latest second it occurs on in UTC. We can then query for a small superset of possible events in "October 2016" for any viewer pretty easily.
Also, start laying the groundwork for using fewer epochs in the rest of the code, and for reducing the role of sequence indexes (I plan to keep some sequences indexes around, probably, since they're nice in the UI, but not all child events will have indexes since there's no index for an RDATE event).
This doesn't migrate existing events yet or actually read these new columns -- that will come later once the new code is a little more solid.
Test Plan:
- Ran `bin/storage upgrade`.
- Created a new event.
- Saved an existing event.
- Viewed database, saw sensible-looking "UTC Epoch" values.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16652
Summary: This moves room pictures out of the dialog and into it's own PictureController. Also adds a standard image (and removes the "last person to chat" picture (though we could add that back. My plan is though that direct messages use auto use the other person's photo, after we have editengine and room pictures will have a plain, replaceable image.
Test Plan: Set a new room picture, remove a picture. Run migration, see old images properly set with new image.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16669
Summary:
Fixes T11705. I did not realize that `ON DUPLICATE KEY UPDATE` was order-dependent, so the "reset" clause of this `IF(...)` never actually worked.
Reorder it so we check if we're changing the message type //first//, then actually change the message type.
This makes the count reset properly when a failing repository succeeds, or a working repository fails.
Test Plan:
- On `master`, forced a working repository to fail a `bin/repository update`, saw the message change types (expected) but keep the old count (wrong!).
- With this patch, repeated the process and saw the count reset properly.
- Ran the patch, verified counts reset to 0.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11705
Differential Revision: https://secure.phabricator.com/D16623
Summary: Ref T11217. This just adds the table that we'll store tokens in. It doesn't make use of the table at all yet. This is mostly pulled from this diff (D16178). Specifically I mostly followed Evan's instructions related to the token table here: D16178#189120.
Test Plan: I ran `./bin/storage upgrade` successfully and there were no schema errors.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T11217
Differential Revision: https://secure.phabricator.com/D16621
Summary: Fixes T10681. Adds a search API endpoint and an edit API endpoint for Phurl URLs. I still need to add the ability to search by name, alias, URL, and maybe description.
Test Plan: Test the methods through `/conduit/method/phurls.search/` and `/conduit/method/phurls.edit/`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T10681
Differential Revision: https://secure.phabricator.com/D16600
Summary:
Ref T4190. Currently only have the endpoint and controller working. I added caching so subsequent attempts to proxy the same image should result in the same redirect URL. Still need to:
- Write a remarkup rule that uses the endpoint
Test Plan: Hit /file/imageproxy/?uri=http://i.imgur.com/nTvVrYN.jpg and are served the picture
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T4190
Differential Revision: https://secure.phabricator.com/D16581
Summary:
Ref T11665. Currently, when a repository hits an error, we retry it after 15s. This is correct if the error was temporary/transient/config-related (e.g., bad network or administrator setting up credentials) but not so great if the error is long-lasting (completely bad authentication, invalid URI, etc), as it can pile up to a meaningful amount of unnecessary load over time.
Instead, record how many times in a row we've hit an error and adjust backoff behavior: first error is 15s, then 30s, 45s, etc.
Additionally, when computing the backoff for an empty repository, use the repository creation time as though it was the most recent commit. This is a good proxy which gives us reasonable backoff behavior.
This required removing the `CODE_WORKING` messages, since they would have reset the error count. We could restore them (as a different type of message), but I think they aren't particularly useful since cloning usually doesn't take too long and there's more status information avilable now than there was when this stuff was written.
Test Plan:
- Ran `bin/phd debug pull`.
- Saw sensible, increasing backoffs selected for repositories with errors.
- Saw sensible backoffs selected for empty repositories.
Reviewers: chad
Maniphest Tasks: T11665
Differential Revision: https://secure.phabricator.com/D16575
Summary:
Minor rebuild / redesign of Conpherence. Most of this is new UX and tossing out things like widgets, device fallbacks. I expect some of the UI to get more polished after next pass, but most everything here is in place.
- Removed "Widgets", now just a single Participants pane
- Added "Topic"
- New header
- Settings, Edit are action icons
- Removed a lot of JS
- Simplified CSS as much as I could
Test Plan:
Desktop, Tablet, Mobile. Adding and removing people. Setting new topics, new rooms.
{F1828662}
{F1828669}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16550
Summary: Fixes T11532. The language selection for pastes is now a typeahead that is backed by `pygments.dropdown-choices`. There is still a bit of weirdness around making "auto-detection" the default state. To actually select a different language, you first need to remove the "auto detect" option that is pre-populated in a new paste. Other than that, it works as intended.
Test Plan:
Create a new paste with a file extension that can be auto-detected.
Created a new paste and manually selected the language
Edited a paste and changed the language.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T11532
Differential Revision: https://secure.phabricator.com/D16463
Summary: Fixes T9117. Adds a migration to remove ponder vote data.
Test Plan: I added a bunch of lines to phabricator_user.edge with type 18 and they were successfully removed by this patch
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim
Maniphest Tasks: T9117
Differential Revision: https://secure.phabricator.com/D16452
Summary: Ref T11522. This migrates any "badcommit" data (which probably only exists at Facebook and on 1-2 other installs in the wild) to the new "hint" table.
Test Plan:
- Wrote some bad commit annotations to the badcommit table.
- Viewed them in the web UI and used `bin/repository reparse --change ...` to reparse them. Saw "this is bad" messages.
- Ran migration, verified that valid "badcommit" rows were successfully migrated to become "hint" rows.
- Viewed the new web UI and re-parsed the change, saw "unreadable commit" messages.
- Viewed a good commit; reparsed a good commit.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11522
Differential Revision: https://secure.phabricator.com/D16435
Summary:
Ref T11522. This provides storage for tracking rewritten commits (new feature) and unreadable commits (existing feature, but really hacky).
This doesn't do anything yet, just adds a table and a CLI tool for updating it. I'll document the tool once it works. You just pipe in some JSON, but I need to document the format.
Test Plan:
- Piped JSON for "none", "rewritten" and "unreadable" hints into `bin/repository hint`.
- Examined the database to see that the table was written properly.
- Tried to pipe bad JSON in, invalid hint types, etc. Got reasonable human-readable error messages.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11522
Differential Revision: https://secure.phabricator.com/D16434
Summary: Adds a schema patch that removes conduit_connectionlog. This table hasn't been used in 8ish months so it's probably safe to get rid of.
Test Plan: Apply the patch locally and confirm that the table does indeed get dropped.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D16438
Summary:
Fixes T11453. Currently, commit message summaries are limited to 80 bytes. This may only be 20-40 characters for CJK languages or langauges with Cyrillic script.
Increase storage size to 255, then truncate to the shorter of 255 bytes or 80 glyphs. This preserves the same behavior for latin languages, but is less tight for Russian, etc.
Some minor additional changes:
- Provide a way to ask "how much data fits in this column?" so we don't have to duplicate column lengths across summary checks or UI errors like "title too long".
- Remove the `text80` datatype, since no other columns use it and we have no use cases (or likely use cases) for it.
Test Plan:
- Made a commit with a Cyrillic title, saw reasonable summarization in UI:
{F1757522}
- Added and ran unit tests.
- Grepped for removed `SUMMARY_MAX_LENGTH` constant.
- Grepped for removed `text80` data type.
Reviewers: avivey, chad
Reviewed By: avivey
Subscribers: avivey
Maniphest Tasks: T11453
Differential Revision: https://secure.phabricator.com/D16385
Summary: Ref T8116. Add search-by-name and per-package / per-publisher search to Packages.
Test Plan: Searched publishers, packages, versions by name. Searched packages by publisher. Searched versions by package.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8116
Differential Revision: https://secure.phabricator.com/D16320
Summary:
Ref T8116. A version has:
- a package (like "Arcanist") which it belongs to;
- a name (like "v3.1.5").
The name is immutable and unique, like the package key and publisher key.
Policy stuff:
- Versions have the exact same policies as their packages.
- You must be able to edit a package to create new versions of it.
This is still entirely uninteresting.
Test Plan: {F1731703}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8116
Differential Revision: https://secure.phabricator.com/D16316
Summary:
Ref T8116. A package has:
- a publisher (like "Phacility"), from the previous revision;
- a name (like "Arcanist");
- a package key (like "arcanist").
The package key is immutable, like the publisher key.
This gives a package a full key like "phacility/arcanist".
Policy stuff:
- You must be able to view a publisher to view a package (currently, everyone can always see all publishers).
- You must be able to edit a publisher to create a new package inside it.
- Packages have separate view/edit permissions.
This still does nothing interesting.
Test Plan: {F1731663}
Reviewers: chad
Reviewed By: chad
Subscribers: eadler
Maniphest Tasks: T8116
Differential Revision: https://secure.phabricator.com/D16315
Summary:
Ref T8116. Partially scavenged from D14152. This roughs in a new Packages application for Arcanist extensions and third-party applications, and adds a "Publisher" object.
A "Publisher" represents an individual or entity who is publishing a package, like "Phacility". It's explicitly //not// necessarily the original author -- just the primary entity vouching for the safety of the code.
A publisher just has a name and a unique key for now. For example, Phacility might have "Phacility" and "phacility", respectively.
Unique keys are immutable, e.g., the package "phacility/arcanist" will always be exactly the same package by exactly the same publisher.
Test Plan: {F1731621}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8116
Differential Revision: https://secure.phabricator.com/D16314
Summary:
Fix T11339.
Now, old and new are both simple lists of phids, and the rendering should make sense.
Test Plan: Viewed existing transaction with all 3 states.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T11339
Differential Revision: https://secure.phabricator.com/D16311
Summary:
Ref T11326. Normally, events occur at a specific epoch, independent of the viewer. For example, if we're having a meeting in 35 hours, every user who looks at the event will see that it starts 35 hours from now.
But when an event is "All Day", the start time and end time depend on the //viewer//. A day like "Christmas" does not start at the same time for everyone: it starts sooner if you're in a more-eastern timezone. Baiscally, an event on "July 15th" starts whenever "July 15th" starts for whoever is looking at it.
Previously, we stored these events by using the western-most and eastern-most timezones as the start and end times (the earliest possible start and latest possible end).
This worked OK, but we get into a bunch of trouble with EditEngine, mostly because each field can be updated individually now. We can't easily tell if an event is all-day or not when reading or updating the start time and end time, and making that easier would introduce a huge amount of complexity.
Instead, when we update the start or end time, we write //two// times:
- The epoch timestamp of the time the user entered, which is the start time we will use if the event is a normal event.
- The epoch timestamp of 12:00 AM in UTC on the same date as the //local// date the user entered. This is pretty much like just storing the date the user actually typed. This is what w'ell use if the event is an all-day event.
Then, no matter whether the event is later made all-day or not, we have all the information we need to display it correctly.
Test Plan:
- Created and edited all-day events.
- Migrated existing all-day events, which appeared to survive without problems. (Note that events all-day which were created or edited in the last couple of days `master` won't survive this mutation correctly and will need to be fixed.)
- Created and edited normal, recurring, and recurring all-day events.
- Swapped back to `stable`, created an event, specifically migrated it forward, made sure it survived with times intact.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16305
Summary: Ref T10909. Ref T9224. We label this field "Host" in the UI; make the storage format consistent.
Test Plan:
- Viewed month view, day view, detail view of an event.
- Created a new event, saw myself as the host.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9224, T10909
Differential Revision: https://secure.phabricator.com/D16291
Summary:
Ref T9275. When you create a recurring event which recurs forever, we want to avoid writing an infinite number of rows to the database.
Currently, we write a row to the database right before you edit the event. Until then, we refer to it as `E123/999` or whatever ("instance 999 of event 123").
This creates a big mess with trying to make recurring events work with EditEngine, Subscriptions, Projects, Flags, Tokens, etc -- all of this stuff assumes that whatever you're working with has a PHID.
I poked at letting this stuff work without a PHID a little bit, but that looked like a gigantic mess.
Instead, generate an event "stub" a little sooner (when you look at the event detail page). This is basically just an ID/PHID to refer to the instance.
Then, when you edit the stub, "materialize" it into a real event.
This still has some issues, but I think it's more promising than the other approach was.
Also:
- Removes dead user profile calendar controller.
- Replaces comments with EditEngine comments.
Test Plan:
- Commented on a recurring event.
- Awarded tokens to a recurring event.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9275
Differential Revision: https://secure.phabricator.com/D16248
Summary:
Fixes T11307. Fixes T8124. Currently, builtin files are tracked by using a special transform with an invalid source ID.
Just use a dedicated column instead. The transform thing is too clever/weird/hacky and exposes us to issues with the "file" and "transform" tables getting out of sync (possibly the issue in T11307?) and with race conditions.
Test Plan:
- Loaded profile "edit picture" page, saw builtins.
- Deleted all builtin files, put 3 second sleep in the storage engine write, loaded profile page in two windows.
- Before patch: one of them failed with a race.
- After patch: both of them loaded.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8124, T11307
Differential Revision: https://secure.phabricator.com/D16271
