Summary: Just making profiles a little nicer, adds a big picture, easier mechanism for updating photos. Also larger profile pictures... need to re-thumb?
Test Plan:
View my profile, edit my picture, view a stranger, see profile. Check mobile, tablet, desktop. Check action menu on mobile.
{F2559394}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17295
Summary: Adds most up to date version of FontAwesome
Test Plan: {icon snowflake-o}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17293
Summary: Ref T12174. Removed selected state CSS on mobile devices.
Test Plan: Set Chrome to responsive and check each break point.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17292
Summary: I removed this for cleaner UI on desktop, but still looks nice on mobile.
Test Plan: Test mobile, tablet, desktop search box.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17291
Summary: Ref T12174. More white and bolder. So crisp
Test Plan: Hover.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17286
Summary:
Ref T12174.
- Fix text antialiasing pop-in at the end of the animation in Safari.
- Fix flickery animation replay when mousing inside a tooltip element, or rapidly switching between tooltip elements.
- To accomplish this: don't play the animation if the last tip was hidden less than 500ms ago.
Test Plan:
- Viewed a tooltip in Safari.
- Waved cursor wildly over application items, both left-right (within an item) and up-down (between items).
Tooltips appeared stable and non-flickery in all cases.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17272
Summary: Ref T11957. Needs some more polish, but I think everything here is square.
Test Plan: Add personal/global items to home, test mobile. Test workboards / colors.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: 20after4, rfreebern, Korvin
Maniphest Tasks: T11957
Differential Revision: https://secure.phabricator.com/D17259
Summary: Ref T12139. Adds sorting by shortname. Also I sorted everything else. No reason. It didn't help
Test Plan: `:star`
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12139
Differential Revision: https://secure.phabricator.com/D17246
Summary: Moves the fonts around for better Windows fallback
Test Plan: Windows 10 Edge / Chrome
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17245
Summary:
This adds a more complete emoji datasource, with a typeahead and autocomplete. It works by pulling in a raw datasource from EmojiOne (I chose Unicode 8, but they have a Unicode 9 datasource as well) and transforming it for speed/need. If we build more robustness or an actual picker into the Remarkup bar, having the additional keywords, etc, might be important. When Unicode 9 support is more prevalent, we should only need to update the single file.
Tossing up as a proof of concept on engineering direction. Also I can't quite get the autocomplete to complete.
Test Plan: Test UIExamples, Autocomplete, and TypeaheadSource
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12139
Differential Revision: https://secure.phabricator.com/D17244
Summary:
Fixes T12152. When creating a subproject task directly from a parent project, we can get column information back about the subproject column, which isn't visible. Just ignore this rather than trying to draw a card into a column which does not exist.
This flow (see test plan) is a little unintuitive since the card never appears on the workboard, but this class of things is probably roughly somewhere in T4900.
Test Plan:
- Create a parent project A.
- Create subproject B.
- On the workboard for A, select "Create Task..." from the dropdown.
- Add tag "B" in the dialog that pops up.
- Save the task.
Note that this flow creates a task tagged only with "B", since "A" and "B" are mutually exclusive tags.
Before patch: UI freezes (JX.Mask is never removed), JS error in console.
After patch: workflow completes. Note that card is (correctly) not visible: it's in an invisible subproject column.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12152
Differential Revision: https://secure.phabricator.com/D17242
Summary: Ref T12139, installs 'Segoe UI Emoji' as a standard font call for color emoji on Windows devices.
Test Plan: Review Emoji on Win 10 Chrome / Edge, Mac Chrome / Safari.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12139
Differential Revision: https://secure.phabricator.com/D17241
Summary: Removes the often funny, but never really used but will cause us bug reports someday.... cat facts.
Test Plan: Install cat facts, run storage upgrade, see no cat facts in menu.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12126
Differential Revision: https://secure.phabricator.com/D17233
Summary:
Earlier, I made some changes so that when you create or edit an inline, the comment at the bottom of the page updates (even though you didn't fiddle with the stacked actions inputs).
At the last second I broke them by spelling this wrong while cleaning things up, so they didn't actually work. Spell the property correctly ("showPreview", not "shouldPreview").
Also, we have some JS which rewrites "Not Visible" into "View", but it fires in an inconvenient way now and is flickery for me. Ideally this should get cleaned up slightly better eventualy, but at least make is stop doing so much flickery layout for now.
Test Plan:
- Wrote no comment on a revision.
- Added an inline.
- Saw comment preview properly update immediately.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17229
Summary:
See D17210. Currently, this handler needs to be installed on each menu that doesn't build with the default behavior.
Rather than copy-pasting it to the user menu, try to make it a default behavior. This adds a new rule: don't close the menu if the item is a dynamic item built in JS with PHUIXActionView.
This allows dynamic items to control the menu themselves, while giving static items the desired default behavior.
Test Plan:
- Opened menus on: dashboards, user menu, timeline comments. Clicked stuff. Menus went away.
- Other menus still seemed to work right: Diffusion, Favorites, mobile menu.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17222
Summary: Builds out more UI to reinforce just who you are in this world... A perfect person.
Test Plan:
Look at myself a lot.
{F2435202}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17224
Summary: Ref T5867. We have a bit more common JS now.
Test Plan: Loaded some pages, saw fewer requests. Sanity-checked these resources against Multimeter in production.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5867
Differential Revision: https://secure.phabricator.com/D17220
Summary: Ref T5867. I sure love Javascript.
Test Plan: Navigated between Home, Diffusion and Differential, opening the user profile menu. Saw appropraite help items.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5867
Differential Revision: https://secure.phabricator.com/D17214
Summary:
Still lots to fix here, punting up since I'm running into a few roadblocks.
TODO:
[] Sort Personal/Global correctly
[] Quicksand in Help Items correctly on page changes
Test Plan: Verify new menus work on desktop, tablet, mobile. Test logged in menus, logged out menus. Logging out via a menu, verify each link works as expected. Help menus get build when using an app like Maniphest, Differential. Check that search works, preferences still save.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12107
Differential Revision: https://secure.phabricator.com/D17209
Summary:
In D16157, dropdown menus got an overly-broad check for not closing when an item is clicked.
Specifically, we don't want to close the menu if the item is really opening a submenu, like "Edit Related Objects..." does on mobile.
The check for this is too broad, and also doesn't close the menu if the item has workflow.
Instead, use a narrower check.
Test Plan:
- Menu still stays open when toggling submenus like "Edit Related Objects".
- Menu now closes properly when using workflow items like "Edit Comment" or "Remove Comment".
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17210
Summary: Never really used this to full potential and takes up a lot of code and space. Remove option for now and make all profile nav menus small by default.
Test Plan: Review user, project, workboard. Set new menus.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T5867
Differential Revision: https://secure.phabricator.com/D17206
Summary: Moves basic colors into the processor.
Test Plan: Review a diff in sandbox with and without change.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17173
Summary: Ref T12089. This reverts back to using "bright" colors for full width changes in green and red.
Test Plan:
Review diffs, see full width colors.
{F2333933}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12089
Differential Revision: https://secure.phabricator.com/D17171
Summary: Fixes T12090. In obscure situations lost to the mists of time, the `changes` column could be `null`. Force a string cast so the migration finishes, even though these changesets are likely meaningless.
Test Plan:
I did a force-reapply as a sanity check:
```
$ ./bin/storage upgrade -f --apply phabricator:20161213.diff.01.hunks.php
```
That went cleanly; it would only have caught dramatic errors, but I didn't completely butcher things.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12090
Differential Revision: https://secure.phabricator.com/D17168
Summary:
Fixes T10968. In rare situations, we can generate a diff, then hit an error which causes this update to fail.
When it does, we tend to get stuck in a loop creating diffs, which can fill the database up with garbage. We saw this once in the Phacility cluster, and one instance hit it, too.
Instead: when we create a diff, keep track of which commit we generated it from. The next time through, reuse it if we already built it.
Test Plan:
- Used `bin/differential attach-commit <commit> <revision>` to hit this code.
- Simulated a filesystem write failure, saw the diff get reused.
- Also did a normal update, which worked properly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10968
Differential Revision: https://secure.phabricator.com/D17164
Summary: Ref T5867, adds a customPHID field, nullable, and lets you query by it... i think? Not fully able to grok all the EditEngine stuff, but I think this is the right place for the query.
Test Plan: Not wired to anything, but pulling up project menu, editing, all still works.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T5867
Differential Revision: https://secure.phabricator.com/D17149
Summary:
Fixes T10687. Fixes T12064.
- Primarily, adds a margin around the edge of the screen for the purposes of aligning the tooltip.
- Also, tries to flip the tooltip if it can (e.g., if the tooltip normally goes east, try west first), then tries other positions exhastively.
Test Plan:
{F2309363}
{F2309364}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12064, T10687
Differential Revision: https://secure.phabricator.com/D17145
Summary: Minor color saturation here, ideal for low quality monitors.
Test Plan:
Review new colors in various scenarios.
{F2305178}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17141
Summary: Tweaks the diff colors here a bit, as well as making full diffs slightly easier to read in full. Ref T12060
Test Plan:
Tested prose diffs, email prose diffs, and a regular Differential revision.
{F2304056}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12060
Differential Revision: https://secure.phabricator.com/D17138
Summary: Ref T3612. Doesn't render correctly, need help please. Adds a download icon into the renderfilelinkview to allow easier downloads.
Test Plan: Click on link, get download, click on file, get lightbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16980
Summary:
Ref T11114. When a user selects "Accept", and then selects "Reject", remove the "Accept". It does not make sense to both accept and reject a revision.
For now, every one of the "actions" conflicts: accept, reject, resign, claim, close, commandeer, etc, etc. I couldn't come up with any combinations that it seems like users are reasonably likely to want to try, and we haven't received combo-action requests in the past that I can recall.
Test Plan:
- Selected "Accept", then selected "Reject". One replaced the other.
- Selected "Accept", then selected "Change Subscribers". Both co-existed happily.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17132
Summary: Ref T11114. This comments nearly working on EditEngine. Only significant issue I caught is that the "View" link doesn't render properly because it depends on JS which is tricky to hook up. I'll clean that up in a future diff.
Test Plan: {F2279201}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17116
Summary: Ref T11114. This begins restoring comment actions to Differential, but on top of EditEngine.
Test Plan: {F2263148}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17107
Summary:
Fixes T12049. This expands "Haunted" comment panels to EditEngine, and by extension to all EditEngine applications.
Eventual goal is to remove custom commenting code in Differential and replace it with EditEngine code.
Changes from current "haunt" mode:
- This only has one mode ("pinned"), not two ("pinned", "pinned with preview"). There's an inline preview and scroll behavior is a little better.
- Now has a UI action button.
Slightly tricky stuff:
- This interacts with "Fullscreen" mode since it doesn't make sense to pin a full-screen comment area.
- This should only be available for comments, not for remarkup fields like "Description" in "Edit Task".
Test Plan:
- Pinned/unpinned in Maniphest.
- Pinned/fullscreened/unfullscreened/unpinned.
- Checked that "Edit Task" doesn't allow pinning for "Description", etc.
- Pressed "?", read about pressing "Z".
- Pressed "Z".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12049
Differential Revision: https://secure.phabricator.com/D17105
Summary: More obvious crumbs, remove border on images, better image spacing.
Test Plan: Build out a test blog, click on crumbs, view spacing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17101
Summary: Build ngram indexs, adds search by name capability.
Test Plan: Search for a dashboard by partial name, search for a panel by partial name.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17090
Summary: Fixes T12020. These callsites to `getPhrictionSlug()` were missed when that method was removed. They're very old (early 2014, late 2011).
Test Plan:
These are tricky to test because the migrations are so ancient, but `bin/storage upgrade --force --apply phabricator:20140521.projectslug.2.mig.php` gave me //plausible// results.
The other migration is so ancient that it can't apply to a modern database so I'm just kind of winging that one. We probably have essentially no installs which will ever apply it again, though.
Reviewers: chad, avivey
Reviewed By: avivey
Maniphest Tasks: T12020
Differential Revision: https://secure.phabricator.com/D17060
Summary: Reorgaizes the CSS here a bit, by object list style, adds in a new drag ui class, which will be used in menu ordering.
Test Plan:
Workboards, Home Apps.
{F2126266}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17057
Summary:
Ref T8475. This forces installs to migrate hunks to the modern format.
We stopped writing to the legacy format a very long time ago (2+ years?) without issues.
This doesn't destroy any data. T8623 has guidance and I'll publish more changelog guidance.
Test Plan: Faked some legacy data and migrated it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8475
Differential Revision: https://secure.phabricator.com/D17039
Summary: Didn't grep this good enough.
Test Plan: `bin/storage upgrade -f --apply ..`, got a clean apply.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D17046
Summary: Allows users set an icon (for reuse on upcoming home) for their dashboard based on 16 descriminating choices.
Test Plan: Create a new dashboard, set new icon. Edit an existing dashboard, set icon.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17042
Summary: Adds authorPHID to panels so we can default to the panels you made.
Test Plan: Run upgrade, visit manage panels, see my panels. Create a new panel. Edit a panel.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17036
Summary: Adds an authorPHIDs, populates olds ones.
Test Plan: Make a new Dashboard, see that I created it.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17022
Summary: We're currently applying these styles to labels, restrict them to only list elements that have an href.
Test Plan: Test a label, an anchor, and a button as an action item.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17031
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: Fixes T11990. Chrome/Ubuntu doesn't respect `outline: auto`, so this is the only workaround I could fine. Somewhat nicer than stock Chrome.
Test Plan: tab tab tab tab, ubuntu, chrome, windows, safari.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11990
Differential Revision: https://secure.phabricator.com/D17023
Summary: Use one set of spacing everywhere for action menus
Test Plan: Review dropdowns, action lists.
Reviewers: epriestley, tyhtest
Reviewed By: tyhtest
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17006
Summary: Form inputs with just text or certain selects had extra space. This uses better text centering.
Test Plan: Review forms on all Settings pages, Custom Policy, Herald.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17005
Summary:
Persona is going to be decommed November 30th, 2016.
It is highly unlikely that anyone is currently using persona as a real
login method at this point.
Test Plan: tried locally to add auth adapter.
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D16371
Summary: Ref T3612. Hides badges on the comment panel.
Test Plan: Give myself a badge, leave a comment, see no badge UI.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16979
Summary: Ref T3612, prevents lightbox from spawning from inside a lightbox.
Test Plan: Click on file lightbox, leave file comment, click file comment, get take to file page instead of another lightbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16978
Summary: Ref T3612, this adds a anchor around the large icon with hover state so you can download from here as well.
Test Plan: Hover over .ics file, click, get download.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16977
Summary: Fixes T11791. We do this in durable column, but not in regular Conpherence. I think this is the right place? Not sure how this will feel with high lag.
Test Plan: Submit lots of text in a Conpherence.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11791
Differential Revision: https://secure.phabricator.com/D16969
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: This is still reasonably functional and useful to people, and we don't have better mechanics to offset the change.
Test Plan: New Workboard, set Workboard color, test mobile, desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16964
Summary: Ref T3612. Mobilizes the new lightbox, changes large buttons to circle icons like Conpherence.
Test Plan: Click each new button on desktop, mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16961
Summary: Ref T3612. Moves the listener to the frame of the image.
Test Plan: Click on image, no close, click on grey frame, closes image. Test image and document, clicking on arrows.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16959
Summary: Ref T3612. Passes in file size and file icon for non-images.
Test Plan: Review a PDF and PSD in a lightbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16957
Summary: Spruce up the file embeds a little more, hover state, icons, file size.
Test Plan:
Add a psd and pdf, see new icons. Check differential, still see icons there too. Test mobile, desktop.
{F2042539}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16950
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: Removes the viewable restriction on embedded files. Builds a basic lightbox UI for commenting.
Test Plan:
Add psd, pdf to Maniphest task, clicked on download, comment, left comment. Closed box.
{F1943726}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16917
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 comment box, you can put text into it, hit enter, and see it come back.
Test Plan: Put text into box, see it come back.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16907
Summary: Basic work in progress, but should show timeline comments for files when in lightbox mode. Looks reasonable.
Test Plan: click on images, see comments from timeline.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T3612
Differential Revision: https://secure.phabricator.com/D16896
Summary:
Ref T8510. Use "\n" as a delimiter between name sections. Specifically, project "AAA" with tag "zzz" should be a better match for query "AAA" than project "AAA BBB" is.
Make use of this delimiter slighlty more obvious in the UI.
Test Plan:
- Created projects "Phacility" and "Phacility Core Access".
- Typed "Phacility".
- Before patch: first hit is "Phacility Core Access".
- After patch: first hit is "Phacility".
- Viewed debugging output table, saw visual explanation of behavior.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8510
Differential Revision: https://secure.phabricator.com/D16886
Summary:
Fixes T11886. D16882 prevented workboard cards from being dragged between columns because it reduced the effective document height to almost nothing (~78px, just the menu bars).
Instead, use the larger of the document height and viewport height as "infinity".
Test Plan: Dragged columns between workboards successfully.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11886
Differential Revision: https://secure.phabricator.com/D16885
Summary: Visually, I think these are much cleaner (with colors), and provide more workspace. I also don't really use the sidenav here and if I did, it would be to go back to the project homepage. I think this is overall better. If navigation page to project home is difficult or hard to find, we can maybe make a better header / crumbs bar to reduce that.
Test Plan: New project -> basic new board. Existing project -> color board. Desktop, Mobile, Fullscreen
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16882
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: Fixes some outlying issues with new comment box and Phame Posts.
Test Plan: Review CSS in sandbox, post new reply, see proper spacing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16869
Summary:
Ref T11816. We currently try to expand the picker control from the little calendar icon. This works alright on desktop, but not great on tablet/mobile.
On tablet/mobile, center the control on screen instead. Also, mask the background since it can get a bit busy because we can't really control what's under the control anymore.
Finally, move the little popup thing around if the user resizes the window.
Test Plan: {F1922773}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11816
Differential Revision: https://secure.phabricator.com/D16866
Summary:
Ref T11853. My CSS change for the more enormous policy dialog was a little too broad, and affected the "You shall not pass!" dialog too.
Narrow the scope of the CSS rules.
Also add a missing "." that I caught.
Test Plan:
- Looked at policy exception dialogs.
- Looked at policy explanation dialogs.
- Looked at the end of that sentence.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11853
Differential Revision: https://secure.phabricator.com/D16841
Summary: On really wide selects, text will go over the background image. Give it more padding.
Test Plan: Review selects on email preferences page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16842
Summary: Some aftermath fallout from rebuilding the comment box.
Test Plan: Go fullscreen, pop back out.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16836
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
Summary: Redesign the action comment box for better use in two column, mobile, nuance.
Test Plan: Test in mobile / desktop / tablet, adding and removing actions. Actionless comment boxes, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: cspeckmim, Korvin
Differential Revision: https://secure.phabricator.com/D16811
Summary:
Ref T8510. We had two issues with mixed-case result sorting, like typing `@joe` to match user `Joe`.
- The fallback sort was not normalized properly, so "J" could sort after "j". Instead, normalize values for sorting.
- The `prefix_hits` and older `priority_hits` mechanisms were competing destructively. The `prefix_hits` mechanism completely replaces the `priority_hits` mechanism. Instead, use only the `prefix_hits` mechanism.
Test Plan:
- Copied results for "joe" from WMF.
- Hard-coded the controller to return them.
- Searched for `@joe`.
- After patches, first hit is user "Joe".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8510
Differential Revision: https://secure.phabricator.com/D16826
Summary:
Ref T7643. When you do something like this:
- Edit a task description.
- Click "Show Details" on the resulting transaction.
- Get a prose diff dialog showing the change.
...now add some "Old" and "New" tabs. These are useful for:
- reverting to the old text by copy/pasting;
- reading just the new/old text if the diff is noisy;
- sometimes just nice to have?
(This looks a little rough but I didn't want to put a negative margin on tab groups inside dialogs? Not sure what the best fix here is.)
Test Plan: {F1909390}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7643
Differential Revision: https://secure.phabricator.com/D16817
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:
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
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: I think maybe these should be more separate from JX.Title, but seems to work ok. May build new favicons just for messages though. Proof of concept UI.
Test Plan: Send message on one browser, see red icon in other browser. Click on menu, count and favicon switch back to normal.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16734
Summary:
See D16734.
- Add ".ico" files to the Celerity map.
- Add a formal route for "/favicon.ico".
- Remove instructions to configure `/rsrc/` and `/favicon.ico` rewrite rules.
Long ago, we served resources directly via `/rsrc/` in at least some cases. As we added more features, this stopped working more and more often (for example, Apache can never serve CSS this way, because it doesn't know how to post-process `{$variables}`).
In modern code (until this change), only `/favicon.ico` is still expected to be served this way.
Instead, serve it with an explicit route via controller (this allows different Sites to have different favicons, for example).
Remove the instructions suggesting the old rewrite rules be configured. It's OK if they're still in place -- they won't break anything, so we don't need to rush to get users to delete them.
We should keep "webroot/favicon.ico" in place for now, since it needs to be there for users with the old rewrite rule.
Test Plan:
- Ran celerity map.
- Loaded `/favicon.ico`, got resource via route.
- Used `celerity_generate_resource_uri()` to get paths to other icons, loaded them, got icons.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16737
Summary: This has been causing scrolling issues for me for a while, turns out the footer behind the Conpherence layout can bubble up and cause scrolling issues if you don't hit the scrollarea in the right place with your finger. Hiding it via CSS or removing the footer in the configuration resolves the issue on my phone on secure.
Test Plan: Test with and without footer on secure, hide footer in CSS since it's not visible anyways.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16732
Summary: I seemed to have missed this, lots of funky desktop padding on mobile.
Test Plan: Review Config / Maniphest / etc on a mobile browser.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16731
Summary: Fixes T11764. Moves rendering of the column to client-side, which can skip if it detects we're on mobile.
Test Plan: Open column on desktop, switch to mobile, don't see column. Toggle column on mobile on and off. Switch back to desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11764
Differential Revision: https://secure.phabricator.com/D16725
Summary: tablet viewpoints here are wide enough to support the participant column, go ahead and give it dedicated space.
Test Plan: review tablet, desktop, and mobile breakpoints in conpherence with column toggled on and off
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16724
Summary:
Ref T10747. When a user drops a ".ics" file or a bunch of ".ics" files into a calendar view, import the events.
(Possibly we should just do this if you drop ".ics" files into any application, but we can look at that later.)
Test Plan: Dropped some .ics files into calendar views, got imports.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10747
Differential Revision: https://secure.phabricator.com/D16722
Summary: This removes 'full-display', 'minimal-display' from Conpherence, which I recall was because we had 2 UIs for column and regular chat. I'm also tossing in slightly nicer search results, with a link to the actual message and the full date shown for context.
Test Plan: Post a message in mobile, tablet, full conpherence, and in durable column. Clean up UI in durable column. Do a search in Full UI, click on result date, get taken to the message... usually. My test data is a little wonky, but I think this works most of the time.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16710
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: This focuses the search field when the user opens search, and then the textarea for pontificate if the search box is closed.
Test Plan: Open/Close/Open/Close
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16709
Summary: Basically all here, but still probably needs some polish (links to jump? full dates?). Looks much better, still duplicates messages though sometimes. Needs to debug that more.
Test Plan:
Revisit search UI inside Conpherence, outside Conpherence, and normal room searches in Conpherence.
{F1870748}
{F1870749}
{F1870750}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16708
Summary: I passed this in as a config, but need to parse it live when threads change, otherwise the wrong room could be searched.
Test Plan: Search in one room, click a second, search again, see correct results.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16707
Summary: Hides the search form itself as well when not in use.
Test Plan: Write lots of posts, scroll up to "see more messages", see I can click and load messages now.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16706
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: Fixes the send on enter flash, only uses the `Threads` loading animation on changing threads, not sending a message.
Test Plan: Change threads, post a message.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16705
Summary: Adds a search bar toggle and results for searching inside a Conpherence Room. The UI of the results itself are not styled yet, and will follow up with another diff.
Test Plan: Go to Conpherence, search for "asdf", get lots of results. Search for nothing, get no change, search for something fictitious, get no threads found (will follow up with search result UI).
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16697
Summary: Ref T11730. Removes unused code since this is now it's own page.
Test Plan: rebuild maps, grep for javelin code, classnames
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16700
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: Background is now always white, spacing in header is more consistent
Test Plan: test mobile, table, desktop application search apps.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16691
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 color needs to be fixed since we want the collapse icon over the menu.
Test Plan: Shrink window so area has to scroll, see collapse above the rest of the menu.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16674
Summary: Removes the special CSS to change the sidebar when the header is called. Also switched to using a more standard background color. I'd also like to make collapsed the default, but can't find best way to do that.
Test Plan: Vist profile, projects, collapse and open nav, try a workboard color. Still all good.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16673
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: Ref T11730. Removes the front end crop feature. Will follow up with proper removal, but this seems broken outright.
Test Plan:
Edit a room, don't seen "Crop" feature. Upload new photo, works fine.
- grep for `ConpherencePicCropControl`
- grep for `aphront-crop`
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11730
Differential Revision: https://secure.phabricator.com/D16665
Summary: This adds the room image to the main header in full Conpherence. It's nice, plus I plan to move the image edit workflow to it to simplify the move to EditEngine. I plan to build some default images for Conpherence which should be better about denoting the room, not just the last person writing.
Test Plan: Click on lots of rooms with and without topics.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16666
Summary: This probably stopped working when we switch to a standard nav view. Re-scope CSS. Also make scrolling to last room a little easier.
Test Plan: Make 20 rooms, see the old ones vanish. Click See More, scroll to bottom, click last room
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16656
Summary: Depending on when packages loaded, this CSS sometimes gets overwritten. Make it more specific and always present.
Test Plan: Reload a lot
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16657
Summary: Remove policy icons from durable column, create a basic nux layout and style.
Test Plan: leave all rooms, pop open chat, see helpful text and button.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16655
Summary: When the durable column is re-opened, scroll user to latest message. Though later we should scroll them to the last read.
Test Plan: Minimize window, reload page, pop up window, see proper scroll position.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16649
Summary: A bit better styling, this adds an indication icon for if you're connected or not (and later, away, etc).
Test Plan: Test in Notifications menu, Conpherence full, Durable Column.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16647
Summary: Sends and stores additional body classes at the page level. Removes old ones, sets new ones.
Test Plan: home -> application search -> colored workboard -> config -> home with persistent chat open and minimized.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16646
Summary: This exposes the chat window to a larger audience beside people who accidentaly hit `\`.
Test Plan:
Lots of clicks and reloads.
{F1856043}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16643
Summary: Creates a background that renders inside the Quicksand frame, through sorcery.
Test Plan: Turn on Quicksand, visit lots of pages. See correct background colors. This probably blows something up I'm not testing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16642
Summary: Lets chat sit under the footer, if present.
Test Plan: View new spacing with footer and minimized chat
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16640
Summary:
Since I plan to add collapsing, this widens the chat window and moves the switcher to the side, for more visual space for conversation.
TODO: make a magical minimizer so I can always have it open.
Test Plan:
Tested on my large display and little Macbook.
{F1854092}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16635
Summary: Adds a CSS class if comments come in from the same user in the past 2 minutes for cleaner UI. Note will have to find some better display UI when comment editing comes.
Test Plan: Test lots of random Conpherence messages with different transactions, different people, and quick commenting.
Reviewers: scp, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16632
Summary: This feels pretty reasonable with little effort, and I think I'd use it more than the full column.
Test Plan:
Chat a lot on various pages.... still some quicksand quirks around various pages.
{F1853487}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: scp, Korvin
Differential Revision: https://secure.phabricator.com/D16627
Summary: Fixes T11622. Moves the remarkup upload button into the text area on mobile/tablet.
Test Plan: Mobile/Tablet/Desktop Conpherence
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11622
Differential Revision: https://secure.phabricator.com/D16626
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: Thought I had added this, but maybe got reverted. Hides the status line on mobile.
Test Plan: Check tablet, desktop, and mobile. Don't see a status message.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16618
Summary: Fixes T11623. Enables send-on-enter and shift-enter for linebreaks, per durable column. Also cleaned up UI for Joining Room or Logging In.
Test Plan: See room I can join, click Join Room. Leave Room, Log out, visit room with login prompt. Login, Join Room again.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11623
Differential Revision: https://secure.phabricator.com/D16595
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:
Fixes T11703. This mostly avoids rounding errors.
If point values include "0.001", we also get three digits of precision: 1.000.
Maybe useful if your point field is called "bitcoins" or something.
Test Plan:
Before:
{F1851404}
After:
{F1851405}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11703
Differential Revision: https://secure.phabricator.com/D16601
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: Fixes T11676. Instead of trying to fit task titles to the display, truncate them and let the table scroll.
Test Plan:
Table now scrolls when cramped:
{F1843396}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11676
Differential Revision: https://secure.phabricator.com/D16583
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: Roughly, if user isn't in any rooms, search for joinable ones. If no results, show big NUX banner.
Test Plan: Left all rooms, got fallback, joined room, left room. Create new instance, see new NUX. Set instance to public, visit Conpherence with and without public rooms.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16563
Summary: Mostly quality of life in renames and moving everything to the view class. Minor CSS tweaks. Fix room handles getting added when adding a new user.
Test Plan: Add to room, remove from room.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16560
Summary: Darkens the topic, adds fun colors to action icons.
Test Plan: Hover over new action icons. See darker topic text.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16556
Summary: These three files are already in core. Don't re-package them.
Test Plan: reload
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16555
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: These floats were wonky in FF, adding some hardening to make sure they clear.
Test Plan:
Firefox, Chrome, IE.
{F1815981}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16547
Summary:
Fixes T11586. First pass at a class for displaying invisible characters. Still need to:
- Write a couple unit tests
- Add some styling to the .invisible-special spans
- Actually start using the class when displaying form errors to users
Currently this makes the string `"\nab\x00c\x01d\te\nf"` look like:
{F1812711}
Test Plan:
Unit tests all pass and run in <1ms:
{F1812998}
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley, yelirekim
Maniphest Tasks: T11586
Differential Revision: https://secure.phabricator.com/D16541
Summary: This adds a "column" icon into crumbs, like in workboards, for expanding or hiding the "Widget Pane". This is per user sticky and defaults to off.
Test Plan: View a Conpherence Room, see no widgets by default. Toggle it on, see widget. Reload page, see widget stick. Verify mobile, tablets ignore hiding.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10364
Differential Revision: https://secure.phabricator.com/D16533
Summary: This label is too wide when a !!wild scrollbar appears!!
Test Plan: shrink height of tall thread list in Conpherence, see no wrapping.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16536
Summary:
Moves search and new room out of crumb bar and into the thread column. This opens up space to add "Edit Room" into the crumbs area as an action link. Also removed 'widget-edit' pathways and javascript.
Fixes T8972
Test Plan:
Search, New, Edit rooms. Click different rooms, see correct URL each time.
{F1813226}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T8972
Differential Revision: https://secure.phabricator.com/D16543
Summary: This makes all favicons a background circle w/color and white gear, to be less confusing to an actual gear icon.
Test Plan:
review many different versions in sandbox.
{F1811707,size=full}
Reviewers: epriestley, avivey
Reviewed By: avivey
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16539
Summary: Swaps to new logo, new color for default favicon. Adds red, yellow, dark for other uses. perhaps.
Test Plan: reload, see new favicon
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16530
Summary: Cleans up a number of inconsistencies, makes Conpherence lighter, easier to read, make names larger, darker.
Test Plan: Review Conpherence chat in Main App, Durable Column, Mobile, Tablet
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16531
Summary: Fixes T10781. Re-uses sidenav styles from global space, though a little hacky. Fix crumb border, fix nux experience. Fiddle spacing.
Test Plan:
Use Conpherence as new user, make a room, post some messages.
{F1805658}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10781
Differential Revision: https://secure.phabricator.com/D16528
Summary: Fixes T10131. Adds new CSS to better present document pages for printing. Added a print link to Phriction.
Test Plan: Tested Phriction, Phame, and Legalpad.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10131
Differential Revision: https://secure.phabricator.com/D16524
Summary: If you don't have any rooms, we currently show no menu and users have no logical means of closing the column. This lets "Hide Column" at least still appear. Fixes T9195
Test Plan: Fresh install, hit {key \}, see menu. Close Column.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9195
Differential Revision: https://secure.phabricator.com/D16516
Summary: In new PHUITwoColumnLayout, some new colors were used that aren't in the High Contrast PostProcessor. Adding them and proper fallbacks.
Test Plan: High Contrast on/off on a timeline page.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16515
Summary: The spacing here is incorrect and no longer needed.
Test Plan: Test phone layout of project, profile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16511
Summary: Keyboard key CSS for Remarkup.
Test Plan:
edit test in html, see new CSS
{F1803550}
Reviewers: stratosgear, epriestley
Reviewed By: stratosgear, epriestley
Subscribers: stratosgear, Korvin
Differential Revision: https://secure.phabricator.com/D16506
Summary:
Ref T11589. When we hit a fatal setup issue (essentially always a connection failure) //after// we've already survived them on at least one request, we can be pretty sure a server went down and that the problem is not a setup/configuration issue.
In this case, show a friendlier error page instead of the fairly detailed technical one.
Test Plan:
- Broke MySQL config.
- Restarted Apache.
- Got the "admin/setup" error page:
{F1803268}
- Fixed the MySQL config.
- Loaded any page, to put us "in flight".
- Broke MySQL config.
- Loaded any page.
- Got the friendly "in flight" error page:
{F1803271}
If you want to design this better, easiest way to get to it is:
- Set `mysql.port` to `9999` in `conf/local/local.json`.
- Reload any page while already running (don't restart).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11589
Differential Revision: https://secure.phabricator.com/D16503
Summary: For phabricator. Adds a Slack auth adapater and icon.
Test Plan:
Create a new Slack Application for login, generate id and secret. Activate login and registration for Slack. Create a new account with Slack credentials. Log out. Log in with Slack credentials. Set my avatar with Slack. Slack. Slack.
{F1802649}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16496
Summary:
Two minor issues that I caught in the log while fixing Phame permissions:
- We had a JS bug which would cause us to immediately generate two comment previews at the exact same time -- one for loading the page, and one for "switching to desktop". Instead, only generate the "switch to desktop" preview if we really switched to desktop from a different device layout.
- These two requests could end up reading/writing the VersionedDraft table at exactly the same time fairly often (e.g., after a comment submission, the page would load, send two preview requests at exactly the same time, and they'd race fairly reliably for me locally). If we do race, recover from the race.
Test Plan:
Submitted some Phame comments.
- No more error log errors about VersionedDraft keys.
- Saw only one preview request when loading the page instead of two.
Here's the specific stack trace I caught:
```
[Mon Sep 05 12:15:33.639930 2016] [:error] [pid 50608] [client 127.0.0.1:55278] [2016-09-05 14:15:33] EXCEPTION: (AphrontDuplicateKeyQueryException) #1062: Duplicate entry 'PHID-POST-fknnpzjnsdgc3rqobhst-PHID-USER-pr5rjpuilpfserepsd2k-13' for key 'key_object' at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:314]
[Mon Sep 05 12:15:33.640801 2016] [:error] [pid 50608] [client 127.0.0.1:55278] arcanist(head=master, ref.master=9e82ef979e81), corgi(head=master, ref.master=5b9171222bc9), instances(head=stable, ref.master=485bc8128198, ref.stable=2983bc917601), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=phame2, ref.master=4b6da9735ba7, ref.phame2=4b6da9735ba7), phutil(head=stable, ref.master=97f05269fdb1, ref.stable=c14343ee620e), services(head=stable, ref.master=1fcb5cdb7582, ref.stable=2d8088a5b4b3)
[Mon Sep 05 12:15:33.640815 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #0 <#2> AphrontBaseMySQLDatabaseConnection::throwCommonException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:348]
[Mon Sep 05 12:15:33.640830 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #1 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryCodeException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:289]
[Mon Sep 05 12:15:33.640833 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #2 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryException(mysqli) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:185]
[Mon Sep 05 12:15:33.640836 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #3 <#2> AphrontBaseMySQLDatabaseConnection::executeRawQuery(string) called at [<phutil>/src/xsprintf/queryfx.php:8]
[Mon Sep 05 12:15:33.640839 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #4 <#2> queryfx(AphrontMySQLiDatabaseConnection, string, string, string, array, string)
[Mon Sep 05 12:15:33.640841 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #5 <#2> call_user_func_array(string, array) called at [<phutil>/src/aphront/storage/connection/AphrontDatabaseConnection.php:42]
[Mon Sep 05 12:15:33.640844 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #6 <#2> AphrontDatabaseConnection::query(string, string, string, array, string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1261]
[Mon Sep 05 12:15:33.640846 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #7 <#2> LiskDAO::insertRecordIntoDatabase(string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1106]
[Mon Sep 05 12:15:33.640849 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #8 <#2> LiskDAO::insert() called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1075]
[Mon Sep 05 12:15:33.640851 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #9 <#2> LiskDAO::save() called at [<phabricator>/src/applications/draft/storage/PhabricatorVersionedDraft.php:65]
[Mon Sep 05 12:15:33.640854 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #10 <#2> PhabricatorVersionedDraft::loadOrCreateDraft(string, string, integer) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1669]
[Mon Sep 05 12:15:33.640857 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #11 <#2> PhabricatorEditEngine::buildCommentResponse(PhamePost) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:894]
[Mon Sep 05 12:15:33.640859 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #12 <#2> PhabricatorEditEngine::buildResponse() called at [<phabricator>/src/applications/phame/controller/post/PhamePostEditController.php:60]
[Mon Sep 05 12:15:33.640862 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #13 <#2> PhamePostEditController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:237]
[Mon Sep 05 12:15:33.640865 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #14 phlog(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27]
[Mon Sep 05 12:15:33.640868 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #15 PhabricatorAjaxRequestExceptionHandler::handleRequestException(AphrontRequest, AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:644]
[Mon Sep 05 12:15:33.640870 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #16 AphrontApplicationConfiguration::handleException(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:242]
[Mon Sep 05 12:15:33.640873 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #17 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:149]
[Mon Sep 05 12:15:33.640879 2016] [:error] [pid 50608] [client 127.0.0.1:55278] #18 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:17]
```
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16494
Summary: Review multimeter logs for new CSS/JS resources that should be in packages.
Test Plan: Review some pages in sandbox, no CSS errors.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16490
Summary: Ref T11132, swaps in new UI for welcome page using guide modules
Test Plan: Test instance and non instance guides. Test each setting. Unclear on how to test people / Phacility. Just change the URL link?
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16482
Summary: This adds status icons, locked, hidden, editable, customized, to the list of options in config. Makes it easier to read and assertain state.
Test Plan:
View a hidden, customized, editable, and locked.
{F1796320}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16475
Summary: Previously we collapsed all table search results, but the new UI doesn't need it. Remove unused methods and fix CSS.
Test Plan: Legalpad Signatures, Phortune Accounts.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16469
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: Ref T11132, significantly cleans up the Config app, new layout, icons, spacing, etc. Some minor todos around re-designing "issues", mobile support, and maybe another pass at actual Group pages.
Test Plan: Visit and test every page in the config app, set new items, resolve setup issues, etc.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam, Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16468
Summary: Ref T11132. This gets rid of the red bar for admins and instead shows a new menu item next to notifications/chat if there are unresolved configuration issues. Menu goes away if there are no issues. May move this later into the bell icon, but think think might be the right place to start especially for NUX and updates. Maybe limit the number of items?
Test Plan:
Tested with some, lots, and no config issues.
{F1790156}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16461
Summary: I plan to reuse these styles with Config, maybe also Almanac, etc.
Test Plan: Review /guides/, see same styles.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16467
Summary: Really long text strings break this ui when first setting up Phabricator.
Test Plan: MySQL sql_mode setup issue.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16455
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: Previously, the chatbot docs instructed users to get certificates for the conduit API and put the cert in a `conduit.cert` config key. In order to get the chatbot to work, I needed to instead get an API key and put it in the `conduit.token` config entry.
Test Plan: Doc fix. Tried the new documented way and it worked.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D16443
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: Ref T11132. Will work on CSS tomorrow, but wanted to rough in the UI Steps to get guidance. Not sure what you have in mind for the "app" part, if you want to explain it and I build or you build.
Test Plan: Visit each page and click on links. Very rough and unfinished.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16419
Summary: Ref T11132. Adds a background color option to PHUIIconView, for use whereever, and NUX. Also normalize icon placement for mixed image/icon result list.
Test Plan: Test in UIExamples, and Global Settings.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16424
Summary: I don't think we use footicons, removing that CSS. States were added but only used in Auth, convert them to statusIcon instead.
Test Plan: Visit Auth, UIExamples, grep for `setState`
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16418
Summary: Fixes T11491, I believe these are the only two that need to be specified.
Test Plan: Checked Create pages for diffs, tasks, projects, events, blogs, login, etc. Mobile/Desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11491
Differential Revision: https://secure.phabricator.com/D16416
Summary: Ref T11132, Ref T11478. Builds out a basic PHUICMSView and Guides Application, no content / modules.
Test Plan: Go to /guides/, see blank states for new guides.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132, T11478
Differential Revision: https://secure.phabricator.com/D16414
Summary:
Fixes T11468. I was able to find //a// reproduction case for the issue, at least, described in my comments there.
When the server sends back position information for columns we aren't displaying, for whatever reason, just ignore that information.
Future work in T4900 to do more synchronization may revisit this.
Test Plan: See T11468#190177 for steps. Did that stuff with this patch, got a clean edit.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11468
Differential Revision: https://secure.phabricator.com/D16397
Summary: Fixes T11457, I guess. It feels wierd when you drag it a bit, and Im not positive if it's the calc css or javascript.
Test Plan: Enable filetree, shorten browser, see scrolling appear if files too long.
Reviewers: avivey, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11457
Differential Revision: https://secure.phabricator.com/D16393
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: When using spaces in the logo header, nowrap the text so it properly displays when too long on mobile, tablet.
Test Plan: Snowmen
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16380
Summary: Converts final call site to PHUIDocumentViewPro.
Test Plan: grep for PHUIDocumentView, view new Welcome Page
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16379
Summary:
This updates the eye logo and removes the formal wordmark "Phabricator" as an image. Instead we'll use the new updated eye logo and plain text for "Phabricator", both of which are more friendly and less industrial.
Installs that already use the `header-logo` customization setting will need to rebuild their logo to 80px x 80px. They will then also get to use plain text to whitebox their install as they see fit.
Test Plan:
Tested new logo at desktop, tablet, and mobile sizes. Set a random instance name, saw new wordmark. Created a really long wordmark of MMMMMMMMMMMM, saw text cut off so UI doesn't break. May need some additional tweaking, but I think we covered the most edge cases here.
{F1751791, size=full}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: edibiase, bjshively, yelirekim, Korvin
Maniphest Tasks: T4214, T11096
Differential Revision: https://secure.phabricator.com/D16373
Summary: Makes sidenav disappear again on projects/profiles, but shows it on home again (tablet views).
Test Plan: Visit Profile/Projects/Home on mobile, desktop, and tablet. See nav disappear correctly.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16369
Summary: Fixes T11416. Unclear what the side-effects of this would be, so bark if you find something. Previously, we'd have to overflow and scroll, which is kind of a pain since you're hiding content on long code blocks. This just wraps long lines, and preserves line breaks globally.
Test Plan: Test feed, profile, comments, inline comments, triple backticks.
Reviewers: avivey, epriestley
Reviewed By: avivey, epriestley
Subscribers: Korvin
Maniphest Tasks: T11416
Differential Revision: https://secure.phabricator.com/D16361
Summary: Adds a class for explicitly hiding the sidenav.
Test Plan: Set Config to Enable Filetree. View a diff, see tree. Press `f`, see it go away. Reload page, see persistence.
Reviewers: avivey, epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16359
Summary: This moves aphront-side-nav to use same table css display as profile nav. Slightly less code to support. Cleans up AppSearch UI, think I've gotten all the edge cases here, but bang on it, can hold until after release cut.
Test Plan: Config, Maniphest, Differential, Diffusion, Home.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16346
Summary:
Fixes T11386. Ref T4788.
- Apparently fix weird strikethrough effect? Spooky!
- Provide a little icon hint in the left column about which tasks are direct parents/children, vs just reachable somehow. I don't think this is super useful/important, but seems maybe nice?
Test Plan: {F1740779}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T11386
Differential Revision: https://secure.phabricator.com/D16342
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:
Ref T11326. This isn't perfect, but should be a little easier to use and less weird/confusing.
Generally, provide a "Query > Month > Day" crumb on day views, and a "Wed, July 3" header.
Generally, provide a "Query > Month" crumb on month views, and a "July 2019" header.
Also try to fix a bit of padding/spacing on the day view.
Test Plan: {F1739128}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16338
Summary:
Ref T11326. When viewing "February", add a class to dates in January and March to let them be styled a little differently as a UI hint.
For now, I've given them a grey background. (Calendar.app changes the date number color instead.)
Test Plan: {F1738990}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16336
Summary:
Ref T11326. This doesn't go quite as far as the mock in T11326#185932, but gets rid of the easy margins.
Also cleans up some of the border rules so they're simpler and more consistent (no weird ragged edges on the far right).
Test Plan: {F1738951}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16335
Summary:
Ref T11326. Currently, we link Calendar days using hidden DOM nodes.
This is nice because it's simple, and right-clicking a day works properly. However, it's a bit ugly/unintuitive, messy, and unclear. It's especially messy because days are really two different rows, one for events and one for day/week numbers.
Instead, use JS to highlight day cells. You can still right-click by clicking the actual day number, which seems like a reasonable compromise.
Test Plan: {F1738941}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16334
Summary: When we have an info view in a column, the css isn't specific enough to override the core info-view css.
Test Plan: Review an importing repository, see info view properly spaced.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16331
Summary: This padding is a little off / custom. Normalizes it to the form on mobile.
Test Plan: Review some settings forms, save form changes.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16330
Summary: Mobile forms are super tight, this opens them up a little bit.
Test Plan: Review editing a document, task, mobile, tablet.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16329
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. This just cleans things up a little and removes some of the obvious layout/CSS issues.
Test Plan:
- Viewed day view before/after. Also viewed profile panel.
Before:
{F1725547}
After:
{F1725548}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16308
Summary:
Ref T11326. This just inches things forward a little bit:
- Make it easier to see current day.
- Line-through cancelled events.
- Don't colorize the whole event title, just use an Attending/Invited/Custom icon.
- Slightly subtler treatment for all-day events.
Test Plan: See screenshot in T11326.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11326
Differential Revision: https://secure.phabricator.com/D16306
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: Fixes T9202.
Test Plan:
- Viewed day in 12-hour, saw "8:00 PM".
- Viewed day in 24-hour, saw "16:00".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9202, T10932
Differential Revision: https://secure.phabricator.com/D16290
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
Summary: Fixes T11305, Ref T7754. Makes this menu dropdown act like actions and collapse to a fa-bars menu.
Test Plan:
View on mobile, desktop, browser. Click an action, spawn new page.
{F1717953}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T7754, T11305
Differential Revision: https://secure.phabricator.com/D16265
Summary: fix T11290.
Test Plan: Paste language type, view in web and in emails (It uses quotes in HTML emails, which I think is something else).
Reviewers: epriestley, chad, #blessed_reviewers
Reviewed By: chad, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T11290
Differential Revision: https://secure.phabricator.com/D16252
Summary: Ref T10252. This is similar to D16259, but makes KeyboardShortcutManager more relaxed about `altKey` when typing obscure characters.
Test Plan: Pressed Option + Shift + 7 on a German keyboard layout, saw Conphernece sidebar toggle.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10252
Differential Revision: https://secure.phabricator.com/D16260
Summary:
Ref T10252. On the German keyboard layout, you must type "Alt" + "L" to generate an "@" character.
We currently ignore this event, assuming it's a keyboard command. However, I think we can safely continue so that autocomplete works on German layouts.
Test Plan:
- Switched keyboard layout to German.
- Typed Alt + L to generate an "@".
- Typed some username text.
- Got autocompleter.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10252
Differential Revision: https://secure.phabricator.com/D16259
Summary: Ref T9360. These weren't getting set properly, also make them nullable since they're optional.
Test Plan: run upgrade, make a new blog with and without a parent domain. Edit a current blog.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D16242
Summary:
Ref T9360. Moves PhamePost to CommentEditEngine.
[x] HTTP Parameters dropdown on New Post goes to 404
[x] Implement EditEngine Comments
Test Plan: Make Post, Make Comment, Laugh.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D16222
Summary: Ref T11244. 8 more tokens. Probably need better math on the selector?
Test Plan: Award Dat Boi.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: putnam, Korvin
Maniphest Tasks: T11244
Differential Revision: https://secure.phabricator.com/D16228
Summary: Ref T4788. It's not easy to tell at a glance which objects are open vs closed. Try to make that a bit more clear. This could probably use some more tweaking.
Test Plan: {F1708330}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16219
Summary:
Ref T4788. This fixes all the bugs I was immediately able to catch:
- "Directory-Like" graph shapes could draw too many vertical lines.
- "Reverse-Directory-Like" graph shapes could draw too few vertical lines.
- Terminated, branched graph shapes drew the very last line to the wrong place.
This covers the behavior with tests, so we should be able to fix more stuff later without breaking anything.
Test Plan:
- Added failing tests and made them pass.
{F1708158}
{F1708159}
{F1708160}
{F1708161}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16216
Summary: New tokens, slightly larger (18x18 vs 16x16). I think these all feel decent, I might tweak the thumbs icons a little more color-wise.
Test Plan:
Use Tokens.
{F1707411}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11244
Differential Revision: https://secure.phabricator.com/D16211
Summary: Ref T10628. Cleans up remaining weird, unused tab behaviors in ObjectBoxView to simplify ObjectBox.
Test Plan: Toggled tabs in Files.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10628
Differential Revision: https://secure.phabricator.com/D16208
Summary:
Ref T4788. When closing a task as a duplicate of another task, you can only select one task, since it doesn't really make sense to merge one task into several other tasks (this operation is //possible//, but probably not what anyone ever wants to do, I think?).
Make the UI understand this: after you select a task, disable all of the "select" buttons in the UI to make this clear.
Test Plan:
- Used "Close as Duplicate", only allowed to select 1 task.
- Used other editors like "Merge Duplicates In", allowed to select lots of tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16203
Summary: Fixes T11236, breaks long words and inlines the spans.
Test Plan:
Use a diff with super long text like SSH keys, set Font to 24/48 Impact
{F1705910}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11236
Differential Revision: https://secure.phabricator.com/D16198
Summary:
Fixes T11225. The primary issue here is that this rule is bleeding down too far. It appears that it's only intended to put space around the inline as a whole.
Spacing still isn't //perfect// since a few other rules are bleeding, but it feels reasonable now instead of being clearly broken.
Test Plan:
- Added "background: red;" to figure out what was being affected.
- Before:
{F1704081}
- After:
{F1704084}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11225
Differential Revision: https://secure.phabricator.com/D16184
Summary:
Ref T9838.
Add a Properties field to Revision, and update a `wasAcceptedBeforeClose` when closing a revision.
Test Plan:
A quick run through the obvious steps (Close with commit/manually, with or w/o accept) and calling `differential.query` shows the `wasAcceptedBeforeClose` property was setup correctly.
Pushing closed + accepted passes the relevant herald, which was my immediate issue; Pushing un-accepted is blocked.
Test the "commit" rule (Different from "pre-commit") by hacking the DB and running the "has accepted revision" rule in a test-console.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T9838
Differential Revision: https://secure.phabricator.com/D15085
Summary: We haven't refreshed this in a while.
Test Plan: Saw unit test times drop about 1.5 seconds locally.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D16176
Summary: Ref T9897. This moves "Domain" to "DomainFullURI" to allow setting of https or for some reason, a port. I guess.
Test Plan: Try to break by setting a path, or fake protocol. Set to http, or https, see correct redirects. Verify domain still gets written.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D16173
Summary: Fixes T11209. We want to always break tags when displaying them in a list, but not in general (remarkup).
Test Plan: Fake a tag on a differental revision with a really long name. See wrapping.
Reviewers: avivey, epriestley
Reviewed By: avivey, epriestley
Subscribers: avivey, Korvin
Maniphest Tasks: T11209
Differential Revision: https://secure.phabricator.com/D16175
Summary: Fixes T11166. Adds some class, and space to the preview widget.
Test Plan: Test Maniphest, Ponder, etc, without a footer.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11166
Differential Revision: https://secure.phabricator.com/D16168
Summary:
Ref T11179. Ref T4768. Currently, on `master`, if two users open "Edit Revisions" at the same time, then add revisions A and B, only the last state wins (just "B").
Instead, apply these as "add A" and "add B" so they merge in a natural way.
Test Plan:
- Opened edit dialog in two windows.
- Added "A" in one, "B" in the other.
- Saved both.
- Saw "Added A" and "Added B" transactions, instead of "Added A" and "Removed A, added B".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4768, T11179
Differential Revision: https://secure.phabricator.com/D16164
Summary: Just removing the animation for now, can't find anything decent, I think that the issue is the animation is applying to all items in the list, and not just as a list as a single block. That is, I'd like to slide down all three at one. Any animation that slides them down when attached to each item makes them overlap at the first frame and it's a little distracting. Not a big deal to leave this out for now. Whatever we come up with should likely be applied to phuix-dropdown as well.
Test Plan: Clicky Clicky.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16159
Summary:
Ref T11179. Alternative to D16152. I think this turned out a bit better than the other one did.
Currently, we render two copies of the menu (one for mobile, one for desktop). A big chunk of this is sharing the nodes instead: when you open the mobile dropdown menu, it steals the nodes from the document. When you close it, it puts them back. Magic! Sneaky!
Test Plan:
{F1695499}
{F1695500}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11179
Differential Revision: https://secure.phabricator.com/D16157
Summary: Ref T11034. Ref T4788. This allows you to resize the typeahead browse dialog if you want. I plan to let you resize the object selector dialog in the future.
Test Plan: {F1695433}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788, T11034
Differential Revision: https://secure.phabricator.com/D16156
Summary:
Ref T11034. This seems a little more promising. Two problems at the moment:
- This doesn't actually provide any useful information at all right now.
- Many object types have no profile images.
Test Plan:
{F1695254}
{F1695255}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11034
Differential Revision: https://secure.phabricator.com/D16155
Summary: Ref T9897. Adds a Parent Site and Parent Domain field to allow external sites to link back to parent.
Test Plan: Set up ```local.blog.phacility.com```, set parent site to "Phacility" and parent domain to "local.www.phacility.com". Get new crumbs at Blog and Post levels.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D16150
Summary: Makes the crumbs background and border disappear in the live view of Phame.
Test Plan: Go live, see no crumb bg. Test blog, post, mobile, desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16146
Summary: Adds a new header layout for Phame Blog. Subtitles now also.
Test Plan:
With Image, With Subtitle, Without Image, Without Subtitle. Mobile, Tablet, Desktop.
{F1691506}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16147
Summary:
Ref T11153. If you have a build plan like this:
- Lease machine A.
- Lease machine B.
- Run client-tests on machine A.
- Run server-tests on machine B.
...and we get machine A quickly, then finish the tests, we currently do not release machine A until the whole plan finishes.
In the best case, this wastes resources (something else could be using that machine for a while).
In a worse case, this wastes a lot of resources (if machine B is slow to acquire, or the server tests are much slower than the client tests, machine A will get tied up for a really long time).
In the absolute worst case, this might deadlock things.
Instead, release artifacts as soon as no waiting/running steps take them as inputs. In this case, we'd release machine A as soon as we finished running the client tests.
In the case where machines A and B are resources of the same type, this should prevent deadlocks. In all cases, this should improve build throughput at least somewhat.
Test Plan:
I wrote this build plan which runs a "fast" step (10 seconds) and a "slow" step (120 seconds):
{F1691190}
Before the patch, running this build plan held the lease on the "fast" machine for the full 120 seconds, then released both leases at the same time at the very end.
After this patch, I ran this plan and observed the "fast" lease get released after 10 seconds, while the "slow" lease was held for the full 120.
(Also added some `var_dump()` into things to sanity check the logic; it appeared correct.)
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11153
Differential Revision: https://secure.phabricator.com/D16145
Summary: This should get locked to 100% of viewport.
Test Plan: Really wide image on a mobile screen.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16144
Summary: Fixes T10901. Allows blogs to have headers. I've built this in a basic way, any file, max-height is 240. Should bleed into top crumbs, so any spacing you want you should add to the file itself. Might have to see how users break this.
Test Plan: Set a blog header, see blog header, remove blog header, see no blog header. Check mobile, tablet, desktop break points.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10901
Differential Revision: https://secure.phabricator.com/D16141
Summary: This is the backend half of uploading an image as a header for Phame Blogs. Allows you to upload image, or delete it. Ref T10901
Test Plan:
Go to Manage Blog, visit Edit Header Image, Upload snarky file. See snarky file on Manage page. Edit Header Image, click delete, save, see file goes away.
{F1690966}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10901
Differential Revision: https://secure.phabricator.com/D16140
Summary:
Ref T9028. This allows us to detect when commits are unreachable:
- When a ref (tag, branch, etc) is moved or deleted, store the old thing it pointed at in a list.
- After discovery, go through the list and check if all the stuff on it is still reachable.
- If something isn't, try to follow its ancestors back until we find something that is reachable.
- Then, mark everything we found as unreachable.
- Finally, rebuild the repository summary table to correct the commit count.
Test Plan:
- Deleted a ref, ran `pull` + `refs`, saw oldref in database.
- Ran `discover`, saw it process the oldref, mark the unreachable commit, and update the summary table.
- Visited commit page, saw it properly marked.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9028
Differential Revision: https://secure.phabricator.com/D16133
Summary:
Ref T9789. `Transaction` and `Editor` classes are the last major pieces of infrastructure that haven't been fully modularized.
Some of the specific issues are:
- `Editor` classes rely on a bunch of `instanceof` stuff in the base class to pick up transaction types like "subscribe", "projects", etc. Instead, applications should be adding these, and third-party applications should be able to add them.
- Code is spread across `Transaction` and `Editor` classes somewhat oddly. For example, generating old/new values would probably make more sense at the `Transaction` level, but it currently exists at the `Editor` level.
- Both types of classes have a lot of functions based on `switch()` statements, which require a ton of boilerplate and are just generally kind of hard to work with.
This creates classes for each type of transaction, and moves almost all of the logic to them. These classes are simpler and more focused than the old stuff was, and can organize related code better.
This starts inching toward defining `CoreTransactions` for features shared across applications. It only defines the "Create" transaction so far, but at some point I plan to move all the other shared transactions to Core and let them control which objects they're available for.
Test Plan:
- Created pastes with web UI and API.
- Edited all paste properites.
- Archived/activated.
- Verified files got reasonable names.
- Reviewed timeline and feed.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9789
Differential Revision: https://secure.phabricator.com/D16111
Summary:
Ref T7643. When a large block of prose text is edited (like a wiki page), summarize the diff when sending mail.
For now, I'm still showing the whole thing in the web UI, since it's a bit more manageable there.
Also try to fix newlines in Airmail.
Test Plan:
This web diff:
{F1682591}
..became this mail diff:
{F1682592}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7643
Differential Revision: https://secure.phabricator.com/D16098
Summary:
Ref T8510. Sort prefix matches above non-prefix matches, so that "Ape Discovery" does not match "discovery" better than "Discovery".
Sort functions last.
Rename function internal strings so they don't get over-promoted the prefix-match rules.
Add kind of a hack to get "Project X" sorting above all the "Project X (Milestone 1)" results.
Test Plan:
Created "Ape Discovery", "Baboon Discovery", "Chimpanzee Discovery", etc.
Main project now sorts above milestones:
{F1681773}
Prefix matches now sort above other matches:
{F1681774}
Function results (rarely used) are now less prominent:
{F1681775}
Better function results here:
{F1681776}
More function results:
{F1681777}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8510
Differential Revision: https://secure.phabricator.com/D16094
Summary: Ref T6523. Allows you to click stuff instead of using drag-and-drop.
Test Plan: On iOS simulator, created and updated a mock.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6523
Differential Revision: https://secure.phabricator.com/D16088
Summary:
Ref T6916. Current rules tend to make videos gigantic. Just embed them at actual size, scaling them down if they're too big to fit.
Browsers generally provide some kind of "expand / fullscreen" element automatically anyway.
Test Plan: Viewed videos locally, saw them sized a little more reasonably.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6916
Differential Revision: https://secure.phabricator.com/D16076
Summary:
Fixes T10402.
I tried about 50 variations on the wording and notification layout, this seemed by far the most reasonable.
Didn't implement a way to ignore the warning, which might be required - but figured this is serious and broken enough while being completely invisible 99% of the time that it's worth shouting about.
Test Plan: Messed around with $_SERVER['HTTPS'] on the server side and client_uri on the client side - saw reasonable results in all combinations.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Maniphest Tasks: T10402
Differential Revision: https://secure.phabricator.com/D16064
Summary: Just adds a little more space to the quick create menu.
Test Plan: Test stock and modded quick create menu.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16057
Summary:
Ref T3353. This hooks the prose engine up to the UI and throws away the hard-wrapping hacks.
These are likely still very rough in many cases, but are hopefully a big step forward from the old version in the vast majority of cases.
Test Plan: {F1677809}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T3353
Differential Revision: https://secure.phabricator.com/D16056
Summary:
Ref T4103. This just adds a single global default setting group, not full profiles.
Primarily, I'm not sure how administrators are supposed to set profiles for users, since most ways user accounts get created don't really support setting roles.. When we figure that out, it should be reasonably easy to extend this. There also isn't much of a need for this now, since pretty much everyone just wants to turn off mail.
Test Plan:
- Edited personal settings.
- Edited global settings.
- Edited a bot's settings.
- Tried to edit some other user's settings.
- Saw defaults change appropriately as I edited global and personal settings.
{F1677266}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16048
Summary:
Ref T4103. Ref T10078. This moves profile image caches to new usercache infrastructure.
These dirty automatically based on configuration and User properties, so add some stuff to make that happen.
This reduces the number of queries issued on every page by 1.
Test Plan: Browsed around, changed profile image, viewed as self, viewed as another user, verified no more query to pull this information on every page
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103, T10078
Differential Revision: https://secure.phabricator.com/D16040
Summary:
Ref T4103. This isn't completely perfect but should let us move forward without also expanding scope into "too much mail".
I split the existing "Mail Preferences" into two panels: a "Mail Delivery" panel for the EditEngine settings, and a "2000000 dropdowns" panel for the two million dropdowns. This one retains the old code more or less unmodified.
Test Plan:
- Ran unit tests, which cover most of this stuff.
- Grepped for all removed constants.
- Ran migrations, inspected database results.
- Changed settings in both modified panels.
- This covers a lot of ground, but anything I missed will hopefully be fairly obvious.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16038
Summary:
Ref T4103. These settings long-predate proper settings and are based on hard-coded user properties. Turn them into real settings.
(I didn't try to migrate the value since they're trivial to restore and only useful to developers.)
Test Plan:
- Toggled console on/off.
- Swapped tabs.
- Reloaded page, everything stayed sticky.
Reviewers: chad
Reviewed By: chad
Subscribers: eadler
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16029
Summary:
Ref T11076. Ref T9897. Bad links on Phame blogs are currently made worse because we try to prompt you to login on a non-cookie domain.
Instead, just 404 in a vanilla way. Do so cleanly on external domains.
Test Plan: {F1672399}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897, T11076
Differential Revision: https://secure.phabricator.com/D16010
Summary:
Ref T4103. These are currently stored on the user, for historic/performance reasons.
Since I want administrators to be able to set defaults for translations and timezones at a minimum and there's no longer a meaningful performance penalty for moving them off the user record, turn them into real preferences and then nuke the columns.
Test Plan:
- Set settings to unusual values.
- Ran migrations.
- Verified my unusual settings survived.
- Created a new user.
- Edited all settings with old and new UIs.
- Reconciled client/server timezone disagreement.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16005
Summary:
Ref T4103. This doesn't get everything, but takes care of most of the easy stuff.
The tricky-ish bit here is that I need to move timezones, pronouns and translations to proper settings. I expect to pursue that next.
Test Plan:
- Grepped for `loadPreferences` to identify callsites.
- Changed start-of-week setting, loaded Calendar, saw correct start.
- Visited welcome page, read "Adjust Settings" point.
- Loaded Conpherence -- I changed behavior here slightly (switching threads drops the title glyph) but it wasn't consistent to start with and this seems like a good thing to push to the next version of Conpherence.
- Enabled Filetree, toggled in Differential.
- Disabled Filetree, no longer visible in Differential.
- Changed "Unified Diffs" preference to "Small Screens" vs "Always".
- Toggled filetree in Diffusion.
- Edited a task, saw sensible projects in policy dropdown.
- Viewed user profile, uncollapsed/collapsed side nav, reloaded page, sticky'd.
- Toggled "monospaced textareas", used a comment box, got appropriate fonts.
- Toggled durable column.
- Disabled title glyphs.
- Changed monospaced font to 18px/36px impact.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16004
Summary:
Ref T4103. Currently, we issue a `SELECT * FROM user_preferences ... WHERE userPHID = ...` on every page to load the viewer's settings.
There are several other questionable data accesses on every page too, most of which could benefit from improved caching strategies (see T4103#178122).
This query will soon get more expensive, since it may need to load several objects (e.g., the user's settings and their "role profile" settings). Although we could put that data on the User and do both in one query, it's nicer to put it on the Preferences object ("This inherits from profile X") which means we need to do several queries.
Rather than paying a greater price, we can cheat this stuff into the existing query where we load the user's session by providing a user cache table and doing some JOIN magic. This lets us issue one query and try to get cache hits on a bunch of caches cheaply (well, we'll be in trouble at the MySQL JOIN limit of 61 tables, but have some headroom).
For now, just get it working:
- Add the table.
- Try to get user settings "for free" when we load the session.
- If we miss, fill user settings into the cache on-demand.
- We only use this in one place (DarkConsole) for now. I'll use it more widely in the next diff.
Test Plan:
- Loaded page as logged-in user.
- Loaded page as logged-out user.
- Examined session query to see cache joins.
- Changed settings, saw database cache fill.
- Toggled DarkConsole on and off.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16001
Summary: Nip/Tuck.
Test Plan: With and without calendar events.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16003
Summary: Possible side effect of fixing other info views yesterday. Removes bottom margin on empty member boxes.
Test Plan: Review various projects with and without members.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16002
Summary: Because new+old stack, these colors were darker than intended. Lightening them up a little bit.
Test Plan: Review a diff with new + old code and addtion subtractions.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15994
Summary:
Ref T4103. This give preferences a PHID, policy/transaction interfaces, a transaction table, and a Query class.
This doesn't actually change how they're edited, yet.
Test Plan:
- Ran migrations.
- Inspected database for date created, date modified, PHIDs.
- Changed some of my preferences.
- Deleted a user's preferences, verified they reset properly.
- Set some preferences as a new user, got a new row.
- Destroyed a user, verified their preferences were destroyed.
- Sent Conpherence messages.
- Send mail.
- Tried to edit another user's settings.
- Tried to edit a bot's settings as a non-admin.
- Edited a bot's settings as an admin (technically, none of the editable settings are actually stored in the settings table, currently).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D15991
Summary: When you save settings or don't have reviewers, we show an info box inside an object box. This rule is squashing the margin. Blame seems to indicate I added it for Differential updates, but in re-visiting Differential, I'm unable to trigger any broken CSS with this change. But keep an eye out?
Test Plan: Badges, Settings, Differential
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15992
Summary:
Ref T4103. This removes these options:
{F1660585}
The jump nav option came from T916, when we had a separate jump nav on the home page. Essentially no one has ever been confused by the behavior of search or disabled this feature. Here are the stats for this install:
| Total Users | 36656 |
| Have Set Any Preference | 3084 |
| Have Disabled Jump | 6
| Are Not "Security Researchers" | 2
| Any Account Activity | 0
The "/" option came in the same change, but the preference came from T989. This keystroke conflicts with a default Firefox keystroke. Almost no one cares about this either, but I count 6 real users who have disabled the behavior. I suspect the number of real users who //use// it may be smaller.
In Safari and Firefox, the "tab" key does the same thing.
In Chrome, the "tab" key does the same thing if {nav Preferences > Web Content > "Pressing Tab highlights..."} is disabled.
Upshot: jump nav is great, bulk of the change in T989 was clearly great, specific preferences that came out of it seem not-so-great and now is a good time to kill them as we head into T4103.
Test Plan:
- Grepped for removed constants.
- Pressed "/".
- Searched for `T123`.
- Viewed settings.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D15976
Summary: Ref T9606
Test Plan: Open people profile for a user with events today/tomorrow, see a panel under badges panel with event list
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9606
Differential Revision: https://secure.phabricator.com/D15851
Summary:
Ref T3025. Chrome gives us an easily-accessible, much better guess at which timezone the user is in.
Firefox also exposes "Intl" but this doesn't seem to be a reliable method to read the timezone.
Test Plan:
In Chrome, swapped my system date/time between zones, clicked the "reconcile" popup, got the dropdown prefilled accurately.
In Safari (no `Intl` API) got the normal flow with no default selected.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T3025
Differential Revision: https://secure.phabricator.com/D15962
Summary: Ref T3025. This adds a check for different client/server timezone offsets and gives users an option to fix them or ignore them.
Test Plan:
- Fiddled with timezone in Settings and System Preferences.
- Got appropriate prompts and behavior after simulating various trips to and from exotic locales.
In particular, this slightly tricky case seems to work correctly:
- Travel to NY.
- Ignore discrepancy (you're only there for a couple hours for an important meeting, and returning to SF on a later flight).
- Return to SF for a few days.
- Travel back to NY.
- You should be prompted again, since you left the timezone after you ignored the discrepancy.
{F1654528}
{F1654529}
{F1654530}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T3025
Differential Revision: https://secure.phabricator.com/D15961
Summary: Bumping this up higher since two column views get extra tight fast below 900 px. This felt most correct to me, dialing it back from first attempt at 960. Mostly I don't want to ever accidentally trigger it when I'm on the 12" MacBook. Ref T10926
Test Plan: Durable Column, Workboards, Dashboards, Tasks.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: avivey, Korvin
Maniphest Tasks: T10926
Differential Revision: https://secure.phabricator.com/D15960
Summary: Makes the background transparent for uploaded thumbs. This page in general needs lots of work, but here's the minimum. Fixes T10986
Test Plan: Edit a Mock with a transparent jeff.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10986
Differential Revision: https://secure.phabricator.com/D15957
Summary: Couple of edge cases here I never cleaned up. This inlines points and projects better, with spacing and use of grey to better differentate from project tag colors.
Test Plan:
Review edge cases on workboard with multiple short and long project names.
{F1653998}
{F1653999}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15956
Summary:
Ref T10917. Converts web UI edits to transactions.
This is about 95% "the right way", and then I cheated on the last 5% instead of building a real EditEngine. We don't need it for anything else right now and some of the dialog workflows here are a little weird so I'm just planning to skip it for the moment unless it ends up being easier to do after the next phase (mail notifications) or something like that.
Test Plan: {F1652160}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10917
Differential Revision: https://secure.phabricator.com/D15947
Summary:
Ref T10917. Currently, when you delete an SSH key, we really truly delete it forever.
This isn't very consistent with other applications, but we built this stuff a long time ago before we were as rigorous about retaining data and making it auditable.
In partiular, destroying data isn't good for auditing after security issues, since it means we can't show you logs of any changes an attacker might have made to your keys.
To prepare to improve this, stop destoying data. This will allow later changes to become transaction-oriented and show normal transaction logs.
The tricky part here is that we have a `UNIQUE KEY` on the public key part of the key.
Instead, I changed this to `UNIQUE (key, isActive)`, where `isActive` is a nullable boolean column. This works because MySQL does not enforce "unique" if part of the key is `NULL`.
So you can't have two rows with `("A", 1)`, but you can have as many rows as you want with `("A", null)`. This lets us keep the "each key may only be active for one user/object" rule without requiring us to delete any data.
Test Plan:
- Ran schema changes.
- Viewed public keys.
- Tried to add a duplicate key, got rejected (already associated with another object).
- Deleted SSH key.
- Verified that the key was no longer actually deleted from the database, just marked inactive (in future changes, I'll update the UI to be more clear about this).
- Uploaded a new copy of the same public key, worked fine (no duplicate key rejection).
- Tried to upload yet another copy, got rejected.
- Generated a new keypair.
- Tried to upload a duplicate to an Almanac device, got rejected.
- Generated a new pair for a device.
- Trusted a device key.
- Untrusted a device key.
- "Deleted" a device key.
- Tried to trust a deleted device key, got "inactive" message.
- Ran `bin/ssh-auth`, got good output with unique keys.
- Ran `cat ~/.ssh/id_rsa.pub | ./bin/ssh-auth-key`, got good output with one key.
- Used `auth.querypublickeys` Conduit method to query keys, got good active keys.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10917
Differential Revision: https://secure.phabricator.com/D15943
Summary: Ref T10939. This adds UI, transactions, etc, to adjust dominion rules.
Test Plan:
- Read documentation.
- Changed dominion rules.
- Created packages on `/` ("A") and `/x` ("B") with "Auto Review: Review".
- Touched `/x`.
- Verified that A and B were added with strong dominion.
- Verified that only B was added when A was set to weak dominion.
- Viewed file in Diffusion, saw correct ownership with strong/weak dominion rules.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10939
Differential Revision: https://secure.phabricator.com/D15936
Summary: Fixes T10975. The "scramble attached file permissions when an object is saved" code is misfiring here too. See T10778 + D15803 for prior work.
Test Plan:
- Ran `bin/storage upgrade -f`.
- Edited the view policy of an OAuth server (prepatch: fatal; postpatch: worked great).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10975
Differential Revision: https://secure.phabricator.com/D15938
Summary: Fixes T10959. This is the smallest/simplest fix that I could come up with, and I wasn't able to break it. Basically, I removed "line-height" and then adjusted other rules until the defaults looked reasonable again.
Test Plan:
Here's `24px / 48px impact` or something like it:
{F1310445}
{F1310446}
Here's normal stuff working properly without weird artifacts on the highlighting:
{F1310447}
Also tested Firefox and Chrome and got similar results.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: wxm20073527, Korvin
Maniphest Tasks: T10959
Differential Revision: https://secure.phabricator.com/D15905
Summary:
Ref T10939. Ref T8887. This moves toward letting packages automatically become reviewers or blocking reviewers of owned code.
This change adds an "Auto Review" option to packages. Because adding reviewers/blocking reviewers is a little tricky, it doesn't actually have these options yet -- just a "subscribe" option. I'll do the reviewer work in the next update.
Test Plan:
Created a revision in a package with "Auto Review: Subscribe to Changes". The package got subscribed.
{F1311677}
{F1311678}
{F1311679}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8887, T10939
Differential Revision: https://secure.phabricator.com/D15915
Summary:
Ref T10923. This makes the "Clone URI" UI a little nicer:
- Show whether each URI is read-only, read-write, or external.
- Clicking the button selects the URI.
- Add a link to manage the appropriate credentials.
Test Plan: {F1308302, size=full}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15891
Summary:
Ref T10923. When regenerating the URI index for a repository, index every URI.
- Also, make the index slightly stricter (domain + path instead of just path). Excluding the domain made more sense when we were generating only first-party URIs.
- Make the index smarter about `/diffusion/123/` URIs.
- Show normalized URIs in `diffusion.repository.search` results.
Test Plan:
- Ran migration.
- Verified sensible-looking results in database.
- Searched for a repository URI by first-party clone URI.
- Searched for a repository URI by mirror URI.
- Used `diffusion.repository.search` to get information about repository URIs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10923
Differential Revision: https://secure.phabricator.com/D15876
Summary:
Ref T9790. This prepares the syntax color rules to be reused in mail.
This goes about halfway toward T5701 by sort-of supporting different styles but not really.
Test Plan:
- Ran `bin/celerity syntax` to regenerate syntax map.
- Viewed some highlighted code, didn't see any differences.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9790
Differential Revision: https://secure.phabricator.com/D15846
Summary:
Ref T10748. This needs more extensive testing and is sure to have some rough edges, but seems to basically work so far.
Throwing this up so I can work through it more deliberately and make notes.
Test Plan:
- Ran migration.
- Used `bin/repository list` to list existing repositories.
- Used `bin/repository update <repository>` to update various repositories.
- Updated a migrated, hosted Git repository.
- Updated a migrated, observed Git repository.
- Converted an observed repository into a hosted repository by toggling the I/O mode of the URI.
- Conveted a hosted repository into an observed repository by toggling it back.
- Created and activated a new empty hosted Git repository.
- Created and activated an observed Git repository.
- Updated a mirrored repository.
- Cloned and pushed over HTTP.
- Tried to HTTP push a read-only repository.
- Cloned and pushed over SSH.
- Tried to SSH push a read-only repository.
- Updated several Mercurial repositories.
- Updated several Subversion repositories.
- Created and edited repositories via the API.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15842
Summary:
Ref T10748. This migrates and swaps mirroring to `PhabricatorRepositoryURI`, obsoleting `PhabricatorRepositoryMirror`.
This prevents you from editing, adding or disabling mirrors unless you know a secret URI (until the UI cuts over fully), but existing mirroring is not affected.
Test Plan:
- Added a mirroring URI to an old repository.
- Verified it worked with `bin/repository mirror`.
- Migrated forward.
- Verified it still worked with `bin/repository mirror`.
- Wow, mirroring: https://github.com/epriestley/locktopia-mirror
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15841
Summary:
Ref T4039. Long ago these were more freely editable and there were some security concerns around creating a repository, then setting its local path to point somewhere it shouldn't.
Local paths are no longer editable so there's no real reason we need to provide a uniqueness guarantee anymore, but you could still make a mistake with `bin/repository move-paths` by accident, and it's a little cleaner to pull them out into their own column with a key.
(We still don't -- and, largely can't -- guarantee that two paths aren't //equivalent// since one might be symlinked to the other, or symlinked only on some hosts, or whatever, but the primary value here is as a sanity check that you aren't goofing things up and pointing a bunch of repositories at the same working copy by mistake.)
Test Plan:
- Ran migrations.
- Grepped for `local-path`.
- Listed and moved paths with `bin/repository`.
- Created a new repository, verified its local path populated correctly.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4039
Differential Revision: https://secure.phabricator.com/D15837
Summary: Seems to work ok, if you give `size=wide` to an image file, we blow it out a bit in DocumentPro mode.
Test Plan:
Test in Phame and Maniphest.
{F1256717}
{F1256718}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15840
Summary: Feels a little hairy, but should be fine overall. Ups the css specificity enough that it's always displayed the same. The main problem is headers and boxes get put everywhere, and sometimes override each other. Fixes T10757
Test Plan: review a countdown in a phame post and phriction document. Check diviner, phriction for regressions.
Reviewers: epriestley, #phacility
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10757
Differential Revision: https://secure.phabricator.com/D15664
Summary:
Ref T10748. These:
- Look nice.
- Hint at panel contents / effects.
- Hint which panels have been customized.
- Allow panels with issues or errors to be highlighted with an alert/attention icon.
Test Plan: {F1256156}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10748
Differential Revision: https://secure.phabricator.com/D15836
Summary: Fixes T10906, Fixes T10820. Adds new icons, grey-er colors for previous states. Also, I think fixed a few bugs?
Test Plan: Fake each state, verify icon is as intended.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10820, T10906
Differential Revision: https://secure.phabricator.com/D15830
Summary:
Fixes T10905. This reverts D15823, which didn't work well for tasks with very long titles (the title would break as a block element).
This is slightly more magic but works with long titles.
Test Plan: Did everything from D15823, but also with long titles. Triple-click, wrapping, and mobile/device worked in Safari, Firefox and Chrome.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10905
Differential Revision: https://secure.phabricator.com/D15824
Summary:
Fixes T10905. In Firefox, triple clicking the new headers doesn't select the entire line, so you can't easily copy/paste an entire task title or revision name. It works fine in Safari/Chrome.
This seems to fix that without breaking anything.
Test Plan:
- Viewed headers in Safari, Firefox, Chrome.
- Triple-clicked headers in Safari, Firefox, Chrome.
- Viewed tablet/device layouts in Safari, Firefox, Chrome.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10905
Differential Revision: https://secure.phabricator.com/D15823
Summary:
Ref T10748. Ref T10366. This adds a new EditEngine, EditController, Editor, Query, and Transaction for RepositoryURIs.
None of these really do anything helpful yet, and these URIs are still unused in the actual application.
Test Plan: {F1249794}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10366, T10748
Differential Revision: https://secure.phabricator.com/D15815
Summary:
We're sometimes getting duplicate notifications right now. I think this is because multiple windows are racing and becoming leaders.
Clean this up a little:
- Fix the `timeoout` typo.
- Only try to usurp once.
- Use different usurp and expire delays, so we don't fire them at the exact same time.
Not sure if this'll work or not but it should theoretically be a little cleaner.
Test Plan:
- Quit Safari, reopened Safari, still saw a fast reconnect to the notification server (this is the goal of usurping).
- Did normal notification stuff like opening a chat in two windows, got notifications.
- Hard to reproduce the race for sure, but this at least fixes the outright `timeoout` bug.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15806
Summary:
Fixes T10778. This is a result of T10262: when we save a form configuration and adjust the policy, we try to scramble attached file secrets.
There aren't going to be any attached files, but there's also no edge table, so we fail.
We could skip this code, but we'll likely need an edge table here sooner or later so it's probably simpler in the long run to just add an empty one.
Test Plan:
- Ran `bin/storage upgrade`, got a clean bill of health.
- Saved a form configuration after making a policy edit, no more `edge` exception.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10778
Differential Revision: https://secure.phabricator.com/D15803
Summary:
Ref T10866. Fixes T10386. This attempts to make it a little more plausible to follow these directions:
- Use simpler language in general.
- Remove language suggesting that HTTP requires no additional configuration.
- Suggest using a load balancer or an ugly port number instead of swapping SSH to a different port.
- Be more granular about `sudo` setup.
- Organize better?
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10386, T10866
Differential Revision: https://secure.phabricator.com/D15796
Summary:
Ref T10860. This allows us to recover if the connection to the database is lost during a push.
If we lose the connection to the master database during a push, we would previously freeze the repository. This is very safe, but not very operator-friendly since you have to go manually unfreeze it.
We don't need to be quite this aggressive about freezing things. The repository state is still consistent after we've "upgraded" the lock by setting `isWriting = 1`, so we're actually fine even if we lost the global lock.
Instead of just freezing the repository immediately, sit there in a loop waiting for the master to come back up for a few minutes. If it recovers, we can release the lock and everything will be OK again.
Basically, the changes are:
- If we can't release the lock at first, sit in a loop trying really hard to release it for a while.
- Add a unique lock identifier so we can be certain we're only releasing //our// lock no matter what else is going on.
- Do the version reads on the same connection holding the lock, so we can be sure we haven't lost the lock before we do that read.
Test Plan:
- Added a `sleep(10)` after accepting the write but before releasing the lock so I could run `mysqld stop` and force this issue to occur.
- Pushed like this:
```
$ echo D >> record && git commit -am D && git push
[master 707ecc3] D
1 file changed, 1 insertion(+)
# Push received by "local001.phacility.net", forwarding to cluster host.
# Waiting up to 120 second(s) for a cluster write lock...
# Acquired write lock immediately.
# Waiting up to 120 second(s) for a cluster read lock on "local001.phacility.net"...
# Acquired read lock immediately.
# Device "local001.phacility.net" is already a cluster leader and does not need to be synchronized.
# Ready to receive on cluster host "local001.phacility.net".
Counting objects: 3, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 254 bytes | 0 bytes/s, done.
Total 3 (delta 1), reused 0 (delta 0)
BEGIN SLEEP
```
- Here, I stopped `mysqld` from the CLI in another terminal window.
```
END SLEEP
# CRITICAL. Failed to release cluster write lock!
# The connection to the master database was lost while receiving the write.
# This process will spend 300 more second(s) attempting to recover, then give up.
```
- Here, I started `mysqld` again.
```
# RECOVERED. Link to master database was restored.
# Released cluster write lock.
To ssh://local@localvault.phacility.com/diffusion/26/locktopia.git
2cbf87c..707ecc3 master -> master
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10860
Differential Revision: https://secure.phabricator.com/D15792
Summary:
Ref T8952. Currently, when an application (most commonly Herald, but sometimes Drydock, Diffusion, etc) publishes a feed story, we get an empty grey box for it in feed.
Instead, give the story a little application icon kind of "profile picture"-like thing.
Test Plan:
Here's how it looks:
{F1239003}
Feel free to tweak/counter-diff.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8952
Differential Revision: https://secure.phabricator.com/D15773
Summary: Fixes T9295
Test Plan: Create event, open datepicker for start date, choose 1/31/2016, open datepicker again, click right button to scroll month. New suggested date should be 2/29/2016
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9295
Differential Revision: https://secure.phabricator.com/D15727
Summary:
Fixes T10840. When rendering mail, this rule wasn't falling through in quite the right way.
Also adjust where the rules are for this so the special styles show up in Maniphest, etc.
Test Plan:
Made this comment:
{F1238266}
Which produced this HTML:
{F1238267}
...and sent this mail:
{F1238283}
Reviewers: hach-que, chad
Reviewed By: chad
Maniphest Tasks: T10840
Differential Revision: https://secure.phabricator.com/D15767
Summary: Ref T4292. When we write a push log, also log which node received the request.
Test Plan: {F1230467}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15759
Summary: Ref T4292. This will let the UI and future `bin/repository` tools give administrators more tools to understand problems when reporting or resolving them.
Test Plan:
- Pushed fully clean repository.
- Pushed previously-pushed repository.
- Forced write to abort, inspected useful information in the database.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4292
Differential Revision: https://secure.phabricator.com/D15748
Summary:
Fixes T10830.
- The return code from `storage adjust` did not propagate correct.
- There was one column issue which I missed the first time around because I had a bunch of unrelated stuff locally.
Test Plan:
- Ran `bin/storage upgrade -f` with failures, used `echo $?` to make sure it exited nonzero.
- Got fully clean `bin/storage adjust` by dropping all my extra local tables.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10830
Differential Revision: https://secure.phabricator.com/D15746
Summary:
Fixes T10830. Ref T10366. I wasn't writing to this table yet so I didn't build it, but the fact that `bin/storage adjust` would complain slipped my mind.
- Add the table.
- Make the tests run `adjust`. This is a little slow (a few extra seconds) but we could eventually move some steps like this to run server-side only.
Test Plan: Ran `bin/storage upgrade -f`, got a clean `adjust`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10366, T10830
Differential Revision: https://secure.phabricator.com/D15744
Summary: Ref T7691 (errata). This shows links to Phriction documents in red if they're missing, and links to Phriction documents in grey with a lock icon if the user doesn't have the correct permissions to see the document.
Test Plan:
Tested a bunch of different configurations:
```
[[ ./../ ]] Back to Main Document
[[ ./../subdocument_2]] Mmmm more documents
[[ ./../invisible_document]] Mmmm more documents
[[ ./../ | Explicit Title ]] Back to Main Document
[[ ./../subdocument_2 | Explicit Title ]] Mmmm more documents
[[ ./../invisible_document | Explicit Title ]] Mmmm more documents
[[ ]] Absolute link
[[ subdocument_2 ]] Absolute link
[[ invisible_document ]] Absolute link
[[ | Explicit Title ]] Absolute link
[[ subdocument_2 | Explicit Title ]] Absolute link
[[ invisible_document | Explicit Title ]] Absolute link
```
Got the expected result:
{F1221106}
Reviewers: epriestley, chad, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T7691
Differential Revision: https://secure.phabricator.com/D15733
Summary: Fixes T9296
Test Plan: Create an event, change start time to `3PM`, end value should update to `4:00 PM`, not `4:0 PM`
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9296
Differential Revision: https://secure.phabricator.com/D15725
