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

revert(app): Privacy policy changes and opt-in strategy #14537

Merged
merged 4 commits into from
Feb 26, 2024

Conversation

b-cooper
Copy link
Contributor

@b-cooper b-cooper commented Feb 21, 2024

Overview

Revert changes to the privacy settings toggle messaging and prompts in the Desktop and On Device Display apps. Behavior should be consistent with 7.1.1 as far as opting-in/out of analytics.

Review requests

  • Confirm that there is nothing unintentional reverted within the 4 constituent commits.

Risk assessment

low

Copy link

codecov bot commented Feb 21, 2024

Codecov Report

Attention: 25 lines in your changes are missing coverage. Please review.

Comparison is base (f30f52a) 67.74% compared to head (cf86c70) 67.75%.

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                   @@
##           chore_release-7.2.0   #14537      +/-   ##
=======================================================
+ Coverage                67.74%   67.75%   +0.01%     
=======================================================
  Files                     2517     2521       +4     
  Lines                    72087    72121      +34     
  Branches                  9284     9284              
=======================================================
+ Hits                     48834    48867      +33     
  Misses                   21035    21035              
- Partials                  2218     2219       +1     
Flag Coverage Δ
app 64.69% <64.78%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
app/src/App/OnDeviceDisplayApp.tsx 76.66% <ø> (ø)
...nisms/Devices/hooks/useProtocolRunAnalyticsData.ts 64.70% <100.00%> (+1.06%) ⬆️
...ms/Devices/hooks/useTrackCreateProtocolRunEvent.ts 81.81% <ø> (-1.52%) ⬇️
...rganisms/Devices/hooks/useTrackProtocolRunEvent.ts 88.88% <ø> (-1.12%) ⬇️
...sms/OnDeviceDisplay/NameRobot/ConfirmRobotName.tsx 100.00% <ø> (ø)
app/src/pages/AppSettings/index.tsx 85.71% <ø> (ø)
app/src/pages/ConnectViaEthernet/index.tsx 68.75% <ø> (ø)
app/src/pages/ConnectViaUSB/index.tsx 88.88% <ø> (ø)
app/src/pages/ConnectViaWifi/index.tsx 84.37% <ø> (ø)
app/src/pages/Devices/RobotSettings/index.tsx 85.71% <100.00%> (+1.09%) ⬆️
... and 13 more

... and 2 files with indirect coverage changes

@b-cooper b-cooper changed the title App revert analytics changes revert(app): Privacy policy changes and opt-in strategy Feb 22, 2024
@b-cooper b-cooper marked this pull request as ready for review February 23, 2024 15:59
@b-cooper b-cooper requested a review from a team as a code owner February 23, 2024 15:59
Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

Privacy tab seems to be fine & working as expected everywhere except for here. Not sure why, i'll dig into it.

Screen Shot 2024-02-26 at 10 22 59 AM

@jerader
Copy link
Collaborator

jerader commented Feb 26, 2024

Privacy tab seems to be fine & working as expected everywhere except for here. Not sure why, i'll dig into it.

@b-cooper I did some digging, do we need to add log aggregation back? Looks like its not being returned from the getRobotSettings selector.

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

approving since we discussed on slack that the bug I commented about seems dev server specific.

@b-cooper b-cooper merged commit 9607d67 into chore_release-7.2.0 Feb 26, 2024
27 of 28 checks passed
@b-cooper b-cooper deleted the app_revert-analytics-changes branch February 26, 2024 21:56
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