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: non-nullable privacy setting #2382

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Oct 30, 2024

This suggests a change on top of another PR (#2361) - I think it would be more user-friendly to provide the options.experimental.privacy as a non-nullable field so users can set its properties directly.

See code comments for some more details

#skip-changelog

Copy link
Contributor

github-actions bot commented Oct 30, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against a336309

Copy link

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (feat/redact-screenshots-via-view-hierarchy@4e062e8). Learn more about missing BASE report.

Additional details and impacted files
@@                              Coverage Diff                              @@
##             feat/redact-screenshots-via-view-hierarchy    #2382   +/-   ##
=============================================================================
  Coverage                                              ?   85.08%           
=============================================================================
  Files                                                 ?       81           
  Lines                                                 ?     2809           
  Branches                                              ?        0           
=============================================================================
  Hits                                                  ?     2390           
  Misses                                                ?      419           
  Partials                                              ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vaind vaind mentioned this pull request Oct 30, 2024
7 tasks
@vaind
Copy link
Collaborator Author

vaind commented Nov 5, 2024

@martinhaintz WDYT about these changes to your PR?

Copy link
Contributor

@martinhaintz martinhaintz left a comment

Choose a reason for hiding this comment

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

Looks good for me. Thanks for this PR.

@vaind vaind merged commit 21851d9 into feat/redact-screenshots-via-view-hierarchy Nov 11, 2024
48 of 50 checks passed
@vaind vaind deleted the feat/privacy-setting-automatic branch November 11, 2024 12:25
martinhaintz added a commit that referenced this pull request Nov 24, 2024
* use view hierachy for screenshots

* fix tests and distinguish between ScheduledScreenshotRecorderConfig and ScreenshotRecorderConfig.

* removed unused imports

* remove unused test

* add changelog

* rename variable

* add internal

* split into screenshot, screenreplay and redaction options

* fix comments

* export redaction options and remove unused dependencies

* renaming to SentryPrivacyOptions

* add explanation for setRedactionOptions

* fix deprecation warnings

* Update flutter/lib/src/sentry_flutter.dart

Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>

* Update flutter/lib/src/sentry_flutter.dart

Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>

* Update flutter/lib/src/sentry_privacy_options.dart

Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>

* feat: non-nullable privacy setting (#2382)

* refactor: make privacy setting non-nullable and automatically set screenshot redaction when user accesses it.

* Apply suggestions from code review

* formatting

* keep recorder instance in memory

* Update flutter/lib/src/sentry_screenshot_options.dart

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* update comment for sentry screenshot options

* fixed screenshot size mismatch

* renamed some variables to make it more readable

* fix unit tests

* reorganized tests

* update changelog

* screenshots now also working with canvas kit

* Update CHANGELOG.md

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* Update flutter/lib/src/sentry_flutter_options.dart

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* Update flutter/lib/src/sentry_screenshot_options.dart

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* renamed screenshot settings

* removed quality from recorder config.

* Update CHANGELOG.md

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* Update CHANGELOG.md

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* Update flutter/lib/src/event_processor/screenshot_event_processor.dart

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* add test for screenshots with flutter html renderer

* Update flutter/lib/src/sentry_screenshot_options.dart

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* add changelog for supported html renderer

* removed unused import

* revert: screenshot recorder resolution handling (#2409)

* add high-risk-code file filters

* revert: screenshot recorder resolution handling

* analyzer issues

* chore: update example

* improve screenshot recorder logs

* fixup variable visibility

* dart format

---------

Co-authored-by: Martin Haintz <martin.haintz@gmail.com>

* fix pixelRatio

* Update flutter/test/replay/scheduled_recorder_test.dart

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* fix unit tests

* Update flutter/lib/src/screenshot/recorder.dart

Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>

* reverted changes for html renderer as it is not working

* fix unit tests and errors

* add comment for quality

* moved code back to previous position

* rearrange imports

* move screenshot options into global options

* change wording from redaction to masking and similar.

* add import for screenshot event processor

---------

Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>
Co-authored-by: Ivan Dlugos <6349682+vaind@users.noreply.github.com>
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.

2 participants