fix(remove): Remove works on foreign datasets, even if logbook missing

When removing a dataset, ignore logbook "not found" errors, since we're
removing the dataset anyway, it doesn't matter if something is missing.
Fmt the mock_client added in the previous PR.

cmd/add.go

@@ -9,8 +9,6 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// TODO: Tests.
-
 // NewAddCommand creates an add command
 func NewAddCommand(f Factory, ioStreams ioes.IOStreams) *cobra.Command {
 	o := &AddOptions{IOStreams: ioStreams}

cmd/remove_integration_test.go

@@ -542,3 +542,74 @@ func TestRemoveIfWorkingDirectoryIsNotFound(t *testing.T) {
 		t.Error(err)
 	}
 }
+
+// Test that a dataset can be removed even if the logbook is missing
+func TestRemoveEvenIfLogbookGone(t *testing.T) {
+	run := NewFSITestRunner(t, "qri_test_remove_no_logbook")
+	defer run.Delete()
+
+	workDir := run.CreateAndChdirToWorkDir("remove_no_logbook")
+
+	// Init as a linked directory
+	run.MustExec(t, "qri init --name remove_no_logbook --format csv")
+
+	// Save the new dataset
+	run.MustExec(t, "qri save")
+
+	// Go up one directory
+	parentDir := filepath.Dir(workDir)
+	os.Chdir(parentDir)
+
+	// Remove the logbook
+	logbookFile := filepath.Join(run.RepoRoot.rootPath, "qri/logbook.qfb")
+	if _, err := os.Stat(logbookFile); os.IsNotExist(err) {
+		t.Fatal("logbook does not exist")
+	}
+	err := os.Remove(logbookFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Remove all should still work, even though the logbook is gone.
+	if err := run.ExecCommand("qri remove --revisions=all me/remove_no_logbook"); err != nil {
+		t.Error(err)
+	}
+}
+
+// Test that an added dataset can be removed
+func TestRemoveEvenIfForeignDataset(t *testing.T) {
+	run := NewTestRunnerWithMockRemoteClient(t, "test_peer", "remove_foreign")
+	defer run.Delete()
+
+	// Save a foreign dataset
+	run.MustExec(t, "qri add other_peer/their_dataset")
+
+	// Remove all should still work, even though the dataset is foreign
+	if err := run.ExecCommand("qri remove --revisions=all other_peer/their_dataset"); err != nil {
+		t.Error(err)
+	}
+}
+
+// Test that an added dataset can be removed even if the logbook is missing
+func TestRemoveEvenIfForeignDatasetWithNoOplog(t *testing.T) {
+	run := NewTestRunnerWithMockRemoteClient(t, "test_peer", "remove_no_oplog")
+	defer run.Delete()
+
+	// Save a foreign dataset
+	run.MustExec(t, "qri add other_peer/their_dataset")
+
+	// Remove the logbook
+	logbookFile := filepath.Join(run.RepoRoot.rootPath, "qri/logbook.qfb")
+	if _, err := os.Stat(logbookFile); os.IsNotExist(err) {
+		t.Fatal("logbook does not exist")
+	}
+	err := os.Remove(logbookFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Remove all should still work, even though the dataset is foreign with no logbook
+	if err := run.ExecCommand("qri remove --revisions=all other_peer/their_dataset"); err != nil {
+		t.Error(err)
+	}
+}

lib/datasets.go

@@ -21,6 +21,7 @@ import (
 	"github.com/qri-io/qri/base/dsfs"
 	"github.com/qri-io/qri/dsref"
 	"github.com/qri-io/qri/fsi"
+	"github.com/qri-io/qri/logbook/oplog"
 	"github.com/qri-io/qri/p2p"
 	"github.com/qri-io/qri/repo"
 )
@@ -669,7 +670,11 @@ func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
 		// Write the deletion to the logbook.
 		book := r.inst.Repo().Logbook()
 		if err := book.WriteDatasetDelete(ctx, repo.ConvertToDsref(ref)); err != nil {
-			return err
+			// If the logbook is missing, it's not an error worth stopping for, since we're
+			// deleting the dataset anyway. This can happen from adding a foreign dataset.
+			if err != oplog.ErrNotFound {
+				return err
+			}
 		}
 		// remove the ref from the ref store
 		if err := r.inst.Repo().DeleteRef(ref); err != nil {

remote/mock_client.go

@@ -5,8 +5,8 @@ import (
 	"fmt"
 
 	"github.com/qri-io/dataset"
-	cfgtest "github.com/qri-io/qri/config/test"
 	"github.com/qri-io/qri/base/dsfs"
+	cfgtest "github.com/qri-io/qri/config/test"
 	"github.com/qri-io/qri/dsref"
 	"github.com/qri-io/qri/p2p"
 	"github.com/qri-io/qri/repo"
@@ -53,9 +53,8 @@ func (c *MockClient) AddDataset(ctx context.Context, ref *repo.DatasetRef, remot
 
 	// Construct a simple dataset
 	ds := dataset.Dataset{
-		Commit: &dataset.Commit {
-		},
-		Structure: &dataset.Structure {
+		Commit: &dataset.Commit{},
+		Structure: &dataset.Structure{
 			Format: "json",
 			Schema: dataset.BaseSchemaObject,
 		},