campaigns: restore pre-3.20 instance compatibility (#370)

The author name and e-mail fields were always sent, but the schema
Sourcegraph 3.19 uses to validate changeset specs don't include these
fields. Since we're doing version detection for feature flags (quirks)
now anyway, let's use that to not set the author name and e-mail if
they're not explicitly provided when talking to a 3.19 instance.

If the user provides an author name and/or e-mail and sends a changeset
spec to a pre-3.20 instance, the request will fail, but since those are
3.20 features that's OK.

Author: LawnGnome

CHANGELOG.md

@@ -17,6 +17,8 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Fixed
 
+- Restored backward compatibility when creating campaigns against Sourcegraph 3.19, provided author details are not provided in the campaign spec. [#370](https://github.com/sourcegraph/src-cli/pull/370)
+
 ### Removed
 
 ## 3.21.6

internal/campaigns/changeset_spec.go

@@ -28,6 +28,6 @@ type CreatedChangeset struct {
 type GitCommitDescription struct {
 	Message     string `json:"message"`
 	Diff        string `json:"diff"`
-	AuthorName  string `json:"authorName"`
-	AuthorEmail string `json:"authorEmail"`
+	AuthorName  string `json:"authorName,omitempty"`
+	AuthorEmail string `json:"authorEmail,omitempty"`
 }

internal/campaigns/executor.go

@@ -85,12 +85,13 @@ func (ts *TaskStatus) IsCompleted() bool {
 type executor struct {
 	ExecutorOpts
 
-	cache   ExecutionCache
-	client  api.Client
-	logger  *LogManager
-	creator *WorkspaceCreator
-	tasks   sync.Map
-	tempDir string
+	cache    ExecutionCache
+	client   api.Client
+	features featureFlags
+	logger   *LogManager
+	creator  *WorkspaceCreator
+	tasks    sync.Map
+	tempDir  string
 
 	par           *parallel.Run
 	doneEnqueuing chan struct{}
@@ -99,12 +100,13 @@ type executor struct {
 	specsMu sync.Mutex
 }
 
-func newExecutor(opts ExecutorOpts, client api.Client) *executor {
+func newExecutor(opts ExecutorOpts, client api.Client, features featureFlags) *executor {
 	return &executor{
 		ExecutorOpts:  opts,
 		cache:         opts.Cache,
 		creator:       opts.Creator,
 		client:        client,
+		features:      features,
 		doneEnqueuing: make(chan struct{}),
 		logger:        NewLogManager(opts.TempDir, opts.KeepLogs),
 		tempDir:       opts.TempDir,
@@ -208,7 +210,7 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
 				diff = result.Commits[0].Diff
 			}
 
-			spec := createChangesetSpec(task, diff)
+			spec := createChangesetSpec(task, diff, x.features)
 
 			status.Cached = true
 			status.ChangesetSpec = spec
@@ -267,7 +269,7 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
 	}
 
 	// Build the changeset spec.
-	spec := createChangesetSpec(task, string(diff))
+	spec := createChangesetSpec(task, string(diff), x.features)
 
 	status.ChangesetSpec = spec
 	x.updateTaskStatus(task, status)
@@ -305,16 +307,18 @@ func reachedTimeout(cmdCtx context.Context, err error) bool {
 	return errors.Is(err, context.DeadlineExceeded)
 }
 
-func createChangesetSpec(task *Task, diff string) *ChangesetSpec {
+func createChangesetSpec(task *Task, diff string, features featureFlags) *ChangesetSpec {
 	repo := task.Repository.Name
 
 	var authorName string
 	var authorEmail string
 
 	if task.Template.Commit.Author == nil {
-		// user did not provide author info, so use defaults
-		authorName = "Sourcegraph"
-		authorEmail = "[email protected]"
+		if features.includeAutoAuthorDetails {
+			// user did not provide author info, so use defaults
+			authorName = "Sourcegraph"
+			authorEmail = "[email protected]"
+		}
 	} else {
 		authorName = task.Template.Commit.Author.Name
 		authorEmail = task.Template.Commit.Author.Email

internal/campaigns/executor_test.go

@@ -144,7 +144,7 @@ func TestExecutor_Integration(t *testing.T) {
 				opts.Timeout = 5 * time.Second
 			}
 
-			executor := newExecutor(opts, client)
+			executor := newExecutor(opts, client, featuresAllEnabled())
 
 			template := &ChangesetTemplate{}
 			for _, r := range tc.repos {

internal/campaigns/features.go

@@ -0,0 +1,32 @@
+package campaigns
+
+import (
+	"github.com/pkg/errors"
+	"github.com/sourcegraph/src-cli/internal/api"
+)
+
+// featureFlags represent features that are only available on certain
+// Sourcegraph versions and we therefore have to detect at runtime.
+type featureFlags struct {
+	includeAutoAuthorDetails bool
+	useGzipCompression       bool
+}
+
+func (ff *featureFlags) setFromVersion(version string) error {
+	for _, feature := range []struct {
+		flag       *bool
+		constraint string
+		minDate    string
+	}{
+		{&ff.includeAutoAuthorDetails, ">= 3.20.0", "2020-09-10"},
+		{&ff.useGzipCompression, ">= 3.21.0", "2020-10-12"},
+	} {
+		value, err := api.CheckSourcegraphVersion(version, feature.constraint, feature.minDate)
+		if err != nil {
+			return errors.Wrap(err, "failed to check version returned by Sourcegraph")
+		}
+		*feature.flag = value
+	}
+
+	return nil
+}

internal/campaigns/features_test.go

@@ -0,0 +1,8 @@
+package campaigns
+
+func featuresAllEnabled() featureFlags {
+	return featureFlags{
+		includeAutoAuthorDetails: true,
+		useGzipCompression:       true,
+	}
+}

internal/campaigns/service.go

@@ -17,9 +17,9 @@ import (
 )
 
 type Service struct {
-	allowUnsupported   bool
-	client             api.Client
-	useGzipCompression bool
+	allowUnsupported bool
+	client           api.Client
+	features         featureFlags
 }
 
 type ServiceOpts struct {
@@ -71,17 +71,11 @@ func (svc *Service) DetermineFeatureFlags(ctx context.Context) error {
 		return errors.Wrap(err, "failed to query Sourcegraph version to check for available features")
 	}
 
-	supportsGzip, err := api.CheckSourcegraphVersion(version, ">= 3.21.0", "2020-10-12")
-	if err != nil {
-		return errors.Wrap(err, "failed to check version returned by Sourcegraph")
-	}
-	svc.useGzipCompression = supportsGzip
-
-	return nil
+	return svc.features.setFromVersion(version)
 }
 
 func (svc *Service) newRequest(query string, vars map[string]interface{}) api.Request {
-	if svc.useGzipCompression {
+	if svc.features.useGzipCompression {
 		return svc.client.NewGzippedRequest(query, vars)
 	}
 	return svc.client.NewRequest(query, vars)
@@ -201,7 +195,7 @@ type ExecutorOpts struct {
 }
 
 func (svc *Service) NewExecutor(opts ExecutorOpts) Executor {
-	return newExecutor(opts, svc.client)
+	return newExecutor(opts, svc.client, svc.features)
 }
 
 func (svc *Service) NewWorkspaceCreator(dir string, cleanArchives bool) *WorkspaceCreator {