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

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

Conversation

alexandreferris
Copy link
Contributor

@alexandreferris alexandreferris commented Jul 15, 2024

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 way of observing when either a new tracking identifier was received or it needed to be generated

Causes (Optional)

Not implemented.

Solutions

This is Part 2, which solves Observing if:

  • there is a receiving tracking identifier from another client.
  • there is already a tracking identifier in use for current client.
  • or if there is none, generating a new tracking identifier. (sending part will be done from AR)

Dependencies (Optional)

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

Notes (Optional)

This is Part 2

… and set tracking identifier to user config metadata
…lytics_user_profile_part2

# Conflicts:
#	logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt
#	logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/handler/DataTransferEventHandler.kt
#	logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/handler/DataTransferEventHandlerTest.kt
#	logic/src/commonTest/kotlin/com/wire/kalium/logic/util/arrangement/repository/UserConfigRepositoryArrangement.kt
#	persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt
Copy link
Contributor

github-actions bot commented Jul 15, 2024

Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit 35144c8.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jul 15, 2024

Datadog Report

Branch report: feat/add_analytics_user_profile_part2
Commit report: c93cf4d
Test service: kalium-jvm

✅ 0 Failed, 3023 Passed, 105 Skipped, 12.04s Total Time

@echoes-hq echoes-hq bot added the echoes: bugs Technical or functional defects in the product label Jul 16, 2024
@yamilmedina yamilmedina changed the title feat: add analytics user profile - part 2 (WPB-8978) feat: add analytics user profile - Part 2 (WPB-8978) Jul 18, 2024
@alexandreferris alexandreferris removed WIP work in progress DO NOT MERGE labels Jul 19, 2024
@alexandreferris alexandreferris requested review from a team, typfel, yamilmedina, borichellow, MohamadJaara and vitorhugods and removed request for a team July 19, 2024 08:54
suspend fun getTrackingIdentifier(): String?
suspend fun observeTrackingIdentifier(): Flow<Either<StorageFailure, String>>
suspend fun setPreviousTrackingIdentifier(identifier: String)
suspend fun getPreviousTrackingIdentifier(): String?
Copy link
Member

Choose a reason for hiding this comment

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

What is the previousTrackingIdentifier, how does it compare with getTrackingIdentifier?

Every time we setTrackingIdentifier(N) we setPreviousTrackingIdentifier(N-1)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we set the PreviouTrackingIdentifier when we receive a new TrackingIdentifier from remote (meaning a new device of the user was logged in somewhere else and we now need to use this new tracking identifier).

Both identifiers are kept until a proper analytics migration of the new identifier is done, then the previous identifier is deleted.

Copy link
Contributor

@yamilmedina yamilmedina left a comment

Choose a reason for hiding this comment

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

Great work as always 👏 added some suggestions about naming and docs, that can help us in the future, looking sharp 🔪

Copy link

sonarcloud bot commented Jul 22, 2024

@alexandreferris alexandreferris merged commit e55ce2d into feat/add_analytics_user_profile_epic Jul 22, 2024
20 checks passed
@alexandreferris alexandreferris deleted the feat/add_analytics_user_profile_part2 branch July 22, 2024 08:04
alexandreferris added a commit that referenced this pull request Jul 22, 2024
* 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
alexandreferris added a commit that referenced this pull request Aug 1, 2024
* 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

(cherry picked from commit f534d36)
github-merge-queue bot pushed a commit that referenced this pull request Aug 2, 2024
* feat: Add Anonymous Analytics User Profile EPIC - (WPB-8978) (#2887)

* 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

(cherry picked from commit f534d36)

* feat: Add packing and sending DataTransfer message - (WPB-8978) (#2896)

* feat: add syncManager to AnalyticsIdentifierManager in order to wait for current device to be available

* feat: add implementation of packDataTransfer message

* test: add tests

(cherry picked from commit 337c415)

* chore: Improve isSelfATeamMember UseCase (WPB-8978) (#2924)

* chore: add slow sync repository to observe for last sync completion before returning default false value for isSelfATeamMember

* chore: add parameter for SlowSyncRepository for new usage

* tests: adjust existing tests for the usecase

(cherry picked from commit 9dbf5f5)
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.

4 participants