Summary: Fixes T6693.
Test Plan:
Made a bunch of comments on a diff with differential, being sure to leave inlines here and there. This reproduced the issue in T6693. With this patch this issue no longer reproduces!
Successfully "showed older changes" in Maniphest too.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6693
Differential Revision: https://secure.phabricator.com/D10931
Summary: Updates header to use font-icons instead of images.
Test Plan: Test desktop and mobile layouts, Chrome, FF, Safari, IE.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10930
Summary: Ref T4712. This adds pagination. Future diffs will need to deploy `buildTransactionTimeline` everywhere and massage this stuff as necessary if we hit any special cases.
Test Plan: Set page size to "5" to make it need to paginate often. Verified proper transactions loaded in and the javascript actions worked.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4712
Differential Revision: https://secure.phabricator.com/D10887
Summary: Converts PHUIObjectItemView to use display: table rows and columns for more flexible layouts. Slightly increases spacing, improves mobile layouts. Fixes T5502
Test Plan: Tested in multiple applications and UIExamples. Ran through mobile, tablet, and desktop break points. Used IE8-IE10, Firefox, Chrome, Safari on both Mac and Windows.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5502
Differential Revision: https://secure.phabricator.com/D10917
Summary: Fixes T6657. Some other rule got stronger and started overriding this one, I think. Removes bullets from checkbox lists.
Test Plan: Looked at a checkbox list.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T6657
Differential Revision: https://secure.phabricator.com/D10912
Summary: Fixes positioning issues by creating another container to hold the abs. positioned arrows. (Issues primarily presented on Workboards).
Test Plan: Test UI arrows on a workboard, applciation launcher, and in UIExamples.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10897
Summary:
Ref T6615. Mixing ASC and DESC ordering on a multipart key makes it dramatically less effective (or perhaps totally ineffective).
Reverse the meaning of the `priority` column so it goes in the same direction as the `id` column (both ascending, lower values execute sooner).
Test Plan:
- Queued 1.2M tasks with `bin/worker flood`.
- Processed ~1 task/second with `bin/phd debug taskmaster` before patch.
- Applied patch, took ~5 seconds for ~1.2M rows.
- Processed ~100-200 tasks/second with `bin/phd debug taskmaster` after patch.
- "Next in Queue" query on daemon page dropped from 1.5s to <1ms.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: aklapper, 20after4, epriestley
Maniphest Tasks: T6615
Differential Revision: https://secure.phabricator.com/D10895
Summary: Make captions feel less floaty, align with their input element.
Test Plan:
Visit a few forms with captions, test mobile and desktop layouts
{F236871}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10891
Summary:
Ref T6240. Some discussion in that task. In instance/cluster environments, daemons need to make Conduit calls that bypass policy checks.
We can't just let anyone add SSH keys with this capability to the web directly, because then an adminstrator could just add a key they own and start signing requests with it, bypassing policy checks.
Add a `bin/almanac trust-key --id <x>` workflow for trusting keys. Only trusted keys can sign requests.
Test Plan:
- Generated a user key.
- Generated a device key.
- Trusted a device key.
- Untrusted a device key.
- Hit the various errors on trust/untrust.
- Tried to edit a trusted key.
{F236010}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6240
Differential Revision: https://secure.phabricator.com/D10878
Summary: For actions like "Close" that are in theory stopping the timeline, we should display some disruption to the line itself.
Test Plan:
Tested in UIExamples
{F236077}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10884
Summary: Fixes T3189. Now if you say #projects in a commit message they will associate nicely with the commit. Also we record transactions about all this project editing fun.
Test Plan: tested migration by associating some projects with commits and verifying they still showed up post migration. tested adding / removing projects by doing so from the UI, noting transactions written nicely as well
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Projects: #projects
Maniphest Tasks: T3189
Differential Revision: https://secure.phabricator.com/D10877
Summary: Fixes T6152, T6237. This introduces a viewPolicy column to the DifferentialDiff, and re-jiggers the DifferentialDiff policy implementation such that things behave as before once associated with a revision, else use the DifferentialDiff policy column value.
Test Plan: made a diff with a non-standard view policy and noted that policy was still selected in the revision step. arc lint.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6237, T6152
Differential Revision: https://secure.phabricator.com/D10875
Summary: Ref T6237. This sets us up for some future work like T6152, T6200 and generally cleaning up this workflow a bit. Tried to do as little as possible so not exposing transaction view yet. (Though that timeline is going to be a little funky in the common case of just the lone create transaction.)
Test Plan: made a diff from web ui and it worked. made a herald rule to block certain diffs then tried to make such a diff and saw UI letting me know i was blocked
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6237
Differential Revision: https://secure.phabricator.com/D10869
Summary: Fixes T6102. Give "priority" treatment to strings that are exact matches.
Test Plan:
made a bunch of projects with the word project in them including "Project". before patch, "Project" wouldn't even show up if I typed "Project" - now its the second result right after the application "Projects".
{F235120}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6102
Differential Revision: https://secure.phabricator.com/D10867
Summary: Fixes T4234 (Policy and Herald). I will have an additional diff for Maniphest Batch Editor, which looks jank in many other ways.
Test Plan:
Test editing a policy and some herald rules. Stuff lines up better.
{F235086}
{F235087}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4234
Differential Revision: https://secure.phabricator.com/D10864
Summary: Fixes T1768. This is mostly a data cleanliness issue as duplicate rows don't really do anything, but let's clear it up now.
Test Plan: made some duplicate rows by adding the same auditor multiple times. ran ./bin/storage upgrade and it worked perfectly!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T1768
Differential Revision: https://secure.phabricator.com/D10849
Summary: Right now, if no comment is selected the JS executes and throws an exception. Instead, if nothing is selected just do nothing. Fixes T6107.
Test Plan: opened up a commit in diffusion with an inline comment. pressed 'r' and saw no exceptions and nothing happen. pressed 'n' to select the next inline comment and then 'r' and it worked. opened up a commit in diffusion without any inline comments. pressed 'r' and saw no exceptions and nothing happen. opened up a diff in differential with an inline comment. pressed 'r' and saw no exceptons and nothing happened. pressed 'n' to select the next inline comment and then 'r' and it worked.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6107
Differential Revision: https://secure.phabricator.com/D10843
Summary: See <https://github.com/phacility/phabricator/issues/760>. We removed these methods in D10832 but still need the migration to be able to do project checks.
Test Plan: Ran on a test wiki with `/`, `/projects/` and `/projects/example/`. The first two pages didn't try to use project policies; the third one did.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10836
We have at least one project with `null` as a viewPolicy. This should get
sorted out separately, but make the migration robust against it.
Auditors: btrahan
Summary: Ref T4029. this diff makes the pertinent database changes AND adds the migration script. This is important to get the data backend straightened away before we fully ship T4029. Next diff will expose the edit controls for these policies and whatever else work is needed to get that part done right.
Test Plan: made sure the lone project page on my wiki had a project with restrictive view policy. Post migration verified correct policy applied to this lone project page AND most open policy applied to the others
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4029
Differential Revision: https://secure.phabricator.com/D10814
Summary:
Ref T5833. This fixes a few weird things with this table:
- A bunch of columns were nullable for no reason.
- We stored an MD5 hash of the key (unusual) but never used it and callers were responsible for manually populating it.
- We didn't perform known-key-text lookups by using an index.
Test Plan:
- Ran migrations.
- Faked duplicate keys, saw them clean up correctly.
- Added new keys.
- Generated new keys.
- Used `bin/auth-ssh` and `bin/auth-ssh-key`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10805
Summary: Missed this in previous pass. Send these as links in HTML emails.
Test Plan: Register a new user that nees approval.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10815
Summary: Ref T1191. A couple of installs have hit issues with this table, so clean it up before adjustment adds a unique key to it.
Test Plan: Dropped key, added duplicate rows, ran patch, got cleanup, ran adjust to get the key back.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10799
Summary:
Ref T1191. Use `storage quickstart` to regenerate `quickstart.sql` using modern schema construction statements.
This puts new installs into utf8mb4 mode immediately without requiring storage adjustment.
Test Plan:
- Ran `arc unit --everything`, which uses quickstart.
- Ran `bin/storage upgrade --namespace temp`, to quickstart a new namespace.
- Ran `bin/storage upgrade --namespace temp --disable-utf8mb4`, to quickstart a new namespace without utf8mb4 support.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10797
Summary:
Fixes T6487. Ref T1191. Ref T4029. D10756 introduced, but did not populate, this column. This can cause it to fill with `"\0\0\0..."` after adjustment.
Regardless of the adjustment issue, it's nice to populate this column anyway because there's no fundamental reason an object can't have mail sent about it without being saved first, even though it may not practically be possible in the codebase today.
Test Plan:
- Ran `storage upgrade`, saw the column populate for older documents.
- Forced a couple of keys to bad values (too short or with "\0") and saw the migration fix them.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4029, T1191, T6487
Differential Revision: https://secure.phabricator.com/D10804
Summary: I scoped this wrong and didn't test.
Test Plan: Hover on logo
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10801
Summary: Ref T5833. Since these will no longer be bound specifically to users, bring them to a more central location.
Test Plan:
- Edited SSH keys.
- Ran `bin/ssh-auth` and `bin/ssh-auth-key`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10791
Summary: Ref T4214, this breaks the 'eye' out as a separate image 40px x 40px. We also now show the eye on mobile, as we have enough room for both currently.
Test Plan: Tested default and nightmaremoon colors, tested mobile, tablet and desktop layouts.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4214
Differential Revision: https://secure.phabricator.com/D10794
Summary:
Ref T5833. Allow services and devices to be tagged with projects.
(These fluff apply implementations are a good example of the issue discussed in T6403.)
Test Plan: {F229569}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10782
Summary:
Ref T5833. Currently, we have an `AlmanacDeviceProperty`, but it doesn't use CustomFields and is specific to devices. Make this more generic:
- Reuse most of the CustomField infrastructure (so we can eventually get easy support for nice editor UIs, etc).
- Make properties more generic so Services, Bindings and Devices can all have them.
The major difference between this implementation and existing CustomField implementations is that all other implementations are application-authoritative: the application code determines what the available list of fields is.
I want Almanac to be a bit more freeform (basically: you can write whatever properties you want, and we'll put nice UIs on them if we have a nice UI available). For example, we might have some sort of "ServiceTemplate" that says "a database binding should usually have the fields 'writable', 'active', 'credential'", which would do things like offer these as options and put a nice UI on them, but you should also be able to write whatever other properties you want and add services without building a specific service template for them.
This involves a little bit of rule bending, but ends up pretty clean. We can adjust CustomField to accommodate this a bit more gracefully later on if it makes sense.
Test Plan: {F229172}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10777
Summary: We're having trouble with this pointing at the wrong file after D10778; I assume that making it match the others is the proper fix...
Test Plan: Crossed fingers.
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10785
Summary: Fixes T6469. Changes the default icon into text instead. Added the text to hidden boards and now display when reordering as well.
Test Plan:
Moved a bunch of columns, tested reordering. Seems more clear.
{F229626}
{F229627}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6469
Differential Revision: https://secure.phabricator.com/D10784
Summary: Fixes T6452, provides actual href instead of just #
Test Plan: Control click and open link in new window, is correct task. Click and see item added to selection list.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6452
Differential Revision: https://secure.phabricator.com/D10774
Summary: Fixes T6440. The issue is when a nested span appears, we don't apply the spacing.
Test Plan: Test a new diff in my sandbox that exhibits the issue. Ensure spacing now aligns.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6440
Differential Revision: https://secure.phabricator.com/D10768
Summary:
This implements as little as possible to stick a working transactions + editor codepath in the basic create / edit flow. Aside from the transaction tables, this also required adding a mailKey to a phrictionDocument.
Future work would include adding more transactions types for things like "move" and all the pertinent support. Even future work is to add things like policies which will work easily in the transaction framework. Ref T4029.
Test Plan:
- made a wiki doc
- edit a wiki doc
- had someone subscribe to a wiki doc and edited it
For all three, the edits worked, a reasonable email was sent out, and feed stories were generated.
- made a wiki doc at a /location/like/this
document "stubs" were made as expected in /location and /location/like
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: chad, Korvin, epriestley
Maniphest Tasks: T4029
Differential Revision: https://secure.phabricator.com/D10756
Summary:
Ref T1191. Notable stuff:
- Adds `--disable-utf8mb4` to `bin/storage` to make it easier to test what things will (approximately) do on old MySQL. This isn't 100% perfect but should catch all the major stuff. It basically makes us pretend the server is an old server.
- Require utf8mb4 to dump a quickstart.
- Fix some issues with quickstart generation, notably special casing the FULLTEXT handling.
- Add an `--unsafe` flag to `bin/storage adjust` to let it truncate data to fix schemata.
- Fix some old patches which don't work if the default table charset is utf8mb4.
Test Plan:
- Dumped a quickstart.
- Loaded the quickstart with utf8mb4.
- Loaded the quickstart with `--disable-utf8mb4` (verified that we get binary columns, etc).
- Adjusted schema with `--disable-utf8mb4` (got a long adjustment with binary columns, some truncation stuff with weird edge case test data).
- Adjusted schema with `--disable-utf8mb4 --unsafe` (got truncations and clean adjust).
- Adjusted schema back without `--disable-utf8mb4` (got a long adjustment with utf8mb4 columns, some invalid data on truncated utf8).
- Adjusted schema without `--disable-utf8mb4`, but with `--unsafe` (got truncations on the invalid data).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10757
Summary:
I've been advised that that the MediaWiki flower needs to
appear with the square brackets for proper MediaWiki trademark
presentation.
See https://phabricator.wikimedia.org/T543 for relevant discussion.
Test Plan:
view login page with MediaWiki oauth plugin enabled,
see that the logo includes square brackets around the flower
Reviewers: epriestley, qgil, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: Korvin, epriestley
Projects: #auth, #wikimedia
Differential Revision: https://secure.phabricator.com/D10752
Summary: Ref T5833. Allows you to bind a service (like `db.example.com`) to one or more interfaces (for example, to specify a pool with one read/write host and two read-only hosts). You can't configure which hosts have which properties yet, but you can add all the relevant interfaces to the service. Next diff will start supporting service, binding, and device properties like "is writable", "is active", etc., so that Almanac will be able to express operations like "change which database is writable", "disable writes", "bring a device down", etc.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10745
Summary: I keep finding edge cases where this is causing issues, like PHUITagView in Audit as a property. Going to revert.
Test Plan: Review an Audit, tag is not cut off.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10740
Summary: We removed other gradients, but not this one, for consistency.
Test Plan: UIExamples
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10737
