removing old sticky search code #789
Conversation
malmstein
requested review from
CDRussell,
cmonfortep,
marcosholgado and
subsymbolic
as code owners
|
I seem to remember getting translations for some notifications experiment... could it be this one? I've not checked but maybe there are some unused strings in untranslated that need removing :) |
|
Those were not translated but thanks for the reminder! |
subsymbolic
left a comment
subsymbolic
left a comment
There was a problem hiding this comment.
Thanks for submitting this, I always enjoy a good cleanup PR. Looks good a few final old remnants of Sticky Search to clear.
|
|
||
| @Test | ||
| fun whenPrivacyNotificationClearDataAndSearchPromptCanShowThenBothAreScheduled() = runBlocking<Unit> { | ||
| fun whenPrivacyNotificationClearDataCanShowThenBothAreScheduled() = runBlocking<Unit> { |
There was a problem hiding this comment.
These test names were changed in the PR you're reverting here and since they are no longer accurate we should put the back to how they were:
- What is happening: both notification can be shown and then the scheduler scheduled one of them
- What the test name is saying: both notification can be show (correct) and both are being scheduled (incorrect only one is scheduled)
Happy to pair on this, just ping me on zoom if you would like to.
| Timber.i("User changed search notification setting, is now enabled: $enabled") | ||
| settingsDataStore.searchNotificationEnabled = enabled | ||
| if (enabled){ | ||
| notificationScheduler.launchStickySearchNotification() |
There was a problem hiding this comment.
You can remove the private val notificationScheduler: AndroidNotificationScheduler from this class now that it is no longer used. Also clean up imports afterwards as you'll have an unused import.
| } | ||
| } | ||
|
|
||
| private fun isSearchNotificationFeatureEnabled(variant: Variant): Boolean { |
There was a problem hiding this comment.
You can delete the import com.duckduckgo.app.statistics.Variant import now too
|
|
||
| CHANGE_APP_ICON_OPENED("m_ic"), | ||
|
|
||
| QUICK_SEARCH_PROMPT_NOTIFICATION_SHOWN("m_qs_pn_s"), |
There was a problem hiding this comment.
You might have done this already, but just in case, can you delete any Grafana references too?
|
|
||
| private fun showSearchNotification(enabled: Boolean) { | ||
| if (enabled) { | ||
| searchNotificationToggle.show() |
There was a problem hiding this comment.
Delete the import com.duckduckgo.app.global.view.show and import com.duckduckgo.app.global.view.gone imports too
|
|
||
| @Test | ||
| fun whenPrivacyNotificationAndClearNotificationCannotShowButSearchPromptCanShowThenNotificationScheduled() = runBlocking<Unit> { | ||
| fun whenPrivacyNotificationAndClearNotificationCannotShowThenNotificationScheduled() = runBlocking<Unit> { |
There was a problem hiding this comment.
Do you mean whenPrivacyNotificationAndClearNotificationCannotShowThenNoNotificationScheduled?
3 participants
Task/Issue URL: https://app.asana.com/0/488551667048375/1171986548569356
Description:
The Sticky Search Experiment has now finished, it's time cleanup and remove all the experiment code.
Steps to test this PR: