Skip to content

RUM-6566: Implement touch override#2334

Merged
jonathanmos merged 1 commit into
developfrom
jmoskovich/rum-6566/override-touch-privacy
Oct 23, 2024
Merged

RUM-6566: Implement touch override#2334
jonathanmos merged 1 commit into
developfrom
jmoskovich/rum-6566/override-touch-privacy

Conversation

@jonathanmos

@jonathanmos jonathanmos commented Oct 20, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

Implements an api to override touch privacy on views.

Example of the api:

val view = root.findViewById...
view.setSessionReplayTouchPrivacy([TouchPrivacy])

Note:

• Touch privacy is calculated every time an ACTION_DOWN is performed. In this way we can apply the override to the entire motion.
• Views can be overridden to hide touches, but also to show touches (if the general default was hide).
• Webviews are not affected by overrides, aside from hidden and touch privacy (so they are affected by this change).
• Setting hidden on views only affects whether a view is replaced by a placeholder. Hiding the touches on the view requires a separate touch override.
• If a touch falls within two override areas, one that has been overridden to HIDE and another that has been overridden to SHOW, HIDE will take priority.

Motivation

Part of the fine grained masking effort.

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@codecov-commenter

codecov-commenter commented Oct 20, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 69.01408% with 22 lines in your changes missing coverage. Please review.

Project coverage is 70.33%. Comparing base (774c798) to head (a168657).
Report is 14 commits behind head on develop.

Files with missing lines Patch % Lines
...ssionreplay/internal/recorder/TreeViewTraversal.kt 17.39% 18 Missing and 1 partial ⚠️
...internal/recorder/DefaultOnDrawListenerProducer.kt 0.00% 2 Missing ⚠️
...roid/sessionreplay/internal/TouchPrivacyManager.kt 94.74% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2334      +/-   ##
===========================================
- Coverage    70.41%   70.33%   -0.08%     
===========================================
  Files          739      740       +1     
  Lines        27536    27591      +55     
  Branches      4614     4620       +6     
===========================================
+ Hits         19388    19405      +17     
- Misses        6866     6894      +28     
- Partials      1282     1292      +10
Files with missing lines Coverage Δ
...android/sessionreplay/PrivacyOverrideExtensions.kt 100.00% <100.00%> (ø)
...com/datadog/android/sessionreplay/SessionReplay.kt 62.07% <100.00%> (+2.81%) ⬆️
.../sessionreplay/internal/DefaultRecorderProvider.kt 93.55% <100.00%> (ø)
...oid/sessionreplay/internal/SessionReplayFeature.kt 98.73% <100.00%> (ø)
...nreplay/internal/recorder/SessionReplayRecorder.kt 97.06% <100.00%> (+0.04%) ⬆️
...nreplay/internal/recorder/ViewOnDrawInterceptor.kt 89.47% <100.00%> (+0.28%) ⬆️
...lay/internal/recorder/WindowCallbackInterceptor.kt 100.00% <100.00%> (ø)
...ternal/recorder/callback/RecorderWindowCallback.kt 91.95% <100.00%> (+0.29%) ⬆️
...nternal/recorder/listener/WindowsOnDrawListener.kt 92.50% <100.00%> (+0.39%) ⬆️
...roid/sessionreplay/internal/TouchPrivacyManager.kt 94.74% <94.74%> (ø)
... and 2 more

... and 30 files with indirect coverage changes

@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-6566/override-touch-privacy branch 4 times, most recently from 3fb35c6 to 279abf1 Compare October 21, 2024 11:31
@jonathanmos
jonathanmos changed the base branch from develop to jmoskovich/rum-6566/extract-privacy-helper October 21, 2024 11:32
@jonathanmos
jonathanmos marked this pull request as ready for review October 21, 2024 12:39
@jonathanmos
jonathanmos requested review from a team as code owners October 21, 2024 12:39
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-6566/override-touch-privacy branch from 279abf1 to 5d21381 Compare October 22, 2024 08:10
mariusc83
mariusc83 reviewed Oct 22, 2024
View reviewed changes
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/TouchPrivacyManager.kt Outdated
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/TouchPrivacyManager.kt Outdated
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/TreeViewTraversal.kt
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/TouchPrivacyManager.kt
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-6566/override-touch-privacy branch from 5d21381 to 191fdd5 Compare October 22, 2024 09:41
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-6566/override-touch-privacy branch from 191fdd5 to 27524eb Compare October 22, 2024 12:37
@jonathanmos
jonathanmos changed the base branch from jmoskovich/rum-6566/extract-privacy-helper to develop October 22, 2024 13:48
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-6566/override-touch-privacy branch from 27524eb to a168657 Compare October 22, 2024 14:01
@jonathanmos
jonathanmos merged commit 992020c into develop Oct 23, 2024
@jonathanmos
jonathanmos deleted the jmoskovich/rum-6566/override-touch-privacy branch October 23, 2024 07:42
xgouchet
xgouchet approved these changes Oct 23, 2024
View reviewed changes
Comment thread ...replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/TouchPrivacyManagerTest.kt
Comment on lines +42 to +44
fun `M add to nextOverrideAreas W addTouchOverrideArea()`(
forge: Forge
) {

@xgouchet xgouchet Oct 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

You can simply use the @Forgery annotation for enums (same applies to all methods in this test class). Doing so means that the Forge extension keeps track of the generated value when a test fails when the value is on an annotated property or function param. E.g.:

    @Forgery lateinit var fakeTouchPrivacy: TouchPrivacy
    // Test will output `field TouchPrivacyManagerTest::fakeTouchPrivacy = SHOW`

    fun `M add to nextOverrideAreas W addTouchOverrideArea()`(
        @Forgery fakePrivacyOverride: TouchPrivacy
        // Test will output `param fails for any input::arg0 = SHOW`
    ) {
        // …
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
@ambushwork ambushwork ambushwork approved these changes
+1 more reviewer
@xgouchet xgouchet xgouchet approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jonathanmos @codecov-commenter @xgouchet @mariusc83 @ambushwork