Skip to content

Commit 3ad788b

Browse files
aitorvsaitorvs
authored
Fix query change pixels (#1065)
* Removed the OS and device form factory from RQ pixels * Overrode the user agent for RQ pixel API calls * Fire rq.0 upon refreshing while in SERP * Fine-tuned the omnibar selection logic to match iOS and meet requirements for sending rq.0 * Removed unnecessary comment * Avoid sending RQ.1 pixel when it should not at app (re)start
1 parent 2e4f69f commit 3ad788b

File tree

9 files changed

+311
-57
lines changed

9 files changed

+311
-57
lines changed

‎app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,46 @@ class BrowserTabViewModelTest {
12181218
assertCommandIssued<Command.Refresh>()
12191219
}
12201220

1221+
@Test
1222+
fun whenRefreshRequestedWithQuerySearchThenFireQueryChangePixelZero() {
1223+
loadUrl("query")
1224+
1225+
testee.onRefreshRequested()
1226+
1227+
verify(mockPixel).fire("rq_0")
1228+
}
1229+
1230+
@Test
1231+
fun whenRefreshRequestedWithUrlThenDoNotFireQueryChangePixel() {
1232+
loadUrl("https://example.com")
1233+
1234+
testee.onRefreshRequested()
1235+
1236+
verify(mockPixel, never()).fire("rq_0")
1237+
}
1238+
1239+
@Test
1240+
fun whenUserSubmittedQueryWithPreviousBlankQueryThenDoNotSendQueryChangePixel() {
1241+
whenever(mockOmnibarConverter.convertQueryToUrl("another query", null)).thenReturn("another query")
1242+
loadUrl("")
1243+
1244+
testee.onUserSubmittedQuery("another query")
1245+
1246+
verify(mockPixel, never()).fire("rq_0")
1247+
verify(mockPixel, never()).fire("rq_1")
1248+
}
1249+
1250+
@Test
1251+
fun whenUserSubmittedQueryWithDifferentPreviousQueryThenSendQueryChangePixel() {
1252+
whenever(mockOmnibarConverter.convertQueryToUrl("another query", null)).thenReturn("another query")
1253+
loadUrl("query")
1254+
1255+
testee.onUserSubmittedQuery("another query")
1256+
1257+
verify(mockPixel, never()).fire("rq_0")
1258+
verify(mockPixel).fire("rq_1")
1259+
}
1260+
12211261
@Test
12221262
fun whenUserBrowsingPressesBackAndBrowserCanGoBackThenNavigatesToPreviousPageAndHandledTrue() {
12231263
setupNavigation(isBrowsing = true, canGoBack = true, stepsToPreviousPage = 2)

‎app/src/androidTest/java/com/duckduckgo/app/global/api/ApiRequestInterceptorTest.kt‎

Lines changed: 21 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -16,75 +16,51 @@
1616

1717
package com.duckduckgo.app.global.api
1818

19+
import android.webkit.WebSettings
1920
import androidx.test.platform.app.InstrumentationRegistry
20-
import okhttp3.*
21+
import com.duckduckgo.app.browser.useragent.UserAgentProvider
22+
import com.duckduckgo.app.global.device.ContextDeviceInfo
2123
import org.junit.Assert.assertTrue
2224
import org.junit.Before
2325
import org.junit.Test
24-
import java.util.concurrent.TimeUnit
2526

2627
class ApiRequestInterceptorTest {
2728

2829
private lateinit var testee: ApiRequestInterceptor
2930

30-
private val fakeChain: Interceptor.Chain = FakeChain()
31+
private lateinit var userAgentProvider: UserAgentProvider
3132

3233
@Before
3334
fun before() {
34-
testee = ApiRequestInterceptor(InstrumentationRegistry.getInstrumentation().context)
35+
userAgentProvider = UserAgentProvider(
36+
WebSettings.getDefaultUserAgent(InstrumentationRegistry.getInstrumentation().context),
37+
ContextDeviceInfo(InstrumentationRegistry.getInstrumentation().context)
38+
)
39+
40+
testee = ApiRequestInterceptor(
41+
InstrumentationRegistry.getInstrumentation().context,
42+
userAgentProvider
43+
)
3544
}
3645

3746
@Test
3847
fun whenAPIRequestIsMadeThenUserAgentIsAdded() {
3948
val packageName = InstrumentationRegistry.getInstrumentation().context.applicationInfo.packageName
4049

41-
val response = testee.intercept(fakeChain)
50+
val response = testee.intercept(FakeChain("http://example.com"))
4251

4352
val regex = "ddg_android/.*\\($packageName; Android API .*\\)".toRegex()
4453
val result = response.request.header(Header.USER_AGENT)!!
4554
assertTrue(result.matches(regex))
4655
}
4756

48-
// implement just request and proceed methods
49-
private class FakeChain : Interceptor.Chain {
50-
override fun call(): Call {
51-
TODO("Not yet implemented")
52-
}
53-
54-
override fun connectTimeoutMillis(): Int {
55-
TODO("Not yet implemented")
56-
}
57-
58-
override fun connection(): Connection? {
59-
TODO("Not yet implemented")
60-
}
61-
62-
override fun proceed(request: Request): Response {
63-
return Response.Builder().request(request).protocol(Protocol.HTTP_2).code(200).message("").build()
64-
}
65-
66-
override fun readTimeoutMillis(): Int {
67-
TODO("Not yet implemented")
68-
}
69-
70-
override fun request(): Request {
71-
return Request.Builder().url("http://example.com").build()
72-
}
73-
74-
override fun withConnectTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain {
75-
TODO("Not yet implemented")
76-
}
77-
78-
override fun withReadTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain {
79-
TODO("Not yet implemented")
80-
}
81-
82-
override fun withWriteTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain {
83-
TODO("Not yet implemented")
84-
}
57+
@Test
58+
fun whenAPIRequestIsRqPixelThenOverrideHeader() {
59+
val fakeChain = FakeChain("https://improving.duckduckgo.com/t/rq_0")
8560

86-
override fun writeTimeoutMillis(): Int {
87-
TODO("Not yet implemented")
88-
}
61+
val response = testee.intercept(fakeChain)
62+
val header = response.request.header(Header.USER_AGENT)!!
63+
val regex = "Mozilla/.* \\(Linux; Android.*\\) AppleWebKit/.* \\(KHTML, like Gecko\\) Version/.* Chrome/.* Mobile DuckDuckGo/.* Safari/.*".toRegex()
64+
assertTrue(header.matches(regex))
8965
}
9066
}

‎app/src/androidTest/java/com/duckduckgo/app/global/api/FakeChain.kt‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright (c) 2021 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.app.global.api
18+
19+
import okhttp3.*
20+
import java.util.concurrent.TimeUnit
21+
22+
class FakeChain(private val url: String) : Interceptor.Chain {
23+
override fun call(): Call {
24+
TODO("Not yet implemented")
25+
}
26+
27+
override fun connectTimeoutMillis(): Int {
28+
TODO("Not yet implemented")
29+
}
30+
31+
override fun connection(): Connection? {
32+
TODO("Not yet implemented")
33+
}
34+
35+
override fun proceed(request: Request): Response {
36+
return Response.Builder().request(request).protocol(Protocol.HTTP_2).code(200).message("").build()
37+
}
38+
39+
override fun readTimeoutMillis(): Int {
40+
TODO("Not yet implemented")
41+
}
42+
43+
override fun request(): Request {
44+
return Request.Builder().url(url).build()
45+
}
46+
47+
override fun withConnectTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain {
48+
TODO("Not yet implemented")
49+
}
50+
51+
override fun withReadTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain {
52+
TODO("Not yet implemented")
53+
}
54+
55+
override fun withWriteTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain {
56+
TODO("Not yet implemented")
57+
}
58+
59+
override fun writeTimeoutMillis(): Int {
60+
TODO("Not yet implemented")
61+
}
62+
}

‎app/src/androidTest/java/com/duckduckgo/app/global/api/PixelReQueryInterceptorTest.kt‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright (c) 2021 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.app.global.api
18+
19+
import okhttp3.HttpUrl.Companion.toHttpUrl
20+
import org.junit.Assert.assertEquals
21+
import org.junit.Before
22+
import org.junit.Test
23+
24+
class PixelReQueryInterceptorTest {
25+
26+
private lateinit var pixelReQueryInterceptor: PixelReQueryInterceptor
27+
28+
@Before
29+
fun setup() {
30+
pixelReQueryInterceptor = PixelReQueryInterceptor()
31+
}
32+
33+
@Test
34+
fun whenRq0PixelIsSendThenRemoveDeviceAndFormFactor() {
35+
assertEquals(
36+
EXPECTED_RQ_0_URL.toHttpUrl(),
37+
pixelReQueryInterceptor.intercept(FakeChain(RQ_0_PHONE_URL)).request.url
38+
)
39+
40+
assertEquals(
41+
EXPECTED_RQ_0_URL.toHttpUrl(),
42+
pixelReQueryInterceptor.intercept(FakeChain(RQ_0_TABLET_URL)).request.url
43+
)
44+
}
45+
46+
@Test
47+
fun whenRq1PixelIsSendThenRemoveDeviceAndFormFactor() {
48+
assertEquals(
49+
EXPECTED_RQ_1_URL.toHttpUrl(),
50+
pixelReQueryInterceptor.intercept(FakeChain(RQ_1_PHONE_URL)).request.url
51+
)
52+
53+
assertEquals(
54+
EXPECTED_RQ_1_URL.toHttpUrl(),
55+
pixelReQueryInterceptor.intercept(FakeChain(RQ_1_TABLET_URL)).request.url
56+
)
57+
}
58+
59+
@Test
60+
fun whenPixelOtherThanRqIsSendThenDoNotModify() {
61+
assertEquals(
62+
EXPECTED_OTHER_PIXEL_PHONE_URL.toHttpUrl(),
63+
pixelReQueryInterceptor.intercept(FakeChain(OTHER_PIXEL_PHONE_URL)).request.url
64+
)
65+
66+
assertEquals(
67+
EXPECTED_OTHER_PIXEL_TABLET_URL.toHttpUrl(),
68+
pixelReQueryInterceptor.intercept(FakeChain(OTHER_PIXEL_TABLET_URL)).request.url
69+
)
70+
71+
}
72+
73+
private companion object {
74+
private const val RQ_0_PHONE_URL = "https://improving.duckduckgo.com/t/rq_0_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1"
75+
private const val RQ_0_TABLET_URL = "https://improving.duckduckgo.com/t/rq_0_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1"
76+
private const val RQ_1_PHONE_URL = "https://improving.duckduckgo.com/t/rq_1_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1"
77+
private const val RQ_1_TABLET_URL = "https://improving.duckduckgo.com/t/rq_1_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1"
78+
private const val OTHER_PIXEL_PHONE_URL = "https://improving.duckduckgo.com/t/my_pixel_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1"
79+
private const val OTHER_PIXEL_TABLET_URL = "https://improving.duckduckgo.com/t/my_pixel_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1"
80+
81+
private const val EXPECTED_RQ_0_URL = "https://improving.duckduckgo.com/t/rq_0?atb=v255-7zu&appVersion=5.74.0&test=1"
82+
private const val EXPECTED_RQ_1_URL = "https://improving.duckduckgo.com/t/rq_1?atb=v255-7zu&appVersion=5.74.0&test=1"
83+
private const val EXPECTED_OTHER_PIXEL_PHONE_URL = "https://improving.duckduckgo.com/t/my_pixel_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1"
84+
private const val EXPECTED_OTHER_PIXEL_TABLET_URL = "https://improving.duckduckgo.com/t/my_pixel_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1"
85+
}
86+
}

‎app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import android.annotation.SuppressLint
2020
import android.graphics.Bitmap
2121
import android.net.Uri
2222
import android.os.Message
23+
import android.util.Patterns
2324
import android.view.ContextMenu
2425
import android.view.MenuItem
2526
import android.view.View
@@ -548,7 +549,7 @@ class BrowserTabViewModel(
548549

549550
if (oldQuery == newQuery) {
550551
pixel.fire(String.format(Locale.US, PixelName.SERP_REQUERY.pixelName, PixelParameter.SERP_QUERY_NOT_CHANGED))
551-
} else {
552+
} else if (oldQuery.toString().isNotBlank()) { // blank means no previous search, don't send pixel
552553
pixel.fire(String.format(Locale.US, PixelName.SERP_REQUERY.pixelName, PixelParameter.SERP_QUERY_CHANGED))
553554
}
554555
}
@@ -626,6 +627,10 @@ class BrowserTabViewModel(
626627
}
627628

628629
fun onRefreshRequested() {
630+
val omnibarContent = currentOmnibarViewState().omnibarText
631+
if (!Patterns.WEB_URL.matcher(omnibarContent).matches()) {
632+
fireQueryChangedPixel(currentOmnibarViewState().omnibarText)
633+
}
629634
navigationAwareLoginDetector.onEvent(NavigationEvent.UserAction.Refresh)
630635
if (currentGlobalLayoutState() is Invalidated) {
631636
recoverTabWithQuery(url.orEmpty())

‎app/src/main/java/com/duckduckgo/app/browser/omnibar/KeyboardAwareEditText.kt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ package com.duckduckgo.app.browser.omnibar
1818

1919
import android.content.Context
2020
import android.graphics.Rect
21+
import android.text.Editable
2122
import android.util.AttributeSet
23+
import android.util.Patterns
2224
import android.view.KeyEvent
2325
import androidx.appcompat.widget.AppCompatEditText
2426
import com.duckduckgo.app.global.view.showKeyboard
@@ -33,10 +35,24 @@ class KeyboardAwareEditText : AppCompatEditText {
3335
constructor(context: Context, attrs: AttributeSet?) : this(context, attrs, 0)
3436
constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr)
3537

38+
private var didFocusAlready = false
39+
40+
private fun Editable.isWebUrl(): Boolean {
41+
return Patterns.WEB_URL.matcher(this.toString()).matches()
42+
}
43+
3644
override fun onFocusChanged(focused: Boolean, direction: Int, previouslyFocusedRect: Rect?) {
3745
super.onFocusChanged(focused, direction, previouslyFocusedRect)
46+
setSelectAllOnFocus(!didFocusAlready)
3847
if (focused) {
48+
if (didFocusAlready && text != null && text?.isWebUrl() == false) {
49+
// trigger the text change listener so that we can show autocomplete
50+
text = text
51+
// cursor at the end of the word
52+
setSelection(text!!.length)
53+
}
3954
showKeyboard()
55+
didFocusAlready = true
4056
}
4157
}
4258

0 commit comments

Comments
 (0)