diff --git a/README.md b/README.md
index 1ea0717..e136ce2 100644
--- a/README.md
+++ b/README.md
@@ -352,6 +352,24 @@ vault it is leaving. A picker inside the form would do all of that as a side eff
What a move cannot do is reach a machine that has already synced the host, which is the same limit
everything else about revocation has. Keys, passwords and buckets take theirs from a standing "new items go
to" picker on the Keychain screen and cannot be moved yet.
+
+**A group can be moved too, and it takes its contents with it** — MOVE beside EDIT and DELETE over the group
+cards, or "Move to another vault…" on the card's own menu. That is the desktop only, because the phone draws
+groups as headings in the host list and has never had a way to delete or move one. It is the same re-seal
+and tombstone underneath, applied to every item involved: the group, the groups nested inside it, and every
+host filed under any of them, each taking a new id in the destination. Moving less than that was never
+coherent — the machines and the child groups are items of the vault the group is leaving, so a group that
+travelled alone would leave half a shelf behind. What stays is the group it was itself nested under, which
+belongs to the old vault, so it arrives at the top level; the hosts' tags stay for the same reason. Keys and
+passwords are kept, because those genuinely resolve across vaults, and the sentence afterwards names any
+that are now outside the destination.
+
+**Deleting a group asks what should become of the hosts under it.** The default answer keeps them: the
+reference is cleared and they move to UNGROUPED. Ticking the box deletes them with it. Both answers are a
+change — the deletion used to leave the hosts holding an id that no longer resolved, which looked the same
+and cost nothing, and stopped being the right shape once the deletion could take them with it. A group with
+nothing filed under it is not asked. Whichever answer is given, the groups nested inside take the deleted
+group's place in the tree rather than being orphaned to the top level.
Both default to your personal vault and neither moves on its own, because an item put in a shared vault is
visible to everybody holding that vault's key. Choosing a vault in the host editor also decides which groups
it can be filed under: a group is an item like any other and lives in exactly one vault.
@@ -598,6 +616,16 @@ keychain plus a terminal — and the spike that gates all of it.
than to a shell that never opens. Given a walk that had to be cycle-safe anyway, refusing to nest bought
nothing.
+ Membership living on the host has one further consequence, and it took two goes to settle. Deleting a
+ group could not clear it without rewriting every host under the heading, so at first it did not: the hosts
+ kept an id that resolved to nothing and turned up under UNGROUPED, which reads identically and costs no
+ writes. That held until the deletion had to be able to take the hosts *with* it — a group is sometimes a
+ heading being tidied away and sometimes a project that has been decommissioned, and nothing in the code can
+ tell which. Once a deletion knows which hosts it means, leaving them naming something that has gone is a
+ state kept for no reason, so both answers now write: N deletions, or N hosts with the reference cleared.
+ The dangling case still has to be survived everywhere it is read, because a group deleted on *another*
+ machine arrives exactly that way.
+
Inserting a snippet types it at the prompt and stops. Pressing Enter is a per-snippet decision, off by
default, and the reason is worth stating: a terminal is one input stream with no notion of being at a
prompt — the remote may be in an editor, or at a password prompt with the echo off — so this client cannot
diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md
index 027f97d..0292ce6 100644
--- a/docs/design-import-gaps.md
+++ b/docs/design-import-gaps.md
@@ -157,7 +157,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference
> | **Add Telnet**, and **Serial** in the toolbar | Omitted. `ISshConnection` is the only transport there is. This is also why the card subtitle's `ssh` is a constant today rather than a reading — it is stated in `HostRowViewModel.Summary`, which is the one place in this interface where a constant is printed on purpose. |
> | **+ SSH ID, Certificate, FIDO2** | Omitted. `IDENTITIES` and `CERTIFICATES` have been on this document's list since the first import — neither is even a reserved `SyncEntityType` — and there is no security-key path anywhere in the SSH layer. One control offering three item types that do not exist. |
> | The **Backspace / Default** row | Omitted. It is a terminal setting, and the client has no preferences store and no frame to carry one to the renderer — see the Preferences section. It would be a control whose value could not survive the window closing. |
-> | The **chevron beside the vault name** | The name alone, and the move behind the pane's ⋯ menu instead. A host *can* now be moved between vaults, so the gap is no longer that there is nothing to offer — it is that a chevron on a subtitle implies an edit, and this is not one: the two vaults are encrypted under different keys, so it is a re-seal into one and a tombstone in the other, the host takes a new id, and its group and tags stay behind. A control that implied "just change this field" would be describing something else. Where a *new* host goes is still asked in the host editor, as a picker beside the name; keys, passwords and buckets take theirs from the keychain screen's standing picker and cannot be moved yet. |
+> | The **chevron beside the vault name** | The name alone, and the move behind the pane's ⋯ menu instead. A host *can* now be moved between vaults, so the gap is no longer that there is nothing to offer — it is that a chevron on a subtitle implies an edit, and this is not one: the two vaults are encrypted under different keys, so it is a re-seal into one and a tombstone in the other, the host takes a new id, and its group and tags stay behind. A control that implied "just change this field" would be describing something else. Where a *new* host goes is still asked in the host editor, as a picker beside the name. A group moves too, from MOVE over the group cards, and takes its nested groups and every host filed under them; keys, passwords and buckets take theirs from the keychain screen's standing picker and cannot be moved yet. |
> | **Show more ⌄** | Not drawn as a disclosure. What it would hide — notes, the relay switch, forgetting the host key — is in the editor, one press away, and a second fold inside a pane that already scrolls is a second place for a field to be missing from. |
> | **Port Forwarding** in the sidebar | Nothing, for the third time in this document. |
> | The host grid's toolbar avatar, share and tag-filter controls | Omitted, as in v3 and for the same reasons. |
diff --git a/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml b/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml
index 0d08387..93b1e01 100644
--- a/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml
+++ b/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml
@@ -41,6 +41,21 @@
Text="{Binding PendingDeletion.Usage}" />
+
+
+
+
+
diff --git a/src/DodoSSH.Client.App/Views/HostDrawer.axaml b/src/DodoSSH.Client.App/Views/HostDrawer.axaml
index c8a0a04..08dffe6 100644
--- a/src/DodoSSH.Client.App/Views/HostDrawer.axaml
+++ b/src/DodoSSH.Client.App/Views/HostDrawer.axaml
@@ -55,10 +55,11 @@
renderer. They are listed in docs/design-import-gaps.md with what ships instead, and none of them is
drawn disabled.
- The design's fifth missing control was the vault picker's chevron, and half of it now exists: a host
- being *created* is asked which vault it goes into, in the editor below. What still does not exist is
- the other half — moving an existing host — because the two vaults are encrypted under different keys,
- so that is a delete and a retype rather than an edit.
+ The design's fifth missing control was the vault picker's chevron, and both halves of what it stood for
+ now exist without it: a host being *created* is asked which vault it goes into, in the editor below, and
+ an existing one is moved from the pane's ⋯ menu. The chevron itself stays undrawn, because a chevron on
+ a subtitle implies an edit and this is not one — the two vaults are encrypted under different keys, so a
+ move is a re-seal into one and a tombstone in the other, and the host takes a new id.
-->
+
@@ -251,6 +258,42 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs
index 860fbf8..35a971c 100644
--- a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs
+++ b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs
@@ -982,6 +982,27 @@ internal sealed record DeletionRequest(
{
/// Whether anything depends on the item, which is the line worth reading twice.
internal bool HasUsage => Usage.Length > 0;
+
+ ///
+ /// The second question this deletion has to ask, or empty where it has none.
+ ///
+ ///
+ ///
+ /// Only a group has one, and it is the one deletion in this application whose scope is not decided by
+ /// what is being deleted: a group is a heading, and the machines under it may be the reason the heading
+ /// existed or may be forty perfectly good hosts that want a different shelf. Nothing here can tell which,
+ /// so it is asked — see , which is the answer.
+ ///
+ ///
+ /// A sentence rather than a flag, because the card that draws it is shared by six kinds of deletion and
+ /// must not grow a branch per kind. Empty is "there is no second question", which is also what a group
+ /// with nothing filed under it gets.
+ ///
+ ///
+ internal string Choice { get; init; } = string.Empty;
+
+ /// Whether this deletion has a second question to put.
+ internal bool HasChoice => Choice.Length > 0;
}
///
@@ -1540,13 +1561,19 @@ internal sealed partial class VaultViewModel(
///
///
/// The same picker the host editor has, on the form beside it, and for the same reason: this is the
- /// decision that makes the thing shared, it cannot be changed afterwards, and the only other control
- /// that could have answered it is a standing preference on a different screen. A group is where hosts
- /// are filed and what lends them a port, a username and a key — so putting one in a shared vault is how
- /// a team gets an arrangement rather than twenty machines in a heap, which is most of what sharing is
+ /// decision that makes the thing shared, it cannot be changed by saving, and the only other
+ /// control that could have answered it is a standing preference on a different screen. A group is where
+ /// hosts are filed and what lends them a port, a username and a key — so putting one in a shared vault is
+ /// how a team gets an arrangement rather than twenty machines in a heap, which is most of what sharing is
/// for.
///
///
+ /// Changed afterwards it can be, by , which is a separate act for the reason
+ /// moving a host is: it re-seals the group, everything nested inside it and every host filed under them
+ /// into a second vault's key, and nothing that happens as a side effect of pressing SAVE on a form should
+ /// be that.
+ ///
+ ///
/// Filled from , so it offers what every other "file this into" control does:
/// vaults this session can both read and write.
///
@@ -1984,6 +2011,48 @@ internal sealed partial class VaultViewModel(
SelectedHost is { IsReadOnly: false } row
&& session.ReadableVaults.Any(vault => vault.CanWrite && vault.VaultId != row.VaultId);
+ ///
+ /// Whether the panel asking which vault to move the group to is up.
+ ///
+ ///
+ /// The host's panel again — see — in the one place a group can be acted on,
+ /// which is the GROUPS section of the hosts screen rather than the drawer. It takes that section's two
+ /// buttons the way the deletion question does, and for the same reason: a MOVE that could be pressed
+ /// again while its own picker was open would be a second panel about the same shelf.
+ ///
+ [ObservableProperty]
+ [NotifyPropertyChangedFor(nameof(ShowsGroupActions))]
+ private bool isMovingGroup;
+
+ /// Which group the open move panel is about. Null when it is closed.
+ ///
+ private Guid? movingGroupId;
+
+ /// Where the group could be moved: every vault this session can write to but its own.
+ internal ObservableCollection MoveGroupVaultChoices { get; } = [];
+
+ [ObservableProperty]
+ private VaultChoiceViewModel? selectedMoveGroupVault;
+
+ ///
+ /// Whether the group the buttons are aimed at has anywhere to move to.
+ ///
+ ///
+ ///
+ /// Counted per group rather than asked once of the session, exactly as
+ /// is: a group already in the only other writable vault has nowhere to go.
+ ///
+ ///
+ /// The read-only test is the group's own and not the whole subtree's. That is deliberate rather than
+ /// sloppy — this decides whether a button is worth drawing, and walking every descendant and every host
+ /// under them to answer it would be that walk on every selection. The subtree is checked once, where it
+ /// decides something, in .
+ ///
+ ///
+ internal bool CanMoveGroupTarget =>
+ GroupTarget is { IsReadOnly: false } row
+ && session.ReadableVaults.Any(vault => vault.CanWrite && vault.VaultId != row.VaultId);
+
///
/// What the drawer's header says it is about.
///
@@ -2004,10 +2073,12 @@ internal sealed partial class VaultViewModel(
///
///
///
- /// The vault's name and not a picker for it, although the design draws one with a chevron. An item
- /// cannot be moved between vaults here — that is a delete and a retype, because the two are encrypted
- /// under different keys — so a control offering the move would be offering something no layer below
- /// this can do. Where a *new* item goes is chosen in the editor's own picker; see
+ /// The vault's name and not a picker for it, although the design draws one with a chevron. A host and a
+ /// group can both be moved between vaults — see and — but
+ /// not from here and not by saving: the two vaults are encrypted under different keys, so a move is a
+ /// re-seal into one and a tombstone in the other, and every item involved takes a new id. A chevron on a
+ /// subtitle implies an edit and would be describing something else. Where a *new* item goes is chosen in
+ /// the editor's own picker; see
///
///
/// A group being renamed says its vault here for the same reason a host being edited does, and it is
@@ -2559,6 +2630,26 @@ internal sealed partial class VaultViewModel(
[ObservableProperty]
private DeletionRequest? pendingDeletion;
+ ///
+ /// The answer to : whether a group's hosts go with it.
+ ///
+ ///
+ ///
+ /// Off is the answer that keeps the machines, and it is off by default and reset to off on every
+ /// question — see . A tick left standing from the last group
+ /// deleted would delete forty hosts on behalf of somebody who was only tidying a heading away, and there
+ /// is no undo on either side of it.
+ ///
+ ///
+ /// A tick rather than a pair of options, and deliberately not two equally weighted answers: they are not
+ /// equally weighted. Keeping the hosts is recoverable — they turn up under UNGROUPED and can be filed
+ /// again — and deleting them is not, so the safe answer is the one that needs no decision and the
+ /// destructive one is the one that has to be reached for.
+ ///
+ ///
+ [ObservableProperty]
+ private bool deletionTakesTheHostsToo;
+
internal bool IsConfirmingDeletion => PendingDeletion is not null;
/// Whether the sidebar's row of host buttons is showing.
@@ -2598,11 +2689,17 @@ internal sealed partial class VaultViewModel(
/// Whether the group panel's buttons are showing.
///
- /// Hidden with nothing to act on as well as while the question is up. A pair of buttons over a grid of
- /// cards nobody has chosen between is a pair whose subject the user has to work out, and the answer
- /// would be "neither" — see .
+ /// Hidden with nothing to act on as well as while the question is up. A row of buttons over a grid of
+ /// cards nobody has chosen between is a row whose subject the user has to work out, and the answer
+ /// would be "none of them" — see .
+ ///
+ /// The move panel takes the row as well, on the same reasoning the host pane's menu hides for it: both
+ /// panels are drawn where the buttons were, and leaving them there would offer to open a panel that is
+ /// already open.
+ ///
///
- internal bool ShowsGroupActions => GroupTarget is not null && !IsConfirmingGroupDeletion;
+ internal bool ShowsGroupActions =>
+ GroupTarget is not null && !IsConfirmingGroupDeletion && !IsMovingGroup;
/// Whether this vault has any groups, which is what makes the sidebar draw headings.
internal bool HasGroups => Groups.Count > 0;
@@ -4768,9 +4865,10 @@ internal sealed partial class VaultViewModel(
EditingGroupId = row.EntityId;
- // The group's own vault, and it does not move — the same rule an existing host's follows, and the
+ // The group's own vault, and no save moves it — the same rule an existing host's follows, and the
// same reason: the two are encrypted under different keys, so saving anywhere else would leave a
- // copy behind rather than move anything. The picker is not drawn for an existing group at all.
+ // copy behind rather than move anything. The picker is not drawn for an existing group at all, and
+ // the move that does work is MoveGroup, from the buttons over the cards.
editingGroupVaultId = row.VaultId;
editingGroupVaultName = row.HasVaultBadge ? row.VaultName : string.Empty;
@@ -4997,9 +5095,10 @@ internal sealed partial class VaultViewModel(
/// Moves a half-typed group into the vault just chosen for it.
///
///
- /// Only while creating, for the reason gives: a group cannot
- /// be moved between vaults, so a path that reassigned this on a rename would write a second group into
- /// the other vault and leave the original standing with the old name.
+ /// Only while creating, for the reason gives: a save cannot
+ /// move a group between vaults, so a path that reassigned this on a rename would write a second group
+ /// into the other vault and leave the original standing with the old name. Moving one is
+ /// , which does the re-seal and the tombstone this could not.
///
partial void OnGroupEditorSelectedVaultChanged(VaultChoiceViewModel? value)
{
@@ -5103,12 +5202,28 @@ internal sealed partial class VaultViewModel(
Status = string.Empty;
}
- /// Asks whether the group being acted on should go.
+ /// Asks whether the group being acted on should go, and what should become of its hosts.
///
- /// The count is the whole reason this asks rather than acting. Deleting a group does not delete the hosts
- /// in it and deliberately does not rewrite them either — they keep an id that no longer resolves and turn
- /// up under the ungrouped heading — so what the user needs to know is exactly how many machines are about
- /// to move, and that none of them are going anywhere else.
+ ///
+ /// The hosts are a second question rather than a stated consequence, and that is a reversal. This
+ /// used to say what would happen to them — they stay, and turn up under UNGROUPED — because a group's
+ /// deletion did not touch them at all. That answer was right for one of the two things people delete a
+ /// group for and wrong for the other: a heading being tidied away should leave its machines alone, and a
+ /// project that has been decommissioned is a shelf and everything on it. Nothing here can tell which of
+ /// the two it is looking at, so it is asked. See .
+ ///
+ ///
+ /// The count is still the reason this asks rather than acting, and it now counts twice over: it is the
+ /// number of machines about to be unfiled, or — with the box ticked — the number about to be destroyed.
+ /// A group with nothing under it gets no second question and no tick, because there is nothing for
+ /// either to be about.
+ ///
+ ///
+ /// Refused with a host editor open, which the deletion of a single host is not. The difference is
+ /// that this one writes to hosts: unfiling rewrites every machine under the heading, and doing that
+ /// beneath a half-typed edit of one of them is the save nobody asked for that
+ /// refuses for the same reason.
+ ///
///
/// Aims where EDIT does: at the selected card, or at the open group when no card is selected. See
/// .
@@ -5122,6 +5237,17 @@ internal sealed partial class VaultViewModel(
return;
}
+ // Only where it would write to a host. A heading with nothing under it is deleted with an editor
+ // open, because nothing on that form is about to be rewritten underneath it.
+ if (row.HostCount > 0 && AHostEditorIsInTheWay())
+ {
+ return;
+ }
+
+ // Disarms a move aimed at the same group, on the reasoning MoveHost uses in the other direction: two
+ // panels about one shelf, one of which destroys it, is not something to make anybody read carefully.
+ CancelMoveGroupCommand.Execute(null);
+
PendingDeletion = new DeletionRequest(
DeletionTarget.Group,
row.EntityId,
@@ -5136,13 +5262,52 @@ internal sealed partial class VaultViewModel(
row.HostCount switch
{
0 => string.Empty,
- 1 => "1 host is filed under it. The host stays; it moves to UNGROUPED.",
- _ => $"{row.HostCount} hosts are filed under it. They stay; they move to UNGROUPED.",
- });
+ 1 => "1 host is filed under it. Left as it is, the host stays and moves to UNGROUPED.",
+ _ => $"{row.HostCount} hosts are filed under it. Left as it is, they stay and move to "
+ + "UNGROUPED.",
+ })
+ {
+ Choice = row.HostCount switch
+ {
+ 0 => string.Empty,
+ 1 => "Delete the host filed under it as well.",
+ _ => $"Delete the {row.HostCount} hosts filed under it as well.",
+ },
+ };
}
- /// Queues a tombstone for the group that was agreed to.
- private async Task DeleteGroupNowAsync(Guid entityId, CancellationToken cancellationToken)
+ ///
+ /// Queues the tombstone for the group that was agreed to, and settles what pointed at it.
+ ///
+ ///
+ ///
+ /// Nothing is left pointing at the group. That is the change: the hosts filed under it are
+ /// rewritten — unfiled, or deleted where that was asked for — and the groups nested inside it take its
+ /// place in the tree rather than being orphaned into roots. It costs one write per item and it is the
+ /// honest cost of the question above it. The dangling reference the list used to absorb is still handled
+ /// everywhere it is read, because a group deleted on another machine still arrives that way.
+ ///
+ ///
+ /// Everything that points at it is written before the group's own tombstone. A crash in the middle
+ /// then leaves a heading standing over fewer things, which is visible and can simply be deleted again;
+ /// the other order leaves a group gone with its members still naming it, which is exactly the state this
+ /// exists to stop producing. It also matters against a merge: the group's tombstone can lose one, and if
+ /// it does, the unfiling has already been recorded on its own items rather than riding on it.
+ ///
+ ///
+ /// A read-only host is skipped and counted rather than rewritten. Unfiling it would re-encode a
+ /// payload this build cannot fully represent, which is the same refusal editing and moving one already
+ /// make — and the cost of skipping is a dangling id, which is the behaviour every reader here already
+ /// survives. Deleting one is not skipped: a tombstone re-encodes nothing.
+ ///
+ ///
+ /// The group.
+ /// Whether the hosts filed under it were agreed to go too.
+ /// Cancellation token.
+ private async Task DeleteGroupNowAsync(
+ Guid entityId,
+ bool takesTheHosts,
+ CancellationToken cancellationToken)
{
if (Groups.FirstOrDefault(row => row.EntityId == entityId) is not { } row)
{
@@ -5150,10 +5315,24 @@ internal sealed partial class VaultViewModel(
return;
}
+ // Its own vault's, both of them. A host in another vault naming this group is a reference the editor
+ // cannot make and the drop gesture refuses, so the only way to hold one is a hand-edited payload —
+ // and rewriting somebody else's vault because a group in this one went is worse than the dangle.
+ var filed = Hosts
+ .Where(host => host.VaultId == row.VaultId && host.Host.GroupId == entityId)
+ .ToList();
+
+ var nested = Groups
+ .Where(group => group.VaultId == row.VaultId && group.Group.ParentId == entityId)
+ .ToList();
+
await RunAsync(
"Deleting…",
async () =>
{
+ var skipped = await ReleaseWhatPointedAtAsync(row, filed, nested, takesTheHosts, cancellationToken)
+ .ConfigureAwait(true);
+
// The row's own vault, which is why the row is re-found above rather than the id being
// enough: a tombstone written to the active vault would delete nothing and leave a
// colleague's group standing while this machine reported it gone.
@@ -5168,12 +5347,91 @@ internal sealed partial class VaultViewModel(
}
await ReloadAsync(cancellationToken).ConfigureAwait(true);
- Status = $"Deleted the group '{row.Label}'.";
+
+ Status = $"Deleted the group '{row.Label}'."
+ + WhatBecameOfTheHosts(filed.Count, takesTheHosts, skipped);
}).ConfigureAwait(true);
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
}
+ ///
+ /// Rewrites everything naming the group about to go, and says how many could not be.
+ ///
+ ///
+ /// Before the tombstone and in this order, which gives the reasons for.
+ ///
+ /// How many hosts were left holding the reference because they could not be rewritten.
+ private async Task ReleaseWhatPointedAtAsync(
+ HostGroupRowViewModel row,
+ IEnumerable filed,
+ IEnumerable nested,
+ bool takesTheHosts,
+ CancellationToken cancellationToken)
+ {
+ var skipped = 0;
+
+ foreach (var host in filed)
+ {
+ if (takesTheHosts)
+ {
+ await session.Hosts
+ .DeleteAsync(host.VaultId, host.EntityId, cancellationToken)
+ .ConfigureAwait(true);
+
+ continue;
+ }
+
+ if (host.IsReadOnly)
+ {
+ skipped++;
+ continue;
+ }
+
+ await session.Hosts
+ .UpdateAsync(
+ host.VaultId,
+ host.EntityId,
+ host.Host with { GroupId = null },
+ cancellationToken)
+ .ConfigureAwait(true);
+ }
+
+ // Promoted to where the group they were under sat, rather than to the top level. A nested group whose
+ // parent goes has not been moved by anybody, and dropping it to a root would rearrange a tree on a
+ // delete that was about one heading.
+ foreach (var child in nested.Where(child => !child.IsReadOnly))
+ {
+ await session.HostGroups
+ .UpdateAsync(
+ child.VaultId,
+ child.EntityId,
+ child.Group with { ParentId = row.Group.ParentId },
+ cancellationToken)
+ .ConfigureAwait(true);
+ }
+
+ return skipped;
+ }
+
+ /// What the group's deletion did to the machines under it, said only when there were any.
+ ///
+ /// The skipped count is named rather than folded into the total, because those hosts are the ones still
+ /// holding the deleted group's id: they show up under UNGROUPED like the rest, so nothing looks wrong,
+ /// and the sentence is the only place anybody is told that updating this client is what finishes the job.
+ ///
+ private static string WhatBecameOfTheHosts(int filed, bool takesTheHosts, int skipped) =>
+ (filed, takesTheHosts, skipped) switch
+ {
+ (0, _, _) => string.Empty,
+ (1, true, _) => " Its host went with it.",
+ (_, true, _) => $" Its {filed} hosts went with it.",
+ (1, false, 0) => " Its host moved to UNGROUPED.",
+ (_, false, 0) => $" Its {filed} hosts moved to UNGROUPED.",
+ _ => $" Its {filed} hosts moved to UNGROUPED, except {skipped} written by a newer version of "
+ + "DodoSSH — those still name the group that has gone. Update to unfile them.",
+ };
+
/// Abandons the editor.
[RelayCommand]
private void CancelEdit()
@@ -5457,11 +5715,7 @@ internal sealed partial class VaultViewModel(
{
MoveVaultChoices.Clear();
- foreach (var choice in session.ReadableVaults
- .Where(vault => vault.CanWrite && vault.VaultId != vaultId)
- .OrderByDescending(vault => vault.IsPersonal)
- .ThenBy(vault => vault.Name, StringComparer.CurrentCulture)
- .Select(vault => new VaultChoiceViewModel(vault.VaultId, vault.Name, vault.IsPersonal)))
+ foreach (var choice in WritableVaultsBesides(vaultId))
{
MoveVaultChoices.Add(choice);
}
@@ -5469,6 +5723,357 @@ internal sealed partial class VaultViewModel(
SelectedMoveVault = MoveVaultChoices.FirstOrDefault();
}
+ /// Every vault this session can write to except one, in the order a picker should offer them.
+ ///
+ /// Shared by the host's picker and the group's rather than written twice. The order is the one every
+ /// vault picker in this application uses: your own first, then the shared ones by name — a list that
+ /// re-sorted itself per control would make "the second entry" mean something different on each.
+ ///
+ private IEnumerable WritableVaultsBesides(Guid vaultId) =>
+ session.ReadableVaults
+ .Where(vault => vault.CanWrite && vault.VaultId != vaultId)
+ .OrderByDescending(vault => vault.IsPersonal)
+ .ThenBy(vault => vault.Name, StringComparer.CurrentCulture)
+ .Select(vault => new VaultChoiceViewModel(vault.VaultId, vault.Name, vault.IsPersonal));
+
+ ///
+ /// Opens the panel that asks which vault the group — and everything under it — should move to.
+ ///
+ ///
+ ///
+ /// The host's panel, one level up, and the reason it exists at all is that the host's did not go far
+ /// enough. Moving twenty machines into a shared vault one at a time meant twenty round trips through a
+ /// menu, and each one arrived stripped of the group it had been filed under, so the shelf had to be
+ /// rebuilt by hand on the other side. Moving the shelf is the operation people were actually attempting.
+ ///
+ ///
+ /// Refused for a group written by a newer client, as editing one is, and refused with a host editor open,
+ /// as is: this rewrites hosts.
+ ///
+ ///
+ [RelayCommand]
+ private void MoveGroup()
+ {
+ if (GroupTarget is not { } row || AHostEditorIsInTheWay() || AGroupEditorIsInTheWay())
+ {
+ return;
+ }
+
+ if (row.IsReadOnly)
+ {
+ Status = "This group was written by a newer version of DodoSSH. Moving it would re-encode it "
+ + "here and lose what this build cannot read. Update first.";
+ return;
+ }
+
+ BuildMoveGroupVaultChoices(row.VaultId);
+
+ if (MoveGroupVaultChoices.Count == 0)
+ {
+ Status = $"There is nowhere to move '{row.Label}' to: this is the only vault you can write to.";
+ return;
+ }
+
+ // As MoveHost disarms a deletion aimed at the same host.
+ PendingDeletion = null;
+ movingGroupId = row.EntityId;
+ IsMovingGroup = true;
+ Status = string.Empty;
+ }
+
+ /// Abandons the group's move panel.
+ [RelayCommand]
+ private void CancelMoveGroup()
+ {
+ if (!IsMovingGroup)
+ {
+ return;
+ }
+
+ IsMovingGroup = false;
+ movingGroupId = null;
+ MoveGroupVaultChoices.Clear();
+ SelectedMoveGroupVault = null;
+ Status = string.Empty;
+ }
+
+ ///
+ /// Moves the group, everything nested inside it and every host filed under any of them.
+ ///
+ ///
+ ///
+ /// The whole subtree goes, and taking less than that was never coherent. A group's children are
+ /// items of the vault it is leaving: move the parent alone and they are left naming a tombstone, so they
+ /// surface as roots in the vault the user has just emptied — half a shelf here and half there, from one
+ /// gesture that said "move this". The hosts are the same argument and are the half the user asked about.
+ ///
+ ///
+ /// The groups go first, top down, and the hosts last. Each item is re-sealed under the
+ /// destination's key and takes a new id — see VaultItemRepository.MoveAsync — so nothing that
+ /// points at a group can be written until that group has landed and its new id is known. Top down for
+ /// the same reason one level up: a child's parent must already be over there. What an interruption
+ /// leaves is therefore hosts still in the vault they started in, under UNGROUPED, which is visible and
+ /// re-movable; the reverse order would leave hosts in the destination filed under nothing.
+ ///
+ ///
+ /// The parent is left behind and the tags are dropped, and both are the same rule the host's move
+ /// follows: a parent group and a tag are items of the vault being left, so a reference carried across
+ /// would resolve on this machine — groups and tags resolve over every readable vault — and dangle for
+ /// everybody else in the destination. The moved group becomes a root, which is what the trail will show,
+ /// and it is said in the sentence afterwards rather than discovered.
+ ///
+ ///
+ /// Keys and passwords are kept, on the host and on the group's own defaults, because those do
+ /// genuinely resolve across vaults — one key on twenty hosts in three vaults is the arrangement they
+ /// exist for. What is reported is a binding now outside the destination, since that is precisely what
+ /// the other holders of it will not be able to resolve.
+ ///
+ ///
+ [RelayCommand]
+ private async Task ConfirmMoveGroupAsync(CancellationToken cancellationToken)
+ {
+ if (GroupTarget is not { } row
+ || movingGroupId != row.EntityId
+ || SelectedMoveGroupVault is not { } target)
+ {
+ return;
+ }
+
+ var subtree = SubtreeOf(row);
+ var moving = subtree.Select(group => group.EntityId).ToHashSet();
+
+ var filed = Hosts
+ .Where(host => host.VaultId == row.VaultId && host.Host.GroupId is { } id && moving.Contains(id))
+ .ToList();
+
+ // Asked once, over everything that is about to be re-encoded, and refused as a whole rather than
+ // half-done: a move that skipped the items it could not represent would file some of the shelf in one
+ // vault and leave the rest in the other, which is the state this operation exists to prevent.
+ if (subtree.Any(group => group.IsReadOnly) || filed.Any(host => host.IsReadOnly))
+ {
+ Status = "Something under this group was written by a newer version of DodoSSH. Moving it would "
+ + "re-encode it here and lose what this build cannot read. Update first.";
+ return;
+ }
+
+ var name = target.Name;
+ var stranded = BindingsOutside(subtree, filed, target.VaultId);
+ var uprooted = row.Group.ParentId is not null;
+ var tagged = filed.Count(host => host.Host.TagIds.Count > 0);
+
+ CancelMoveGroupCommand.Execute(null);
+
+ await RunAsync(
+ "Moving…",
+ async () =>
+ {
+ var landed = await ReSealTheSubtreeAsync(subtree, filed, target.VaultId, cancellationToken)
+ .ConfigureAwait(true);
+
+ await ReloadAsync(cancellationToken).ConfigureAwait(true);
+
+ // By its new id, as a moved host's pane is: leaving the buttons aimed at a card that no
+ // longer exists would read as the shelf having been deleted rather than moved.
+ SelectedGroup = VisibleGroups.FirstOrDefault(card => card.EntityId == landed);
+
+ Status = $"Moved '{row.Label}' to {name}.{WhatCameAlong(subtree.Count, filed.Count)}"
+ + $"{WhatStayedBehind(uprooted, tagged)}{stranded}";
+ }).ConfigureAwait(true);
+
+ await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
+ }
+
+ ///
+ /// Re-seals a group, its nested groups and their hosts under another vault's key.
+ ///
+ ///
+ /// The order and what it costs an interruption are 's to explain. What
+ /// lives here is the id map every write after the first depends on: each item lands with an id of the
+ /// destination's making, so a parent's is looked up rather than reused, and a host's group is the entry
+ /// its old group left behind.
+ ///
+ /// The group and its nested groups, each after its parent.
+ /// The hosts under any of them.
+ /// The vault they are all going to.
+ /// Cancellation token.
+ /// The id the group at the root of it has in its new vault.
+ private async Task ReSealTheSubtreeAsync(
+ List subtree,
+ IEnumerable filed,
+ Guid vaultId,
+ CancellationToken cancellationToken)
+ {
+ var root = subtree[0].EntityId;
+ var remapped = new Dictionary();
+
+ foreach (var group in subtree)
+ {
+ // Null for the root, whose parent is staying behind in the vault it came from; every other group
+ // in the list was discovered by its own parent, so that parent has already landed.
+ var parent = group.EntityId == root
+ ? (Guid?)null
+ : remapped[group.Group.ParentId!.Value];
+
+ remapped[group.EntityId] = await session.HostGroups
+ .MoveAsync(
+ group.VaultId,
+ vaultId,
+ group.EntityId,
+ group.Group with { ParentId = parent },
+ cancellationToken)
+ .ConfigureAwait(true);
+ }
+
+ foreach (var host in filed)
+ {
+ await session.Hosts
+ .MoveAsync(
+ host.VaultId,
+ vaultId,
+ host.EntityId,
+ host.Host with
+ {
+ GroupId = remapped[host.Host.GroupId!.Value],
+ TagIds = TagSet.Empty,
+ },
+ cancellationToken)
+ .ConfigureAwait(true);
+ }
+
+ return remapped[root];
+ }
+
+ ///
+ /// The group and every group nested under it, each one after the group it hangs from.
+ ///
+ ///
+ ///
+ /// A breadth-first walk from the root, and the order is the whole reason it is one: every group but the
+ /// first is discovered by its parent, so a caller writing the list in order always has the
+ /// parent's new id in hand before it needs it.
+ ///
+ ///
+ /// Its own vault's only, because a child in another vault is a level half the readers cannot resolve and
+ /// the editor refuses to make one.
+ ///
+ ///
+ /// Cycle-safe, and it has to be. A group already found is not walked again and not added twice —
+ /// the same visited set every walk over this tree carries, for the reason
+ /// gives: two offline clients can each re-parent one group under
+ /// the other, and the pair that results was never shown to an editor. Without the set, a group that is
+ /// its own parent would be appended to this list for as long as there was memory to append to.
+ ///
+ ///
+ private List SubtreeOf(HostGroupRowViewModel root)
+ {
+ var ordered = new List { root };
+ var found = new HashSet { root.EntityId };
+
+ for (var index = 0; index < ordered.Count; index++)
+ {
+ var parent = ordered[index].EntityId;
+
+ foreach (var child in Groups.Where(
+ group => group.VaultId == root.VaultId && group.Group.ParentId == parent))
+ {
+ if (found.Add(child.EntityId))
+ {
+ ordered.Add(child);
+ }
+ }
+ }
+
+ return ordered;
+ }
+
+ /// What the move brought with it, said as the counts the user can check against the cards.
+ private static string WhatCameAlong(int groups, int hosts)
+ {
+ var nested = groups switch
+ {
+ 1 => string.Empty,
+ 2 => " with the group inside it",
+ _ => $" with the {groups - 1} groups inside it",
+ };
+
+ return hosts switch
+ {
+ 0 when groups == 1 => string.Empty,
+ 0 => $" It went{nested}, and no hosts were filed under any of them.",
+ 1 => $" Its host came too{nested}.",
+ _ => $" Its {hosts} hosts came too{nested}.",
+ };
+ }
+
+ /// The two things a group cannot take across, said only where it had one.
+ private static string WhatStayedBehind(bool uprooted, int tagged) =>
+ (uprooted, tagged > 0) switch
+ {
+ (true, true) => " The group it was nested under stayed behind and the hosts' tags were dropped —"
+ + " both belong to the vault it came from, so it now sits at the top level.",
+ (true, false) => " The group it was nested under stayed behind — a parent belongs to the vault it"
+ + " is in — so it now sits at the top level.",
+ (false, true) => " The hosts' tags were left behind — a tag belongs to the vault it is in.",
+ _ => string.Empty,
+ };
+
+ ///
+ /// The warning about keys or passwords that are not in the vault the group has moved to.
+ ///
+ ///
+ /// Counted and named to one, where names the only one there can be. A shelf
+ /// of twenty machines may strand five different keys, and twenty sentences is not a status line — but a
+ /// bare number is not actionable either, so the first is named and the rest are counted. Both the hosts'
+ /// own bindings and the groups' defaults are asked, because a group that lends a key is the one case
+ /// where a machine can be unable to connect without naming anything itself.
+ ///
+ private string BindingsOutside(
+ IEnumerable groups,
+ IEnumerable hosts,
+ Guid vaultId)
+ {
+ var stranded = groups
+ .SelectMany(group => new[] { group.Group.DefaultSshKeyId, group.Group.DefaultCredentialId })
+ .Concat(hosts.SelectMany(host => new[] { host.Host.SshKeyId, host.Host.CredentialId }))
+ .OfType()
+ .Distinct()
+ .Select(LabelOfBindingOutside)
+ .OfType()
+ .ToList();
+
+ return stranded.Count switch
+ {
+ 0 => string.Empty,
+ 1 => $" It still authenticates with '{stranded[0]}', which is in another vault — everybody else "
+ + "in this one will find that binding unresolvable.",
+ _ => $" It still authenticates with '{stranded[0]}' and {stranded.Count - 1} other key(s) or "
+ + "password(s) in other vaults — everybody else in this one will find those bindings "
+ + "unresolvable.",
+ };
+
+ string? LabelOfBindingOutside(Guid entityId) =>
+ (Keys.FirstOrDefault(key => key.EntityId == entityId) is { } key && key.VaultId != vaultId
+ ? key.Label
+ : null)
+ ?? (Credentials.FirstOrDefault(row => row.EntityId == entityId) is { } credential
+ && credential.VaultId != vaultId
+ ? credential.Label
+ : null);
+ }
+
+ /// Fills the group move panel's picker with every vault this session can write to but that one.
+ private void BuildMoveGroupVaultChoices(Guid vaultId)
+ {
+ MoveGroupVaultChoices.Clear();
+
+ foreach (var choice in WritableVaultsBesides(vaultId))
+ {
+ MoveGroupVaultChoices.Add(choice);
+ }
+
+ SelectedMoveGroupVault = MoveGroupVaultChoices.FirstOrDefault();
+ }
+
/// Asks whether the selected host should go.
///
/// A terminal already open on the host is disclosed rather than prevented, because deleting a host does
@@ -6314,6 +6919,10 @@ internal sealed partial class VaultViewModel(
return;
}
+ // Read before the disarming below, which resets it. The answer belongs to the question that was on
+ // screen, and taking it afterwards would read whatever the next question starts from.
+ var takesTheHosts = DeletionTakesTheHostsToo;
+
PendingDeletion = null;
switch (request.Target)
@@ -6331,7 +6940,8 @@ internal sealed partial class VaultViewModel(
break;
case DeletionTarget.Group:
- await DeleteGroupNowAsync(request.EntityId, cancellationToken).ConfigureAwait(true);
+ await DeleteGroupNowAsync(request.EntityId, takesTheHosts, cancellationToken)
+ .ConfigureAwait(true);
break;
case DeletionTarget.ObjectStore:
@@ -8065,9 +8675,11 @@ internal sealed partial class VaultViewModel(
}
DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId);
+ CloseTheGroupMovePanelIfAimedElsewhere();
OnPropertyChanged(nameof(GroupTarget));
OnPropertyChanged(nameof(ShowsGroupActions));
+ OnPropertyChanged(nameof(CanMoveGroupTarget));
}
///
@@ -8086,16 +8698,38 @@ internal sealed partial class VaultViewModel(
partial void OnGroupFilterChanged(HostGroupRowViewModel? value)
{
DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId);
+ CloseTheGroupMovePanelIfAimedElsewhere();
OnPropertyChanged(nameof(GroupTarget));
OnPropertyChanged(nameof(ShowsGroupActions));
+ OnPropertyChanged(nameof(CanMoveGroupTarget));
RebuildGroupLevel();
RebuildVisibleHosts();
}
+ /// Folds the group's move panel away once the buttons under it point at something else.
+ ///
+ /// By entity id and not by row, for the reason compares ids: every
+ /// row object in the list is replaced on every reload, so a panel closed on row identity would fold up
+ /// once a minute under somebody who was still choosing a vault in it. It is cleared rather than re-aimed,
+ /// because which vault to move to is a choice about the shelf it was asked for.
+ ///
+ private void CloseTheGroupMovePanelIfAimedElsewhere()
+ {
+ if (IsMovingGroup && movingGroupId != GroupTarget?.EntityId)
+ {
+ CancelMoveGroupCommand.Execute(null);
+ }
+ }
+
partial void OnPendingDeletionChanged(DeletionRequest? value)
{
+ // Back to "keep them" on every question, including the one that disarms it. A tick is the answer to
+ // the group that was named in the sentence above it and to nothing else; carried into the next
+ // question it would delete a second group's machines on the strength of a decision about the first.
+ DeletionTakesTheHostsToo = false;
+
OnPropertyChanged(nameof(IsConfirmingDeletion));
OnPropertyChanged(nameof(IsConfirmingHostDeletion));
OnPropertyChanged(nameof(IsConfirmingGroupDeletion));
diff --git a/src/DodoSSH.Client.Sync/HostGroupRepository.cs b/src/DodoSSH.Client.Sync/HostGroupRepository.cs
index 8a3b05f..aacdd3b 100644
--- a/src/DodoSSH.Client.Sync/HostGroupRepository.cs
+++ b/src/DodoSSH.Client.Sync/HostGroupRepository.cs
@@ -11,11 +11,19 @@ namespace DodoSSH.Client.Sync;
/// The fifth facade over the same generic repository, and like the fourth it needed no new sync logic at all.
///
///
-/// Deleting a group does not touch the hosts in it. There is deliberately no DeleteAsync
-/// overload that unfiles its members: one user action would become N host writes, N outbox rows and N chances
-/// to merge against an edit nobody made, and the group's own tombstone can still lose a merge — by which time
-/// the membership it was clearing is gone. Hosts left holding a dangling id fall under the ungrouped heading,
-/// which is where the interface handles it. See .
+/// Deleting a group still does not touch the hosts in it, here. There is deliberately no
+/// DeleteAsync overload that unfiles its members: one call would become N host writes, N outbox rows
+/// and N chances to merge against an edit nobody made, and the group's own tombstone can still lose a merge —
+/// by which time the membership it was clearing would be gone. A host left holding a dangling id falls under
+/// the ungrouped heading, so this layer is safe whatever happens above it. See
+/// .
+///
+///
+/// What changed above it is that the deletion is no longer silent about the choice: the interface asks
+/// whether the hosts should go too, and writes them one by one through HostRepository either way —
+/// as N deletions, or as N hosts with the reference cleared. That is the same N writes, made where somebody
+/// asked for them and where the count can be shown before the fact rather than issued by a repository call
+/// that reads like one delete.
///
///
public sealed class HostGroupRepository(
@@ -48,6 +56,25 @@ public sealed class HostGroupRepository(
CancellationToken cancellationToken) =>
groups.UpdateAsync(vaultId, entityId, group, cancellationToken);
+ ///
+ ///
+ ///
+ ///
+ /// The new id matters more for a group than for anything else that can be moved, because a group is the
+ /// one item other items point at. Everything naming the old id — the hosts filed under it, the groups
+ /// nested inside it — has to be rewritten with the id this returns, or it is left pointing at a
+ /// tombstone. That rewriting is the caller's, for the reason the secret parameter is: this layer
+ /// cannot know which references cross with the group and which stay behind.
+ ///
+ ///
+ public Task MoveAsync(
+ Guid fromVaultId,
+ Guid toVaultId,
+ Guid entityId,
+ HostGroupSecret group,
+ CancellationToken cancellationToken) =>
+ groups.MoveAsync(fromVaultId, toVaultId, entityId, group, cancellationToken);
+
///
public Task DeleteAsync(Guid vaultId, Guid entityId, CancellationToken cancellationToken) =>
groups.DeleteAsync(vaultId, entityId, cancellationToken);
diff --git a/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs
index c9633cb..cb2ca1e 100644
--- a/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs
+++ b/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs
@@ -557,9 +557,11 @@ public sealed class ScreenLayoutTests : IAsyncLifetime
}
///
- /// The question replaces the group's two buttons rather than stacking under them — the same rule every
- /// other pair in this application follows — and it is the taller of the two, because it says how many
- /// hosts are about to move.
+ /// The question replaces the group's row of buttons rather than stacking under it — the same rule every
+ /// other row in this application follows — and it is the tallest thing this section draws: a heading, a
+ /// consequence, a boxed count, and now a tick with a sentence beside it asking whether the machines go
+ /// too. The tick is the part worth measuring, because it is a wrapping paragraph inside a control whose
+ /// own height the layout does not obviously account for.
///
[Fact]
public async Task TheHostsScreenFitsWhileAGroupDeletionIsBeingConfirmed()
@@ -570,10 +572,43 @@ public sealed class ScreenLayoutTests : IAsyncLifetime
vault.DeleteGroupCommand.Execute(null);
vault.IsConfirmingGroupDeletion.ShouldBeTrue("the question has to be up for this to measure it");
+ vault.PendingDeletion.ShouldNotBeNull().HasChoice
+ .ShouldBeTrue("the hosts filed under it are what makes this the long shape");
await MeasureHostsAsync(faults => faults.ShouldBeEmpty("with the group question up"));
}
+ ///
+ ///
+ /// The group's move panel, which takes that same row of buttons and is the wordier of the two: a
+ /// heading, a combo box, a wrapping paragraph naming everything that travels and everything that does
+ /// not, and two buttons — above a wrap of group cards and the host grid, all of which still have to fit
+ /// under it.
+ ///
+ ///
+ /// The state is set here rather than through MoveGroupCommand, which would refuse: this fixture's
+ /// account holds one vault, and the command declines rather than open a picker with nothing in it. The
+ /// flow that fills it is covered in DodoSSH.Client.App.Tests.
+ ///
+ ///
+ [Fact]
+ public async Task TheHostsScreenFitsWithTheGroupMovePanelOpen()
+ {
+ await SeedGroupsAsync(3);
+
+ vault.SelectedGroup = vault.Groups[0];
+
+ vault.MoveGroupVaultChoices.Add(
+ new VaultChoiceViewModel(Guid.CreateVersion7(), "Platform Engineering secrets", false));
+
+ vault.SelectedMoveGroupVault = vault.MoveGroupVaultChoices[0];
+ vault.IsMovingGroup = true;
+
+ vault.ShowsGroupActions.ShouldBeFalse("the panel takes the row rather than sharing it");
+
+ await MeasureHostsAsync(faults => faults.ShouldBeEmpty("with the group move panel up"));
+ }
+
// ---- The vault screen ----
[Fact]
diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs
index cf35c3e..6fce402 100644
--- a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs
+++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs
@@ -3801,13 +3801,20 @@ public sealed class ShellFlowTests : IAsyncLifetime
}
///
- /// Deleting a group deliberately does not rewrite the hosts in it — one delete would otherwise become N
- /// writes, N outbox rows and N chances to merge against a change nobody made — so those hosts keep an id
- /// that resolves to nothing. "The group is gone" and "this host is in no group" have to look the same,
- /// because to the person reading the list they are the same thing.
+ ///
+ /// Deleting a group leaves the machines under it alone and stops them naming it. It used to do
+ /// only the first: the reference was left dangling and the list resolved it to nothing, which looked
+ /// identical and cost no writes. The tick is what changed that — a deletion that can take the hosts with
+ /// it has to be a deletion that knows which hosts it means, and once it knows, leaving them holding the
+ /// id of something that has gone is a state kept for no reason.
+ ///
+ ///
+ /// The tick is deliberately not touched here, which is the point of the assertions: the default answer
+ /// is the one that keeps the machines.
+ ///
///
[Fact]
- public async Task DeletingAGroup_LeavesItsHostsUnderTheUngroupedHeading()
+ public async Task DeletingAGroup_UnfilesItsHostsRatherThanLeavingThemNamingIt()
{
await UnlockedAsync();
var vault = shell.Vault!;
@@ -3816,32 +3823,136 @@ public sealed class ShellFlowTests : IAsyncLifetime
await AddGroupAsync(vault, "production");
await FileAsync(vault, "prod-db", "production");
- var groupId = vault.Groups.Single().EntityId;
-
vault.SelectedGroup = vault.Groups.Single();
vault.DeleteGroupCommand.Execute(null);
- vault.PendingDeletion.ShouldNotBeNull().Usage
+ var question = vault.PendingDeletion.ShouldNotBeNull();
+
+ question.Usage
.ShouldContain("1 host", Case.Sensitive, "the count is what makes the question worth reading");
+ question.HasChoice.ShouldBeTrue("a group with a host under it has a second question");
+ vault.DeletionTakesTheHostsToo.ShouldBeFalse("the safe answer is the one nobody has to choose");
+
+ // The pass that follows every write on this screen reports what it moved and supersedes the
+ // confirmation, for a deletion as much as for a save — so it is made to fail, and what the sentence
+ // says is asserted in the state where somebody actually reads it.
+ server.SyncFailure = new HttpRequestException("The server is having a bad day.");
+
await vault.ConfirmDeleteCommand.ExecuteAsync(null);
vault.Groups.ShouldBeEmpty();
vault.HasGroups.ShouldBeFalse();
- // The host keeps the id, which is what makes this cheap; the list is what resolves it to nothing.
- vault.Hosts.Single().Host.GroupId.ShouldBe(groupId);
+ vault.Hosts.Single().Host.GroupId.ShouldBeNull(vault.Status);
+ vault.Hosts.Single().GroupLabel.ShouldBeEmpty();
vault.SidebarRows.ShouldAllBe(row => row is HostRowViewModel);
- // The card says the same thing the phone's list does: nothing. An id nobody can name is drawn as no
- // group rather than as a GUID on a chip.
- vault.Hosts.Single().GroupLabel.ShouldBeEmpty();
+ vault.Status.ShouldContain("UNGROUPED", Case.Sensitive);
}
///
- /// The picker keeps a placeholder entry for a group the vault no longer has, exactly as the
+ /// The other answer, and the reason the question is asked at all: a group is sometimes a heading being
+ /// tidied away and sometimes a project that has been decommissioned, and nothing in the view model can
+ /// tell which of the two it is looking at.
+ ///
+ [Fact]
+ public async Task DeletingAGroupWithTheTickSet_TakesItsHostsWithIt()
+ {
+ await UnlockedAsync();
+ var vault = shell.Vault!;
+
+ await AddHostAsync(vault, "prod-db");
+ await AddHostAsync(vault, "prod-web");
+ await AddGroupAsync(vault, "production");
+ await FileAsync(vault, "prod-db", "production");
+
+ vault.SelectedGroup = vault.Groups.Single();
+ vault.DeleteGroupCommand.Execute(null);
+
+ vault.PendingDeletion.ShouldNotBeNull().Choice.ShouldContain("host");
+ vault.DeletionTakesTheHostsToo = true;
+
+ await vault.ConfirmDeleteCommand.ExecuteAsync(null);
+
+ vault.Groups.ShouldBeEmpty();
+
+ // Only the machine that was filed under it. A deletion aimed at a heading must not reach the hosts
+ // that were never on it.
+ vault.Hosts.Select(row => row.Label).ShouldBe(["prod-web"], vault.Status);
+ }
+
+ ///
+ /// The answer is not carried from one question to the next. A tick left standing would delete the next
+ /// group's machines on the strength of a decision about the last one's, and there is no undo on either
+ /// side of that.
+ ///
+ [Fact]
+ public async Task AskingAboutASecondGroup_StartsFromKeepingItsHosts()
+ {
+ await UnlockedAsync();
+ var vault = shell.Vault!;
+
+ await AddHostAsync(vault, "prod-db");
+ await AddGroupAsync(vault, "production");
+ await AddGroupAsync(vault, "staging");
+ await FileAsync(vault, "prod-db", "production");
+
+ vault.SelectedGroup = vault.Groups.Single(
+ row => string.Equals(row.Label, "production", StringComparison.Ordinal));
+
+ vault.DeleteGroupCommand.Execute(null);
+ vault.DeletionTakesTheHostsToo = true;
+ vault.CancelDeleteCommand.Execute(null);
+
+ vault.SelectedGroup = vault.Groups.Single(
+ row => string.Equals(row.Label, "staging", StringComparison.Ordinal));
+
+ vault.DeleteGroupCommand.Execute(null);
+
+ vault.DeletionTakesTheHostsToo.ShouldBeFalse("every question starts from keeping the machines");
+ }
+
+ ///
+ /// Unfiling rewrites every host under the heading, so it is refused with a host editor open for the
+ /// reason a drop onto a group card is: rewriting the saved host under a half-typed edit of it would be a
+ /// save nobody asked for, and one they could then not cancel. Deleting a single host is not refused,
+ /// because that one writes nothing to a form anybody is looking at.
+ ///
+ [Fact]
+ public async Task DeletingAGroupWhileTheHostEditorIsOpen_IsRefused()
+ {
+ await UnlockedAsync();
+ var vault = shell.Vault!;
+
+ await AddHostAsync(vault, "prod-db");
+ await AddGroupAsync(vault, "production");
+ await FileAsync(vault, "prod-db", "production");
+
+ vault.SelectedHost = vault.Hosts.Single();
+ vault.EditSelectedHostCommand.Execute(null);
+ vault.EditorLabel = "half-typed";
+
+ vault.SelectedGroup = vault.Groups.Single();
+ vault.DeleteGroupCommand.Execute(null);
+
+ vault.PendingDeletion.ShouldBeNull("the question was never put");
+ vault.IsEditing.ShouldBeTrue("and the edit is still there to finish");
+ vault.Status.ShouldContain("editing");
+ }
+
+ ///
+ ///
+ /// The picker keeps a placeholder entry for a group the vault does not have, exactly as the
/// authentication picker does for a deleted key. Without it the picker would open on "No group" and
/// somebody editing the host's port would unfile it by saving.
+ ///
+ ///
+ /// The dangling id is imported rather than produced by deleting the group, and that is a consequence of
+ /// the change above rather than a contrivance: a group deleted here now unfiles its hosts on the
+ /// way out, so the only way a host still names one is that the group went on another machine and this
+ /// client has yet to be told — which is exactly what a host arriving with an id nothing resolves is.
+ ///
///
[Fact]
public async Task EditingAHostWhoseGroupIsGone_DoesNotUnfileItBySaving()
@@ -3849,15 +3960,13 @@ public sealed class ShellFlowTests : IAsyncLifetime
await UnlockedAsync();
var vault = shell.Vault!;
- await AddHostAsync(vault, "prod-db");
- await AddGroupAsync(vault, "production");
- await FileAsync(vault, "prod-db", "production");
+ var groupId = Guid.CreateVersion7();
- var groupId = vault.Groups.Single().EntityId;
+ await vault.ImportHostsAsync(
+ [new HostSecret { Label = "prod-db", Hostname = "db.internal", GroupId = groupId }],
+ Token);
- vault.SelectedGroup = vault.Groups.Single();
- vault.DeleteGroupCommand.Execute(null);
- await vault.ConfirmDeleteCommand.ExecuteAsync(null);
+ vault.Groups.ShouldBeEmpty("nothing in this keychain answers to that id");
vault.SelectedHost = vault.Hosts.Single();
vault.EditSelectedHostCommand.Execute(null);
diff --git a/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs b/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs
index c09bdb2..ab0ce0b 100644
--- a/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs
+++ b/tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs
@@ -875,6 +875,155 @@ public sealed class VaultSharingTests : IAsyncLifetime
.EntityId.ShouldBeNull("only 'no parent' is left once the group is going somewhere else");
}
+ ///
+ ///
+ /// Moving the shelf rather than what is on it, which is the operation people were attempting one host at
+ /// a time: a group cannot go anywhere alone, because the machines filed under it and the groups nested
+ /// inside it are items of the vault it is leaving. All of them are re-sealed under the destination's key
+ /// and all of them take new ids, so what this asserts is not only that they arrived but that the tree
+ /// arrived — the child is still under the parent, and the host is still under the child, through two
+ /// levels of ids that were rewritten on the way across.
+ ///
+ ///
+ /// The parent the moved group was nested under is asserted gone, and that is the honest half. A
+ /// parent belongs to the vault it is in, so carrying the reference would leave everybody else in the
+ /// destination looking at a group hanging from nothing. It arrives at the top level and the sentence
+ /// says so.
+ ///
+ ///
+ [Fact]
+ public async Task MovingAGroupToAnotherVault_TakesItsHostsAndItsNestedGroupsWithIt()
+ {
+ await UnlockedAsync();
+
+ var vaults = shell.Vaults;
+
+ await CreateVaultAsync(vaults, "Platform secrets");
+
+ var vault = shell.Vault!;
+ var sharedVaultId = vaults.SelectedVault!.VaultId;
+
+ await vault.LoadAsync(Token);
+
+ await SeedNestedShelfAsync(vault);
+
+ var production = Named(vault, "production");
+
+ production.VaultId.ShouldNotBe(sharedVaultId, "this test is meaningless with both in one vault");
+ production.Group.ParentId.ShouldNotBeNull("it was nested, which is what has to stay behind");
+
+ vault.SelectedGroup = production;
+ vault.CanMoveGroupTarget.ShouldBeTrue("there is a second vault this session can write to");
+
+ vault.MoveGroupCommand.Execute(null);
+
+ vault.IsMovingGroup.ShouldBeTrue(vault.Status);
+ vault.ShowsGroupActions.ShouldBeFalse("the panel takes the row of buttons rather than sharing it");
+ vault.MoveGroupVaultChoices.ShouldNotContain(choice => choice.VaultId == production.VaultId);
+
+ vault.SelectedMoveGroupVault =
+ vault.MoveGroupVaultChoices.Single(choice => choice.VaultId == sharedVaultId);
+
+ // The pass that follows every write on this screen is made to fail, so that the move's own sentence
+ // is still on the status line to be read — the same arrangement, and for the same reason, as the
+ // host's move test above.
+ server.SyncFailure = new IOException("The server is not answering.");
+
+ await vault.ConfirmMoveGroupCommand.ExecuteAsync(null);
+
+ var moved = Named(vault, "production");
+ var nested = Named(vault, "web");
+
+ moved.VaultId.ShouldBe(sharedVaultId, vault.Status);
+ moved.EntityId.ShouldNotBe(production.EntityId, "an id belongs to one vault");
+ moved.Group.ParentId.ShouldBeNull("a parent belongs to the vault the group came from");
+
+ nested.VaultId.ShouldBe(sharedVaultId, "a group inside it cannot be left in the other vault");
+ nested.Group.ParentId.ShouldBe(moved.EntityId, "and it is still nested under the group it was in");
+
+ var host = vault.Hosts.Single(row => string.Equals(row.Label, "prod-db", StringComparison.Ordinal));
+
+ host.VaultId.ShouldBe(sharedVaultId, "the hosts came with the shelf");
+ host.Host.GroupId.ShouldBe(nested.EntityId, "and are still filed where they were");
+
+ // The group it was nested under is the one thing that stayed, and it stayed where it was.
+ Named(vault, "estate").VaultId.ShouldBe(production.VaultId);
+
+ vault.SelectedGroup?.EntityId.ShouldBe(moved.EntityId, "the buttons follow the group they moved");
+ vault.Status.ShouldContain("Platform secrets");
+ vault.Status.ShouldContain("top level");
+ }
+
+ ///
+ /// The move is refused where it would have nowhere to go, by the command rather than by an empty picker
+ /// — the same answer MoveHostCommand gives one level down.
+ ///
+ [Fact]
+ public async Task MovingAGroupWithNowhereToMoveIt_SaysSoRatherThanOpeningAnEmptyPicker()
+ {
+ await UnlockedAsync();
+
+ var vault = shell.Vault!;
+
+ await vault.LoadAsync(Token);
+
+ vault.NewGroupCommand.Execute(null);
+ vault.GroupEditorLabel = "production";
+ await vault.SaveGroupCommand.ExecuteAsync(null);
+
+ vault.SelectedGroup = vault.Groups.ShouldHaveSingleItem();
+
+ vault.CanMoveGroupTarget.ShouldBeFalse("the personal vault is the only one there is");
+
+ vault.MoveGroupCommand.Execute(null);
+
+ vault.IsMovingGroup.ShouldBeFalse();
+ vault.MoveGroupVaultChoices.ShouldBeEmpty();
+ vault.Status.ShouldContain("only vault you can write to");
+ }
+
+ /// The group card with a given name, re-found because every row is replaced on every reload.
+ private static HostGroupRowViewModel Named(VaultViewModel vault, string label) =>
+ vault.Groups.Single(row => string.Equals(row.Label, label, StringComparison.Ordinal));
+
+ ///
+ /// Builds estate › production › web in the personal vault, with prod-db on the innermost shelf.
+ ///
+ ///
+ /// Three levels, because two would not tell a subtree that was walked from one that was assumed a single
+ /// level deep — the middle group is the one that has to arrive with a rewritten parent and a rewritten
+ /// child at once.
+ ///
+ private static async Task SeedNestedShelfAsync(VaultViewModel vault)
+ {
+ await AddGroupAsync(vault, "estate", under: null);
+ await AddGroupAsync(vault, "production", under: "estate");
+ await AddGroupAsync(vault, "web", under: "production");
+
+ vault.NewHostCommand.Execute(null);
+ vault.EditorLabel = "prod-db";
+ vault.EditorHostname = "db.internal";
+ vault.EditorSelectedGroup = vault.EditorGroupChoices.Single(
+ choice => string.Equals(choice.Label, "web", StringComparison.Ordinal));
+
+ await vault.SaveHostCommand.ExecuteAsync(null);
+ }
+
+ /// Adds a group, optionally nested under one already there.
+ private static async Task AddGroupAsync(VaultViewModel vault, string label, string? under)
+ {
+ vault.NewGroupCommand.Execute(null);
+ vault.GroupEditorLabel = label;
+
+ if (under is not null)
+ {
+ vault.GroupEditorSelectedParent = vault.GroupEditorParentChoices.Single(
+ choice => string.Equals(choice.Label, under, StringComparison.Ordinal));
+ }
+
+ await vault.SaveGroupCommand.ExecuteAsync(null);
+ }
+
///
///
/// Dragging a host card onto a group card is the one gesture that files a host without opening its