Skip to content

Commit 6418a69

Browse files
committed
Find an unanswered copy by its title, since it keeps its original's dates
A copy whose start got no answer told the caller to list the destination's recordings sorted by created_at, newest first, as if the copy would top the list. Basecamp's Recording::Copier copies created_at from the original, so the copy sorts where the original would, and on a busy list or card table the caller looked at the top, saw nothing, and could copy again. The hint now lists the destination's recordings in the source's status without a sort, then says to look for the original's title in the destination container by name and id. The --profile and --account scope stays on the listing command, ahead of that sentence.
1 parent 3d50ed5 commit 6418a69

7 files changed

Lines changed: 50 additions & 20 deletions

File tree

‎internal/commands/cards.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,6 +2071,7 @@ func cardFiling(app *appctx.App, kind basecamp.FilingType, card *basecamp.Card,
20712071
showCmd: "basecamp cards show",
20722072
lookFor: recordingCopyLookFor("Kanban::Card", cardTableProjectID(table), card.Status),
20732073
landing: "The copy lands in that table's Triage, or Done if completed",
2074+
match: copyMatch(card.ID, card.Title, table.Title, table.ID),
20742075
read: func(ctx context.Context, id int64) (filedRecording, error) {
20752076
filed, err := app.Account().Cards().Get(ctx, id)
20762077
if err != nil {

‎internal/commands/cards_filing_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ func TestCardsCopyThatFailsUpstreamIsNotRetryable(t *testing.T) {
264264

265265
assert.False(t, e.Retryable, "a 502 can follow a copy Basecamp already started")
266266
assert.Contains(t, e.Hint, "may have started the copy")
267-
assert.Contains(t, e.Hint, "basecamp recordings list Kanban::Card --in 999 --status active --sort created_at --direction desc --all")
267+
assert.Contains(t, e.Hint, "basecamp recordings list Kanban::Card --in 999 --status active --all")
268268
assert.Len(t, transport.posts, 1)
269269
}
270270

@@ -279,7 +279,7 @@ func TestCardsCopyWithNoReadableAnswerMayHaveStarted(t *testing.T) {
279279

280280
assert.False(t, e.Retryable, "Basecamp may have started the copy")
281281
assert.Contains(t, e.Hint, "may have started the copy")
282-
assert.Contains(t, e.Hint, "basecamp recordings list Kanban::Card --in 999 --status active --sort created_at --direction desc --all")
282+
assert.Contains(t, e.Hint, "basecamp recordings list Kanban::Card --in 999 --status active --all")
283283
assert.Len(t, transport.posts, 1)
284284
})
285285
}
@@ -440,7 +440,7 @@ func TestCardsCopyStillRunningSaysNotToCopyAgain(t *testing.T) {
440440

441441
assert.Equal(t, output.CodeInProgress, e.Code)
442442
assert.Contains(t, e.Hint, "Do not copy it again")
443-
assert.Contains(t, e.Hint, "basecamp recordings list Kanban::Card --in 999 --status active --sort created_at --direction desc --all")
443+
assert.Contains(t, e.Hint, "basecamp recordings list Kanban::Card --in 999 --status active --all")
444444
assert.Len(t, transport.posts, 1)
445445
}
446446

‎internal/commands/filing.go‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package commands
22

33
import (
4+
"cmp"
45
"context"
56
"errors"
67
"fmt"
@@ -38,9 +39,11 @@ type recordingFiling struct {
3839
// showCmd shows a recording of this kind given its id: "basecamp cards show".
3940
showCmd string
4041
// lookFor lists where a copy lands, so a copy that may have started is
41-
// checked for before copying again; landing says where in it, before lookFor.
42+
// checked for before copying again; landing says where in it, before
43+
// lookFor, and match what tells the copy apart there, after it.
4244
lookFor string
4345
landing string
46+
match string
4447
// read reads back the recording the filing made, by its new id.
4548
read func(ctx context.Context, id int64) (filedRecording, error)
4649

@@ -152,9 +155,9 @@ func (f recordingFiling) show(id int64) string {
152155
return fmt.Sprintf("%s %d%s", f.showCmd, id, f.scope)
153156
}
154157

155-
// check is the command that lists where a copy lands.
158+
// check lists where a copy lands and says how to tell the copy apart there.
156159
func (f recordingFiling) check() string {
157-
return f.lookFor + f.scope
160+
return f.lookFor + f.scope + ", then " + f.match
158161
}
159162

160163
func (f recordingFiling) start(ctx context.Context, app *appctx.App) (*basecamp.Filing, error) {
@@ -366,15 +369,26 @@ var filingWait = struct{ first, max, total time.Duration }{
366369
}
367370

368371
// recordingCopyLookFor lists the recordings of one type in a project in the
369-
// source's status, newest first and every page. A copy keeps the source's
370-
// status, so this shows a copy whatever state it was made in. Basecamp's
371-
// recordings listing serves Todo, Todolist (groups included), Message,
372-
// Document, Upload, Vault, Kanban::Card and Schedule::Entry.
372+
// source's status, every page. A copy keeps the source's status, so this shows
373+
// a copy whatever state it was made in. Basecamp's recordings listing serves
374+
// Todo, Todolist (groups included), Message, Document, Upload, Vault,
375+
// Kanban::Card and Schedule::Entry.
373376
func recordingCopyLookFor(recordableType string, projectID int64, status string) string {
374377
if status == "" {
375378
status = "active"
376379
}
377-
return fmt.Sprintf("basecamp recordings list %s --in %d --status %s --sort created_at --direction desc --all", recordableType, projectID, status)
380+
return fmt.Sprintf("basecamp recordings list %s --in %d --status %s --all", recordableType, projectID, status)
381+
}
382+
383+
// copyMatch says what tells a copy of a recording apart in the container it
384+
// lands in. A copy keeps its original's title, created_at and updated_at, so
385+
// no sort puts it first; its title in that container finds it.
386+
func copyMatch(sourceID int64, title, container string, containerID int64) string {
387+
name := fmt.Sprintf("the copy of #%d", sourceID)
388+
if title != "" {
389+
name = fmt.Sprintf("%q", title)
390+
}
391+
return fmt.Sprintf("look for %s in %s (#%d); a copy has its original's title and dates", name, cmp.Or(container, "the destination"), containerID)
378392
}
379393

380394
// pollUntil runs check, backing off between runs, until it reports done or

‎internal/commands/todolists.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,6 +1123,7 @@ func listFiling(cmd *cobra.Command, app *appctx.App, kind basecamp.FilingType, l
11231123
showCmd: "basecamp todolists show",
11241124
lookFor: recordingCopyLookFor("Todolist", projectID, list.Status),
11251125
landing: "The copy lands in that To-dos",
1126+
match: copyMatch(list.ID, list.Name, todoset.Title, todoset.ID),
11261127
read: readFiledTodolist(app, list.ID),
11271128
}, nil
11281129
}
@@ -1157,6 +1158,7 @@ func groupFiling(cmd *cobra.Command, app *appctx.App, kind basecamp.FilingType,
11571158
showCmd: "basecamp todolists show",
11581159
lookFor: recordingCopyLookFor("Todolist", todolistProjectID(into), group.Status),
11591160
landing: "The copy lands in that list",
1161+
match: copyMatch(group.ID, group.Name, into.Name, into.ID),
11601162
read: readFiledTodolist(app, group.ID),
11611163
}, nil
11621164
}

‎internal/commands/todolists_filing_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func TestTodolistsCopyToAnotherProjectIsNotRetried(t *testing.T) {
177177

178178
assert.Len(t, transport.posts, 1)
179179
assert.False(t, e.Retryable)
180-
assert.Contains(t, e.Hint, "basecamp recordings list Todolist --in 999 --status active --sort created_at --direction desc --all")
180+
assert.Contains(t, e.Hint, "basecamp recordings list Todolist --in 999 --status active --all")
181181
}
182182

183183
// A filing's recovery points at `todolists show <id>`, which reads the list by

‎internal/commands/todos.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2651,6 +2651,7 @@ func todoFiling(app *appctx.App, kind basecamp.FilingType, todo *basecamp.Todo,
26512651
showCmd: "basecamp todos show",
26522652
lookFor: recordingCopyLookFor("Todo", todolistProjectID(list), todo.Status),
26532653
landing: "The copy lands in that list",
2654+
match: copyMatch(todo.ID, todo.Title, list.Name, list.ID),
26542655
read: func(ctx context.Context, id int64) (filedRecording, error) {
26552656
filed, err := app.Account().Todos().Get(ctx, id)
26562657
if err != nil {

‎internal/commands/todos_filing_test.go‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -493,17 +493,29 @@ func TestTodosCopySeveralToAnotherProjectNeverRetriesAFailedCopy(t *testing.T) {
493493
assert.False(t, e.Retryable)
494494
assert.Contains(t, e.Message, "Copied 0 of 2 to-dos")
495495
assert.Contains(t, e.Hint, "may have started the copy")
496-
assert.Contains(t, e.Hint, "basecamp recordings list Todo --in 999 --status active --sort created_at --direction desc --all")
496+
assert.Contains(t, e.Hint, "basecamp recordings list Todo --in 999 --status active --all")
497+
}
498+
499+
// A copy keeps its original's created_at, so no sort brings it to the top of
500+
// the listing: the hint says to find it by its title in the destination list.
501+
func TestTodosCopyRecoveryFindsTheCopyByItsTitle(t *testing.T) {
502+
transport := &mockTodoFilingTransport{postStatus: 502}
503+
run := runTodoFilingCommand(t, transport, NewTodosCmd(), "copy", "456", "--to-project", "Dest", "--to-list", "Later")
504+
e := output.AsError(run.err)
505+
require.NotNil(t, run.err)
506+
507+
assert.NotContains(t, e.Hint, "--sort")
508+
assert.Contains(t, e.Hint, `basecamp recordings list Todo --in 999 --status active --all, then look for "Task 456" in Later (#902); a copy has its original's title and dates`)
497509
}
498510

499511
// A copy keeps the source's status, so the listing that looks for it must
500-
// show that status, newest first and every page.
512+
// show that status, every page.
501513
func TestCopyRecoveryListingShowsTheCopyInItsStatus(t *testing.T) {
502-
assert.Equal(t, "basecamp recordings list Todo --in 999 --status active --sort created_at --direction desc --all",
514+
assert.Equal(t, "basecamp recordings list Todo --in 999 --status active --all",
503515
recordingCopyLookFor("Todo", 999, ""))
504-
assert.Equal(t, "basecamp recordings list Todolist --in 999 --status archived --sort created_at --direction desc --all",
516+
assert.Equal(t, "basecamp recordings list Todolist --in 999 --status archived --all",
505517
recordingCopyLookFor("Todolist", 999, "archived"))
506-
assert.Equal(t, "basecamp recordings list Kanban::Card --in 999 --status trashed --sort created_at --direction desc --all",
518+
assert.Equal(t, "basecamp recordings list Kanban::Card --in 999 --status trashed --all",
507519
recordingCopyLookFor("Kanban::Card", 999, "trashed"))
508520
}
509521

@@ -582,7 +594,7 @@ func TestTodosRefuseATodoNamedTwiceBeforeFilingAny(t *testing.T) {
582594
// would miss the copy and invite a second one.
583595
func TestFilingFollowUpsNameTheProfileAndAnAccountGivenThisRun(t *testing.T) {
584596
const scope = " --profile work --account 99999"
585-
todoListing := "basecamp recordings list Todo --in 999 --status active --sort created_at --direction desc --all" + scope
597+
todoListing := "basecamp recordings list Todo --in 999 --status active --all" + scope
586598
for name, tc := range map[string]struct {
587599
transport http.RoundTripper
588600
group func() *cobra.Command
@@ -612,12 +624,12 @@ func TestFilingFollowUpsNameTheProfileAndAnAccountGivenThisRun(t *testing.T) {
612624
"a list copy that may have started": {
613625
&mockTodoFilingTransport{postStatus: 502}, NewTodolistsCmd,
614626
[]string{"copy", "310", "--to-project", "Dest"},
615-
"basecamp recordings list Todolist --in 999 --status active --sort created_at --direction desc --all" + scope,
627+
"basecamp recordings list Todolist --in 999 --status active --all" + scope,
616628
},
617629
"a card copy that may have started": {
618630
&mockFilingTransport{postStatus: 502}, NewCardsCmd,
619631
[]string{"copy", "456", "--to-project", "Dest"},
620-
"basecamp recordings list Kanban::Card --in 999 --status active --sort created_at --direction desc --all" + scope,
632+
"basecamp recordings list Kanban::Card --in 999 --status active --all" + scope,
621633
},
622634
"a card move's breadcrumb": {
623635
&mockFilingTransport{statuses: []string{"completed"}}, NewCardsCmd,

0 commit comments

Comments
 (0)