1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-21 04:50:55 +01:00
Commit graph

2429 commits

Author SHA1 Message Date
Chad Little
922822bd2d Wrap really long text properly in diffs
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
2016-06-29 21:21:04 -07:00
epriestley
84ce863116 Select <td> padding for inlines more surgically
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
2016-06-28 14:50:51 -07:00
Aviv Eyal
de6349dd67 Revision substate CLOSED_FROM_ACCEPTED
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
2016-06-27 20:29:47 +00:00
epriestley
e984f0eb76 Update quickstart.sql
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
2016-06-24 14:49:03 -07:00
Chad Little
af5001db64 Allow PhameBlog to take a full URI instead of just a domain name
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
2016-06-24 14:11:52 -07:00
Chad Little
fd20b89b56 Break white-space on tags in any property list
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
2016-06-24 13:42:53 -07:00
Chad Little
3bb3170a2c Add space under comment previews
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
2016-06-22 12:18:33 -07:00
epriestley
4bbe6f307a Resolve relationship edit conflicts more naturally
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
2016-06-22 11:17:30 -07:00
Chad Little
ee77c5c8f1 Clean up menu folding in action list
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
2016-06-21 14:39:55 -07:00
epriestley
56d3197fe0 Fold task-relationship actions into an accordion dropdown
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
2016-06-20 19:14:27 -07:00
epriestley
3198aa1659 Allow dialogs to opt in to being resizable
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
2016-06-20 19:13:58 -07:00
epriestley
6f275ba144 Render browse results with global result style
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
2016-06-20 16:49:02 -07:00
Chad Little
967945e4b4 Add ability to link back to parent site in external phame blogs
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
2016-06-20 12:50:27 -07:00
Chad Little
431ca4aac3 Reduce crumbs UI in Phame live views
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
2016-06-18 07:54:05 -07:00
Chad Little
20affe9ce8 Add new super hero header for Phame Blog
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
2016-06-18 14:21:39 +00:00
epriestley
96c51028e5 In Harbormaster, release artifacts as soon as no waiting/running build steps will use them
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
2016-06-17 16:13:56 -07:00
Chad Little
b9615a701b Fix width on Phame blog header image on mobile
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
2016-06-17 13:19:54 -07:00
Chad Little
35bb320cfb Bump Phame blog header up to 320px height.
Summary: More corgi space.

Test Plan: Upload Corgi, see larger Corgi.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16143
2016-06-17 12:53:29 -07:00
Chad Little
e384e945d0 Add basic support for Phame blog headers
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
2016-06-17 12:08:51 -07:00
Chad Little
0d4902a0e5 Add ability to set a header image per Phame blog
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
2016-06-17 11:23:21 -07:00
epriestley
1c63ac6a3a When a ref is moved or deleted, put it on a list; later, check for reachability
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
2016-06-16 11:21:38 -07:00
epriestley
33ec855449 Modularize application transactions in Paste, mostly
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
2016-06-14 06:13:28 -07:00
Chad Little
63d413e201 Don't truncate blogs or posts in Phame sidebar
Summary: Removes the CSS truncation, so it's easier to track drafts. Ref T9897

Test Plan: Really long draft titles.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16107
2016-06-13 11:21:40 -07:00
epriestley
fb156af480 Render prose diffs in email as summaries
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
2016-06-10 09:39:58 -07:00
epriestley
ad0562e15e Improve some typeahead matching behaviors
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
2016-06-09 13:22:34 -07:00
epriestley
4d12c58dcf Allow Pholio mocks to be created and edited without drag-and-drop
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
2016-06-09 08:43:38 -07:00
Chad Little
78fab485b4 Break-word on all propertly list values
Summary: Fixes T10827. Adds break-word on all values.

Test Plan: Really long text.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10827

Differential Revision: https://secure.phabricator.com/D16081
2016-06-08 10:09:41 -07:00
Chad Little
3453507230 Add print styles for Phortune carts
Summary: Fixes T10899, adds styles specifically for printing Phortune carts.

Test Plan: Print a Phortune cart

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T10899

Differential Revision: https://secure.phabricator.com/D16079
2016-06-08 07:52:18 -07:00
epriestley
9469e3d6ac Embed videos in Remarkup at actual size
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
2016-06-07 15:00:26 -07:00
epriestley
e85b169590 Rebuild Celerity map. 2016-06-07 13:46:34 -07:00
Asher Baker
f0eb6f4fe0 Add client-side check for protocol mismatch
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
2016-06-07 15:54:48 +00:00
epriestley
d1999557dc Crunchy celery. 2016-06-06 14:52:55 -07:00
Chad Little
511f703657 Tidy up quick create menu spacing
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
2016-06-06 20:57:58 +00:00
epriestley
b3477bfc56 Render sometimes-legible prose diffs in the Phabricator UI
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
2016-06-06 12:33:42 -07:00
epriestley
421bf2e548 Allow administrators to configure global default settings
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
2016-06-05 13:15:06 -07:00
epriestley
6f1053c206 Convert user profile images into a standard cache
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
2016-06-05 08:52:15 -07:00
epriestley
7ef6c0a523 Modularize all the mail preferences
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
2016-06-05 08:50:07 -07:00
epriestley
109917a94b Turn DarkConsole settings into real settings
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
2016-06-04 14:43:43 -07:00
epriestley
03e54afc14 Give Phame blogs an explicit 404 controller
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
2016-06-02 09:12:21 -07:00
epriestley
ebd8f3c987 Make translation, timezone and pronoun into real settings
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
2016-06-02 06:29:47 -07:00
epriestley
edfc6a6934 Convert some loadPreferences() to getUserSetting()
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
2016-06-02 06:29:20 -07:00
epriestley
9180f429eb Provide a general-purpose, modular user cache for settings and other similar data
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
2016-06-02 06:28:56 -07:00
Chad Little
8b7f8cb61f Clean up Calendar sidebar on profiles
Summary: Nip/Tuck.

Test Plan: With and without calendar events.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D16003
2016-06-01 13:52:50 -07:00
Chad Little
0b1e8d0296 Fix margin on blank members boxes on projects
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
2016-06-01 13:24:47 -07:00
Chad Little
2dc4096ea1 Lighten up red and green in diffs
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
2016-05-31 20:25:28 +00:00
epriestley
b256f2d7b2 Prepare UserPreferences for transactions
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
2016-05-31 12:28:44 -07:00
Chad Little
e64dfbcbd8 Fix top margin on infoview + two-column
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
2016-05-31 12:07:53 -07:00
epriestley
a4e5780043 Remove "Search Preferences"
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
2016-05-26 06:21:47 -07:00
lkassianik
de645301b5 Adding a calendar preview panel to people profile
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
2016-05-23 10:39:25 -07:00
epriestley
2a00f185eb When the JS "Intl" API is available, use it to guess the timezone
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
2016-05-22 09:14:02 -07:00
epriestley
8b9df5e90f Update Celerity map. 2016-05-21 13:27:56 -07:00
epriestley
a91004ef1b Detect timezone discrepancies and prompt users to reconcile them
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
2016-05-21 13:25:23 -07:00
Chad Little
e902fc0e2a Move Tablet breakpoint from 768 -> 920
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
2016-05-21 12:11:27 -07:00
epriestley
ed92d1d844 Regenerate the Celerity map.
Auditors: chad
2016-05-20 16:26:11 -07:00
Chad Little
804a5db41a Add transparency to Pholio Edit thumbs
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
2016-05-20 13:43:22 -07:00
Chad Little
969d3be035 Clean up "points" display on workboards
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
2016-05-20 13:06:57 -07:00
epriestley
da6b3de65c Use transactions to apply web UI SSH key edits
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
2016-05-19 15:00:18 -07:00
epriestley
0308d580d7 Deactivate SSH keys instead of destroying them completely
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
2016-05-18 14:54:28 -07:00
epriestley
809c7bf996 Allow users to manage package dominion rules
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
2016-05-17 10:57:43 -07:00
epriestley
875b866715 Add missing "oauth_server_edge" tables
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
2016-05-17 08:50:27 -07:00
Chad Little
03a1deba23 Remove hard-coding of diff line height
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
2016-05-16 10:21:37 -07:00
epriestley
52ac242eb3 Implement "Auto Review" in packages with a "Subscribe" option
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
2016-05-13 17:21:58 -07:00
epriestley
b21b43131c Clean up display of clone URIs a little bit
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
2016-05-11 13:14:55 -07:00
epriestley
576b73dc53 Index all repository URIs, not just the "primary" repository URI
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
2016-05-11 06:36:06 -07:00
epriestley
01289f3f48 Generate syntax highlighting CSS from a reusable map
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
2016-05-05 02:50:48 -07:00
epriestley
29d1115037 Swap Repository Edit UI to new code
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
2016-05-04 16:19:57 -07:00
epriestley
42eaa88f80 Cut mirroring over to new URIs
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
2016-05-04 16:16:16 -07:00
epriestley
dd2b10b8f8 Guarantee repositories have unique local paths
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
2016-05-04 16:09:52 -07:00
Chad Little
d85386488b Add "wide" remarkup image support for Documents
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
2016-05-03 17:27:30 -07:00
Chad Little
db065e6758 Increase specificity in blue property boxes
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
2016-05-03 17:26:31 -07:00
epriestley
c3afddec9c Add icons to the new repository edit nav
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
2016-05-03 08:01:18 -07:00
Chad Little
45ca0472a9 Spiffy up feed icon defaults
Summary: Uses standard background color of profile icons.

Test Plan: Review herald, commit stories.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15831
2016-05-01 13:09:27 -07:00
Chad Little
562d427f09 New icons, colors for per reviewer status
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
2016-05-01 12:52:40 -07:00
epriestley
e8dca5a4f9 Alternate fix for Firefox triple click selection
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
2016-04-29 16:59:43 -07:00
epriestley
06abeb2010 Allow tall two-column/curtain headers to be triple-clicked in Firefox
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
2016-04-29 16:43:27 -07:00
epriestley
616c9ae887 Rough sketch of new repository URI editing
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
2016-04-29 09:21:00 -07:00
epriestley
58b55c2fa6 Probably improve behavior around duplicate notifications
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
2016-04-27 03:56:55 -07:00
epriestley
467c4e84e5 Add an edge table to the search database
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
2016-04-26 11:26:26 -07:00
epriestley
8d9bc401e4 Improve Diffusion hosting setup instructions somewhat?
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
2016-04-26 10:06:59 -07:00
epriestley
892a9a1f07 Make cluster repositories more resistant to freezing
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
2016-04-25 11:37:31 -07:00
epriestley
df8c3c4fa5 Give application actors in feed reasonable icons
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
2016-04-20 14:38:55 -07:00
lkassianik
eeccaf99b6 When scrolling forward a month in calendar date picker from 1/31, next chosen date should be 2/29, not 3/1.
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
2016-04-20 09:15:45 -07:00
epriestley
11f8fffe5b Fix Phriction document linking in mail bodies
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
2016-04-20 06:55:00 -07:00
epriestley
6edf181a7e Record which cluster host received a push
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
2016-04-19 13:06:30 -07:00
epriestley
f424f9f2d2 Record more details about where a write is taking place while holding a cluster lock
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
2016-04-18 11:55:27 -07:00
epriestley
595f203816 Correct RepositoryURI schema and propagate adjust exit code correctly
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
2016-04-18 08:11:22 -07:00
epriestley
9352ed8abb Add missing RepositoryURI table + run storage adjustments in tests
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
2016-04-18 07:54:02 -07:00
June Rhodes
27227b8010 Show missing Phriction documents as red links, invisible documents with a lock
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
2016-04-17 01:49:54 +00:00
lkassianik
42a8776228 Time controls should format end date value correctly (not 5:0 PM)
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
2016-04-15 12:18:12 -07:00
lkassianik
899856a1d4 Links on badge card should be accessible
Summary: Ref T10710

Test Plan: Open user profile with badge, flip badge card, open awarder link.

Reviewers: epriestley, chad, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: chad, Korvin

Maniphest Tasks: T10710

Differential Revision: https://secure.phabricator.com/D15699
2016-04-15 07:55:31 -07:00
epriestley
0534002894 Add coverage tooltips in Diffusion file browse mode
Summary: Fixes T10816. The way these work is a little unusual since these chunks of file-rendering code are unusuall performance-sensitive, so the Differential version doesn't adapt directly to Diffusion. Both can possibly be unified at some point in the future, although they do slightly different things.

Test Plan: {F1220170}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10816

Differential Revision: https://secure.phabricator.com/D15719
2016-04-15 06:59:38 -07:00
epriestley
2930733ac9 Complete modernization of Aphlict configuration
Summary:
Fixes T10697. This finishes bringing the rest of the config up to cluster power levels.

Phabricator is now given an arbitrarily long list of notification servers.

Each Aphlict server is given an arbitrarily long list of ports to run services on.

Users are free to make them meet in the middle by proxying whatever they want to whatever else they want.

This should also accommodate clustering fairly easily in the future.

Also rewrote the status UI and changed a million other things. 🐗

Test Plan:
{F1217864}

{F1217865}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15703
2016-04-14 04:57:00 -07:00
epriestley
4244cad990 Move toward multi-master replicated repositories
Summary:
Ref T4292. This mostly implements the locking/versioning logic for multi-master repositories. It is only active on Git SSH pathways, and doesn't actually do anything useful yet: it just does bookkeeping so far.

When we read (e.g., `git fetch`) the logic goes like this:

- Get the read lock (unique to device + repository).
  - Read all the versions of the repository on every other device.
  - If any node has a newer version:
    - Fetch the newer version.
    - Increment our version to be the same as the version we fetched.
- Release the read lock.
- Actually do the fetch.

This makes sure that any time you do a read, you always read the most recently acknowledged write. You may have to wait for an internal fetch to happen (this isn't actually implemented yet) but the operation will always work like you expect it to.

When we write (e.g., `git push`) the logic goes like this:

- Get the write lock (unique to the repository).
  - Do all the read steps so we're up to date.
  - Mark a write pending.
    - Do the actual write.
  - Bump our version and mark our write finished.
- Release the write lock.

This allows you to write to any replica. Again, you might have to wait for a fetch first, but everything will work like you expect.

There's one notable failure mode here: if the network connection between the repository node and the database fails during the write, the write lock might be released even though a write is ongoing.

The "isWriting" column protects against that, by staying locked if we lose our connection to the database. This will currently "freeze" the repository (prevent any new writes) until an administrator can sort things out, since it'd dangerous to continue doing writes (we may lose data).

(Since we won't actually acknowledge the write, I think, we could probably smooth this out a bit and make it self-healing //most// of the time: basically, have the broken node rewind itself by updating from another good node. But that's a little more complex.)

Test Plan:
  - Pushed changes to a cluster-mode repository.
  - Viewed web interface, saw "writing" flag and version changes.
  - Pulled changes.
  - Faked various failures, got sensible states.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4292

Differential Revision: https://secure.phabricator.com/D15688
2016-04-12 08:57:57 -07:00
Chad Little
3f9e8e675b Tidy up project panel errors
Summary: Makes the fallback a little cleaner on long titles. Wow we have a lot of error states here.

Test Plan: New Milestone with no tasks.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15686
2016-04-11 23:13:18 +00:00
Chad Little
75ba6058b9 Update to FontAwesome 4.6.0
Summary: Updates to the latest icon packages

Test Plan: View UIExamples, find new Icons

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15684
2016-04-11 11:34:59 -07:00
epriestley
071741c61d When Phabricator is in read-only mode, explain why
Summary:
Ref T4571. Allows users to click the "read-only mode" notification to get more information about why an install is in read-only mode.

Installs can be in this mode for several reasons (explicit administrative action, no masters defined, no masters reachable), and it's useful to be able to tell the difference.

Test Plan: {F1212930}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4571

Differential Revision: https://secure.phabricator.com/D15671
2016-04-10 12:19:18 -07:00
epriestley
3f51b78539 Lay cluster.databases configuration groundwork for database clustering
Summary:
Ref T4571. This adds a new option which allows you to upgrade your one-host configuration to a multi-host configuration by configuring it.

Doing this currently does nothing. I wrote a lot of words about what it is //supposed// to do in the future, though.

Test Plan:
  - Tried to configure the option in all the possible bad ways, got errors.
  - Read documentation.

Reviewers: chad

Reviewed By: chad

Subscribers: eadler

Maniphest Tasks: T4571

Differential Revision: https://secure.phabricator.com/D15663
2016-04-09 13:41:16 -07:00
epriestley
49d93dcf98 Add a cluster.read-only option
Summary:
Ref T4571. There will be a very long path beyond this, but add a basic read-only mode. You can explicitly enable this to put Phabricator in a sort of "maintenance" mode today if you're swapping databases or something.

In the long term, we'll automatically degrade into this mode if the master database is down.

Test Plan:
  - Enabled read-only mode.
  - Browsed around.
  - Didn't immediately see anything that was totally 100% broken.

Most stuff is 80-90% broken right now. For example:

  - Stuff like submitting comments doesn't work, and gives you a confusing, unhelpful error.
  - None of the UI really knows that it's read-only. EditEngine stuff should all hide itself and say "you can't add new comments while an install is in read-only mode", for example, but currently does not.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4571

Differential Revision: https://secure.phabricator.com/D15662
2016-04-09 13:40:47 -07:00
Chad Little
57e606b395 Update Settings/Config UI
Summary: Testing out a new 'nav' layout in Settings / Config. Spent a few days here and couldn't find much better overall.

Test Plan: View each page in Settings and in Config. Save some config options. Test mobile, desktop, tablet.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15659
2016-04-08 22:00:38 +00:00
lkassianik
1f423c3bd1 Make badges searchable by name
Summary: Closes T10690

Test Plan: Open Badges application, go to Advanced Search, search for a badge by its name and see result.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin

Maniphest Tasks: T10690

Differential Revision: https://secure.phabricator.com/D15656
2016-04-07 12:25:11 -07:00