Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NT-1916: Feature flag comment threading #1243

Merged
merged 9 commits into from
May 14, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import org.json.JSONObject
interface ExperimentsClientType {

fun ExperimentsClientType.attributes(experimentData: ExperimentData, optimizelyEnvironment: OptimizelyEnvironment): Map<String, *> {
return ExperimentUtils.attributes(experimentData, appVersion(), OSVersion(), optimizelyEnvironment)
return ExperimentUtils.attributes(experimentData, appVersion(), OSVersion(), versionCode(), optimizelyEnvironment)
}

/**
Expand Down Expand Up @@ -42,9 +42,11 @@ interface ExperimentsClientType {
return properties
}

fun versionCode(): Int
fun appVersion(): String
fun enabledFeatures(user: User?): List<String>
fun isFeatureEnabled(feature: OptimizelyFeature.Key, experimentData: ExperimentData): Boolean
fun isFeatureEnabled(feature: OptimizelyFeature.Key): Boolean
fun optimizelyEnvironment(): OptimizelyEnvironment
fun OSVersion(): String
fun track(eventKey: String, experimentData: ExperimentData)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import com.optimizely.ab.android.sdk.OptimizelyManager
class OptimizelyExperimentsClient(private val optimizelyManager: OptimizelyManager, private val optimizelyEnvironment: OptimizelyEnvironment) : ExperimentsClientType {
override fun appVersion(): String = BuildConfig.VERSION_NAME

override fun versionCode() = BuildConfig.VERSION_CODE

override fun OSVersion(): String = Build.VERSION.RELEASE

override fun track(eventKey: String, experimentData: ExperimentData) {
Expand All @@ -34,6 +36,10 @@ class OptimizelyExperimentsClient(private val optimizelyManager: OptimizelyManag
return optimizelyClient().isFeatureEnabled(feature.key, userId(), attributes(experimentData, this.optimizelyEnvironment))
}

override fun isFeatureEnabled(feature: OptimizelyFeature.Key): Boolean {
return optimizelyClient().isFeatureEnabled(feature.key, userId(), attributes(ExperimentData(null, null, null), this.optimizelyEnvironment))
}

override fun variant(experiment: OptimizelyExperiment.Key, experimentData: ExperimentData): OptimizelyExperiment.Variant {
val user = experimentData.user
val variationString: String? = if (user?.isAdmin == true) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.kickstarter.libs.models

class OptimizelyFeature {
enum class Key(val key: String) {
LIGHTS_ON("android_lights_on")
LIGHTS_ON("android_lights_on"),
COMMENT_THREADING("android_comment_threading")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,16 @@ import java.util.Locale

object ExperimentUtils {

fun attributes(experimentData: ExperimentData, appVersion: String, OSVersion: String, optimizelyEnvironment: OptimizelyEnvironment): Map<String, Any?> {
/**
* Attributes we need to send alongside with the Experiment Data or Feature Flag,
* Optimizely will use this attributes for setting up audiences.
*/
fun attributes(experimentData: ExperimentData, appVersion: String, OSVersion: String, versionCode: Int, optimizelyEnvironment: OptimizelyEnvironment): Map<String, Any?> {
return mapOf(
Pair("distinct_id", getInstanceId(optimizelyEnvironment)),
Pair("session_app_release_version", appVersion),
Pair("session_app_release_version_number", appVersion.replace(".", "").toInt()),
Pair("session_app_release_version_code", versionCode),
Pair("session_os_version", String.format("Android %s", OSVersion)),
Pair("session_ref_tag", experimentData.intentRefTag?.tag()),
Pair("session_referrer_credit", experimentData.cookieRefTag?.tag()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,26 @@ import org.json.JSONObject
import rx.Observable
import rx.subjects.PublishSubject

open class MockExperimentsClientType(private val variant: OptimizelyExperiment.Variant, private val optimizelyEnvironment: OptimizelyEnvironment) : ExperimentsClientType {
constructor(variant: OptimizelyExperiment.Variant) : this(variant, OptimizelyEnvironment.DEVELOPMENT)
constructor() : this(OptimizelyExperiment.Variant.CONTROL, OptimizelyEnvironment.DEVELOPMENT)
open class MockExperimentsClientType(private val variant: OptimizelyExperiment.Variant, private val optimizelyEnvironment: OptimizelyEnvironment, private val enabledFlag: Boolean) : ExperimentsClientType {
constructor(variant: OptimizelyExperiment.Variant) : this(variant, OptimizelyEnvironment.DEVELOPMENT, false)
constructor() : this(OptimizelyExperiment.Variant.CONTROL, OptimizelyEnvironment.DEVELOPMENT, false)
constructor(enabled: Boolean) : this(OptimizelyExperiment.Variant.CONTROL, OptimizelyEnvironment.DEVELOPMENT, enabled)

class ExperimentsEvent internal constructor(internal val eventKey: String, internal val attributes: Map<String, *>, internal val tags: Map<String, *>?)

private val experimentEvents: PublishSubject<ExperimentsEvent> = PublishSubject.create()
val eventKeys: Observable<String> = this.experimentEvents.map { e -> e.eventKey }

override fun versionCode(): Int = 999999999

override fun appVersion(): String = "9.9.9"

override fun enabledFeatures(user: User?): List<String> = emptyList()

override fun isFeatureEnabled(feature: OptimizelyFeature.Key, experimentData: ExperimentData): Boolean = false

override fun isFeatureEnabled(feature: OptimizelyFeature.Key): Boolean = this.enabledFlag

override fun optimizelyEnvironment(): OptimizelyEnvironment = this.optimizelyEnvironment

override fun optimizelyProperties(experimentData: ExperimentData): Map<String, Any> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ class ProjectActivity :
.observeOn(AndroidSchedulers.mainThread())
.subscribe { this.startCommentsActivity(it) }

this.viewModel.outputs.startCommentsThreadedActivity()
.compose(bindToLifecycle())
.observeOn(AndroidSchedulers.mainThread())
.subscribe { this.startCommentsThreadedActivity(it) }

this.viewModel.outputs.startCreatorBioWebViewActivity()
.compose(bindToLifecycle())
.observeOn(AndroidSchedulers.mainThread())
Expand Down Expand Up @@ -665,6 +670,10 @@ class ProjectActivity :
startActivityWithTransition(intent, R.anim.slide_in_right, R.anim.fade_out_slide_out_left)
}

private fun startCommentsThreadedActivity(projectAndData: Pair<Project, ProjectData>) {
// TODO: Start the new activity defined in https://kickstarter.atlassian.net/browse/NT-1920
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leighdouglas you can start the new activity in this method, for the task https://kickstarter.atlassian.net/browse/NT-1920

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! will do as soon as this lands and will update my PR

}

private fun startShareIntent(projectNameAndShareUrl: Pair<String, String>) {
val name = projectNameAndShareUrl.first
val shareMessage = this.ksString.format(getString(this.projectShareCopyString), "project_title", name)
Expand Down
16 changes: 16 additions & 0 deletions app/src/main/java/com/kickstarter/viewmodels/ProjectViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.kickstarter.libs.ExperimentsClientType
import com.kickstarter.libs.KSCurrency
import com.kickstarter.libs.RefTag
import com.kickstarter.libs.models.OptimizelyExperiment
import com.kickstarter.libs.models.OptimizelyFeature
import com.kickstarter.libs.rx.transformers.Transformers.combineLatestPair
import com.kickstarter.libs.rx.transformers.Transformers.errors
import com.kickstarter.libs.rx.transformers.Transformers.ignoreValues
Expand Down Expand Up @@ -219,6 +220,9 @@ interface ProjectViewModel {
/** Emits when we should start [com.kickstarter.ui.activities.CommentsActivity]. */
fun startCommentsActivity(): Observable<Pair<Project, ProjectData>>

/** Emits when we should start [com.kickstarter.ui.activities. TODO: DEFINE NAME https://kickstarter.atlassian.net/browse/NT-1920]. */
fun startCommentsThreadedActivity(): Observable<Pair<Project, ProjectData>>

/** Emits when we should start the creator bio [com.kickstarter.ui.activities.CreatorBioActivity]. */
fun startCreatorBioWebViewActivity(): Observable<Project>

Expand Down Expand Up @@ -308,6 +312,7 @@ interface ProjectViewModel {
private val showUpdatePledgeSuccess = PublishSubject.create<Void>()
private val startCampaignWebViewActivity = PublishSubject.create<ProjectData>()
private val startCommentsActivity = PublishSubject.create<Pair<Project, ProjectData>>()
private val startCommentsThreadedActivity = PublishSubject.create<Pair<Project, ProjectData>>()
private val startCreatorBioWebViewActivity = PublishSubject.create<Project>()
private val startCreatorDashboardActivity = PublishSubject.create<Project>()
private val startLoginToutActivity = PublishSubject.create<Void>()
Expand Down Expand Up @@ -482,11 +487,19 @@ interface ProjectViewModel {
.subscribe(this.startCreatorBioWebViewActivity)

currentProject
.filter { !optimizely.isFeatureEnabled(OptimizelyFeature.Key.COMMENT_THREADING) }
.compose<Project>(takeWhen(this.commentsTextViewClicked))
.compose<Pair<Project, ProjectData>>(combineLatestPair(projectData))
.compose(bindToLifecycle())
.subscribe(this.startCommentsActivity)

currentProject
.filter { optimizely.isFeatureEnabled(OptimizelyFeature.Key.COMMENT_THREADING) }
.compose<Project>(takeWhen(this.commentsTextViewClicked))
.compose<Pair<Project, ProjectData>>(combineLatestPair(projectData))
.compose(bindToLifecycle())
.subscribe(this.startCommentsThreadedActivity)

currentProject
.compose<Project>(takeWhen(this.creatorDashboardButtonClicked))
.compose(bindToLifecycle())
Expand Down Expand Up @@ -1069,6 +1082,9 @@ interface ProjectViewModel {
@NonNull
override fun startCommentsActivity(): Observable<Pair<Project, ProjectData>> = this.startCommentsActivity

@NonNull
override fun startCommentsThreadedActivity(): Observable<Pair<Project, ProjectData>> = this.startCommentsThreadedActivity
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think to keep parity with iOS, we are eventually going to name the comments activity RootCommentsActivity, so you may want to change the name of this method from threaded to root

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


@NonNull
override fun startCreatorBioWebViewActivity(): Observable<Project> = this.startCreatorBioWebViewActivity

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ class ExperimentUtilsTest : KSRobolectricTestCase() {
.backedProjectsCount(10)
.build()
val experimentData = ExperimentData(user, RefTag.discovery(), RefTag.search())
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "10", OptimizelyEnvironment.DEVELOPMENT)
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "10", 99999, OptimizelyEnvironment.DEVELOPMENT)
assertNotNull(attributes["distinct_id"])
assertEquals("9.9.9", attributes["session_app_release_version"])
assertEquals(999, attributes["session_app_release_version_number"])
assertEquals(99999, attributes["session_app_release_version_code"])
assertEquals("Android 10", attributes["session_os_version"])
assertEquals("discovery", attributes["session_ref_tag"])
assertEquals("search", attributes["session_referrer_credit"])
Expand All @@ -33,9 +35,11 @@ class ExperimentUtilsTest : KSRobolectricTestCase() {
.backedProjectsCount(10)
.build()
val experimentData = ExperimentData(user, RefTag.discovery(), RefTag.search())
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "10", OptimizelyEnvironment.PRODUCTION)
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "10", 99999, OptimizelyEnvironment.PRODUCTION)
assertNull(attributes["distinct_id"])
assertEquals("9.9.9", attributes["session_app_release_version"])
assertEquals(999, attributes["session_app_release_version_number"])
assertEquals(99999, attributes["session_app_release_version_code"])
assertEquals("Android 10", attributes["session_os_version"])
assertEquals("discovery", attributes["session_ref_tag"])
assertEquals("search", attributes["session_referrer_credit"])
Expand All @@ -47,9 +51,11 @@ class ExperimentUtilsTest : KSRobolectricTestCase() {
@Test
fun attributes_loggedOutUser_notProd() {
val experimentData = ExperimentData(null, RefTag.discovery(), RefTag.search())
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "9", OptimizelyEnvironment.DEVELOPMENT)
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "9", 99999, OptimizelyEnvironment.DEVELOPMENT)
assertNotNull(attributes["distinct_id"])
assertEquals("9.9.9", attributes["session_app_release_version"])
assertEquals(999, attributes["session_app_release_version_number"])
assertEquals(99999, attributes["session_app_release_version_code"])
assertEquals("Android 9", attributes["session_os_version"])
assertEquals("discovery", attributes["session_ref_tag"])
assertEquals("search", attributes["session_referrer_credit"])
Expand All @@ -61,9 +67,11 @@ class ExperimentUtilsTest : KSRobolectricTestCase() {
@Test
fun attributes_loggedOutUser_prod() {
val experimentData = ExperimentData(null, RefTag.discovery(), RefTag.search())
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "9", OptimizelyEnvironment.PRODUCTION)
val attributes = ExperimentUtils.attributes(experimentData, "9.9.9", "9", 99999, OptimizelyEnvironment.PRODUCTION)
assertNull(attributes["distinct_id"])
assertEquals("9.9.9", attributes["session_app_release_version"])
assertEquals(999, attributes["session_app_release_version_number"])
assertEquals(99999, attributes["session_app_release_version_code"])
assertEquals("Android 9", attributes["session_os_version"])
assertEquals("discovery", attributes["session_ref_tag"])
assertEquals("search", attributes["session_referrer_credit"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class ProjectViewModelTest : KSRobolectricTestCase() {
private val showUpdatePledgeSuccess = TestSubscriber<Void>()
private val startCampaignWebViewActivity = TestSubscriber<ProjectData>()
private val startCommentsActivity = TestSubscriber<Pair<Project, ProjectData>>()
private val startCommentsThreadedActivity = TestSubscriber<Pair<Project, ProjectData>>()
private val startCreatorBioWebViewActivity = TestSubscriber<Project>()
private val startCreatorDashboardActivity = TestSubscriber<Project>()
private val startLoginToutActivity = TestSubscriber<Void>()
Expand Down Expand Up @@ -107,6 +108,7 @@ class ProjectViewModelTest : KSRobolectricTestCase() {
this.vm.outputs.projectData().map { pD -> pD.project().isStarred }.subscribe(this.savedTest)
this.vm.outputs.startCampaignWebViewActivity().subscribe(this.startCampaignWebViewActivity)
this.vm.outputs.startCommentsActivity().subscribe(this.startCommentsActivity)
this.vm.outputs.startCommentsThreadedActivity().subscribe(this.startCommentsThreadedActivity)
this.vm.outputs.startCreatorBioWebViewActivity().subscribe(this.startCreatorBioWebViewActivity)
this.vm.outputs.startCreatorDashboardActivity().subscribe(this.startCreatorDashboardActivity)
this.vm.outputs.startMessagesActivity().subscribe(this.startMessagesActivity)
Expand Down Expand Up @@ -576,17 +578,82 @@ class ProjectViewModelTest : KSRobolectricTestCase() {
}

@Test
fun testStartCommentsActivity() {
fun testStartCommentsActivity_FeatureFlagOff() {
val project = ProjectFactory.project()
val projectData = ProjectDataFactory.project(project)
val projectAndData = Pair.create(project, projectData)

setUpEnvironment(environment())
setUpEnvironment(
environment().toBuilder()
.optimizely(MockExperimentsClientType(false))
.build()
)

// Start the view model with a project.
this.vm.intent(Intent().putExtra(IntentKey.PROJECT, project))

this.vm.inputs.commentsTextViewClicked()
this.startCommentsActivity.assertValues(projectAndData)
this.startCommentsThreadedActivity.assertNoValues()
}

@Test
fun testStartCommentsActivity_FeatureFlagOn() {
val project = ProjectFactory.project()
val projectData = ProjectDataFactory.project(project)
val projectAndData = Pair.create(project, projectData)

setUpEnvironment(
environment().toBuilder()
.optimizely(MockExperimentsClientType(true))
.build()
)

// Start the view model with a project.
this.vm.intent(Intent().putExtra(IntentKey.PROJECT, project))

this.vm.inputs.commentsTextViewClicked()
this.startCommentsActivity.assertNoValues()
this.startCommentsThreadedActivity.assertValues(projectAndData)
}

@Test
fun testStartCommentsThreadedActivity_FeatureFlagOn() {
val project = ProjectFactory.project()
val projectData = ProjectDataFactory.project(project)
val projectAndData = Pair.create(project, projectData)

setUpEnvironment(
environment().toBuilder()
.optimizely(MockExperimentsClientType(true))
.build()
)

// Start the view model with a project.
this.vm.intent(Intent().putExtra(IntentKey.PROJECT, project))

this.vm.inputs.commentsTextViewClicked()
this.startCommentsThreadedActivity.assertValues(projectAndData)
this.startCommentsActivity.assertNoValues()
}

@Test
fun testStartCommentsThreadedActivity_FeatureFlagOff() {
val project = ProjectFactory.project()
val projectData = ProjectDataFactory.project(project)
val projectAndData = Pair.create(project, projectData)

setUpEnvironment(
environment().toBuilder()
.optimizely(MockExperimentsClientType(false))
.build()
)

// Start the view model with a project.
this.vm.intent(Intent().putExtra(IntentKey.PROJECT, project))

this.vm.inputs.commentsTextViewClicked()
this.startCommentsThreadedActivity.assertNoValues()
this.startCommentsActivity.assertValues(projectAndData)
}

Expand Down