Skip to content

Commit d31091d

Browse files
authored
SERP Settings Sync: Remove KBG flag if feature enabled (#7014)
Task/Issue URL: https://app.asana.com/1/137249556945/project/72649045549333/task/1211702809875729?focus=true ### Description Added a conditional check for the `serpSettingsSync` feature flag before applying the DuckAI SERP hiding logic to ensure when we complete the rest of the work we're getting the final behaviour we expect and the kbg param is not inteferring with anyhing. `serpSettingsSync` is now off by default so we do not mess with kbg for internal users until the rest of the work is complete. ### Steps to test this PR _KBG Param_ - [x] Ensure Duck.ai is enabled in native settings - [x] Perform a search e.g. FPL - [x] Check logs that the search url contains `kbg=-1`​ - [x] Enable `serpSettingsSync`​ feature flag (off by default for now) - [x] Perform a search e.g. FPL - [x] Check logs that the search url does **not** contain `kbg=-1`​ ### UI changes N/A
1 parent 8e18447 commit d31091d

File tree

5 files changed

+36
-4
lines changed

5 files changed

+36
-4
lines changed

app/src/main/java/com/duckduckgo/app/browser/DuckDuckGoRequestRewriter.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import com.duckduckgo.common.utils.AppUrl.ParamKey
2424
import com.duckduckgo.common.utils.AppUrl.ParamValue
2525
import com.duckduckgo.duckchat.api.DuckChat
2626
import com.duckduckgo.experiments.api.VariantManager
27+
import com.duckduckgo.settings.api.SettingsPageFeature
2728
import logcat.logcat
2829

2930
interface RequestRewriter {
@@ -39,6 +40,7 @@ class DuckDuckGoRequestRewriter(
3940
private val appReferrerDataStore: AppReferrerDataStore,
4041
private val duckChat: DuckChat,
4142
private val androidConfigFeatures: AndroidBrowserConfigFeature,
43+
private val settingsPageFeature: SettingsPageFeature,
4244
) : RequestRewriter {
4345

4446
private val hideDuckAiSerpKillSwitch by lazy { androidConfigFeatures.hideDuckAiInSerpKillSwitch().isEnabled() }
@@ -80,8 +82,11 @@ class DuckDuckGoRequestRewriter(
8082
val sourceValue = if (appReferrerDataStore.installedFromEuAuction) ParamValue.SOURCE_EU_AUCTION else ParamValue.SOURCE
8183

8284
builder.appendQueryParameter(ParamKey.HIDE_SERP, ParamValue.HIDE_SERP)
83-
if (!duckChat.isEnabled() && hideDuckAiSerpKillSwitch) {
84-
builder.appendQueryParameter(ParamKey.HIDE_DUCK_AI, ParamValue.HIDE_DUCK_AI)
85+
if (!settingsPageFeature.serpSettingsSync().isEnabled()) {
86+
// Once serpSettingsSync feature is permanently enabled this can be removed.
87+
if (!duckChat.isEnabled() && hideDuckAiSerpKillSwitch) {
88+
builder.appendQueryParameter(ParamKey.HIDE_DUCK_AI, ParamValue.HIDE_DUCK_AI)
89+
}
8590
}
8691
builder.appendQueryParameter(ParamKey.SOURCE, sourceValue)
8792
}

app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ import com.duckduckgo.privacy.config.api.AmpLinks
9292
import com.duckduckgo.privacy.config.api.Gpc
9393
import com.duckduckgo.privacy.config.api.TrackingParameters
9494
import com.duckduckgo.request.filterer.api.RequestFilterer
95+
import com.duckduckgo.settings.api.SettingsPageFeature
9596
import com.duckduckgo.subscriptions.api.Subscriptions
9697
import com.duckduckgo.user.agent.api.UserAgentProvider
9798
import dagger.Module
@@ -113,8 +114,17 @@ class BrowserModule {
113114
appReferrerDataStore: AppReferrerDataStore,
114115
duckChat: DuckChat,
115116
androidBrowserConfigFeature: AndroidBrowserConfigFeature,
117+
settingsPageFeature: SettingsPageFeature,
116118
): RequestRewriter {
117-
return DuckDuckGoRequestRewriter(urlDetector, statisticsStore, variantManager, appReferrerDataStore, duckChat, androidBrowserConfigFeature)
119+
return DuckDuckGoRequestRewriter(
120+
urlDetector,
121+
statisticsStore,
122+
variantManager,
123+
appReferrerDataStore,
124+
duckChat,
125+
androidBrowserConfigFeature,
126+
settingsPageFeature,
127+
)
118128
}
119129

120130
@Provides

app/src/test/java/com/duckduckgo/app/browser/DuckDuckGoRequestRewriterTest.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import com.duckduckgo.duckchat.api.DuckChat
3030
import com.duckduckgo.experiments.api.VariantManager
3131
import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
3232
import com.duckduckgo.feature.toggles.api.Toggle.State
33+
import com.duckduckgo.settings.api.SettingsPageFeature
3334
import org.junit.Assert.*
3435
import org.junit.Before
3536
import org.junit.Test
@@ -46,6 +47,7 @@ class DuckDuckGoRequestRewriterTest {
4647
private val mockVariantManager: VariantManager = mock()
4748
private val mockAppReferrerDataStore: AppReferrerDataStore = mock()
4849
private val duckChat: DuckChat = mock()
50+
private val settingsPageFeature: SettingsPageFeature = FakeFeatureToggleFactory.create(SettingsPageFeature::class.java)
4951
private val androidBrowserConfigFeature: AndroidBrowserConfigFeature = FakeFeatureToggleFactory.create(AndroidBrowserConfigFeature::class.java)
5052
private lateinit var builder: Uri.Builder
5153

@@ -64,6 +66,7 @@ class DuckDuckGoRequestRewriterTest {
6466
mockAppReferrerDataStore,
6567
duckChat,
6668
androidBrowserConfigFeature,
69+
settingsPageFeature,
6770
)
6871
builder = Uri.Builder()
6972
}
@@ -152,6 +155,18 @@ class DuckDuckGoRequestRewriterTest {
152155
assertFalse(uri.queryParameterNames.contains(ParamKey.HIDE_DUCK_AI))
153156
}
154157

158+
@Test
159+
fun whenSerpSettingsSyncIsEnabledThenDoNotHideDuckAi() {
160+
settingsPageFeature.serpSettingsSync().setRawStoredState(State(true))
161+
whenever(duckChat.isEnabled()).thenReturn(false)
162+
androidBrowserConfigFeature.hideDuckAiInSerpKillSwitch().setRawStoredState(State(true))
163+
164+
testee.addCustomQueryParams(builder)
165+
166+
val uri = builder.build()
167+
assertFalse(uri.queryParameterNames.contains(ParamKey.HIDE_DUCK_AI))
168+
}
169+
155170
@Test
156171
fun whenShouldRewriteRequestAndUrlIsSerpQueryThenReturnTrue() {
157172
val uri = "http://duckduckgo.com/?q=weather".toUri()

app/src/test/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import com.duckduckgo.duckchat.api.DuckChat
2828
import com.duckduckgo.experiments.api.VariantManager
2929
import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
3030
import com.duckduckgo.feature.toggles.api.Toggle.State
31+
import com.duckduckgo.settings.api.SettingsPageFeature
3132
import org.junit.Assert.*
3233
import org.junit.Before
3334
import org.junit.Test
@@ -43,6 +44,7 @@ class QueryUrlConverterTest {
4344
private val mockAppReferrerDataStore: AppReferrerDataStore = mock()
4445
private val duckChat: DuckChat = mock()
4546
private val androidBrowserConfigFeature: AndroidBrowserConfigFeature = FakeFeatureToggleFactory.create(AndroidBrowserConfigFeature::class.java)
47+
private val settingsPageFeature: SettingsPageFeature = FakeFeatureToggleFactory.create(SettingsPageFeature::class.java)
4648
private val requestRewriter =
4749
DuckDuckGoRequestRewriter(
4850
DuckDuckGoUrlDetectorImpl(),
@@ -51,6 +53,7 @@ class QueryUrlConverterTest {
5153
mockAppReferrerDataStore,
5254
duckChat,
5355
androidBrowserConfigFeature,
56+
settingsPageFeature,
5457
)
5558
private val testee: QueryUrlConverter = QueryUrlConverter(requestRewriter)
5659

settings/settings-api/src/main/java/com/duckduckgo/settings/api/SettingsPageFeature.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,5 @@ interface SettingsPageFeature {
3636
fun hideAiGeneratedImagesOption(): Toggle
3737

3838
@Toggle.DefaultValue(DefaultFeatureValue.FALSE)
39-
@Toggle.InternalAlwaysEnabled
4039
fun serpSettingsSync(): Toggle
4140
}

0 commit comments

Comments
 (0)