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: enable recovery flow structure #2775

Merged
merged 13 commits into from
Nov 20, 2023
Merged

feat: enable recovery flow structure #2775

merged 13 commits into from
Nov 20, 2023

Conversation

iamacook
Copy link
Member

@iamacook iamacook commented Nov 8, 2023

What it solves

Resolves #2760

How this PR fixes it

This adds a new transaction flow for enabling recovery from a template "Recovery" section of the settings (later to be moved).

How to test it

Open the "Recovery" section of a Safe and observe a new area that allows for the enabling of recovery. Enabling will take you through a new flow according to the designs.

Screenshots

enable-recovery

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

Copy link

github-actions bot commented Nov 8, 2023

Branch preview

❌ Deploy failed!

@iamacook iamacook linked an issue Nov 8, 2023 that may be closed by this pull request
5 tasks
Copy link

github-actions bot commented Nov 8, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

Copy link

github-actions bot commented Nov 8, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
75.23% (+0.03% 🔼)
10240/13611
🔴 Branches
49.84% (-0.06% 🔻)
2084/4181
🔴 Functions
57.77% (-0.01% 🔻)
1521/2633
🟡 Lines
76.77% (+0.03% 🔼)
9265/12068
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟡
... / EnableRecoveryFlowEmailHint.tsx
60% 100% 0% 60%
🟢
... / setup.ts
100% 100% 100% 100%

Test suite run success

1138 tests passing in 160 suites.

Report generated by 🧪jest coverage report action from 7244e2e

Comment on lines +87 to +90
{
label: 'Recovery',
href: AppRoutes.settings.recovery,
},
Copy link
Member Author

Choose a reason for hiding this comment

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

I intend merge the settings areas according to the design in a separate PR.

@iamacook iamacook marked this pull request as ready for review November 13, 2023 15:55
Copy link

github-actions bot commented Nov 14, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

Copy link
Member

@johannesmoormann johannesmoormann 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! Left a few small design comments. There is also a test failing in the useLoadRecovery hook

src/components/settings/Recovery/index.tsx Show resolved Hide resolved
<Typography variant="body2" mt={2}>
For security reasons, we highly recommend adding an email address. You will be notified once a Guardian
initiates recovery and be able to reject it if it&apos;s a malicious attempt.
</Typography>
Copy link
Member

Choose a reason for hiding this comment

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

The Figma design has a button "Got it" on this hint, I assume to close it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@TanyaEfremova, should this button close the widget indefinitely or only in the current flow? We don't allow users to close hints in any other flows (I personally don't think we should include this).

src/components/tx-flow/flows/EnableRecovery/index.tsx Outdated Show resolved Hide resolved
src/components/tx-flow/flows/EnableRecovery/index.tsx Outdated Show resolved Hide resolved
src/components/tx-flow/flows/EnableRecovery/index.tsx Outdated Show resolved Hide resolved
src/services/recovery/__tests__/setup.test.ts Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Nov 20, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@iamacook iamacook merged commit 41841cc into recovery-epic Nov 20, 2023
4 of 11 checks passed
@iamacook iamacook deleted the enable-recovery branch November 20, 2023 11:12
@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Recovery] Enable module flow - email verification
3 participants