Duck.ai: Terms and Conditions duplicate acceptance pixel#7987
Duck.ai: Terms and Conditions duplicate acceptance pixel#7987
Conversation
When a user accepts terms and conditions and had already accepted them previously, fire a pixel indicating a duplicate acceptance. The pixel varies based on whether Sync is enabled or not. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
...src/test/kotlin/com/duckduckgo/duckchat/impl/helper/RealDuckChatTermsOfServiceHandlerTest.kt
Outdated
Show resolved
Hide resolved
...chat-impl/src/main/java/com/duckduckgo/duckchat/impl/helper/DuckChatTermsOfServiceHandler.kt
Outdated
Show resolved
Hide resolved
duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/store/DuckChatDataStore.kt
Show resolved
Hide resolved
duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/pixel/DuckChatPixels.kt
Outdated
Show resolved
Hide resolved
The terms acceptance comes via REPORT_METRIC, not as a separate JS method. Move the handler invocation into sendReportMetricPixel so it triggers when USER_DID_ACCEPT_TERMS_AND_CONDITIONS is reported. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The handler call now lives inside the when block alongside pixel firing, so the test verifies both the handler delegation and the pixel fire. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RealDuckChatPixels depended on DuckChatTermsOfServiceHandler which depended on DuckChatPixels, causing a Dagger cycle. Fix by having the handler return a TermsAcceptanceResult instead of firing pixels directly, letting RealDuckChatPixels fire the duplicate pixels. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…/impl/helper/RealDuckChatTermsOfServiceHandlerTest.kt Co-authored-by: Josh Leibstein <joshliebe@gmail.com>
…mpl/helper/DuckChatTermsOfServiceHandler.kt Co-authored-by: Josh Leibstein <joshliebe@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
...chat-impl/src/main/java/com/duckduckgo/duckchat/impl/helper/DuckChatTermsOfServiceHandler.kt
Outdated
Show resolved
Hide resolved
joshliebe
left a comment
There was a problem hiding this comment.
Couldn’t repro but LGTM 🚀
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing handler for
userDidAcceptTermsAndConditionsJS method- Added a when-branch for METHOD_ACCEPT_TERMS_AND_CONDITIONS in processJsCallbackMessage that calls sendReportMetricPixel(USER_DID_ACCEPT_TERMS_AND_CONDITIONS), so standalone JS messages are no longer silently dropped.
Or push these changes by commenting:
@cursor push 0fef55cad8
Preview (0fef55cad8)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/helper/DuckChatJSHelper.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/helper/DuckChatJSHelper.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/helper/DuckChatJSHelper.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/helper/DuckChatJSHelper.kt
@@ -183,6 +183,11 @@
}
}
+ METHOD_ACCEPT_TERMS_AND_CONDITIONS -> {
+ duckChatPixels.sendReportMetricPixel(ReportMetric.USER_DID_ACCEPT_TERMS_AND_CONDITIONS)
+ null
+ }
+
METHOD_TOGGLE_PAGE_CONTEXT -> {
val isEnabled = data?.optBoolean(ENABLED)
if (isEnabled != null) {
diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/helper/RealDuckChatJSHelperTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/helper/RealDuckChatJSHelperTest.kt
--- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/helper/RealDuckChatJSHelperTest.kt
+++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/helper/RealDuckChatJSHelperTest.kt
@@ -25,6 +25,7 @@
import com.duckduckgo.duckchat.impl.ReportMetric.USER_DID_SELECT_FIRST_HISTORY_ITEM
import com.duckduckgo.duckchat.impl.ReportMetric.USER_DID_SUBMIT_FIRST_PROMPT
import com.duckduckgo.duckchat.impl.ReportMetric.USER_DID_SUBMIT_PROMPT
+import com.duckduckgo.duckchat.impl.ReportMetric.USER_DID_ACCEPT_TERMS_AND_CONDITIONS
import com.duckduckgo.duckchat.impl.ReportMetric.USER_DID_TAP_KEYBOARD_RETURN_KEY
import com.duckduckgo.duckchat.impl.helper.RealDuckChatJSHelper.Companion.DUCK_CHAT_FEATURE_NAME
import com.duckduckgo.duckchat.impl.helper.RealDuckChatJSHelper.Companion.METHOD_GET_PAGE_CONTEXT
@@ -1269,6 +1270,25 @@
}
@Test
+ fun whenUserDidAcceptTermsAndConditionsThenPixelSent() = runTest {
+ val featureName = "aiChat"
+ val method = "userDidAcceptTermsAndConditions"
+ val id = "123"
+
+ assertNull(
+ testee.processJsCallbackMessage(
+ featureName,
+ method,
+ id,
+ null,
+ pageContext = viewModel.updatedPageContext,
+ ),
+ )
+
+ verify(mockDuckChatPixels).sendReportMetricPixel(USER_DID_ACCEPT_TERMS_AND_CONDITIONS)
+ }
+
+ @Test
fun whenOpenKeyboardThenResponseSent() = runTest {
val featureName = "aiChat"
val method = "openKeyboard"
duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/helper/DuckChatJSHelper.kt
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Unhandled JS method silently drops terms acceptance calls
- Removed
userDidAcceptTermsAndConditionsfrom the standalone JS methods list since it had no handler in processJsCallbackMessage and the terms acceptance is exclusively handled via thereportMetricpathway, consistent with all other ReportMetric values.
- Removed
Or push these changes by commenting:
@cursor push 598f05a331
Preview (598f05a331)
diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandler.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandler.kt
--- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandler.kt
+++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandler.kt
@@ -62,7 +62,6 @@
"getAIChatPageContext",
"togglePageContextTelemetry",
"submitAIChatPageContext",
- "userDidAcceptTermsAndConditions",
)
}
}
diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandlerTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandlerTest.kt
--- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandlerTest.kt
+++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/messaging/DuckChatContentScopeJsMessageHandlerTest.kt
@@ -41,7 +41,7 @@
@Test
fun `only contains valid methods`() {
val methods = handler.methods
- assertTrue(methods.size == 14)
+ assertTrue(methods.size == 13)
assertTrue(methods[0] == "getAIChatNativeHandoffData")
assertTrue(methods[1] == "getAIChatNativeConfigValues")
assertTrue(methods[2] == "openAIChat")
@@ -55,7 +55,6 @@
assertTrue(methods[10] == "getAIChatPageContext")
assertTrue(methods[11] == "togglePageContextTelemetry")
assertTrue(methods[12] == "submitAIChatPageContext")
- assertTrue(methods[13] == "userDidAcceptTermsAndConditions")
}
private val callback = object : JsMessageCallback() {| "getAIChatPageContext", | ||
| "togglePageContextTelemetry", | ||
| "submitAIChatPageContext", | ||
| "userDidAcceptTermsAndConditions", |
There was a problem hiding this comment.
Unhandled JS method silently drops terms acceptance calls
Medium Severity
"userDidAcceptTermsAndConditions" is registered as a standalone JS method in DuckChatContentScopeJsMessageHandler.methods, but processJsCallbackMessage in RealDuckChatJSHelper has no case for it — it falls through to the else branch which silently logs and returns null. The terms acceptance logic only works via the reportMetric pathway. No other ReportMetric value is registered as a standalone method in the handler, making this inconsistent. If JS calls this method directly, the acceptance state won't be persisted and no pixel will fire.
Please tell me if this was useful or not with a 👍 or 👎.



Task/Issue URL: https://app.asana.com/1/137249556945/task/1213610121584461
Description
When a user accepts Duck.ai terms and conditions and had already accepted them previously, fire a pixel to track this duplicate acceptance. Two separate pixels are sent depending on whether Sync is enabled:
m_aichat_terms_accepted_duplicate_sync_onm_aichat_terms_accepted_duplicate_sync_offA new
DuckChatTermsOfServiceHandlerclass encapsulates this logic, keeping it out of the already-largeRealDuckChatJSHelper. The acceptance state is persisted in DataStore via a newDUCK_AI_TERMS_ACCEPTEDboolean key.Steps to test this PR
Duplicate T&C acceptance pixel
sync_onorsync_offdepending on Sync state)UI changes
None
Note
Low Risk
Low risk: adds a new persisted boolean flag and additional pixel firing paths without changing core chat or sync behavior.
Overview
Adds a new
USER_DID_ACCEPT_TERMS_AND_CONDITIONSreport metric and JS messaging method (userDidAcceptTermsAndConditions) to signal when the user accepts Duck.ai terms.On terms acceptance,
RealDuckChatPixelsnow persists aDUCK_AI_TERMS_ACCEPTEDflag and fires a base acceptance pixel, plus an additional duplicate-acceptance pixel when the flag was already set (DUCK_CHAT_TERMS_ACCEPTED_DUPLICATE_SYNC_ON/_SYNC_OFFdepending on Sync state) via a newDuckChatTermsOfServiceHandlerabstraction.Extends datastore and unit tests to cover the new persistence and pixel behavior.
Written by Cursor Bugbot for commit 5fbdeb6. This will update automatically on new commits. Configure here.