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.

Sign up for GitHub

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

Jump to bottom

feat: add analytics user profile - Part 1 (WPB-8978) #2868

Merged
merged 4 commits into from
Jul 12, 2024
Merged

feat: add analytics user profile - Part 1 (WPB-8978) #2868

merged 4 commits into from
Jul 12, 2024

Conversation

alexandreferris
Copy link
Contributor

@alexandreferris alexandreferris commented Jul 8, 2024 •
edited by github-actions bot
Loading

TaskWPB-8978 [Android] Countly analytics ID and user properties

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³

  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

There was no common identifier between same user logged in different devices.

Causes (Optional)

Not implemented.

Solutions

This is only Part 1, which solves:

  • Receiving the event for DataTransfer with tracking identifier
  • Saving this value into user metadata table (saving it from Kalium and on a table because it will later be used in the frontend client, and if in the future Kalium is also used by other platforms, only a small tweak on frontend clients will need to be made for receiving/setting the current identifier - from sharedPreferences (Android) to this metadata table)

Dependencies (Optional)

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

  • Run this branch with AR
  • Login from AR and watch metadata table for logged in user
  • Login from web (fresh login)
  • Metadata table should now have an entry for analytics_tracking_identifier

Notes (Optional)

This is only Part 1

@alexandreferris alexandreferris self-assigned this Jul 8, 2024
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jul 8, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 8, 2024 •
edited
Loading

Test Results

2 696 tests   - 406   2 688 ✔️  - 309   58m 32s ⏱️ + 55m 1s
       2 suites  - 531          8 💤  -   97 
       2 files    - 531          0 ❌ ±    0 

Results for commit c2a1105. ± Comparison against base commit a89ab59.

This pull request removes 3102 and adds 2696 tests. Note that renamed tests count towards both. 
PocIntegrationTest ‑ givenApiWhenGettingACMEDirectoriesThenReturnAsExpectedBasedOnNetworkState
PocIntegrationTest ‑ givenEmailAndPasswordWhenLoggingInThenRegisterClientAndLogout
com.wire.kalium.HttpClientConnectionSpecsTest ‑ givenOkHttpSingleton_whenBuildingClearTextTrafficOkhttpClient_thenEnsureConnectionSpecClearText[jvm]
com.wire.kalium.HttpClientConnectionSpecsTest ‑ givenTheHttpClientIsCreated_ThenEnsureOnlySupportedSpecsArePresent[jvm]
com.wire.kalium.api.base.authenticated.notification.AccessUpdateTest ‑ givenPayloadWithAccessRoleAndDeprecatedAccessRoleField_whenDecoding_thenDeprecatedFieldIsPreferred[jvm]
com.wire.kalium.api.base.authenticated.notification.AccessUpdateTest ‑ givenPayloadWithDeprecatedAccessRoleField_whenDecoding_thenSuccess[jvm]
com.wire.kalium.api.base.authenticated.notification.AccessUpdateTest ‑ givenPayload_whenDecoding_thenSuccess[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ givenNoLocationInHeader_whenCallingSendAcmeRequestApi_theResponseShouldBeConfigureCorrectly[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ givenNoNonce_whenCallingSendAcmeRequestApi_theResponseShouldBeMissingNonce[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ givingASuccessfulResponse_whenGettingACMEFederationCertificateChain_thenAllCertificatesShouldBeParsed[jvm]
…

com.wire.kalium.logic.cache.SelfConversationIdProviderTest ‑ givenFailure_thenErrorIsPropagated
com.wire.kalium.logic.cache.SelfConversationIdProviderTest ‑ givenMLSClientHasBeenRegistered_thenMLSAndProteusSelfConversationAreReturned
com.wire.kalium.logic.cache.SelfConversationIdProviderTest ‑ givenMLSClientHasNotBeenRegistered_thenProteusSelfConversationIsReturned
com.wire.kalium.logic.client.E2EIClientProviderTest ‑ givenIsNewClientTrue_whenGettingE2EIClient_newAcmeEnrollmentCalled
com.wire.kalium.logic.client.E2EIClientProviderTest ‑ givenMLSClientWithE2EI_whenGettingE2EIClient_callsNewActivationEnrollment
com.wire.kalium.logic.client.E2EIClientProviderTest ‑ givenMLSClientWithoutE2EI_whenGettingE2EIClient_callsNewRotateEnrollment
com.wire.kalium.logic.client.E2EIClientProviderTest ‑ givenSelfUserNotFound_whenGettingE2EIClient_ReturnsError
com.wire.kalium.logic.configuration.CustomServerConfigRepositoryTest ‑ givenStoredConfigLinksAndVersionInfoData_whenAddingNewOne_thenCommonApiShouldBeCalculatedAndConfigShouldBeStored
com.wire.kalium.logic.configuration.CustomServerConfigRepositoryTest ‑ givenStoredConfig_whenAddingNewOne_thenNewOneShouldBeInsertedAndReturned
com.wire.kalium.logic.configuration.CustomServerConfigRepositoryTest ‑ givenStoredConfig_whenAddingTheSameOneWithNewApiVersionParams_thenStoredOneShouldBeUpdatedAndReturned
…
This pull request removes 105 skipped tests and adds 8 skipped tests. Note that renamed tests count towards both. 
PocIntegrationTest ‑ givenApiWhenGettingACMEDirectoriesThenReturnAsExpectedBasedOnNetworkState
PocIntegrationTest ‑ givenEmailAndPasswordWhenLoggingInThenRegisterClientAndLogout
com.wire.kalium.api.common.ACMEApiTest ‑ whenCallingGeTrustAnchorsApi_theResponseShouldBeConfigureCorrectly[jvm]
com.wire.kalium.api.common.ACMEApiTest ‑ whenCallingSendChallengeRequestApi_theResponseShouldBeConfigureCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenAValidEmail_whenActivationEmailWIthCode_theRequestShouldBeConfiguredCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenAValidEmail_whenRegisteringAccountWithEMail_theRequestShouldBeConfiguredCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenAValidEmail_whenSendingActivationEmail_theRequestShouldBeConfiguredCorrectly[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenActivationCodeFail_thenErrorIsPropagated[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenRegistrationFail_whenRegisteringAccountWithEMMail_thenErrorIsPropagated[jvm]
com.wire.kalium.api.v0.user.register.RegisterApiV0Test ‑ givenSendActivationCodeFail_thenErrorIsPropagated[jvm]
…

com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenACorrectlyEncryptedBackup_whenRestoringWithWrongPassword_thenTheRightErrorIsThrown
com.wire.kalium.logic.feature.backup.RestoreBackupUseCaseTest ‑ givenAnEncryptedBackupFileFromDifferentUserID_whenRestoring_thenTheRightErrorIsThrown
com.wire.kalium.logic.feature.call.CallManagerTest ‑ givenCallManager_whenCallingMessageIsReceived_then_wcall_recv_msg_IsCalled
com.wire.kalium.logic.feature.conversation.ObserveConversationListDetailsUseCaseTest ‑ null
com.wire.kalium.logic.feature.session.DeleteSessionUseCaseTest ‑ givenSuccess_WhenDeletingSessionLocally_thenSuccessAndResourcesAreFreed
com.wire.kalium.logic.sync.incremental.EventProcessingHistoryTest ‑ measureContainsTimeOverLargeAmountOfEvents
com.wire.kalium.persistence.dao.message.MessageDAOBenchmarkTest ‑ null
com.wire.kalium.persistence.kmmSettings.EncryptedSettingsBuilderTest ‑ givenShouldEncryptDataIsTrue_whenEncryptingData_thenShouldEncryptWithoutFailing

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

Datadog Report

Branch report: feat/add_analytics_user_profile_part1
Commit report: f406220
Test service: kalium-jvm

✅ 0 Failed, 3001 Passed, 105 Skipped, 11.92s Total Time

@alexandreferris alexandreferris requested review from a team, yamilmedina, MohamadJaara, mchenani, ohassine and saleniuk and removed request for a team July 8, 2024 15:00
@alexandreferris alexandreferris merged commit 9d2b507 into feat/add_analytics_user_profile_epic Jul 12, 2024
22 checks passed
@alexandreferris alexandreferris deleted the feat/add_analytics_user_profile_part1 branch July 12, 2024 09:33
This was referenced Jul 15, 2024
Merged
Merged
alexandreferris added a commit that referenced this pull request Jul 22, 2024
@alexandreferris
feat: Add Anonymous Analytics User Profile EPIC - (WPB-8978) (#2887)
f534d36 
* feat: add analytics user profile - Part 1 (WPB-8978) (#2868)

* feat: add method to set/insert tracking identifier into user metadata table

* feat: add MessageContent for DataTransfer with TrackingIdentifier

* feat: add DataTransferEventHandler to handle new data transfer events and set tracking identifier to user config metadata

* chore: remove needless blank line

* feat: add analytics user profile - Part 2 (WPB-8978) (#2877)

* feat: add method to set/insert tracking identifier into user metadata table

* feat: add MessageContent for DataTransfer with TrackingIdentifier

* feat: add DataTransferEventHandler to handle new data transfer events and set tracking identifier to user config metadata

* chore: remove needless blank line

* feat: add get for current tracking identifier and get and set for previous tracking identifier

* feat: add usage and logic handling for current and previous tracking identifiers

* feat: add tests

* feat: add better handling of receiving already existing tracking id and tests

* feat: add new analytics logger to kalium logger

* feat: add usecase to observe analytics tracking identifier

* feat: add tests for observeAnalyticsTrackingIdentifierStatus

* feat: add none AnalyticsIdentifierResult to be used in AR and remove abstract attribute

* feat: add proper mapping to observer result and verification on Either Left is tracking identifier already exists

* feat: add tests for observer with new logic

* chore: add user scope logger

* chore: add `as` instantiation of right value for usecase result

* feat: move AnalyticsIdentifierResult to new data module and update its usages

* feat: add usecase to delete previous tracking identifier

* chore: add missing imports

* test: add test for DeletePreviousTrackingIdentifierUseCase

* chore: remove delete previous tracking identifier use case

* feat: add extra sealed interface for better handling of analytics identifier result

* feat: add AnalyticsIdentifierManager to handle migration complete and propagating tracking identifier

* chore: add missing extension from new sealed interface

* test: add tests for AnalyticsIdentifierManager

* chore: adjust detekt

* chore: remove unused import

* chore: rename user config current tracking identifier

* chore: add docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Garzas Garzas Garzas approved these changes

@saleniuk saleniuk saleniuk approved these changes

@yamilmedina yamilmedina Awaiting requested review from yamilmedina yamilmedina was automatically assigned from wireapp/android

@MohamadJaara MohamadJaara Awaiting requested review from MohamadJaara MohamadJaara was automatically assigned from wireapp/android

@mchenani mchenani Awaiting requested review from mchenani mchenani was automatically assigned from wireapp/android

@ohassine ohassine Awaiting requested review from ohassine ohassine was automatically assigned from wireapp/android

Assignees

@alexandreferris alexandreferris

Labels
🚨 Potential breaking changes 👕 size: L type: feature ✨
Projects
None yet
Milestone
No milestone
3 participants
@alexandreferris @Garzas @saleniuk