Skip to content

RUM-3634: Apply privacy settings to TextCompositionGroupMapper for compose#2121

Merged
ambushwork merged 1 commit into
feature/sr_composefrom
yl/compose-sr-privacy
Jul 12, 2024
Merged

RUM-3634: Apply privacy settings to TextCompositionGroupMapper for compose#2121
ambushwork merged 1 commit into
feature/sr_composefrom
yl/compose-sr-privacy

Conversation

@ambushwork

Copy link
Copy Markdown
Member

What does this PR do?

Apply the privacy settings to TextCompositionGroupMapper for compose, the text masking strategy keeps same as TextViewMapper in Android View

Motivation

  • RUM-3634

Screenrecord

USER_INPUT MASK
https://mobile-integration.datadoghq.com/rum/replay/sessions/e55f5ad0-145d-4c2c-bc5b-1c4bbcd35ba9?applicationId=38030dde-f9f9-4e52-9443-b9804a030080&seed=70a2b95d-ba66-4083-ad43-81134787c57b&ts=1720118965911 https://mobile-integration.datadoghq.com/rum/replay/sessions/5b0a74de-9928-4fb6-a273-e5d2a4f7ded3?applicationId=38030dde-f9f9-4e52-9443-b9804a030080&seed=49b1537e-3107-4da9-b7a6-49c24ed46145&ts=1720119348991

Additional Notes

  • StringObfuscator is changed from internal class to @InternalApi, so the class can be shared between module.

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)

@ambushwork
ambushwork requested review from a team as code owners July 4, 2024 18:57
@ambushwork
ambushwork force-pushed the yl/compose-sr-privacy branch from c381c5e to 7621d7a Compare July 4, 2024 19:05
@codecov-commenter

codecov-commenter commented Jul 4, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 44.82759% with 16 lines in your changes missing coverage. Please review.

Project coverage is 82.13%. Comparing base (56e91c0) to head (56b3fe3).

Additional details and impacted files
@@                  Coverage Diff                   @@
##           feature/sr_compose    #2121      +/-   ##
======================================================
+ Coverage               81.95%   82.13%   +0.18%     
======================================================
  Files                     512      512              
  Lines                   18291    18295       +4     
  Branches                 2813     2814       +1     
======================================================
+ Hits                    14989    15026      +37     
+ Misses                   2538     2506      -32     
+ Partials                  764      763       -1     
Files Coverage Δ
...d/sessionreplay/compose/internal/data/UiContext.kt 80.00% <100.00%> (+5.00%) ⬆️
...y/internal/recorder/obfuscator/StringObfuscator.kt 33.33% <ø> (ø)
...e/internal/mappers/ButtonCompositionGroupMapper.kt 7.69% <0.00%> (ø)
...ose/internal/mappers/TextCompositionGroupMapper.kt 52.63% <90.91%> (+48.55%) ⬆️
...compose/internal/mappers/ComposeWireframeMapper.kt 11.84% <0.00%> (-2.22%) ⬇️

... and 34 files with indirect coverage changes

@ambushwork
ambushwork requested a review from xgouchet July 5, 2024 07:40
@ambushwork
ambushwork force-pushed the yl/compose-sr-privacy branch from 7621d7a to a87ba4b Compare July 5, 2024 15:39
xgouchet
xgouchet previously approved these changes Jul 10, 2024
@ambushwork

Copy link
Copy Markdown
Member Author

@xgouchet I just rebased the branch due to some merge conflicts

}

@RepeatedTest(8)
fun `M return to correct wireframe W map`() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
fun `M return to correct wireframe W map`() {
fun `M return the correct wireframe W map`() {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

updated

SessionReplayPrivacy.MASK_USER_INPUT -> if (fakeUiContext.isInUserInputLayout) {
FIXED_INPUT_MASK
} else {
String()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

question: why String() preferred over ""?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

personal habit of giving a empty string : ) now fixed

}
}
.setPrivacy(SessionReplayPrivacy.MASK_USER_INPUT)
.setPrivacy(SessionReplayPrivacy.MASK)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

minor: what is the reason for this change?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

for the test purpose, it shouldn't be in the commit, reverted now

jonathanmos
jonathanmos previously approved these changes Jul 11, 2024
@ambushwork
ambushwork merged commit 8d4b730 into feature/sr_compose Jul 12, 2024
@ambushwork
ambushwork deleted the yl/compose-sr-privacy branch July 12, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants