Reorder data extraction, define review profile in-place rather than sectioned for each data domain

api4/license.go

@@ -301,31 +301,31 @@ func getPrevTrialLicense(c *Context, w http.ResponseWriter, r *http.Request) {
 }
 
 func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
+	// Only admins can request a true up review.
+	if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
+		c.SetPermissionError(model.PermissionManageLicenseInformation)
+		return
+	}
+
+	if c.App.Cloud() == nil {
+		c.Err = model.NewAppError("requestRenewalLink", "api.license.upgrade_needed.app_error", nil, "", http.StatusForbidden)
+		return
+	}
+
 	license := c.App.Channels().License()
 	if license == nil {
 		http.Error(w, "A License is required to perform a true-up review", http.StatusBadRequest)
 		return
 	}
 
+	// Subscription Data
 	userId := c.AppContext.Session().UserId
 	subscription, err := c.App.Cloud().GetSubscription(userId)
-	if err != nil {
+	if err != nil || subscription == nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
-	reviewProfile := model.TrueUpReviewProfile{}
-
-	// Server Data
-	reviewProfile.ServerId = c.App.TelemetryId()
-	reviewProfile.ServerVersion = model.CurrentVersion
-	reviewProfile.ServerInstallationType = os.Getenv(telemetry.EnvVarInstallType)
-
-	// License Data
-	reviewProfile.LicenseId = license.Id
-	reviewProfile.LicensedSeats = subscription.Seats
-	reviewProfile.LicensePlan = license.SkuName
-
 	// Customer Info & Usage Analytics
 	activeUserCount, err := c.App.Srv().Store().Status().GetTotalActiveUsersCount()
 	if err != nil {
@@ -333,9 +333,6 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	reviewProfile.CustomerName = license.Customer.Name
-	reviewProfile.ActiveUsers = activeUserCount
-
 	// Webhook, calls, boards, and playbook counts
 	incomingWebhookCount, err := c.App.Srv().Store().Webhook().GetIncomingTotal()
 	if err != nil {
@@ -348,9 +345,6 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	reviewProfile.TotalIncomingWebhooks = incomingWebhookCount
-	reviewProfile.TotalOutgoingWebhooks = outgoingWebhookCount
-
 	// Plugin Data
 	trueUpReviewPlugins := model.TrueUpReviewPlugins{
 		ActivePluginNames:   []string{},
@@ -368,9 +362,8 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 			trueUpReviewPlugins.TotalInactivePlugins += 1
 		}
 	}
-	reviewProfile.Plugins = trueUpReviewPlugins
 
-	// Authentication Data
+	// Authentication Features
 	mfaUsed := c.App.Config().ServiceSettings.EnforceMultifactorAuthentication
 	ldapUsed := c.App.Config().LdapSettings.Enable
 	samlUsed := c.App.Config().SamlSettings.Enable
@@ -385,13 +378,28 @@ func requestTrueUpReview(c *Context, w http.ResponseWriter, r *http.Request) {
 		model.TrueUpReviewAuthFeatureGuestAccess: guessAccessAllowed,
 	}
 
-	reviewProfile.AuthenticationFeatures = []string{}
+	authFeatureList := []string{}
 	for feature, used := range authFeatures {
 		if used != nil && *used {
-			reviewProfile.AuthenticationFeatures = append(reviewProfile.AuthenticationFeatures, feature)
+			authFeatureList = append(authFeatureList, feature)
 		}
 	}
 
+	reviewProfile := model.TrueUpReviewProfile{
+		ServerId:               c.App.TelemetryId(),
+		ServerVersion:          model.CurrentVersion,
+		ServerInstallationType: os.Getenv(telemetry.EnvVarInstallType),
+		LicenseId:              license.Id,
+		LicensedSeats:          subscription.Seats,
+		LicensePlan:            license.SkuName,
+		CustomerName:           license.Customer.Name,
+		ActiveUsers:            activeUserCount,
+		TotalIncomingWebhooks:  incomingWebhookCount,
+		TotalOutgoingWebhooks:  outgoingWebhookCount,
+		Plugins:                trueUpReviewPlugins,
+		AuthenticationFeatures: authFeatureList,
+	}
+
 	// Convert true up review profile struct to map
 	var telemetryProperties map[string]interface{}
 	reviewProfileJson, err := json.Marshal(reviewProfile)

api4/license_test.go

@@ -330,3 +330,38 @@ func TestRequestRenewalLink(t *testing.T) {
 		require.Equal(t, http.StatusBadRequest, resp.StatusCode)
 	})
 }
+
+func TestRequestTrueUpReview(t *testing.T) {
+	th := Setup(t)
+	defer th.TearDown()
+
+	subscription := &model.Subscription{
+		ID:          "MySubscriptionID",
+		CustomerID:  "MyCustomer",
+		ProductID:   "SomeProductId",
+		AddOns:      []string{},
+		StartAt:     1000000000,
+		EndAt:       2000000000,
+		CreateAt:    1000000000,
+		Seats:       10,
+		IsFreeTrial: "true",
+		DNS:         "some.dns.server",
+		TrialEndAt:  2000000000,
+		LastInvoice: &model.Invoice{},
+	}
+
+	th.App.Srv().SetLicense(model.NewTestLicense())
+
+	cloud := mocks.CloudInterface{}
+	cloud.Mock.On("GetSubscription", mock.Anything).Return(subscription, nil)
+
+	cloudImpl := th.App.Srv().Cloud
+	th.App.Srv().Cloud = &cloud
+	defer func() {
+		th.App.Srv().Cloud = cloudImpl
+	}()
+
+	resp, err := th.SystemAdminClient.DoAPIPost("/license/review", "")
+	require.Nil(t, err)
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+}