Summary:
Ref T10246. Drydock still has a few outstanding issues, but it's mostly minor UI stuff and the documentation has reasonable caveats about it.
Broadly, I don't expect to make any major changes to Drydock in the future (i.e., all the fundamentals seem sound at this point) and it doesn't have any major technical debt or, like, obsolete APIs or anything.
Test Plan: Saw "Drydock" as not-a-prototype in Applications.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10246
Differential Revision: https://secure.phabricator.com/D15401
Summary: Fixes T10449. Almanac doesn't do a whole lot for the average user, but is in good shape technically and works well, and exposing it in the cluster won't let installs destroy themselves now.
Test Plan: Re-read documentation; grepped for `TODO` (there are a couple, but reasonable to push off); browsed around all the UI things (new two-column looks great), called API methods.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15400
Summary: Ref T10449. Modernize the AlmanacDevice code a bit.
Test Plan:
- Created a device.
- Edited a device.
- Listed devices.
- Viewed a device.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15399
Summary: Ref T10449. This modernizes the service creation/editing flow and updates the list view code a little bit.
Test Plan:
- Created a service.
- Edited a service.
- Browsed services.
- Hit policy exception for editing cluster services with no permission.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15398
Summary: Fix an issue where you've already answered, moved the summary section.
Test Plan: Review an answer with a wiki that i've already answered
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15395
Summary: Ref T10457. Use modern controller and UI tech to build the list view and actions.
Test Plan:
- Viewed operation list.
- Viewed operation detail.
- Checked menus on mobile.
{F1139757}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15393
Summary:
Ref T10457.
- Let blueprints be tagged so you can search and annotate them a little more easily.
- Give each blueprint type an optional icon to make things a little easier to parse visually.
Test Plan:
- Tagged blueprints.
- Searched by tags.
- Looked at nice little icons.
{F1139712}
Reviewers: chad
Reviewed By: chad
Subscribers: yelirekim
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15392
Summary:
Ref T10457. Fixes T10024. This primarily just modernizes blueprints to use EditEngine.
This also fixes T10024, which was an issue with stored properties not being flagged correctly.
Also slightly improves typeaheads for blueprints (more information, disabled state).
Test Plan:
- Created and edited various types of blueprints.
- Set and removed limits.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10024, T10457
Differential Revision: https://secure.phabricator.com/D15390
Summary:
Ref T10457. The ngram indexing seems to be working well; extend it into Drydock.
Also clean up the list controller a little bit.
Test Plan:
- Ran migrations.
- Searched for blueprints by name.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15389
Summary: I kinda like these to differentiate the headers and different object types. Somethings duplicitive, but helps orient the clean header a bit.
Test Plan: Review each in sandbox.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15394
Summary: Fixes T10474. This had the same root cause as T10024 -- a missing call to `didSetValueFromStorage()` because of the way the fields work.
Test Plan:
- Edited a text panel before change, without changing text: got silly transaction.
- Made change, edited text panel without changing text, no transaction.
- Made a real edit, got a good transaction.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10474
Differential Revision: https://secure.phabricator.com/D15391
Summary: Fixes T10504. The "Create Blog" buttons weren't generated by EditEngine, but should be, so that the UI and policies are in sync.
Test Plan:
- Viewed blog list as user with and without permission to create blogs. Saw correct button state.
- Tried to create blogs, saw correct result.
- Viewed empty state of home, clicked "New Blog" buttons.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10504
Differential Revision: https://secure.phabricator.com/D15384
Summary: Updates the Fund application to use a two column layout
Test Plan: Make an initiative.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15383
Summary: Simplifies building pages a little more, adds a helper method to just add a property section to the main column automatically above other content.
Test Plan: Review Ponder, Herald, Passphrase, Countdown.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15377
Summary: Updates Almanac to the new layout, adds some header icons for interest.
Test Plan: Click on all the different almanac pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15381
Summary:
Ref T5822.
- After a log is closed, compress it if possible.
- Provide `bin/harbormaster archive-logs` to make it easier to change the storage format of logs.
Test Plan:
- Ran `bin/harbormaster archive-logs` on a bunch of logs, compressing and decompressing them without issues (same hashes, same decompressed size across multiple iterations).
- Ran new builds, verified logs were compressed after they closed.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5822
Differential Revision: https://secure.phabricator.com/D15380
Summary: Ref T5822. This will make it easier to compress and archive chunks without needing to hold them in memory.
Test Plan: Ran a build, looked at some logs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5822
Differential Revision: https://secure.phabricator.com/D15378
Summary:
Ref T10457. Currently, this table is an ad-hoc table, but can easily be turned into a normal table.
This will make iterating over log chunks to compress and archive them easier.
Test Plan: Viewed logs, ran `bin/storage adjust` with no issues.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15376
Summary:
Ref T10457. Currently, every `append()` call necessarily generates queries, and these queries are slightly inefficient if a large block of data is appended to a partial log (they do about twice as much work as they technically need to).
Use `PhutilRope` to buffer `append()` input so the logic is a little cleaner and we could add a rule like "flush logs no more than once every 500ms" later.
Test Plan:
- Ran a build, saw logs.
- Set chunk size very small, ran build, saw logs, verified small logs in database.
{F1137115}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15375
Summary:
Ref T5822. This prepares for inline compression and garbage collection of build logs.
This reduces the API surface area and removes a log from the "wait" step that would just log a message every 15 seconds. (If this is actually useful, I think we find a better way to communicate it.)
Test Plan:
Ran a build, saw a log:
{F1136691}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5822
Differential Revision: https://secure.phabricator.com/D15371
Summary: Keeping in step with the gradients on the grey buttons, this adds slight gradients and hover states to blue and green. I feel like they are much more obviously buttons now in the UI (on headers for example).
Test Plan:
Review UI Buttons, check differential, actions, other random buttons.
{F1137208}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15379
Summary: This should consistenly apply the styling regardless of font or size of the Header. Fixes T10485
Test Plan: Visit a Task and a Countdown in a different Space.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10485
Differential Revision: https://secure.phabricator.com/D15374
Summary:
Rolls out a new "Object Page" design with PHUITwoColumnView. This is reasonably polished, but wanted to post it up for you now for feedback before chasing down minor bugs. This implements TwoColumn in the following applications:
- Ponder
- Paste
- Slowvote
- Countdown
- Projects
- Profile
- Passphrase
This helped track down display issues and inconsistencies and make sure the layout was flexible for different pages.
Test Plan:
Test each of the applications on mobile, tablet, and desktop breakpoints.
{F1135705}
{F1135706}
{F1135707}
{F1135708}
{F1135709}
{F1135710}
{F1135711}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15366
Summary:
Ref T10457. Skipped tests are almost always well-behaved (e.g., `testWindows()`, but the test is running on Linux) and not interesting, and we do not expect well-written, solid systems to necessarily have 0 skips.
Although skips //could// indicate that you have missing dependencies on a build server, and thus be a bit interesting, I think they almost always indicate that a particular test is not expected to run in the current environment.
If we wanted to tackle this problem in granular detail, we could eventually add a "Missing" status or similar which would serve as "a skip you //could// reasonably fix in this environment", but I don't think that's too interesting.
Test Plan:
Here's an example of a build result with skips: B10875
{F1136511}
I think this is clearer as "Passed", as this is the expected production state of the build.
Locally, looked at some builds.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15369
Summary: Fixes T10478.
Test Plan: Paged projects by "Created" without errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10478
Differential Revision: https://secure.phabricator.com/D15367
Summary: Ref T10457. This gives unit test results a more first-class treatment in the Differential UI, and consolidates some rendering code.
Test Plan:
Before:
{F1135536}
After:
{F1135537}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15365
Summary:
Ref T10457. When tests fail, it currently takes several clicks to figure out //why// they failed.
In this project, map rebuilds and `liberate` are fairly common failure conditions, but verifying that they were the root issue requires jumping into a build, then scrolling through a log.
Instead, display details if they're available.
Test Plan:
Before:
{F1135453}
After:
{F1135454}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9951, T10457
Differential Revision: https://secure.phabricator.com/D15363
Summary: Ref T10457. These lack color and iconography and are difficult to parse. Make them easier to read.
Test Plan:
Before:
{F1135396}
After:
{F1135399}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15362
Summary: Ref T10457. This is mostly just for consitency, but I imagine it will make managing large/complex build processes easier, and if we support Herald rules it would eventually let you write "Build plan's tags include [whatever]" to apply behavior to a group of plans.
Test Plan: {F1133107}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15360
Summary: Ref T10457. Allow build plans to be queried by name.
Test Plan:
- Searched for plans by name.
- Renamed a plan, searched for new name.
{F1133085}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15359
Summary:
Fixes T10466. Currently, clicking "Disable Mail" or "Enable Mail" on a project toggles an edge, but it gets a default "added an edge" story and transaction record.
These are confusing, useless and not interesting, so just hide them.
Test Plan:
- Before patch: clicked enable/disable mail, saw "added an edge" / "removed an edge" stories in feed and project history.
- After patch: clicked enable/disable mail, saw nothing in feed or project history.
- (Note that this patch is not retroactive for feed, so already-published stories won't unpublish.)
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10466
Differential Revision: https://secure.phabricator.com/D15361
Summary:
Ref T10457.
- Use EditEngine for Build Plans.
- Fix some minor issues with crumbs being inconsistent.
- Fix some minor issues with mobile menus not being consistent/available.
Test Plan:
- Created and edited build plans.
- Poked around in mobile width, verified mobile menu had the right stuff in it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15357
Summary:
Fixes T10011.
- Modernize searching for buildables.
- Prepare for `harbormaster.buildable.search`.
- Allow users to query by status (see T10011).
- Collapse the four weird "commit / diff / revision / repository" fields into two slightly less weird fields with more UI hinting?
Test Plan: {F1131918}
Reviewers: chad
Reviewed By: chad
Subscribers: Luke081515.2
Maniphest Tasks: T10011
Differential Revision: https://secure.phabricator.com/D15356
Summary:
Ref T10457. This makes diffs/revisions show the revision as the buildable title, and commits show the commit as the title.
Previously, the title was "Buildable X".
Also makes icons/colors/labels more consitent.
Test Plan: {F1131885}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15355
Summary:
Ref T10457. We currently have these weird, out-of-place methods on Harbormaster queries that just load handles. These were written before HandlePool, and HandlePool is now more convenient, simpler, and more efficient.
Drop this stuff in favor of using handle pools off `$viewer`.
Test Plan: Looked at buildable list, looked at buildable detail, grepped for removed methods.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15354
Summary: Fixes T9614. This is kind of silly, but stop users from fighting turf wars over build resources or showing up on an install and just aborting a bunch of builds for the heck of it.
Test Plan:
- Restarted / paused / aborted / etc builds.
- Tried to do the same for builds I didn't have edit permission on the build plan for, got errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9614
Differential Revision: https://secure.phabricator.com/D15353
Summary: Fixes T10458. These steps are obsolete and have not worked since the last updates to Drydock. They may eventually return in some form, but get rid of them for now since they're confusing.
Test Plan:
- Created a build plan with these steps.
- Removed these steps.
- Verified the build plan showed that the steps were invalid, and that I could delete them.
- Deleted them.
- Added new steps, no obsolete steps were available for selection.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10458
Differential Revision: https://secure.phabricator.com/D15352
Summary:
Ref T10449. Currently, we store classes (like "AlmanacClusterRepositoryServiceType") in the database.
Instead, store types (like "cluster.repository").
This is a small change, but types are a little more flexible (they let us freely reanme classes), a little cleaner (fewer magic strings in the codebase), and a little better for API usage (they're more human readable).
Make this minor usability change now, before we unprototype.
Also make services searchable by type.
Also remove old Almanac API endpoints.
Test Plan:
- Ran migration, verified all data migrated properly.
- Created, edited, rebound, and changed properties of services.
- Searched for services by service type.
- Reviewed available Conduit methods.
Reviewers: chad
Reviewed By: chad
Subscribers: yelirekim
Maniphest Tasks: T10449
Differential Revision: https://secure.phabricator.com/D15346
Summary:
Fixes T9762. Ref T10246.
**Disabling Bindings**: Previously, there was no formal way to disable bindings. The internal callers sometimes check some informal property on the binding, but this is a common need and deserves first-class support in the UI. Allow bindings to be disabled.
**Deleting Interfaces**: Previously, you could not delete interfaces. Now, you can delete unused interfaces.
Also some minor cleanup and slightly less mysterious documentation.
Test Plan: Disabled bindings and deleted interfaces.
Reviewers: chad
Reviewed By: chad
Subscribers: yelirekim
Maniphest Tasks: T9762, T10246
Differential Revision: https://secure.phabricator.com/D15345
Summary:
Fixes T6741. Ref T10246. Broadly, we want to protect Almanac cluster services:
- Today, against users in the Phacility cluster accidentally breaking their own instances.
- In the future, against attackers compromising administrative accounts and adding a new "cluster database" which points at hardware they control.
The way this works right now is really complicated: there's a global "can create cluster services" setting, and then separate per-service and per-device locks.
Instead, change "Can Create Cluster Services" into "Can Manage Cluster Services". Require this permission (in addition to normal permissions) to edit or create any cluster service.
This permission can be locked to "No One" via config (as we do in the Phacility cluster) so we only need this one simple setting.
There's also zero reason to individually lock //some// of the cluster services.
Also improve extended policy errors.
The UI here is still a little heavy-handed, but should be good enough for the moment.
Test Plan:
- Ran migrations.
- Verified that cluster services and bindings reported that they belonged to the cluster.
- Edited a cluster binding.
- Verified that the bound device was marked as a cluster device
- Moved a cluster binding, verified the old device was unmarked as a cluster device.
- Tried to edit a cluster device as an unprivileged user, got a sensible error.
{F1126552}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6741, T10246
Differential Revision: https://secure.phabricator.com/D15339
Summary: Evidently I only tested adding a question, not an answer. Properly set the getter. Also, fixed some header spacing.
Test Plan: Add a question, add an answer. See everything work, proper spacing.
Reviewers: epriestley, avivey
Reviewed By: avivey
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15341
