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

Fix: Should not take screenshot when SDK is disabled #3328

Conversation

ziyad-elabid-nw
Copy link

@ziyad-elabid-nw ziyad-elabid-nw commented Oct 8, 2023

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

This PR suggests a solution for not taking screenshot when the SDK is diabled.

💡 Motivation and Context

related to this issue : #3317

💚 How did you test it?

I wrote a unit test for the case when enabled is false

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

🔮 Next steps

@ziyad-elabid-nw ziyad-elabid-nw force-pushed the Fix-should-not-take-screenshot-when-sdk-is-disabled branch from 1e73ec6 to 3f7d9c4 Compare October 8, 2023 00:17
@ziyad-elabid-nw ziyad-elabid-nw force-pushed the Fix-should-not-take-screenshot-when-sdk-is-disabled branch from 3f7d9c4 to b1b2a4c Compare October 8, 2023 00:18
@ziyad-elabid-nw ziyad-elabid-nw changed the title Fix should not take screenshot when sdk is disabled Fix: Should not take screenshot when sdk is disabled Oct 8, 2023
@ziyad-elabid-nw ziyad-elabid-nw changed the title Fix: Should not take screenshot when sdk is disabled Fix: Should not take screenshot when SDK is disabled Oct 8, 2023
@krystofwoldrich
Copy link
Member

@ziyad-elabid-nw Thank you for opening the PR, sadly not including the integration doesn't fix the issue, as the Screenshots are taken regardless.

I've updated the integration itself here, so the unexpected behavior is removed. When the SDK is disabled integrations are added as usual but the setup method is not called.

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.

2 participants