Skip to content

Make import additive: merge into existing parents instead of rejecting duplicates #65

Description

@meduzapat

Problem

Load and Import currently share the same code path in MainWindow::readConfigFile(), distinguished only by a wipe boolean. Duplicate-parent rejection in isValid() is appropriate for project loading but is too strict for imports.

Example

  • Existing project contains a Group named Players.
  • Imported configuration also contains a Group named Players.

Current: the imported Players is rejected as a duplicate, and all its children are dropped with it.
Desired: reuse the existing Players, merge in any new children, skip duplicated children, and preserve local values on conflict.

Current Behavior — Research

Load/Import entry point

  • src/Ui/MainWindow.cpp:380-425readConfigFile(..., bool wipe)
    • openProject() calls with wipe=true (load).
    • prepareDialogs() import path calls with wipe=false (import).
  • Both paths feed the same dialog load(...) calls. There is no separate import code path today; "import" is just "load without wipe".

LOAD lifecycle

src/Ui/DataDialogs/DialogForm.cpp:47-84createItems():

  1. createData(rawItem)
  2. wireChildrenDialogs()
  3. retrieveData()
  4. isValid() — throws on duplicate
  5. currentData->wipe() + storeData()
  6. items->create(currentData) → registers via CollectionHandler::add()

Duplicate rejection sites (action == LOAD)

File Line Parent
src/Ui/DataDialogs/DialogGroup.cpp 178 Group
src/Ui/DataDialogs/DialogAnimation.cpp 69 Animation
src/Ui/DataDialogs/DialogInput.cpp 135 Input
src/Ui/DataDialogs/DialogDevice.cpp 166-171 Device

Pattern:

if (currentData->getCollectionHandler()->isIdSet(uid)) {
    if (action != Actions::EDIT or currentData->createUniqueId() != uid)
        throw Message("...already exists");
}

Secondary silent rejection: src/Ui/Storage/CollectionHandler.cpp:83-88add() returns silently if the id already exists.

"All" group precedent

src/Ui/DataDialogs/DialogGroup.cpp:96-109 already implements the desired pattern for a singleton: if allGroup exists, the existing pointer is reused and subsequent loads accumulate into it. This is exactly the shape import needs, generalized.

Parents affected

  • Mergeable (Parent + children): Group, Animation, Input, Profile.
  • Leaf / strict (no merge semantics): Device — hardware identity must stay unique; duplicates should still be rejected on import.

Proposed Options

Option A — reuse existing parent Option B — temp parent + merge Option C — IMPORT action
Shape On duplicate parent during import, drop currentData, switch to the existing Data*, route children into it Build the parent into a throwaway Data*, then walk children against the existing parent and merge Add Actions::IMPORT alongside ADD/LOAD/EDIT; isValid() resolves duplicate parents to the existing one, child dialogs check per-child uniqueness
Pros Minimal moving parts; mirrors the existing "All" group pattern Cleanest separation; validation runs in isolation before touching live data Explicit and discoverable; per-dialog behavior stays local; matches the existing action-dispatch idiom
Cons isValid() becomes context-sensitive; need to delete the unused currentData without leaking from CollectionHandler Doubles allocation; merge step is extra code; children created against the temp parent need re-parenting (BoxButton/registration churn) Touches every Dialog* that overrides isValid(); need a clear contract for "favor local" at child level
Impact on Load None None None (LOAD branch untouched)
Impact on Import Becomes additive; local values win by virtue of being kept Becomes additive; merge policy explicit Becomes additive; per-parent policy explicit
Complexity Low High Medium

Recommendation: Option C, with Option A's "reuse existing pointer" trick used as the implementation inside IMPORT handling. This keeps the action-driven architecture intact, makes the import contract explicit at each dialog, and avoids the temp-object churn of Option B.

Sketch (Option C)

  1. Extend Actions in src/Ui/DataDialogs/DialogForm.hpp:48-52:
    enum class Actions : uint8_t { ADD, LOAD, IMPORT, EDIT };
  2. MainWindow::readConfigFile(..., bool wipe) → pass wipe ? LOAD : IMPORT into the dialog batch entry points instead of relying on wipe alone.
  3. In each mergeable parent dialog (DialogGroup, DialogAnimation, DialogInput, DialogProfile), override the duplicate branch:
    • LOAD → throw (current behavior).
    • IMPORT → discard currentData, swap to the existing Data* from the CollectionHandler, mark the parent as "reused" so createItems() skips wipe + storeData + create.
  4. Children of a reused parent run their own createItems() against the existing parent's child BoxButtonCollection. Per-child duplicate handling:
    • Default policy: local wins — duplicate child is skipped, no error raised, optionally counted in the import report.
  5. Device keeps LOAD-style strictness under IMPORT (reject duplicates with a collected error).
  6. Discarded currentData must be deleted explicitly: CollectionHandler::add() is a no-op on duplicate but does not take ownership in that case (src/Ui/Storage/CollectionHandler.cpp:83-88). Confirm and free.

Requirements

  • Project loading behavior unchanged.
  • Import is additive; local data is authoritative on every conflict.
  • No silent overwrite of existing user data.
  • New incoming children absorbed wherever the parent type supports merging.
  • Report skipped/merged items in the existing batch-error message channel from MainWindow.cpp:252-265 (as informational entries, not errors).
  • Follow existing architecture and style per AGENTS.md.

Open Questions

  • Should the per-import report distinguish "merged into existing parent" vs "skipped duplicate child" vs "rejected (Device)"?
  • For Profile, does the embedded Transition record participate in merge, or is it always local-wins as a single unit?
  • Does Input's dual collections (COLLECTION_INPUT_SOURCES, COLLECTION_INPUT_LINKMAPS) need different policies?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions