Merge branch version/0-44-0-RC1 to adopt changes from PR #3009

internal/analytics/constants/constants.go

@@ -228,3 +228,12 @@ const UpdateErrorNoInstaller = "Downloaded update does not have installer"
 
 // UpdateErrorInstallPath is sent if the install path could not be detected
 const UpdateErrorInstallPath = "Could not detect install path"
+
+// CatInteractions is the event category used for tracking user interactions.
+const CatInteractions = "interactions"
+
+// ActVcsConflict is the event action sent when `state pull` results in a conflict.
+const ActVcsConflict = "vcs-conflict"
+
+// LabelVcsConflictMergeStrategyFailed is the label to use when a merge fails.
+const LabelVcsConflictMergeStrategyFailed = "Failed"

internal/runners/pull/pull.go

@@ -5,6 +5,8 @@ import (
 	"strings"
 
 	"github.com/ActiveState/cli/internal/analytics"
+	anaConst "github.com/ActiveState/cli/internal/analytics/constants"
+	"github.com/ActiveState/cli/internal/analytics/dimensions"
 	"github.com/ActiveState/cli/internal/config"
 	"github.com/ActiveState/cli/internal/constants"
 	"github.com/ActiveState/cli/internal/errs"
@@ -13,6 +15,7 @@ import (
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/primer"
 	"github.com/ActiveState/cli/internal/prompt"
+	"github.com/ActiveState/cli/internal/rtutils/ptr"
 	"github.com/ActiveState/cli/internal/runbits"
 	buildscriptRunbits "github.com/ActiveState/cli/internal/runbits/buildscript"
 	"github.com/ActiveState/cli/internal/runbits/commit"
@@ -117,27 +120,33 @@ func (p *Pull) Run(params *PullParams) (rerr error) {
 		// the remoteCommit ID. The commitID returned from MergeCommit with this
 		// strategy should just be the remote commit ID.
 		// If this call fails then we will try a recursive merge.
+		strategy := bpModel.MergeCommitStrategyFastForward
+
 		bp := model.NewBuildPlannerModel(p.auth)
 		params := &model.MergeCommitParams{
 			Owner:     remoteProject.Owner,
 			Project:   remoteProject.Project,
 			TargetRef: localCommit.String(),
 			OtherRef:  remoteCommit.String(),
-			Strategy:  bpModel.MergeCommitStrategyFastForward,
+			Strategy:  strategy,
 		}
 
 		resultCommit, mergeErr := bp.MergeCommit(params)
 		if mergeErr != nil {
 			logging.Debug("Merge with fast-forward failed with error: %s, trying recursive overwrite", mergeErr.Error())
-			c, err := p.performMerge(*remoteCommit, *localCommit, remoteProject, p.project.BranchName())
+			strategy = bpModel.MergeCommitStrategyRecursiveOverwriteOnConflict
+			c, err := p.performMerge(*remoteCommit, *localCommit, remoteProject, p.project.BranchName(), strategy)
 			if err != nil {
+				p.notifyMergeStrategy(anaConst.LabelVcsConflictMergeStrategyFailed, *localCommit, remoteProject)
 				return errs.Wrap(err, "performing merge commit failed")
 			}
 			resultingCommit = &c
 		} else {
 			logging.Debug("Fast-forward merge succeeded, setting commit ID to %s", resultCommit.String())
 			resultingCommit = &resultCommit
 		}
+
+		p.notifyMergeStrategy(string(strategy), *localCommit, remoteProject)
 	}
 
 	commitID, err := commitmediator.Get(p.project)
@@ -170,7 +179,7 @@ func (p *Pull) Run(params *PullParams) (rerr error) {
 	return nil
 }
 
-func (p *Pull) performMerge(remoteCommit, localCommit strfmt.UUID, namespace *project.Namespaced, branchName string) (strfmt.UUID, error) {
+func (p *Pull) performMerge(remoteCommit, localCommit strfmt.UUID, namespace *project.Namespaced, branchName string, strategy bpModel.MergeStrategy) (strfmt.UUID, error) {
 	// Re-enable in DX-2307.
 	//err := p.mergeBuildScript(strategies, remoteCommit)
 	//if err != nil {
@@ -189,7 +198,7 @@ func (p *Pull) performMerge(remoteCommit, localCommit strfmt.UUID, namespace *pr
 		Project:   namespace.Project,
 		TargetRef: localCommit.String(),
 		OtherRef:  remoteCommit.String(),
-		Strategy:  bpModel.MergeCommitStrategyRecursiveOverwriteOnConflict,
+		Strategy:  strategy,
 	}
 	resultCommit, err := bp.MergeCommit(params)
 	if err != nil {
@@ -253,3 +262,10 @@ func resolveRemoteProject(prj *project.Project) (*project.Namespaced, error) {
 
 	return ns, nil
 }
+
+func (p *Pull) notifyMergeStrategy(strategy string, commitID strfmt.UUID, namespace *project.Namespaced) {
+	p.analytics.EventWithLabel(anaConst.CatInteractions, anaConst.ActVcsConflict, strategy, &dimensions.Values{
+		CommitID:         ptr.To(commitID.String()),
+		ProjectNameSpace: ptr.To(namespace.String()),
+	})
+}

test/integration/pull_int_test.go

@@ -6,10 +6,13 @@ import (
 	"runtime"
 	"testing"
 
+	"github.com/ActiveState/cli/internal/analytics/client/sync/reporters"
+	anaConst "github.com/ActiveState/cli/internal/analytics/constants"
 	"github.com/ActiveState/cli/internal/constants"
 	"github.com/ActiveState/cli/internal/fileutils"
 	"github.com/ActiveState/cli/internal/testhelpers/e2e"
 	"github.com/ActiveState/cli/internal/testhelpers/tagsuite"
+	bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model"
 	"github.com/ActiveState/cli/pkg/platform/runtime/buildscript"
 	"github.com/ActiveState/cli/pkg/project"
 	"github.com/ActiveState/cli/pkg/projectfile" // remove in DX-2307
@@ -38,6 +41,8 @@ func (suite *PullIntegrationTestSuite) TestPull() {
 	//suite.Require().True(fileutils.DirExists(projectConfigDir))
 	//suite.Assert().True(fileutils.FileExists(filepath.Join(projectConfigDir, constants.CommitIdFileName)))
 
+	suite.assertMergeStrategyNotification(ts, string(bpModel.MergeCommitStrategyFastForward))
+
 	cp = ts.Spawn("pull")
 	cp.Expect("already up to date")
 	cp.ExpectExitCode(0)
@@ -83,6 +88,8 @@ func (suite *PullIntegrationTestSuite) TestPull_Merge() {
 	cp = ts.SpawnCmd("bash", "-c", fmt.Sprintf("cd %s && %s history | head -n 10", wd, exe))
 	cp.Expect("Merged")
 	cp.ExpectExitCode(0)
+
+	suite.assertMergeStrategyNotification(ts, string(bpModel.MergeCommitStrategyRecursiveOverwriteOnConflict))
 }
 
 func (suite *PullIntegrationTestSuite) TestMergeBuildScript() {
@@ -124,6 +131,14 @@ func (suite *PullIntegrationTestSuite) TestMergeBuildScript() {
 	suite.Assert().Contains(string(bytes), ">>>>>>>", "No merge conflict markers are in build script")
 }
 
+func (suite *PullIntegrationTestSuite) assertMergeStrategyNotification(ts *e2e.Session, strategy string) {
+	conflictEvents := filterEvents(parseAnalyticsEvents(suite, ts), func(e reporters.TestLogEntry) bool {
+		return e.Category == anaConst.CatInteractions && e.Action == anaConst.ActVcsConflict
+	})
+	suite.Assert().Equal(1, len(conflictEvents), "Should have a single VCS Conflict event report")
+	suite.Assert().Equal(strategy, conflictEvents[0].Label)
+}
+
 func TestPullIntegrationTestSuite(t *testing.T) {
 	suite.Run(t, new(PullIntegrationTestSuite))
 }