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

Conversation

Arkariang
Copy link
Contributor

📲 What

  • Added feature Flag on Optimizely
  • Added new attributes to optimizaly calls
    "session_app_release_version_number" = 2131 version (2.13.1) in numbers,
    "session_app_release_version_code" = BuildConfig.VERSION_CODE

🤔 Why

  • Feature Flag for the new Treaded Coment Screen
  • Adding the new attributes will provide more flexibility around versioning for feature flags

👀 See

Screen Shot 2021-05-12 at 2 51 36 PM

  • Feature Flag On
    FeatureFlagOn
  • Feature Flag Off
    FeatureFlagOff

| | |

📋 QA

  • Enable the feature flag, when you press the comments row in the project page no new screen will be presented, that screen is work in progress
  • Disable the feature flag, the old comments screen will be presented

Story 📖

NT-1916

@codecov
Copy link

codecov bot commented May 12, 2021

Codecov Report

Merging #1243 (3889ac9) into master (876e613) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1243   +/-   ##
=========================================
  Coverage     74.79%   74.79%           
  Complexity      734      734           
=========================================
  Files           221      221           
  Lines          6682     6682           
  Branches        403      403           
=========================================
  Hits           4998     4998           
  Misses         1551     1551           
  Partials        133      133           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 876e613...3889ac9. Read the comment docs.

@Arkariang Arkariang marked this pull request as ready for review May 12, 2021 23:01
@@ -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

Copy link
Contributor

@sunday-okpoluaefe sunday-okpoluaefe left a comment

Choose a reason for hiding this comment

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

lgtm!

@@ -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!

@Arkariang Arkariang merged commit 60e6098 into master May 14, 2021
@Arkariang Arkariang deleted the imartin/NT-1916-feature-flag-comment-threading branch May 14, 2021 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants