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

[MBL-1063] Use transcend privacy flow for user data requests #1911

Merged
merged 9 commits into from Jan 19, 2024

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Jan 11, 2024

📲 What

Update "request data" and "delete account" buttons to open the new transcend privacy flow in Safari. This allows us to delete the old request flow. (The strings cannot be deleted if Android is still using them.)

Note: I moved screenshots in two different commits; one to change the images and a separate one to change the test names. This mostly led them to be correctly marked as moved, but not entirely.

👀 See

JIRA

Before 🐛 After 🦋
Screenshot 2024-01-16 at 9 48 20 AM image
Screenshot 2024-01-16 at 9 52 32 AM image

✅ Acceptance criteria

⏰ TODO (probably in separate PR)

  • Exact URL should depend on app language (planning to add this url to our string translations & update our code to use that once it's ready)

@ifosli ifosli added the draft label Jan 11, 2024
@ifosli ifosli added this to the release-5.12.0 milestone Jan 11, 2024
@ifosli ifosli self-assigned this Jan 11, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2024

Codecov Report

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

Comparison is base (70bd65c) 83.74% compared to head (b456a64) 83.74%.

Files Patch % Lines
...ews/Cells/SettingsPrivacyDeleteOrRequestCell.swift 92.30% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #1911    +/-   ##
========================================
  Coverage   83.74%   83.74%            
========================================
  Files        1229     1223     -6     
  Lines      112188   111811   -377     
  Branches    29869    29750   -119     
========================================
- Hits        93955    93640   -315     
+ Misses      17204    17149    -55     
+ Partials     1029     1022     -7     

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

@nativeksr
Copy link
Collaborator

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

@ifosli ifosli added needs review and removed draft labels Jan 16, 2024
@ifosli ifosli removed this from the release-5.12.0 milestone Jan 16, 2024
Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Testing looks good. Thanks for making a ticket to address the hardcoded URL

@@ -48,10 +61,8 @@ internal final class SettingsPrivacyDeleteOrRequestCell: UITableViewCell, ValueC
||> settingsSeparatorStyle

_ = self.deleteAccountLabel
|> UILabel.lens.textColor .~ .ksr_alert
|> UILabel.lens.font .~ .ksr_body()
|> UILabel.lens.numberOfLines .~ 2
Copy link
Contributor

Choose a reason for hiding this comment

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

should we move these to configureWith(... to keep all of these deleteAccountLabel styles together?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather keep them here. I think it's often useful to treat styles that depend on input separately from styles that don't.

@ifosli ifosli merged commit 66e61bd into main Jan 19, 2024
5 checks passed
@ifosli ifosli deleted the transcendPrivacyFlow branch January 19, 2024 22:30
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.

None yet

4 participants