Public Access
Keep an open editor's pickers in step with the vault
The host editor's four pickers were snapshots taken when it opened, and the comment on EditorAuthenticationChoices said why: a picker whose contents move under somebody halfway through a form is worse than a list a minute stale, and only one editor could be open at a time anyway, so the only way to add a key was to close this one. The second half of that stopped being true when AHostEditorIsInTheWay was split from AVaultEditorIsInTheWay. The host editor is the Hosts screen's business and the keychain's editors are the Vault screen's; neither refuses the other now, which was the right split — it stopped three quarters of a screen going inert over an editor the user was not looking at — but it left the assumption those snapshots rested on false and nothing to notice. So the ordinary way of using the feature was the broken one. Somebody starts editing a host, finds there is no key to bind it to, goes to KEYS, makes one, and comes back to a picker that does not have it — with the fix being to throw the form away and start again. The same for a password, a tag, a group, and for a whole vault made on the Teams screen because the host being typed belongs to the team rather than to the person typing it: the vault they had just made for it was the one place they could not file it. RefreshOpenEditors refills whichever editor is open, and it hangs off ReloadAsync rather than off the twenty-odd commands that write to the vault. That is the choice worth stating, because it is what makes a sync count as well as a save: a key pulled from another machine reaches the open editor by the same path a key typed here does, and a place that wrote to the vault without refreshing the editor would be a bug nobody would find for months. What the old comment was protecting against is real, so every picker is put back onto what it was already showing, by id, and not one typed field is touched. An editor that reset its own bindings because a background sync landed would be a worse bug than the stale list this fixes — it would rebind a host as a side effect of somebody else's work. The placeholder entries go back too, which is the case 3.4 measures: a group deleted on another machine mid-edit still cannot unfile the host when the form is saved. The group editor gets the same treatment for the same reasons; it shares the drawer, and its default binding is lent to every host under it. The snippet editor's vault picker was the same copy of the same list and went stale the same way. It watches TargetVaults rather than the reload, because that screen has always been a wrapper over the vault's collections and has no reload of its own to hang off — which is how it already follows Snippets. The move panels are deliberately left alone. A vault arriving from a sync while one is open still will not appear in it, but a move panel is opened by the act that fills it and its picker resets its selection to the first entry on every rebuild, so refreshing it would move a destination somebody had chosen. Same class of bug, different answer, and not this change. Five tests, and four of them were checked failing with the RefreshOpenEditors call commented out: a key reaching the open host editor and binding when chosen, an item arriving without moving a selection that was already made, a tag arriving as an unworn chip, a key reaching the group editor, and a vault reaching the host and snippet editors without moving either. Manual check 7.12 sits beside 7.11, which is this same bug on the files screen's picker, and says what the worse failure would look like: a picker that moves rather than one that does not notice.
This commit is contained in:
@@ -2494,6 +2494,130 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
||||
vault.Status.ShouldContain("host");
|
||||
}
|
||||
|
||||
// ---- Keeping an open editor's pickers in step with the vault ----
|
||||
|
||||
/// <remarks>
|
||||
/// The other side of the split guard, and the bug it left behind. The host editor and the keychain's
|
||||
/// editors are on different screens and refuse each other no longer, so a key is very often added
|
||||
/// <em>because</em> the host in front of the user needs one — with that host's editor still standing on the
|
||||
/// Hosts screen. The picker was a snapshot taken when the editor opened, so the key never appeared in it
|
||||
/// and the only way to reach it was to abandon the edit and start again.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AKeyAddedWithTheHostEditorOpen_AppearsInItsAuthenticationPicker()
|
||||
{
|
||||
await UnlockedAsync();
|
||||
var vault = shell.Vault!;
|
||||
|
||||
await AddHostAsync(vault, "prod-db");
|
||||
vault.SelectedHost = vault.Hosts[0];
|
||||
vault.EditSelectedHostCommand.Execute(null);
|
||||
vault.EditorPort = 2244;
|
||||
|
||||
await AddKeyAsync(vault, "deploy");
|
||||
|
||||
vault.IsEditing.ShouldBeTrue("adding a key must not close the host editor");
|
||||
vault.EditorPort.ShouldBe(2244, "nor discard what has been typed into it");
|
||||
|
||||
var offered = vault.EditorAuthenticationChoices.Single(
|
||||
choice => string.Equals(choice.Label, "deploy", StringComparison.Ordinal));
|
||||
|
||||
offered.Kind.ShouldBe(AuthenticationKind.SshKey);
|
||||
offered.EntityId.ShouldBe(vault.Keys[0].EntityId);
|
||||
|
||||
// A real entry rather than a label: choosing it and saving is what the user came here to do.
|
||||
vault.EditorSelectedAuthentication = offered;
|
||||
await vault.SaveHostCommand.ExecuteAsync(null);
|
||||
|
||||
vault.Hosts.ShouldHaveSingleItem().Host.SshKeyId.ShouldBe(vault.Keys[0].EntityId);
|
||||
vault.Hosts[0].Host.Port.ShouldBe(2244);
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The counterweight, and the reason the refill restores each picker by id rather than reloading the
|
||||
/// stored host: a list that grows under somebody halfway through a form must not move what they had
|
||||
/// already chosen in it, and the save that follows must not rebind the host to something nobody picked.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AnItemArrivingWhileTheHostEditorIsOpen_LeavesItsSelectionWhereItWas()
|
||||
{
|
||||
var vault = await ReadyToConnectAsync();
|
||||
await AddKeyAsync(vault, "deploy");
|
||||
|
||||
var keyId = vault.Keys.ShouldHaveSingleItem().EntityId;
|
||||
|
||||
await BindKeyAsync(vault, vault.Hosts[0], keyId);
|
||||
|
||||
vault.SelectedHost = vault.Hosts[0];
|
||||
vault.EditSelectedHostCommand.Execute(null);
|
||||
|
||||
await AddCredentialAsync(vault, "pg-primary");
|
||||
|
||||
vault.EditorAuthenticationChoices.ShouldContain(
|
||||
choice => choice.Kind == AuthenticationKind.Credential);
|
||||
|
||||
vault.EditorSelectedAuthentication.ShouldNotBeNull().EntityId
|
||||
.ShouldBe(keyId, "a credential appearing must not unbind the host from its key");
|
||||
|
||||
await vault.SaveHostCommand.ExecuteAsync(null);
|
||||
|
||||
vault.Hosts.ShouldHaveSingleItem().Host.SshKeyId.ShouldBe(keyId);
|
||||
vault.Hosts[0].Host.CredentialId.ShouldBeNull();
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// Tags reach the same editor by a different route — the keychain screen rather than the box under the
|
||||
/// chips — and a chip that only appeared on the next open would send the user round the same detour.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task ATagAddedWithTheHostEditorOpen_AppearsAmongItsChips()
|
||||
{
|
||||
await UnlockedAsync();
|
||||
var vault = shell.Vault!;
|
||||
|
||||
await AddHostAsync(vault, "prod-db");
|
||||
vault.SelectedHost = vault.Hosts[0];
|
||||
vault.EditSelectedHostCommand.Execute(null);
|
||||
|
||||
await AddTagAsync(vault, "production");
|
||||
|
||||
vault.HasTagChoices.ShouldBeTrue();
|
||||
|
||||
var chip = vault.EditorTagChoices.ShouldHaveSingleItem();
|
||||
chip.Label.ShouldBe("production");
|
||||
chip.IsWorn.ShouldBeFalse("appearing is not the same as being put on");
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// The group editor shares the drawer with the host editor and its own picker was the same snapshot, so
|
||||
/// the same detour applied to the default binding a whole group of hosts inherits.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task AKeyAddedWithTheGroupEditorOpen_AppearsInItsDefaultBindingPicker()
|
||||
{
|
||||
await UnlockedAsync();
|
||||
var vault = shell.Vault!;
|
||||
|
||||
await AddGroupAsync(vault, "platform");
|
||||
|
||||
vault.SelectedGroup = vault.Groups.ShouldHaveSingleItem();
|
||||
vault.EditGroupCommand.Execute(null);
|
||||
vault.GroupEditorDefaultPort = 2222;
|
||||
|
||||
await AddKeyAsync(vault, "deploy");
|
||||
|
||||
vault.IsEditingGroup.ShouldBeTrue("adding a key must not close the group editor");
|
||||
vault.GroupEditorDefaultPort.ShouldBe(2222, "nor discard what has been typed into it");
|
||||
|
||||
vault.GroupEditorSelectedAuthentication = vault.GroupEditorAuthenticationChoices.Single(
|
||||
choice => string.Equals(choice.Label, "deploy", StringComparison.Ordinal));
|
||||
|
||||
await vault.SaveGroupCommand.ExecuteAsync(null);
|
||||
|
||||
vault.Groups.ShouldHaveSingleItem().Group.DefaultSshKeyId.ShouldBe(vault.Keys[0].EntityId);
|
||||
vault.Groups[0].Group.DefaultPort.ShouldBe(2222);
|
||||
}
|
||||
|
||||
// ---- Binding a key to a host ----
|
||||
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user