Keep the most recently updated note in each set of duplicates

README.md

@@ -224,7 +224,7 @@ sn get notes --title "Meeting Notes" --rich
 # Edit a note in $EDITOR
 sn edit note --title "Meeting Notes"
 
-# See which notes the app marked as copies, then delete them
+# Review duplicate notes, then delete all but the newest of each set
 sn delete duplicates --dry-run
 sn delete duplicates
 

cmd/sncli/duplicates.go

@@ -10,9 +10,9 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-// maxDuplicatesListed caps how many duplicates are listed before the rest are
-// summarised, so a large account does not fill the terminal.
-const maxDuplicatesListed = 20
+// maxDuplicateGroupsListed caps how many sets of duplicates are listed before
+// the rest are summarised, so a large account does not fill the terminal.
+const maxDuplicateGroupsListed = 20
 
 func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error {
 	sess, _, err := cache.GetSession(common.NewHTTPClient(), opts.useSession, opts.sessKey, opts.server, opts.debug)
@@ -35,21 +35,23 @@ func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error {
 		return fmt.Errorf("failed to find duplicate notes: %w", err)
 	}
 
-	writeDuplicatesKept(c, found.KeptOriginalMissing)
+	writeDuplicatesKept(c, found.KeptNoOriginal)
 
-	if len(found.Deleted) == 0 {
+	toDelete := found.Deleted()
+
+	if len(toDelete) == 0 {
 		_, _ = fmt.Fprintln(c.App.Writer, "no duplicate notes found")
 
 		return nil
 	}
 
-	writeDuplicatesFound(c, found.Deleted, dryRun)
+	writeDuplicateGroups(c, found.Groups, len(toDelete), dryRun)
 
 	if dryRun {
 		return nil
 	}
 
-	if !c.Bool("yes") && !confirmDeleteDuplicates(c, len(found.Deleted)) {
+	if !c.Bool("yes") && !confirmDeleteDuplicates(c, len(toDelete)) {
 		return nil
 	}
 
@@ -60,41 +62,47 @@ func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error {
 		return fmt.Errorf("failed to delete duplicate notes: %w", err)
 	}
 
-	_, _ = fmt.Fprintln(c.App.Writer, color.Green.Sprintf("%d duplicate notes deleted", len(out.Deleted)))
+	_, _ = fmt.Fprintln(c.App.Writer, color.Green.Sprintf("%d duplicate notes deleted", len(out.Deleted())))
 
 	return nil
 }
 
-func writeDuplicatesFound(c *cli.Context, dups []sncli.DuplicateNote, dryRun bool) {
-	heading := fmt.Sprintf("%d duplicate notes to delete:", len(dups))
+func writeDuplicateGroups(c *cli.Context, groups []sncli.DuplicateGroup, numToDelete int, dryRun bool) {
+	heading := fmt.Sprintf("%d notes to delete, in %d sets of duplicates:", numToDelete, len(groups))
 	if dryRun {
-		heading = fmt.Sprintf("%d duplicate notes would be deleted:", len(dups))
+		heading = fmt.Sprintf("%d notes would be deleted, in %d sets of duplicates:", numToDelete, len(groups))
 	}
 
 	_, _ = fmt.Fprintln(c.App.Writer, heading)
 
-	for x, dup := range dups {
-		if x == maxDuplicatesListed {
-			_, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf("  ... and %d more", len(dups)-maxDuplicatesListed))
+	for x, group := range groups {
+		if x == maxDuplicateGroupsListed {
+			_, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf("  ... and %d more sets", len(groups)-maxDuplicateGroupsListed))
 
 			break
 		}
 
-		_, _ = fmt.Fprintf(c.App.Writer, "  %s  %s\n", dup.UUID, dup.Title)
+		_, _ = fmt.Fprintf(c.App.Writer, "  keep    %s  %s  %s\n",
+			group.Keep.UpdatedDate(), group.Keep.UUID, group.Keep.Title)
+
+		for _, dup := range group.Delete {
+			_, _ = fmt.Fprintf(c.App.Writer, "  %s  %s  %s  %s\n",
+				color.Red.Sprint("delete"), dup.UpdatedDate(), dup.UUID, dup.Title)
+		}
 	}
 }
 
-func writeDuplicatesKept(c *cli.Context, kept []sncli.DuplicateNote) {
+func writeDuplicatesKept(c *cli.Context, kept []sncli.NoteSummary) {
 	if len(kept) == 0 {
 		return
 	}
 
 	_, _ = fmt.Fprintln(c.App.Writer, color.Yellow.Sprintf(
-		"%d duplicates kept, as the note each was copied from is no longer in the account:", len(kept)))
+		"%d copies kept, as the note each was made from is no longer in the account:", len(kept)))
 
 	for x, dup := range kept {
-		if x == maxDuplicatesListed {
-			_, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf("  ... and %d more", len(kept)-maxDuplicatesListed))
+		if x == maxDuplicateGroupsListed {
+			_, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf("  ... and %d more", len(kept)-maxDuplicateGroupsListed))
 
 			break
 		}
@@ -104,7 +112,7 @@ func writeDuplicatesKept(c *cli.Context, kept []sncli.DuplicateNote) {
 }
 
 func confirmDeleteDuplicates(c *cli.Context, num int) bool {
-	_, _ = fmt.Fprintf(c.App.Writer, "delete %d duplicate notes? ", num)
+	_, _ = fmt.Fprintf(c.App.Writer, "delete %d notes? ", num)
 
 	var input string
 

internal/sncli/duplicates.go

@@ -2,61 +2,156 @@ package sncli
 
 import (
 	"fmt"
+	"sort"
+	"time"
 
 	"github.com/jonhadfield/gosn-v2/cache"
-	"github.com/jonhadfield/gosn-v2/common"
 	"github.com/jonhadfield/gosn-v2/items"
 )
 
-// DuplicateNote is a note that Standard Notes marked as a copy of another,
-// by setting duplicate_of when the note was duplicated.
-type DuplicateNote struct {
-	UUID string
-	// Title of the duplicate itself
+// NoteSummary identifies a note in duplicate output.
+type NoteSummary struct {
+	UUID  string
 	Title string
-	// OriginalUUID is the note this one was copied from
-	OriginalUUID string
-	// OriginalPresent reports whether that original is still in the account.
-	// When it is not, deleting the duplicate would lose the content, so the
-	// duplicate is kept.
-	OriginalPresent bool
+	// UpdatedAt is the note's last-updated timestamp, as the API reports it.
+	UpdatedAt int64
+	// UpdatedAtText is the same time as text, for display.
+	UpdatedAtText string
+	// Marked reports whether the note carries duplicate_of, meaning Standard
+	// Notes created it by duplicating another note.
+	Marked bool
 }
 
-// FindDuplicateNotes returns the notes marked as duplicates, and whether the
-// note each was copied from still exists.
-func FindDuplicateNotes(in items.Items) []DuplicateNote {
-	live := make(map[string]struct{})
+// UpdatedDate returns the note's last-updated time as a date and time for
+// display, or the raw value if it cannot be parsed.
+func (n NoteSummary) UpdatedDate() string {
+	t, err := time.Parse(timeLayout, n.UpdatedAtText)
+	if err != nil {
+		return n.UpdatedAtText
+	}
+
+	return t.Format("2006-01-02 15:04")
+}
+
+// DuplicateGroup is a note and the copies made from it, linked by
+// duplicate_of. The most recently updated note is kept and the rest deleted,
+// so that editing a copy after duplicating it does not lose that work.
+type DuplicateGroup struct {
+	Keep   NoteSummary
+	Delete []NoteSummary
+}
+
+func summariseNote(note *items.Note) NoteSummary {
+	return NoteSummary{
+		UUID:          note.GetUUID(),
+		Title:         note.Content.GetTitle(),
+		UpdatedAt:     note.GetUpdatedAtTimestamp(),
+		UpdatedAtText: note.GetUpdatedAt(),
+		Marked:        note.GetDuplicateOf() != "",
+	}
+}
+
+// newerThan reports whether a should be kept in preference to b. The most
+// recently updated wins; on a tie an original is preferred over a copy, and
+// then the lower UUID, so the choice is stable.
+func newerThan(a, b NoteSummary) bool {
+	switch {
+	case a.UpdatedAt != b.UpdatedAt:
+		return a.UpdatedAt > b.UpdatedAt
+	case a.Marked != b.Marked:
+		return !a.Marked
+	default:
+		return a.UUID < b.UUID
+	}
+}
+
+// FindDuplicateGroups groups notes with the copies made from them and reports
+// which to keep. It also returns copies whose original is no longer in the
+// account: those are the only remaining copy of their content, so they are
+// left alone.
+func FindDuplicateGroups(in items.Items) (groups []DuplicateGroup, keptNoOriginal []NoteSummary) {
+	live := make(map[string]*items.Note)
 
 	for _, item := range in {
-		if item.GetContentType() == common.SNItemTypeNote && !item.IsDeleted() {
-			live[item.GetUUID()] = struct{}{}
+		if note, isNote := item.(*items.Note); isNote && !note.IsDeleted() {
+			live[note.GetUUID()] = note
 		}
 	}
 
-	var duplicates []DuplicateNote
+	// union-find over the live notes, joining each copy to the note it was
+	// made from, so that a copy of a copy ends up in one group
+	parent := make(map[string]string, len(live))
 
-	for _, item := range in {
-		note, isNote := item.(*items.Note)
-		if !isNote || note.IsDeleted() || note.GetDuplicateOf() == "" {
-			continue
+	var find func(string) string
+
+	find = func(u string) string {
+		if parent[u] != u {
+			parent[u] = find(parent[u])
 		}
 
+		return parent[u]
+	}
+
+	for uuid := range live {
+		parent[uuid] = uuid
+	}
+
+	for uuid, note := range live {
 		original := note.GetDuplicateOf()
-		_, originalPresent := live[original]
+		if original == "" {
+			continue
+		}
+
+		if _, ok := live[original]; !ok {
+			keptNoOriginal = append(keptNoOriginal, summariseNote(note))
+
+			continue
+		}
 
-		duplicates = append(duplicates, DuplicateNote{
-			UUID:            note.GetUUID(),
-			Title:           note.Content.GetTitle(),
-			OriginalUUID:    original,
-			OriginalPresent: originalPresent,
-		})
+		parent[find(uuid)] = find(original)
 	}
 
-	return duplicates
+	members := make(map[string][]NoteSummary)
+
+	for uuid, note := range live {
+		root := find(uuid)
+		members[root] = append(members[root], summariseNote(note))
+	}
+
+	for _, group := range members {
+		if len(group) < 2 {
+			continue
+		}
+
+		keep := group[0]
+
+		for _, candidate := range group[1:] {
+			if newerThan(candidate, keep) {
+				keep = candidate
+			}
+		}
+
+		var toDelete []NoteSummary
+
+		for _, candidate := range group {
+			if candidate.UUID != keep.UUID {
+				toDelete = append(toDelete, candidate)
+			}
+		}
+
+		sort.Slice(toDelete, func(i, j int) bool { return toDelete[i].UUID < toDelete[j].UUID })
+
+		groups = append(groups, DuplicateGroup{Keep: keep, Delete: toDelete})
+	}
+
+	sort.Slice(groups, func(i, j int) bool { return groups[i].Keep.UUID < groups[j].Keep.UUID })
+	sort.Slice(keptNoOriginal, func(i, j int) bool { return keptNoOriginal[i].UUID < keptNoOriginal[j].UUID })
+
+	return groups, keptNoOriginal
 }
 
-// DeleteDuplicateNotesConfig deletes notes marked as duplicates of another
-// note.
+// DeleteDuplicateNotesConfig deletes the superseded notes in each set of
+// duplicates.
 type DeleteDuplicateNotesConfig struct {
 	Session *cache.Session
 	// DryRun reports what would be deleted without deleting anything.
@@ -66,15 +161,27 @@ type DeleteDuplicateNotesConfig struct {
 
 // DeleteDuplicateNotesOutput describes what was, or would be, deleted.
 type DeleteDuplicateNotesOutput struct {
-	// Deleted holds the duplicates removed, or, for a dry run, those that
-	// would be removed.
-	Deleted []DuplicateNote
-	// KeptOriginalMissing holds duplicates left alone because the note they
-	// were copied from is no longer in the account.
-	KeptOriginalMissing []DuplicateNote
+	// Groups holds each set of duplicates, with the note kept and those
+	// deleted.
+	Groups []DuplicateGroup
+	// KeptNoOriginal holds copies left alone because the note they were made
+	// from is no longer in the account.
+	KeptNoOriginal []NoteSummary
+}
+
+// Deleted returns every note deleted, across all groups.
+func (o DeleteDuplicateNotesOutput) Deleted() []NoteSummary {
+	var all []NoteSummary
+
+	for _, group := range o.Groups {
+		all = append(all, group.Delete...)
+	}
+
+	return all
 }
 
-// Run finds duplicates and, unless this is a dry run, deletes them.
+// Run finds duplicates and, unless this is a dry run, deletes all but the most
+// recently updated note in each set.
 func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) {
 	var out DeleteDuplicateNotesOutput
 
@@ -103,26 +210,20 @@ func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) {
 		}
 	}
 
-	for _, dup := range FindDuplicateNotes(all) {
-		if dup.OriginalPresent {
-			out.Deleted = append(out.Deleted, dup)
-
-			continue
-		}
+	out.Groups, out.KeptNoOriginal = FindDuplicateGroups(all)
 
-		out.KeptOriginalMissing = append(out.KeptOriginalMissing, dup)
-	}
+	toDelete := out.Deleted()
 
-	if i.DryRun || len(out.Deleted) == 0 {
+	if i.DryRun || len(toDelete) == 0 {
 		_ = i.Session.CacheDB.Close()
 
 		return out, nil
 	}
 
 	var notesToDelete items.Notes
 
-	for _, dup := range out.Deleted {
-		note := byUUID[dup.UUID]
+	for _, summary := range toDelete {
+		note := byUUID[summary.UUID]
 		if note == nil || note.GetContent() == nil {
 			continue
 		}