Summary:
Ref T13658. This just scrubs some of the simple references from the codebase.
Most of what's left is in documentation which won't be relevant for a fork and/or which I need to separately revise (or more-or-less delete) at some point anyway.
I removed the "install RHEL" and "install Ubuntu" scripts outright since I don't have any reasonable way to test them and don't plan to maintain them.
Test Plan: Grepped for "phacility", "epriestley"; ran unit tests.
Reviewers: cspeckmim
Reviewed By: cspeckmim
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13658
Differential Revision: https://secure.phabricator.com/D21678
Summary: Ref T13641. These conditions are swapped, and "activeBindings" loads more data than necessary while "bindings" doesn't load enough.
Test Plan: Called method with each attachment, got good results instead of an exception.
Maniphest Tasks: T13641
Differential Revision: https://secure.phabricator.com/D21657
Summary: Ref T13641. Now that we can provide an "Active Devices" query, provide it and make it the default.
Test Plan: Viewed Almanac devices, got a list of active devices. Clicked "All Devices" to get all devices.
Maniphest Tasks: T13641
Differential Revision: https://secure.phabricator.com/D21636
Summary: Ref T13065. Ref T13641. Migrate "mailKey" and drop the column.
Test Plan: Ran "bin/storage upgrade", got a clean report, saw keys migrate in mail properties table.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13641, T13065
Differential Revision: https://secure.phabricator.com/D21632
Summary: Ref T13065. Ref T13641. Migrate "mailKey" and drop the column.
Test Plan: Ran "bin/storage upgrade", got a clean report, saw mail keys migrated to mail properties table.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13641, T13065
Differential Revision: https://secure.phabricator.com/D21631
Summary: Ref T13065. Ref T13641. Migrate "mailKey" and drop the column.
Test Plan: Ran "bin/storage upgrade", got a clean report and saw binding mail keys in the mail properties table.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13641, T13065
Differential Revision: https://secure.phabricator.com/D21630
Summary: Ref T13641. Ref T13065. Migrate and drop the onboard "mailKey" column for Almanac Services.
Test Plan: Ran storage migration, got clean report, saw migrated value in mail properties table.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13641, T13065
Differential Revision: https://secure.phabricator.com/D21629
Summary: Ref T13641. Make "active bindings" a real query and make callers that only care about active bindings only query for active bindings.
Test Plan:
- Queried for "bindings" and "activeBindings" via Conduit.
- Disabled/enabled devices, saw binding status update in UI.
- Loaded Diffusion cluster layout.
- Grepped for `needBindings()`, `getActiveBindings()`, etc.
Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13641
Differential Revision: https://secure.phabricator.com/D21628
Summary:
Ref T13641. Add a "status" property with most of the relevant support code.
This currently has no impact on use of the device or bindings by Diffusion or Drydock: they ignore the status of devices bound to services.
Test Plan:
- Created a new device.
- Changed the status of a device via web and API.
- Queried for devices via API.
- Searched for active and disabled devices.
- Viewed UI in list view, detail view.
- Used typeahead to add a new binding to an interface on a disabled device, got disabled hint in typeahead UI.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13641
Differential Revision: https://secure.phabricator.com/D21627
Summary: Ref T13641. Provide minor modernizations before adding a "Disabled" state.
Test Plan: Browsed devices, created a new device.
Maniphest Tasks: T13641
Differential Revision: https://secure.phabricator.com/D21626
Summary: Ref T13065. See similar changes attached to that task.
Test Plan: Ran migration, got a clean database state, saw mail keys populate in mail property table.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13065, T13641
Differential Revision: https://secure.phabricator.com/D21625
Summary:
See <https://discourse.phabricator-community.org/t/i-cant-create-almanac-space/4424/>.
Almanac namespaces have never really had property support, but they implemented the interface in the original implementation.
At the time, this had no effect. Later changes integrated properties into the edit flows and broke this no-op integration.
Remove the interface for now. They could be given property support later, but need a bit of support code.
This feature is very rarely used and primarily useful for Phacility instances.
Test Plan: Created new namespaces and edited namespaces, browsed namespace UI.
Differential Revision: https://secure.phabricator.com/D21543
Summary:
Ref T13529. Now that instances can be renamed, an instance may have multiple valid SSH usernames and the preferred SSH username may not be the intenal instance name.
`PhacilitySiteSource` should already always set `diffusion.ssh-username` correctly, to the current preferred SSH username (which may be "new-name" after a rename from "old-name"), so we should never be able to reach this code without an accurate `diffusion.ssh-username` value available.
The code to resolve names into instances also already works for both "ssh old-name@..." and "ssh new-name@...".
So I believe this code has no beneficial effects and only causes harm: it may force us to return "old-name" when falling through would correctly return "new-name".
Test Plan:
- Previously: renamed an instance, then SSH'd to it using both the old and new names. Both work.
- Previously: verified that `diffusion.ssh-username` is set correctly after a rename.
- Verified that Diffusion "Clone" UI now shows "new-name" after an instance rename.
- The real question here is: does this break something I'm not thinking of? And the change probably has to go to production to answer that.
Maniphest Tasks: T13529
Differential Revision: https://secure.phabricator.com/D21259
Summary:
Depends on D20933. Ref T13362. This reorganizes Config a bit and attempts to simplify it.
Subsections are now in a landing page console and groupings have been removed. We "only" have 75 values you can edit from the web UI nowadays, which is still a lot, but less overwhelming than it was in the past. And the trend is generally downward, as config is removed/simplified or moved into application settings.
This also gets rid of the "gigantic blobs of JSON in the UI".
Test Plan: Browsed all Config sections.
Maniphest Tasks: T13362
Differential Revision: https://secure.phabricator.com/D20934
Summary: Depends on D20931. Ref T13362. Move all "Console"-style interfaces to use a consistent layout based on a new "LauncherView" which just centers the content.
Test Plan: Viewed all affected interfaces.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13362
Differential Revision: https://secure.phabricator.com/D20933
Summary:
Depends on D20292. Ref T13259. This converts the rest of the `getPagingValueMap()` callsites to operate on internal cursors instead.
These are pretty one-off for the most part, so I'll annotate them inline.
Test Plan:
- Grouped tasks by project, sorted by title, paged through them, saw consistent outcomes.
- Queried edges with "edge.search", paged through them using the "after" cursor.
- Poked around the other stuff without catching any brokenness.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13259
Differential Revision: https://secure.phabricator.com/D20293
Summary:
Depends on D20291. Ref T13259. Move all the simple cases (where paging depends only on the partial object and does not depend on keys) to a simple wrapper.
This leaves a smaller set of more complex cases where we care about external data or which keys were requested that I'll convert in followups.
Test Plan: Poked at things, but a lot of stuff is still broken until everything is converted.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13259
Differential Revision: https://secure.phabricator.com/D20292
Summary: Ref T13250. See D20149. Mostly: clarify semantics. Partly: remove magic "null" behavior.
Test Plan: Poked around, but mostly just inspection since these are pretty much one-for-one.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: yelirekim
Maniphest Tasks: T13250
Differential Revision: https://secure.phabricator.com/D20154
Summary: Ref T13250. See D20149. In a number of cases, we use `setQueryParams()` immediately after URI construction. To simplify this slightly, let the constructor take parameters, similar to `HTTPSFuture`.
Test Plan: See inlines.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13250
Differential Revision: https://secure.phabricator.com/D20151
Summary:
Depends on D20115. See <https://discourse.phabricator-community.org/t/transaction-search-endpoint-does-not-work-on-differential-diffs/2369/>.
Currently, `getApplicationTransactionCommentObject()` throws by default. Subclasses must override it to `return null` to indicate that they don't support comments.
This is silly, and leads to a bunch of code that does a `try / catch` around it, and at least some code (here, `transaction.search`) which doesn't `try / catch` and gets the wrong behavior as a result.
Just make it `return null` by default, meaning "no support for comments". Then remove the `try / catch` stuff and all the `return null` implementations.
Test Plan:
- Grepped for `getApplicationTransactionCommentObject()`, fixed each callsite / definition.
- Called `transaction.search` on a diff with transactions (i.e., not a sourced-from-commit diff).
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: jbrownEP
Differential Revision: https://secure.phabricator.com/D20121
Summary:
See PHI1030. When you edit an Almanac object, we attempt to discover all the related objects so we can dirty the repository cluster routing cache: if you modify a Device or Service that's part of a clustered repository, we need to blow away our cached view of the layout.
Currently, we don't correctly find linked Bindings when editing a Device, so we may miss Services which have keys that need to be disabled. Instead, discover these linked objects.
See D17000 for the original implementation and more context.
Test Plan:
- Used `var_dump()` to dump out the discovered objects and dirtied cache keys.
- Before change: editing a Service dirties repository routing keys (this is correct), but editing a Device does not.
- After change: editing a Device now correctly dirties repository routing keys.
Reviewers: amckinley
Reviewed By: amckinley
Differential Revision: https://secure.phabricator.com/D20003
Summary:
Depends on D19919. Ref T11351. This method appeared in D8802 (note that "get...Object" was renamed to "get...Transaction" there, so this method was actually "new" even though a method of the same name had existed before).
The goal at the time was to let Harbormaster post build results to Diffs and have them end up on Revisions, but this eventually got a better implementation (see below) where the Harbormaster-specific code can just specify a "publishable object" where build results should go.
The new `get...Object` semantics ultimately broke some stuff, and the actual implementation in Differential was removed in D10911, so this method hasn't really served a purpose since December 2014. I think that broke the Harbormaster thing by accident and we just lived with it for a bit, then Harbormaster got some more work and D17139 introduced "publishable" objects which was a better approach. This was later refined by D19281.
So: the original problem (sending build results to the right place) has a good solution now, this method hasn't done anything for 4 years, and it was probably a bad idea in the first place since it's pretty weird/surprising/fragile.
Note that `Comment` objects still have an unrelated method with the same name. In that case, the method ties the `Comment` storage object to the related `Transaction` storage object.
Test Plan: Grepped for `getApplicationTransactionObject`, verified that all remaining callsites are related to `Comment` objects.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T11351
Differential Revision: https://secure.phabricator.com/D19920
Summary:
Depends on D19918. Ref T11351. In D19918, I removed all calls to this method. Now, remove all implementations.
All of these implementations just `return $timeline`, only the three sites in D19918 did anything interesting.
Test Plan: Used `grep willRenderTimeline` to find callsites, found none.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T11351
Differential Revision: https://secure.phabricator.com/D19919
Summary: Suppress an unhelpful Almanac transaction and document the location of the secret clustering management capability. I thought maybe implementing `shouldHide` and checking for `isCreate` would work, but the binding apparently gets created before an interface is bound to it.
Test Plan: Looked at a fresh binding and didn't see "Unknown Object(??)", ran bin/diviner and saw expected output.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D19917
Summary:
Ref T13222. See PHI683. Currently, you can "Change subtype..." via Conduit and the bulk editor, but not via the comment action stack or edit forms.
In PHI683 an install is doing this often enough that they'd like it to become a first-class action. I've generally been cautious about pushing this action to become a first-class action (there are some inevitable rough edges and I don't want to add too much complexity if there isn't a use case for it) but since we have evidence that users would find it useful and nothing has exploded yet, I'm comfortable taking another step forward.
Currently, `EditEngine` has this sort of weird `setIsConduitOnly()` method. This actually means more like "this doesn't show up on forms". Make it better align with that. In particular, a "conduit only" field can already show up in the bulk editor, which is goofy. Change this to `setIsFormField()` and convert/simplify existing callsites.
Test Plan:
There are a lot of ways to reach EditEngine so this probably isn't entirely exhaustive, but I think I got pretty much anything which is likely to break:
- Searched for `setIsConduitOnly()` and `getIsConduitOnly()`, converted all callsites to `setIsFormField()`.
- Searched for `setIsLockable()`, `setIsReorderable()` and `setIsDefaultable()` and aligned these calls to intent where applicable.
- Created an Almanac binding.
- Edited an Almanac binding.
- Created an Almanac service.
- Edited an Almanac service.
- Edited a binding property.
- Deleted a binding property.
- Created and edited a badge.
- Awarded and revoked a badge.
- Created and edited an event.
- Made an event recurring.
- Created and edited a Conpherence thread.
- Edited and updated the diff for a revision.
- Created and edited a repository.
- Created and disabled repository URIs.
- Created and edited a blueprint.
- Created and edited tasks.
- Created a paste, edited/archived a paste.
- Created/edited/archived a package.
- Created/edited a project.
- Made comments.
- Moved tasks on workboards via comment action stack.
- Changed task subtype via comment action stack.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19842
Summary: Depends on D19810. Ref T13217. Ref T13216. I mostly used `grep implode | grep OR` and `grep implode | grep AND` to find these -- not totally exhaustive but should be a big chunk of the callsites that are missing `%LO` / `%LA`.
Test Plan:
These are tricky to test exhaustively, but I made an attempt to hit most of them:
- Browsed Almanac interfaces.
- Created/browsed Calendar events.
- Enabled/disabled/showed the lock log.
- Browsed repositories.
- Loaded Facts UI.
- Poked at Multimeter.
- Used typeahead for users and projects.
- Browsed Phriction.
- Ran various fulltext searches.
Not sure these are reachable:
- All the lint stuff might be dead/unreachable/nonfunctional?
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: yelirekim
Maniphest Tasks: T13217, T13216
Differential Revision: https://secure.phabricator.com/D19814
Summary:
Ref T13216. When a repository is clustered, we run this cleanup code (to tell the repository to update, and log some timing information) on both nodes. Currently, we do slightly too much work, which is unnecessary and can be a bit confusing to human readers.
The double update message doesn't hurt anything, but there's no reason to write it twice.
Likewise, the second timing information update query doesn't do anything: there's no PushEvent object with the right identifier, so it just updates nothing. We don't need to run it, and it's confusing that we do.
Instead, only do these writes if we're actually the final node with the repository on it.
Test Plan: Added some logging, saw double writes/updates before the change and no doubles afterwards, with no other behavioral changes.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13216
Differential Revision: https://secure.phabricator.com/D19778
Summary:
Fixes T13184. In Almanac, interfaces are always added to devices. However, if you "Add New Interface" and then "Cancel", you go to the nonexistent `/interface/` page.
Instead, return to the device page.
Test Plan: From a device page, clicked "Add Interface" and then "Cancel". Ended up back where I was.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13184
Differential Revision: https://secure.phabricator.com/D19573
Summary: See discussion in D19379. The 4-tuple of (device, network, address, port) should be unique.
Test Plan: Created lots of duplicate interfaces, bound those interfaces to various services, observed migration script clean things up correctly.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D19388
Summary:
The name of networks should be unique.
Also adds support for exact-name queries for AlamanacNetworks.
Test Plan: Applied migration with existing duplicates, saw networks renamed, attempted to add duplicates, got a nice error message.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D19379
Summary: Ref T13076. This will be used by the metric collection system to iterate over the cluster devices.
Test Plan: Created some cluster and non-cluster devices, searched and saw expected results.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T13076
Differential Revision: https://secure.phabricator.com/D19368
Summary:
Depends on D19356. Fixes T10883. Ref T13120.
- Add a "writable" property to the bindings, defaulting to "true" with a nice dropdown.
- When selecting hosts, allow callers to request a writable host.
- If the caller wants a writable host, only return hosts if they're writable.
- In SVN and Mercurial, we sometimes return only writable hosts when we //could// return read-only hosts, but figuring out if these request are read-only or read-write is currently tricky. Since these repositories can't really cluster yet, this shouldn't matter too much today.
Test Plan:
- Without any config changes, viewed repositories via web UI and pushed/pulled via SSH and HTTP.
- Made all nodes in the cluster read-only by disabling "writable", pulled and hit the web UI (worked), tried to push via SSH and HTTP (got errors about read-only).
- Put everything back, pulled and pushed.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T10883
Differential Revision: https://secure.phabricator.com/D19357
Summary:
Ref T10883. Ref T13120. There's an existing "closed" property on repository services that stops new repositories from being allocated there.
Turn it into a nice boolean.
Test Plan: Toggled the value on/off using a nice `<select />` with helpful labels instead of a text area.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T10883
Differential Revision: https://secure.phabricator.com/D19355
Summary:
Depends on D19342. Ref T12414. Ref T13120. This adds an EditEngine extension for editing Almanac properties.
The actual wire format is a little weird. Normally, we'd have a transaction for each property, but since you can pick any property names you want we can't really do that (we'd have to generate infinite transactions).
The transaction wire format anticipates that transactions may eventually get some kind of metadata -- each transaction looks like this:
```
{
"type": "title",
"value": "Example title"
}
```
...and we can add more keys there. For example, I could have made this transaction look like this:
```
{
"type": "property.set",
"almanac.property.key": "some-key",
"value": "some-value"
}
```
However, I don't want to just accept any possible key freely, and it might be a decent chunk of work to formalize this better. It also doesn't feel great.
I just built special transaction types intead, so you:
```
{
"type": "property.set",
"value": {
"some-key": "some-value",
...
}
}
```
Internally, we may generate more than one transaction as a result (if the "value" has more than one key).
This feels a bit more natural and is probably easier for clients to use anyway.
Test Plan: Set and deleted Service, Device and Binding properties via the API.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19343
Summary:
Depends on D19341. Ref T12414. Ref T13120.
- Fix a bug where default-valued properties didn't get rendered in grey as they're supposed to (as a hint that the value isn't customized).
- When resetting a builtin property won't do anything, visually disable the button as a hint.
- Allow Services to specify properties on their Bindings.
- Specify that repository bindings have a "protocol" property, so it becomes an explicit thing in the UI. Previously, you had to read the documentation to figure this out.
- When editing bindings, use the EditField and its configuration if possible. This turns the "Protocol" property into a dropdown in the UI where you select between "http", "https" and "ssh".
- Give the "protocol" binding a smart default based on the port number of the corresponding interface.
Test Plan:
- Viewed properties on Services, Devices and Bindings.
- Saw them render sensibly, and grey out + grey button when a builtin value has a default setting.
- Saw "Protocol" appear as a default property on repository cluster bindings and get a smart value.
- Edited "protocol", got a nice dropdown.
{F5518791}
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19342
Summary:
Depends on D19340. Ref T12414. Ref T13120. See T12414 for some discussion about direction here.
Since I think retaining "enabled/disabled" as a simple flag is reasonable, expose it via the API for readers and writers.
Also expose binding properties.
Test Plan:
- Searched for bindings and properties with "alamanc.binding.search".
- Enabled and disabled bindings with "almanac.binding.edit".
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19341
Summary:
Depends on D19338. Ref T13120. Ref T12414. These are the last of the new API methods.
This stuff still doesn't work:
- You can't actually enable/disable bindings yet. I want to take a look at the use cases and consider changing "disabled" to "status", or providing a different way to solve the problem.
- You can't edit properties via the API. I expect to enable this for all `AlmanacPropertyInterface` objects with an extension in a future change.
Test Plan:
- Searched for bindings via API.
- Viewed binding web UI for API methods.
- Created bindings via API.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19340
Summary: Depends on D19337. Ref T13120. Ref T12414. These are slightly more substantive than namespace/network, but pretty much standard fare.
Test Plan:
- Searched for interfaces with "almanac.interface.search".
- Created and edited interfaces with "almanac.interface.edit".
- Created and edited interfaces with web UI since some stuff got tweaked.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19338
Summary: Depends on D19336. Ref T13120. Ref T12414. These are simple, straightforward, and uninteresting.
Test Plan:
- Searched for namespaces with "almanac.namespace.search".
- Created and edited namespaces with "almanac.namespace.edit".
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19337
Summary: Depends on D19335. Ref T13120. Ref T12414. There are many good ways to spell "almanac", but stick with convention here.
Test Plan: (O_O)
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19336
Summary: Depends on D19334. Ref T13120. Ref T12414. These are pretty straightforward, but no one really has a use case for them anyway today so they're primarily just for completeness.
Test Plan:
- Queried networks with `almanac.network.search`.
- Created and edited networks with `almanac.network.edit`.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19335
Summary:
Depends on D19329. Ref T13120. Ref T12414. Recent changes have mostly modularized Almanac transactions, but the "property" transactions remained written in an older style with the logic on the Editor/Transaction classes.
This moves them to modern modular transactions. These end up being a little bit copy-pastey, but it doesn't feel too terribly bad.
Test Plan: Created, edited, and deleted properties on services, devices and bindings. Grepped for removed constants.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19334
Summary:
Depends on D19328. Ref T13120. Ref T12414.
Prior work has left us with just a NAME transaction here, which is straightforward to modularize.
Test Plan:
- Created and renamed devices.
- Tried to set no name, a bad name, a duplicate name (got errors).
- Tried to create/rename into a namespace I could not edit (got an error).
- Grepped for `AlmanacDeviceTransaction::`.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19329
Summary:
Depends on D19325. Ref T13120. Ref T12414.
This no longer has any callers in the upstream or in Phacility support libraries, so get rid of it.
This will make modularizing Device transactions significantly easier, since the other transactions are reasonable, normal sorts of transactions.
For existing devices, this leaves some "author edited this object." transactions in the log. I might just leave those since they aren't really hurting anything, or maybe I'll clean them up or hide them later once I have more confidence that these changes are stable.
Test Plan: Grepped for `TYPE_INTERFACE` and `AlmanacDeviceTransaction`, found no callsites.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19328
Summary:
Depends on D19324. Ref T13120. Ref T12414.
This moves "Destroy Interface" to use Interface transactions instead of Device transactions, so we can ultimately get rid of the complex and difficult-to-modernize `AlmanacDeviceTransaction::TYPE_INTERFACE`.
This transaction is a bit weird since it makes the interface delete itself, but this should work OK for now. At some point in the future I'd probably want to change this into more of a "disable" action, but I don't think we face any immediate peril by retaining this behavior for now.
Test Plan:
- Destroyed interfaces on devices using the web UI, saw them vanish.
- Ran daemons, nothing fataled/exploded even though the transaction is weird and destroys the object it affects.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19325
Summary:
Depends on D19323. Ref T13120. Ref T12414.
Move editing to modern stuff and fix some implementation errors from D19323 (mostly copy/paste stuff).
Test Plan:
- Created and edited interfaces.
- Tried to create/edit an interface with a bogus/empty address/port, got errors.
- Tried to create an interface on a bogus device, got an error.
- Tried to create an interface on a device I could not edit, got an error.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19324
Summary:
Depends on D19322. Ref T13120. Ref T12414.
Currently, `AlmanacDevice` has a bit of a beast of a `TYPE_INTERFACE` transaction that fully creates a complex Interface object. This isn't very flexible or consistent, and Interfaces are complex enough to reasonably have their own object behaviors (for example, they have their own PHIDs).
The complexity of this transaction makes modularizing `AlmanacDevice` transactions tricky. To simplify this, move Interface toward having its own set of normal transactions.
This change just adds some reasonable-looking transactions; it doesn't actually hook them up in the UI or make them reachable. I'll test that they actually work as I swap the UI over.
We may also have some code using the `TYPE_INTERFACE` transaction in Phacility support stuff, so that may need to wait a week to actually phase out.
Test Plan: Ran `bin/storage upgrade` and `arc liberate`. This code isn't reachable yet.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19323
Summary: Depends on D19321. Ref T13120. Ref T12414. Move transactions for Almanac Networks (just "name") to ModularTransactions.
Test Plan:
- Created a new network.
- Renamed a network.
- Tried to create a network with no name (got an error).
- Grepped for `AlmanacNetworkTransaction::`.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19322
Summary: Depends on D19320. Ref T13120. Ref T12414. Move transactions for Almanac Bindings to ModularTransactions.
Test Plan:
- Created a new binding.
- Tried to create a duplicate binding, got an error.
- Edited a binding to rebind it to a different device.
- Disabled and enabled bindings.
- Grepped for `AlmanacBindingTransaction::` constants.
When a binding is created, it currently renders a bad "changed the interface from ??? to X" transaction. This is because creation isn't currently using EditEngine. I plan to swap it shortly, which will turn this into a real "Create" transaction and fix the issue.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13120, T12414
Differential Revision: https://secure.phabricator.com/D19321
