From 0c6ebd520cfee181a80286cf56b9d3cd5d6d9fa9 Mon Sep 17 00:00:00 2001
From: Jonathan Breedlove <breedloj@amazon.com>
Date: Sun, 7 Jul 2024 21:28:58 -0700
Subject: [PATCH] Emit telemetry change event before persisting setting (#4637)

---
 .../jetbrains/settings/AwsSettings.kt         |  2 +-
 .../jetbrains/settings/AwsSettingsTest.kt     | 75 +++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/AwsSettingsTest.kt

diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/AwsSettings.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/AwsSettings.kt
index 3a566aa474..0455264938 100644
--- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/AwsSettings.kt
+++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/settings/AwsSettings.kt
@@ -47,11 +47,11 @@ class DefaultAwsSettings : PersistentStateComponent<AwsConfiguration>, AwsSettin
     override var isTelemetryEnabled: Boolean
         get() = state.isTelemetryEnabled ?: true
         set(value) {
-            state.isTelemetryEnabled = value
             val enablementElement = if (value) "aws_enabledTelemetry" else "aws_disabledTelemetry"
             TelemetryService.getInstance().setTelemetryEnabled(value) {
                 UiTelemetry.click(null as Project?, enablementElement)
             }
+            state.isTelemetryEnabled = value
         }
 
     override var promptedForTelemetry: Boolean
diff --git a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/AwsSettingsTest.kt b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/AwsSettingsTest.kt
new file mode 100644
index 0000000000..27dfe4d26e
--- /dev/null
+++ b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/settings/AwsSettingsTest.kt
@@ -0,0 +1,75 @@
+// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package software.aws.toolkits.jetbrains.settings
+
+import com.intellij.openapi.application.ApplicationManager
+import com.intellij.testFramework.DisposableRule
+import com.intellij.testFramework.ProjectRule
+import com.intellij.testFramework.replaceService
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Rule
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import org.mockito.kotlin.argumentCaptor
+import org.mockito.kotlin.inOrder
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.spy
+import software.aws.toolkits.core.telemetry.TelemetryBatcher
+import software.aws.toolkits.core.telemetry.TelemetryPublisher
+import software.aws.toolkits.jetbrains.services.telemetry.NoOpPublisher
+import software.aws.toolkits.jetbrains.services.telemetry.TelemetryService
+
+class AwsSettingsTest {
+    private class TestTelemetryService(
+        publisher: TelemetryPublisher = NoOpPublisher(),
+        batcher: TelemetryBatcher
+    ) : TelemetryService(publisher, batcher)
+
+    @Rule
+    @JvmField
+    val projectRule = ProjectRule()
+
+    @Rule
+    @JvmField
+    val disposableRule = DisposableRule()
+
+    private lateinit var telemetryService: TelemetryService
+    private lateinit var batcher: TelemetryBatcher
+    private lateinit var awsSettings: DefaultAwsSettings
+    private lateinit var awsConfiguration: AwsConfiguration
+
+    @BeforeEach
+    fun setup() {
+        batcher = mock()
+        telemetryService = spy(TestTelemetryService(batcher = batcher))
+        awsSettings = spy(DefaultAwsSettings())
+        awsConfiguration = spy(AwsConfiguration())
+        awsSettings.loadState(awsConfiguration)
+        ApplicationManager.getApplication().replaceService(TelemetryService::class.java, telemetryService, disposableRule.disposable)
+        ApplicationManager.getApplication().replaceService(AwsSettings::class.java, awsSettings, disposableRule.disposable)
+    }
+
+    @Test
+    fun `telemetry event batched before setting isTelemetryEnabled to false`() {
+        verifyTelemetryEventOrder(false)
+    }
+
+    @Test
+    fun `telemetry event batched before setting isTelemetryEnabled to true`() {
+        verifyTelemetryEventOrder(true)
+    }
+
+    private fun verifyTelemetryEventOrder(value: Boolean) {
+        val inOrder = inOrder(telemetryService, batcher, awsConfiguration)
+        val changeCaptor = argumentCaptor<Boolean>()
+        val onChangeEventCaptor = argumentCaptor<(Boolean) -> Unit>()
+
+        AwsSettings.getInstance().isTelemetryEnabled = value
+
+        inOrder.verify(telemetryService).setTelemetryEnabled(changeCaptor.capture(), onChangeEventCaptor.capture())
+        assertThat(changeCaptor.firstValue).isEqualTo(value)
+        inOrder.verify(batcher).onTelemetryEnabledChanged(value, onChangeEventCaptor.firstValue)
+        inOrder.verify(awsConfiguration).isTelemetryEnabled = value
+    }
+}