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

EP-513-4: Remove Datalake Events And Worker #1236

Merged
merged 55 commits into from
May 7, 2021

Conversation

sunday-okpoluaefe
Copy link
Contributor

@sunday-okpoluaefe sunday-okpoluaefe commented May 5, 2021

📲 What

Android: Remove Datalake Events And Worker

🤔 Why

  • With the completion of the Segment events, we need to remove all of the code used for sending Datalake/Koala events as they are no longer needed.

🛠 How

  • Removed LakeTrackingClient from the application

  • Remove LAKE from the TrackingClientType

  • Removed KoalaEvent enum and the affecting functions from the app

  • Removed All unused functions from the AnalyticsEvents class

  • Removed Any additional references to the DataLake are removed from the app

  • Removed LakeWorker needs to be removed

  • Deleted the provider for the Lake worker, and the provider for the retrofit service from the AppModule

  • Updated Tests to reflect the removed changes

👀 See

Trello, screenshots, external resources?

Before 🐛 After 🦋

📋 QA

Instructions for anyone to be able to QA this work.

Story 📖

https://kickstarter.atlassian.net/browse/EP-513
https://kickstarter.atlassian.net/browse/EP-514

sunday-okpoluaefe and others added 30 commits May 4, 2021 14:28
…starter/android-oss into sunday-ep-523-remove-datalake-evens
This reverts commit 77e2fa2.

# Conflicts:
#	app/src/main/java/com/kickstarter/libs/KoalaContext.kt
…starter/android-oss into sunday-ep-523-remove-datalake-evens
…starter/android-oss into sunday-ep-523-remove-datalake-evens
…ens' into sunday-ep-523-remove-datalake-evens

# Conflicts:
#	app/src/main/java/com/kickstarter/libs/AnalyticEvents.kt
#	app/src/main/java/com/kickstarter/libs/KoalaContext.kt
#	app/src/main/java/com/kickstarter/ui/activities/ProfileActivity.kt
#	app/src/main/java/com/kickstarter/viewmodels/MessageThreadsViewModel.java
#	app/src/test/java/com/kickstarter/viewmodels/MessageThreadsViewModelTest.java
…datalake-evens

# Conflicts:
#	app/src/main/java/com/kickstarter/ApplicationModule.java
@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #1236 (72ca831) into master (0f5733c) will increase coverage by 0.11%.
The diff coverage is 92.59%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1236      +/-   ##
============================================
+ Coverage     74.71%   74.83%   +0.11%     
- Complexity      734      738       +4     
============================================
  Files           221      221              
  Lines          6684     6640      -44     
  Branches        402      402              
============================================
- Hits           4994     4969      -25     
+ Misses         1557     1538      -19     
  Partials        133      133              
Impacted Files Coverage Δ Complexity Δ
...main/java/com/kickstarter/ui/data/LoginReason.java 83.33% <ø> (+24.50%) 1.00 <0.00> (ø)
...ickstarter/viewmodels/MessageThreadsViewModel.java 95.53% <ø> (-0.53%) 0.00 <0.00> (ø)
.../com/kickstarter/viewmodels/CommentsViewModel.java 96.51% <50.00%> (ø) 0.00 <0.00> (ø)
.../com/kickstarter/viewmodels/MessagesViewModel.java 96.86% <83.33%> (ø) 0.00 <0.00> (ø)
...n/java/com/kickstarter/libs/PushNotifications.java 5.43% <100.00%> (+5.43%) 3.00 <1.00> (+3.00)
...com/kickstarter/viewmodels/DiscoveryViewModel.java 97.99% <100.00%> (-0.08%) 0.00 <0.00> (ø)
...com/kickstarter/viewmodels/LoginToutViewModel.java 77.87% <100.00%> (-1.30%) 0.00 <0.00> (ø)
...ickstarter/viewmodels/ProjectUpdatesViewModel.java 94.11% <100.00%> (ø) 0.00 <0.00> (ø)
...va/com/kickstarter/viewmodels/SearchViewModel.java 96.39% <100.00%> (-0.07%) 0.00 <0.00> (ø)
...va/com/kickstarter/viewmodels/SignupViewModel.java 98.68% <100.00%> (-0.02%) 0.00 <0.00> (ø)
... and 4 more

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 0f5733c...72ca831. Read the comment docs.

@hadia hadia marked this pull request as ready for review May 6, 2021 13:47
@@ -25,11 +25,11 @@
private final PublishSubject<ActivityResult> activityResult = PublishSubject.create();
private final PublishSubject<Bundle> arguments = PublishSubject.create();
protected final PublishSubject<Boolean> isExpanded = PublishSubject.create();
protected final AnalyticEvents lake;
protected final AnalyticEvents analyticEvents;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the re-naming 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

😊😊

@@ -335,15 +339,16 @@ private void displayNotificationFromUpdateActivity(final @NonNull PushNotificati
return taskStackBuilder.getPendingIntent(envelope.signature(), PendingIntent.FLAG_UPDATE_CURRENT);
}

private @NonNull PendingIntent messageThreadIntent(final @NonNull PushNotificationEnvelope envelope,
@VisibleForTesting
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@Arkariang Arkariang left a comment

Choose a reason for hiding this comment

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

Just two small comments left. Apart from that amazing job on this guys @sunday-okpoluaefe @hadia 👏 🎆 🚀

lakeTrackingClient.eventNames.subscribe(lakeTest)
return lakeTrackingClient
}
// private fun lakeTrackingClient(mockCurrentConfig: MockCurrentConfig, experimentsClientType: MockExperimentsClientType): MockTrackingClient {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be removed

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

import org.junit.Test

class PushNotificationsTest : KSRobolectricTestCase() {
private val application: Application = ApplicationProvider.getApplicationContext()
Copy link
Contributor

Choose a reason for hiding this comment

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

KSRobolectricTestCase has a protected method application() to fetch the context Application, take a look at SegmentTest.kt:73

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

@hadia hadia merged commit 03acaf0 into master May 7, 2021
@hadia hadia deleted the sunday-ep-523-remove-datalake-evens branch May 7, 2021 22:27
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