feat(mods): import-details editing, version-unknown state, latest-arrival fix - #241
Merged
Merged
Conversation
…known row state Give every non-linked mod row a permanent correction surface for its import details (name, source association, release tag) plus the derived Nexus-version-unknown row state the load-order association path will resolve. - IModRepository.EditImportDetails: one atomic manifest write folding rename, source switch, and latest-tag retag. The FileId lock (any version with a FileId blocks every identity change), the removeOlderVersions confirm guard for multi-version identity resets, the tag-collision refusal, and the duplicate-identity guard are enforced at the primitive so programmatic callers get the same rules. - The edit-import-details modal (IEditImportDetailsFactory + ShowEditImportDetailsAsync): shared ImportSourceValidator rules extracted from the import card so both surfaces validate identically; a version is required when saving as Nexus so the dialog can never create an unknown state; a FileId-grounded mod degrades to name-only; a multi-version identity change swaps to an inline removal confirm, never a nested modal. Entry: a drawn-pencil button at the end of the shared action strip (one definition, both densities). - The derived version-unknown state (NexusSource + empty latest tag, no storage): the update action enables with unknown-tooltip variants, the badge never appends an empty version, the updates-only filter keeps unknown rows, the pin dropdown is suppressed, UpdateEligibility matches an empty expected version against an empty installed tag (verified end to end through the download queue's dequeue-time revalidation), and the automatic-update batch excludes unknown rows (manual click only).
The inline failure message lived only in the editing step, so a save refused from the removal-confirm step (e.g. the duplicate-identity guard) showed nothing and the button appeared inert. Hoist the failure area out of the swapped panels so both steps share it, with a test pinning the refused-confirm path.
Address the PR 1 code review findings on the edit-details surface. - Surface the primitive's disk failures (manifest write, folder deletion) inline: the Apply catch filter now includes IOException + UnauthorizedAccessException, so a full disk or an AV lock mid-save no longer terminates the app with the modal open. - Close the stale version-count dead-end: Save re-reads the container before the removal-confirm decision (a download for the same container completing while the dialog is open), and the primitive's confirm guard now throws the typed RemovalConfirmationRequiredException, which Apply catches by swapping to the confirm step over fresh state instead of a terminal inline failure. - Refuse the duplicate-untracked-name rename inline: saving as Untracked under a name another untracked container already holds would silently shadow the dedupe index; the dialog blocks it with a localized message. - Reject a non-empty versionTag with an Untracked destination at the primitive (ArgumentException): an untracked container is single-version by construction, and a stray tag would break the empty-tag upsert contract. Reflected in the interface + reference docs. - Reword the removal-confirm copy to cover the Nexus-to-Untracked swap, rename an overstating test to what it pins, and cover the zero-version container branch of the primitive.
The identity lock stays container-wide (any version's FileId), but a tag change is now refused when the LATEST version record carries its own FileId: the installed copy came from a download, so Nexus supplied its version. A hand-imported latest on a previously-downloaded container carries no FileId of its own and stays resolvable, preserving the load-order migration dedupe path onto grounded containers. Unchanged tags remain allowed no-ops, so a grounded rename is unaffected.
Host editing instead of prompting: the batch-import card below the Mods toolbar gains an edit mode, and the edit-import-details modal is removed entirely (dialog, factory, IDialogService method back to seven, DialogService wiring, composition registration). The card now runs two exclusive modes over one shared editing form. StartEdit(containerId) activates it titled 'Edit import details', prefilled from the container (name, source choice, bare mod id, the latest version's tag); the policy picker hides; Save applies the EditImportDetails primitive with the same validation the batch form enforces (shared ImportSourceValidator; a version required when saving as Nexus). The FileId grounding degrades fields: any version's FileId locks the id + source with the 'downloaded from Nexus' hint, the latest record's own FileId additionally locks the version field (the per-record tag lock), the name is never locked. A multi-version identity change swaps the form for an inline removal-confirm stage, with the save-time state refresh + the typed RemovalConfirmationRequiredException catch covering a version landing while the card is open; refused saves and disk failures surface inline. A successful save deactivates the card and raises ImportDetailsEdited, the mod list's reload signal. Mutual exclusion is the shared inactive gate: StartBatch and StartEdit each refuse while the card is active, and an active card (either mode) keeps the Add button + drops gated. The pencil moves first in the shared action strip (between the source badge and the Enabled checkbox) and binds IsEnabled to editability rather than hiding, the update-action-cell pattern.
- Gate the batch Import command on batch mode (IsBatchEditing, not the shared editing state), so a programmatic call mid-edit can no longer index the empty path queue. - Pin the pencil contract in the shared-templates test: IsEnabled (never IsVisible) bound to CanEditImportDetails, preceding the Enabled checkbox in document order, with the leading compact margins pinned; refresh the stale margin enumeration comments to the actual values. - Drop the orphaned EditDetails_Header key and reword the EditDetails comments that still described the deleted modal (ESC/title-bar-close wording) to describe the hosted card. - Update CancelBatch's doc: it cancels the active card in either mode. - Port the two dropped dialog tests: the save trims the tag/name and parses the id/URL to the canonical identity, and a refused confirm save surfaces the failure without leaving the confirm stage.
Replace the per-record lock model with the operator's final rule: downloaded mods are not editable, period. - Grounding widens to a FileId OR a RemoteUploadedAt on a version record (only the download path records either; the timestamp covers downloads from before FileId persistence). - Any grounded version refuses every EditImportDetails mutation, name-only included; the degraded name-only mode, the per-record tag lock, and the field-level grounding machinery are gone. The row pencil disables for grounded containers (with linked + morphed rows) and StartEdit refuses on top of it, defense in depth. - The name is Untracked-only: a Nexus-destination edit cannot change the name (Nexus owns it; the update check's name-sync would revert a user typing). The card's name field follows the source choice; the id, version, and source switch stay editable for ungrounded containers. Tests rewritten to the new model: both grounding shapes refuse at the primitive and at StartEdit, the name-rule matrix (rename-with-switch allowed, Nexus name refused incl. the programmatic path), the pencil predicate across all row shapes, and the surviving branches (removal confirm + recover, duplicate identity, untracked-name conflict, IO families) reseeded as ungrounded.
…e rule Address review findings on e4c9435: drop the stale tag-lock sentence, add the downloaded-row state to the pencil comment, and repeat the grounded guard in the parent command so the defense-in-depth layering matches the pencil's three disabled states.
…rved A disabled pencil was indistinguishable from an enabled one in dark mode. The button now binds IsVisible to CanEditImportDetails inside a host Panel the strip always lays out, sized to the pencil's 28-DIP footprint and carrying its leading compact margin: an editable row shows the pencil exactly where it was, a non-editable row shows empty space of the same width, the Enabled checkbox never shifts, and the hidden button leaves the a11y tree, focus order, and hit testing naturally. Mirrors the strip's existing update-action-cell pattern.
…the prune An incomplete fix of #232 shipped: on a container mixing a manual import with a download, the manual's fresh ImportedAt outranked the download's older RemoteUploadedAt under the effective-timestamp key forever, so the downloaded version never became IsLatest, LatestPolicy kept resolving the manual import, and the startup prune then deleted the downloaded version folder as unreferenced. Two parts: - WithLatestMarked now ranks by the most recent ARRIVAL: a manual import (no RemoteUploadedAt) with the newest ImportedAt is latest (the user just brought the content in; the load-order migration case lands as latest), otherwise the newest downloaded version by RemoteUploadedAt (ImportedAt tie-break) is latest, with manual imports ignored in that branch. All-manual and all-download containers keep their previous outcomes; the #232 case (an older remote file arriving later never flips latest) still holds. The dedup branch is not a new arrival (the reused entry keeps its original stamp). Mirrored in the UI test fake. - ModCleanup.PruneUnreferenced now also references every referenced container's CURRENT latest version folder, regardless of the entry's own policy: a pinned entry can no longer let the prune delete the container's newest version on restart, and the latest folder survives until any stale flag self-heals on the next add/remove. Unreferenced superseded versions are still reclaimed. Existing manifests whose latest flag is already wrong on disk self-heal only when a new AddVersion or RemoveVersion re-evaluates: re-download or re-import the mod once to repair; no migration.
The Fluent dark theme renders disabled text at near-invisible opacity, so the name field on a Nexus-sourced container read as EMPTY: the operator could not tell which mod they were editing. Lock the field as read-only instead (IsReadOnly bound to !IsNameEditable; the IsEnabled binding is dropped): read-only text renders at full contrast and stays selectable. The name field is the only locked field on the card, and the VM's IsNameEditable is unchanged; only the presentation moves. A source-scan test pins the read-only contract.
Opening the edit card (or a batch) left the search box + filter toggles live; typing in the search box could hide the row being edited under its own open editor. The toolbar's projection-touching controls (the search box, the hide-disabled + updates-only toggles, the density selector, the check-now refresh) now disable for the whole time the card is active in either mode, through ModListViewModel's new IsListToolingEnabled projection (the IsAddEnabled shape: a plain read over ImportWorkflow.IsActive, re-fired through the same child IsActive subscription). Row-level controls stay live (they do not affect the projection); the Add button keeps its existing gate. The bindings land on the search TextBox + the two toolbar clusters, so one binding per cluster covers every control inside.
The edit editor always sat at the top of the page, splitting attention between the clicked row and the typing surface. The edit mode now renders as a leading band section INSIDE the edited row's template (an ItemsControl cannot host elements between items); batch mode is unchanged and keeps the top card below the toolbar. Mechanics: - One shared ContentControl + ModRowEditBandTemplate precedes both density roots in the row template (root Panel becomes a vertical StackPanel), hosting the SAME ImportWorkflowView the top card uses, so the form, the removal-confirm stage, and the failure area move with the edit mode. The band follows the ActiveDownload morph pattern: the parent assigns the row's IsEditTarget + EditBandContext (the workflow VM), so the form instantiates only on the editing row. - The edit target is tracked by container id (the workflow's new EditTargetContainerId, propagated through the shared child subscription) and re-assigned on every Reload, so a mid-edit reload re-attaches the band to the rebuilt row instance. The top host binds the new IsBatchActive projection. - The editing row is anchored like an order-locked row: grip not hit-testable, move commands refused, drag math skips it as a destination, and the planner keeps its exact index (the lock toggle visuals still read OrderLocked alone). A download morph arriving on the edited container closes the edit automatically. Opening the band brings its row into view (BringIntoView posted at Loaded priority).
Final pre-PR review nits: the import form's header describes its two hosts, the morph-close comment states the actual trigger (a download targeting the container, not a completed download), and the promotion test drops the retired effective-timestamp key from its name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First PR of #238. Adds the universal correction surface for a mod container's import details, the derived version-unknown row state it resolves, and fixes #240.
Edit import details
IModRepository.EditImportDetails: one manifest-level primitive for editing a container's name, source association, and release tag. Enforces the grounding rules at the data layer: a container is download-grounded when any version carries a FileId or a RemoteUploadedAt (the timestamp covers mods downloaded before FileId persistence existed), and a grounded container refuses every edit, including name-only. The mod id lock, the duplicate-identity guard, the tag-collision guard, and the explicitremoveOlderVersionsconfirm flag (typedRemovalConfirmationRequiredExceptionfor programmatic recovery) all live here.ImportSourceValidatoras the per-item import form; policy picker hidden; Save applies the primitive; a multi-version identity change swaps to an inline removal confirm. The band targets the container id, re-attaches across mid-edit reloads, anchors the editing row against reorder (grip, move buttons, drag), auto-closes when a download morphs the row, and scrolls into view on open.Version-unknown state
Latest-arrival fix (fixes #240)
WithLatestMarkedranks IsLatest by the most recent arrival: a manual import (no remote facts) as the newest arrival is latest; otherwise the newest downloaded version by remote publish date wins. A download arriving over a manually imported container now correctly takes over, while a deliberate old-file download still cannot displace a newer one.Supporting changes