Summary: database migration + drop old view code. Fixes T5604.
Test Plan: grepped src/ for TYPE_CCS (no hits); viewed some tasks with old cc transactions and noted they still rendered correctly post data conversion
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5604
Differential Revision: https://secure.phabricator.com/D11015
Summary:
Fixes T6741. This allows Almanac services to be locked from the CLI. Locked services (and their bindings, interfaces and devices) can not be edited. This serves two similar use cases:
- For normal installs, you can protect cluster configuration from an attacker who compromises an account (or generally harden services which are intended to be difficult to edit).
- For Phacility, we can lock externally-managed instance cluster configuration without having to pull any spooky tricks.
Test Plan:
- Locked and unlocked services.
- Verified locking a service locks connected properties, bindings, binding properties, interfaces, devices, and device properties.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6741
Differential Revision: https://secure.phabricator.com/D11006
Summary:
Ref T5833. This allows services to be typed, to distinguish between different kinds of services. This makes a few things easier:
- It's easier for clients to select the services they're interested in (see note in T5873 about Phacility). This isn't a full-power solution, but gets is some of the way there.
- It's easier to set appropriate permissions around when modifications to the Phabricator cluster are allowed. These service nodes need to be demarcated as special in some way no matter what (see T6741). This also defines a new policy for users who are permitted to create services.
- It's easier to browse/review/understand services.
- Future diffs will allow ServiceTypes to specify more service structure (for example, default properties) to make it easier to configure services correctly. Instead of a free-for-all, you'll get a useful list of things that consumers of the service expect to read.
The "custom" service type allows unstructured/freeform services to be created.
Test Plan:
- Created a new service (and hit error cases).
- Edited an existing service.
- Saw service types on list and detail views.
- Poked around new permission stuff.
- Ran `almanac.queryservices` with service class specification.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10995
Summary:
Ref T5955. Summary of intended changes:
**Improve Granularity of Authorization**: Currently, users have one Conduit Certificate. This isn't very flexible, and means that you can't ever generate an API token with limited permissions or IP block controls (see T6706). This moves toward a world where you can generate multiple tokens, revoke them individually, and assign disparate privileges to them.
**Standardize Token Management**: This moves Conduit to work the same way that sessions, OAuth authorizations, and temporary tokens already work, instead of being this crazy bizarre mess.
**Make Authentication Faster**: Authentication currently requires a handshake (conduit.connect) to establish a session, like the web UI. This is unnecessary from a security point of view and puts an extra round trip in front of all Conduit activity. Essentially no other API anywhere works like this.
**Make Authentication Simpler**: The handshake is complex, and involves deriving hashes. The session is also complex, and creates issues like T4377. Handshake and session management require different inputs.
**Make Token Management Simpler**: The certificate is this huge long thing right now, which is not necessary from a security perspective. There are separate Arcanist handshake tokens, but they have a different set of issues. We can move forward to a token management world where neither of these problems exist.
**Lower Protocol Barrier**: The simplest possible API client is very complex right now. It should be `curl`. Simplifying authentication is a necessary step toward this.
**Unblock T2783**: T2783 is blocked on nodes in the cluster making authenticated API calls to other nodes. This provides a simpler way forward than the handshake mess (or enormous-hack-mess) which would currently be required.
Test Plan:
- Generated tokens.
- Generated tokens for a bot account.
- Terminated tokens (and for a bot account).
- Terminated all tokens (and for a bot account).
- Ran GC and saw it reap all the expired tokens.
NOTE: These tokens can not actually be used to authenticate yet!
{F249658}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5955
Differential Revision: https://secure.phabricator.com/D10985
Summary:
Ref T2783. This is primarily exploratory and just figuring out what we're blocked on:
- Allow a Repository to be bound to a Service. The Service may eventually define multiple read/write nodes, etc.
- There's no UI to do this binding yet, you have to touch the database manually.
- If a repository is bound to a Service, effect Conduit calls via calls to the remote service instead of executing them in-process.
- These don't actually work yet since there's no authentication (see T5955).
Test Plan:
- Made a nice Service with a nice Binding to a nice Interface on a nice Device.
- Force-associated a repository with the service using a raw MySQL query.
- Saw Phabricator try to make a remote call to the service (on localhost) and fail because of missing auth stuff.
- Also ran `almanac.queryservices`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D10982
Summary: Fixes T5604. This should fix some random bugs, lets us move forward more easily, and all that good stuff about killing code debt.
Test Plan:
- Conduit method maniphest.createtask
- verified creating user subscribed
- verified subscription transaction
- Conduit method maniphest.update
- verified subscribers set as specified to ccPHIDs parameter
- verified subscription transaction
- Herald
- verified herald rule to add subscriber worked
- verified no subscribers removed accidentally
- edit controller
- test create and verify author gets added IFF they put themselves in subscribers control box
- test update gets set to exactly what user enters
- lipsum generator'd tasks work
- bulk add subscribers works
- bulk remove subscriber works
- detail controller
- added myself by leaving a comment
- added another user via explicit action
- added another user via implicit mention
- task merge via search attach controller
- mail reply handler
- add subscriber via ./bin/mail receive-test
- unsubscribe via ./bin/mail receive-test
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T5604
Differential Revision: https://secure.phabricator.com/D10965
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:
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: 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 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: 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: 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: 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 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:
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 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: Ref T5833. An interface is an IP (maybe v4, maybe v6) and port on a specified network (public internet, VPN, NAT block, etc).
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10718
Summary: Ref T5833. This differentiates address spaces like the public internet from VPNs, so when a service is available at `192.168.0.1`, we'll know it's on some specific NAT block or whatever.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10715
Summary: Ref T5833. The "uninteresting" part of this object is virtually identical to AlmanacService.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10714
Summary: Ref T5833. See that task for functional goals and some discussion of design.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10713
Summary: Ref T2787. When order statuses change, send merchants and users email about it.
Test Plan: Used `bin/mail` to review mail.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10694
Summary: Ref T2787. I mostly just want these in place so I can glue emails to them, but they're also useful on their own.
Test Plan: {F216515}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10688
Summary:
Ref T2787. Make this a little more concrete with explicit membership instead of a general edit policy. In particular, we need to know who to email when orders happen, and can't reasonably do that with an edit policy.
I imagine this might eventually get more nuanced (e.g., users who can only approve orders vs users who can manage the merchant itself) but that's a long ways away.
Test Plan: {F216284}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10681
Summary: Ref T5835. Make fund stories publish to feed and send email.
Test Plan: Made edits, etc., saw them in feed and outbound email.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5835
Differential Revision: https://secure.phabricator.com/D10677
Summary:
Ref T2787. This has some rough edges but basically works.
- Users can cancel orders that are in incomplete states (or in complete states, if the application allows them to -- for example, some future application might allow cancellation of billed-but-not-shipped orders).
- Merchant controllers can partially or fully refund orders from any state after payment.
Test Plan: This is still rough around the edges, but issued Stripe and WePay refunds.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10664
Summary:
Ref T2787.
- Allow merchants to disable payment providers.
- Show more useful information about providers on the payments page.
- Make test vs live more clear.
- Show merchant status.
- Add a description to merchants to flesh them out a bit -- the merchant areas of responsibilities seem to be fitting well with accounts, etc.
Test Plan: {F215109}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10662
Summary:
Ref T2787. Builds on D10649 by rebining existing objects (carts, charges, etc) to merchantPHIDs and providerPHIDs instead of an implicit global merchant and weird global artifacts (providerType / providerKey).
Basically:
- When you create something that users can pay for, you specify a merchant to control where the payment goes.
- Accounts are install-wide, but payment methods are bound to merchants. This seems to do a reasonable job of balancing usability and technical concerns.
- Replace a bunch of weird links between objects with standard PHIDs.
- Improve "add payment method" flow.
Test Plan: Went through the Fund flow with Stripe and WePay, funding an initiative.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10652
Summary:
Ref T2787. Instead of making providers global configuration, make them a thing on merchants with web configuration.
Payment methods and some of the pyament workflow needs to be retooled a bit after this, but this seemed like a reasonable cutoff point for this diff.
Test Plan: See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10649
Summary:
Ref T2787. Currently, you add payment providers (Stripe, Paypal, etc) in global configuration.
Generally, this approach is cumbersome, limiting, and often hard for users to figure out. It also doesn't provide a natural way to segment payment receivers or provide web access to administrative payment functions like issuing refunds, canceling orders, etc. I think that stuff definitely needs to be in the web UI, and the rule for access to it can't reasonably just be "all administrators" in a lot of reasonable cases.
The only real advantage is that it prevents an attacker from adjusting settings and pointing something at an account they control. But this attack can be mitigated through notifications, some sort of CLI-only merchant lock, payment accounts being relatively identifiable, etc.
So introduce "merchants", which are basically payable entities. An individual merchant will have attached Paypal, Stripe, etc., accounts, and access rules. When you buy something in an application, the merchant to pay is also specified. They also provide an umbrella for dealing with permissions down the line.
This may get a //little// cumbersome because if there are several merchants your saved card information is not shared across them. I think that will be fine in the normal case (most installs will have only one merchant). Even if it isn't and we leave providers global, I think introducing this is the right call from a web UI / permissions point of view. I'll play around with it in the next couple of diffs and figure out exactly where the line goes.
Test Plan: Listed, created, edited, viewed merchants.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10648
Summary: Ref T2787. Similar to D10634, give applications more control over the cart workflow. For now this just means they get to pick exit URIs, but in the future they can manage more details of cart behavior.
Test Plan: Funded an initiative and got returned to the initiative instead of dead-ending in Phortune.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10638
Summary: Ref T2787. `Product` is currently a fairly heavy object, but as Phortune develops it makes a lot of sense to make it a lighter object and put more product logic in applications. Convert it into a fairly lightweight reference to applications. The idea is that Phortune is mostly providing a cart flow, and applications manage the details of products.
Test Plan: Funded an initiative for $1.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10634
Summary:
Ref T2787. Phortune currently stores a bunch of stuff as `...inUSDCents`. This ends up being pretty cumbersome and I worry it will create a huge headache down the road (and possibly not that far off if we do Coinbase/Bitcoin soon). Even now, it's more of a pain than I figured it would be.
Instead:
- Provide an application-level serialization mechanism.
- Provide currency serialization.
- Store currency in an abstract way (currently, as "1.23 USD") that can handle currencies in the future.
- Change all `...inUSDCents` to `..asCurrency`.
- This generally simplifies all the application code.
- Also remove some columns which don't make sense or don't make sense anymore. Notably, `Product` is going to get more abstract and mostly be provided by applications.
Test Plan:
- Created a new product.
- Purchased a product.
- Backed an initiative.
- Ran unit tests.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2787
Differential Revision: https://secure.phabricator.com/D10633
Summary:
Ref T4209. This creates storage for public keys against authorized hosts, such that servers can be authorized to make Conduit calls as the omnipotent user.
Servers are registered into this system by running the following command once:
```
bin/almanac register
```
NOTE: This doesn't implement authorization between servers, just the storage of public keys.
Placing this against Almanac seemed like the most sensible place, since I'm imagining in future that the `register` command will accept more information (like the hostname of the server so it can be found in the service directory).
Test Plan: Ran `bin/almanac register` and saw the host (and public key information) appear in the database.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4209
Differential Revision: https://secure.phabricator.com/D10400
Summary:
Ref T1191. Although I fixed some of the mutations earlier (in D10598), I missed the column mutations under old versions of MySQL. In particular, this isn't valid:
- `ALTER TABLE ... MODIFY columnName VARCHAR(64) COLLATE binary`
Issue the permitted version of this instead, which is:
- `ALTER TABLE ... MODIFY columnName VARBINARY(64)`
Also fixed an issue where a clean schema had the wrong nullability for a column in the draft table. Force it to the expected nullability.
The other trick here is around the one column with a FULLTEXT index on it, which needs a little massaging.
Test Plan:
- Forced my local install to return `false` for utf8mb4 support.
- Did a clean adjust into `binary` columns.
- Poked around, added emoji to things.
- Reverted the fake check and did a clean adjust into `utf8mb4` columns.
- Emoji survived.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: fabe, epriestley
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10627
Summary:
Ref T1191. Now that the whole database is covered, we don't need to do as much work to build expected schemata. Doing them database-by-database was helpful in converting, but is just reudndant work now.
Instead of requiring every application to build its Lisk objects, just build all Lisk objects.
I removed `harbormaster.lisk_counter` because it is unused.
It would be nice to autogenerate edge schemata, too, but that's a little trickier.
Test Plan: Database setup issues are all green.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, hach-que
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10620
Summary:
Ref T1191. Long ago, Maniphest generated with 40-character mail keys. These prevent the migration to `bytes20`. We had about 300 of these on secure.phabricator.com from several years ago.
Just truncate them. This adjusts reply-to addresses, but it's very likely that none are relevant anymore.
Test Plan: Ran migration on `secure.phabricator.com` to truncate keys.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10615
Summary: Ref T1191. This predates the mdoern patch stuff and may exist on very, very old installs. By the time they apply this patch, it's guaranteed it won't matter anymore. Drop it to make the schemata consistent with expectations.
Test Plan: Ran patch on installs with and without the table.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10611
Summary:
Ref T1191. This destroys surplus columns:
- Pholio's transaction comments have a `mockID` column, but this is not used. The `imageID` column is used instead.
- Phragment has an unused `description` column.
- Releeph has an unused `summary` column.
Test Plan:
- Grepped for usage of these columns.
- Checked that these exist in production, too.
- Ran upgrades.
- Added Pholio inline comments.
- Saw fewer warnings.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, hach-que
Maniphest Tasks: T1191
Differential Revision: https://secure.phabricator.com/D10591
