diff --git a/app.go b/app.go index e246511..b0099a2 100644 --- a/app.go +++ b/app.go @@ -1704,6 +1704,13 @@ func copyDirContents(src, dst string) error { if err != nil { return err } + // Create the destination BEFORE writing into it. Without this the first + // os.Create failed with ENOENT, which reads as "not exist" — indistinguishable + // from a source folder that simply has nothing to copy, and callers testing + // for that swallowed it. + if err := os.MkdirAll(dst, 0o755); err != nil { + return err + } for _, e := range entries { srcPath := filepath.Join(src, e.Name()) dstPath := filepath.Join(dst, e.Name()) @@ -14099,10 +14106,7 @@ func (a *App) DuplicateProfile(id int64, newName string) (profile.Profile, error applog.Printf("duplicate profile: copy operating: %v", err) } } - if _, err := a.db.ExecContext(a.ctx, - `INSERT INTO qsl_templates (name, profile_id, json, is_default, created_at, updated_at) - SELECT name, ?, json, is_default, created_at, updated_at - FROM qsl_templates WHERE profile_id = ?`, p.ID, id); err != nil { + if err := a.copyQSLTemplatesToProfile(id, p.ID); err != nil { applog.Printf("duplicate profile: copy qsl templates: %v", err) } return p, nil diff --git a/app_qsl_designer.go b/app_qsl_designer.go index 7c0cff2..5f9ba38 100644 --- a/app_qsl_designer.go +++ b/app_qsl_designer.go @@ -827,18 +827,32 @@ func (a *App) QSLCopyTemplateToActiveProfile(id int64) (int64, error) { } srcDir := qslcard.TemplateDir(a.qslDir(), id) dstDir := qslcard.TemplateDir(a.qslDir(), rec.ID) - if err := copyDirContents(srcDir, dstDir); err != nil && !os.IsNotExist(err) { - // Roll back rather than leave a design whose pictures are missing. + // A design with no pictures at all has no folder, and that is not a failure. + // Anything else IS: this used to be written as "ignore os.IsNotExist", which + // also swallowed every ENOENT raised while copying — so a failed copy looked + // like a design that had nothing to copy, and the operator got "hero photo + // not found" with no clue why. + if _, statErr := os.Stat(srcDir); statErr == nil { + if err := copyDirContents(srcDir, dstDir); err != nil { + // Roll back rather than leave a design whose pictures are missing. + _ = a.qslTemplates.Delete(a.ctx, rec.ID) + _ = qslcard.RemoveTemplateDir(a.qslDir(), rec.ID) + return 0, fmt.Errorf("copy template assets: %w", err) + } + } + // Every failure from here rolls back BOTH halves: deleting only the row would + // leave the copied pictures on disk under an id nothing refers to. + rollback := func() { _ = a.qslTemplates.Delete(a.ctx, rec.ID) - return 0, fmt.Errorf("copy template assets: %w", err) + _ = qslcard.RemoveTemplateDir(a.qslDir(), rec.ID) } t, err := qslcard.Parse([]byte(rec.JSON)) if err != nil { - _ = a.qslTemplates.Delete(a.ctx, rec.ID) + rollback() return 0, err } if err := qslcard.Validate(t, qslcard.PhotoExistsIn(dstDir)); err != nil { - _ = a.qslTemplates.Delete(a.ctx, rec.ID) + rollback() return 0, fmt.Errorf("copied design is incomplete: %w", err) } applog.Printf("qsl: copied template %q (id %d) into the active profile as %q (id %d)", @@ -846,6 +860,50 @@ func (a *App) QSLCopyTemplateToActiveProfile(id int64) (int64, error) { return rec.ID, nil } +// copyQSLTemplatesToProfile duplicates one profile's QSL designs into another, +// PICTURES INCLUDED. +// +// This was an INSERT … SELECT, which cloned the rows and nothing else. A design +// references its photos by name, relative to its own asset folder, so every +// duplicated design pointed at a folder that did not exist: no thumbnail in the +// designer, and nothing to print. The rows looked right, which is why it went +// unnoticed — the damage is entirely on disk. +// +// Row by row rather than in one statement, because each new design needs its id +// before it can own a folder. A design whose files cannot be copied is dropped +// rather than kept empty: an operator who sees the design listed will believe +// it works. +func (a *App) copyQSLTemplatesToProfile(fromProfile, toProfile int64) error { + if a.qslTemplates == nil { + return fmt.Errorf("db not initialized") + } + all, err := a.qslTemplates.List(a.ctx) + if err != nil { + return err + } + for _, src := range all { + if src.ProfileID == nil || *src.ProfileID != fromProfile { + continue + } + rec := qslcard.Record{Name: src.Name, JSON: src.JSON, IsDefault: src.IsDefault} + to := toProfile + rec.ProfileID = &to + if err := a.qslTemplates.Save(a.ctx, &rec); err != nil { + return err + } + srcDir := qslcard.TemplateDir(a.qslDir(), src.ID) + if _, statErr := os.Stat(srcDir); statErr != nil { + continue // a design with no pictures at all — nothing to carry over + } + if err := copyDirContents(srcDir, qslcard.TemplateDir(a.qslDir(), rec.ID)); err != nil { + _ = a.qslTemplates.Delete(a.ctx, rec.ID) + _ = qslcard.RemoveTemplateDir(a.qslDir(), rec.ID) + return fmt.Errorf("copy assets of %q: %w", src.Name, err) + } + } + return nil +} + func firstNonEmptyStr(a, b string) string { if strings.TrimSpace(a) != "" { return a diff --git a/changelog.json b/changelog.json index 33222ba..fc862a8 100644 --- a/changelog.json +++ b/changelog.json @@ -13,7 +13,9 @@ "DX cluster: the US county column now shows the county the station is logged with, so it agrees with the Info panel.", "QSL: the manager (QSL_VIA) and the routing method (QSL_SENT_VIA / QSL_RCVD_VIA) are now separate fields, as ADIF defines them.", "ADIF import: QSL_SENT_VIA no longer lands in the manager field, and QSL_RCVD_VIA is no longer discarded. Both are exported.", - "QSL: logs where a routing word sits in the manager field are counted at startup and corrected only if you accept." + "QSL: logs where a routing word sits in the manager field are counted at startup and corrected only if you accept.", + "QSL designer: copying a design from another profile failed with \"hero photo not found\" — the pictures were never copied.", + "QSL designer: duplicating a profile cloned its card designs without their pictures, leaving designs that could not be printed." ], "fr": [ "Amplificateurs : coche ceux qui partagent un combiner et ON, OFF et OPERATE agissent sur tous à la fois. Chacun garde ses mesures.", @@ -26,7 +28,9 @@ "Cluster DX : la colonne comté US affiche le comté du log quand la station y figure, donc identique au panneau Info.", "QSL : le manager (QSL_VIA) et le mode d’envoi (QSL_SENT_VIA / QSL_RCVD_VIA) sont désormais deux champs distincts, comme le veut l’ADIF.", "Import ADIF : QSL_SENT_VIA ne se retrouve plus dans le champ manager, et QSL_RCVD_VIA n’est plus perdu. Les deux sont exportés.", - "QSL : les logs où un mode d’envoi occupe le champ manager sont comptés au démarrage et corrigés seulement si vous acceptez." + "QSL : les logs où un mode d’envoi occupe le champ manager sont comptés au démarrage et corrigés seulement si vous acceptez.", + "Concepteur QSL : copier un design depuis un autre profil échouait sur « photo introuvable » — les images n’étaient jamais copiées.", + "Concepteur QSL : dupliquer un profil clonait ses designs sans leurs images, donnant des cartes impossibles à imprimer." ] }, { diff --git a/copydir_test.go b/copydir_test.go new file mode 100644 index 0000000..89b9b98 --- /dev/null +++ b/copydir_test.go @@ -0,0 +1,61 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// TestCopyDirContentsCreatesDestination pins the bug that broke copying a QSL +// design between profiles. +// +// copyDirContents wrote straight into dst without creating it, so the first +// os.Create failed with ENOENT. The caller was written as "ignore +// os.IsNotExist", meaning "the source has no assets, that is fine" — and ENOENT +// from the failed write matched it exactly. Nothing was copied, no error was +// reported, and the operator saw "copied design is incomplete: hero photo file +// … not found" from the validation two lines later. +func TestCopyDirContentsCreatesDestination(t *testing.T) { + base := t.TempDir() + src := filepath.Join(base, "src") + if err := os.MkdirAll(filepath.Join(src, "sub"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "img_25c06bda.jpg"), []byte("hero"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, "sub", "back.png"), []byte("back"), 0o644); err != nil { + t.Fatal(err) + } + + // dst does not exist — the case of every first copy. + dst := filepath.Join(base, "dst") + if err := copyDirContents(src, dst); err != nil { + t.Fatalf("copyDirContents: %v", err) + } + for name, want := range map[string]string{ + "img_25c06bda.jpg": "hero", + "sub/back.png": "back", + } { + got, err := os.ReadFile(filepath.Join(dst, filepath.FromSlash(name))) + if err != nil { + t.Errorf("%s: %v", name, err) + continue + } + if string(got) != want { + t.Errorf("%s = %q, want %q", name, got, want) + } + } +} + +// A missing SOURCE is still an error from copyDirContents itself — it is the +// caller that decides whether a design with no pictures is acceptable, and it +// can only decide that if this does not quietly succeed. +func TestCopyDirContentsMissingSource(t *testing.T) { + base := t.TempDir() + if err := copyDirContents(filepath.Join(base, "nope"), filepath.Join(base, "dst")); err == nil { + t.Fatal("copying a missing source returned no error") + } else if !os.IsNotExist(err) { + t.Fatalf("want a not-exist error, got %v", err) + } +}