From dd2e325c24d43b1843356809adba0d1365df99cd Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Tue, 11 Apr 2023 12:52:54 -0400
Subject: [PATCH 1/9] Ensure admins can send true up telemetry, even if
 telemetry is disabled.

---
 server/channels/api4/license.go                     | 13 ++++++++-----
 server/channels/einterfaces/cloud.go                |  3 +++
 .../src/components/analytics/true_up_review.tsx     |  4 ----
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go
index 9911c241e1..02420a824d 100644
--- a/server/channels/api4/license.go
+++ b/server/channels/api4/license.go
@@ -354,15 +354,18 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 	// Do not send true-up review data if the user has already requested one for the quarter.
 	// And only send a true-up review via as a one-time telemetry request if telemetry is disabled.
 	telemetryEnabled := c.App.Config().LogSettings.EnableDiagnostics
-	if telemetryEnabled != nil && !*telemetryEnabled {
+	if telemetryEnabled != nil && *telemetryEnabled {
 		// Send telemetry data
 		c.App.Srv().GetTelemetryService().SendTelemetry(model.TrueUpReviewTelemetryName, profileMap)
-
-		// Update the review status to reflect the completion.
-		status.Completed = true
-		c.App.Srv().Store().TrueUpReview().Update(status)
+	} else {
+		// Telemetry is disabled, submit true up review profile via CWS.
+		c.App.Cloud().SubmitTrueUpReview(profileMap)
 	}
 
+	// Update the review status to reflect the completion.
+	status.Completed = true
+	c.App.Srv().Store().TrueUpReview().Update(status)
+
 	// Encode to string rather than byte[] otherwise json.Marshal will encode it further.
 	encodedData := b64.StdEncoding.EncodeToString(profileMapJson)
 	responseContent := struct {
diff --git a/server/channels/einterfaces/cloud.go b/server/channels/einterfaces/cloud.go
index 70cdc4676a..1dd2ea65ac 100644
--- a/server/channels/einterfaces/cloud.go
+++ b/server/channels/einterfaces/cloud.go
@@ -48,4 +48,7 @@ type CloudInterface interface {
 
 	SelfServeDeleteWorkspace(userID string, deletionRequest *model.WorkspaceDeletionRequest) error
 	SubscribeToNewsletter(userID string, req *model.SubscribeNewsletterRequest) error
+
+	// Used only for when a customer has telemetry disabled. In this scenario, true up review telemetry will be submitted via CWS.
+	SubmitTrueUpReview(trueUpReviewProfile map[string]any) error
 }
diff --git a/webapp/channels/src/components/analytics/true_up_review.tsx b/webapp/channels/src/components/analytics/true_up_review.tsx
index 5098999d63..c5ce7b38b2 100644
--- a/webapp/channels/src/components/analytics/true_up_review.tsx
+++ b/webapp/channels/src/components/analytics/true_up_review.tsx
@@ -223,10 +223,6 @@ const TrueUpReview: React.FC = () => {
         return null;
     }
 
-    if (telemetryEnabled) {
-        return null;
-    }
-
     pageVisited(TELEMETRY_CATEGORIES.TRUE_UP_REVIEW, 'pageview_true_up_review');
 
     return (

From 33d3c906543aa8513c4b9643844cfc44d5588120 Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Tue, 11 Apr 2023 14:15:56 -0400
Subject: [PATCH 2/9] Add mocks/layers.

---
 plugin/api_timer_layer_generated.go                |  2 +-
 plugin/hooks_timer_layer_generated.go              |  2 +-
 server/channels/api4/license.go                    |  6 +++++-
 server/channels/einterfaces/cloud.go               |  2 +-
 .../channels/einterfaces/mocks/CloudInterface.go   | 14 ++++++++++++++
 5 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/plugin/api_timer_layer_generated.go b/plugin/api_timer_layer_generated.go
index a084188c62..c54c6ac7bb 100644
--- a/plugin/api_timer_layer_generated.go
+++ b/plugin/api_timer_layer_generated.go
@@ -11,8 +11,8 @@ import (
 	"net/http"
 	timePkg "time"
 
-	"github.com/mattermost/mattermost-server/v6/server/channels/einterfaces"
 	"github.com/mattermost/mattermost-server/v6/model"
+	"github.com/mattermost/mattermost-server/v6/server/channels/einterfaces"
 )
 
 type apiTimerLayer struct {
diff --git a/plugin/hooks_timer_layer_generated.go b/plugin/hooks_timer_layer_generated.go
index 6093048d54..87e79ca7e6 100644
--- a/plugin/hooks_timer_layer_generated.go
+++ b/plugin/hooks_timer_layer_generated.go
@@ -11,8 +11,8 @@ import (
 	"net/http"
 	timePkg "time"
 
-	"github.com/mattermost/mattermost-server/v6/server/channels/einterfaces"
 	"github.com/mattermost/mattermost-server/v6/model"
+	"github.com/mattermost/mattermost-server/v6/server/channels/einterfaces"
 )
 
 type hooksTimerLayer struct {
diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go
index 02420a824d..985bada409 100644
--- a/server/channels/api4/license.go
+++ b/server/channels/api4/license.go
@@ -359,7 +359,11 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 		c.App.Srv().GetTelemetryService().SendTelemetry(model.TrueUpReviewTelemetryName, profileMap)
 	} else {
 		// Telemetry is disabled, submit true up review profile via CWS.
-		c.App.Cloud().SubmitTrueUpReview(profileMap)
+		err := c.App.Cloud().SubmitTrueUpReview(c.AppContext.Session().UserId, profileMap)
+		if err != nil {
+			c.SetJSONEncodingError(err)
+			return
+		}
 	}
 
 	// Update the review status to reflect the completion.
diff --git a/server/channels/einterfaces/cloud.go b/server/channels/einterfaces/cloud.go
index 1dd2ea65ac..fc5446cd34 100644
--- a/server/channels/einterfaces/cloud.go
+++ b/server/channels/einterfaces/cloud.go
@@ -50,5 +50,5 @@ type CloudInterface interface {
 	SubscribeToNewsletter(userID string, req *model.SubscribeNewsletterRequest) error
 
 	// Used only for when a customer has telemetry disabled. In this scenario, true up review telemetry will be submitted via CWS.
-	SubmitTrueUpReview(trueUpReviewProfile map[string]any) error
+	SubmitTrueUpReview(userID string, trueUpReviewProfile map[string]any) error
 }
diff --git a/server/channels/einterfaces/mocks/CloudInterface.go b/server/channels/einterfaces/mocks/CloudInterface.go
index f84300dbec..5800844da0 100644
--- a/server/channels/einterfaces/mocks/CloudInterface.go
+++ b/server/channels/einterfaces/mocks/CloudInterface.go
@@ -594,6 +594,20 @@ func (_m *CloudInterface) SelfServeDeleteWorkspace(userID string, deletionReques
 	return r0
 }
 
+// SubmitTrueUpReview provides a mock function with given fields: userID, trueUpReviewProfile
+func (_m *CloudInterface) SubmitTrueUpReview(userID string, trueUpReviewProfile map[string]interface{}) error {
+	ret := _m.Called(userID, trueUpReviewProfile)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, map[string]interface{}) error); ok {
+		r0 = rf(userID, trueUpReviewProfile)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // SubscribeToNewsletter provides a mock function with given fields: userID, req
 func (_m *CloudInterface) SubscribeToNewsletter(userID string, req *model.SubscribeNewsletterRequest) error {
 	ret := _m.Called(userID, req)

From 382894b41cff910a7c1babf643e8cd2d0ba46640 Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Wed, 12 Apr 2023 09:56:46 -0400
Subject: [PATCH 3/9] revert change to show true up review when telemetry is
 enabled, always send true up data to CWS for telemetry capture.

---
 server/channels/api4/license.go               | 19 ++++++-------------
 .../components/analytics/true_up_review.tsx   |  4 ++++
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go
index 985bada409..358960272f 100644
--- a/server/channels/api4/license.go
+++ b/server/channels/api4/license.go
@@ -351,19 +351,12 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Do not send true-up review data if the user has already requested one for the quarter.
-	// And only send a true-up review via as a one-time telemetry request if telemetry is disabled.
-	telemetryEnabled := c.App.Config().LogSettings.EnableDiagnostics
-	if telemetryEnabled != nil && *telemetryEnabled {
-		// Send telemetry data
-		c.App.Srv().GetTelemetryService().SendTelemetry(model.TrueUpReviewTelemetryName, profileMap)
-	} else {
-		// Telemetry is disabled, submit true up review profile via CWS.
-		err := c.App.Cloud().SubmitTrueUpReview(c.AppContext.Session().UserId, profileMap)
-		if err != nil {
-			c.SetJSONEncodingError(err)
-			return
-		}
+	// True-up is only enabled when telemetry is disabled. When telemetry is enabled, we already have all the data necessary
+	// for true-up reviews to be completed.
+	err = c.App.Cloud().SubmitTrueUpReview(c.AppContext.Session().UserId, profileMap)
+	if err != nil {
+		c.SetJSONEncodingError(err)
+		return
 	}
 
 	// Update the review status to reflect the completion.
diff --git a/webapp/channels/src/components/analytics/true_up_review.tsx b/webapp/channels/src/components/analytics/true_up_review.tsx
index c5ce7b38b2..5098999d63 100644
--- a/webapp/channels/src/components/analytics/true_up_review.tsx
+++ b/webapp/channels/src/components/analytics/true_up_review.tsx
@@ -223,6 +223,10 @@ const TrueUpReview: React.FC = () => {
         return null;
     }
 
+    if (telemetryEnabled) {
+        return null;
+    }
+
     pageVisited(TELEMETRY_CATEGORIES.TRUE_UP_REVIEW, 'pageview_true_up_review');
 
     return (

From 77e1fbfbc832a96eae76a45ed586b7cf3e31075d Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Wed, 12 Apr 2023 10:12:33 -0400
Subject: [PATCH 4/9] Change error upon failure of true up review submission to
 CWS.

---
 server/channels/api4/license.go | 2 +-
 server/i18n/en.json             | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go
index 358960272f..abbdf8bcfb 100644
--- a/server/channels/api4/license.go
+++ b/server/channels/api4/license.go
@@ -355,7 +355,7 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 	// for true-up reviews to be completed.
 	err = c.App.Cloud().SubmitTrueUpReview(c.AppContext.Session().UserId, profileMap)
 	if err != nil {
-		c.SetJSONEncodingError(err)
+		c.Err = model.NewAppError("requestTrueUpReview", "api.license.true_up_review.failed_to_submit", nil, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
diff --git a/server/i18n/en.json b/server/i18n/en.json
index e91fbf2656..d16e605ef0 100644
--- a/server/i18n/en.json
+++ b/server/i18n/en.json
@@ -2089,6 +2089,10 @@
     "id": "api.license.true_up_review.create_error",
     "translation": "Could not create true up status record"
   },
+  {
+    "id": "api.license.true_up_review.failed_to_submit",
+    "translation": "Failed to submit true up review profile to CWS."
+  },
   {
     "id": "api.license.true_up_review.get_status_error",
     "translation": "Could not get true up status records"

From eebd57ead11a89a2327ec747b0a09d41afe4a86a Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Wed, 12 Apr 2023 15:06:55 -0400
Subject: [PATCH 5/9] Add ok response code to hopefully fix tests.

---
 server/channels/api4/license.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go
index abbdf8bcfb..a1b7806dbf 100644
--- a/server/channels/api4/license.go
+++ b/server/channels/api4/license.go
@@ -370,6 +370,7 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 	}{Content: encodedData}
 	response, _ := json.Marshal(responseContent)
 
+	w.WriteHeader(http.StatusOK)
 	w.Write(response)
 }
 

From 0b731f4330eea1bc643ac8f73d9c8eedc65068bd Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Wed, 12 Apr 2023 16:22:10 -0400
Subject: [PATCH 6/9] fix tests.

---
 server/channels/api4/license_test.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/server/channels/api4/license_test.go b/server/channels/api4/license_test.go
index 08a9e57305..8769fae1fc 100644
--- a/server/channels/api4/license_test.go
+++ b/server/channels/api4/license_test.go
@@ -521,6 +521,14 @@ func TestTrueUpReviewStatus(t *testing.T) {
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
 	t.Run("returns 200 when status retrieved", func(t *testing.T) {
+		cloud := mocks.CloudInterface{}
+
+		cloudImpl := th.App.Srv().Cloud
+		defer func() {
+			th.App.Srv().Cloud = cloudImpl
+		}()
+		th.App.Srv().Cloud = &cloud
+
 		resp, err := th.SystemAdminClient.DoAPIGet("/license/review/status", "")
 		require.NoError(t, err)
 		require.Equal(t, http.StatusOK, resp.StatusCode)

From 7cc866ed89d1e3859470d5f97aac87f97c00331a Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Thu, 13 Apr 2023 09:34:48 -0400
Subject: [PATCH 7/9] actually fix tests through mocks.

---
 model/client4.go                     | 14 ++++++++++++++
 server/channels/api4/license_test.go | 22 +++++++++++++---------
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/model/client4.go b/model/client4.go
index d6cc62ba0f..74b65948b0 100644
--- a/model/client4.go
+++ b/model/client4.go
@@ -8803,3 +8803,17 @@ func (c *Client4) GetWorkTemplatesByCategory(category string) ([]*WorkTemplate,
 	err = json.NewDecoder(r.Body).Decode(&templates)
 	return templates, BuildResponse(r), err
 }
+
+func (c *Client4) SubmitTrueUpReview(req map[string]any) (*Response, error) {
+	reqBytes, err := json.Marshal(req)
+	if err != nil {
+		return nil, NewAppError("SubmitTrueUpReview", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+	r, err := c.DoAPIPostBytes(c.licenseRoute()+"/review", reqBytes)
+	if err != nil {
+		return BuildResponse(r), nil
+	}
+	defer closeBody(r)
+
+	return BuildResponse(r), nil
+}
diff --git a/server/channels/api4/license_test.go b/server/channels/api4/license_test.go
index 8769fae1fc..1a4b4a7803 100644
--- a/server/channels/api4/license_test.go
+++ b/server/channels/api4/license_test.go
@@ -484,7 +484,19 @@ func TestRequestTrueUpReview(t *testing.T) {
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
 	t.Run("returns status 200 when telemetry data sent", func(t *testing.T) {
-		resp, err := th.SystemAdminClient.DoAPIPost("/license/review", "")
+		th.Client.Login(th.SystemAdminUser.Email, th.SystemAdminUser.Password)
+
+		cloud := mocks.CloudInterface{}
+		cloud.Mock.On("SubmitTrueUpReview", mock.Anything, mock.Anything).Return(nil)
+
+		cloudImpl := th.App.Srv().Cloud
+		defer func() {
+			th.App.Srv().Cloud = cloudImpl
+		}()
+		th.App.Srv().Cloud = &cloud
+
+		var reviewProfile map[string]any
+		resp, err := th.Client.SubmitTrueUpReview(reviewProfile)
 		require.NoError(t, err)
 		require.Equal(t, http.StatusOK, resp.StatusCode)
 	})
@@ -521,14 +533,6 @@ func TestTrueUpReviewStatus(t *testing.T) {
 	th.App.Srv().SetLicense(model.NewTestLicense())
 
 	t.Run("returns 200 when status retrieved", func(t *testing.T) {
-		cloud := mocks.CloudInterface{}
-
-		cloudImpl := th.App.Srv().Cloud
-		defer func() {
-			th.App.Srv().Cloud = cloudImpl
-		}()
-		th.App.Srv().Cloud = &cloud
-
 		resp, err := th.SystemAdminClient.DoAPIGet("/license/review/status", "")
 		require.NoError(t, err)
 		require.Equal(t, http.StatusOK, resp.StatusCode)

From 27d959485e7c0e1b94d6c8054cbafcf898bcb02a Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Thu, 13 Apr 2023 09:54:58 -0400
Subject: [PATCH 8/9] move setup/teardown into each test.

---
 server/channels/api4/license_test.go | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/server/channels/api4/license_test.go b/server/channels/api4/license_test.go
index 1a4b4a7803..d673e13543 100644
--- a/server/channels/api4/license_test.go
+++ b/server/channels/api4/license_test.go
@@ -478,12 +478,11 @@ func TestRequestRenewalLink(t *testing.T) {
 }
 
 func TestRequestTrueUpReview(t *testing.T) {
-	th := Setup(t)
-	defer th.TearDown()
-
-	th.App.Srv().SetLicense(model.NewTestLicense())
-
 	t.Run("returns status 200 when telemetry data sent", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+		th.App.Srv().SetLicense(model.NewTestLicense())
+
 		th.Client.Login(th.SystemAdminUser.Email, th.SystemAdminUser.Password)
 
 		cloud := mocks.CloudInterface{}
@@ -502,6 +501,10 @@ func TestRequestTrueUpReview(t *testing.T) {
 	})
 
 	t.Run("returns 501 when ran by cloud user", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+		th.App.Srv().SetLicense(model.NewTestLicense())
+
 		th.App.Srv().SetLicense(model.NewTestLicense("cloud"))
 
 		resp, err := th.SystemAdminClient.DoAPIPost("/license/review", "")
@@ -512,12 +515,19 @@ func TestRequestTrueUpReview(t *testing.T) {
 	})
 
 	t.Run("returns 403 when user does not have permissions", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+		th.App.Srv().SetLicense(model.NewTestLicense())
+
 		resp, err := th.Client.DoAPIPost("/license/review", "")
 		require.Error(t, err)
 		require.Equal(t, http.StatusForbidden, resp.StatusCode)
 	})
 
 	t.Run("returns 400 when license is nil", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
 		th.App.Srv().SetLicense(nil)
 
 		resp, err := th.SystemAdminClient.DoAPIPost("/license/review", "")

From aa7939264fccaf044fb4085ea66f9b69a87cf82b Mon Sep 17 00:00:00 2001
From: Conor Macpherson <conor.macpherson@mattermost.com>
Date: Mon, 17 Apr 2023 15:09:54 -0400
Subject: [PATCH 9/9] Check if telemetry is disable, and only submit the true
 up profile if it is disabled.

---
 server/channels/api4/license.go | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/server/channels/api4/license.go b/server/channels/api4/license.go
index a1b7806dbf..ff63704774 100644
--- a/server/channels/api4/license.go
+++ b/server/channels/api4/license.go
@@ -351,12 +351,15 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// True-up is only enabled when telemetry is disabled. When telemetry is enabled, we already have all the data necessary
-	// for true-up reviews to be completed.
-	err = c.App.Cloud().SubmitTrueUpReview(c.AppContext.Session().UserId, profileMap)
-	if err != nil {
-		c.Err = model.NewAppError("requestTrueUpReview", "api.license.true_up_review.failed_to_submit", nil, err.Error(), http.StatusInternalServerError)
-		return
+	// True-up is only enabled when telemetry is disabled.
+	// When telemetry is enabled, we already have all the data necessary for true-up reviews to be completed.
+	telemetryEnabled := c.App.Config().LogSettings.EnableDiagnostics
+	if telemetryEnabled != nil && !*telemetryEnabled {
+		err = c.App.Cloud().SubmitTrueUpReview(c.AppContext.Session().UserId, profileMap)
+		if err != nil {
+			c.Err = model.NewAppError("requestTrueUpReview", "api.license.true_up_review.failed_to_submit", nil, err.Error(), http.StatusInternalServerError)
+			return
+		}
 	}
 
 	// Update the review status to reflect the completion.